diff --git a/skills/prowler-pr/SKILL.md b/skills/prowler-pr/SKILL.md index c2d87716adc..e147d37fbe3 100644 --- a/skills/prowler-pr/SKILL.md +++ b/skills/prowler-pr/SKILL.md @@ -132,6 +132,18 @@ Follow conventional commits: 4. ✅ Branch is up to date with main 5. ✅ Commits are clean and descriptive +## Before Re-Requesting Review (REQUIRED) + +Resolve or respond to **every** open inline review thread before re-requesting review: + +1. **Agreed + fixed**: Commit the change. Reply with the commit hash so the reviewer can verify quickly: + > Fixed in `abc1234`. +2. **Agreed but deferred**: Explain why it's out of scope for this PR and where it's tracked. +3. **Disagreed**: Reply with clear technical reasoning. Do not leave threads silently open. +4. **Re-request review** only after all threads are in a clean state — either resolved or explicitly responded to. + +> **Rule of thumb**: A reviewer should never have to wonder "did they see my comment?" when they re-open the PR. + ## Resources - **Documentation**: See [references/](references/) for links to local developer guide diff --git a/skills/prowler-ui/SKILL.md b/skills/prowler-ui/SKILL.md index b1e68f81708..3e6407889e8 100644 --- a/skills/prowler-ui/SKILL.md +++ b/skills/prowler-ui/SKILL.md @@ -186,6 +186,109 @@ cd ui && pnpm run build cd ui && pnpm start ``` +## Batch vs Instant Component API (REQUIRED) + +When a component supports both **batch** (deferred, submit-based) and **instant** (immediate callback) behavior, model the coupling with a discriminated union — never as independent optionals. Coupled props must be all-or-nothing. + +```typescript +// ❌ NEVER: Independent optionals — allows invalid half-states +interface FilterProps { + onBatchApply?: (values: string[]) => void; + onInstantChange?: (value: string) => void; + isBatchMode?: boolean; +} + +// ✅ ALWAYS: Discriminated union — one valid shape per mode +type BatchProps = { + mode: "batch"; + onApply: (values: string[]) => void; + onCancel: () => void; +}; + +type InstantProps = { + mode: "instant"; + onChange: (value: string) => void; + // onApply/onCancel are forbidden here via structural exclusion + onApply?: never; + onCancel?: never; +}; + +type FilterProps = BatchProps | InstantProps; +``` + +This makes invalid prop combinations a compile error, not a runtime surprise. + +## Reuse Shared Display Utilities First (REQUIRED) + +Before adding **local** display maps (labels, provider names, status strings, category formatters), search `ui/types/*` and `ui/lib/*` for existing helpers. + +```typescript +// ✅ CHECK THESE FIRST before creating a new map: +// ui/lib/utils.ts → general formatters +// ui/types/providers.ts → provider display names, icons +// ui/types/findings.ts → severity/status display maps +// ui/types/compliance.ts → category/group formatters + +// ❌ NEVER add a local map that already exists: +const SEVERITY_LABELS: Record = { + critical: "Critical", + high: "High", + // ...duplicating an existing shared map +}; + +// ✅ Import and reuse instead: +import { severityLabel } from "@/types/findings"; +``` + +If a helper doesn't exist and will be used in 2+ places, add it to `ui/lib/` or `ui/types/` and reuse it. Keep local only if used in exactly one place. + +## Derived State Rule (REQUIRED) + +Avoid `useState` + `useEffect` patterns that mirror props or searchParams — they create sync bugs and unnecessary re-renders. Derive values directly from the source of truth. + +```typescript +// ❌ NEVER: Mirror props into state via effect +const [localFilter, setLocalFilter] = useState(filter); +useEffect(() => { setLocalFilter(filter); }, [filter]); + +// ✅ ALWAYS: Derive directly +const localFilter = filter; // or compute inline +``` + +If local state is genuinely needed (e.g., optimistic UI, pending edits before submit), add a short comment: + +```typescript +// Local state needed: user edits are buffered until "Apply" is clicked +const [pending, setPending] = useState(initialValues); +``` + +## Strict Key Typing for Label Maps (REQUIRED) + +Avoid `Record` when the key set is known. Use an explicit union type or a const-key object so typos are caught at compile time. + +```typescript +// ❌ Loose — typos compile silently +const STATUS_LABELS: Record = { + actve: "Active", // typo, no error +}; + +// ✅ Tight — union key +type Status = "active" | "inactive" | "pending"; +const STATUS_LABELS: Record = { + active: "Active", + inactive: "Inactive", + pending: "Pending", + // actve: "Active" ← compile error +}; + +// ✅ Also fine — const satisfies +const STATUS_LABELS = { + active: "Active", + inactive: "Inactive", + pending: "Pending", +} as const satisfies Record; +``` + ## QA Checklist Before Commit - [ ] `pnpm run typecheck` passes @@ -199,6 +302,15 @@ cd ui && pnpm start - [ ] Accessibility: keyboard navigation, ARIA labels - [ ] Mobile responsive (if applicable) +## Pre-Re-Review Checklist (Review Thread Hygiene) + +Before requesting re-review from a reviewer: + +- [ ] Every unresolved inline thread has been either fixed or explicitly answered with a rationale +- [ ] If you agreed with a comment: the change is committed and the commit hash is mentioned in the reply +- [ ] If you disagreed: the reply explains why with clear reasoning — do not leave threads silently open +- [ ] Re-request review only after all threads are in a clean state + ## Migrations Reference | From | To | Key Changes | diff --git a/skills/typescript/SKILL.md b/skills/typescript/SKILL.md index 046465a75b5..4d664d31890 100644 --- a/skills/typescript/SKILL.md +++ b/skills/typescript/SKILL.md @@ -102,6 +102,38 @@ function isUser(value: unknown): value is User { } ``` +## Coupled Optional Props (REQUIRED) + +Do not model semantically coupled props as independent optionals — this allows invalid half-states that compile but break at runtime. Use discriminated unions with `never` to make invalid combinations impossible. + +```typescript +// ❌ BEFORE: Independent optionals — half-states allowed +interface PaginationProps { + onPageChange?: (page: number) => void; + pageSize?: number; + currentPage?: number; +} + +// ✅ AFTER: Discriminated union — shape is all-or-nothing +type ControlledPagination = { + controlled: true; + currentPage: number; + pageSize: number; + onPageChange: (page: number) => void; +}; + +type UncontrolledPagination = { + controlled: false; + currentPage?: never; + pageSize?: never; + onPageChange?: never; +}; + +type PaginationProps = ControlledPagination | UncontrolledPagination; +``` + +**Key rule:** If two or more props are only meaningful together, they belong to the same discriminated union branch. Mixing them as independent optionals shifts correctness responsibility from the type system to runtime guards. + ## Import Types ```typescript diff --git a/ui/CHANGELOG.md b/ui/CHANGELOG.md index 4e1a50d510d..c66da3952c6 100644 --- a/ui/CHANGELOG.md +++ b/ui/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to the **Prowler UI** are documented in this file. ### 🔄 Changed +- Findings filters now use a batch-apply pattern with an Apply Filters button, filter summary strip, and independent filter options instead of triggering API calls on every selection - Google Workspace provider support [(#10333)](https://github.com/prowler-cloud/prowler/pull/10333) - Image (Container Registry) provider support in UI: badge icon, credentials form, and provider-type filtering [(#10167)](https://github.com/prowler-cloud/prowler/pull/10167) - Organization and organizational unit row actions (Edit Name, Update Credentials, Test Connections, Delete) in providers table dropdown [(#10317)](https://github.com/prowler-cloud/prowler/pull/10317) diff --git a/ui/app/(prowler)/_overview/_components/accounts-selector.tsx b/ui/app/(prowler)/_overview/_components/accounts-selector.tsx index 68ffec27067..362396f03fa 100644 --- a/ui/app/(prowler)/_overview/_components/accounts-selector.tsx +++ b/ui/app/(prowler)/_overview/_components/accounts-selector.tsx @@ -27,7 +27,11 @@ import { MultiSelectValue, } from "@/components/shadcn/select/multiselect"; import { useUrlFilters } from "@/hooks/use-url-filters"; -import type { ProviderProps, ProviderType } from "@/types/providers"; +import { + getProviderDisplayName, + type ProviderProps, + type ProviderType, +} from "@/types/providers"; const PROVIDER_ICON: Record = { aws: , @@ -46,60 +50,73 @@ const PROVIDER_ICON: Record = { openstack: , }; -interface AccountsSelectorProps { +/** Common props shared by both batch and instant modes. */ +interface AccountsSelectorBaseProps { providers: ProviderProps[]; + /** + * Currently selected provider types (from the pending ProviderTypeSelector state). + * Used only for contextual description/empty-state messaging — does NOT narrow + * the list of available accounts, which remains independent of provider selection. + */ + selectedProviderTypes?: string[]; +} + +/** Batch mode: caller controls both pending state and notification callback (all-or-nothing). */ +interface AccountsSelectorBatchProps extends AccountsSelectorBaseProps { + /** + * Called instead of navigating immediately. + * Use this on pages that batch filter changes (e.g. Findings). + * + * @param filterKey - The raw filter key without "filter[]" wrapper, e.g. "provider_id__in" + * @param values - The selected values array + */ + onBatchChange: (filterKey: string, values: string[]) => void; + /** + * Pending selected values controlled by the parent. + * Reflects pending state before Apply is clicked. + */ + selectedValues: string[]; } -export function AccountsSelector({ providers }: AccountsSelectorProps) { +/** Instant mode: URL-driven — neither callback nor controlled value. */ +interface AccountsSelectorInstantProps extends AccountsSelectorBaseProps { + onBatchChange?: never; + selectedValues?: never; +} + +type AccountsSelectorProps = + | AccountsSelectorBatchProps + | AccountsSelectorInstantProps; + +export function AccountsSelector({ + providers, + onBatchChange, + selectedValues, + selectedProviderTypes, +}: AccountsSelectorProps) { const searchParams = useSearchParams(); const { navigateWithParams } = useUrlFilters(); const filterKey = "filter[provider_id__in]"; const current = searchParams.get(filterKey) || ""; - const selectedTypes = searchParams.get("filter[provider_type__in]") || ""; - const selectedTypesList = selectedTypes - ? selectedTypes.split(",").filter(Boolean) - : []; - const selectedIds = current ? current.split(",").filter(Boolean) : []; - const visibleProviders = providers - // .filter((p) => p.attributes.connection?.connected) - .filter((p) => - selectedTypesList.length > 0 - ? selectedTypesList.includes(p.attributes.provider) - : true, - ); + const urlSelectedIds = current ? current.split(",").filter(Boolean) : []; + + // In batch mode, use the parent-controlled pending values; otherwise, use URL state. + const selectedIds = onBatchChange ? selectedValues : urlSelectedIds; + const visibleProviders = providers; + // .filter((p) => p.attributes.connection?.connected) const handleMultiValueChange = (ids: string[]) => { + if (onBatchChange) { + onBatchChange("provider_id__in", ids); + return; + } navigateWithParams((params) => { params.delete(filterKey); if (ids.length > 0) { params.set(filterKey, ids.join(",")); } - - // Auto-deselect provider types that no longer have any selected accounts - if (selectedTypesList.length > 0) { - // Get provider types of currently selected accounts - const selectedProviders = providers.filter((p) => ids.includes(p.id)); - const selectedProviderTypes = new Set( - selectedProviders.map((p) => p.attributes.provider), - ); - - // Keep only provider types that still have selected accounts - const remainingProviderTypes = selectedTypesList.filter((type) => - selectedProviderTypes.has(type as ProviderType), - ); - - // Update provider_type__in filter - if (remainingProviderTypes.length > 0) { - params.set( - "filter[provider_type__in]", - remainingProviderTypes.join(","), - ); - } else { - params.delete("filter[provider_type__in]"); - } - } }); }; @@ -115,9 +132,12 @@ export function AccountsSelector({ providers }: AccountsSelectorProps) { ); }; + // Build a contextual description based on currently selected provider types. + // This is purely for user guidance (aria label + empty state) and does NOT + // narrow the list of available accounts — all providers remain selectable. const filterDescription = - selectedTypesList.length > 0 - ? `Showing accounts for ${selectedTypesList.join(", ")} providers` + selectedProviderTypes && selectedProviderTypes.length > 0 + ? `Accounts for ${selectedProviderTypes.map(getProviderDisplayName).join(", ")}` : "All connected cloud provider accounts"; return ( @@ -176,8 +196,8 @@ export function AccountsSelector({ providers }: AccountsSelectorProps) { ) : (
- {selectedTypesList.length > 0 - ? "No accounts available for selected providers" + {selectedProviderTypes && selectedProviderTypes.length > 0 + ? `No accounts available for ${selectedProviderTypes.map(getProviderDisplayName).join(", ")}` : "No connected accounts available"}
)} diff --git a/ui/app/(prowler)/_overview/_components/provider-type-selector.tsx b/ui/app/(prowler)/_overview/_components/provider-type-selector.tsx index 6c62a34667b..74d728fd746 100644 --- a/ui/app/(prowler)/_overview/_components/provider-type-selector.tsx +++ b/ui/app/(prowler)/_overview/_components/provider-type-selector.tsx @@ -152,22 +152,60 @@ const PROVIDER_DATA: Record< }, }; -type ProviderTypeSelectorProps = { +/** Common props shared by both batch and instant modes. */ +interface ProviderTypeSelectorBaseProps { providers: ProviderProps[]; -}; +} + +/** Batch mode: caller controls both pending state and notification callback (all-or-nothing). */ +interface ProviderTypeSelectorBatchProps extends ProviderTypeSelectorBaseProps { + /** + * Called instead of navigating immediately. + * Use this on pages that batch filter changes (e.g. Findings). + * + * @param filterKey - The raw filter key without "filter[]" wrapper, e.g. "provider_type__in" + * @param values - The selected values array + */ + onBatchChange: (filterKey: string, values: string[]) => void; + /** + * Pending selected values controlled by the parent. + * Reflects pending state before Apply is clicked. + */ + selectedValues: string[]; +} + +/** Instant mode: URL-driven — neither callback nor controlled value. */ +interface ProviderTypeSelectorInstantProps + extends ProviderTypeSelectorBaseProps { + onBatchChange?: never; + selectedValues?: never; +} + +type ProviderTypeSelectorProps = + | ProviderTypeSelectorBatchProps + | ProviderTypeSelectorInstantProps; export const ProviderTypeSelector = ({ providers, + onBatchChange, + selectedValues, }: ProviderTypeSelectorProps) => { const searchParams = useSearchParams(); const { navigateWithParams } = useUrlFilters(); const currentProviders = searchParams.get("filter[provider_type__in]") || ""; - const selectedTypes = currentProviders + const urlSelectedTypes = currentProviders ? currentProviders.split(",").filter(Boolean) : []; + // In batch mode, use the parent-controlled pending values; otherwise, use URL state. + const selectedTypes = onBatchChange ? selectedValues : urlSelectedTypes; + const handleMultiValueChange = (values: string[]) => { + if (onBatchChange) { + onBatchChange("provider_type__in", values); + return; + } navigateWithParams((params) => { // Update provider_type__in if (values.length > 0) { @@ -175,10 +213,6 @@ export const ProviderTypeSelector = ({ } else { params.delete("filter[provider_type__in]"); } - - // Clear account selection when changing provider types - // User should manually select accounts if they want to filter by specific accounts - params.delete("filter[provider_id__in]"); }); }; diff --git a/ui/app/(prowler)/findings/page.tsx b/ui/app/(prowler)/findings/page.tsx index 91b2bffc045..30a258e61bb 100644 --- a/ui/app/(prowler)/findings/page.tsx +++ b/ui/app/(prowler)/findings/page.tsx @@ -24,10 +24,6 @@ import { extractSortAndKey, hasDateOrScanFilter, } from "@/lib"; -import { - createProviderDetailsMappingById, - extractProviderIds, -} from "@/lib/provider-helpers"; import { ScanEntity, ScanProps } from "@/types"; import { FindingProps, SearchParamsProps } from "@/types/components"; @@ -124,12 +120,6 @@ export default async function Findings({ const uniqueCategories = metadataInfoData?.data?.attributes?.categories || []; const uniqueGroups = metadataInfoData?.data?.attributes?.groups || []; - // Extract provider IDs and details using helper functions - const providerIds = providersData ? extractProviderIds(providersData) : []; - const providerDetails = providersData - ? createProviderDetailsMappingById(providerIds, providersData) - : []; - // Extract scan UUIDs with "completed" state and more than one resource const completedScans = scansData?.data?.filter( (scan: ScanProps) => @@ -151,9 +141,6 @@ export default async function Findings({
({ + Check: () => , + X: () => , +})); + +// Mock @/components/shadcn to avoid next-auth import chain +vi.mock("@/components/shadcn", () => ({ + Button: ({ + children, + disabled, + onClick, + "aria-label": ariaLabel, + variant, + size, + }: { + children?: React.ReactNode; + disabled?: boolean; + onClick?: () => void; + "aria-label"?: string; + variant?: string; + size?: string; + }) => ( + + ), +})); + +vi.mock("@/lib/utils", () => ({ + cn: (...classes: (string | undefined | false)[]) => + classes.filter(Boolean).join(" "), +})); + +import { ApplyFiltersButton } from "@/components/filters/apply-filters-button"; + +// ── Future E2E coverage ──────────────────────────────────────────────────── +// TODO (E2E): Full apply-filters button flow should be covered in Playwright tests: +// - Button appears disabled when no filters have been staged +// - Button shows correct count after staging multiple filters +// - Clicking Apply pushes all pending filters to the URL in one navigation event +// - Clicking Discard resets pending state to current URL state (staged filters disappear) +// ────────────────────────────────────────────────────────────────────────── + +describe("ApplyFiltersButton", () => { + // ── No changes ─────────────────────────────────────────────────────────── + + describe("when hasChanges is false", () => { + it("should render the Apply Filters button as disabled", () => { + // Given / When + render( + , + ); + + // Then + const applyButton = screen.getByRole("button", { + name: "Apply Filters", + }); + expect(applyButton).toBeDisabled(); + }); + + it("should NOT render the discard (X) button when there are no changes", () => { + // Given / When + render( + , + ); + + // Then + expect( + screen.queryByRole("button", { + name: /discard/i, + }), + ).not.toBeInTheDocument(); + }); + + it("should show 'Apply Filters' label without count", () => { + // Given / When + render( + , + ); + + // Then + expect( + screen.getByRole("button", { name: "Apply Filters" }), + ).toBeInTheDocument(); + }); + }); + + // ── Has changes ────────────────────────────────────────────────────────── + + describe("when hasChanges is true", () => { + it("should render the Apply Filters button as enabled", () => { + // Given / When + render( + , + ); + + // Then + const applyButton = screen.getByRole("button", { + name: "Apply Filters (2)", + }); + expect(applyButton).not.toBeDisabled(); + }); + + it("should show the change count in the button label", () => { + // Given / When + render( + , + ); + + // Then + expect( + screen.getByRole("button", { name: "Apply Filters (3)" }), + ).toBeInTheDocument(); + }); + + it("should show 'Apply Filters' (without count) when changeCount is 0 but hasChanges is true", () => { + // Given — hasChanges=true but changeCount=0 (edge case) + render( + , + ); + + // Then + expect( + screen.getByRole("button", { name: "Apply Filters" }), + ).toBeInTheDocument(); + }); + + it("should render the discard (X) button", () => { + // Given / When + render( + , + ); + + // Then + expect( + screen.getByRole("button", { name: /discard pending filter changes/i }), + ).toBeInTheDocument(); + }); + }); + + // ── onApply interaction ────────────────────────────────────────────────── + + describe("onApply", () => { + it("should call onApply when the Apply Filters button is clicked", async () => { + // Given + const user = userEvent.setup(); + const onApply = vi.fn(); + const onDiscard = vi.fn(); + + render( + , + ); + + // When + await user.click( + screen.getByRole("button", { name: "Apply Filters (1)" }), + ); + + // Then + expect(onApply).toHaveBeenCalledTimes(1); + expect(onDiscard).not.toHaveBeenCalled(); + }); + + it("should NOT call onApply when the button is disabled", async () => { + // Given + const user = userEvent.setup(); + const onApply = vi.fn(); + + render( + , + ); + + // When + await user.click(screen.getByRole("button", { name: "Apply Filters" })); + + // Then — disabled button should not fire + expect(onApply).not.toHaveBeenCalled(); + }); + }); + + // ── onDiscard interaction ──────────────────────────────────────────────── + + describe("onDiscard", () => { + it("should call onDiscard when the Discard button is clicked", async () => { + // Given + const user = userEvent.setup(); + const onApply = vi.fn(); + const onDiscard = vi.fn(); + + render( + , + ); + + // When + await user.click( + screen.getByRole("button", { name: /discard pending filter changes/i }), + ); + + // Then + expect(onDiscard).toHaveBeenCalledTimes(1); + expect(onApply).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/ui/components/filters/apply-filters-button.tsx b/ui/components/filters/apply-filters-button.tsx new file mode 100644 index 00000000000..be0fa183a73 --- /dev/null +++ b/ui/components/filters/apply-filters-button.tsx @@ -0,0 +1,64 @@ +"use client"; + +import { Check, X } from "lucide-react"; + +import { Button } from "@/components/shadcn"; +import { cn } from "@/lib/utils"; + +export interface ApplyFiltersButtonProps { + /** Whether there are pending changes that differ from the applied (URL) state */ + hasChanges: boolean; + /** Number of filter keys that have pending changes */ + changeCount: number; + /** Called when the user clicks "Apply Filters" */ + onApply: () => void; + /** Called when the user clicks the discard (X) action */ + onDiscard: () => void; + /** Optional extra class names for the outer wrapper */ + className?: string; +} + +/** + * Displays an "Apply Filters" button with an optional discard action. + * + * - Shows the count of pending changes when `hasChanges` is true. + * - The apply button is disabled (and visually muted) when there are no changes. + * - The discard (X) button only appears when there are pending changes. + * - Uses Prowler's shadcn `Button` component. + */ +export const ApplyFiltersButton = ({ + hasChanges, + changeCount, + onApply, + onDiscard, + className, +}: ApplyFiltersButtonProps) => { + const label = + changeCount > 0 ? `Apply Filters (${changeCount})` : "Apply Filters"; + + return ( +
+ + + {hasChanges && ( + + )} +
+ ); +}; diff --git a/ui/components/filters/clear-filters-button.tsx b/ui/components/filters/clear-filters-button.tsx index 40dbaf92163..0cc586cc705 100644 --- a/ui/components/filters/clear-filters-button.tsx +++ b/ui/components/filters/clear-filters-button.tsx @@ -17,6 +17,19 @@ export interface ClearFiltersButtonProps { showCount?: boolean; /** Use link style (text only, no button background) */ variant?: "link" | "default"; + /** + * Optional callback for batch mode. When provided, this is called INSTEAD + * of pushing URL params directly. Useful for clearing pending filter state + * without immediately navigating. + */ + onClear?: () => void; + /** + * In batch mode, the number of pending filter keys that have non-empty values. + * When provided alongside `onClear`, overrides the URL-based count shown by + * `showCount`. This ensures the displayed count reflects the pending state + * (not the last-applied URL state) while the user is editing filters. + */ + pendingCount?: number; } export const ClearFiltersButton = ({ @@ -24,6 +37,8 @@ export const ClearFiltersButton = ({ ariaLabel = "Reset", showCount = false, variant = "link", + onClear, + pendingCount, }: ClearFiltersButtonProps) => { const router = useRouter(); const pathname = usePathname(); @@ -51,17 +66,27 @@ export const ClearFiltersButton = ({ router.push(`${pathname}?${params.toString()}`, { scroll: false }); }, [router, searchParams, pathname]); - // Only show button if there are filters other than the excluded ones - if (filterCount === 0) { + // In batch mode: use pendingCount if provided; otherwise fall back to URL count. + // In instant mode: always use URL count. + const displayCount = + onClear && pendingCount !== undefined ? pendingCount : filterCount; + + // In instant mode: hide when no URL filters exist + if (!onClear && filterCount === 0) { + return null; + } + + // In batch mode: hide when there are no pending or URL filters to clear + if (onClear && displayCount === 0) { return null; } - const displayText = showCount ? `Clear Filters (${filterCount})` : text; + const displayText = showCount ? `Clear Filters (${displayCount})` : text; return ( + + ))} + + +
+ ); +}; diff --git a/ui/components/filters/index.ts b/ui/components/filters/index.ts index 7e2de31cd76..05167beb028 100644 --- a/ui/components/filters/index.ts +++ b/ui/components/filters/index.ts @@ -1,6 +1,8 @@ +export * from "./apply-filters-button"; export * from "./clear-filters-button"; export * from "./custom-checkbox-muted-findings"; export * from "./custom-date-picker"; export * from "./custom-provider-inputs"; export * from "./data-filters"; export * from "./filter-controls"; +export * from "./filter-summary-strip"; diff --git a/ui/components/findings/findings-filters.tsx b/ui/components/findings/findings-filters.tsx index 857a49f958c..9d7a7225ba4 100644 --- a/ui/components/findings/findings-filters.tsx +++ b/ui/components/findings/findings-filters.tsx @@ -5,24 +5,28 @@ import { useState } from "react"; import { AccountsSelector } from "@/app/(prowler)/_overview/_components/accounts-selector"; import { ProviderTypeSelector } from "@/app/(prowler)/_overview/_components/provider-type-selector"; +import { ApplyFiltersButton } from "@/components/filters/apply-filters-button"; import { ClearFiltersButton } from "@/components/filters/clear-filters-button"; import { CustomCheckboxMutedFindings } from "@/components/filters/custom-checkbox-muted-findings"; import { CustomDatePicker } from "@/components/filters/custom-date-picker"; import { filterFindings } from "@/components/filters/data-filters"; +import { + FilterChip, + FilterSummaryStrip, +} from "@/components/filters/filter-summary-strip"; import { Button } from "@/components/shadcn"; import { ExpandableSection } from "@/components/ui/expandable-section"; import { DataTableFilterCustom } from "@/components/ui/table"; -import { useRelatedFilters } from "@/hooks"; -import { getCategoryLabel, getGroupLabel } from "@/lib/categories"; -import { FilterEntity, FilterType, ScanEntity, ScanProps } from "@/types"; -import { ProviderProps } from "@/types/providers"; +import { useFilterBatch } from "@/hooks/use-filter-batch"; +import { formatLabel, getCategoryLabel, getGroupLabel } from "@/lib/categories"; +import { FilterType, FINDING_STATUS_DISPLAY_NAMES, ScanEntity } from "@/types"; +import { DATA_TABLE_FILTER_MODE, FilterParam } from "@/types/filters"; +import { getProviderDisplayName, ProviderProps } from "@/types/providers"; +import { SEVERITY_DISPLAY_NAMES } from "@/types/severities"; interface FindingsFiltersProps { /** Provider data for ProviderTypeSelector and AccountsSelector */ providers: ProviderProps[]; - providerIds: string[]; - providerDetails: { [id: string]: FilterEntity }[]; - completedScans: ScanProps[]; completedScanIds: string[]; scanDetails: { [key: string]: ScanEntity }[]; uniqueRegions: string[]; @@ -32,10 +36,73 @@ interface FindingsFiltersProps { uniqueGroups: string[]; } +/** + * Maps raw filter param keys (e.g. "filter[severity__in]") to human-readable labels. + * Used to render chips in the FilterSummaryStrip. + * Typed as Record so TypeScript enforces exhaustiveness — any + * addition to FilterParam will cause a compile error here if the label is missing. + */ +const FILTER_KEY_LABELS: Record = { + "filter[provider_type__in]": "Provider", + "filter[provider_id__in]": "Account", + "filter[severity__in]": "Severity", + "filter[status__in]": "Status", + "filter[delta__in]": "Delta", + "filter[region__in]": "Region", + "filter[service__in]": "Service", + "filter[resource_type__in]": "Resource Type", + "filter[category__in]": "Category", + "filter[resource_groups__in]": "Resource Group", + "filter[scan__in]": "Scan ID", + "filter[inserted_at]": "Date", + "filter[muted]": "Muted", +}; + +/** + * Formats a raw filter value into a human-readable display string. + * - Provider types: uses shared getProviderDisplayName utility + * - Severities: uses shared SEVERITY_DISPLAY_NAMES (e.g. "critical" → "Critical") + * - Status: uses shared FINDING_STATUS_DISPLAY_NAMES (e.g. "FAIL" → "Fail") + * - Categories: uses getCategoryLabel (handles IAM, EC2, IMDSv1, etc.) + * - Resource groups: uses getGroupLabel (underscore-delimited) + * - Date (filter[inserted_at]): returns the ISO date string as-is (YYYY-MM-DD) + * - Other values: uses formatLabel as a generic fallback (avoids naive capitalisation) + */ +const formatFilterValue = (filterKey: string, value: string): string => { + if (!value) return value; + if (filterKey === "filter[provider_type__in]") { + return getProviderDisplayName(value); + } + if (filterKey === "filter[severity__in]") { + return ( + SEVERITY_DISPLAY_NAMES[ + value.toLowerCase() as keyof typeof SEVERITY_DISPLAY_NAMES + ] ?? formatLabel(value) + ); + } + if (filterKey === "filter[status__in]") { + return ( + FINDING_STATUS_DISPLAY_NAMES[ + value as keyof typeof FINDING_STATUS_DISPLAY_NAMES + ] ?? formatLabel(value) + ); + } + if (filterKey === "filter[category__in]") { + return getCategoryLabel(value); + } + if (filterKey === "filter[resource_groups__in]") { + return getGroupLabel(value); + } + // Date filter: preserve ISO date string (YYYY-MM-DD) — do not run through formatLabel + if (filterKey === "filter[inserted_at]") { + return value; + } + // Generic fallback: handles hyphen/underscore-delimited IDs with smart capitalisation + return formatLabel(value); +}; + export const FindingsFilters = ({ providers, - providerIds, - providerDetails, completedScanIds, scanDetails, uniqueRegions, @@ -46,12 +113,17 @@ export const FindingsFilters = ({ }: FindingsFiltersProps) => { const [isExpanded, setIsExpanded] = useState(false); - const { availableScans } = useRelatedFilters({ - providerIds, - providerDetails, - completedScanIds, - scanDetails, - enableScanRelation: true, + const { + pendingFilters, + setPending, + applyAll, + discardAll, + clearAll, + hasChanges, + changeCount, + getFilterValue, + } = useFilterBatch({ + defaultParams: { "filter[muted]": "false" }, }); // Custom filters for the expandable section (removed Provider - now using AccountsSelector) @@ -92,7 +164,7 @@ export const FindingsFilters = ({ { key: FilterType.SCAN, labelCheckboxGroup: "Scan ID", - values: availableScans, + values: completedScanIds, valueLabelMapping: scanDetails, index: 7, }, @@ -100,17 +172,72 @@ export const FindingsFilters = ({ const hasCustomFilters = customFilters.length > 0; + // Build FilterChip[] from pendingFilters — one chip per individual value, not per key. + // Skip filter[muted]="false" — it is the silent default and should not appear as a chip. + const filterChips: FilterChip[] = []; + Object.entries(pendingFilters).forEach(([key, values]) => { + if (!values || values.length === 0) return; + const label = FILTER_KEY_LABELS[key as FilterParam] ?? key; + values.forEach((value) => { + // Do not show a chip for the default muted=false state + if (key === "filter[muted]" && value === "false") return; + filterChips.push({ + key, + label, + value, + displayValue: formatFilterValue(key, value), + }); + }); + }); + + // Handler for removing a single chip: update the pending filter to remove that value. + // setPending handles both "filter[key]" and "key" formats internally. + const handleChipRemove = (filterKey: string, value: string) => { + const currentValues = pendingFilters[filterKey] ?? []; + const nextValues = currentValues.filter((v) => v !== value); + setPending(filterKey, nextValues); + }; + + // Derive pending muted state for the checkbox. + // Note: "filter[muted]" participates in batch mode — applyAll includes it + // when present in pending state, and the defaultParams option ensures + // filter[muted]=false is applied as a fallback when no muted value is pending. + const pendingMutedValue = pendingFilters["filter[muted]"]; + const mutedChecked = + pendingMutedValue !== undefined + ? pendingMutedValue[0] === "include" + : undefined; + + // For the date picker, read from pendingFilters + const pendingDateValues = pendingFilters["filter[inserted_at]"]; + const pendingDateValue = + pendingDateValues && pendingDateValues.length > 0 + ? pendingDateValues[0] + : undefined; + return (
- {/* First row: Provider selectors + Muted checkbox + More Filters button + Clear Filters */} + {/* First row: Provider selectors + Muted checkbox + More Filters button + Apply/Clear */}
- +
- +
- + setPending(filterKey, [value])} + checked={mutedChecked} + /> {hasCustomFilters && ( )} - + { + if (!values || values.length === 0) return false; + // filter[muted]=false is the silent default — don't count it as active + if ( + key === "filter[muted]" && + values.length === 1 && + values[0] === "false" + ) + return false; + return true; + }).length + } + /> +
+ {/* Summary strip: shown below filter bar when there are pending changes */} + + {/* Expandable filters section */} {hasCustomFilters && ( } + prependElement={ + + setPending(filterKey, value ? [value] : []) + } + value={pendingDateValue} + /> + } hideClearButton + mode={DATA_TABLE_FILTER_MODE.BATCH} + onBatchChange={setPending} + getFilterValue={getFilterValue} /> )} diff --git a/ui/components/ui/table/data-table-filter-custom-batch.test.tsx b/ui/components/ui/table/data-table-filter-custom-batch.test.tsx new file mode 100644 index 00000000000..8777b4c203c --- /dev/null +++ b/ui/components/ui/table/data-table-filter-custom-batch.test.tsx @@ -0,0 +1,278 @@ +import { render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +import { FilterOption } from "@/types/filters"; + +// ── next/navigation mock ──────────────────────────────────────────────────── +const mockPush = vi.fn(); +const mockUpdateFilter = vi.fn(); + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ push: mockPush }), + usePathname: () => "/findings", + useSearchParams: () => new URLSearchParams(), +})); + +// ── useUrlFilters mock — tracks whether updateFilter is called ─────────────── +vi.mock("@/hooks/use-url-filters", () => ({ + useUrlFilters: () => ({ updateFilter: mockUpdateFilter }), +})); + +// ── context (optional dependency used by useUrlFilters) ──────────────────── +vi.mock("@/contexts", () => ({ + useFilterTransitionOptional: () => null, +})); + +// ── MultiSelect mock — renders a simple { + const selected = Array.from(e.target.selectedOptions).map( + (o) => o.value, + ); + onValuesChange?.(selected); + }} + > + + + + +
+ ), + MultiSelectTrigger: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), + MultiSelectValue: ({ placeholder }: { placeholder: string }) => ( + {placeholder} + ), + MultiSelectContent: ({ children }: { children: React.ReactNode }) => ( + <>{children} + ), + MultiSelectSelectAll: ({ children }: { children: React.ReactNode }) => ( + + ), + MultiSelectSeparator: () =>
, + MultiSelectItem: ({ + children, + value, + }: { + children: React.ReactNode; + value: string; + }) => , +})); + +// ── ClearFiltersButton stub ───────────────────────────────────────────────── +vi.mock("@/components/filters/clear-filters-button", () => ({ + ClearFiltersButton: () => , +})); + +// ── Other component stubs ─────────────────────────────────────────────────── +vi.mock( + "@/components/compliance/compliance-header/compliance-scan-info", + () => ({ + ComplianceScanInfo: () => null, + }), +); +vi.mock("@/components/ui/entities/entity-info", () => ({ + EntityInfo: () => null, +})); +vi.mock("@/lib/helper-filters", () => ({ + isScanEntity: () => false, + isConnectionStatus: () => false, +})); + +import { DataTableFilterCustom } from "./data-table-filter-custom"; + +// ── Future E2E coverage ──────────────────────────────────────────────────── +// TODO (E2E): Integration tests for DataTableFilterCustom in batch mode: +// - In batch mode, selecting filters does NOT navigate the browser immediately +// - Multiple filter selections accumulate in pending state +// - Pressing Apply sends a single router.push with all staged filters +// - Pressing Discard reverts staged selections to match the current URL +// ────────────────────────────────────────────────────────────────────────── + +const severityFilter: FilterOption = { + key: "filter[severity__in]", + labelCheckboxGroup: "Severity", + values: ["critical", "high"], +}; + +describe("DataTableFilterCustom — batch vs instant mode", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + // ── Default / instant mode ─────────────────────────────────────────────── + + describe("instant mode (default)", () => { + it("should call updateFilter (URL update) when a selection changes", async () => { + // Given + const user = userEvent.setup(); + + render(); + + // When — simulate a value change on the mock select + const select = screen.getByTestId("multiselect-trigger"); + await user.selectOptions(select, ["critical"]); + + // Then — instant mode pushes to URL via updateFilter + expect(mockUpdateFilter).toHaveBeenCalledTimes(1); + expect(mockUpdateFilter).toHaveBeenCalledWith( + "filter[severity__in]", + expect.any(Array), + ); + }); + + it("should NOT call onBatchChange in instant mode", async () => { + // Given + const user = userEvent.setup(); + const onBatchChange = vi.fn(); + + render( + , + ); + + // When + const select = screen.getByTestId("multiselect-trigger"); + await user.selectOptions(select, ["critical"]); + + // Then + expect(onBatchChange).not.toHaveBeenCalled(); + }); + + it("should render without mode prop (backward compatibility)", () => { + // Given / When + render(); + + // Then — renders without crashing + expect(screen.getByText("Severity")).toBeInTheDocument(); + }); + }); + + // ── Batch mode ─────────────────────────────────────────────────────────── + + describe("batch mode", () => { + it("should call onBatchChange instead of updateFilter when selection changes", async () => { + // Given + const user = userEvent.setup(); + const onBatchChange = vi.fn(); + const getFilterValue = vi.fn().mockReturnValue([]); + + render( + , + ); + + // When + const select = screen.getByTestId("multiselect-trigger"); + await user.selectOptions(select, ["critical"]); + + // Then — batch mode notifies caller instead of URL + expect(onBatchChange).toHaveBeenCalledTimes(1); + expect(onBatchChange).toHaveBeenCalledWith( + "filter[severity__in]", + expect.any(Array), + ); + expect(mockUpdateFilter).not.toHaveBeenCalled(); + }); + + it("should read selected values from getFilterValue in batch mode", () => { + // Given — batch mode with pre-seeded pending state + const onBatchChange = vi.fn(); + const getFilterValue = vi + .fn() + .mockImplementation((key: string) => + key === "filter[severity__in]" ? ["critical"] : [], + ); + + render( + , + ); + + // Then — the mock multiselect receives the pending values + const multiselect = screen.getByTestId("multiselect"); + expect(multiselect).toHaveAttribute( + "data-values", + JSON.stringify(["critical"]), + ); + + // getFilterValue must have been called for the filter key + expect(getFilterValue).toHaveBeenCalledWith("filter[severity__in]"); + }); + + it("should pass empty array to MultiSelect when getFilterValue returns empty", () => { + // Given + const getFilterValue = vi.fn().mockReturnValue([]); + + render( + , + ); + + // Then — multiselect gets empty values + const multiselect = screen.getByTestId("multiselect"); + expect(multiselect).toHaveAttribute("data-values", JSON.stringify([])); + }); + }); + + // ── hideClearButton ────────────────────────────────────────────────────── + + describe("hideClearButton prop", () => { + it("should hide the ClearFiltersButton when hideClearButton is true", () => { + // Given / When + render( + , + ); + + // Then + expect( + screen.queryByRole("button", { name: "Clear" }), + ).not.toBeInTheDocument(); + }); + + it("should show the ClearFiltersButton by default", () => { + // Given / When + render(); + + // Then + expect(screen.getByRole("button", { name: "Clear" })).toBeInTheDocument(); + }); + }); +}); diff --git a/ui/components/ui/table/data-table-filter-custom.tsx b/ui/components/ui/table/data-table-filter-custom.tsx index 3b5a6701931..69d03ad630f 100644 --- a/ui/components/ui/table/data-table-filter-custom.tsx +++ b/ui/components/ui/table/data-table-filter-custom.tsx @@ -22,6 +22,7 @@ import { ProviderEntity, ScanEntity, } from "@/types"; +import { DATA_TABLE_FILTER_MODE, DataTableFilterMode } from "@/types/filters"; import { ProviderConnectionStatus } from "@/types/providers"; export interface DataTableFilterCustomProps { @@ -30,12 +31,33 @@ export interface DataTableFilterCustomProps { prependElement?: React.ReactNode; /** Hide the clear filters button and active badges (useful when parent manages this) */ hideClearButton?: boolean; + /** + * Controls when filter selections are pushed to the URL. + * - "instant" (default): each selection immediately updates the URL (legacy behavior, backward-compatible). + * - "batch": selections accumulate in pending state; caller manages when to push URL. + */ + mode?: DataTableFilterMode; + /** + * Called in "batch" mode when a filter value changes. + * The key is the raw filter key (e.g. "filter[severity__in]" or "severity__in"). + * Only invoked when mode === "batch". + */ + onBatchChange?: (filterKey: string, values: string[]) => void; + /** + * Returns the current selected values for a filter in "batch" mode. + * Replaces reading from URL searchParams when mode === "batch". + * Only used when mode === "batch". + */ + getFilterValue?: (filterKey: string) => string[]; } export const DataTableFilterCustom = ({ filters, prependElement, hideClearButton = false, + mode = DATA_TABLE_FILTER_MODE.INSTANT, + onBatchChange, + getFilterValue, }: DataTableFilterCustomProps) => { const { updateFilter } = useUrlFilters(); const searchParams = useSearchParams(); @@ -109,6 +131,13 @@ export const DataTableFilterCustom = ({ }; const pushDropdownFilter = (filter: FilterOption, values: string[]) => { + if (mode === DATA_TABLE_FILTER_MODE.BATCH && onBatchChange) { + // In batch mode, notify the caller instead of updating the URL + onBatchChange(filter.key, values); + return; + } + + // Instant mode (default): push to URL immediately // If this filter defaults to "all selected" and the user selected all items, // clear the URL param to represent "no specific filter" (i.e., all). const allSelected = @@ -123,6 +152,12 @@ export const DataTableFilterCustom = ({ }; const getSelectedValues = (filter: FilterOption): string[] => { + if (mode === DATA_TABLE_FILTER_MODE.BATCH && getFilterValue) { + // In batch mode, read from pending state provided by the caller + return getFilterValue(filter.key); + } + + // Instant mode (default): read from URL searchParams const filterKey = filter.key.startsWith("filter[") ? filter.key : `filter[${filter.key}]`; diff --git a/ui/hooks/use-filter-batch.test.ts b/ui/hooks/use-filter-batch.test.ts new file mode 100644 index 00000000000..00ac517719c --- /dev/null +++ b/ui/hooks/use-filter-batch.test.ts @@ -0,0 +1,588 @@ +import { act, renderHook } from "@testing-library/react"; +import { beforeEach, describe, expect, it, vi } from "vitest"; + +// --- Mock next/navigation --- +const mockPush = vi.fn(); +let mockSearchParamsValue = new URLSearchParams(); + +vi.mock("next/navigation", () => ({ + useRouter: () => ({ push: mockPush }), + usePathname: () => "/findings", + useSearchParams: () => mockSearchParamsValue, +})); + +import { useFilterBatch } from "./use-filter-batch"; + +/** + * Helper to re-assign the mocked searchParams and re-import the hook. + * Because useSearchParams() is called inside the hook on every render, + * we just update the module-level variable and force a re-render. + */ +function setSearchParams(params: Record) { + mockSearchParamsValue = new URLSearchParams(params); +} + +describe("useFilterBatch", () => { + beforeEach(() => { + vi.clearAllMocks(); + mockSearchParamsValue = new URLSearchParams(); + }); + + // ── Initial state ────────────────────────────────────────────────────────── + + describe("initial state", () => { + it("should have empty pending filters when there are no URL params", () => { + // Given + setSearchParams({}); + + // When + const { result } = renderHook(() => useFilterBatch()); + + // Then + expect(result.current.pendingFilters).toEqual({}); + expect(result.current.hasChanges).toBe(false); + expect(result.current.changeCount).toBe(0); + }); + + it("should initialize pending filters from URL search params on mount", () => { + // Given + setSearchParams({ + "filter[severity__in]": "critical,high", + "filter[status__in]": "FAIL", + }); + + // When + const { result } = renderHook(() => useFilterBatch()); + + // Then + expect(result.current.pendingFilters).toEqual({ + "filter[severity__in]": ["critical", "high"], + "filter[status__in]": ["FAIL"], + }); + expect(result.current.hasChanges).toBe(false); + }); + }); + + // ── Excluded keys ────────────────────────────────────────────────────────── + + describe("excluded keys", () => { + it("should exclude filter[search] from batch operations", () => { + // Given — search is excluded from batch; muted now participates in batch + setSearchParams({ + "filter[search]": "some-search-term", + "filter[muted]": "false", + "filter[severity__in]": "critical", + }); + + // When + const { result } = renderHook(() => useFilterBatch()); + + // Then — severity and muted are in pendingFilters; search is excluded + expect(result.current.pendingFilters).toEqual({ + "filter[muted]": ["false"], + "filter[severity__in]": ["critical"], + }); + expect(result.current.pendingFilters["filter[search]"]).toBeUndefined(); + // muted is now part of batch (not excluded) + expect(result.current.pendingFilters["filter[muted]"]).toEqual(["false"]); + }); + }); + + // ── setPending ───────────────────────────────────────────────────────────── + + describe("setPending", () => { + it("should update pending state for a given key", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + // When + act(() => { + result.current.setPending("filter[severity__in]", ["critical", "high"]); + }); + + // Then + expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([ + "critical", + "high", + ]); + }); + + it("should auto-prefix key with filter[] when not already prefixed", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + // When + act(() => { + result.current.setPending("severity__in", ["critical"]); + }); + + // Then — key is stored with filter[] prefix + expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([ + "critical", + ]); + }); + + it("should keep the key but with empty array when values is empty", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + // Pre-condition: set a value first + act(() => { + result.current.setPending("filter[severity__in]", ["critical"]); + }); + + // When — clear the filter by passing empty array + act(() => { + result.current.setPending("filter[severity__in]", []); + }); + + // Then + expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([]); + }); + }); + + // ── getFilterValue ───────────────────────────────────────────────────────── + + describe("getFilterValue", () => { + it("should return pending values for a key that has been set", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + act(() => { + result.current.setPending("filter[severity__in]", ["critical", "high"]); + }); + + // When + const values = result.current.getFilterValue("filter[severity__in]"); + + // Then + expect(values).toEqual(["critical", "high"]); + }); + + it("should return an empty array for a key that has not been set", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + // When + const values = result.current.getFilterValue("filter[unknown_key]"); + + // Then + expect(values).toEqual([]); + }); + + it("should auto-prefix key when calling getFilterValue without filter[]", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + act(() => { + result.current.setPending("filter[severity__in]", ["critical"]); + }); + + // When — key without prefix + const values = result.current.getFilterValue("severity__in"); + + // Then + expect(values).toEqual(["critical"]); + }); + }); + + // ── hasChanges & changeCount ─────────────────────────────────────────────── + + describe("hasChanges", () => { + it("should be false when pending matches the URL state", () => { + // Given + setSearchParams({ "filter[severity__in]": "critical" }); + const { result } = renderHook(() => useFilterBatch()); + + // Then — initial state = URL state, so no changes + expect(result.current.hasChanges).toBe(false); + }); + + it("should be true when pending differs from the URL state", () => { + // Given + setSearchParams({ "filter[severity__in]": "critical" }); + const { result } = renderHook(() => useFilterBatch()); + + // When — change pending + act(() => { + result.current.setPending("filter[severity__in]", ["critical", "high"]); + }); + + // Then + expect(result.current.hasChanges).toBe(true); + }); + }); + + describe("changeCount", () => { + it("should be 0 when pending matches URL", () => { + // Given + setSearchParams({ "filter[severity__in]": "critical" }); + const { result } = renderHook(() => useFilterBatch()); + + // Then + expect(result.current.changeCount).toBe(0); + }); + + it("should count the number of changed filter keys", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + // When — add two different pending filters + act(() => { + result.current.setPending("filter[severity__in]", ["critical"]); + result.current.setPending("filter[status__in]", ["FAIL"]); + }); + + // Then — 2 keys differ from URL (which has neither) + expect(result.current.changeCount).toBe(2); + }); + + it("should decrease changeCount when a pending filter is reset to match URL", () => { + // Given — URL has severity=critical, pending adds status=FAIL + setSearchParams({ "filter[severity__in]": "critical" }); + const { result } = renderHook(() => useFilterBatch()); + + act(() => { + result.current.setPending("filter[status__in]", ["FAIL"]); + }); + + expect(result.current.changeCount).toBe(1); + + // When — reset status back to empty (matching URL which has no status) + act(() => { + result.current.setPending("filter[status__in]", []); + }); + + // Then + expect(result.current.changeCount).toBe(0); + }); + }); + + // ── applyAll ─────────────────────────────────────────────────────────────── + + describe("applyAll", () => { + it("should call router.push with all pending filters serialized as URL params", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + act(() => { + result.current.setPending("filter[severity__in]", ["critical", "high"]); + }); + + // When + act(() => { + result.current.applyAll(); + }); + + // Then + expect(mockPush).toHaveBeenCalledTimes(1); + const calledUrl: string = mockPush.mock.calls[0][0]; + expect(calledUrl).toContain("filter%5Bseverity__in%5D=critical%2Chigh"); + }); + + it("should reset page number when a page param exists in the URL", () => { + // Given — simulate a URL that already has page=3 + mockSearchParamsValue = new URLSearchParams({ + "filter[severity__in]": "critical", + page: "3", + }); + + const { result } = renderHook(() => useFilterBatch()); + + act(() => { + result.current.setPending("filter[status__in]", ["FAIL"]); + }); + + // When + act(() => { + result.current.applyAll(); + }); + + // Then — page should be reset to 1 + const calledUrl: string = mockPush.mock.calls[0][0]; + expect(calledUrl).toContain("page=1"); + }); + + it("should preserve excluded params (filter[search], filter[muted]) in the URL", () => { + // Given + mockSearchParamsValue = new URLSearchParams({ + "filter[search]": "my-search", + "filter[muted]": "false", + }); + + const { result } = renderHook(() => useFilterBatch()); + + act(() => { + result.current.setPending("filter[severity__in]", ["critical"]); + }); + + // When + act(() => { + result.current.applyAll(); + }); + + // Then — search and muted should still be present + const calledUrl: string = mockPush.mock.calls[0][0]; + expect(calledUrl).toContain("filter%5Bsearch%5D=my-search"); + expect(calledUrl).toContain("filter%5Bmuted%5D=false"); + }); + }); + + // ── clearAll ─────────────────────────────────────────────────────────────── + + describe("clearAll", () => { + it("should clear all pending filters including provider and account keys", () => { + // Given — user has pending provider, account, severity, and status filters + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + act(() => { + result.current.setPending("filter[provider_type__in]", [ + "aws", + "azure", + ]); + result.current.setPending("filter[provider_id__in]", [ + "provider-uuid-1", + ]); + result.current.setPending("filter[severity__in]", ["critical"]); + result.current.setPending("filter[status__in]", ["FAIL"]); + }); + + // Pre-condition — all filters are pending + expect( + result.current.pendingFilters["filter[provider_type__in]"], + ).toEqual(["aws", "azure"]); + expect(result.current.pendingFilters["filter[provider_id__in]"]).toEqual([ + "provider-uuid-1", + ]); + expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([ + "critical", + ]); + + // When + act(() => { + result.current.clearAll(); + }); + + // Then — pending state must be TRULY EMPTY (no keys at all, not even with empty arrays) + expect(result.current.pendingFilters).toEqual({}); + // getFilterValue normalises missing keys to [] so all selectors show "all selected" + expect( + result.current.getFilterValue("filter[provider_type__in]"), + ).toEqual([]); + expect(result.current.getFilterValue("filter[provider_id__in]")).toEqual( + [], + ); + expect(result.current.getFilterValue("filter[severity__in]")).toEqual([]); + expect(result.current.getFilterValue("filter[status__in]")).toEqual([]); + }); + + it("should also clear provider/account keys that came from the URL (applied state)", () => { + // Given — URL has provider and account filters applied + setSearchParams({ + "filter[provider_type__in]": "aws", + "filter[provider_id__in]": "provider-uuid-1", + "filter[severity__in]": "critical", + }); + const { result } = renderHook(() => useFilterBatch()); + + // Pre-condition — filters are loaded from URL into pending + expect( + result.current.pendingFilters["filter[provider_type__in]"], + ).toEqual(["aws"]); + expect(result.current.pendingFilters["filter[provider_id__in]"]).toEqual([ + "provider-uuid-1", + ]); + + // When + act(() => { + result.current.clearAll(); + }); + + // Then — pending state must be truly empty (no keys, not { key: [] }) + expect(result.current.pendingFilters).toEqual({}); + // provider and account must be cleared even though they came from the URL + expect( + result.current.getFilterValue("filter[provider_type__in]"), + ).toEqual([]); + expect(result.current.getFilterValue("filter[provider_id__in]")).toEqual( + [], + ); + expect(result.current.getFilterValue("filter[severity__in]")).toEqual([]); + }); + + it("should mark hasChanges as true after clear when URL still has applied filters", () => { + // Given — URL has filters applied + setSearchParams({ + "filter[provider_type__in]": "aws", + "filter[severity__in]": "critical", + }); + const { result } = renderHook(() => useFilterBatch()); + + // Pre-condition — no pending changes (matches URL) + expect(result.current.hasChanges).toBe(false); + + // When — clear all + act(() => { + result.current.clearAll(); + }); + + // Then — hasChanges must be true (pending is empty, URL still has filters) + expect(result.current.hasChanges).toBe(true); + }); + + it("should NOT clear excluded keys (filter[search]) but DOES clear filter[muted]", () => { + // Given — URL has search (excluded) plus muted and severity (both in batch) + setSearchParams({ + "filter[search]": "my-search", + "filter[muted]": "false", + "filter[severity__in]": "critical", + }); + const { result } = renderHook(() => useFilterBatch()); + + // Pre-condition — muted and severity are in pendingFilters; search is excluded + expect(result.current.pendingFilters["filter[search]"]).toBeUndefined(); + expect(result.current.pendingFilters["filter[muted]"]).toEqual(["false"]); + + // When + act(() => { + result.current.clearAll(); + }); + + // Then — severity and muted are cleared; search remains excluded (undefined in pending) + expect(result.current.getFilterValue("filter[severity__in]")).toEqual([]); + expect(result.current.pendingFilters["filter[search]"]).toBeUndefined(); + // muted is a batch key, so it gets cleared by clearAll + expect(result.current.pendingFilters["filter[muted]"]).toBeUndefined(); + }); + + it("should clear applied URL filters even if they were explicitly removed from pendingFilters", () => { + // This covers the edge case where pendingFilters diverged from URL state + // (e.g., URL has provider filter but the key was removed from pending via removePending) + setSearchParams({ + "filter[provider_type__in]": "gcp", + "filter[severity__in]": "high", + }); + const { result } = renderHook(() => useFilterBatch()); + + // Remove the provider key from pending (diverge from URL state) + act(() => { + result.current.removePending("filter[provider_type__in]"); + }); + + // Pre-condition — provider is gone from pending but still in URL + expect( + result.current.pendingFilters["filter[provider_type__in]"], + ).toBeUndefined(); + + // When — clearAll should clear BOTH pending keys AND applied URL keys + act(() => { + result.current.clearAll(); + }); + + // Then — severity is cleared + expect(result.current.getFilterValue("filter[severity__in]")).toEqual([]); + // provider_type__in was in the URL (applied state), so clearAll must handle it + expect( + result.current.getFilterValue("filter[provider_type__in]"), + ).toEqual([]); + }); + }); + + // ── discardAll ───────────────────────────────────────────────────────────── + + describe("discardAll", () => { + it("should reset pending to match the current URL state", () => { + // Given — URL has severity=critical + setSearchParams({ "filter[severity__in]": "critical" }); + const { result } = renderHook(() => useFilterBatch()); + + // Add a pending change + act(() => { + result.current.setPending("filter[severity__in]", ["critical", "high"]); + result.current.setPending("filter[status__in]", ["FAIL"]); + }); + + expect(result.current.hasChanges).toBe(true); + + // When + act(() => { + result.current.discardAll(); + }); + + // Then — pending should match URL again + expect(result.current.pendingFilters).toEqual({ + "filter[severity__in]": ["critical"], + }); + expect(result.current.hasChanges).toBe(false); + }); + }); + + // ── URL sync (back/forward) ──────────────────────────────────────────────── + + describe("URL sync", () => { + it("should re-sync pending state when searchParams change (e.g., browser back/forward)", () => { + // Given — initial empty URL + setSearchParams({}); + const { result, rerender } = renderHook(() => useFilterBatch()); + + // Add a pending change + act(() => { + result.current.setPending("filter[severity__in]", ["critical"]); + }); + + expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([ + "critical", + ]); + + // When — simulate browser back by changing searchParams externally + act(() => { + mockSearchParamsValue = new URLSearchParams({ + "filter[severity__in]": "high", + }); + }); + rerender(); + + // Then — pending should re-sync from new URL + expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([ + "high", + ]); + }); + }); + + // ── removePending ────────────────────────────────────────────────────────── + + describe("removePending", () => { + it("should remove a single filter key from pending state", () => { + // Given + setSearchParams({}); + const { result } = renderHook(() => useFilterBatch()); + + act(() => { + result.current.setPending("filter[severity__in]", ["critical"]); + result.current.setPending("filter[status__in]", ["FAIL"]); + }); + + // When + act(() => { + result.current.removePending("filter[severity__in]"); + }); + + // Then + expect( + result.current.pendingFilters["filter[severity__in]"], + ).toBeUndefined(); + expect(result.current.pendingFilters["filter[status__in]"]).toEqual([ + "FAIL", + ]); + }); + }); +}); diff --git a/ui/hooks/use-filter-batch.ts b/ui/hooks/use-filter-batch.ts new file mode 100644 index 00000000000..45cfe4eae0d --- /dev/null +++ b/ui/hooks/use-filter-batch.ts @@ -0,0 +1,257 @@ +"use client"; + +import { usePathname, useRouter, useSearchParams } from "next/navigation"; +import { useEffect, useState } from "react"; + +// Filters that are managed by the batch hook (excludes system defaults) +const EXCLUDED_FROM_BATCH = ["filter[search]"]; + +/** + * Snapshot of pending (un-applied) filter state. + * Keys are raw filter param names, e.g. "filter[severity__in]". + * Values are arrays of selected option strings. + */ +export interface PendingFilters { + [filterKey: string]: string[]; +} + +export interface UseFilterBatchReturn { + /** Current pending filter values — local state, not yet in URL */ + pendingFilters: PendingFilters; + /** Update a single pending filter. Does NOT touch the URL. */ + setPending: (key: string, values: string[]) => void; + /** Apply all pending filters to URL in a single router.push */ + applyAll: () => void; + /** Discard all pending changes, reset pending to the current URL state */ + discardAll: () => void; + /** + * Clear all pending filters to an empty state (no filters selected). + * Unlike `discardAll`, this does NOT reset to the URL state — it sets + * pending to `{}` (truly empty). The user must click Apply to push + * the empty state to the URL. + * Includes provider/account keys and all batch-managed filter keys. + */ + clearAll: () => void; + /** Remove a single filter key from pending state */ + removePending: (key: string) => void; + /** Whether pending state differs from the current URL */ + hasChanges: boolean; + /** Number of filter keys that differ from the URL */ + changeCount: number; + /** Get current value for a filter (pending if set, else from URL) */ + getFilterValue: (key: string) => string[]; +} + +/** + * Derives the applied (URL-backed) filter state from `searchParams`. + * Returns only the filter keys that are not excluded from batch management. + */ +function deriveAppliedFromUrl(searchParams: URLSearchParams): PendingFilters { + const applied: PendingFilters = {}; + + Array.from(searchParams.entries()).forEach(([key, value]) => { + if (!key.startsWith("filter[")) return; + if (EXCLUDED_FROM_BATCH.includes(key)) return; + if (!value) return; + + applied[key] = value.split(",").filter(Boolean); + }); + + return applied; +} + +/** + * Compares two PendingFilters objects for shallow equality. + * Two states are equal when they contain the same keys and the same sorted values. + */ +function areFiltersEqual(a: PendingFilters, b: PendingFilters): boolean { + const keysA = Object.keys(a).filter((k) => a[k].length > 0); + const keysB = Object.keys(b).filter((k) => b[k].length > 0); + + if (keysA.length !== keysB.length) return false; + + return keysA.every((key) => { + if (!b[key]) return false; + const sortedA = [...a[key]].sort(); + const sortedB = [...b[key]].sort(); + if (sortedA.length !== sortedB.length) return false; + return sortedA.every((v, i) => v === sortedB[i]); + }); +} + +/** + * Counts the number of filter keys that differ between pending and applied. + */ +function countChanges( + pending: PendingFilters, + applied: PendingFilters, +): number { + const pendingKeys = Object.keys(pending).filter((k) => pending[k].length > 0); + const appliedKeys = Object.keys(applied).filter((k) => applied[k].length > 0); + + // Merge all unique keys without Set iteration + const allKeys = Array.from(new Set([...pendingKeys, ...appliedKeys])); + + let count = 0; + allKeys.forEach((key) => { + const p = pending[key] ?? []; + const a = applied[key] ?? []; + const sortedP = [...p].sort(); + const sortedA = [...a].sort(); + if ( + sortedP.length !== sortedA.length || + !sortedP.every((v, i) => v === sortedA[i]) + ) { + count++; + } + }); + + return count; +} + +export interface UseFilterBatchOptions { + /** + * Default URL params to apply when applyAll() is called and they are not + * already present in the params. Useful for page-level filter defaults + * (e.g. `{ "filter[muted]": "false" }` on the Findings page). + */ + defaultParams?: Record; +} + +/** + * Manages a two-state (pending → applied) filter model for the Findings view. + * + * - Pending state lives only in this hook (React `useState`). + * - Applied state is owned by the URL (`searchParams`). + * - `applyAll()` performs a single `router.push()` with the full pending state. + * - `discardAll()` resets pending to match the current URL. + * - Browser back/forward automatically re-syncs pending state from the new URL. + */ +export const useFilterBatch = ( + options?: UseFilterBatchOptions, +): UseFilterBatchReturn => { + const router = useRouter(); + const pathname = usePathname(); + const searchParams = useSearchParams(); + + const [pendingFilters, setPendingFilters] = useState(() => + deriveAppliedFromUrl(new URLSearchParams(searchParams.toString())), + ); + + // Sync pending state whenever the URL changes (back/forward nav or external update). + // `searchParams` from useSearchParams() is stable between renders in Next.js App Router. + useEffect(() => { + const applied = deriveAppliedFromUrl( + new URLSearchParams(searchParams.toString()), + ); + setPendingFilters(applied); + }, [searchParams]); + + const setPending = (key: string, values: string[]) => { + const filterKey = key.startsWith("filter[") ? key : `filter[${key}]`; + setPendingFilters((prev) => ({ + ...prev, + [filterKey]: values, + })); + }; + + const removePending = (key: string) => { + const filterKey = key.startsWith("filter[") ? key : `filter[${key}]`; + setPendingFilters((prev) => { + const next = { ...prev }; + delete next[filterKey]; + return next; + }); + }; + + const applyAll = () => { + // Start from the current URL params to preserve non-batch params. + // Only filter[search] is excluded from batch management and preserved from the URL as-is. + const params = new URLSearchParams(searchParams.toString()); + + // Remove all existing batch-managed filter params + Array.from(params.keys()).forEach((key) => { + if (key.startsWith("filter[") && !EXCLUDED_FROM_BATCH.includes(key)) { + params.delete(key); + } + }); + + // Write the pending state + Object.entries(pendingFilters).forEach(([key, values]) => { + const nonEmpty = values.filter(Boolean); + if (nonEmpty.length > 0) { + params.set(key, nonEmpty.join(",")); + } + }); + + // Apply caller-supplied defaults for any params not already set + if (options?.defaultParams) { + Object.entries(options.defaultParams).forEach(([key, value]) => { + if (!params.has(key)) { + params.set(key, value); + } + }); + } + + // Reset pagination + if (params.has("page")) { + params.set("page", "1"); + } + + const queryString = params.toString(); + const targetUrl = queryString ? `${pathname}?${queryString}` : pathname; + router.push(targetUrl, { scroll: false }); + }; + + const discardAll = () => { + const applied = deriveAppliedFromUrl( + new URLSearchParams(searchParams.toString()), + ); + setPendingFilters(applied); + }; + + /** + * Clears ALL pending batch filters to an empty state (no filters selected). + * + * Unlike `discardAll`, this resets pending to `{}` — not to the current URL + * state. This covers both: + * - Keys that are already in `pendingFilters` (pending-only or URL-loaded) + * - Keys that are in the applied (URL) state but were removed from pending + * via `removePending` (edge case: diverged state) + * + * The user must click Apply to push the empty state to the URL. + * `applyAll()` removes all batch-managed URL params first, so even keys + * absent from `pendingFilters` will be removed from the URL on apply. + */ + const clearAll = () => { + // Return a truly empty object — no filters pending at all. + // `getFilterValue` normalises missing keys to [] so selectors will show + // their "all selected" / placeholder state immediately. + setPendingFilters({}); + }; + + const getFilterValue = (key: string): string[] => { + const filterKey = key.startsWith("filter[") ? key : `filter[${key}]`; + return pendingFilters[filterKey] ?? []; + }; + + const appliedFilters = deriveAppliedFromUrl( + new URLSearchParams(searchParams.toString()), + ); + const hasChanges = !areFiltersEqual(pendingFilters, appliedFilters); + const changeCount = hasChanges + ? countChanges(pendingFilters, appliedFilters) + : 0; + + return { + pendingFilters, + setPending, + applyAll, + discardAll, + clearAll, + removePending, + hasChanges, + changeCount, + getFilterValue, + }; +}; diff --git a/ui/types/components.ts b/ui/types/components.ts index 787f58c51ce..33be9f25526 100644 --- a/ui/types/components.ts +++ b/ui/types/components.ts @@ -95,6 +95,16 @@ export const FINDING_STATUS = { export type FindingStatus = (typeof FINDING_STATUS)[keyof typeof FINDING_STATUS]; +/** + * Maps raw finding status values to human-readable display strings. + * Follows the same pattern as SEVERITY_DISPLAY_NAMES in types/severities.ts. + */ +export const FINDING_STATUS_DISPLAY_NAMES: Record = { + PASS: "Pass", + FAIL: "Fail", + MANUAL: "Manual", +}; + export const SEVERITY = { INFORMATIONAL: "informational", LOW: "low", diff --git a/ui/types/filters.ts b/ui/types/filters.ts index b8e8105a815..f9413b479b4 100644 --- a/ui/types/filters.ts +++ b/ui/types/filters.ts @@ -42,3 +42,36 @@ export enum FilterType { CATEGORY = "category__in", RESOURCE_GROUPS = "resource_groups__in", } + +/** + * Controls the filter dispatch behavior of DataTableFilterCustom. + * - "instant": every selection immediately updates the URL (legacy/default behavior) + * - "batch": selections accumulate in pending state; URL only updates on explicit apply + */ +export const DATA_TABLE_FILTER_MODE = { + INSTANT: "instant", + BATCH: "batch", +} as const; + +export type DataTableFilterMode = + (typeof DATA_TABLE_FILTER_MODE)[keyof typeof DATA_TABLE_FILTER_MODE]; + +/** + * Exhaustive union of all URL filter param keys used in Findings filters. + * Use this instead of `string` to ensure FILTER_KEY_LABELS and other + * param-keyed records stay in sync with the actual filter surface. + */ +export type FilterParam = + | "filter[provider_type__in]" + | "filter[provider_id__in]" + | "filter[severity__in]" + | "filter[status__in]" + | "filter[delta__in]" + | "filter[region__in]" + | "filter[service__in]" + | "filter[resource_type__in]" + | "filter[category__in]" + | "filter[resource_groups__in]" + | "filter[scan__in]" + | "filter[inserted_at]" + | "filter[muted]";