From 266ee5a6c6e6146f3d4f47d52cbe831846b622bd Mon Sep 17 00:00:00 2001 From: Prowler Bot Date: Mon, 6 Jul 2026 13:44:48 +0200 Subject: [PATCH] fix(ui): enable triage editing in compliance findings table (#11847) Co-authored-by: Alejandro Bailo <59607668+alejandrobailo@users.noreply.github.com> --- .../findings-view/findings-view.ssr.tsx | 5 +- .../alerts/_components/alert-form-modal.tsx | 35 +- .../client-accordion-content.test.ts | 26 ++ .../client-accordion-content.tsx | 148 ++++---- .../use-requirement-findings.test.ts | 325 ++++++++++++++++++ .../use-requirement-findings.ts | 146 ++++++++ .../table/finding-note-modal.test.tsx | 34 +- .../findings/table/finding-note-modal.tsx | 38 +- .../table/finding-triage-cells.test.tsx | 2 +- .../table/use-resource-drawer-bootstrap.ts | 27 +- ui/lib/finding-detail.ts | 4 +- ui/lib/finding-triage.test.ts | 222 ++++++++++++ ui/lib/finding-triage.ts | 46 +++ ui/lib/helper.ts | 15 - ui/lib/utils.ts | 36 ++ 15 files changed, 944 insertions(+), 165 deletions(-) create mode 100644 ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts create mode 100644 ui/components/compliance/compliance-accordion/use-requirement-findings.ts create mode 100644 ui/lib/finding-triage.test.ts diff --git a/ui/app/(prowler)/_overview/graphs-tabs/findings-view/findings-view.ssr.tsx b/ui/app/(prowler)/_overview/graphs-tabs/findings-view/findings-view.ssr.tsx index a9cef2cb36..9a7a184a42 100644 --- a/ui/app/(prowler)/_overview/graphs-tabs/findings-view/findings-view.ssr.tsx +++ b/ui/app/(prowler)/_overview/graphs-tabs/findings-view/findings-view.ssr.tsx @@ -7,7 +7,7 @@ import { ColumnLatestFindings } from "@/components/overview/new-findings-table/t import { CardTitle } from "@/components/shadcn"; import { DataTable } from "@/components/ui/table"; import { FINDINGS_FILTERED_SORT, MUTED_FILTER } from "@/lib"; -import { createDict } from "@/lib/helper"; +import { createDict } from "@/lib/utils"; import { FindingProps, SearchParamsProps } from "@/types"; import { pickFilterParams } from "../../_lib/filter-params"; @@ -45,7 +45,8 @@ export async function FindingsViewSSR({ searchParams }: FindingsViewSSRProps) { const scan = scanDict[finding.relationships?.scan?.data?.id]; const resource = resourceDict[finding.relationships?.resources?.data?.[0]?.id]; - const provider = providerDict[scan?.relationships?.provider?.data?.id]; + const provider = + providerDict[scan?.relationships?.provider?.data?.id ?? ""]; return { ...finding, diff --git a/ui/app/(prowler)/alerts/_components/alert-form-modal.tsx b/ui/app/(prowler)/alerts/_components/alert-form-modal.tsx index 8a9effd890..409055b2bd 100644 --- a/ui/app/(prowler)/alerts/_components/alert-form-modal.tsx +++ b/ui/app/(prowler)/alerts/_components/alert-form-modal.tsx @@ -594,22 +594,31 @@ const AlertFormModalContent = ({ {errors.root && (
{errors.root}
)} -
- - {editingAlert && ( - + )} + - )} - +
diff --git a/ui/components/compliance/compliance-accordion/client-accordion-content.test.ts b/ui/components/compliance/compliance-accordion/client-accordion-content.test.ts index 7f724e20b7..a036fc0aa8 100644 --- a/ui/components/compliance/compliance-accordion/client-accordion-content.test.ts +++ b/ui/components/compliance/compliance-accordion/client-accordion-content.test.ts @@ -13,4 +13,30 @@ describe("client accordion content", () => { expect(source).toContain("getStandaloneFindingColumns"); expect(source).not.toContain("getColumnFindings"); }); + + it("wires triage update and note loading actions into compliance findings", () => { + expect(source).toContain("updateFindingTriage"); + expect(source).toContain("loadLatestFindingTriageNote"); + expect(source).toContain("onTriageUpdateAction"); + expect(source).toContain("onTriageNoteLoadAction"); + }); + + it("refetches findings after mutelist-shortcut triage updates like the resource drawer", () => { + expect(source).toContain("shouldRefreshAfterTriageUpdate"); + expect(source).toContain("reload()"); + }); + + it("delegates data fetching to the hook instead of effect/ref choreography", () => { + expect(source).toContain("useRequirementFindings"); + expect(source).not.toContain("useEffect"); + expect(source).not.toContain("useRef"); + }); + + it("gates the skeleton on the hook loading state and surfaces fetch errors", () => { + // A disabled fetch (e.g. "No findings" status) must not skeleton forever, + // and a failed fetch must offer a retry instead of hanging. + expect(source).toContain("isLoading && requirement.status"); + expect(source).not.toContain("findings === null"); + expect(source).toContain("Try again"); + }); }); diff --git a/ui/components/compliance/compliance-accordion/client-accordion-content.tsx b/ui/components/compliance/compliance-accordion/client-accordion-content.tsx index 0ccd793c12..1ad22792f4 100644 --- a/ui/components/compliance/compliance-accordion/client-accordion-content.tsx +++ b/ui/components/compliance/compliance-accordion/client-accordion-content.tsx @@ -2,21 +2,26 @@ import { AlertTriangle } from "lucide-react"; import { useSearchParams } from "next/navigation"; -import { useEffect, useRef, useState } from "react"; -import { getFindings } from "@/actions/findings/findings"; +import { + loadLatestFindingTriageNote, + updateFindingTriage, +} from "@/actions/findings"; import { getStandaloneFindingColumns, SkeletonTableFindings, } from "@/components/findings/table"; -import { Alert, AlertDescription } from "@/components/shadcn"; +import { Alert, AlertDescription, Button } from "@/components/shadcn"; import { Accordion } from "@/components/ui/accordion/Accordion"; import { DataTable } from "@/components/ui/table"; -import { createDict, FINDINGS_DEFAULT_SORT, MUTED_FILTER } from "@/lib"; +import { FINDINGS_DEFAULT_SORT, MUTED_FILTER } from "@/lib"; import { INVALID_CONFIG_NOTE } from "@/lib/compliance/commons"; import { getComplianceMapper } from "@/lib/compliance/compliance-mapper"; +import { shouldRefreshAfterTriageUpdate } from "@/lib/finding-triage"; import { Requirement } from "@/types/compliance"; -import { FindingProps, FindingsResponse } from "@/types/components"; +import type { UpdateFindingTriageInput } from "@/types/findings-triage"; + +import { useRequirementFindings } from "./use-requirement-findings"; interface ClientAccordionContentProps { requirement: Requirement; @@ -31,100 +36,53 @@ export const ClientAccordionContent = ({ scanId, disableFindings = false, }: ClientAccordionContentProps) => { - const [findings, setFindings] = useState(null); - const [expandedFindings, setExpandedFindings] = useState([]); const searchParams = useSearchParams(); const pageNumber = searchParams.get("page") || "1"; const pageSize = searchParams.get("pageSize") || "10"; const complianceId = searchParams.get("complianceId"); const openFindingId = searchParams.get("id"); const sort = searchParams.get("sort") || FINDINGS_DEFAULT_SORT; - const loadedPageRef = useRef(null); - const loadedPageSizeRef = useRef(null); - const loadedSortRef = useRef(null); - const loadedMutedRef = useRef(null); - const isExpandedRef = useRef(false); const region = searchParams.get("filter[region__in]") || ""; // Respect the user's muted preference from the URL; default to EXCLUDE // so the requirement view stays consistent with every other findings // surface in the app (findings page, resource drawer, overview widgets). const mutedFilter = searchParams.get("filter[muted]") || MUTED_FILTER.EXCLUDE; - useEffect(() => { - async function loadFindings() { - if ( - !disableFindings && - requirement.check_ids?.length > 0 && - requirement.status !== "No findings" && - (loadedPageRef.current !== pageNumber || - loadedPageSizeRef.current !== pageSize || - loadedSortRef.current !== sort || - loadedMutedRef.current !== mutedFilter || - !isExpandedRef.current) - ) { - loadedPageRef.current = pageNumber; - loadedPageSizeRef.current = pageSize; - loadedSortRef.current = sort; - loadedMutedRef.current = mutedFilter; - isExpandedRef.current = true; + const checks = requirement.check_ids || []; - try { - const checkIds = requirement.check_ids; - const encodedSort = sort.replace(/^\+/, ""); - const findingsData = await getFindings({ - filters: { - "filter[check_id__in]": checkIds.join(","), - "filter[scan]": scanId, - "filter[muted]": mutedFilter, - ...(region && { "filter[region__in]": region }), - }, - page: parseInt(pageNumber, 10), - pageSize: parseInt(pageSize, 10), - sort: encodedSort, - }); - - setFindings(findingsData); - - if (findingsData?.data) { - // Create dictionaries for resources, scans, and providers - const resourceDict = createDict("resources", findingsData); - const scanDict = createDict("scans", findingsData); - const providerDict = createDict("providers", findingsData); - - // Expand each finding with its corresponding resource, scan, and provider - const expandedData = findingsData.data.map( - (finding: FindingProps) => { - const scan = scanDict[finding.relationships?.scan?.data?.id]; - const resource = - resourceDict[finding.relationships?.resources?.data?.[0]?.id]; - const provider = - providerDict[scan?.relationships?.provider?.data?.id]; - - return { - ...finding, - relationships: { scan, resource, provider }, - }; - }, - ); - setExpandedFindings(expandedData); - } - } catch (error) { - console.error("Error loading findings:", error); - } - } - } - - loadFindings(); - }, [ - requirement, + const { + findings, + expandedFindings, + isLoading, + error, + patchTriageUpdate, + reload, + } = useRequirementFindings({ + enabled: + !disableFindings && + checks.length > 0 && + requirement.status !== "No findings", + checkIds: checks, scanId, pageNumber, pageSize, sort, region, mutedFilter, - disableFindings, - ]); + }); + + const handleTriageUpdate = async (input: UpdateFindingTriageInput) => { + await updateFindingTriage(input); + + // Mutelist-shortcut statuses mute the finding server-side; refetch so the + // list honors the muted filter, matching the resource drawer behavior. + if (shouldRefreshAfterTriageUpdate(input)) { + reload(); + return; + } + + patchTriageUpdate(input); + }; const renderDetails = () => { if (!complianceId) { @@ -148,7 +106,6 @@ export const ClientAccordionContent = ({ ); } - const checks = requirement.check_ids || []; const checksList = (
@@ -174,7 +131,26 @@ export const ClientAccordionContent = ({ ]; const renderFindingsTable = () => { - if (findings === null && requirement.status !== "MANUAL") { + if (error) { + return ( + + + + {error} + + + + ); + } + + if (isLoading && requirement.status !== "MANUAL") { return ; } @@ -184,8 +160,12 @@ export const ClientAccordionContent = ({

Findings

diff --git a/ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts b/ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts new file mode 100644 index 0000000000..87067fd020 --- /dev/null +++ b/ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts @@ -0,0 +1,325 @@ +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { FINDING_TRIAGE_STATUS } from "@/types/findings-triage"; + +import { useRequirementFindings } from "./use-requirement-findings"; + +const findingsActionsMock = vi.hoisted(() => ({ + getFindings: vi.fn(), +})); + +vi.mock("@/actions/findings", () => findingsActionsMock); + +function makeFindingsResponse() { + return { + data: [ + { + id: "finding-1", + attributes: { muted: false, status: "FAIL" }, + triage: { + findingId: "finding-1", + findingUid: "uid-1", + triageId: "triage-1", + notesCount: 0, + status: FINDING_TRIAGE_STATUS.UNDER_REVIEW, + label: "Under Review", + hasVisibleNote: false, + isMuted: false, + canEdit: true, + billingHref: "https://prowler.com/pricing", + }, + relationships: { + scan: { data: { id: "scan-1" } }, + resources: { data: [{ id: "resource-1" }] }, + }, + }, + ], + included: [ + { + type: "scans", + id: "scan-1", + relationships: { provider: { data: { id: "provider-1" } } }, + }, + { type: "resources", id: "resource-1" }, + { type: "providers", id: "provider-1" }, + ], + meta: { pagination: { count: 1, pages: 1 } }, + }; +} + +function defaultOptions(overrides?: Record) { + return { + enabled: true, + checkIds: ["check_1", "check_2"], + scanId: "scan-1", + pageNumber: "1", + pageSize: "10", + sort: "+severity", + region: "", + mutedFilter: "false", + ...overrides, + }; +} + +async function flushAsync() { + await act(async () => { + await new Promise((resolve) => setTimeout(resolve, 0)); + }); +} + +describe("useRequirementFindings", () => { + beforeEach(() => { + findingsActionsMock.getFindings.mockReset(); + findingsActionsMock.getFindings.mockResolvedValue(makeFindingsResponse()); + }); + + it("should fetch findings with the requirement filters and strip the sort plus sign", async () => { + // Given / When + renderHook(() => useRequirementFindings(defaultOptions())); + await flushAsync(); + + // Then + expect(findingsActionsMock.getFindings).toHaveBeenCalledTimes(1); + expect(findingsActionsMock.getFindings).toHaveBeenCalledWith({ + filters: { + "filter[check_id__in]": "check_1,check_2", + "filter[scan]": "scan-1", + "filter[muted]": "false", + }, + page: 1, + pageSize: 10, + sort: "severity", + }); + }); + + it("should expand findings with their included scan, resource, and provider", async () => { + // Given / When + const { result } = renderHook(() => + useRequirementFindings(defaultOptions()), + ); + await flushAsync(); + + // Then + const [expanded] = result.current.expandedFindings; + expect(expanded.relationships).toEqual({ + scan: expect.objectContaining({ id: "scan-1" }), + resource: expect.objectContaining({ id: "resource-1" }), + provider: expect.objectContaining({ id: "provider-1" }), + }); + expect(result.current.findings?.meta?.pagination?.count).toBe(1); + }); + + it("should not fetch when disabled or without check ids", async () => { + // Given / When + renderHook(() => + useRequirementFindings(defaultOptions({ enabled: false })), + ); + renderHook(() => useRequirementFindings(defaultOptions({ checkIds: [] }))); + await flushAsync(); + + // Then + expect(findingsActionsMock.getFindings).not.toHaveBeenCalled(); + }); + + it("should not report loading when the fetch is disabled", async () => { + // Given / When + const disabled = renderHook(() => + useRequirementFindings(defaultOptions({ enabled: false })), + ); + const withoutChecks = renderHook(() => + useRequirementFindings(defaultOptions({ checkIds: [] })), + ); + await flushAsync(); + + // Then — a skipped fetch must not look like a pending one. + expect(disabled.result.current.isLoading).toBe(false); + expect(withoutChecks.result.current.isLoading).toBe(false); + }); + + it("should report loading until the fetch settles", async () => { + // Given + let resolveFetch: (value: unknown) => void = () => {}; + findingsActionsMock.getFindings.mockImplementationOnce( + () => new Promise((resolve) => (resolveFetch = resolve)), + ); + + // When + const { result } = renderHook(() => + useRequirementFindings(defaultOptions()), + ); + + // Then + expect(result.current.isLoading).toBe(true); + + // When + act(() => { + resolveFetch(makeFindingsResponse()); + }); + await flushAsync(); + + // Then + expect(result.current.isLoading).toBe(false); + }); + + it("should expose an error and stop loading when the fetch fails", async () => { + // Given + const consoleErrorSpy = vi + .spyOn(console, "error") + .mockImplementation(() => {}); + findingsActionsMock.getFindings.mockRejectedValue( + new Error("network down"), + ); + + // When + const { result } = renderHook(() => + useRequirementFindings(defaultOptions()), + ); + await flushAsync(); + + // Then — the caller can render an error state instead of a skeleton. + expect(result.current.error).toBe("Could not load findings."); + expect(result.current.isLoading).toBe(false); + expect(result.current.findings).toBeNull(); + + consoleErrorSpy.mockRestore(); + }); + + it("should clear the error and recover on reload", async () => { + // Given: first fetch fails, retry succeeds + const consoleErrorSpy = vi + .spyOn(console, "error") + .mockImplementation(() => {}); + findingsActionsMock.getFindings + .mockRejectedValueOnce(new Error("network down")) + .mockResolvedValueOnce(makeFindingsResponse()); + + const { result } = renderHook(() => + useRequirementFindings(defaultOptions()), + ); + await flushAsync(); + expect(result.current.error).toBe("Could not load findings."); + + // When + act(() => { + result.current.reload(); + }); + await flushAsync(); + + // Then + expect(result.current.error).toBeNull(); + expect(result.current.findings).not.toBeNull(); + + consoleErrorSpy.mockRestore(); + }); + + it("should not refetch when only the checkIds array identity changes", async () => { + // Given + const { rerender } = renderHook((props) => useRequirementFindings(props), { + initialProps: defaultOptions(), + }); + await flushAsync(); + + // When: same values, fresh array identity (parent re-render) + rerender(defaultOptions()); + await flushAsync(); + + // Then + expect(findingsActionsMock.getFindings).toHaveBeenCalledTimes(1); + }); + + it("should refetch when a query parameter changes", async () => { + // Given + const { rerender } = renderHook((props) => useRequirementFindings(props), { + initialProps: defaultOptions(), + }); + await flushAsync(); + + // When + rerender(defaultOptions({ pageNumber: "2" })); + await flushAsync(); + + // Then + expect(findingsActionsMock.getFindings).toHaveBeenCalledTimes(2); + expect(findingsActionsMock.getFindings).toHaveBeenLastCalledWith( + expect.objectContaining({ page: 2 }), + ); + }); + + it("should refetch on reload keeping previous data visible meanwhile", async () => { + // Given + const { result } = renderHook(() => + useRequirementFindings(defaultOptions()), + ); + await flushAsync(); + + // When + act(() => { + result.current.reload(); + }); + + // Then: previous data is not cleared while the refetch is in flight + expect(result.current.findings).not.toBeNull(); + await flushAsync(); + expect(findingsActionsMock.getFindings).toHaveBeenCalledTimes(2); + }); + + it("should patch the matching row triage optimistically", async () => { + // Given + const { result } = renderHook(() => + useRequirementFindings(defaultOptions()), + ); + await flushAsync(); + + // When + act(() => { + result.current.patchTriageUpdate({ + findingId: "finding-1", + findingUid: "uid-1", + triageId: "triage-1", + notesCount: 0, + status: FINDING_TRIAGE_STATUS.REMEDIATING, + previousStatus: FINDING_TRIAGE_STATUS.UNDER_REVIEW, + isMuted: false, + }); + }); + + // Then + expect(result.current.expandedFindings[0]?.triage).toEqual( + expect.objectContaining({ + status: FINDING_TRIAGE_STATUS.REMEDIATING, + label: "Remediating", + }), + ); + }); + + it("should ignore stale responses after the query changes", async () => { + // Given: first request resolves late, second resolves immediately + let resolveFirst: (value: unknown) => void = () => {}; + const staleResponse = { + ...makeFindingsResponse(), + meta: { pagination: { count: 99, pages: 9 } }, + }; + findingsActionsMock.getFindings + .mockImplementationOnce( + () => new Promise((resolve) => (resolveFirst = resolve)), + ) + .mockResolvedValueOnce(makeFindingsResponse()); + + const { result, rerender } = renderHook( + (props) => useRequirementFindings(props), + { initialProps: defaultOptions() }, + ); + + // When: the query changes before the first request settles + rerender(defaultOptions({ pageNumber: "2" })); + await flushAsync(); + act(() => { + resolveFirst(staleResponse); + }); + await flushAsync(); + + // Then: the stale response never overwrites the fresh one + expect(result.current.findings?.meta?.pagination?.count).toBe(1); + }); +}); diff --git a/ui/components/compliance/compliance-accordion/use-requirement-findings.ts b/ui/components/compliance/compliance-accordion/use-requirement-findings.ts new file mode 100644 index 0000000000..596069e877 --- /dev/null +++ b/ui/components/compliance/compliance-accordion/use-requirement-findings.ts @@ -0,0 +1,146 @@ +"use client"; + +import { useEffect, useState } from "react"; + +import { getFindings } from "@/actions/findings"; +import { applyOptimisticFindingTriageRowsUpdate } from "@/lib/finding-triage"; +import { createDict } from "@/lib/utils"; +import { FindingProps, FindingsResponse } from "@/types/components"; +import type { UpdateFindingTriageInput } from "@/types/findings-triage"; + +interface UseRequirementFindingsOptions { + enabled: boolean; + checkIds: string[]; + scanId: string; + pageNumber: string; + pageSize: string; + sort: string; + region: string; + mutedFilter: string; +} + +interface UseRequirementFindingsReturn { + findings: FindingsResponse | null; + expandedFindings: FindingProps[]; + isLoading: boolean; + error: string | null; + patchTriageUpdate: (input: UpdateFindingTriageInput) => void; + reload: () => void; +} + +const FINDINGS_LOAD_ERROR = "Could not load findings."; + +export function useRequirementFindings({ + enabled, + checkIds, + scanId, + pageNumber, + pageSize, + sort, + region, + mutedFilter, +}: UseRequirementFindingsOptions): UseRequirementFindingsReturn { + const [findings, setFindings] = useState(null); + const [expandedFindings, setExpandedFindings] = useState([]); + const [error, setError] = useState(null); + const [reloadNonce, setReloadNonce] = useState(0); + + // Depend on the joined value, not the array: the requirement prop gets a + // fresh identity on every parent render and must not retrigger the fetch. + const checkIdsKey = checkIds.join(","); + const isFetchEnabled = enabled && checkIdsKey.length > 0; + // A skipped fetch is not a pending one; without this the caller would show + // a skeleton forever for requirements whose fetch never runs. + const isLoading = isFetchEnabled && findings === null && error === null; + + useEffect(() => { + if (!isFetchEnabled) { + return; + } + + let cancelled = false; + + const loadFindings = async () => { + setError(null); + try { + const findingsData = await getFindings({ + filters: { + "filter[check_id__in]": checkIdsKey, + "filter[scan]": scanId, + "filter[muted]": mutedFilter, + ...(region && { "filter[region__in]": region }), + }, + page: parseInt(pageNumber, 10), + pageSize: parseInt(pageSize, 10), + sort: sort.replace(/^\+/, ""), + }); + + if (cancelled) return; + + setFindings(findingsData); + + if (findingsData?.data) { + const resourceDict = createDict("resources", findingsData); + const scanDict = createDict("scans", findingsData); + const providerDict = createDict("providers", findingsData); + + const expandedData = findingsData.data.map( + (finding: FindingProps) => { + const scan = scanDict[finding.relationships?.scan?.data?.id]; + const resource = + resourceDict[finding.relationships?.resources?.data?.[0]?.id]; + const provider = + providerDict[scan?.relationships?.provider?.data?.id ?? ""]; + + return { + ...finding, + relationships: { scan, resource, provider }, + }; + }, + ); + setExpandedFindings(expandedData); + } + } catch (error) { + if (!cancelled) { + console.error("Error loading findings:", error); + setError(FINDINGS_LOAD_ERROR); + } + } + }; + + loadFindings(); + + return () => { + cancelled = true; + }; + }, [ + isFetchEnabled, + checkIdsKey, + scanId, + pageNumber, + pageSize, + sort, + region, + mutedFilter, + reloadNonce, + ]); + + const patchTriageUpdate = (input: UpdateFindingTriageInput) => { + setExpandedFindings((currentFindings) => + applyOptimisticFindingTriageRowsUpdate(currentFindings, input), + ); + }; + + const reload = () => { + setReloadNonce((value) => value + 1); + }; + + return { + findings, + expandedFindings, + isLoading, + error, + patchTriageUpdate, + reload, + }; +} diff --git a/ui/components/findings/table/finding-note-modal.test.tsx b/ui/components/findings/table/finding-note-modal.test.tsx index 4d40c382a3..eddb893333 100644 --- a/ui/components/findings/table/finding-note-modal.test.tsx +++ b/ui/components/findings/table/finding-note-modal.test.tsx @@ -49,6 +49,7 @@ beforeAll(() => { }); }); +import { DOCS_URLS } from "@/lib/external-urls"; import { FINDING_TRIAGE_DISABLED_REASON, FINDING_TRIAGE_STATUS, @@ -147,6 +148,21 @@ describe("FindingNoteModal", () => { ).toBeVisible(); }); + it("should render a documentation link without requiring Remediating status", () => { + // Given / When + renderNoteModal(); + + // Then + const docsLink = screen.getByRole("link", { + name: /triage documentation/i, + }); + expect(docsLink).toHaveAttribute("href", DOCS_URLS.FINDINGS_TRIAGE); + expect(docsLink).toHaveAttribute("target", "_blank"); + expect( + screen.queryByText(/automatically changed to Resolved/i), + ).not.toBeInTheDocument(); + }); + it("should send existing note changes with noteId and without duplicate-note status payload", async () => { // Given const user = userEvent.setup(); @@ -157,7 +173,7 @@ describe("FindingNoteModal", () => { const textarea = screen.getByLabelText("Note text"); await user.clear(textarea); await user.type(textarea, "Documented owner follow-up."); - await user.click(screen.getByRole("button", { name: "Save changes" })); + await user.click(screen.getByRole("button", { name: "Save" })); // Then expect(onTriageUpdateAction).toHaveBeenCalledWith({ @@ -189,7 +205,7 @@ describe("FindingNoteModal", () => { // When const textarea = screen.getByLabelText("Note text"); await user.type(textarea, " Initial triage note. "); - await user.click(screen.getByRole("button", { name: "Save changes" })); + await user.click(screen.getByRole("button", { name: "Save" })); // Then expect(onTriageUpdateAction).toHaveBeenCalledWith({ @@ -214,7 +230,7 @@ describe("FindingNoteModal", () => { // When await user.clear(screen.getByLabelText("Note text")); - await user.click(screen.getByRole("button", { name: "Save changes" })); + await user.click(screen.getByRole("button", { name: "Save" })); // Then expect(onTriageUpdateAction).toHaveBeenCalledWith({ @@ -239,7 +255,7 @@ describe("FindingNoteModal", () => { // When await user.clear(screen.getByLabelText("Note text")); await user.type(screen.getByLabelText("Note text"), "Changed note"); - await user.click(screen.getByRole("button", { name: "Save changes" })); + await user.click(screen.getByRole("button", { name: "Save" })); // Then expect( @@ -278,7 +294,7 @@ describe("FindingNoteModal", () => { const textarea = screen.getByLabelText("Note text"); await user.clear(textarea); await user.type(textarea, "Documenting the resolution."); - await user.click(screen.getByRole("button", { name: "Save changes" })); + await user.click(screen.getByRole("button", { name: "Save" })); // Then expect(onTriageUpdateAction).toHaveBeenCalledWith( @@ -306,9 +322,7 @@ describe("FindingNoteModal", () => { ).not.toBeInTheDocument(); await user.click(screen.getByRole("button", { name: "Cancel" })); expect(onOpenChange).toHaveBeenCalledWith(false); - expect( - screen.getByRole("button", { name: "Save changes" }), - ).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Save" })).toBeInTheDocument(); }); it("should disable controls and show the Cloud upsell badge for non-paying users", () => { @@ -325,7 +339,7 @@ describe("FindingNoteModal", () => { screen.getByRole("combobox", { name: "Triage status" }), ).toHaveAttribute("data-disabled", ""); expect(screen.getByLabelText("Note text")).toBeDisabled(); - expect(screen.getByRole("button", { name: "Save changes" })).toBeDisabled(); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); expect( screen.getByRole("link", { name: "Available in Prowler Cloud" }), ).toHaveAttribute("href", "https://prowler.com/pricing"); @@ -360,7 +374,7 @@ describe("FindingNoteModal", () => { ); // When - await user.click(screen.getByRole("button", { name: "Save changes" })); + await user.click(screen.getByRole("button", { name: "Save" })); // Then await waitFor(() => diff --git a/ui/components/findings/table/finding-note-modal.tsx b/ui/components/findings/table/finding-note-modal.tsx index 90c9241017..95e86051b5 100644 --- a/ui/components/findings/table/finding-note-modal.tsx +++ b/ui/components/findings/table/finding-note-modal.tsx @@ -1,5 +1,6 @@ "use client"; +import { ExternalLink, Info } from "lucide-react"; import { type FormEvent, useRef, useState } from "react"; import { ProviderTypeIcon } from "@/components/icons/providers-badge/provider-type-icon"; @@ -11,7 +12,6 @@ import { TooltipTrigger, } from "@/components/shadcn/tooltip"; import { CloudFeatureBadgeLink } from "@/components/shared/cloud-feature-badge"; -import { CustomLink } from "@/components/ui/custom/custom-link"; import { DOCS_URLS } from "@/lib/external-urls"; import { FINDING_TRIAGE_DISABLED_REASON, @@ -131,10 +131,25 @@ export function FindingNoteModal({ unbreakable content (e.g. resource UIDs) widens the grid track past the modal instead of truncating. */}
-
-
+
+ + Learn how triage states work in the + +
+ +
+
{findingContext.providerType ? ( - + ) : ( {findingContext.provider?.slice(0, 3).toUpperCase() ?? "—"} @@ -192,12 +207,7 @@ export function FindingNoteModal({ {shouldShowRemediatingInfo && ( - - {REMEDIATING_INFO_COPY}.{" "} - - Learn more - - + {REMEDIATING_INFO_COPY}. )} @@ -225,10 +235,12 @@ export function FindingNoteModal({
-
+ {/* mt-3 lifts the gap-5 form spacing to 32px so the distance to the + footer matches the launch scan and alert modals. */} +
diff --git a/ui/components/findings/table/finding-triage-cells.test.tsx b/ui/components/findings/table/finding-triage-cells.test.tsx index 3d0a417860..74fd48764c 100644 --- a/ui/components/findings/table/finding-triage-cells.test.tsx +++ b/ui/components/findings/table/finding-triage-cells.test.tsx @@ -373,7 +373,7 @@ describe("finding triage cells", () => { screen.getByRole("dialog", { name: "Add Triage Note" }), ).toBeVisible(); expect(screen.getByLabelText("Note text")).toBeDisabled(); - expect(screen.getByRole("button", { name: "Save changes" })).toBeDisabled(); + expect(screen.getByRole("button", { name: "Save" })).toBeDisabled(); expect( screen.getByRole("link", { name: "Available in Prowler Cloud" }), ).toHaveAttribute("href", "https://prowler.com/pricing"); diff --git a/ui/components/resources/table/use-resource-drawer-bootstrap.ts b/ui/components/resources/table/use-resource-drawer-bootstrap.ts index 0fd33dba56..675bf337b0 100644 --- a/ui/components/resources/table/use-resource-drawer-bootstrap.ts +++ b/ui/components/resources/table/use-resource-drawer-bootstrap.ts @@ -3,11 +3,7 @@ import { useEffect, useState } from "react"; import { getResourceDrawerData } from "@/actions/resources"; -import { - applyOptimisticTriageSummaryUpdate, - getOptimisticTriageMutedReason, - shouldMarkFindingMutedForTriageUpdate, -} from "@/lib/finding-triage"; +import { applyOptimisticFindingTriageRowsUpdate } from "@/lib/finding-triage"; import { MetaDataProps } from "@/types"; import type { UpdateFindingTriageInput } from "@/types/findings-triage"; import { OrganizationResource } from "@/types/organizations"; @@ -57,26 +53,7 @@ export function useResourceDrawerBootstrap({ const patchTriageUpdate = (input: UpdateFindingTriageInput) => { setFindingsData((findings) => - findings.map((finding) => { - if (!finding.triage || finding.triage.findingId !== input.findingId) { - return finding; - } - - const shouldMarkMuted = shouldMarkFindingMutedForTriageUpdate(input); - - return { - ...finding, - triage: applyOptimisticTriageSummaryUpdate(finding.triage, input), - attributes: { - ...finding.attributes, - muted: shouldMarkMuted ? true : finding.attributes.muted, - muted_reason: - shouldMarkMuted && input.isMuted !== true && input.status - ? getOptimisticTriageMutedReason(input.status) - : finding.attributes.muted_reason, - }, - }; - }), + applyOptimisticFindingTriageRowsUpdate(findings, input), ); }; diff --git a/ui/lib/finding-detail.ts b/ui/lib/finding-detail.ts index 5566b9f994..73bf2e33aa 100644 --- a/ui/lib/finding-detail.ts +++ b/ui/lib/finding-detail.ts @@ -23,12 +23,12 @@ export function expandFindingWithRelationships( const scan = scanDict[finding.relationships?.scan?.data?.id]; const resource = resourceDict[finding.relationships?.resources?.data?.[0]?.id]; - const provider = providerDict[scan?.relationships?.provider?.data?.id]; + const provider = providerDict[scan?.relationships?.provider?.data?.id ?? ""]; return { ...finding, relationships: { ...finding.relationships, scan, resource, provider }, - } as FindingProps; + } as unknown as FindingProps; } export function findingToFindingResourceRow( diff --git a/ui/lib/finding-triage.test.ts b/ui/lib/finding-triage.test.ts new file mode 100644 index 0000000000..a08eb29804 --- /dev/null +++ b/ui/lib/finding-triage.test.ts @@ -0,0 +1,222 @@ +import { describe, expect, it } from "vitest"; + +import { + FINDING_TRIAGE_STATUS, + type FindingTriageSummary, +} from "@/types/findings-triage"; + +import { + applyOptimisticFindingTriageRowsUpdate, + applyOptimisticFindingTriageRowUpdate, +} from "./finding-triage"; + +interface TestFindingRowAttributes { + muted: boolean; + muted_reason?: string; + status: string; +} + +interface TestFindingRow { + id: string; + triage?: FindingTriageSummary; + attributes: TestFindingRowAttributes; +} + +function makeTriageSummary( + overrides?: Partial, +): FindingTriageSummary { + return { + findingId: "finding-1", + findingUid: "uid-1", + triageId: "triage-1", + notesCount: 0, + status: FINDING_TRIAGE_STATUS.UNDER_REVIEW, + label: "Under Review", + hasVisibleNote: false, + isMuted: false, + canEdit: true, + billingHref: "https://prowler.com/pricing", + ...overrides, + }; +} + +function makeFindingRow(overrides?: Partial): TestFindingRow { + return { + id: "finding-1", + triage: makeTriageSummary(), + attributes: { + muted: false, + muted_reason: undefined, + status: "FAIL", + }, + ...overrides, + }; +} + +describe("finding triage optimistic row updates", () => { + it("should patch matching finding row triage and muted attributes", () => { + // Given + const finding = makeFindingRow(); + + // When + const result = applyOptimisticFindingTriageRowUpdate(finding, { + findingId: "finding-1", + findingUid: "uid-1", + triageId: "triage-1", + notesCount: 0, + status: FINDING_TRIAGE_STATUS.RISK_ACCEPTED, + previousStatus: FINDING_TRIAGE_STATUS.UNDER_REVIEW, + isMuted: false, + note: "Accepted by owner.", + }); + + // Then + expect(result).not.toBe(finding); + expect(result.triage).toEqual( + expect.objectContaining({ + status: FINDING_TRIAGE_STATUS.RISK_ACCEPTED, + label: "Risk Accepted", + hasVisibleNote: true, + notesCount: 1, + isMuted: true, + }), + ); + expect(result.attributes).toEqual( + expect.objectContaining({ + muted: true, + muted_reason: "Finding triage status changed to Risk Accepted.", + status: "FAIL", + }), + ); + }); + + it("should leave non-matching rows unchanged when patching a list", () => { + // Given + const matchingFinding = makeFindingRow(); + const otherFinding = makeFindingRow({ + id: "finding-2", + triage: makeTriageSummary({ + findingId: "finding-2", + findingUid: "uid-2", + triageId: "triage-2", + }), + }); + + // When + const result = applyOptimisticFindingTriageRowsUpdate( + [matchingFinding, otherFinding], + { + findingId: "finding-1", + findingUid: "uid-1", + triageId: "triage-1", + notesCount: 0, + status: FINDING_TRIAGE_STATUS.REMEDIATING, + previousStatus: FINDING_TRIAGE_STATUS.UNDER_REVIEW, + isMuted: false, + }, + ); + + // Then + expect(result[0]?.triage).toEqual( + expect.objectContaining({ + status: FINDING_TRIAGE_STATUS.REMEDIATING, + label: "Remediating", + }), + ); + expect(result[1]).toBe(otherFinding); + }); + + it("should preserve muted attributes when leaving a mutelist-shortcut status", () => { + // Given: a finding muted by a previous shortcut transition. The server + // never removes the mute rule when the status moves on, so the optimistic + // update must not unmute the row either. + const finding = makeFindingRow({ + triage: makeTriageSummary({ + status: FINDING_TRIAGE_STATUS.RISK_ACCEPTED, + label: "Risk Accepted", + isMuted: true, + }), + attributes: { + muted: true, + muted_reason: "Finding triage status changed to Risk Accepted.", + status: "FAIL", + }, + }); + + // When + const result = applyOptimisticFindingTriageRowUpdate(finding, { + findingId: "finding-1", + findingUid: "uid-1", + triageId: "triage-1", + notesCount: 0, + status: FINDING_TRIAGE_STATUS.REMEDIATING, + previousStatus: FINDING_TRIAGE_STATUS.RISK_ACCEPTED, + isMuted: true, + }); + + // Then + expect(result.triage).toEqual( + expect.objectContaining({ + status: FINDING_TRIAGE_STATUS.REMEDIATING, + label: "Remediating", + isMuted: true, + }), + ); + expect(result.attributes).toEqual( + expect.objectContaining({ + muted: true, + muted_reason: "Finding triage status changed to Risk Accepted.", + }), + ); + }); + + it("should not overwrite muted_reason when an already muted finding enters a shortcut status", () => { + // Given: muted through some other channel (e.g. a mutelist rule). + const finding = makeFindingRow({ + triage: makeTriageSummary({ isMuted: true }), + attributes: { + muted: true, + muted_reason: "Muted by mutelist rule.", + status: "FAIL", + }, + }); + + // When: no new mute rule is created for already muted findings, so the + // optimistic reason must keep the original one. + const result = applyOptimisticFindingTriageRowUpdate(finding, { + findingId: "finding-1", + findingUid: "uid-1", + triageId: "triage-1", + notesCount: 0, + status: FINDING_TRIAGE_STATUS.RISK_ACCEPTED, + previousStatus: FINDING_TRIAGE_STATUS.UNDER_REVIEW, + isMuted: true, + }); + + // Then + expect(result.attributes).toEqual( + expect.objectContaining({ + muted: true, + muted_reason: "Muted by mutelist rule.", + }), + ); + }); + + it("should leave rows without triage unchanged", () => { + // Given + const finding = makeFindingRow({ triage: undefined }); + + // When + const result = applyOptimisticFindingTriageRowUpdate(finding, { + findingId: "finding-1", + findingUid: "uid-1", + triageId: null, + notesCount: 0, + note: "No triage payload on this row.", + isMuted: false, + }); + + // Then + expect(result).toBe(finding); + }); +}); diff --git a/ui/lib/finding-triage.ts b/ui/lib/finding-triage.ts index 4c4d36920b..a7fefef66c 100644 --- a/ui/lib/finding-triage.ts +++ b/ui/lib/finding-triage.ts @@ -5,6 +5,16 @@ import { type UpdateFindingTriageInput, } from "@/types/findings-triage"; +interface FindingTriageRowAttributes { + muted?: boolean; + muted_reason?: string; +} + +export interface FindingTriageRow { + triage?: FindingTriageSummary; + attributes: FindingTriageRowAttributes; +} + export const shouldMarkFindingMutedForTriageUpdate = ( input: UpdateFindingTriageInput, ): boolean => Boolean(input.status && isMutelistShortcutStatus(input.status)); @@ -45,3 +55,39 @@ export const applyOptimisticTriageSummaryUpdate = ( : {}), }; }; + +export const applyOptimisticFindingTriageRowUpdate = < + TRow extends FindingTriageRow, +>( + finding: TRow, + input: UpdateFindingTriageInput, +): TRow => { + if (!finding.triage || finding.triage.findingId !== input.findingId) { + return finding; + } + + const shouldMarkMuted = shouldMarkFindingMutedForTriageUpdate(input); + + return { + ...finding, + triage: applyOptimisticTriageSummaryUpdate(finding.triage, input), + attributes: { + ...finding.attributes, + muted: shouldMarkMuted ? true : finding.attributes.muted, + muted_reason: + shouldMarkMuted && input.isMuted !== true && input.status + ? getOptimisticTriageMutedReason(input.status) + : finding.attributes.muted_reason, + }, + }; +}; + +export const applyOptimisticFindingTriageRowsUpdate = < + TRow extends FindingTriageRow, +>( + findings: TRow[], + input: UpdateFindingTriageInput, +): TRow[] => + findings.map((finding) => + applyOptimisticFindingTriageRowUpdate(finding, input), + ); diff --git a/ui/lib/helper.ts b/ui/lib/helper.ts index 9674ea0d73..dd61b238c3 100644 --- a/ui/lib/helper.ts +++ b/ui/lib/helper.ts @@ -383,21 +383,6 @@ export const checkTaskStatus = async ( export const wait = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); -// Helper function to create dictionaries by type -export function createDict(type: string, data: any) { - const includedField = data?.included?.filter( - (item: { type: string }) => item.type === type, - ); - - if (!includedField || includedField.length === 0) { - return {}; - } - - return Object.fromEntries( - includedField.map((item: { id: string }) => [item.id, item]), - ); -} - export const parseStringify = (value: any) => JSON.parse(JSON.stringify(value)); export const convertFileToUrl = (file: File) => URL.createObjectURL(file); diff --git a/ui/lib/utils.ts b/ui/lib/utils.ts index c649ce1787..7a37948ac7 100644 --- a/ui/lib/utils.ts +++ b/ui/lib/utils.ts @@ -28,3 +28,39 @@ export function getOptionalText(value: unknown): string | undefined { ? value : undefined; } + +interface IncludedApiItemRelationshipRef { + id: string; +} + +interface IncludedApiItemRelationship { + data?: IncludedApiItemRelationshipRef; +} + +interface IncludedApiItem { + id: string; + type: string; + attributes?: Record; + relationships?: Record; +} + +// Indexes a JSON:API `included` array by id for the requested resource type. +export function createDict( + type: string, + data: { included?: unknown[] } | null | undefined, +): Record { + const includedField = data?.included?.filter( + (item): item is T => + typeof item === "object" && + item !== null && + (item as IncludedApiItem).type === type, + ); + + if (!includedField || includedField.length === 0) { + return {}; + } + + return Object.fromEntries( + includedField.map((item): [string, T] => [item.id, item]), + ); +}