diff --git a/web/app/components/NotificationBox.nuxt.test.ts b/web/app/components/NotificationBox.nuxt.test.ts index 95eb9e9d..8e92c40d 100644 --- a/web/app/components/NotificationBox.nuxt.test.ts +++ b/web/app/components/NotificationBox.nuxt.test.ts @@ -12,12 +12,14 @@ import { seedQueryData, } from "~/composables/api/test-helpers" import NotificationBox from "./NotificationBox.vue" +import NotificationRow from "./NotificationRow.vue" import { at, findButtonByText, mockAuthOrganization, renderedIconNames, seedAuthOrganization, + settleMutations, t, } from "./test-helpers" @@ -82,6 +84,7 @@ describe("", { concurrent: false }, () => { afterEach(() => { disposeMockEndpoints() + useWebSocketStateStore().state = null vi.useRealTimers() }) @@ -93,6 +96,7 @@ describe("", { concurrent: false }, () => { const wrapper = await mountBox() expect(wrapper.text()).toContain(t("notification.empty-title")) + expect(wrapper.text()).toContain(t("notification.empty-description")) }) it("lists one row per notification", async ({ expect }) => { @@ -215,6 +219,297 @@ describe("", { concurrent: false }, () => { expect(wrapper.emitted("close-notification-box")).toHaveLength(1) }) + // filter changes share the mounted query and the endpoint registry. + describe("filtering", { concurrent: false }, () => { + it("selects all notifications by default", async ({ expect }) => { + seedNotifications([ + makeNotification(), + makeNotification({ id: "read", read: true }), + ]) + + const wrapper = await mountBox() + + expect(wrapper.get("[role='group']").attributes("aria-label")).toBe( + t("notification.filters.label"), + ) + expect(wrapper.get("button[aria-pressed='true']").text()).toBe( + t("notification.filters.all"), + ) + expect(rows(wrapper)).toHaveLength(2) + }) + + it.for([ + { filter: "unread", read: false }, + { filter: "read", read: true }, + ])( + "fetches $filter notifications beyond the first unfiltered page", + async ({ filter, read }, { expect }) => { + disposeMockEndpoints() + const notifications = Array.from({ length: 52 }, (_unused, index) => + makeNotification({ + id: `n${index}`, + read: index < 50 ? !read : read, + }), + ) + const listCalls = mockEndpoint("GET", "/api/notifications", (call) => { + const filtered = + call.query["filter-read_eq"] === undefined + ? notifications + : notifications.filter( + (notification) => + String(notification.read) === call.query["filter-read_eq"], + ) + + return { + notifications: filtered.slice(0, 50), + pageCount: Math.ceil(filtered.length / 50), + } + }) + const countCalls = mockEndpoint( + "GET", + "/api/notifications/count", + () => ({ + count: notifications.filter((notification) => !notification.read) + .length, + }), + ) + const wrapper = await mountBox() + await settleMutations() + expect(rows(wrapper)).toHaveLength(50) + + await findButtonByText( + wrapper, + t(`notification.filters.${filter}`), + ).trigger("click") + await settleMutations() + + expect(notificationIds(wrapper)).toEqual(["n50", "n51"]) + expect(wrapper.get("button[aria-pressed='true']").text()).toBe( + t(`notification.filters.${filter}`), + ) + expect(infiniteScroll.canLoadMore()).toBe(false) + expect(listCalls.map((call) => call.query)).toEqual([ + { limit: "50", page: "1" }, + { limit: "50", page: "1", "filter-read_eq": String(read) }, + ]) + expect(countCalls.map((call) => call.query)).toEqual([ + { read: "false" }, + ]) + }, + ) + + it("starts a new filter at page one and resumes each filter's cached pages", async ({ + expect, + }) => { + disposeMockEndpoints() + const calls = mockEndpoint("GET", "/api/notifications", (call) => { + const read = call.query["filter-read_eq"] + const filter = typeof read === "string" ? read : "all" + + return { + notifications: [ + makeNotification({ + id: `${filter}-${String(call.query.page)}`, + }), + ], + pageCount: read === "false" ? 2 : 3, + } + }) + seedPages( + [ + [makeNotification({ id: "all-1" })], + [makeNotification({ id: "all-2" })], + ], + 3, + ) + const wrapper = await mountBox() + const scroll = wrapper.get(".overflow-y-auto").element + scroll.scrollTop = 120 + + await findButtonByText(wrapper, t("notification.filters.unread")).trigger( + "click", + ) + await settleMutations() + expect(scroll.scrollTop).toBe(0) + await infiniteScroll.load() + await settleMutations() + expect(notificationIds(wrapper)).toEqual(["false-1", "false-2"]) + expect(infiniteScroll.canLoadMore()).toBe(false) + await findButtonByText(wrapper, t("notification.filters.all")).trigger( + "click", + ) + await settleMutations() + expect(notificationIds(wrapper)).toEqual(["all-1", "all-2"]) + await infiniteScroll.load() + await settleMutations() + + expect(notificationIds(wrapper)).toEqual(["all-1", "all-2", "all-3"]) + expect(calls.map((call) => call.query)).toEqual([ + { limit: "50", page: "1", "filter-read_eq": "false" }, + { limit: "50", page: "2", "filter-read_eq": "false" }, + { limit: "50", page: "3" }, + ]) + }) + + it("describes an empty filtered list without saying the whole inbox is empty", async ({ + expect, + }) => { + seedNotifications([makeNotification()], 1) + const wrapper = await mountBox() + + await findButtonByText(wrapper, t("notification.filters.read")).trigger( + "click", + ) + await settleMutations() + + expect(wrapper.text()).toContain(t("notification.filtered-empty-title")) + expect(wrapper.text()).toContain( + t("notification.filtered-empty-description"), + ) + expect(wrapper.text()).not.toContain(t("notification.empty-title")) + expect(rows(wrapper)).toHaveLength(0) + expect( + findButtonByText(wrapper, t("notification.read-all-button")).exists(), + ).toBe(true) + }) + + it.for([ + { name: "one notification", all: false }, + { name: "all notifications", all: true }, + ])( + "updates filtered results and the unread count after marking $name read", + async ({ all }, { expect }) => { + disposeMockEndpoints() + let notifications = [ + makeNotification(), + makeNotification({ id: "notif-2" }), + ] + const listCalls = mockEndpoint("GET", "/api/notifications", (call) => ({ + notifications: notifications.filter( + (notification) => + call.query["filter-read_eq"] === undefined || + String(notification.read) === call.query["filter-read_eq"], + ), + pageCount: 1, + })) + const countCalls = mockEndpoint( + "GET", + "/api/notifications/count", + () => ({ + count: notifications.filter((notification) => !notification.read) + .length, + }), + ) + const putCalls = mockEndpoint( + "PUT", + "/api/notifications/read-status", + (call) => { + const { ids } = call.body as { ids: string[] } + notifications = notifications.map((notification) => ({ + ...notification, + read: + notification.read || + ids.length === 0 || + ids.includes(notification.id), + })) + + return {} + }, + ) + const wrapper = await mountBox() + await settleMutations() + await findButtonByText( + wrapper, + t("notification.filters.unread"), + ).trigger("click") + await settleMutations() + + await findButtonByText( + wrapper, + t( + all + ? "notification.read-all-button" + : "notification.actions.mark-read", + ), + ).trigger("click") + await settleMutations() + + expect(notificationIds(wrapper)).toEqual(all ? [] : ["notif-2"]) + expect(wrapper.get("button[aria-pressed='true']").text()).toBe( + t("notification.filters.unread"), + ) + expect( + wrapper + .findAll("button") + .some( + (button) => button.text() === t("notification.read-all-button"), + ), + ).toBe(!all) + expect(putCalls.map((call) => call.body)).toEqual([ + { ids: all ? [] : ["notif-1"] }, + ]) + expect(countCalls).toHaveLength(2) + expect(listCalls.map((call) => call.query["filter-read_eq"])).toEqual([ + undefined, + "false", + "false", + ]) + expect(toast.custom).not.toHaveBeenCalled() + }, + ) + + it("preserves the selected filter on live updates and invalidates inactive filters", async ({ + expect, + }) => { + disposeMockEndpoints() + let notifications = [makeNotification()] + let onNotification: () => void = () => undefined + const subscribe = vi.fn((_topic: string, callback: () => void) => { + onNotification = callback + + return vi.fn() + }) + useWebSocketStateStore().state = { subscribe } as never + const listCalls = mockEndpoint("GET", "/api/notifications", () => ({ + notifications, + pageCount: 1, + })) + const countCalls = mockEndpoint( + "GET", + "/api/notifications/count", + () => ({ count: notifications.length }), + ) + const wrapper = await mountBox() + await settleMutations() + await findButtonByText(wrapper, t("notification.filters.unread")).trigger( + "click", + ) + await settleMutations() + notifications = [...notifications, makeNotification({ id: "notif-2" })] + + onNotification() + await settleMutations() + + expect(wrapper.get("button[aria-pressed='true']").text()).toBe( + t("notification.filters.unread"), + ) + expect(notificationIds(wrapper)).toEqual(["notif-1", "notif-2"]) + await findButtonByText(wrapper, t("notification.filters.all")).trigger( + "click", + ) + await settleMutations() + expect(notificationIds(wrapper)).toEqual(["notif-1", "notif-2"]) + expect(listCalls.map((call) => call.query["filter-read_eq"])).toEqual([ + undefined, + "false", + "false", + undefined, + ]) + expect(countCalls).toHaveLength(2) + expect(subscribe).toHaveBeenCalledTimes(1) + }) + }) + describe("loading more", { concurrent: false }, () => { it.for([ { name: "offers more while the server has pages", input: 2 }, @@ -542,6 +837,43 @@ describe("", { concurrent: false }, () => { useWebSocketStateStore().state = null }) + it("still refreshes the unread count when a live list refresh fails", async ({ + expect, + }) => { + disposeMockEndpoints() + let onNotification: () => void = () => undefined + const subscribe = vi.fn((_topic: string, callback: () => void) => { + onNotification = callback + + return vi.fn() + }) + useWebSocketStateStore().state = { subscribe } as never + const listCalls = mockEndpoint("GET", "/api/notifications", () => { + throw createError({ statusCode: 400 }) + }) + const countCalls = mockEndpoint( + "GET", + "/api/notifications/count", + () => ({ count: 2 }), + ) + seedNotifications([]) + const wrapper = await mountBox() + + onNotification() + await settleMutations() + + expect( + findButtonByText(wrapper, t("notification.read-all-button")).exists(), + ).toBe(true) + expect(wrapper.get("button[aria-pressed='true']").text()).toBe( + t("notification.filters.all"), + ) + expect(infiniteScroll.canLoadMore()).toBe(false) + expect(listCalls).toHaveLength(1) + expect(countCalls).toHaveLength(1) + expect(subscribe).toHaveBeenCalledTimes(1) + }) + it("unsubscribes when it goes away", async ({ expect }) => { const unsubscribe = vi.fn() useWebSocketStateStore().state = { @@ -576,13 +908,17 @@ function seedPages(pages: unknown[][], pageCount = pages.length, unread = 0) { // a plain seed leaves the entry without its paging state, and the // box's query cannot mount on it runInApp(() => { - setInfiniteQueryData(useQueryCache(), ["notifications", "list", 50], { - pages: pages.map((notifications) => ({ - notifications: notifications, - pageCount: pageCount, - })), - pageParams: pages.map((_page, index) => index + 1), - }) + setInfiniteQueryData( + useQueryCache(), + ["notifications", "list", 50, "all"], + { + pages: pages.map((notifications) => ({ + notifications: notifications, + pageCount: pageCount, + })), + pageParams: pages.map((_page, index) => index + 1), + }, + ) }) seedQueryData(["notifications", "count", false], { count: unread }) } @@ -613,3 +949,19 @@ function mountBox() { function rows(wrapper: Awaited>) { return wrapper.findAll("[role='link']") } + +function notificationIds(wrapper: Awaited>) { + return wrapper.findAllComponents(NotificationRow).map((row) => { + const notification: unknown = row.props("notification") + if ( + !notification || + typeof notification !== "object" || + !("id" in notification) || + typeof notification.id !== "string" + ) { + throw new Error("Expected a notification with a string id") + } + + return notification.id + }) +} diff --git a/web/app/components/NotificationBox.vue b/web/app/components/NotificationBox.vue index d838651e..b4d09a01 100644 --- a/web/app/components/NotificationBox.vue +++ b/web/app/components/NotificationBox.vue @@ -17,12 +17,17 @@ const { useFetchManyNotifications, useFetchNotificationCount, markNotificationsRead, + invalidateNotifications, } = useNotificationAPI() const { fetchDocumentTree } = useDocumentAPI() const wsState = useWebSocketStateStore() const { fetchOrganization } = useAuthSession() const fetchNotificationCount = useFetchNotificationCount({ read: false }) -const fetchNotifications = useFetchManyNotifications({ limit: 50 }) +const readFilter = ref<"all" | "unread" | "read">("all") +const fetchNotifications = useFetchManyNotifications(() => ({ + limit: 50, + read: readFilter.value === "all" ? undefined : readFilter.value === "read", +})) const now = useNow({ interval: 60 * 1000 }) const list = useTemplateRef("list") // a failed load leaves the list at its end, so without the status check @@ -31,11 +36,16 @@ useInfiniteScroll(list, handleLoadMore, { distance: 100, canLoadMore: () => fetchNotifications.hasNextPage.value && - fetchNotifications.status.value !== "error", + fetchNotifications.status.value !== "error" && + !markNotificationsRead.isLoading.value, }) let unsubWsNotifications: (() => void) | null | undefined = null -const isLoadingMore = ref(false) +const filters = computed(() => [ + { value: "all" as const, label: t("notification.filters.all") }, + { value: "unread" as const, label: t("notification.filters.unread") }, + { value: "read" as const, label: t("notification.filters.read") }, +]) const notifications = computed(() => { const seen = new Set() @@ -73,8 +83,8 @@ onMounted(() => { unsubWsNotifications = wsState.state?.subscribe( WS_NOTIFICATION_CREATION_TOPIC, () => { - void fetchNotificationCount.refetch() - void fetchNotifications.refetch() + // refresh failures are already exposed by the queries + void invalidateNotifications().catch(() => undefined) }, ) }) @@ -82,6 +92,12 @@ onUnmounted(() => { unsubWsNotifications?.() }) +watch(readFilter, () => { + if (list.value) { + list.value.scrollTop = 0 + } +}) + function findDocument(notification: Notification) { return extractDocumentTreeElement( fetchDocumentTree.data.value ?? [], @@ -153,9 +169,7 @@ async function buildNotificationHref(notification: Notification) { } async function handleLoadMore() { - isLoadingMore.value = true await fetchNotifications.loadNextPage() - isLoadingMore.value = false } async function handleNotificationClick(notification: Notification) { @@ -235,16 +249,42 @@ async function handleMarkAllRead() { +
+ + {{ filter.label }} + +
- {{ t("notification.empty-title") }} + {{ + readFilter === "all" + ? t("notification.empty-title") + : t("notification.filtered-empty-title") + }}
- {{ t("notification.empty-description") }} + {{ + readFilter === "all" + ? t("notification.empty-description") + : t("notification.filtered-empty-description") + }}
@@ -270,7 +310,7 @@ async function handleMarkAllRead() { />
-const LIST_KEY_A = ["notifications", "list", 10] as const -const LIST_KEY_B = ["notifications", "list", 5] as const +const LIST_KEY_A = ["notifications", "list", 10, "all"] as const +const LIST_KEY_B = ["notifications", "list", 5, "all"] as const +const UNREAD_KEY = ["notifications", "list", 10, false] as const +const READ_KEY = ["notifications", "list", 10, true] as const function makeNotificationAPI() { return runInApp(() => useNotificationAPI()) @@ -76,6 +79,27 @@ describe("useNotificationAPI", { concurrent: false }, () => { afterEach(disposeMockEndpoints) describe("useFetchManyNotifications", () => { + it.for([ + { read: false, expected: "false" }, + { read: true, expected: "true" }, + ])( + "filters notifications on the server with read=$read", + async ({ read, expected }, { expect }) => { + const page = makePage([makeNotification("n1", read)]) + const calls = mockEndpoint("GET", "/api/notifications", () => page) + const api = makeNotificationAPI() + const params = { limit: 10, read } + const list = runInApp(() => api.useFetchManyNotifications(params)) + + await list.refresh() + + expect(list.data.value).toEqual(makePages(page)) + expect(calls.map((call) => call.query)).toEqual([ + { limit: "10", page: "1", "filter-read_eq": expected }, + ]) + }, + ) + it("fetches the first notification page", async ({ expect }) => { const page = makePage([makeNotification("n1", false)]) const listCalls = mockEndpoint("GET", "/api/notifications", () => page) @@ -113,6 +137,123 @@ describe("useNotificationAPI", { concurrent: false }, () => { expect(listCalls.map((call) => call.query.page)).toEqual(["1", "2"]) }) + it("keeps all, unread and read pages in separate caches when switching filters", async ({ + expect, + }) => { + const calls = mockEndpoint("GET", "/api/notifications", (call) => + makePage( + [ + makeNotification( + notificationPageId(call.query), + call.query["filter-read_eq"] === "true", + ), + ], + 2, + ), + ) + const api = makeNotificationAPI() + const params = ref({ limit: 10 }) + const list = runInApp(() => api.useFetchManyNotifications(params)) + await list.refresh() + await list.loadNextPage() + params.value = { limit: 10, read: false } + await nextTick() + await list.refresh() + await list.loadNextPage() + params.value = { limit: 10, read: true } + await nextTick() + await list.refresh() + + params.value = { limit: 10 } + await nextTick() + await list.refresh() + + expect(list.data.value?.pageParams).toEqual([1, 2]) + expect( + getPages(LIST_KEY_A)?.pages.flatMap((page) => + page.notifications.map((notification) => notification.id), + ), + ).toEqual(["all-1", "all-2"]) + expect( + getPages(UNREAD_KEY)?.pages.flatMap((page) => + page.notifications.map((notification) => notification.id), + ), + ).toEqual(["false-1", "false-2"]) + expect( + getPages(READ_KEY)?.pages.flatMap((page) => + page.notifications.map((notification) => notification.id), + ), + ).toEqual(["true-1"]) + expect(calls.map((call) => call.query)).toEqual([ + { limit: "10", page: "1" }, + { limit: "10", page: "2" }, + { limit: "10", page: "1", "filter-read_eq": "false" }, + { limit: "10", page: "2", "filter-read_eq": "false" }, + { limit: "10", page: "1", "filter-read_eq": "true" }, + ]) + }) + + it("keeps an in-flight multipage refetch on its original filter", async ({ + expect, + }) => { + let releasePage: () => void = () => undefined + let reachedPage: () => void = () => undefined + const reached = new Promise((resolve) => { + reachedPage = resolve + }) + const release = new Promise((resolve) => { + releasePage = resolve + }) + let hold = false + const calls = mockEndpoint("GET", "/api/notifications", async (call) => { + if ( + hold && + call.query.page === "1" && + call.query["filter-read_eq"] === undefined + ) { + reachedPage() + await release + } + + return makePage( + [makeNotification(notificationPageId(call.query), false)], + 2, + ) + }) + const api = makeNotificationAPI() + const params = ref({ limit: 10 }) + const list = runInApp(() => api.useFetchManyNotifications(params)) + await list.refresh() + await list.loadNextPage() + hold = true + const pending = list.refetch() + await reached + + params.value = { limit: 10, read: false } + await nextTick() + await list.refresh() + releasePage() + await pending + + expect( + getPages(LIST_KEY_A)?.pages.flatMap((page) => + page.notifications.map((notification) => notification.id), + ), + ).toEqual(["all-1", "all-2"]) + expect( + list.data.value?.pages.flatMap((page) => + page.notifications.map((notification) => notification.id), + ), + ).toEqual(["false-1"]) + expect(calls.map((call) => call.query)).toEqual([ + { limit: "10", page: "1" }, + { limit: "10", page: "2" }, + { limit: "10", page: "1" }, + { limit: "10", page: "1", "filter-read_eq": "false" }, + { limit: "10", page: "2" }, + ]) + }) + it("refetches every loaded page in order", async ({ expect }) => { const listCalls = mockEndpoint("GET", "/api/notifications", (call) => makePage([makeNotification(`n${String(call.query.page)}`, false)], 2), @@ -159,6 +300,146 @@ describe("useNotificationAPI", { concurrent: false }, () => { }) describe("markNotificationsRead", () => { + it("preserves the error when preparing the optimistic update fails", async ({ + expect, + }) => { + const listCalls = mockEndpoint("GET", "/api/notifications", () => + makePage([]), + ) + const countCalls = mockEndpoint( + "GET", + "/api/notifications/count", + () => ({ count: 0 }), + ) + const putCalls = mockEndpoint( + "PUT", + "/api/notifications/read-status", + () => ({}), + ) + const page = makePage([makeNotification("n1", false)]) + seedPages(LIST_KEY_A, page) + const api = makeNotificationAPI() + const queryCache = runInApp(() => useQueryCache()) + const error = new Error("Cache unavailable") + const getQueryData = vi + .spyOn(queryCache, "getQueryData") + .mockImplementationOnce(() => { + throw error + }) + + try { + await expect( + api.markNotificationsRead.mutateAsync({ ids: ["n1"] }), + ).rejects.toBe(error) + expect(getQueryData).toHaveBeenCalledExactlyOnceWith(LIST_KEY_A) + expect(putCalls).toHaveLength(0) + expect(listCalls).toHaveLength(0) + expect(countCalls).toHaveLength(0) + } finally { + getQueryData.mockRestore() + } + + expect(getPages(LIST_KEY_A)).toEqual(makePages(page)) + }) + + it.for([ + { name: "one notification", ids: ["n1"], remaining: ["n2"] }, + { name: "all notifications", ids: [], remaining: [] }, + ])( + "removes $name from unread pages while updating all pages", + async ({ ids, remaining }, { expect }) => { + const put = mockDeferredEndpoint( + "PUT", + "/api/notifications/read-status", + ) + seedPages( + LIST_KEY_A, + makePage([ + makeNotification("n1", false), + makeNotification("n2", false), + ]), + ) + seedPages( + UNREAD_KEY, + makePage([makeNotification("n1", false)]), + makePage([makeNotification("n2", false)]), + ) + seedPages(READ_KEY, makePage([makeNotification("n3", true)])) + const api = makeNotificationAPI() + + const pending = api.markNotificationsRead.mutateAsync({ ids }) + await put.reached + + expect( + getPages(UNREAD_KEY)?.pages.flatMap((page) => + page.notifications.map((notification) => notification.id), + ), + ).toEqual(remaining) + expect(readStates(LIST_KEY_A)).toEqual([[true, ids.length === 0]]) + expect(getPages(READ_KEY)).toEqual( + makePages(makePage([makeNotification("n3", true)])), + ) + expect(put.calls.map((call) => call.body)).toEqual([{ ids }]) + put.resolve({}) + await pending + }, + ) + + it("restores unread page membership when marking read fails", async ({ + expect, + }) => { + const put = mockDeferredEndpoint("PUT", "/api/notifications/read-status") + const unreadPage = makePage([makeNotification("n1", false)]) + seedPages(LIST_KEY_A, unreadPage) + seedPages(UNREAD_KEY, unreadPage) + seedPages(READ_KEY, makePage([makeNotification("n2", true)])) + const api = makeNotificationAPI() + const pending = api.markNotificationsRead.mutateAsync({ ids: ["n1"] }) + await put.reached + expect(getPages(UNREAD_KEY)?.pages[0]?.notifications).toEqual([]) + + put.reject(createError({ statusCode: 500 })) + + await expect(pending).rejects.toThrow() + expect(getPages(UNREAD_KEY)).toEqual(makePages(unreadPage)) + expect(getPages(LIST_KEY_A)).toEqual(makePages(unreadPage)) + expect(readStates(READ_KEY)).toEqual([[true]]) + expect(put.calls).toHaveLength(1) + }) + + it("rolls back touched pages when a different filter loads during a failed mutation", async ({ + expect, + }) => { + const unreadPage = makePage([makeNotification("n1", false)]) + const calls = mockEndpoint("GET", "/api/notifications", (call) => + call.query["filter-read_eq"] === "false" ? unreadPage : makePage([]), + ) + const put = mockDeferredEndpoint("PUT", "/api/notifications/read-status") + seedPages(LIST_KEY_A, unreadPage) + const api = makeNotificationAPI() + const params = ref({ limit: 10, read: false }) + const list = runInApp(() => api.useFetchManyNotifications(params)) + await list.refresh() + const pending = api.markNotificationsRead.mutateAsync({ ids: ["n1"] }) + await put.reached + expect(getPages(UNREAD_KEY)?.pages[0]?.notifications).toEqual([]) + params.value = { limit: 10, read: true } + await nextTick() + await list.refresh() + + put.reject(createError({ statusCode: 500 })) + + await expect(pending).rejects.toThrow() + expect(getPages(UNREAD_KEY)).toEqual(makePages(unreadPage)) + expect(getPages(LIST_KEY_A)).toEqual(makePages(unreadPage)) + expect(getPages(READ_KEY)).toEqual(makePages(makePage([]))) + expect(calls.map((call) => call.query["filter-read_eq"])).toEqual([ + "false", + "true", + ]) + expect(put.calls).toHaveLength(1) + }) + it("marks every cached notification as read when no ids are given", async ({ expect, }) => { @@ -406,3 +687,10 @@ describe("useNotificationAPI", { concurrent: false }, () => { }) }) }) + +function notificationPageId(query: Record) { + const read = query["filter-read_eq"] + const filter = typeof read === "string" ? read : "all" + + return `${filter}-${String(query.page)}` +} diff --git a/web/app/composables/api/useNotificationAPI.ts b/web/app/composables/api/useNotificationAPI.ts index cd2c4581..398640ae 100644 --- a/web/app/composables/api/useNotificationAPI.ts +++ b/web/app/composables/api/useNotificationAPI.ts @@ -4,7 +4,8 @@ import isDeepEqual from "fast-deep-equal" type NotificationPages = UseInfiniteQueryData const NOTIFICATION_QUERY_KEYS = { - list: (limit: number) => ["notifications", "list", limit] as const, + list: (limit: number, read?: boolean) => + ["notifications", "list", limit, read ?? "all"] as const, count: (read: boolean) => ["notifications", "count", read] as const, listRoot: ["notifications", "list"] as const, countRoot: ["notifications", "count"] as const, @@ -17,27 +18,35 @@ export default function () { function useFetchManyNotifications( paramsRef: MaybeRefOrGetter, ) { - return useInfiniteQuery({ - key: () => NOTIFICATION_QUERY_KEYS.list(toValue(paramsRef).limit), - query: async ({ pageParam }) => { - const searchParams = new URLSearchParams({ - limit: String(toValue(paramsRef).limit), - page: String(pageParam), - }) + return useInfiniteQuery(() => { + const { limit, read } = toValue(paramsRef) - return await $coreAPIClient( - `/api/notifications?${searchParams.toString()}`, - { method: "GET" }, - ) - }, - initialPageParam: 1, - getNextPageParam: (lastPage, _pages, lastPageParam) => - lastPageParam < lastPage.pageCount ? lastPageParam + 1 : null, - refetchOnMount: false, - refetchOnWindowFocus: false, - refetchOnReconnect: false, - staleTime: 60 * 1000, // 1 min - autoRefetch: true, + return { + key: NOTIFICATION_QUERY_KEYS.list(limit, read), + query: async ({ pageParam }) => { + const searchParams = new URLSearchParams({ + limit: String(limit), + page: String(pageParam), + }) + + if (read !== undefined) { + searchParams.set("filter-read_eq", String(read)) + } + + return await $coreAPIClient( + `/api/notifications?${searchParams.toString()}`, + { method: "GET" }, + ) + }, + initialPageParam: 1, + getNextPageParam: (lastPage, _pages, lastPageParam) => + lastPageParam < lastPage.pageCount ? lastPageParam + 1 : null, + refetchOnMount: false, + refetchOnWindowFocus: false, + refetchOnReconnect: false, + staleTime: 60 * 1000, // 1 min + autoRefetch: true, + } }) } @@ -105,6 +114,12 @@ export default function () { notif.read = true } }) + + if (key[3] === false) { + page.notifications = page.notifications.filter( + (notif) => !notif.read, + ) + } }) queryCache.setQueryData(key, data) @@ -119,17 +134,18 @@ export default function () { body: req, }) }, - async onSuccess() { - await queryCache.invalidateQueries({ - key: NOTIFICATION_QUERY_KEYS.listRoot, - }) - await queryCache.invalidateQueries({ - key: NOTIFICATION_QUERY_KEYS.countRoot, - }) - }, + onSuccess: invalidateNotifications, onError(_err, _data, { oldNotifs, newNotifs }) { + if (!newNotifs) { + return + } + const entries = queryCache.getEntries({ key: NOTIFICATION_QUERY_KEYS.listRoot, + // switching filters can populate another cache while the + // mutation is pending; it was not optimistically changed + predicate: (entry) => + newNotifs.some(({ key }) => isDeepEqual(key, entry.key)), }) const cachedNotifs: { key: EntryKey; data: NotificationPages }[] = [] @@ -152,13 +168,21 @@ export default function () { } // rollback - oldNotifs?.forEach(({ key, data }) => { + oldNotifs.forEach(({ key, data }) => { queryCache.setQueryData(key, data) }) }, }) + async function invalidateNotifications() { + await Promise.all([ + queryCache.invalidateQueries({ key: NOTIFICATION_QUERY_KEYS.listRoot }), + queryCache.invalidateQueries({ key: NOTIFICATION_QUERY_KEYS.countRoot }), + ]) + } + return { + invalidateNotifications, useFetchManyNotifications, useFetchNotificationCount, markNotificationsRead, diff --git a/web/app/utils/api/notification.ts b/web/app/utils/api/notification.ts index b130734b..7e92a944 100644 --- a/web/app/utils/api/notification.ts +++ b/web/app/utils/api/notification.ts @@ -2,6 +2,7 @@ export const WS_NOTIFICATION_CREATION_TOPIC = "creation@notifications" export interface NotificationsParams { limit: number + read?: boolean } export interface NotificationsCountParams { diff --git a/web/i18n/locales/en/notification.json b/web/i18n/locales/en/notification.json index 77fa90ef..47b2e46b 100644 --- a/web/i18n/locales/en/notification.json +++ b/web/i18n/locales/en/notification.json @@ -4,6 +4,14 @@ "read-all-button": "Mark all as read", "empty-title": "All caught up", "empty-description": "You'll receive notifications here for page review invitations, triggered hooks, and more", + "filtered-empty-title": "No matching notifications", + "filtered-empty-description": "Try a different filter", + "filters": { + "label": "Filter notifications", + "all": "All", + "unread": "Unread", + "read": "Read" + }, "document-fallback": "Deleted Page", "hook-fallback": "Unknown", "days": {