fix(ui): resolve triage note modal test and address review feedback

ui/app/(prowler)/_overview/graphs-tabs/findings-view/findings-view.ssr.tsx

@@ -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,

ui/components/compliance/compliance-accordion/client-accordion-content.test.ts

@@ -31,4 +31,12 @@ describe("client accordion content", () => {
     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");
+  });
 });

ui/components/compliance/compliance-accordion/client-accordion-content.tsx

@@ -11,7 +11,7 @@ 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 { FINDINGS_DEFAULT_SORT, MUTED_FILTER } from "@/lib";
@@ -50,8 +50,14 @@ export const ClientAccordionContent = ({
 
   const checks = requirement.check_ids || [];
 
-  const { findings, expandedFindings, patchTriageUpdate, reload } =
-    useRequirementFindings({
+  const {
+    findings,
+    expandedFindings,
+    isLoading,
+    error,
+    patchTriageUpdate,
+    reload,
+  } = useRequirementFindings({
     enabled:
       !disableFindings &&
       checks.length > 0 &&
@@ -125,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 />;
     }
 

ui/components/compliance/compliance-accordion/use-requirement-findings.test.ts

@@ -122,6 +122,97 @@ describe("useRequirementFindings", () => {
     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), {

ui/components/compliance/compliance-accordion/use-requirement-findings.ts

@@ -22,10 +22,14 @@ interface UseRequirementFindingsOptions {
 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,
@@ -38,20 +42,26 @@ export function useRequirementFindings({
 }: 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 (!enabled || !checkIdsKey) {
+    if (!isFetchEnabled) {
       return;
     }
 
     let cancelled = false;
 
     const loadFindings = async () => {
+      setError(null);
       try {
         const findingsData = await getFindings({
           filters: {
@@ -80,7 +90,7 @@ export function useRequirementFindings({
               const resource =
                 resourceDict[finding.relationships?.resources?.data?.[0]?.id];
               const provider =
-                providerDict[scan?.relationships?.provider?.data?.id];
+                providerDict[scan?.relationships?.provider?.data?.id ?? ""];
 
               return {
                 ...finding,
@@ -93,6 +103,7 @@ export function useRequirementFindings({
       } catch (error) {
         if (!cancelled) {
           console.error("Error loading findings:", error);
+          setError(FINDINGS_LOAD_ERROR);
         }
       }
     };
@@ -103,7 +114,7 @@ export function useRequirementFindings({
       cancelled = true;
     };
   }, [
-    enabled,
+    isFetchEnabled,
     checkIdsKey,
     scanId,
     pageNumber,
@@ -124,5 +135,12 @@ export function useRequirementFindings({
     setReloadNonce((value) => value + 1);
   };
 
-  return { findings, expandedFindings, patchTriageUpdate, reload };
+  return {
+    findings,
+    expandedFindings,
+    isLoading,
+    error,
+    patchTriageUpdate,
+    reload,
+  };
 }

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");

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(

ui/lib/finding-triage.test.ts

@@ -126,6 +126,82 @@ describe("finding triage optimistic row updates", () => {
     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 });

ui/lib/utils.ts

@@ -29,10 +29,31 @@ export function getOptionalText(value: unknown): string | undefined {
     : undefined;
 }
 
-// Helper function to create dictionaries by type
-export function createDict(type: string, data: any) {
+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: { type: string }) => item.type === type,
+    (item): item is T =>
+      typeof item === "object" &&
+      item !== null &&
+      (item as IncludedApiItem).type === type,
   );
 
   if (!includedField || includedField.length === 0) {
@@ -40,6 +61,6 @@ export function createDict(type: string, data: any) {
   }
 
   return Object.fromEntries(
-    includedField.map((item: { id: string }) => [item.id, item]),
+    includedField.map((item): [string, T] => [item.id, item]),
   );
 }