diff --git a/package.json b/package.json index 42db5ffd..6a231c9a 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "impro", - "version": "0.18.125", + "version": "0.18.126", "type": "module", "scripts": { "start": "rm -rf \"${BUILD_DIR:-build}\" && NODE_ENV=development eleventy --serve", diff --git a/src/js/dataLayer/dataStore.js b/src/js/dataLayer/dataStore.js index 844a9cec..b443f83b 100644 --- a/src/js/dataLayer/dataStore.js +++ b/src/js/dataLayer/dataStore.js @@ -13,6 +13,7 @@ export class DataStore extends ReactiveStore { this.$feedSearchResults = new Signal.State(null); this.$notifications = new Signal.State(null); this.$mentionNotifications = new Signal.State(null); + this.$notificationsLastSeenAt = new Signal.State(null); this.$pinnedItems = new Signal.State(null); this.$bookmarks = new Signal.State(null); this.$drafts = new Signal.State(null); diff --git a/src/js/dataLayer/derived.js b/src/js/dataLayer/derived.js index 507e0f92..db373cee 100644 --- a/src/js/dataLayer/derived.js +++ b/src/js/dataLayer/derived.js @@ -790,6 +790,15 @@ export class Derived extends ReactiveStore { ...rawNotification, author: this.$hydratedProfiles.get(rawNotification.author.did), }; + // The server marks every notification as read the moment updateSeen + // fires, so pages fetched after that arrive with isRead: true even when + // the user hasn't seen them. Recompute read state against the seenAt + // captured from the first page's response instead. + const seenAt = this.dataStore.$notificationsLastSeenAt.get(); + if (seenAt) { + notification.isRead = + new Date(notification.indexedAt) <= new Date(seenAt); + } if (notification.reason === "like" || notification.reason === "repost") { const subject = this.$hydratedPosts.get(notification.reasonSubject) ?? diff --git a/src/js/dataLayer/requests.js b/src/js/dataLayer/requests.js index c3d0e6ab..a0bb2c36 100644 --- a/src/js/dataLayer/requests.js +++ b/src/js/dataLayer/requests.js @@ -835,6 +835,9 @@ export class Requests { : readCollectionCursor(this.dataStore.$notifications); const labelers = this.requireLabelers(); const res = await this.api.getNotifications({ cursor, limit, labelers }); + if (cursor === "") { + this.dataStore.$notificationsLastSeenAt.set(res.seenAt ?? null); + } this.dataStore.setProfiles( res.notifications.map((notification) => notification.author), ); diff --git a/tests/e2e/mockServer.js b/tests/e2e/mockServer.js index 16cf4669..892ef8eb 100644 --- a/tests/e2e/mockServer.js +++ b/tests/e2e/mockServer.js @@ -46,6 +46,7 @@ export class MockServer { this.notifications = []; this.notificationCursor = undefined; this.notificationsDelayMs = 0; + this.notificationsSeenAt = null; this.pinnedFeedUris = []; this.pinnedListUris = []; this.lists = []; @@ -195,6 +196,10 @@ export class MockServer { this.notificationsDelayMs = delayMs; } + setNotificationsSeenAt(seenAt) { + this.notificationsSeenAt = seenAt; + } + addSearchPosts(posts, { sort } = {}) { this.searchPosts.push(...posts); if (sort) { @@ -853,12 +858,19 @@ export class MockServer { return route.fulfill({ status: 200, contentType: "application/json", - body: JSON.stringify({ notifications, cursor: nextCursor }), + body: JSON.stringify({ + notifications, + cursor: nextCursor, + ...(this.notificationsSeenAt + ? { seenAt: this.notificationsSeenAt } + : {}), + }), }); }, ); await page.route("**/xrpc/app.bsky.notification.updateSeen*", (route) => { + this.notificationsSeenAt = route.request().postDataJSON().seenAt; for (const notification of this.notifications) { notification.isRead = true; } diff --git a/tests/e2e/specs/views/notifications.view.test.js b/tests/e2e/specs/views/notifications.view.test.js index 5a11a30f..6828d676 100644 --- a/tests/e2e/specs/views/notifications.view.test.js +++ b/tests/e2e/specs/views/notifications.view.test.js @@ -954,6 +954,61 @@ test.describe("Notifications view", () => { await expect(view).toContainText("Liker 60"); }); + test("should keep page 2 notifications unread after page 1 marks all seen", async ({ + page, + }) => { + // Use like notifications with unique reasonSubjects so they are not grouped + const likedPosts = []; + const notifications = []; + for (let i = 1; i <= 60; i++) { + const post = createPost({ + uri: `at://did:plc:testuser123/app.bsky.feed.post/notifpost${i}`, + text: `Notification post ${i}`, + authorHandle: "testuser.bsky.social", + authorDisplayName: "Test User", + }); + likedPosts.push(post); + + const author = createProfile({ + did: `did:plc:liker${i}`, + handle: `liker${i}.bsky.social`, + displayName: `Liker ${i}`, + }); + notifications.push( + createNotification({ + reason: "like", + author, + reasonSubject: post.uri, + isRead: false, + indexedAt: new Date(Date.now() - i * 60000).toISOString(), + }), + ); + } + + const mockServer = new MockServer(); + mockServer.addPosts(likedPosts); + mockServer.addNotifications(notifications); + // All notifications were indexed after the last seen time, so every page + // is unread; loading page 1 fires updateSeen, which advances the server's + // seen state before page 2 is fetched. + mockServer.setNotificationsSeenAt( + new Date(Date.now() - 61 * 60000).toISOString(), + ); + await mockServer.setup(page); + + await login(page); + await page.goto("/notifications"); + + const view = page.locator("#notifications-view"); + const items = view.locator(".notification-item"); + + await expect(items.first()).toBeVisible({ timeout: 10000 }); + await items.last().scrollIntoViewIfNeeded(); + await expect(items).toHaveCount(60, { timeout: 10000 }); + + await expect(view.locator(".notification-item.unread")).toHaveCount(60); + }); + test("should display error state when notifications fail to load", async ({ page, }) => { diff --git a/tests/unit/specs/dataLayer/derived.test.js b/tests/unit/specs/dataLayer/derived.test.js index daef6307..fba51603 100644 --- a/tests/unit/specs/dataLayer/derived.test.js +++ b/tests/unit/specs/dataLayer/derived.test.js @@ -1577,6 +1577,74 @@ describe("$notifications", () => { assert.deepEqual(result[0].author.badgeLabels, ["verified"]); assert.deepEqual(result[0].author.viewer.following, "fake following"); }); + + it("should override isRead using the captured seenAt", () => { + const dataStore = new DataStore(); + const { derived } = makeDerived(dataStore); + seedNotifications(dataStore, [ + createNotification({ + reason: "follow", + author, + uri: "old", + isRead: false, + indexedAt: "2025-01-15T09:00:00.000Z", + }), + createNotification({ + reason: "follow", + author, + uri: "new", + isRead: true, + indexedAt: "2025-01-15T11:00:00.000Z", + }), + ]); + dataStore.$notificationsLastSeenAt.set("2025-01-15T10:00:00.000Z"); + const result = derived.$notifications.get(); + assert.deepEqual(result[0].isRead, true); + assert.deepEqual(result[1].isRead, false); + }); + + it("should treat a notification indexed exactly at seenAt as read", () => { + const dataStore = new DataStore(); + const { derived } = makeDerived(dataStore); + seedNotifications(dataStore, [ + createNotification({ + reason: "follow", + author, + isRead: false, + indexedAt: "2025-01-15T10:00:00.000Z", + }), + ]); + dataStore.$notificationsLastSeenAt.set("2025-01-15T10:00:00.000Z"); + assert.deepEqual(derived.$notifications.get()[0].isRead, true); + }); + + it("should trust the server isRead when seenAt is null", () => { + const dataStore = new DataStore(); + const { derived } = makeDerived(dataStore); + seedNotifications(dataStore, [ + createNotification({ reason: "follow", author, uri: "r", isRead: true }), + createNotification({ reason: "follow", author, uri: "u", isRead: false }), + ]); + const result = derived.$notifications.get(); + assert.deepEqual(result[0].isRead, true); + assert.deepEqual(result[1].isRead, false); + }); + + it("should recompute isRead when seenAt is captured later", () => { + const dataStore = new DataStore(); + const { derived } = makeDerived(dataStore); + seedNotifications(dataStore, [ + createNotification({ + reason: "follow", + author, + isRead: true, + indexedAt: "2025-01-15T11:00:00.000Z", + }), + ]); + assert.deepEqual(derived.$notifications.get()[0].isRead, true); + dataStore.$notificationsLastSeenAt.set("2025-01-15T10:00:00.000Z"); + assert.deepEqual(derived.$notifications.get()[0].isRead, false); + }); }); describe("$mentionNotifications", () => { @@ -1607,6 +1675,26 @@ describe("$mentionNotifications", () => { assert.deepEqual(result[0].post.uri, "m1"); assert.deepEqual(derived.$mentionNotificationCursor.get(), "mc"); }); + + it("should override isRead using the shared seenAt from the main feed", () => { + const dataStore = new DataStore(); + const { derived } = makeDerived(dataStore); + const author = createProfile({ did: "did:plc:a", handle: "a.test" }); + dataStore.setProfiles([author]); + dataStore.$mentionNotifications.set({ + notifications: [ + createNotification({ + reason: "follow", + author, + isRead: true, + indexedAt: "2025-01-15T11:00:00.000Z", + }), + ], + cursor: null, + }); + dataStore.$notificationsLastSeenAt.set("2025-01-15T10:00:00.000Z"); + assert.deepEqual(derived.$mentionNotifications.get()[0].isRead, false); + }); }); describe("$hydratedPostThreads", () => { diff --git a/tests/unit/specs/dataLayer/requests.test.js b/tests/unit/specs/dataLayer/requests.test.js index d1777963..6e93a35c 100644 --- a/tests/unit/specs/dataLayer/requests.test.js +++ b/tests/unit/specs/dataLayer/requests.test.js @@ -1434,6 +1434,103 @@ describe("loadNotifications", () => { assert.deepEqual(stored.cursor, "fresh"); }); + it("should capture seenAt on first load", async () => { + const dataStore = new DataStore(); + const mockApi = { + getNotifications: async () => ({ + notifications: [ + { reason: "like", uri: "n1", author: { did: "did:plc:liker" } }, + ], + cursor: "next", + seenAt: "2025-01-15T10:00:00.000Z", + }), + getPosts: async () => [], + }; + const requests = makeRequests(mockApi, dataStore); + + await requests.loadNotifications(); + + assert.deepEqual( + dataStore.$notificationsLastSeenAt.get(), + "2025-01-15T10:00:00.000Z", + ); + }); + + it("should overwrite the captured seenAt on reload", async () => { + const dataStore = new DataStore(); + dataStore.$notifications.set({ + notifications: [{ reason: "like", uri: "n1" }], + cursor: "page2", + }); + dataStore.$notificationsLastSeenAt.set("2025-01-14T10:00:00.000Z"); + + const mockApi = { + getNotifications: async () => ({ + notifications: [ + { reason: "follow", uri: "n2", author: { did: "did:plc:f" } }, + ], + cursor: "fresh", + seenAt: "2025-01-15T10:00:00.000Z", + }), + getPosts: async () => [], + }; + const requests = makeRequests(mockApi, dataStore); + + await requests.loadNotifications({ reload: true }); + + assert.deepEqual( + dataStore.$notificationsLastSeenAt.get(), + "2025-01-15T10:00:00.000Z", + ); + }); + + it("should not capture seenAt on subsequent pages", async () => { + const dataStore = new DataStore(); + dataStore.$notifications.set({ + notifications: [{ reason: "like", uri: "n1" }], + cursor: "page2", + }); + dataStore.$notificationsLastSeenAt.set("2025-01-14T10:00:00.000Z"); + + const mockApi = { + getNotifications: async () => ({ + notifications: [ + { reason: "follow", uri: "n2", author: { did: "did:plc:f" } }, + ], + cursor: "page3", + seenAt: "2025-01-15T10:00:00.000Z", + }), + getPosts: async () => [], + }; + const requests = makeRequests(mockApi, dataStore); + + await requests.loadNotifications(); + + assert.deepEqual( + dataStore.$notificationsLastSeenAt.get(), + "2025-01-14T10:00:00.000Z", + ); + }); + + it("should set seenAt to null when the response omits it", async () => { + const dataStore = new DataStore(); + dataStore.$notificationsLastSeenAt.set("2025-01-14T10:00:00.000Z"); + const mockApi = { + getNotifications: async () => ({ + notifications: [ + { reason: "like", uri: "n1", author: { did: "did:plc:liker" } }, + ], + cursor: "next", + }), + getPosts: async () => [], + }; + const requests = makeRequests(mockApi, dataStore); + + await requests.loadNotifications(); + + assert.deepEqual(dataStore.$notificationsLastSeenAt.get(), null); + }); + it("should discard a stale response when a reload lands mid-flight", async () => { const dataStore = new DataStore(); dataStore.$notifications.set({ @@ -1526,6 +1623,25 @@ describe("loadMentionNotifications", () => { assert.deepEqual(dataStore.$mentionNotifications.get().cursor, "next"); }); + it("should not capture seenAt", async () => { + const dataStore = new DataStore(); + const mockApi = { + getNotifications: async () => ({ + notifications: [ + { reason: "mention", uri: "n1", author: { did: "did:plc:m" } }, + ], + cursor: "next", + seenAt: "2025-01-15T10:00:00.000Z", + }), + getPosts: async () => [], + }; + const requests = makeRequests(mockApi, dataStore); + + await requests.loadMentionNotifications(); + + assert.deepEqual(dataStore.$notificationsLastSeenAt.get(), null); + }); + it("should append when cursor matches previous", async () => { const dataStore = new DataStore(); dataStore.$mentionNotifications.set({