From 4da9ed06992ad3afed042246a3aef55db65f43f7 Mon Sep 17 00:00:00 2001 From: pedrooot Date: Wed, 8 Jul 2026 11:09:26 +0200 Subject: [PATCH] feat(compliance): add finding count for scans --- .../client-accordion-content.tsx | 58 ++--- .../use-requirement-findings.test.ts | 222 ++++++++++++------ .../use-requirement-findings.ts | 85 +++++-- 3 files changed, 224 insertions(+), 141 deletions(-) diff --git a/ui/components/compliance/compliance-accordion/client-accordion-content.tsx b/ui/components/compliance/compliance-accordion/client-accordion-content.tsx index 342b6dba65..1c4e22d310 100644 --- a/ui/components/compliance/compliance-accordion/client-accordion-content.tsx +++ b/ui/components/compliance/compliance-accordion/client-accordion-content.tsx @@ -127,51 +127,34 @@ export const ClientAccordionContent = ({ patchTriageUpdate(input); }; - // Per-provider finding tallies for the cross-provider breakdown. - // - // ``count`` is authoritative: it sums each of the provider's scans' real - // ``pagination.count`` (total findings for its checks), NOT the loaded page - // rows — a provider with 300 findings must read 300 here even while the - // table shows page 1. - // - // ``pass``/``fail`` can only be tallied from loaded rows, so they're honest - // only when every one of the provider's scans returned all its rows on this - // page (``pages <= 1``). When a provider is paginated, ``fullyLoaded`` is - // false and the caller renders "—" instead of a misleading partial split. + // Per-provider finding tallies for the cross-provider breakdown, sourced + // ENTIRELY from the authoritative per-scan metadata — never from the loaded + // table rows. ``count`` sums each scan's real ``pagination.count`` and + // ``pass``/``fail`` sum the exact status-filtered counts the hook fetched + // (count-only requests). Because none of this depends on the table's page, + // a paginated provider gets a correct split instead of a "—" placeholder. const providerFindingStats = (() => { const count: Record = {}; const pass: Record = {}; const fail: Record = {}; - const fullyLoaded: Record = {}; if (!isCrossProvider || !scanIdsByProvider) { - return { count, pass, fail, fullyLoaded }; + return { count, pass, fail }; } - const scanToProvider = new Map(); for (const [providerKey, scanIds] of Object.entries(scanIdsByProvider)) { count[providerKey] = 0; pass[providerKey] = 0; fail[providerKey] = 0; - fullyLoaded[providerKey] = true; if (Array.isArray(scanIds)) { for (const sid of scanIds) { - scanToProvider.set(sid, providerKey); const meta = crossProviderScanMeta[sid]; - // No meta yet (scan errored or still loading) or the scan spans - // multiple pages → this provider's pass/fail can't be trusted. - count[providerKey] += meta?.count ?? 0; - if (!meta || meta.pages > 1) fullyLoaded[providerKey] = false; + if (!meta) continue; + count[providerKey] += meta.count ?? 0; + pass[providerKey] += meta.pass ?? 0; + fail[providerKey] += meta.fail ?? 0; } } } - for (const row of findings?.data ?? []) { - const sid = row.relationships?.scan?.data?.id; - const providerKey = sid ? scanToProvider.get(sid) : undefined; - if (!providerKey) continue; - const status = row.attributes?.status; - if (status === "PASS") pass[providerKey] += 1; - else if (status === "FAIL") fail[providerKey] += 1; - } - return { count, pass, fail, fullyLoaded }; + return { count, pass, fail }; })(); const renderDetails = () => { @@ -222,10 +205,6 @@ export const ClientAccordionContent = ({ const findingsCount = providerFindingStats.count[providerKey]; const passCount = providerFindingStats.pass[providerKey] ?? 0; const failCount = providerFindingStats.fail[providerKey] ?? 0; - // Pass/Fail is only honest when the provider's rows are fully - // loaded (single page per scan); otherwise it's a page sample. - const passFailKnown = - providerFindingStats.fullyLoaded[providerKey] ?? false; return ( - {findingsLoaded && passFailKnown ? ( + {findingsLoaded ? ( {passCount} @@ -274,16 +253,7 @@ export const ClientAccordionContent = ({ {failCount} ) : ( - - — - + — )} diff --git a/ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts b/ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts index 1f61ca9d28..610ce94fbf 100644 --- a/ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts +++ b/ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts @@ -68,6 +68,34 @@ async function flushAsync() { }); } +// The cross-provider fan-out fires, per scan, one row-page request plus two +// count-only requests (``filter[status__in]`` PASS / FAIL, used only for their +// ``meta.pagination.count``). Route the mock by scan id + status so the tests +// stay independent of call ordering. +function mockCrossProviderFindings( + config: Record, +) { + findingsActionsMock.getFindings.mockImplementation( + async ({ filters }: { filters: Record }) => { + const scan = filters["filter[scan]"]; + const status = filters["filter[status__in]"]; + const entry = config[scan]; + if (!entry) return undefined; + if (status === "PASS") + return { + data: [], + meta: { pagination: { count: entry.pass ?? 0, pages: 1 } }, + }; + if (status === "FAIL") + return { + data: [], + meta: { pagination: { count: entry.fail ?? 0, pages: 1 } }, + }; + return entry.main; + }, + ); +} + describe("useRequirementFindings", () => { beforeEach(() => { findingsActionsMock.getFindings.mockReset(); @@ -293,32 +321,41 @@ describe("useRequirementFindings", () => { ); }); - it("fans out one fetch per contributing scan and merges the counts in cross-provider mode", async () => { + it("fans out a row-page fetch plus count-only PASS/FAIL per scan and merges the counts in cross-provider mode", async () => { // Given: two providers, one scan each, with disjoint check sets. findingsActionsMock.getFindings.mockReset(); - findingsActionsMock.getFindings - .mockResolvedValueOnce({ - data: [ - { - id: "finding-a", - attributes: { status: "FAIL" }, - relationships: { scan: { data: { id: "scan-a" } } }, - }, - ], - included: [{ type: "scans", id: "scan-a" }], - meta: { pagination: { count: 1, pages: 1 } }, - }) - .mockResolvedValueOnce({ - data: [ - { - id: "finding-b", - attributes: { status: "PASS" }, - relationships: { scan: { data: { id: "scan-b" } } }, - }, - ], - included: [{ type: "scans", id: "scan-b" }], - meta: { pagination: { count: 2, pages: 1 } }, - }); + mockCrossProviderFindings({ + "scan-a": { + main: { + data: [ + { + id: "finding-a", + attributes: { status: "FAIL" }, + relationships: { scan: { data: { id: "scan-a" } } }, + }, + ], + included: [{ type: "scans", id: "scan-a" }], + meta: { pagination: { count: 1, pages: 1 } }, + }, + pass: 0, + fail: 1, + }, + "scan-b": { + main: { + data: [ + { + id: "finding-b", + attributes: { status: "PASS" }, + relationships: { scan: { data: { id: "scan-b" } } }, + }, + ], + included: [{ type: "scans", id: "scan-b" }], + meta: { pagination: { count: 2, pages: 1 } }, + }, + pass: 2, + fail: 0, + }, + }); // When const { result } = renderHook(() => @@ -333,48 +370,77 @@ describe("useRequirementFindings", () => { ); await flushAsync(); - // Then: one request per scan, each scoped to that provider's checks. - expect(findingsActionsMock.getFindings).toHaveBeenCalledTimes(2); + // Then: three requests per scan (row page + PASS count + FAIL count) → 6. + expect(findingsActionsMock.getFindings).toHaveBeenCalledTimes(6); + // The row-page request is scoped to that provider's checks, no status filter. expect(findingsActionsMock.getFindings).toHaveBeenCalledWith( expect.objectContaining({ filters: expect.objectContaining({ "filter[scan]": "scan-a", "filter[check_id__in]": "check_a", }), + pageSize: 10, }), ); - // And the responses are merged: rows concatenated, counts summed. + // And a count-only PASS request (page size 1) for the same scan+checks. + expect(findingsActionsMock.getFindings).toHaveBeenCalledWith( + expect.objectContaining({ + filters: expect.objectContaining({ + "filter[scan]": "scan-a", + "filter[check_id__in]": "check_a", + "filter[status__in]": "PASS", + }), + pageSize: 1, + }), + ); + // Only the row-page responses feed the merge: rows concatenated, counts summed. expect(result.current.findings?.data).toHaveLength(2); expect(result.current.findings?.meta?.pagination?.count).toBe(3); + // Exact per-scan pass/fail come from the count-only responses, not the rows. + expect(result.current.crossProviderScanMeta["scan-a"]).toEqual( + expect.objectContaining({ count: 1, pages: 1, pass: 0, fail: 1 }), + ); + expect(result.current.crossProviderScanMeta["scan-b"]).toEqual( + expect.objectContaining({ count: 2, pages: 1, pass: 2, fail: 0 }), + ); }); it("takes the worst-case page count across scans so pagination reaches every tail", async () => { findingsActionsMock.getFindings.mockReset(); - findingsActionsMock.getFindings - .mockResolvedValueOnce({ - data: [ - { - id: "finding-a", - attributes: { status: "FAIL", severity: "high" }, - relationships: { scan: { data: { id: "scan-a" } } }, - }, - ], - included: [{ type: "scans", id: "scan-a" }], + mockCrossProviderFindings({ + "scan-a": { // AWS spans many pages… - meta: { pagination: { count: 30, pages: 3 } }, - }) - .mockResolvedValueOnce({ - data: [ - { - id: "finding-b", - attributes: { status: "PASS", severity: "low" }, - relationships: { scan: { data: { id: "scan-b" } } }, - }, - ], - included: [{ type: "scans", id: "scan-b" }], + main: { + data: [ + { + id: "finding-a", + attributes: { status: "FAIL", severity: "high" }, + relationships: { scan: { data: { id: "scan-a" } } }, + }, + ], + included: [{ type: "scans", id: "scan-a" }], + meta: { pagination: { count: 30, pages: 3 } }, + }, + pass: 10, + fail: 20, + }, + "scan-b": { // …Azure fits in one. - meta: { pagination: { count: 5, pages: 1 } }, - }); + main: { + data: [ + { + id: "finding-b", + attributes: { status: "PASS", severity: "low" }, + relationships: { scan: { data: { id: "scan-b" } } }, + }, + ], + included: [{ type: "scans", id: "scan-b" }], + meta: { pagination: { count: 5, pages: 1 } }, + }, + pass: 5, + fail: 0, + }, + }); const { result } = renderHook(() => useRequirementFindings( @@ -392,35 +458,45 @@ describe("useRequirementFindings", () => { // table's Next button stays enabled and AWS's later pages are reachable. expect(result.current.findings?.meta?.pagination?.pages).toBe(3); expect(result.current.findings?.meta?.pagination?.count).toBe(35); + // Pass/fail are exact even though AWS is paginated — they come from the + // count-only fetches, not the single loaded page. + expect(result.current.crossProviderScanMeta["scan-a"]).toEqual( + expect.objectContaining({ pages: 3, pass: 10, fail: 20 }), + ); }); it("globally re-sorts the merged rows by the active sort (FAIL/critical first)", async () => { findingsActionsMock.getFindings.mockReset(); // Scan A returns a PASS/low row; scan B a FAIL/critical row. Concatenation // alone would list A before B and contradict the FAIL-first sort. - findingsActionsMock.getFindings - .mockResolvedValueOnce({ - data: [ - { - id: "finding-pass", - attributes: { status: "PASS", severity: "low" }, - relationships: { scan: { data: { id: "scan-a" } } }, - }, - ], - included: [{ type: "scans", id: "scan-a" }], - meta: { pagination: { count: 1, pages: 1 } }, - }) - .mockResolvedValueOnce({ - data: [ - { - id: "finding-fail", - attributes: { status: "FAIL", severity: "critical" }, - relationships: { scan: { data: { id: "scan-b" } } }, - }, - ], - included: [{ type: "scans", id: "scan-b" }], - meta: { pagination: { count: 1, pages: 1 } }, - }); + mockCrossProviderFindings({ + "scan-a": { + main: { + data: [ + { + id: "finding-pass", + attributes: { status: "PASS", severity: "low" }, + relationships: { scan: { data: { id: "scan-a" } } }, + }, + ], + included: [{ type: "scans", id: "scan-a" }], + meta: { pagination: { count: 1, pages: 1 } }, + }, + }, + "scan-b": { + main: { + data: [ + { + id: "finding-fail", + attributes: { status: "FAIL", severity: "critical" }, + relationships: { scan: { data: { id: "scan-b" } } }, + }, + ], + included: [{ type: "scans", id: "scan-b" }], + meta: { pagination: { count: 1, pages: 1 } }, + }, + }, + }); const { result } = renderHook(() => useRequirementFindings( diff --git a/ui/components/compliance/compliance-accordion/use-requirement-findings.ts b/ui/components/compliance/compliance-accordion/use-requirement-findings.ts index 5d41e17e1d..fa21053d9f 100644 --- a/ui/components/compliance/compliance-accordion/use-requirement-findings.ts +++ b/ui/components/compliance/compliance-accordion/use-requirement-findings.ts @@ -42,12 +42,17 @@ interface UseRequirementFindingsOptions { } // Authoritative per-scan findings metadata (the total ``count`` for that -// scan's checks, and how many ``pages`` it spans). Sourced from each -// per-scan response's pagination — not from the loaded rows — so the -// per-provider breakdown can show real totals instead of a page-1 sample. +// scan's checks, how many ``pages`` it spans, and the exact PASS / FAIL +// split). All sourced from each per-scan response's pagination — not from the +// loaded rows — so the per-provider breakdown shows real totals and an honest +// pass/fail split regardless of the table's pagination. ``pass``/``fail`` come +// from dedicated status-filtered count-only fetches (``page[size]=1``, only +// ``meta.pagination.count`` is read). export interface CrossProviderScanMeta { count: number; pages: number; + pass: number; + fail: number; } interface UseRequirementFindingsReturn { @@ -269,24 +274,53 @@ export function useRequirementFindings({ }, ); - const responses = await Promise.all( - jobs.map(({ scanIdForAccount, checks }) => - getFindings({ - filters: { - "filter[check_id__in]": checks, - "filter[scan]": scanIdForAccount, - "filter[muted]": mutedFilter, - ...(region && { "filter[region__in]": region }), - }, - page: parseInt(pageNumber, 10), - pageSize: parseInt(pageSize, 10), - sort: encodedSort, - }), - ), + // Per job, fetch the page of rows (feeds the table) plus two + // count-only requests scoped to PASS / FAIL (``page[size]=1`` — only + // ``meta.pagination.count`` is read). The count-only pair gives the + // per-provider breakdown an exact pass/fail split that does NOT + // depend on how many rows the table happens to have loaded, so a + // paginated provider no longer renders a "—" placeholder. + const readCount = (r: unknown) => + (r as FindingsResponseLike)?.meta?.pagination?.count ?? 0; + + const perJob = await Promise.all( + jobs.map(async ({ scanIdForAccount, checks }) => { + const baseFilters: Record = { + "filter[check_id__in]": checks, + "filter[scan]": scanIdForAccount, + "filter[muted]": mutedFilter, + ...(region && { "filter[region__in]": region }), + }; + const [main, passRes, failRes] = await Promise.all([ + getFindings({ + filters: baseFilters, + page: parseInt(pageNumber, 10), + pageSize: parseInt(pageSize, 10), + sort: encodedSort, + }), + getFindings({ + filters: { ...baseFilters, "filter[status__in]": "PASS" }, + page: 1, + pageSize: 1, + sort: encodedSort, + }), + getFindings({ + filters: { ...baseFilters, "filter[status__in]": "FAIL" }, + page: 1, + pageSize: 1, + sort: encodedSort, + }), + ]); + return { scanIdForAccount, main, passRes, failRes }; + }), ); if (cancelled) return; + // Only the row-page responses feed the merge and the failure check; + // the count-only responses are consumed for scanMeta below. + const responses = perJob.map((job) => job.main); + // ``getFindings`` resolves to ``undefined`` on a failed request // rather than throwing, so a per-scan failure would otherwise be // silently dropped from the merge and the view would look complete. @@ -301,16 +335,19 @@ export function useRequirementFindings({ return; } - // Authoritative per-scan totals, aligned by index with ``jobs`` - // (``responses[i]`` answers ``jobs[i]``). The breakdown reads these - // for real per-provider counts instead of counting loaded rows. + // Authoritative per-scan totals keyed by scan id. ``count``/``pages`` + // come from the row-page response; ``pass``/``fail`` from the two + // count-only responses. The breakdown reads these for real + // per-provider counts instead of counting loaded rows. const scanMeta: Record = {}; - responses.forEach((r, index) => { - if (!r || typeof r !== "object" || !("data" in r)) return; - const pagination = (r as FindingsResponseLike).meta?.pagination; - scanMeta[jobs[index].scanIdForAccount] = { + perJob.forEach(({ scanIdForAccount, main, passRes, failRes }) => { + if (!main || typeof main !== "object" || !("data" in main)) return; + const pagination = (main as FindingsResponseLike).meta?.pagination; + scanMeta[scanIdForAccount] = { count: pagination?.count ?? 0, pages: pagination?.pages ?? 1, + pass: readCount(passRes), + fail: readCount(failRes), }; });