From 8604215b99229a4818a547a22e14bd0ba231215f Mon Sep 17 00:00:00 2001 From: Alan Buscaglia Date: Fri, 20 Mar 2026 12:53:33 +0100 Subject: [PATCH] fix(ui): finalize reviewer feedback for findings batch filters - Enforce batch/instant props with discriminated unions in filter components - Add strict FilterParam typing for findings filter labels - Simplify apply button label logic and clarify batch hook comments --- .../_components/accounts-selector.tsx | 47 ++++++++++++------- .../_components/provider-type-selector.tsx | 36 +++++++++----- .../filters/apply-filters-button.tsx | 7 +-- .../custom-checkbox-muted-findings.tsx | 24 +++++++--- ui/components/filters/custom-date-picker.tsx | 25 ++++++---- ui/components/findings/findings-filters.tsx | 8 ++-- ui/hooks/use-filter-batch.ts | 3 +- ui/types/filters.ts | 20 ++++++++ 8 files changed, 117 insertions(+), 53 deletions(-) diff --git a/ui/app/(prowler)/_overview/_components/accounts-selector.tsx b/ui/app/(prowler)/_overview/_components/accounts-selector.tsx index 3d03d47fd9..362396f03f 100644 --- a/ui/app/(prowler)/_overview/_components/accounts-selector.tsx +++ b/ui/app/(prowler)/_overview/_components/accounts-selector.tsx @@ -50,23 +50,9 @@ const PROVIDER_ICON: Record = { openstack: , }; -interface AccountsSelectorProps { +/** Common props shared by both batch and instant modes. */ +interface AccountsSelectorBaseProps { providers: ProviderProps[]; - /** - * When provided, called instead of navigating immediately. - * Use this on pages that batch filter changes (e.g. Findings). - * The Overview page omits this prop to keep instant-apply behavior. - * - * @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; - /** - * When in batch mode, pass the pending selected values from the parent. - * This allows the component to reflect pending state before Apply is clicked. - * Ignored when `onBatchChange` is not provided (URL-driven mode). - */ - selectedValues?: string[]; /** * Currently selected provider types (from the pending ProviderTypeSelector state). * Used only for contextual description/empty-state messaging — does NOT narrow @@ -75,6 +61,33 @@ interface AccountsSelectorProps { 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[]; +} + +/** 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, @@ -89,7 +102,7 @@ export function AccountsSelector({ 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 selectedIds = onBatchChange ? selectedValues : urlSelectedIds; const visibleProviders = providers; // .filter((p) => p.attributes.connection?.connected) diff --git a/ui/app/(prowler)/_overview/_components/provider-type-selector.tsx b/ui/app/(prowler)/_overview/_components/provider-type-selector.tsx index 94ba251902..74d728fd74 100644 --- a/ui/app/(prowler)/_overview/_components/provider-type-selector.tsx +++ b/ui/app/(prowler)/_overview/_components/provider-type-selector.tsx @@ -152,24 +152,38 @@ 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 { /** - * When provided, called instead of navigating immediately. + * Called instead of navigating immediately. * Use this on pages that batch filter changes (e.g. Findings). - * The Overview page omits this prop to keep instant-apply behavior. * * @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; + onBatchChange: (filterKey: string, values: string[]) => void; /** - * When in batch mode, pass the pending selected values from the parent. - * This allows the component to reflect pending state before Apply is clicked. - * Ignored when `onBatchChange` is not provided (URL-driven mode). + * Pending selected values controlled by the parent. + * Reflects pending state before Apply is clicked. */ - selectedValues?: string[]; -}; + 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, @@ -185,9 +199,7 @@ export const ProviderTypeSelector = ({ : []; // In batch mode, use the parent-controlled pending values; otherwise, use URL state. - const selectedTypes = onBatchChange - ? (selectedValues ?? []) - : urlSelectedTypes; + const selectedTypes = onBatchChange ? selectedValues : urlSelectedTypes; const handleMultiValueChange = (values: string[]) => { if (onBatchChange) { diff --git a/ui/components/filters/apply-filters-button.tsx b/ui/components/filters/apply-filters-button.tsx index 4129fa921d..be0fa183a7 100644 --- a/ui/components/filters/apply-filters-button.tsx +++ b/ui/components/filters/apply-filters-button.tsx @@ -33,11 +33,8 @@ export const ApplyFiltersButton = ({ onDiscard, className, }: ApplyFiltersButtonProps) => { - const label = hasChanges - ? changeCount > 0 - ? `Apply Filters (${changeCount})` - : "Apply Filters" - : "Apply Filters"; + const label = + changeCount > 0 ? `Apply Filters (${changeCount})` : "Apply Filters"; return (
diff --git a/ui/components/filters/custom-checkbox-muted-findings.tsx b/ui/components/filters/custom-checkbox-muted-findings.tsx index ef97cf0e7f..779b060b46 100644 --- a/ui/components/filters/custom-checkbox-muted-findings.tsx +++ b/ui/components/filters/custom-checkbox-muted-findings.tsx @@ -11,21 +11,31 @@ const MUTED_FILTER_VALUES = { INCLUDE: "include", } as const; -interface CustomCheckboxMutedFindingsProps { +/** Batch mode: caller controls both the checked state and the notification callback (all-or-nothing). */ +interface CustomCheckboxMutedFindingsBatchProps { /** - * Called in batch mode instead of navigating directly. + * Called instead of navigating directly. * Receives the filter key ("muted") and the string value ("include" or "false"). - * When provided, the component does NOT call `navigateWithParams`. */ - onBatchChange?: (filterKey: string, value: string) => void; + onBatchChange: (filterKey: string, value: string) => void; /** - * Controlled checked state override for batch mode. - * When provided, this value is used as the checkbox state instead of reading from URL params. + * Controlled checked state from the parent (pending state). * `true` = include muted, `false` = exclude muted. + * `undefined` defers to URL state while pending state is not yet set. */ - checked?: boolean; + checked: boolean | undefined; } +/** Instant mode: URL-driven — neither callback nor controlled value. */ +interface CustomCheckboxMutedFindingsInstantProps { + onBatchChange?: never; + checked?: never; +} + +type CustomCheckboxMutedFindingsProps = + | CustomCheckboxMutedFindingsBatchProps + | CustomCheckboxMutedFindingsInstantProps; + export const CustomCheckboxMutedFindings = ({ onBatchChange, checked: checkedProp, diff --git a/ui/components/filters/custom-date-picker.tsx b/ui/components/filters/custom-date-picker.tsx index 033baac89a..7318ec070f 100644 --- a/ui/components/filters/custom-date-picker.tsx +++ b/ui/components/filters/custom-date-picker.tsx @@ -14,21 +14,30 @@ import { import { useUrlFilters } from "@/hooks/use-url-filters"; import { cn } from "@/lib/utils"; -interface CustomDatePickerProps { +/** Batch mode: caller controls both the pending date value and the notification callback (all-or-nothing). */ +interface CustomDatePickerBatchProps { /** - * Called in batch mode instead of updating the URL directly. - * Receives the filter key and the formatted date string (YYYY-MM-DD). - * When provided, the component does NOT call `updateFilter`. + * Called instead of updating the URL directly. + * Receives the filter key ("inserted_at") and the formatted date string (YYYY-MM-DD). */ - onBatchChange?: (filterKey: string, value: string) => void; + onBatchChange: (filterKey: string, value: string) => void; /** - * Controlled value override for batch mode. - * When provided, this value is used as the displayed date instead of reading from URL params. + * Controlled date value from the parent (pending state). * Expected format: YYYY-MM-DD (or any value parseable by `new Date()`). */ - value?: string; + value: string | undefined; } +/** Instant mode: URL-driven — neither callback nor controlled value. */ +interface CustomDatePickerInstantProps { + onBatchChange?: never; + value?: never; +} + +type CustomDatePickerProps = + | CustomDatePickerBatchProps + | CustomDatePickerInstantProps; + const parseDate = (raw: string | null | undefined): Date | undefined => { if (!raw) return undefined; try { diff --git a/ui/components/findings/findings-filters.tsx b/ui/components/findings/findings-filters.tsx index 3c770f3100..9d7a7225ba 100644 --- a/ui/components/findings/findings-filters.tsx +++ b/ui/components/findings/findings-filters.tsx @@ -20,7 +20,7 @@ import { DataTableFilterCustom } from "@/components/ui/table"; 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 } from "@/types/filters"; +import { DATA_TABLE_FILTER_MODE, FilterParam } from "@/types/filters"; import { getProviderDisplayName, ProviderProps } from "@/types/providers"; import { SEVERITY_DISPLAY_NAMES } from "@/types/severities"; @@ -39,8 +39,10 @@ interface FindingsFiltersProps { /** * 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 = { +const FILTER_KEY_LABELS: Record = { "filter[provider_type__in]": "Provider", "filter[provider_id__in]": "Account", "filter[severity__in]": "Severity", @@ -175,7 +177,7 @@ export const FindingsFilters = ({ const filterChips: FilterChip[] = []; Object.entries(pendingFilters).forEach(([key, values]) => { if (!values || values.length === 0) return; - const label = FILTER_KEY_LABELS[key] ?? key; + 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; diff --git a/ui/hooks/use-filter-batch.ts b/ui/hooks/use-filter-batch.ts index fdaf192f49..45cfe4eae0 100644 --- a/ui/hooks/use-filter-batch.ts +++ b/ui/hooks/use-filter-batch.ts @@ -165,7 +165,8 @@ export const useFilterBatch = ( }; const applyAll = () => { - // Start from the current URL params to preserve non-batch params (e.g. filter[search]) + // 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 diff --git a/ui/types/filters.ts b/ui/types/filters.ts index 72831dd458..f9413b479b 100644 --- a/ui/types/filters.ts +++ b/ui/types/filters.ts @@ -55,3 +55,23 @@ export const DATA_TABLE_FILTER_MODE = { 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]";