fix(oncall): server unread total, Needs you independent of filters, banner independent of setup, rows render first

2026-11-05 00:09:35 +00:00 · 2026-11-04 18:03:27 +05:30
parent aac26c2c54
commit 164a6782b5
4 changed files with 239 additions and 22 deletions
@@ -108,9 +108,10 @@ const history = (wrapper: ReturnType<typeof render>) =>
  wrapper.findComponent('[data-test="oncall-my-deliveries-table"]');
 const historyRows = (wrapper: ReturnType<typeof render>) =>
  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" })]);
@@ -370,6 +370,8 @@ const orgId = computed(() => store.state.selectedOrganization.identifier);
 const nowMicros = useOnCallClock();

 const rows = ref<MyDelivery[]>([]);
+/// The unfiltered inbox: "Needs you" is every open page, whatever History is narrowed to.
+const allRows = ref<MyDelivery[]>([]);

 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<MyDelivery[]>(() => {
  const byPage = new Map<string, MyDelivery>();
-  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<string | null>(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
@@ -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",
@@ -27,9 +27,8 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
    <template #actions>
      <!-- Whether the reader is themselves on call, beside the buttons rather
           than in the body: it qualifies the whole screen, not one section. -->
-      <!-- Hidden while setup is unfinished: a green shift chip there would read as all-good when pages cannot reach anyone yet. -->
      <OnCallShiftBanner
-        v-if="myShift && !isMobile && !showChecklist"
+        v-if="myShift && !isMobile"
        :user-email="viewerEmail"
        :rotation="myShift.rotation"
        :team-name="myShift.teamName"
@@ -60,7 +59,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
         naming the next undone step, over a list that stays fully usable. -->
    <!-- A phone header has no room for the banner, so it leads the body instead. -->
    <OnCallShiftBanner
-      v-if="myShift && isMobile && !showChecklist"
+      v-if="myShift && isMobile"
      class="mx-page-edge mt-2 shrink-0"
      :user-email="viewerEmail"
      :rotation="myShift.rotation"
@@ -95,7 +94,7 @@ along with this program.  If not, see <http://www.gnu.org/licenses/>.
      :data="visibleRows"
      :columns="columns"
      row-key="rowKey"
-      :loading="loading || !setupLoaded"
+      :loading="loading || (!setupLoaded && !responses.length)"
      :streaming="backgroundLoading"
      :error="loadError"
      :forbidden="forbidden"
@@ -1141,7 +1140,7 @@ const statusOptions = computed(() => {
    { value: "all", label: t("oncall.all"), count: total([...SECTION_ORDER]) },
  ];
  // A count before the list has answered would read as "nothing here".
-  const counted = setupLoaded.value && !loading.value;
+  const counted = loaded.value && !loading.value;
  // An open-only fetch never asked for resolved pages, so a tab showing them has no count yet, not zero.
  const known = (tab: FilterTab) => includeResolved.value || !tabShows(tab, "resolved");
  return tabs.map((tab) => ({
@@ -1965,6 +1964,8 @@ async function someTeamWouldPage(
 // The newest row-context read: one started for rows a filter has since replaced must not overwrite the maps.
 let latestRowContextRead = 0;
 let rowContextRead: Promise<void> = Promise.resolve();
+/// Every chunk of the last bulk read failed, so asking it again for the checklist would fail the same way.
+let teamContextUnavailable = false;

 /// Team context for the row teams and the viewer's, and the ladder of the oldest open pages: one request each, sent together.
 async function fetchRowContext(force = false) {
@@ -1981,7 +1982,10 @@ async function fetchRowContext(force = false) {
  const [context, progress] = await Promise.allSettled([
    Promise.allSettled(
      teamChunks.map((ids, i) => readQuery(teamContextQuery(orgId.value, ids, i === 0), force)),
-    ).then((reads) => reads.flatMap((r) => (r.status === "fulfilled" ? r.value : []))),
+    ).then((reads) => {
+      teamContextUnavailable = reads.every((r) => r.status === "rejected");
+      return reads.flatMap((r) => (r.status === "fulfilled" ? r.value : []));
+    }),
    wanted.length
      ? readQuery(responsesProgressQuery(orgId.value, wanted), force)
      : Promise.resolve([]),
@@ -1999,6 +2003,10 @@ async function fetchPolicyPresence(force = false) {
    return;
  }
  const chunks = chunkIds(teams.value.map((team) => team.id));
+  if (teamContextUnavailable) {
+    setup.value.hasPolicy = chunks.length ? await anyTeamPolicy(chunks.flat(), force) : false;
+    return;
+  }
  const reads = await Promise.allSettled(
    chunks.map((ids) => readQuery(teamContextQuery(orgId.value, ids, false), force)),
  );
@@ -2203,7 +2211,7 @@ watch(
 );
 // The first load lands after mount and TanStack resets to page 0 on its own deferred queue; setTimeout(0) runs after it so the restored page wins.
 watch(
-  () => loaded.value && setupLoaded.value,
+  loaded,
  (ready) => {
    if (!ready) return;
    setTimeout(() => oTableRef.value?.restorePage?.(currentPage.value), 0);