Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,28 @@ import InterlinedKit
/// encodes it for us). The user-side `notificationTrayLimit` (10–40, default
/// 20) controls the page size.
///
/// **Tray limit — corrected 2026-09-09 (G35 / issue #43).** The previous note
/// here claimed the server ignores a client `limit` outright. A live probe
/// disproves that: unscoped, `?limit=5` returns 5 rows and `?limit=40` returns
/// 36, and omitting it returns exactly the account's stored limit. Only the
/// `scope=tray` form ignores it. So `tray(limit:)` both sends the value *and*
/// caps the decoded rows client-side — the send is what will start working the
/// moment the tray route honours it, and the cap is what makes the preference
/// observable today.
/// **Scope — corrected 2026-09-15 (GitHub #80).** The tray reads `scope=all`,
/// not `scope=tray`.
///
/// `scope=tray` returns **unread rows only** and ignores `limit`, so the macOS
/// tray emptied as the user read it while the web bell retained recent history.
/// A glanced-at notification simply vanished, with no way back to it. `scope=all`
/// is what the web bell uses — the last N regardless of read state — and it
/// honours `limit`, falling back to the account's stored `notificationTrayLimit`
/// when the parameter is absent. Probed live:
///
/// ```
/// ?scope=tray&limit=5 → 1 item (the single unread one)
/// ?scope=all&limit=3 → 3 items
/// ?scope=all&limit=10 → 10 items
/// ?scope=all → 20 items (the stored preference)
/// ```
///
/// `unreadCount` is returned under both scopes, so the badge is unaffected.
///
/// The client-side `prefix` that used to cap the rows is gone with it: it existed
/// only because `scope=tray` ignored the parameter, and it made the preference
/// *appear* to work while the rows being trimmed were the wrong rows.
///
/// Follows the same DI shape as the other domain services — takes its
/// `APIClientProtocol` as a parameter so unit tests run against a stub.
Expand All @@ -28,10 +42,10 @@ public protocol NotificationsServicing: Sendable {
/// `unreadCount` + the items page).
///
/// - Parameter limit: the account's `notificationTrayLimit`. `nil` (the
/// default, via the `tray()` convenience) leaves the page size entirely
/// to the server. `unreadCount` is never capped — it is the server's own
/// total and stays authoritative for the badge even when the rendered
/// rows are trimmed.
/// default, via the `tray()` convenience) lets the server apply the stored
/// preference itself. `unreadCount` is always the server's own total and
/// stays authoritative for the badge regardless of how many rows come
/// back — a limit that shrinks the list must not shrink the badge.
func tray(limit: Int?) async throws -> NotificationTray

/// Marks a single notification read by id. The service does not return
Expand Down Expand Up @@ -69,17 +83,17 @@ public final class NotificationsService: NotificationsServicing {
}

public func tray(limit: Int?) async throws -> NotificationTray {
// `scope=all` honours `limit` server-side, so the client-side `prefix`
// that used to sit here is gone (GitHub #80). That trim existed because
// `scope=tray` ignored the parameter — it made `notificationTrayLimit`
// *appear* to work while the rows being trimmed were unread-only, so the
// preference could only ever shrink an already-wrong list.
//
// Deliberately not re-added as a belt-and-braces cap: a client-side trim
// over a correctly-limited response is invisible when it agrees with the
// server and wrong when it does not.
let dto = try await api.send(Notifications.tray(limit: limit))
let tray = NotificationTray(from: dto)
// Client-side cap: `scope=tray` ignores the query parameter today, so
// without this the user's limit would have no visible effect. The
// server-authoritative `unreadCount` is preserved as-is — trimming the
// rendered rows must not understate the badge.
guard let limit, limit > 0, tray.items.count > limit else { return tray }
return NotificationTray(
unreadCount: tray.unreadCount,
items: Array(tray.items.prefix(limit))
)
return NotificationTray(from: dto)
}

public func markRead(id: String) async throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,12 @@ final class NotificationsServiceTests: XCTestCase {
// The second row is marked read (readAt present).
XCTAssertTrue(tray.items.last?.isRead ?? false)

// And — the request shape carries `scope=tray`.
// And — the request asks for `scope=all`, the read-inclusive scope the
// web bell uses. `scope=tray` is unread-only, which made the macOS tray
// empty itself as the user read it (GitHub #80).
let recorded = await api.recorded
XCTAssertEqual(recorded.first?.path, "/api/notifications")
XCTAssertEqual(recorded.first?.query["scope"], "tray")
XCTAssertEqual(recorded.first?.query["scope"], "all")
}

func test_givenMalformedTray_whenLoadingTray_thenThrowsDecoding() async throws {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,30 +61,67 @@ final class ViewPreferencesWiringTests: XCTestCase {

// MARK: - Notification tray limit

func test_givenTrayLimit_whenLoadingTray_thenSendsItAndTrimsTheRenderedRows() async throws {
// Given — the server returns more rows than the account's limit. Under
// `scope=tray` it ignores the query parameter (probed 2026-09-09), so
// the client-side cap is what makes the preference observable.
func test_givenTrayLimit_whenLoadingTray_thenSendsItUnderTheReadInclusiveScope() async throws {
// Given — the account's `notificationTrayLimit`. Under `scope=all` the
// server honours it, so the limit travels and the rows come back already
// sized (probed 2026-09-15: `?scope=all&limit=3` → 3, `&limit=10` → 10).
let api = StubAPIClient()
let items = (1...5).map { Fixtures.notificationObject(id: "n-\($0)") }
let items = (1...2).map { Fixtures.notificationObject(id: "n-\($0)") }
await api.enqueue(json: Fixtures.notificationTrayEnvelope(unreadCount: 4, items: items))
let service = NotificationsService(api: api)

// When
let tray = try await service.tray(limit: 2)

// Then — trimmed rows, untouched unread count, and the limit on the wire.
// Then — the server's page is rendered as-is.
XCTAssertEqual(tray.items.map(\.id), ["n-1", "n-2"])
XCTAssertEqual(
tray.unreadCount,
4,
"The server-authoritative unread badge must not be understated by trimming rows"
"The server-authoritative unread badge is independent of how many rows are shown"
)
let recorded = await api.recorded
XCTAssertEqual(recorded.first?.query["scope"], "tray")
XCTAssertEqual(recorded.first?.query["scope"], "all")
XCTAssertEqual(recorded.first?.query["limit"], "2")
}

func test_givenTheServerOverAnswers_whenLoadingTray_thenRowsAreNotTrimmedClientSide() async throws {
// Upstream-behaviour case, and a deliberate choice worth pinning.
//
// The old implementation capped the decoded rows with `prefix(limit)`.
// That existed only because `scope=tray` ignored the parameter, and it
// made the preference *appear* to work while the rows being trimmed were
// unread-only — so it could only ever shrink an already-wrong list.
//
// With `scope=all` the server does the limiting. A belt-and-braces trim
// on top would be invisible when it agrees with the server and wrong
// when it does not, so the client renders what it is given.
let api = StubAPIClient()
let items = (1...5).map { Fixtures.notificationObject(id: "n-\($0)") }
await api.enqueue(json: Fixtures.notificationTrayEnvelope(unreadCount: 4, items: items))
let service = NotificationsService(api: api)

let tray = try await service.tray(limit: 2)

XCTAssertEqual(tray.items.count, 5, "the server's answer is rendered, not second-guessed")
}

func test_givenAllRowsRead_whenLoadingTray_thenTheTrayStillShowsThem() async throws {
// The behaviour this whole change is about. On `scope=tray` a fully-read
// account got an empty tray — a notification you glanced at vanished,
// with no way back to it. The web bell keeps the last N.
let api = StubAPIClient()
let items = (1...3).map { Fixtures.notificationObject(id: "n-\($0)", readAt: "2026-09-14T04:00:00.000Z") }
await api.enqueue(json: Fixtures.notificationTrayEnvelope(unreadCount: 0, items: items))
let service = NotificationsService(api: api)

let tray = try await service.tray(limit: 20)

XCTAssertEqual(tray.items.count, 3, "read notifications stay in the tray")
XCTAssertTrue(tray.items.allSatisfy(\.isRead))
XCTAssertEqual(tray.unreadCount, 0, "and the badge correctly shows nothing unread")
}

func test_givenNoTrayLimit_whenLoadingTray_thenOmitsTheParameterAndKeepsEveryRow() async throws {
// Boundary: `nil` leaves the page size entirely to the server, which
// applies the account's own `notificationTrayLimit`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,31 @@ import Foundation
/// `GET /api/notifications` does **not** use the standard
/// `{ data, pagination }` envelope; it returns
/// `{ unreadCount, items: [...] }`, so it decodes into `NotificationTrayDTO`
/// rather than `Paginated<T>`. The `scope=tray` query parameter is required by
/// the API and defaulted here.
/// rather than `Paginated<T>`.
public enum Notifications {

/// `GET /api/notifications` (with `scope=tray`).
/// `GET /api/notifications` — the tray, read **and** unread.
///
/// `limit` is optional and omitted by default. VERIFIED live 2026-09-09
/// (G35 / issue #43): the route honours `limit` in its *unscoped* form
/// (`?limit=5` → 5 rows, `?limit=40` → 36 rows on the test account) and
/// falls back to the account's `notificationTrayLimit` when absent
/// (no params → exactly 20 rows, matching the stored preference). Under
/// `scope=tray` the server currently ignores `limit` and returns unread
/// rows only, so the preference is *also* enforced client-side by
/// `NotificationsService.tray(limit:)` — see the note there.
public static func tray(scope: String = "tray", limit: Int? = nil) -> Request<NotificationTrayDTO> {
/// The scope defaults to **`all`**, not `tray`. VERIFIED live 2026-09-15
/// (GitHub #80) on the test account:
///
/// ```
/// ?scope=tray → 1 item (the single unread one), `limit` ignored
/// ?scope=tray&limit=5 → 1 item (still just the unread one)
/// ?scope=all&limit=3 → 3 items (read and unread, newest first)
/// ?scope=all&limit=10 → 10 items
/// ?scope=all → 20 items (the account's `notificationTrayLimit`)
/// ```
///
/// `scope=tray` is **unread-only** and ignores `limit`, so on macOS the tray
/// emptied as you read it while the web bell retained recent history. Same
/// feature, different behaviour — and the macOS one was the surprising one.
///
/// `scope=all` is what the web bell uses: the last N regardless of read
/// state, honouring `limit`, and falling back to the account preference when
/// it is absent. `unreadCount` is returned under both scopes, so the badge
/// still comes from the same call.
public static func tray(scope: String = "all", limit: Int? = nil) -> Request<NotificationTrayDTO> {
Request(
method: .get,
path: "/api/notifications",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,9 @@ final class NotificationsEndpointTests: XCTestCase {
XCTAssertEqual(Notifications.tray().path, "/api/notifications")
XCTAssertEqual(Notifications.tray().method, .get)
XCTAssertEqual(Notifications.tray().auth, .bearer)
XCTAssertEqual(Notifications.tray().query.first(where: { $0.name == "scope" })?.value, "tray")
// `all`, not `tray`: `scope=tray` is unread-only and ignores `limit`,
// so the macOS tray emptied as the user read it (GitHub #80).
XCTAssertEqual(Notifications.tray().query.first(where: { $0.name == "scope" })?.value, "all")

XCTAssertEqual(Notifications.markRead(id: "n1").method, .patch)
XCTAssertEqual(Notifications.markRead(id: "n1").path, "/api/notifications/n1/read")
Expand All @@ -40,7 +42,7 @@ final class NotificationsEndpointTests: XCTestCase {
// Happy path: the account's `notificationTrayLimit` reaches the wire.
let request = Notifications.tray(limit: 25)

XCTAssertEqual(request.query.first(where: { $0.name == "scope" })?.value, "tray")
XCTAssertEqual(request.query.first(where: { $0.name == "scope" })?.value, "all")
XCTAssertEqual(request.query.first(where: { $0.name == "limit" })?.value, "25")
}

Expand Down Expand Up @@ -73,7 +75,7 @@ final class NotificationsEndpointTests: XCTestCase {

let received = await transport.received
let comps = URLComponents(url: try XCTUnwrap(received[0].url), resolvingAgainstBaseURL: false)
XCTAssertTrue(comps?.queryItems?.contains(URLQueryItem(name: "scope", value: "tray")) ?? false)
XCTAssertTrue(comps?.queryItems?.contains(URLQueryItem(name: "scope", value: "all")) ?? false)
}

func test_givenOkBody_whenMarkReadSent_thenDecodesOk() async throws {
Expand Down Expand Up @@ -137,3 +139,43 @@ final class NotificationsEndpointTests: XCTestCase {
}
}
}

// MARK: - Scope (GitHub #80)

extension NotificationsEndpointTests {

/// The read-inclusive scope is the default, and the unread-only one is still
/// reachable for anything that genuinely wants it.
///
/// Probed live 2026-09-15 on the test account:
///
/// ?scope=tray&limit=5 -> 1 item (the single unread one)
/// ?scope=all&limit=3 -> 3 items
/// ?scope=all&limit=10 -> 10 items
/// ?scope=all -> 20 items (the account's notificationTrayLimit)
func test_givenTheDefaultScope_whenBuildingTheTrayRequest_thenItIsReadInclusive() {
XCTAssertEqual(
Notifications.tray().query.first(where: { $0.name == "scope" })?.value,
"all"
)
XCTAssertEqual(
Notifications.tray(scope: "tray").query.first(where: { $0.name == "scope" })?.value,
"tray",
"the unread-only scope stays available; it is just not the tray's default"
)
}

func test_givenALimit_whenBuildingTheTrayRequest_thenItTravelsAlongsideTheScope() {
// Boundary: the limit is only meaningful under `scope=all` — `scope=tray`
// ignores it server-side — so the two have to go out together.
let request = Notifications.tray(limit: 40)
XCTAssertEqual(request.query.first(where: { $0.name == "scope" })?.value, "all")
XCTAssertEqual(request.query.first(where: { $0.name == "limit" })?.value, "40")
}

func test_givenNoLimit_whenBuildingTheTrayRequest_thenTheParameterIsOmitted() {
// Omitting it is not the same as sending zero: an absent `limit` makes
// the server apply the account's own stored preference.
XCTAssertNil(Notifications.tray().query.first(where: { $0.name == "limit" })?.value)
}
}