fix(ui): enable triage editing in compliance findings table (#11829)

This commit is contained in:
Alejandro Bailo
2026-07-06 11:03:37 +02:00
committed by GitHub
parent efb86bb7ab
commit 441f2a3c48
15 changed files with 944 additions and 165 deletions
@@ -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,
@@ -594,22 +594,31 @@ const AlertFormModalContent = ({
{errors.root && (
<div className="text-text-error-primary text-sm">{errors.root}</div>
)}
<div className="flex justify-end gap-2">
<Button variant="outline" onClick={() => onOpenChange(false)}>
{/* mt-4 lifts the gap-4 container spacing to 32px so the distance to
the footer matches the launch scan and triage note modals. */}
<div className="mt-4 flex w-full justify-between gap-4">
<Button
variant="outline"
size="lg"
onClick={() => onOpenChange(false)}
>
Cancel
</Button>
{editingAlert && (
<Button
variant="outline"
onClick={handlePreview}
disabled={previewLoading || saving}
>
{previewLoading ? "Running..." : "Test"}
<div className="flex gap-4">
{editingAlert && (
<Button
variant="outline"
size="lg"
onClick={handlePreview}
disabled={previewLoading || saving}
>
{previewLoading ? "Running..." : "Test"}
</Button>
)}
<Button size="lg" onClick={handleSubmit} disabled={saving}>
{submitLabel}
</Button>
)}
<Button onClick={handleSubmit} disabled={saving}>
{submitLabel}
</Button>
</div>
</div>
</div>
</Modal>
@@ -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");
});
});
@@ -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<FindingsResponse | null>(null);
const [expandedFindings, setExpandedFindings] = useState<FindingProps[]>([]);
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<string | null>(null);
const loadedPageSizeRef = useRef<string | null>(null);
const loadedSortRef = useRef<string | null>(null);
const loadedMutedRef = useRef<string | null>(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 = (
<div className="flex items-center px-2 text-sm">
<div className="w-full flex-col">
@@ -174,7 +131,26 @@ export const ClientAccordionContent = ({
];
const renderFindingsTable = () => {
if (findings === null && requirement.status !== "MANUAL") {
if (error) {
return (
<Alert variant="error" className="mt-3">
<AlertTriangle />
<AlertDescription className="flex flex-wrap items-center gap-2">
<span>{error}</span>
<Button
variant="link"
size="link-sm"
className="h-auto p-0"
onClick={reload}
>
Try again
</Button>
</AlertDescription>
</Alert>
);
}
if (isLoading && requirement.status !== "MANUAL") {
return <SkeletonTableFindings />;
}
@@ -184,8 +160,12 @@ export const ClientAccordionContent = ({
<h4 className="mb-2 text-sm font-medium">Findings</h4>
<DataTable
columns={getStandaloneFindingColumns({ openFindingId })}
data={expandedFindings || []}
columns={getStandaloneFindingColumns({
openFindingId,
onTriageUpdateAction: handleTriageUpdate,
onTriageNoteLoadAction: loadLatestFindingTriageNote,
})}
data={expandedFindings}
metadata={findings?.meta}
disableScroll={true}
/>
@@ -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<string, unknown>) {
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);
});
});
@@ -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<FindingsResponse | null>(null);
const [expandedFindings, setExpandedFindings] = useState<FindingProps[]>([]);
const [error, setError] = useState<string | null>(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,
};
}
@@ -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(() =>
@@ -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. */}
<form className="flex min-w-0 flex-col gap-5" onSubmit={handleSubmit}>
<div className="flex items-center gap-4">
<div className="bg-bg-neutral-tertiary flex size-9 shrink-0 items-center justify-center rounded-lg">
<div className="text-text-neutral-secondary flex flex-wrap items-center gap-2 text-sm">
<Info className="size-4 shrink-0" />
<span>Learn how triage states work in the</span>
<Button variant="link" size="link-sm" className="h-auto p-0" asChild>
<a
href={DOCS_URLS.FINDINGS_TRIAGE}
target="_blank"
rel="noopener noreferrer"
>
<ExternalLink className="size-3.5 shrink-0" />
<span>Triage documentation</span>
</a>
</Button>
</div>
<div className="border-border-input-primary flex items-center gap-4 rounded-lg border p-3">
<div className="bg-bg-neutral-tertiary flex size-9 shrink-0 items-center justify-center overflow-hidden rounded-lg">
{findingContext.providerType ? (
<ProviderTypeIcon type={findingContext.providerType} size={22} />
<ProviderTypeIcon type={findingContext.providerType} size={36} />
) : (
<span className="text-text-neutral-secondary text-xs font-semibold">
{findingContext.provider?.slice(0, 3).toUpperCase() ?? "—"}
@@ -192,12 +207,7 @@ export function FindingNoteModal({
{shouldShowRemediatingInfo && (
<Alert variant="info">
<AlertDescription>
{REMEDIATING_INFO_COPY}.{" "}
<CustomLink href={DOCS_URLS.FINDINGS_TRIAGE} size="sm">
Learn more
</CustomLink>
</AlertDescription>
<AlertDescription>{REMEDIATING_INFO_COPY}.</AlertDescription>
</Alert>
)}
@@ -225,10 +235,12 @@ export function FindingNoteModal({
</div>
</div>
<div className="flex w-full justify-end gap-3">
{/* mt-3 lifts the gap-5 form spacing to 32px so the distance to the
footer matches the launch scan and alert modals. */}
<div className="mt-3 flex w-full justify-between gap-4">
<Button
type="button"
variant="ghost"
variant="outline"
size="lg"
onClick={() => onOpenChange(false)}
>
@@ -248,7 +260,7 @@ export function FindingNoteModal({
{isSubmitting
? "Saving..."
: canSubmit || isCloudOnly
? "Save changes"
? "Save"
: "Unavailable"}
</Button>
</span>
@@ -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");
@@ -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),
);
};
+2 -2
View File
@@ -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(
+222
View File
@@ -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>,
): 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>): 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);
});
});
+46
View File
@@ -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),
);
-15
View File
@@ -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);
+36
View File
@@ -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<string, unknown>;
relationships?: Record<string, IncludedApiItemRelationship>;
}
// Indexes a JSON:API `included` array by id for the requested resource type.
export function createDict<T extends IncludedApiItem = IncludedApiItem>(
type: string,
data: { included?: unknown[] } | null | undefined,
): Record<string, T> {
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]),
);
}