From 164a6782b563ea0f13bd189cb3eb3040b494c631 Mon Sep 17 00:00:00 2001 From: ktx-abhay Date: Wed, 4 Nov 2026 18:03:27 +0530 Subject: [PATCH] fix(oncall): server unread total, Needs you independent of filters, banner independent of setup, rows render first --- .../oncall/OnCallMyDeliveries.spec.ts | 109 +++++++++++++++++- .../components/oncall/OnCallMyDeliveries.vue | 34 ++++-- web/src/views/OnCall/OnCallResponses.spec.ts | 96 ++++++++++++++- web/src/views/OnCall/OnCallResponses.vue | 22 ++-- 4 files changed, 239 insertions(+), 22 deletions(-) diff --git a/web/src/components/oncall/OnCallMyDeliveries.spec.ts b/web/src/components/oncall/OnCallMyDeliveries.spec.ts index 13f14127b0..74e4ce8916 100644 --- a/web/src/components/oncall/OnCallMyDeliveries.spec.ts +++ b/web/src/components/oncall/OnCallMyDeliveries.spec.ts @@ -108,9 +108,10 @@ const history = (wrapper: ReturnType) => wrapper.findComponent('[data-test="oncall-my-deliveries-table"]'); const historyRows = (wrapper: ReturnType) => history(wrapper).findAll('[data-test="row"]'); -const answer = (deliveries: MyDelivery[]) => +/// The server counts unread sends, not pages, and ignores both the filter and the row window. +const answer = (deliveries: MyDelivery[], unread = deliveries.filter((d) => !d.read).length) => service.myDeliveries.mockResolvedValue( - axiosReply({ total: deliveries.length, unread: 99, deliveries }), + axiosReply({ total: deliveries.length, unread, deliveries }), ); describe("OnCallMyDeliveries", () => { @@ -351,6 +352,64 @@ describe("OnCallMyDeliveries", () => { expect(wrapper.find('[data-test="oncall-my-deliveries-unread"]').text()).toContain("2"); }); + it("takes the server's total when older unread pages lie past the loaded 100", async () => { + const loaded = Array.from({ length: 100 }, (_, i) => + delivery({ + event_id: `ev_${i}`, + response_id: `resp_${i}`, + subject_id: `al_${i}`, + read: i >= 60, + }), + ); + answer(loaded, 140); + const wrapper = render(); + await flushPromises(); + + expect(wrapper.emitted("unread")?.at(-1)).toEqual([140]); + expect(wrapper.find('[data-test="oncall-my-deliveries-unread"]').text()).toContain("140"); + expect(wrapper.find('[data-test="oncall-my-deliveries-read-all"]').classes()).not.toContain( + "invisible", + ); + }); + + it("keeps the badge when Unread only is ticked", async () => { + const send = (i: number, read: boolean) => + delivery({ + event_id: `ev_${i}`, + response_id: `resp_${i % 50}`, + subject_id: `al_${i}`, + read, + }); + service.myDeliveries.mockImplementation(async (args) => + axiosReply({ + total: 100, + unread: 150, + deliveries: Array.from({ length: 100 }, (_, i) => + send(i, args.unread_only ? false : i >= 40), + ), + }), + ); + const wrapper = render(); + await flushPromises(); + const before = wrapper.emitted("unread")?.at(-1); + + await wrapper.find('[data-test="oncall-my-deliveries-unread-toggle"]').trigger("click"); + await flushPromises(); + + expect(wrapper.emitted("unread")?.at(-1)).toEqual(before); + }); + + it("shows Mark all read when every loaded row is read but older ones are not", async () => { + answer([delivery({ read: true })], 3); + const wrapper = render(); + await flushPromises(); + + expect(wrapper.emitted("unread")?.at(-1)).toEqual([3]); + expect(wrapper.find('[data-test="oncall-my-deliveries-read-all"]').classes()).not.toContain( + "invisible", + ); + }); + it("shows the unread dot on an unread resolved alert", async () => { answer([delivery({ read: false, response_state: "resolved" })]); const wrapper = render(); @@ -372,6 +431,52 @@ describe("OnCallMyDeliveries", () => { expect(service.myDeliveries.mock.calls.at(-1)![0]).toMatchObject({ unread_only: true }); }); + it("keeps a read open page in Needs you while History shows unread only", async () => { + const open = delivery({ + event_id: "ev_open", + response_id: "resp_open", + read: true, + response_state: "triggered", + }); + const unreadOne = delivery({ + event_id: "ev_new", + response_id: "resp_new", + subject_id: "al_new", + title: "disk full", + read: false, + response_state: "resolved", + }); + service.myDeliveries.mockImplementation(async (args) => + axiosReply({ + total: args.unread_only ? 1 : 2, + unread: 1, + deliveries: args.unread_only ? [unreadOne] : [open, unreadOne], + }), + ); + const wrapper = render(); + await flushPromises(); + const needsYou = () => wrapper.findComponent('[data-test="oncall-my-needs-you-table"]'); + expect(needsYou().props("data")).toHaveLength(1); + + await wrapper.find('[data-test="oncall-my-deliveries-unread-toggle"]').trigger("click"); + await flushPromises(); + + expect(historyRows(wrapper)).toHaveLength(1); + expect(needsYou().props("data")).toEqual([open]); + expect(wrapper.find('[data-test="oncall-my-needs-you-ack-resp_open"]').exists()).toBe(true); + }); + + it("reads the unfiltered inbox for Needs you when the page opens on unread only", async () => { + Object.assign(useListViewState("oncall-my-deliveries", {}), { unreadOnly: true }); + answer([delivery({ read: true, response_state: "triggered" })]); + const wrapper = render(); + await flushPromises(); + + const filters = service.myDeliveries.mock.calls.map(([args]) => !!args.unread_only); + expect(filters.sort()).toEqual([false, true]); + expect(wrapper.find('[data-test="oncall-my-needs-you"]').exists()).toBe(true); + }); + /// A read, resolved page still offers "Mark unread" on its row, and nothing to filter. it("keeps the mark-unread button on a read resolved page", async () => { answer([delivery({ read: true, response_state: "resolved" })]); diff --git a/web/src/components/oncall/OnCallMyDeliveries.vue b/web/src/components/oncall/OnCallMyDeliveries.vue index a980224680..60bcdeb767 100644 --- a/web/src/components/oncall/OnCallMyDeliveries.vue +++ b/web/src/components/oncall/OnCallMyDeliveries.vue @@ -370,6 +370,8 @@ const orgId = computed(() => store.state.selectedOrganization.identifier); const nowMicros = useOnCallClock(); const rows = ref([]); +/// The unfiltered inbox: "Needs you" is every open page, whatever History is narrowed to. +const allRows = ref([]); const needsYouKey = computed( () => @@ -467,7 +469,7 @@ const emptyAction = computed<{ label: I18nText; icon: IconName; run: () => void /// The open pages, one row per page with its newest send. const openRows = computed(() => { const byPage = new Map(); - for (const row of rows.value) { + for (const row of allRows.value) { if (row.response_state === "resolved") continue; const seen = byPage.get(row.response_id); if (!seen || seen.at < row.at) byPage.set(row.response_id, row); @@ -571,22 +573,33 @@ let loadedOnce = false; /// Set only when the first load fails, so "nothing was sent" is never said for a read that did not happen. const loadError = ref(null); -async function fetchDeliveries(force = false) { +/// Pages where the loaded window shows them; sends the window did not reach still count, one each. +function unreadPages(loaded: MyDelivery[], serverUnread: number): number { + const unreadSends = loaded.filter(countsAsUnread); + const pages = new Set(unreadSends.map((row) => row.response_id)).size; + return pages + Math.max(0, serverUnread - unreadSends.length); +} + +/// `withOpen: false` re-reads History only: a filter flip changes neither the open pages nor the badge. +async function fetchDeliveries(force = false, withOpen = true) { loading.value = true; const readId = ++latestDeliveriesRead; + const filtered = unreadOnly.value; try { - const data = await readQuery( - myDeliveriesQuery(orgId.value, unreadOnly.value ? { unread_only: true } : {}), - force, - ); + const [data, unfiltered] = await Promise.all([ + readQuery(myDeliveriesQuery(orgId.value, filtered ? { unread_only: true } : {}), force), + filtered && withOpen ? readQuery(myDeliveriesQuery(orgId.value), force) : null, + ]); if (readId !== latestDeliveriesRead) return; rows.value = data?.deliveries ?? []; - if (!unreadOnly.value) { + const all = filtered ? unfiltered : data; + if (all) { + allRows.value = all.deliveries ?? []; if (!openRows.value.length) rememberOpenShape("none"); else rememberOpenShape(openPaged.value ? "paged" : "flat"); } - // Pages, not sends: one page that reached the reader on three channels is one unread. - setUnread(new Set(rows.value.filter(countsAsUnread).map((row) => row.response_id)).size); + // From the unfiltered window and the server's total, so the badge is neither capped at 100 rows nor moved by the filter. + setUnread(unreadPages(allRows.value, data?.unread ?? 0)); loadedOnce = true; loadError.value = null; } catch (err: unknown) { @@ -600,6 +613,7 @@ async function fetchDeliveries(force = false) { return; } rows.value = []; + allRows.value = []; setUnread(0); loadError.value = apiErrorText(err); } finally { @@ -614,7 +628,7 @@ function setUnread(count: number) { function setUnreadOnly(on: boolean) { unreadOnly.value = on; - void fetchDeliveries(); + void fetchDeliveries(false, false); } /// The count travels back on the write, so the badge is right without a second diff --git a/web/src/views/OnCall/OnCallResponses.spec.ts b/web/src/views/OnCall/OnCallResponses.spec.ts index 479bd11149..ea872d6858 100644 --- a/web/src/views/OnCall/OnCallResponses.spec.ts +++ b/web/src/views/OnCall/OnCallResponses.spec.ts @@ -23,6 +23,7 @@ import oncallService from "@/services/oncall"; import store from "@/test/unit/helpers/store"; import { resetListViewState, useListViewState } from "@/composables/useListViewState"; import { resetOnCallPolicyReviewed } from "@/composables/useOnCallPolicyReviewed"; +import { resetOnCallSetupDismissed } from "@/composables/useOnCallSetupDismissed"; import OnCallResponses from "@/views/OnCall/OnCallResponses.vue"; import { toast } from "@/lib/feedback/Toast/useToast"; @@ -381,6 +382,42 @@ describe("OnCallResponses", () => { return wrapper; } + /// The rows are the screen; team context and the checklist only fill cells and a bar around them. + it("renders the rows before the team context and checklist reads answer", async () => { + const teams = deferredReply(); + const context = deferredReply(); + const gaps = deferredReply(); + service.listTeams.mockImplementation(() => teams.promise); + service.teamContext.mockImplementation(() => context.promise); + service.coverageGaps.mockImplementation(() => gaps.promise); + service.listResponses.mockResolvedValue(axiosReply([page()])); + const wrapper = render(); + await flushPromises(); + + const table = wrapper.findComponent({ name: "OTable" }); + expect(table.props("loading")).toBe(false); + expect(table.props("data")).toHaveLength(1); + expect(wrapper.findAll('[data-test="row"]')).toHaveLength(1); + + teams.answer([team]); + context.answer({ teams: [] }); + gaps.answer({ at: 0, total: 0, teams: [] }); + await flushPromises(); + expect(table.props("data")).toHaveLength(1); + }); + + it("keeps the skeleton on an empty list until setup answers", async () => { + const teams = deferredReply(); + service.listTeams.mockImplementation(() => teams.promise); + const wrapper = render(); + await flushPromises(); + expect(wrapper.findComponent({ name: "OTable" }).props("loading")).toBe(true); + + teams.answer([team]); + await flushPromises(); + expect(wrapper.findComponent({ name: "OTable" }).props("loading")).toBe(false); + }); + /// The state counts the section headings carry — the only counts on the /// screen now that the P1/Mine/Unrouted tiles are gone. const sections = (w: VueWrapper) => @@ -1051,6 +1088,16 @@ describe("OnCallResponses", () => { }); afterEach(() => window.localStorage.clear()); + it("asks the missing bulk read once per load, not again for the checklist", async () => { + service.getPolicy.mockResolvedValue(axiosReply({ rungs: [] })); + service.listResponses.mockResolvedValue(axiosReply([page()])); + render(); + await flushPromises(); + + expect(service.teamContext).toHaveBeenCalledTimes(1); + expect(service.getPolicy).toHaveBeenCalledTimes(1); + }); + it("falls back to the team's own policy read", async () => { service.getPolicy.mockResolvedValue(axiosReply({ rungs: [] })); const wrapper = render(); @@ -1098,8 +1145,8 @@ describe("OnCallResponses", () => { resetOnCallPolicyReviewed(); }); - /// Setup must never read as all-good from the shift chip while pages cannot reach anyone yet. - it("hides the shift chip while setup is unfinished", async () => { + /// Being on call is a fact about the reader, so an unfinished setup never hides it. + it("shows the shift chip while setup is unfinished", async () => { const viewer = store.state.userInfo.email; service.listTeams.mockResolvedValue(axiosReply([team])); answerTeams( @@ -1113,7 +1160,50 @@ describe("OnCallResponses", () => { await flushPromises(); expect(wrapper.findComponent({ name: "OnCallSetupChecklist" }).exists()).toBe(true); - expect(wrapper.findComponent({ name: "OnCallShiftBanner" }).exists()).toBe(false); + expect(wrapper.findComponent({ name: "OnCallShiftBanner" }).exists()).toBe(true); + }); + + it("keeps the shift chip after the setup bar is dismissed", async () => { + resetOnCallSetupDismissed(); + const viewer = store.state.userInfo.email; + service.listTeams.mockResolvedValue(axiosReply([team])); + service.listOwnershipRules.mockResolvedValue(axiosReply([])); + service.listResponses.mockResolvedValue( + axiosReply([ + { + id: "resp_1", + org_id: "default", + subject: { subject_type: "alert", source_id: "al_ckt", firing: 1 }, + team_id: null, + priority: 1, + state: "triggered", + responder_role: "owner", + title: "checkout_failing", + opened_at: 1_700_000_000_000_000, + acked_by: null, + acked_at: null, + closed_at: null, + }, + ]), + ); + answerTeams( + () => ({ + on_call: [{ rotation_id: "r1", rotation_name: "Primary", rule: "", user_email: viewer }], + schedule: null, + }), + [team.id], + ); + const wrapper = mountTracked(OnCallResponses, { + global: { plugins: [i18n, store], stubs: { ...stubs, OnCallSetupChecklist: false } }, + }); + await flushPromises(); + + await wrapper.find('[data-test="oncall-setup-dismiss"]').trigger("click"); + await flushPromises(); + + expect(wrapper.find('[data-test="oncall-setup-banner"]').exists()).toBe(false); + expect(wrapper.findComponent({ name: "OnCallShiftBanner" }).exists()).toBe(true); + resetOnCallSetupDismissed(); }); /// A server without the coverage-gap endpoint used to read as "zero gaps", diff --git a/web/src/views/OnCall/OnCallResponses.vue b/web/src/views/OnCall/OnCallResponses.vue index 8620cd6b60..5222e44986 100644 --- a/web/src/views/OnCall/OnCallResponses.vue +++ b/web/src/views/OnCall/OnCallResponses.vue @@ -27,9 +27,8 @@ along with this program. If not, see .