mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-07-23 20:42:02 +00:00
fix(ui): refine filter clear and undo behavior in Findings page (#10446)
This commit is contained in:
@@ -2,6 +2,14 @@
|
||||
|
||||
All notable changes to the **Prowler UI** are documented in this file.
|
||||
|
||||
## [1.23.0] (Prowler UNRELEASED)
|
||||
|
||||
### 🐞 Fixed
|
||||
|
||||
- Clear Filters now resets all filters including muted findings and auto-applies, Clear all in pills only removes pill-visible sub-filters, and the discard icon is now an Undo text button [(#10446)](https://github.com/prowler-cloud/prowler/pull/10446)
|
||||
|
||||
---
|
||||
|
||||
## [1.22.0] (Prowler v5.22.0)
|
||||
|
||||
### 🚀 Added
|
||||
|
||||
@@ -5,7 +5,6 @@ import { describe, expect, it, vi } from "vitest";
|
||||
// Mock lucide-react to avoid SVG rendering issues in jsdom
|
||||
vi.mock("lucide-react", () => ({
|
||||
Check: () => <svg data-testid="check-icon" />,
|
||||
X: () => <svg data-testid="x-icon" />,
|
||||
}));
|
||||
|
||||
// Mock @/components/shadcn to avoid next-auth import chain
|
||||
@@ -74,7 +73,7 @@ describe("ApplyFiltersButton", () => {
|
||||
expect(applyButton).toBeDisabled();
|
||||
});
|
||||
|
||||
it("should NOT render the discard (X) button when there are no changes", () => {
|
||||
it("should NOT render the Undo button when there are no changes", () => {
|
||||
// Given / When
|
||||
render(
|
||||
<ApplyFiltersButton
|
||||
@@ -88,7 +87,7 @@ describe("ApplyFiltersButton", () => {
|
||||
// Then
|
||||
expect(
|
||||
screen.queryByRole("button", {
|
||||
name: /discard/i,
|
||||
name: /undo/i,
|
||||
}),
|
||||
).not.toBeInTheDocument();
|
||||
});
|
||||
@@ -166,7 +165,7 @@ describe("ApplyFiltersButton", () => {
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
|
||||
it("should render the discard (X) button", () => {
|
||||
it("should render the Undo button", () => {
|
||||
// Given / When
|
||||
render(
|
||||
<ApplyFiltersButton
|
||||
@@ -179,7 +178,7 @@ describe("ApplyFiltersButton", () => {
|
||||
|
||||
// Then
|
||||
expect(
|
||||
screen.getByRole("button", { name: /discard pending filter changes/i }),
|
||||
screen.getByRole("button", { name: /undo pending filter changes/i }),
|
||||
).toBeInTheDocument();
|
||||
});
|
||||
});
|
||||
@@ -237,7 +236,7 @@ describe("ApplyFiltersButton", () => {
|
||||
// ── onDiscard interaction ────────────────────────────────────────────────
|
||||
|
||||
describe("onDiscard", () => {
|
||||
it("should call onDiscard when the Discard button is clicked", async () => {
|
||||
it("should call onDiscard when the Undo button is clicked", async () => {
|
||||
// Given
|
||||
const user = userEvent.setup();
|
||||
const onApply = vi.fn();
|
||||
@@ -254,7 +253,7 @@ describe("ApplyFiltersButton", () => {
|
||||
|
||||
// When
|
||||
await user.click(
|
||||
screen.getByRole("button", { name: /discard pending filter changes/i }),
|
||||
screen.getByRole("button", { name: /undo pending filter changes/i }),
|
||||
);
|
||||
|
||||
// Then
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { Check, X } from "lucide-react";
|
||||
import { Check } from "lucide-react";
|
||||
|
||||
import { Button } from "@/components/shadcn";
|
||||
import { cn } from "@/lib/utils";
|
||||
@@ -12,7 +12,7 @@ export interface ApplyFiltersButtonProps {
|
||||
changeCount: number;
|
||||
/** Called when the user clicks "Apply Filters" */
|
||||
onApply: () => void;
|
||||
/** Called when the user clicks the discard (X) action */
|
||||
/** Called when the user clicks the discard (Undo) action */
|
||||
onDiscard: () => void;
|
||||
/** Optional extra class names for the outer wrapper */
|
||||
className?: string;
|
||||
@@ -23,7 +23,7 @@ export interface ApplyFiltersButtonProps {
|
||||
*
|
||||
* - 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.
|
||||
* - The Undo button only appears when there are pending changes.
|
||||
* - Uses Prowler's shadcn `Button` component.
|
||||
*/
|
||||
export const ApplyFiltersButton = ({
|
||||
@@ -52,11 +52,11 @@ export const ApplyFiltersButton = ({
|
||||
{hasChanges && (
|
||||
<Button
|
||||
variant="ghost"
|
||||
size="icon-sm"
|
||||
size="sm"
|
||||
onClick={onDiscard}
|
||||
aria-label="Discard pending filter changes"
|
||||
aria-label="Undo pending filter changes"
|
||||
>
|
||||
<X className="size-4" />
|
||||
Undo
|
||||
</Button>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -2,7 +2,6 @@
|
||||
|
||||
import { XCircle } from "lucide-react";
|
||||
import { usePathname, useRouter, useSearchParams } from "next/navigation";
|
||||
import { useCallback } from "react";
|
||||
|
||||
import { Button } from "../shadcn";
|
||||
|
||||
@@ -52,7 +51,7 @@ export const ClearFiltersButton = ({
|
||||
const filterCount = activeFilters.length;
|
||||
|
||||
// Clear all filters except excluded ones (muted, search)
|
||||
const clearFiltersPreservingExcluded = useCallback(() => {
|
||||
const clearFiltersPreservingExcluded = () => {
|
||||
const params = new URLSearchParams(searchParams.toString());
|
||||
Array.from(params.keys()).forEach((key) => {
|
||||
if (
|
||||
@@ -64,7 +63,7 @@ export const ClearFiltersButton = ({
|
||||
});
|
||||
params.delete("page");
|
||||
router.push(`${pathname}?${params.toString()}`, { scroll: false });
|
||||
}, [router, searchParams, pathname]);
|
||||
};
|
||||
|
||||
// In batch mode: use pendingCount if provided; otherwise fall back to URL count.
|
||||
// In instant mode: always use URL count.
|
||||
|
||||
@@ -118,7 +118,8 @@ export const FindingsFilters = ({
|
||||
setPending,
|
||||
applyAll,
|
||||
discardAll,
|
||||
clearAll,
|
||||
clearAndApply,
|
||||
clearKeys,
|
||||
hasChanges,
|
||||
changeCount,
|
||||
getFilterValue,
|
||||
@@ -198,6 +199,20 @@ export const FindingsFilters = ({
|
||||
setPending(filterKey, nextValues);
|
||||
};
|
||||
|
||||
// Handler for clearing all chips: clears only the filter keys visible as chips,
|
||||
// without touching provider/account selectors.
|
||||
const PROVIDER_KEYS = new Set([
|
||||
"filter[provider_type__in]",
|
||||
"filter[provider_id__in]",
|
||||
]);
|
||||
|
||||
const handleClearAllChips = () => {
|
||||
const chipKeys = Array.from(new Set(filterChips.map((c) => c.key))).filter(
|
||||
(k) => !PROVIDER_KEYS.has(k),
|
||||
);
|
||||
clearKeys(chipKeys);
|
||||
};
|
||||
|
||||
// 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
|
||||
@@ -252,7 +267,7 @@ export const FindingsFilters = ({
|
||||
)}
|
||||
<ClearFiltersButton
|
||||
showCount
|
||||
onClear={clearAll}
|
||||
onClear={clearAndApply}
|
||||
pendingCount={
|
||||
Object.entries(pendingFilters).filter(([key, values]) => {
|
||||
if (!values || values.length === 0) return false;
|
||||
@@ -279,7 +294,7 @@ export const FindingsFilters = ({
|
||||
<FilterSummaryStrip
|
||||
chips={filterChips}
|
||||
onRemove={handleChipRemove}
|
||||
onClearAll={clearAll}
|
||||
onClearAll={handleClearAllChips}
|
||||
/>
|
||||
|
||||
{/* Expandable filters section */}
|
||||
|
||||
@@ -585,4 +585,264 @@ describe("useFilterBatch", () => {
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
// ── clearAndApply ──────────────────────────────────────────────────────────
|
||||
|
||||
describe("clearAndApply", () => {
|
||||
it("should clear all batch-managed filters and push URL immediately", () => {
|
||||
// Given
|
||||
setSearchParams({
|
||||
"filter[severity__in]": "critical",
|
||||
"filter[status__in]": "FAIL",
|
||||
});
|
||||
const { result } = renderHook(() => useFilterBatch());
|
||||
|
||||
// Pre-condition — pending is loaded from URL
|
||||
expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([
|
||||
"critical",
|
||||
]);
|
||||
expect(result.current.pendingFilters["filter[status__in]"]).toEqual([
|
||||
"FAIL",
|
||||
]);
|
||||
|
||||
// When
|
||||
act(() => {
|
||||
result.current.clearAndApply();
|
||||
});
|
||||
|
||||
// Then — pending is empty
|
||||
expect(result.current.pendingFilters).toEqual({});
|
||||
|
||||
// And router.push was called
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
|
||||
// The pushed URL must NOT contain severity or status
|
||||
expect(calledUrl).not.toContain("severity");
|
||||
expect(calledUrl).not.toContain("status");
|
||||
});
|
||||
|
||||
it("should apply defaultParams when clearing", () => {
|
||||
// Given
|
||||
setSearchParams({ "filter[severity__in]": "critical" });
|
||||
const { result } = renderHook(() =>
|
||||
useFilterBatch({ defaultParams: { "filter[muted]": "false" } }),
|
||||
);
|
||||
|
||||
// When
|
||||
act(() => {
|
||||
result.current.clearAndApply();
|
||||
});
|
||||
|
||||
// Then — pushed URL contains the defaultParam
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).toContain("filter%5Bmuted%5D=false");
|
||||
});
|
||||
|
||||
it("should preserve filter[search] (excluded from batch)", () => {
|
||||
// Given — URL has both a search param (excluded) and a batch filter
|
||||
mockSearchParamsValue = new URLSearchParams({
|
||||
"filter[search]": "test",
|
||||
"filter[severity__in]": "critical",
|
||||
});
|
||||
const { result } = renderHook(() => useFilterBatch());
|
||||
|
||||
// When
|
||||
act(() => {
|
||||
result.current.clearAndApply();
|
||||
});
|
||||
|
||||
// Then — search param is preserved; severity is gone
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).toContain("filter%5Bsearch%5D=test");
|
||||
expect(calledUrl).not.toContain("severity");
|
||||
});
|
||||
|
||||
it("should reset pagination to page 1", () => {
|
||||
// Given — URL already has a page param
|
||||
mockSearchParamsValue = new URLSearchParams({
|
||||
"filter[severity__in]": "critical",
|
||||
page: "3",
|
||||
});
|
||||
const { result } = renderHook(() => useFilterBatch());
|
||||
|
||||
// When
|
||||
act(() => {
|
||||
result.current.clearAndApply();
|
||||
});
|
||||
|
||||
// Then — page is reset to 1
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).toContain("page=1");
|
||||
});
|
||||
});
|
||||
|
||||
// ── clearKeys ─────────────────────────────────────────────────────────────
|
||||
|
||||
describe("clearKeys", () => {
|
||||
it("should remove only specified keys and push URL", () => {
|
||||
// Given
|
||||
setSearchParams({});
|
||||
const { result } = renderHook(() => useFilterBatch());
|
||||
|
||||
act(() => {
|
||||
result.current.setPending("filter[severity__in]", ["critical"]);
|
||||
result.current.setPending("filter[status__in]", ["FAIL"]);
|
||||
result.current.setPending("filter[region__in]", ["us-east-1"]);
|
||||
});
|
||||
|
||||
// When
|
||||
act(() => {
|
||||
result.current.clearKeys(["filter[severity__in]"]);
|
||||
});
|
||||
|
||||
// Then — severity is gone; status and region remain
|
||||
expect(
|
||||
result.current.pendingFilters["filter[severity__in]"],
|
||||
).toBeUndefined();
|
||||
expect(result.current.pendingFilters["filter[status__in]"]).toEqual([
|
||||
"FAIL",
|
||||
]);
|
||||
expect(result.current.pendingFilters["filter[region__in]"]).toEqual([
|
||||
"us-east-1",
|
||||
]);
|
||||
|
||||
// And the pushed URL contains the remaining keys but not severity
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).toContain("status");
|
||||
expect(calledUrl).toContain("region");
|
||||
expect(calledUrl).not.toContain("severity");
|
||||
});
|
||||
|
||||
it("should accept keys without 'filter[' prefix", () => {
|
||||
// Given
|
||||
setSearchParams({});
|
||||
const { result } = renderHook(() => useFilterBatch());
|
||||
|
||||
act(() => {
|
||||
result.current.setPending("filter[severity__in]", ["critical"]);
|
||||
});
|
||||
|
||||
// When — pass key without filter[] wrapper
|
||||
act(() => {
|
||||
result.current.clearKeys(["severity__in"]);
|
||||
});
|
||||
|
||||
// Then — severity is cleared
|
||||
expect(
|
||||
result.current.pendingFilters["filter[severity__in]"],
|
||||
).toBeUndefined();
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).not.toContain("severity");
|
||||
});
|
||||
|
||||
it("should preserve provider/account keys not in the cleared list", () => {
|
||||
// Given
|
||||
setSearchParams({});
|
||||
const { result } = renderHook(() => useFilterBatch());
|
||||
|
||||
act(() => {
|
||||
result.current.setPending("filter[provider_type__in]", ["aws"]);
|
||||
result.current.setPending("filter[severity__in]", ["critical"]);
|
||||
result.current.setPending("filter[status__in]", ["FAIL"]);
|
||||
});
|
||||
|
||||
// When — clear only severity and status; leave provider untouched
|
||||
act(() => {
|
||||
result.current.clearKeys([
|
||||
"filter[severity__in]",
|
||||
"filter[status__in]",
|
||||
]);
|
||||
});
|
||||
|
||||
// Then — provider_type__in is still in pending
|
||||
expect(
|
||||
result.current.pendingFilters["filter[provider_type__in]"],
|
||||
).toEqual(["aws"]);
|
||||
expect(
|
||||
result.current.pendingFilters["filter[severity__in]"],
|
||||
).toBeUndefined();
|
||||
expect(
|
||||
result.current.pendingFilters["filter[status__in]"],
|
||||
).toBeUndefined();
|
||||
|
||||
// And the pushed URL retains provider but not severity/status
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).toContain("provider_type__in");
|
||||
expect(calledUrl).not.toContain("severity");
|
||||
expect(calledUrl).not.toContain("status__in");
|
||||
});
|
||||
|
||||
it("should apply defaultParams after clearing", () => {
|
||||
// Given
|
||||
setSearchParams({});
|
||||
const { result } = renderHook(() =>
|
||||
useFilterBatch({ defaultParams: { "filter[muted]": "false" } }),
|
||||
);
|
||||
|
||||
act(() => {
|
||||
result.current.setPending("filter[severity__in]", ["critical"]);
|
||||
});
|
||||
|
||||
// When
|
||||
act(() => {
|
||||
result.current.clearKeys(["filter[severity__in]"]);
|
||||
});
|
||||
|
||||
// Then — defaultParam is present in the pushed URL
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).toContain("filter%5Bmuted%5D=false");
|
||||
});
|
||||
|
||||
it("should reset pagination to page 1", () => {
|
||||
// Given — URL already has a page param
|
||||
mockSearchParamsValue = new URLSearchParams({
|
||||
"filter[severity__in]": "critical",
|
||||
page: "5",
|
||||
});
|
||||
const { result } = renderHook(() => useFilterBatch());
|
||||
|
||||
// When
|
||||
act(() => {
|
||||
result.current.clearKeys(["filter[severity__in]"]);
|
||||
});
|
||||
|
||||
// Then — page is reset to 1
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).toContain("page=1");
|
||||
});
|
||||
|
||||
it("should handle empty keys array gracefully", () => {
|
||||
// Given
|
||||
setSearchParams({});
|
||||
const { result } = renderHook(() => useFilterBatch());
|
||||
|
||||
act(() => {
|
||||
result.current.setPending("filter[severity__in]", ["critical"]);
|
||||
});
|
||||
|
||||
// When — clear no keys at all
|
||||
act(() => {
|
||||
result.current.clearKeys([]);
|
||||
});
|
||||
|
||||
// Then — pending is unchanged
|
||||
expect(result.current.pendingFilters["filter[severity__in]"]).toEqual([
|
||||
"critical",
|
||||
]);
|
||||
|
||||
// And router.push was still called (navigates with current state)
|
||||
expect(mockPush).toHaveBeenCalledTimes(1);
|
||||
const calledUrl: string = mockPush.mock.calls[0][0];
|
||||
expect(calledUrl).toContain("severity");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -32,6 +32,19 @@ export interface UseFilterBatchReturn {
|
||||
* Includes provider/account keys and all batch-managed filter keys.
|
||||
*/
|
||||
clearAll: () => void;
|
||||
/**
|
||||
* Clear all batch-managed filters and immediately navigate (router.push)
|
||||
* with defaultParams applied. Equivalent to clearAll() + applyAll() but
|
||||
* avoids the async state gap between the two calls.
|
||||
*/
|
||||
clearAndApply: () => void;
|
||||
/**
|
||||
* Clear only the specified filter keys from pending state and immediately
|
||||
* navigate (router.push) with the remaining pending filters + defaultParams.
|
||||
* Used by "Clear all" in the pills strip to remove only pill-visible filters
|
||||
* without touching provider/account selectors.
|
||||
*/
|
||||
clearKeys: (keys: string[]) => void;
|
||||
/** Remove a single filter key from pending state */
|
||||
removePending: (key: string) => void;
|
||||
/** Whether pending state differs from the current URL */
|
||||
@@ -164,20 +177,19 @@ export const useFilterBatch = (
|
||||
});
|
||||
};
|
||||
|
||||
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.
|
||||
/** Private helper — builds URLSearchParams from a pending state and pushes. */
|
||||
const buildAndPush = (nextPending: PendingFilters) => {
|
||||
const params = new URLSearchParams(searchParams.toString());
|
||||
|
||||
// Remove all existing batch-managed filter params
|
||||
// Remove all 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]) => {
|
||||
// Re-apply the given pending filters
|
||||
Object.entries(nextPending).forEach(([key, values]) => {
|
||||
const nonEmpty = values.filter(Boolean);
|
||||
if (nonEmpty.length > 0) {
|
||||
params.set(key, nonEmpty.join(","));
|
||||
@@ -203,6 +215,10 @@ export const useFilterBatch = (
|
||||
router.push(targetUrl, { scroll: false });
|
||||
};
|
||||
|
||||
const applyAll = () => {
|
||||
buildAndPush(pendingFilters);
|
||||
};
|
||||
|
||||
const discardAll = () => {
|
||||
const applied = deriveAppliedFromUrl(
|
||||
new URLSearchParams(searchParams.toString()),
|
||||
@@ -230,6 +246,39 @@ export const useFilterBatch = (
|
||||
setPendingFilters({});
|
||||
};
|
||||
|
||||
/**
|
||||
* Clears ALL batch-managed filters and immediately navigates (router.push).
|
||||
*
|
||||
* Works around the async gap between clearAll() + applyAll(): instead of
|
||||
* setting pending to `{}` and then calling applyAll() (which would still
|
||||
* read the old pendingFilters from the closure), this function builds the
|
||||
* target URL directly from an empty pending state and pushes it in one step.
|
||||
* defaultParams (e.g. filter[muted]=false) are applied as usual.
|
||||
*/
|
||||
const clearAndApply = () => {
|
||||
setPendingFilters({});
|
||||
buildAndPush({});
|
||||
};
|
||||
|
||||
/**
|
||||
* Removes only the specified filter keys from pending state and immediately
|
||||
* navigates (router.push) with the remaining filters + defaultParams.
|
||||
*
|
||||
* Used by the pills strip "Clear all" to remove pill-visible filters (severity,
|
||||
* status, delta, region, service, etc.) without touching provider/account selectors.
|
||||
*/
|
||||
const clearKeys = (keys: string[]) => {
|
||||
const normalizedKeys = keys.map((k) =>
|
||||
k.startsWith("filter[") ? k : `filter[${k}]`,
|
||||
);
|
||||
const nextPending: PendingFilters = { ...pendingFilters };
|
||||
normalizedKeys.forEach((k) => {
|
||||
delete nextPending[k];
|
||||
});
|
||||
setPendingFilters(nextPending);
|
||||
buildAndPush(nextPending);
|
||||
};
|
||||
|
||||
const getFilterValue = (key: string): string[] => {
|
||||
const filterKey = key.startsWith("filter[") ? key : `filter[${key}]`;
|
||||
return pendingFilters[filterKey] ?? [];
|
||||
@@ -249,6 +298,8 @@ export const useFilterBatch = (
|
||||
applyAll,
|
||||
discardAll,
|
||||
clearAll,
|
||||
clearAndApply,
|
||||
clearKeys,
|
||||
removePending,
|
||||
hasChanges,
|
||||
changeCount,
|
||||
|
||||
Reference in New Issue
Block a user