From a14d8e6bf5ef5bba400a7d5e692a2ddc7b978f45 Mon Sep 17 00:00:00 2001 From: Bhargav Date: Tue, 3 Nov 2026 12:49:15 +0530 Subject: [PATCH] fix(overview): address review findings on monitoring status, load flicker, forbidden copy and schema lock Counts only enabled rules as monitoring and derives the incidents check from them, keeps the page in its content state until /overview answers, uses the section no-access explanation for the legacy sections, counts schema versions under one cache lock, drops the unused stacked stat layout, and adds end-to-end scenarios that stub /overview. --- src/core/src/overview.rs | 12 +- src/infra/src/schema/mod.rs | 40 ++++-- .../Onboarding/home-first-data.spec.js | 115 +++++++++++++++++- web/src/lib/data/StatStrip/OStatCard.vue | 48 +------- web/src/lib/data/StatStrip/OStatStrip.spec.ts | 61 ---------- .../lib/data/StatStrip/OStatStrip.types.ts | 5 - web/src/lib/data/StatStrip/OStatStrip.vue | 18 +-- web/src/views/OverviewSectionEmpty.spec.ts | 9 +- web/src/views/OverviewSectionEmpty.vue | 9 +- web/src/views/OverviewTab.spec.ts | 50 +++++++- web/src/views/OverviewTab.vue | 15 ++- web/src/views/overview/overviewFormat.spec.ts | 51 +++++++- web/src/views/overview/overviewFormat.ts | 10 +- 13 files changed, 277 insertions(+), 166 deletions(-) diff --git a/src/core/src/overview.rs b/src/core/src/overview.rs index f96df702f2..f1d525f612 100644 --- a/src/core/src/overview.rs +++ b/src/core/src/overview.rs @@ -592,12 +592,12 @@ async fn fetch_streams(org_id: &str, scope: &OverviewScope) -> Result u64 { - let mut total = 0; - for s in streams.iter().filter(|s| is_counted_user_stream(s)) { - total += infra::schema::count_cached_versions_since(org_id, s.stream_type, &s.name, since) - .await as u64; - } - total + let counted: Vec<(StreamType, &str)> = streams + .iter() + .filter(|s| is_counted_user_stream(s)) + .map(|s| (s.stream_type, s.name.as_str())) + .collect(); + infra::schema::count_cached_versions_since(org_id, &counted, since).await as u64 } async fn pending_compaction(org_id: &str, visible_types: &HashSet) -> Option { diff --git a/src/infra/src/schema/mod.rs b/src/infra/src/schema/mod.rs index f6c8b0514b..5f06d7ee87 100644 --- a/src/infra/src/schema/mod.rs +++ b/src/infra/src/schema/mod.rs @@ -241,20 +241,25 @@ pub async fn get_from_db( }) } -/// Counts versions created at or after `since`, excluding the oldest (the stream's creation). +/// Sums the versions created at or after `since` across `streams`, excluding each stream's oldest +/// (its creation), under one cache read lock. pub async fn count_cached_versions_since( org_id: &str, - stream_type: StreamType, - stream_name: &str, + streams: &[(StreamType, &str)], since: i64, ) -> usize { - let cache_key = format!("{org_id}/{stream_type}/{stream_name}"); let r = STREAM_SCHEMAS.read().await; - r.get(&cache_key).map_or(0, |versions| { - let oldest = versions.iter().map(|(t, _)| *t).min(); - let in_window = versions.iter().filter(|(t, _)| *t >= since).count(); - in_window - usize::from(oldest.is_some_and(|t| t >= since)) - }) + streams + .iter() + .map(|(stream_type, stream_name)| { + let cache_key = format!("{org_id}/{stream_type}/{stream_name}"); + r.get(&cache_key).map_or(0, |versions| { + let oldest = versions.iter().map(|(t, _)| *t).min(); + let in_window = versions.iter().filter(|(t, _)| *t >= since).count(); + in_window - usize::from(oldest.is_some_and(|t| t >= since)) + }) + }) + .sum() } #[tracing::instrument(name = "infra:schema:get_versions", skip_all)] @@ -1183,7 +1188,8 @@ mod tests { ); } let count = |name: &'static str, since: i64| async move { - count_cached_versions_since("count_versions_org", StreamType::Logs, name, since).await + count_cached_versions_since("count_versions_org", &[(StreamType::Logs, name)], since) + .await }; assert_eq!(count("three", 200).await, 2); assert_eq!(count("three", 201).await, 1); @@ -1191,6 +1197,20 @@ mod tests { assert_eq!(count("one", 0).await, 0); assert_eq!(count("one", 300).await, 0); assert_eq!(count("missing", 0).await, 0); + + let all = [ + (StreamType::Logs, "three"), + (StreamType::Logs, "one"), + (StreamType::Logs, "missing"), + ]; + assert_eq!( + count_cached_versions_since("count_versions_org", &all, 0).await, + 2 + ); + assert_eq!( + count_cached_versions_since("count_versions_org", &[], 0).await, + 0 + ); } #[tokio::test] diff --git a/tests/ui-testing/playwright-tests/Onboarding/home-first-data.spec.js b/tests/ui-testing/playwright-tests/Onboarding/home-first-data.spec.js index b3222ca520..07cd84594c 100644 --- a/tests/ui-testing/playwright-tests/Onboarding/home-first-data.spec.js +++ b/tests/ui-testing/playwright-tests/Onboarding/home-first-data.spec.js @@ -14,6 +14,33 @@ const OVERVIEW_SECTIONS = [ /\/api\/v2\/[^/]+\/alerts\/incidents/, /\/api\/[^/]+\/traces\/service_graph\/topology\/current/, ]; +const OVERVIEW_ENDPOINT = /\/api\/[^/]+\/overview(\?.*)?$/; +const FIRING_RULE = 'checkout_latency_p95'; + +// one firing rule, pipelines refused and streams failed, so every section state shows on one page +function overviewWithGaps() { + const nowUs = Date.now() * 1000; + return { + streams: { status: 'error', error: 'stubbed failure' }, + alerts: { + status: 'ok', + data: { + total: 4, + enabled: 4, + disabled: 0, + by_state: { firing: 1, normal: 3, notify_failed: 0 }, + by_level: { ok: 0, warning: 0, critical: 1, no_data: 0, unclassified: 0 }, + failed_evaluation: 0, + firing: [{ name: FIRING_RULE, stream: 'checkout_traces', level: 'critical', since: nowUs - 25 * 60 * 1_000_000 }], + firing_by_level: { ok: 0, warning: 0, critical: 1, no_data: 0, unclassified: 0 }, + }, + }, + pipelines: { status: 'forbidden' }, + limits: { status: 'locked' }, + generated_at: nowUs, + cached: false, + }; +} // Cloud lets only root create orgs, so the empty screens use the stack's one empty org and never write to it. async function emptyOrgId(page, name) { @@ -81,6 +108,26 @@ async function failSections(page, status, which = OVERVIEW_SECTIONS) { } } +async function answerSectionsEmpty(page) { + await page.route(OVERVIEW_SECTIONS[0], (route) => route.fulfill({ json: { total: 0, hits: [] } })); + await page.route(OVERVIEW_SECTIONS[1], (route) => route.fulfill({ json: [] })); + await page.route(OVERVIEW_SECTIONS[2], (route) => route.fulfill({ json: { incidents: [], total: 0 } })); + await page.route(OVERVIEW_SECTIONS[3], (route) => route.fulfill({ json: { nodes: [], edges: [] } })); +} + +async function stubOverview(page, reply) { + await page.route(OVERVIEW_ENDPOINT, (route) => route.fulfill(reply)); +} + +// an org whose summary counts no rules takes the not-monitored hero, which would hide the cards under test +async function pinSummaryRules(page) { + await page.route(/\/api\/[^/]+\/summary(\?.*)?$/, async (route) => { + const response = await route.fetch(); + const body = await response.json().catch(() => ({})); + await route.fulfill({ response, json: { ...body, alerts: { ...(body.alerts || {}), num_realtime: 4, num_scheduled: 0 } } }); + }); +} + async function openHome(page, orgId, tab) { await page.evaluate((t) => localStorage.setItem('o2_home_active_tab', t), tab); await page.goto(`${base()}/web/?org_identifier=${orgId}`); @@ -184,10 +231,7 @@ const SCREENS = [ { id: 'h-home-overview-summary-error', async init({ page }) { - await page.route(OVERVIEW_SECTIONS[0], (route) => route.fulfill({ json: { total: 0, hits: [] } })); - await page.route(OVERVIEW_SECTIONS[1], (route) => route.fulfill({ json: [] })); - await page.route(OVERVIEW_SECTIONS[2], (route) => route.fulfill({ json: { incidents: [], total: 0 } })); - await page.route(OVERVIEW_SECTIONS[3], (route) => route.fulfill({ json: { nodes: [], edges: [] } })); + await answerSectionsEmpty(page); await page.route(/\/api\/[^/]+\/summary(\?.*)?$/, (route) => route.fulfill({ status: 500, json: { code: 500, message: 'stubbed failure' } })); }, async run({ page, orgId }) { @@ -373,6 +417,69 @@ const SCREENS = [ await expect(tid(page, 'overview-load-error-empty-state')).toHaveCount(0); }, }, + { + id: 'h-home-overview-section-states', + async init({ page }) { + await answerSectionsEmpty(page); + await pinSummaryRules(page); + await stubOverview(page, { json: overviewWithGaps() }); + }, + async run({ page, orgId }) { + await openHome(page, orgId, 'overview'); + const tileValue = (key) => page.locator(`[data-test="overview-headline-${key}"] [data-test="overview-tile-value"]`); + const tileNote = (key) => page.locator(`[data-test="overview-headline-${key}"] [data-test="overview-tile-note"]`); + + await expect(tid(page, 'overview-status-danger')).toBeVisible({ timeout: 30000 }); + await expect(tid(page, 'overview-status-text')).toHaveText('Needs attention'); + await expect(page.locator('[data-test^="overview-status-item-"]')).toHaveText(['1 alert firing']); + await expect(tid(page, 'overview-status-clear')).toHaveCount(0); + + await expect(tileValue('firing')).toHaveText('1'); + await expect(tileValue('failing')).toHaveText('0'); + await expect(tileValue('delivery')).toHaveText('0'); + await expect(tileValue('pipelines')).toHaveText('—'); + await expect(tileNote('pipelines')).toHaveText('No access'); + await expect(tileValue('quiet')).toHaveText('—'); + await expect(tileNote('quiet')).toHaveText('Could not load'); + + await expect(tid(page, `overview-alerts-firing-${FIRING_RULE}`)).toBeVisible(); + await expect(tid(page, 'overview-alerts-error')).toHaveCount(0); + await expect(tid(page, 'overview-alerts-forbidden')).toHaveCount(0); + + await expect(tid(page, 'overview-data-health-status-pipelines')).toHaveText('No access'); + await expect(tid(page, 'overview-data-health-value-pipelines')).toHaveText('—'); + await expect(tid(page, 'overview-data-health-status-quiet')).toHaveText('Could not load'); + await expect(tid(page, 'overview-data-health-value-quiet')).toHaveText('—'); + await expect(tid(page, 'overview-data-health-retry-btn')).toHaveCount(0); + + await expect(tid(page, 'overview-streams-error')).toBeVisible(); + await expect(tid(page, 'overview-streams-retry-btn')).toBeVisible(); + await expect(tid(page, 'overview-streams-top')).toHaveCount(0); + + await expect(tid(page, 'overview-load-error-empty-state')).toHaveCount(0); + await expect(tid(page, 'overview-no-access-empty-state')).toHaveCount(0); + }, + }, + { + id: 'h-home-overview-forbidden', + async init({ page }) { + await failSections(page, 403); + await stubOverview(page, { status: 403, json: { code: 403, message: 'Unauthorized Access' } }); + }, + async run({ page, orgId }) { + await openHome(page, orgId, 'overview'); + const noAccess = tid(page, 'overview-no-access-empty-state'); + await expect(noAccess).toBeVisible({ timeout: 30000 }); + await expect(noAccess).toContainText("You don't have access"); + await expect(tid(page, 'overview-load-error-empty-state')).toHaveCount(0); + await expect(tid(page, 'overview-load-error-retry-card')).toHaveCount(0); + await expect(page.locator('[data-test$="-retry-btn"]')).toHaveCount(0); + await expect(tid(page, 'overview-headline-strip')).toHaveCount(0); + await expect(page.locator('[data-test="home-page"] [data-test="refresh-button"]')).toHaveCount(0); + await expectNoOverviewToolbar(page); + await expect(page.locator('[data-test^="o-toast-"]')).toHaveCount(0); + }, + }, ]; test.describe('Home before and at first data', () => { diff --git a/web/src/lib/data/StatStrip/OStatCard.vue b/web/src/lib/data/StatStrip/OStatCard.vue index 62caba8d6c..c4d0060cbf 100644 --- a/web/src/lib/data/StatStrip/OStatCard.vue +++ b/web/src/lib/data/StatStrip/OStatCard.vue @@ -24,7 +24,7 @@ import OSkeleton from "@/lib/feedback/Skeleton/OSkeleton.vue"; import OTooltip from "@/lib/overlay/Tooltip/OTooltip.vue"; import OTruncatedText from "@/lib/core/Typography/OTruncatedText.vue"; import type { IconName } from "@/lib/core/Icon/OIcon.icons"; -import type { StatLayout, StatTone, StatTrend } from "./OStatStrip.types"; +import type { StatTone, StatTrend } from "./OStatStrip.types"; import useBreakpoint from "@/composables/useBreakpoint"; const { lgUp } = useBreakpoint(); @@ -52,13 +52,11 @@ const props = withDefaults( clickable?: boolean; /** Active-filter state — accent border. */ selected?: boolean; - /** `stacked` puts the label above a large value and the qualifier on its own line below, for a page's headline numbers. */ - layout?: StatLayout; /** Placeholder for the value until it first arrives; a 0 shown meanwhile reads as a real count. */ loading?: boolean; dataTest?: string; }>(), - { tone: "neutral", clickable: false, selected: false, loading: false, layout: "inline" }, + { tone: "neutral", clickable: false, selected: false, loading: false }, ); const slots = useSlots(); @@ -120,7 +118,6 @@ const TONES: Record = { }; const tc = computed(() => TONES[props.tone ?? "neutral"] ?? TONES.neutral); -const stacked = computed(() => props.layout === "stacked"); // Empty ("" / null / "—") shows a dash; a real 0 shows "0". Both render MUTED // (nothing to draw attention to) — only a positive value gets the tone colour. const displayValue = computed(() => { @@ -157,49 +154,16 @@ const trendClass = computed(() => :is="clickable ? 'button' : 'div'" :type="clickable ? 'button' : undefined" :aria-pressed="clickable ? String(selected) : undefined" - class="rounded-default bg-surface-base flex min-w-0 flex-col overflow-hidden border text-left transition-colors" + class="rounded-default bg-surface-base flex min-w-0 flex-col justify-center gap-1 overflow-hidden border px-2.5 py-1 text-left transition-colors max-lg:px-1.5" :class="[ - stacked - ? 'justify-start gap-0.5 px-3 py-2.5' - : 'justify-center gap-1 px-2.5 py-1 max-lg:px-1.5', selected ? 'border-accent' : 'border-border-default', clickable ? 'cursor-pointer' : '', clickable && !selected ? 'hover:border-accent' : '', - clickable && stacked - ? 'focus-visible:ring-button-ghost-focus-ring focus-visible:ring-3 focus-visible:outline-none' - : '', ]" - :title="label && !lgUp && !stacked ? String(label) : undefined" + :title="label && !lgUp ? String(label) : undefined" :data-test="dataTest" > - -
+
-
+
{ expect(wrapper.text()).toContain("wait time"); }); }); - - describe("stacked layout", () => { - const mountStacked = ( - item: Record = {}, - props: Record = {}, - ) => - mount(OStatStrip, { - props: { - layout: "stacked", - items: [{ key: "c", label: raw("Calls"), value: 5, sub: raw("Not set up"), ...item }], - ...props, - }, - }); - - it("puts the label above the value and the qualifier on its own line below", () => { - const wrapper = mountStacked(); - const text = wrapper.text(); - expect(text.indexOf("Calls")).toBeLessThan(text.indexOf("5")); - expect(text.indexOf("5")).toBeLessThan(text.indexOf("Not set up")); - expect(wrapper.findAll("[data-test='o-stat-card-sub-stacked']")).toHaveLength(1); - expect(text.match(/Not set up/g)).toHaveLength(1); - }); - - it("draws the value large and keeps the qualifier visible on narrow screens", () => { - const wrapper = mountStacked(); - expect(wrapper.get("[data-test='o-stat-card-value']").classes()).toContain("text-3xl"); - expect(wrapper.get("[data-test='o-stat-card-sub-stacked']").classes()).not.toContain( - "max-lg:hidden", - ); - }); - - it("draws no proportion bar unless a max is given", () => { - expect(mountStacked().find(".h-1").exists()).toBe(false); - expect(mountStacked({ max: 10 }).find(".h-1").exists()).toBe(true); - }); - - it("sizes tiles as fractions of the strip instead of the default basis", () => { - const wrapper = mount(OStatStrip, { - props: { items, layout: "stacked", compact: true }, - global: { stubs: { OStatCard: { template: '
' } } }, - }); - const classes = wrapper.findAll("[class]")[1]?.classes() ?? []; - expect(classes).toEqual(expect.arrayContaining(["grow", "basis-1/6", "max-md:basis-1/3"])); - expect(classes).not.toContain("basis-36"); - }); - - it("shows a keyboard focus ring on a clickable stacked tile only", () => { - const ring = (props: Record) => - mountStacked({}, props).get("[data-test='o-stat-card-value']").element.parentElement! - .className; - expect(ring({ selectable: true })).toContain("focus-visible:ring-3"); - expect(ring({})).not.toContain("focus-visible:ring-3"); - }); - - it("leaves the default inline tile unchanged", () => { - const wrapper = mountStacked({}, { layout: undefined }); - expect(wrapper.find("[data-test='o-stat-card-value']").exists()).toBe(false); - expect(wrapper.find("[data-test='o-stat-card-sub-stacked']").exists()).toBe(false); - expect(wrapper.text()).toContain("Not set up"); - }); - }); }); diff --git a/web/src/lib/data/StatStrip/OStatStrip.types.ts b/web/src/lib/data/StatStrip/OStatStrip.types.ts index 1d038e4b78..60bc7781da 100644 --- a/web/src/lib/data/StatStrip/OStatStrip.types.ts +++ b/web/src/lib/data/StatStrip/OStatStrip.types.ts @@ -24,9 +24,6 @@ export type StatTone = | "purple" | "neutral"; -/** `inline` puts the value beside its label; `stacked` puts the label above a large value, for headline numbers. */ -export type StatLayout = "inline" | "stacked"; - /** Optional delta shown beside the value (e.g. "▲ 12%"). */ export interface StatTrend { direction: "up" | "down" | "flat"; @@ -68,8 +65,6 @@ export interface StatItem { * display (not a clickable filter) — e.g. a "Total" tile. Defaults to true. */ selectable?: boolean; - /** Skeleton this tile alone, while the strip as a whole has settled. */ - loading?: boolean; /** data-test hook for the tile root. */ dataTest?: string; } diff --git a/web/src/lib/data/StatStrip/OStatStrip.vue b/web/src/lib/data/StatStrip/OStatStrip.vue index b56ce71236..6a0523c172 100644 --- a/web/src/lib/data/StatStrip/OStatStrip.vue +++ b/web/src/lib/data/StatStrip/OStatStrip.vue @@ -18,7 +18,7 @@ import { computed, ref, watch } from "vue"; import OStatCard from "./OStatCard.vue"; -import type { StatItem, StatLayout } from "./OStatStrip.types"; +import type { StatItem } from "./OStatStrip.types"; const props = withDefaults( defineProps<{ @@ -41,8 +41,6 @@ const props = withDefaults( * Leave off for the standard strip — long labels need the wider basis. */ compact?: boolean; - /** `stacked` tiles show the label above a large value and wrap five, three or two to a row by width. */ - layout?: StatLayout; }>(), { items: () => [], @@ -51,7 +49,6 @@ const props = withDefaults( selectedKey: null, defaultKey: null, compact: false, - layout: "inline", }, ); @@ -74,12 +71,6 @@ const activeKey = computed(() => { return named ? props.selectedKey : props.defaultKey; }); -// fractions of the strip, so a five-tile headline reads 5, 3 + 2, then 2 + 2 + 1 as the width drops -const tileBasis = computed(() => { - if (props.layout === "stacked") return "min-w-0 basis-1/6 max-xl:basis-1/4 max-md:basis-1/3"; - return ["max-lg:shrink-0 max-lg:basis-auto", props.compact ? "basis-36" : "basis-52"]; -}); - const onCardClick = (item: StatItem) => { if (props.selectable && item.selectable !== false) emit("select", item.key); }; @@ -93,19 +84,18 @@ const onCardClick = (item: StatItem) => { { expect(w.find("[data-o2-empty-state], .o2-empty-state").exists()).toBe(true); }); - it("renders the no access message for a forbidden section", () => { + it("explains a forbidden section in the same words as a forbidden card", () => { const w = mountEmpty({ kind: "forbidden" }); - expect(w.get('[data-test="overview-incidents-forbidden-message"]').text()).toContain( - "No access", + const message = w.get('[data-test="overview-incidents-forbidden-message"]'); + expect(message.text()).toContain("You don't have access"); + expect(message.text()).toContain( + "Your role cannot see this data. Ask an org admin for access.", ); + expect(message.text()).not.toContain("No access"); }); it("uses the hyphenated test id for recent events", () => { diff --git a/web/src/views/OverviewSectionEmpty.vue b/web/src/views/OverviewSectionEmpty.vue index 19370b8b4b..2b79e41e0e 100644 --- a/web/src/views/OverviewSectionEmpty.vue +++ b/web/src/views/OverviewSectionEmpty.vue @@ -23,6 +23,14 @@ along with this program. If not, see . class="border-border-default rounded-surface bg-surface-panel overflow-hidden border border-[0.0625em]" > + { - if (props.kind === "forbidden") return t("overview.state.tileForbidden"); if (props.section === "anomalies" && props.noChecks) return t("overview.empty.anomalyNoChecks"); return t(`overview.empty.${props.section}`); }); diff --git a/web/src/views/OverviewTab.spec.ts b/web/src/views/OverviewTab.spec.ts index ab051102e3..487285f683 100644 --- a/web/src/views/OverviewTab.spec.ts +++ b/web/src/views/OverviewTab.spec.ts @@ -976,6 +976,36 @@ describe("OverviewTab", () => { expect(wrapper.find('[data-test="overview-alerts-card"]').exists()).toBe(true); }); + it.each([ + ["403", forbidden], + ["500", serverError], + ])( + "should hold the page, not a full-page hero, while /overview is in flight after every section answered %s", + async (_status, err) => { + givenEnterprise(true); + givenEverySectionFails(err); + const pending = deferred<{ data: unknown }>(); + vi.mocked(overviewService.getOrgOverview).mockReturnValue(pending.promise as never); + + wrapper = mountOverviewTab(); + await flushPromises(); + + expect(wrapper.find(NO_ACCESS).exists()).toBe(false); + expect(wrapper.find(LOAD_ERROR).exists()).toBe(false); + expect(wrapper.find(REFRESH_BTN).exists()).toBe(true); + expect(wrapper.find('[data-test="overview-headline-strip"]').exists()).toBe(true); + expect(refreshLastRun()).toBeNull(); + + pending.resolve({ data: buildHealthyOverview() }); + await flushPromises(); + + expect(wrapper.find(NO_ACCESS).exists()).toBe(false); + expect(wrapper.find(LOAD_ERROR).exists()).toBe(false); + expect(wrapper.find('[data-test="overview-alerts-card"]').exists()).toBe(true); + expect(refreshLastRun()).not.toBeNull(); + }, + ); + it("should clear the refresh button's last-run time on an org switch", async () => { wrapper = mountOverviewTab(); await flushPromises(); @@ -1619,12 +1649,23 @@ describe("OverviewTab", () => { expect(wrapper.findComponent({ name: "ORefreshButton" }).props("tooltip")).toBeUndefined(); }); - it("should show zero counts and no all-clear claim when the overview shows no problem", async () => { + it("should show zero firing, failing and pipeline tiles and only the quiet streams on the status line", async () => { wrapper = mountOverviewTab(); await flushPromises(); - expect(wrapper.get('[data-test="overview-headline-firing"]').text()).toContain("0"); - expect(wrapper.get('[data-test="overview-headline-pipelines"]').text()).toContain("0"); + for (const key of ["firing", "failing", "pipelines"]) { + expect(wrapper.get(`${tile(key)} [data-test="overview-tile-value"]`).text()).toBe("0"); + } + expect(wrapper.get(`${tile("quiet")} [data-test="overview-tile-value"]`).text()).toBe("3"); + expect(wrapper.find('[data-test="overview-status-clear"]').exists()).toBe(false); + expect(wrapper.find('[data-test="overview-status-danger"]').exists()).toBe(false); + expect( + wrapper + .get('[data-test="overview-status-warning"] [data-test="overview-status-text"]') + .text(), + ).toBe("Needs attention"); + const items = wrapper.findAll('[data-test^="overview-status-item-"]'); + expect(items.map((i) => i.text())).toEqual(["3 quiet streams"]); }); it.each([ @@ -1662,8 +1703,7 @@ describe("OverviewTab", () => { wrapper = mountOverviewTab(); await flushPromises(); - const text = wrapper.get(`[data-test="overview-headline-${key}"]`).text(); - expect(text).toContain(expected); + expect(wrapper.get(`${tile(key)} [data-test="overview-tile-value"]`).text()).toBe(expected); }); it("should retry a single card with only the /overview request", async () => { diff --git a/web/src/views/OverviewTab.vue b/web/src/views/OverviewTab.vue index 9b8e3601cd..a85b02ae6d 100644 --- a/web/src/views/OverviewTab.vue +++ b/web/src/views/OverviewTab.vue @@ -968,8 +968,13 @@ const allFailed = computed(() => attemptedSections.value.every(isFailed)); type PageState = "content" | "partial" | "forbidden" | "error" | "not-monitored" | "no-data" | "all-clear"; +// no /overview answer yet leaves the heroes undecided, since each one weighs whether the cards loaded +const overviewUnanswered = computed( + () => + overviewState.loading && !overview.value && !overviewFailed.value && !overviewForbidden.value, +); const pageState = computed(() => { - if (isLoading.value || hasAnyData.value) return "content"; + if (isLoading.value || hasAnyData.value || overviewUnanswered.value) return "content"; if (allFailed.value) { const allForbidden = attemptedSections.value.every((k) => isForbidden(sectionFailure[k])); const cardsLoaded = !!overview.value && !overviewFailed.value && !overviewForbidden.value; @@ -1104,9 +1109,9 @@ const orgId = computed(() => store.state.selectedOrganization?.identifier); const loadAll = async (force = false) => { if (!orgId.value) return; isLoading.value = true; - // outside allSettled: a slow or failed /overview must never hold the page states - void loadOverview(force); + // the full-page heroes weigh whether the cards loaded, so they wait for /overview too await Promise.allSettled([ + loadOverview(force), runSection("recentEvents", (current) => loadHistoryAndSplit(force, current)), runSection("anomalies", (current) => loadAnomalies(force, current)), runSection("incidents", (current) => loadIncidents(force, current)), @@ -1115,8 +1120,8 @@ const loadAll = async (force = false) => { ]); await loadUserStreams(force); isLoading.value = false; - // the refresh button keeps the last good run, so a fresh dot never sits beside the error - if (!allFailed.value && pageState.value !== "error") lastFetched.value = new Date(); + // only a full-page hero means nothing loaded; loaded cards beside failed sections are still a run + if (pageShowsSections.value) lastFetched.value = new Date(); }; // only the latest call may set the result or clear loading diff --git a/web/src/views/overview/overviewFormat.spec.ts b/web/src/views/overview/overviewFormat.spec.ts index 17d4568cb7..32c6b86a0f 100644 --- a/web/src/views/overview/overviewFormat.spec.ts +++ b/web/src/views/overview/overviewFormat.spec.ts @@ -368,13 +368,31 @@ describe("overviewStatus", () => { const checks = overviewChecks({ overview, ...idle }); expect(checks.every((c) => !c.monitors)).toBe(true); expect(overviewStatus(checks)).toEqual({ kind: "unmonitored", items: [] }); - expect( - overviewStatus(overviewChecks({ overview, ...idle, incidents: { state: "ok", count: 0 } })) - .kind, - ).toBe("clear"); + const withIncidents = overviewChecks({ + overview, + ...idle, + incidents: { state: "ok", count: 0 }, + }); + expect(withIncidents.find((c) => c.key === "incidents")?.monitors).toBe(false); + expect(overviewStatus(withIncidents).kind).toBe("unmonitored"); }); - it("counts any one configured source as monitoring", () => { + it("calls incidents with enabled rules clear", () => { + const base = clearOverview(); + const overview = buildOverview({ + ...base, + pipelines: { status: "ok", data: { total: 0, enabled: 0, disabled: 0, errored: 0 } }, + streams: { + status: "ok", + data: { ...base.streams.data!, total_streams: 0, quiet_count: 0, by_type: [] }, + }, + }); + const checks = overviewChecks({ overview, ...idle, incidents: { state: "ok", count: 0 } }); + expect(checks.find((c) => c.key === "incidents")?.monitors).toBe(true); + expect(overviewStatus(checks).kind).toBe("clear"); + }); + + it("counts any one configured source as monitoring, and disabled rules as none", () => { const base = clearOverview(); const noRules = buildOverview({ ...base, @@ -390,7 +408,28 @@ describe("overviewStatus", () => { alerts: { status: "ok", data: { ...base.alerts.data!, total: 2, enabled: 0, disabled: 2 } }, }); const checks = overviewChecks({ overview: disabledRules, ...idle }); - expect(checks.find((c) => c.key === "firing")?.monitors).toBe(true); + expect(checks.find((c) => c.key === "firing")?.monitors).toBe(false); + expect(checks.find((c) => c.key === "failing")?.monitors).toBe(false); + }); + + it("says nothing is monitored when every rule is disabled and nothing else is set up", () => { + const base = clearOverview(); + const overview = buildOverview({ + ...base, + alerts: { + status: "ok", + data: { ...base.alerts.data!, total: 3, enabled: 0, disabled: 3, by_state: {} }, + }, + pipelines: { status: "ok", data: { total: 0, enabled: 0, disabled: 0, errored: 0 } }, + streams: { + status: "ok", + data: { ...base.streams.data!, total_streams: 0, quiet_count: 0, by_type: [] }, + }, + }); + expect(overviewStatus(overviewChecks({ overview, ...idle }))).toEqual({ + kind: "unmonitored", + items: [], + }); }); it("counts a notify-failed rule as firing", () => { diff --git a/web/src/views/overview/overviewFormat.ts b/web/src/views/overview/overviewFormat.ts index 8bca739dd3..8dfb0e3b50 100644 --- a/web/src/views/overview/overviewFormat.ts +++ b/web/src/views/overview/overviewFormat.ts @@ -298,9 +298,11 @@ export function overviewChecks(input: OverviewChecksInput): AttentionCheck[] { }; }; const o = input.overview; - const hasRules = (d: OverviewAlertsData) => d.total > 0; + // a disabled rule watches nothing, matching the tiles' "No enabled rules" note + const hasRules = (d: OverviewAlertsData) => d.enabled > 0; + const firing = fromSection("firing", o?.alerts, (d) => firingCount(d), hasRules); const checks = [ - fromSection("firing", o?.alerts, (d) => firingCount(d), hasRules), + firing, fromSection("failing", o?.alerts, (d) => d.failed_evaluation, hasRules), fromSection( "pipelines", @@ -315,9 +317,9 @@ export function overviewChecks(input: OverviewChecksInput): AttentionCheck[] { (d) => d.total_streams > 0, ), ]; + // incidents are only raised by alert rules, so they monitor exactly when an enabled rule does if (input.incidents) { - const monitors = input.incidents.state === "ok"; - checks.push({ key: "incidents", ...input.incidents, monitors }); + checks.push({ key: "incidents", ...input.incidents, monitors: firing.monitors }); } return checks; }