Skip to content

Commit 36a3978

Browse files
Pablo F.Gclaude
andcommitted
refactor(ui): address review feedback on provider group filter
- Restore Promise.all in scans page (getAll* never reject) - Use FILTER_FIELD.PROVIDER_GROUPS instead of a hardcoded key - Add id prop to ProviderGroupSelector to avoid DOM id collisions Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d6b2703 commit 36a3978

3 files changed

Lines changed: 29 additions & 19 deletions

File tree

ui/app/(prowler)/scans/page.tsx

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -181,18 +181,12 @@ export default async function Scans({
181181
const session = await auth();
182182
const resolvedSearchParams = await searchParams;
183183

184-
const [providersResult, providerGroupsResult] = await Promise.allSettled([
184+
const [providersData, providerGroupsData] = await Promise.all([
185185
getAllProviders(),
186186
getAllProviderGroups(),
187187
]);
188-
const providers =
189-
providersResult.status === "fulfilled"
190-
? (providersResult.value?.data ?? [])
191-
: [];
192-
const providerGroups =
193-
providerGroupsResult.status === "fulfilled"
194-
? (providerGroupsResult.value?.data ?? [])
195-
: [];
188+
const providers = providersData?.data ?? [];
189+
const providerGroups = providerGroupsData?.data ?? [];
196190

197191
const connectedProviders = providers.filter(
198192
(provider: ProviderProps) =>

ui/components/filters/provider-group-selector.test.tsx

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,24 @@ describe("ProviderGroupSelector", () => {
206206
expect(params.has("scanId")).toBe(false);
207207
});
208208

209+
it("defaults the control id and links the sr-only label to it", () => {
210+
render(<ProviderGroupSelector groups={groups} />);
211+
212+
const label = screen.getByText(/Filter by Provider Group/i);
213+
expect(label).toHaveAttribute("for", "provider-group-selector");
214+
expect(label).toHaveAttribute("id", "provider-group-selector-label");
215+
});
216+
217+
it("applies a custom id so multiple instances don't collide", () => {
218+
render(
219+
<ProviderGroupSelector groups={groups} id="resources-provider-group" />,
220+
);
221+
222+
const label = screen.getByText(/Filter by Provider Group/i);
223+
expect(label).toHaveAttribute("for", "resources-provider-group");
224+
expect(label).toHaveAttribute("id", "resources-provider-group-label");
225+
});
226+
209227
it("does not navigate on clear when nothing is selected", () => {
210228
render(<ProviderGroupSelector groups={groups} />);
211229

ui/components/filters/provider-group-selector.tsx

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,17 @@ import {
1212
} from "@/components/shadcn/select/multiselect";
1313
import { useUrlFilters } from "@/hooks/use-url-filters";
1414
import type { ProviderGroup } from "@/types/components";
15+
import { FILTER_FIELD } from "@/types/filters";
1516

16-
const PROVIDER_GROUP_FILTER_KEY = "provider_groups__in";
17+
const PROVIDER_GROUP_FILTER_KEY = FILTER_FIELD.PROVIDER_GROUPS;
1718
const URL_FILTER_KEY = `filter[${PROVIDER_GROUP_FILTER_KEY}]`;
1819

1920
/** Common props shared by both batch and instant modes. */
2021
interface ProviderGroupSelectorBaseProps {
2122
groups: ProviderGroup[];
2223
search?: MultiSelectSearchProp;
24+
/** DOM id for the control; pass a unique one when rendering more than one. */
25+
id?: string;
2326
/**
2427
* Instant mode only: extra URL params to delete when the selection changes
2528
* (e.g. ["page", "scanId"]), mirroring ProviderAccountSelectors. Ignored in
@@ -61,6 +64,7 @@ export function ProviderGroupSelector({
6164
groups,
6265
onBatchChange,
6366
selectedValues,
67+
id = "provider-group-selector",
6468
search = {
6569
placeholder: "Search Provider Groups...",
6670
emptyMessage: "No Provider Groups found.",
@@ -69,6 +73,7 @@ export function ProviderGroupSelector({
6973
}: ProviderGroupSelectorProps) {
7074
const searchParams = useSearchParams();
7175
const { navigateWithParams } = useUrlFilters();
76+
const labelId = `${id}-label`;
7277

7378
const current = searchParams.get(URL_FILTER_KEY) || "";
7479
const urlSelectedIds = current ? current.split(",").filter(Boolean) : [];
@@ -110,19 +115,12 @@ export function ProviderGroupSelector({
110115

111116
return (
112117
<div className="relative">
113-
<label
114-
htmlFor="provider-group-selector"
115-
className="sr-only"
116-
id="provider-group-label"
117-
>
118+
<label htmlFor={id} className="sr-only" id={labelId}>
118119
Filter by Provider Group. Select one or more Provider Groups to filter
119120
results.
120121
</label>
121122
<MultiSelect values={selectedIds} onValuesChange={handleMultiValueChange}>
122-
<MultiSelectTrigger
123-
id="provider-group-selector"
124-
aria-labelledby="provider-group-label"
125-
>
123+
<MultiSelectTrigger id={id} aria-labelledby={labelId}>
126124
{selectedLabel() || (
127125
<MultiSelectValue placeholder="All Provider Groups" />
128126
)}

0 commit comments

Comments
 (0)