feat(ui): filter by provider group across main views#11659
feat(ui): filter by provider group across main views#11659pfe-nazaries wants to merge 13 commits into
Conversation
- Add getAllProviderGroups to fetch every group for filter dropdowns - Add reusable ProviderGroupSelector with batch and instant modes - Add provider group chip/label resolution and the provider_groups filter param Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add the provider group selector to the Findings filters Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add the provider group selector to the Resources filters Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add the provider group selector to the Overview filters - Scope risk plot, risk pipeline, and severity-over-time to the selected group Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add the provider group selector to the Scan Jobs filter bar Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add the provider group selector to the Providers page filters - Mock getAllProviderGroups in the providers page data tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Add FILTER_FIELD field-name source and a generic FilterParam<Field> template - Replace the FilterType enum with the same const dictionary - Add per-view filter param types co-located with each view's action - Tighten findings/resources filter maps and the findings display fn to their params Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
📝 WalkthroughWalkthroughReplaces the ChangesProvider Group Filter Feature
Sequence Diagram(s)sequenceDiagram
participant Browser
participant ServerPage as Server Page<br/>(findings/resources/scans/<br/>providers/home)
participant getAllProviderGroups
participant API as /provider-groups API
participant FilterComponent as Filter Component<br/>(FindingsFilters, etc.)
participant ProviderGroupSelector
Browser->>ServerPage: Request with URL search params
ServerPage->>getAllProviderGroups: call()
loop pages 1..N (max 50)
getAllProviderGroups->>API: GET /provider-groups?page[number]=N&page[size]=100
API-->>getAllProviderGroups: ProviderGroupsResponse page N
end
getAllProviderGroups-->>ServerPage: merged ProviderGroupsResponse
ServerPage->>FilterComponent: render with providerGroups={groups}
FilterComponent->>ProviderGroupSelector: render with groups={providerGroups}
Browser->>ProviderGroupSelector: selects group(s)
ProviderGroupSelector->>Browser: navigateWithParams sets filter[provider_groups__in]=id1,id2
Browser->>ServerPage: Request with filter[provider_groups__in]=id1,id2
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
✅ All necessary |
|
✅ Conflict Markers Resolved All conflict markers have been successfully resolved in this pull request. |
🔒 Container Security ScanImage: ✅ No Vulnerabilities DetectedThe container image passed all security checks. No known CVEs were found.📋 Resources:
|
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/app/(prowler)/_overview/graphs-tabs/risk-plot/risk-plot.ssr.tsx (1)
31-47:⚠️ Potential issue | 🟠 Major | ⚡ Quick winApply provider filters cumulatively, not as mutually-exclusive branches.
The
else ifchain causes active filters to be ignored (e.g.,provider_groups__inpresent skipsprovider_type__in). This can return incorrect scoped data when multiple filters are set.Suggested fix
- if (providerIdFilter) { + if (providerIdFilter) { // Filter by specific provider IDs const selectedIds = String(providerIdFilter) .split(",") .map((id) => id.trim()); - filteredProviders = allProviders.filter((p) => selectedIds.includes(p.id)); - } else if (providerGroupsFilter) { + filteredProviders = filteredProviders.filter((p) => + selectedIds.includes(p.id), + ); + } + + if (providerGroupsFilter) { // Filter by provider group membership const selectedGroupIds = String(providerGroupsFilter) .split(",") .map((id) => id.trim()); - filteredProviders = allProviders.filter((p) => + filteredProviders = filteredProviders.filter((p) => p.relationships.provider_groups?.data?.some((group) => selectedGroupIds.includes(group.id), ), ); - } else if (providerTypeFilter) { + } + + if (providerTypeFilter) { // Filter by provider types const selectedTypes = String(providerTypeFilter) .split(",") .map((t) => t.trim().toLowerCase()); - filteredProviders = allProviders.filter((p) => + filteredProviders = filteredProviders.filter((p) => selectedTypes.includes(p.attributes.provider.toLowerCase()), ); }As per coding guidelines, "URL/filter stability: Ensure filter changes synchronize with URL state and that query params are preserved appropriately across navigation/drilldowns."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/app/`(prowler)/_overview/graphs-tabs/risk-plot/risk-plot.ssr.tsx around lines 31 - 47, The filtering logic in this section uses an else if chain, which prevents multiple filters from being applied simultaneously. When providerIdFilter, providerGroupsFilter, and providerTypeFilter are all present, only the first matching condition executes, ignoring other active filters. Fix this by converting the else if statements to independent if statements so that each filter is applied cumulatively to the filteredProviders array. Start with all providers and progressively apply each filter condition that is present, ensuring that when multiple filters are active in the URL parameters, they all contribute to narrowing the results rather than mutually excluding each other.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ui/actions/findings/findings-filters.ts`:
- Around line 24-31: The findings filter type definition uses inline string
literals in a union type for fields like delta__in, scan, scan_id, scan_id__in,
inserted_at, inserted_at__gte, inserted_at__lte, and muted. Create a const
object with these string values as keys and their corresponding string values,
apply the as const assertion to it, then derive the union type from typeof
objectName[keyof typeof objectName] instead of using the inline union. This
aligns with the repository's typing conventions and improves maintainability.
In `@ui/actions/manage-groups/manage-groups.test.ts`:
- Around line 65-113: Add a new test case in the manage-groups.test.ts file to
verify handling of error payloads on subsequent pages. The test should mock
handleApiResponseMock to resolve successfully for the first page (using
makePage) and then resolve to an error payload object (with error and status
fields) for the second page call. Assert that getAllProviderGroups() returns
undefined in this scenario to prevent silent truncation of data. This covers the
critical regression path where pagination fails mid-fetch after successfully
retrieving initial pages.
In `@ui/actions/manage-groups/manage-groups.ts`:
- Around line 77-84: The pagination logic in the handleApiResponse block for
fetching provider groups does not properly distinguish between legitimate empty
responses and API error responses. When a later page returns an API error
object, it is incorrectly treated as "no more pages" causing partial group data
to be returned. Add a check to detect if the data response contains error
information before treating it as an empty page result, and throw or break the
loop when an error is encountered instead of continuing. This check should be
performed on the ProviderGroupsResponse object in the condition that currently
only checks for !data?.data or empty data arrays.
In `@ui/actions/overview/overview-filters.ts`:
- Around line 7-9: The OverviewFilterParam type uses an inline union type
pattern which violates the coding guidelines. Replace the inline union
"PROVIDER_TYPE" | "PROVIDER_ID" | "PROVIDER_GROUPS" with the required
const-object pattern by creating a const object (e.g., PROVIDER_FIELD_KEYS)
containing these provider field names as keys with `as const`, then modify the
OverviewFilterParam type to derive from keyof typeof of that const object
instead of using the inline union directly.
In `@ui/actions/providers/providers-filters.ts`:
- Around line 9-12: The ProvidersFilterParam type definition currently violates
the mandatory TypeScript type-definition rule by using inline union composition.
Refactor ProvidersFilterParam to follow the const-object pattern established
elsewhere in the codebase by creating a const object that combines the
FILTER_FIELD and PROVIDERS_PAGE_FILTER selections, marking it with `as const`,
and then deriving the type using `typeof` and `keyof typeof` pattern (similar to
how ProvidersPageFilter is defined in types/providers-table.ts). This
consolidates the filter field references into a single const definition and
improves type maintainability.
In `@ui/actions/resources/resources-filters.ts`:
- Around line 7-16: The ResourcesFilterParam type uses inline union syntax
instead of the project's const-derived union convention. Create a const field
map object containing all the filter parameter options (the PROVIDER_TYPE,
PROVIDER_ID, PROVIDER_GROUPS, REGION, SERVICE fields from FILTER_FIELD plus the
"type__in" and "groups__in" string literals), mark it with `as const`, and then
redefine ResourcesFilterParam as FilterParam with a type derived from that const
object using the typeof keyof pattern rather than the current inline union
approach.
In `@ui/actions/scans/scans-filters.ts`:
- Around line 19-24: The ScansFilterParam type uses inline union literals
("state__in", "trigger") which violates the const-object pattern guideline.
Create a dedicated const object (e.g., SCANS_FILTER_FIELD) that contains all the
filter field options as keys, then refactor ScansFilterParam to derive the union
type from this const object using the pattern const X = { ... } as const; type T
= typeof X[keyof typeof X], eliminating the inline union types in the
FilterParam generic.
In `@ui/app/`(prowler)/_overview/_components/provider-group-selector.tsx:
- Around line 60-173: The ProviderGroupSelector component is currently located
in a route-local folder (ui/app/(prowler)/_overview/_components) but is being
reused across multiple features, which violates the coding guideline that
components used in 2 or more places should be moved to a shared location. Move
the entire ProviderGroupSelector component file to a shared module path such as
ui/components/filters/ and then update all import statements throughout the
codebase that reference the old path to point to the new shared location. This
ensures clean dependency direction and avoids cross-feature coupling to overview
internals.
In `@ui/app/`(prowler)/scans/page.tsx:
- Around line 165-170: The Promise.all call with getAllProviders() and
getAllProviderGroups() will cause the entire page to fail if either promise
rejects, making it brittle to transient errors. Replace Promise.all with
Promise.allSettled to handle both fulfilled and rejected states gracefully. Then
update the destructuring of providersData and providerGroupsData to check the
status property of each result and extract the data only when the promise was
fulfilled, defaulting to empty arrays when either promise is rejected.
- Around line 41-51: The pending-row provider filtering currently only includes
PROVIDER_UID_FILTER_KEYS and PROVIDER_TYPE_FILTER_KEYS constants, but is missing
provider-group filter criteria. Add a new constant array
PROVIDER_GROUP_FILTER_KEYS (following the same pattern as the existing UID and
TYPE filter key arrays) that includes the provider-group-related filter keys
from SCANS_PROVIDER_FILTER_FIELD, likely PROVIDER_GROUPS_IN and PROVIDER_GROUPS
fields. This will ensure pending rows use the same provider filtering logic as
backend-filtered scan rows.
In `@ui/components/scans/scans-filter-bar.tsx`:
- Line 3: The ProviderGroupSelector component is currently imported from a
feature-local _overview path, but since it is used across multiple views, it
violates the coding guidelines that require components used in 2+ places to be
moved to shared locations. Move the ProviderGroupSelector component from
`@/app/`(prowler)/_overview/_components/ to an appropriate shared location such as
lib/, types/, or hooks/, then update the import statement in
scans-filter-bar.tsx to reference the new shared path, and ensure all other
files that import this component from the old location are also updated to use
the new shared import path.
In `@ui/hooks/use-related-filters.ts`:
- Around line 44-54: The default providerFilterType value in
UseRelatedFiltersProps does not align with the hook's data selection logic. The
hook selects between providerIds (which map to provider_id__in) and providerUIDs
(which map to provider_uid__in) on line 48, but the default providerFilterType
is set to FILTER_FIELD.PROVIDER (provider__in), which is intended for a
different view. Change the default value of providerFilterType from
FILTER_FIELD.PROVIDER to FILTER_FIELD.PROVIDER_ID, and update the type
constraint for providerFilterType in the UseRelatedFiltersProps interface
(around lines 20-22) to allow PROVIDER | PROVIDER_ID | PROVIDER_UID instead of
just PROVIDER | PROVIDER_UID. This ensures the hook reads from the correct URL
parameter key that matches the actual provider data being used.
---
Outside diff comments:
In `@ui/app/`(prowler)/_overview/graphs-tabs/risk-plot/risk-plot.ssr.tsx:
- Around line 31-47: The filtering logic in this section uses an else if chain,
which prevents multiple filters from being applied simultaneously. When
providerIdFilter, providerGroupsFilter, and providerTypeFilter are all present,
only the first matching condition executes, ignoring other active filters. Fix
this by converting the else if statements to independent if statements so that
each filter is applied cumulatively to the filteredProviders array. Start with
all providers and progressively apply each filter condition that is present,
ensuring that when multiple filters are active in the URL parameters, they all
contribute to narrowing the results rather than mutually excluding each other.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 7ca0b3f6-121c-40f2-a478-3ca68ec634b2
📒 Files selected for processing (37)
ui/actions/finding-groups/finding-groups.tsui/actions/findings/findings-filters.tsui/actions/manage-groups/manage-groups.test.tsui/actions/manage-groups/manage-groups.tsui/actions/overview/overview-filters.tsui/actions/providers/providers-filters.tsui/actions/resources/resources-filters.tsui/actions/scans/scans-filters.tsui/app/(prowler)/_overview/_components/provider-group-selector.test.tsxui/app/(prowler)/_overview/_components/provider-group-selector.tsxui/app/(prowler)/_overview/graphs-tabs/risk-pipeline-view/risk-pipeline-view.ssr.tsxui/app/(prowler)/_overview/graphs-tabs/risk-plot/risk-plot.ssr.tsxui/app/(prowler)/_overview/severity-over-time/_components/finding-severity-over-time.tsxui/app/(prowler)/findings/page.tsxui/app/(prowler)/page.tsxui/app/(prowler)/providers/page.tsxui/app/(prowler)/providers/providers-page.utils.test.tsui/app/(prowler)/providers/providers-page.utils.tsui/app/(prowler)/resources/page.tsxui/app/(prowler)/scans/page.tsxui/components/filters/data-filters.tsui/components/findings/findings-filters.tsxui/components/findings/findings-filters.utils.test.tsui/components/findings/findings-filters.utils.tsui/components/providers/providers-accounts-view.tsxui/components/providers/providers-filters.test.tsxui/components/providers/providers-filters.tsxui/components/resources/resources-filters.tsxui/components/resources/resources-filters.utils.test.tsui/components/resources/resources-filters.utils.tsui/components/scans/scans-filter-bar.test.tsxui/components/scans/scans-filter-bar.tsxui/components/scans/scans-page-shell.tsxui/hooks/use-filter-batch.test.tsui/hooks/use-related-filters.tsui/types/filters.tsui/types/providers-table.ts
CodeRabbit review — addressedTriaged all 12 review threads against the current code. Commit ✅ Fixed (6)
Verified locally: ⏭️ Not changed, with rationaleConst-object refactor of inline filter-param literals (
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
ui/actions/manage-groups/manage-groups.ts (2)
62-70:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove auth-header resolution inside the
tryblock.Line 62 runs before the
try; ifgetAuthHeadersthrows, this action rejects instead of returningundefined, which can take down callers that batch requests.Suggested fix
export const getAllProviderGroups = async (): Promise< ProviderGroupsResponse | undefined > => { - const headers = await getAuthHeaders({ contentType: false }); const pageSize = 100; // Larger page size to minimize API calls const maxPages = 50; // Safety limit: 50 pages × 100 = 5000 groups max let currentPage = 1; const allGroups: ProviderGroupsResponse["data"] = []; let lastResponse: ProviderGroupsResponse | undefined; let hasMorePages = true; try { + const headers = await getAuthHeaders({ contentType: false }); while (hasMorePages && currentPage <= maxPages) { const url = new URL(`${apiBaseUrl}/provider-groups`); url.searchParams.append("page[number]", currentPage.toString()); url.searchParams.append("page[size]", pageSize.toString());🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/actions/manage-groups/manage-groups.ts` around lines 62 - 70, The getAuthHeaders call at the start of the function is outside the try block, so any errors it throws will reject the action instead of being caught and handled gracefully. Move the line that calls getAuthHeaders({ contentType: false }) and assigns the result to the headers variable inside the try block to ensure any errors are properly caught and the action can return undefined instead of rejecting, preventing downstream issues for batch request callers.
71-121:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDo not return partial results when the max-page guard is reached.
If there are still more pages after Line 103 exits due
maxPages, the function currently returns a truncated group list. This silently drops filter options.Suggested fix
while (hasMorePages && currentPage <= maxPages) { ... } + if (hasMorePages && currentPage > maxPages) { + console.error( + `Error fetching all provider groups: exceeded max page limit (${maxPages})`, + ); + return undefined; + } + if (lastResponse) { return { ...lastResponse, data: allGroups,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ui/actions/manage-groups/manage-groups.ts` around lines 71 - 121, The function currently returns partial results when exiting the while loop at maxPages while hasMorePages is still true, which silently truncates the group list. After the while loop completes, check if hasMorePages is still true (indicating the loop was stopped by the maxPages limit rather than fetching all available pages). If hasMorePages is true, return undefined to indicate incomplete results instead of returning the truncated allGroups. Only proceed with the current return logic (combining allGroups with lastResponse metadata) when hasMorePages is false, ensuring complete data or no data is returned.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@ui/actions/manage-groups/manage-groups.ts`:
- Around line 62-70: The getAuthHeaders call at the start of the function is
outside the try block, so any errors it throws will reject the action instead of
being caught and handled gracefully. Move the line that calls getAuthHeaders({
contentType: false }) and assigns the result to the headers variable inside the
try block to ensure any errors are properly caught and the action can return
undefined instead of rejecting, preventing downstream issues for batch request
callers.
- Around line 71-121: The function currently returns partial results when
exiting the while loop at maxPages while hasMorePages is still true, which
silently truncates the group list. After the while loop completes, check if
hasMorePages is still true (indicating the loop was stopped by the maxPages
limit rather than fetching all available pages). If hasMorePages is true, return
undefined to indicate incomplete results instead of returning the truncated
allGroups. Only proceed with the current return logic (combining allGroups with
lastResponse metadata) when hasMorePages is false, ensuring complete data or no
data is returned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 50bc732a-4cbc-4e73-a069-4ee46b954e2a
📒 Files selected for processing (13)
ui/actions/manage-groups/manage-groups.test.tsui/actions/manage-groups/manage-groups.tsui/actions/scans/scans-filters.tsui/app/(prowler)/page.tsxui/app/(prowler)/scans/page.tsxui/components/filters/provider-group-selector.test.tsxui/components/filters/provider-group-selector.tsxui/components/findings/findings-filters.tsxui/components/providers/providers-filters.test.tsxui/components/providers/providers-filters.tsxui/components/resources/resources-filters.tsxui/components/scans/scans-filter-bar.test.tsxui/components/scans/scans-filter-bar.tsx
💤 Files with no reviewable changes (2)
- ui/components/filters/provider-group-selector.test.tsx
- ui/components/filters/provider-group-selector.tsx
|
The group + account combination silently drops the group filter. In Duplication worth folding before it spreads:
Worth verifying (not blocking): Happy to pair on the tests if that helps. |
Resolve conflicts in favor of the FILTER_FIELD refactor: migrate master's new FilterType usages (AccountFilterKey, account selectors, scans) to FILTER_FIELD, adopt master's id-based scans account filter while keeping the provider-group filter, and move the #11659 changelog entry to a new 1.32.0 UNRELEASED block. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
alejandrobailo
left a comment
There was a problem hiding this comment.
Really nice work, the refactor is clean and the test coverage is solid. A few small things I'd like to sort out before merging, mostly consistency cleanups, none of them blockers on their own:
-
In
scans/page.tsxyou switched toPromise.allSettled, but the other pages still usePromise.all. BothgetAllProvidersandgetAllProviderGroupsalready catch internally and returnundefined, so they never reject, which means the.status === "fulfilled"branches here are dead code. I'd go back toPromise.allto keep it consistent with findings/resources/overview. -
In
provider-group-selector.tsxthe filter key is hardcoded as"provider_groups__in"instead of reusingFILTER_FIELD.PROVIDER_GROUPS. Same value today, but it can drift over time and TS won't catch it. Let's reference the constant. -
The selector uses hardcoded DOM ids (
provider-group-selector/provider-group-label). No page renders two of these today so it works, butAccountsSelectoralready takes anidprop for exactly this reason. I'd add one here too just to be safe. -
In the
providerTypeFilterbranch ofrisk-pipeline-view.ssr.tsxwe enumerate the types straight from the filter without intersecting withscopedProviders, so we rely on the API param to scope by group. It's not a data leak, but we can end up firing redundant calls or showing an empty node for a type that has no providers in the selected group. Worth a second look. -
parseFilterIdsdoesn't validate the group ids coming from the URL. Low impact since the backendUUIDInFilterrejects malformed uuids with a 400, but leaving it noted.
- 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>
|
Context
Provider groups let users organize cloud accounts into logical groups. The main views could already be scoped by provider type and provider account, but not by provider group. This PR adds a provider group filter so data can be scoped to the accounts that belong to one or more groups.
Grabacion.de.pantalla.2026-06-22.a.las.9.24.07.mov
Grabacion.de.pantalla.2026-06-22.a.las.9.27.01.mov
Grabacion.de.pantalla.2026-06-22.a.las.9.30.07.mov
Grabacion.de.pantalla.2026-06-22.a.las.9.30.57.mov
Grabacion.de.pantalla.2026-06-22.a.las.9.31.50.mov
Description
ProviderGroupSelectorfilter control, backed by amanage-groupsServer Action that lists the tenant's provider groupsFilterParamtype split into core fields plus per-view typesmanage-groupsaction, and the per-view filter helpersSteps to review
With at least one provider group defined (containing some accounts), open each page and use the new Provider group filter:
//findings/resources/scans/providersConfirm the data is scoped to the accounts in the selected group(s) and that combining it with the existing provider type / account filters behaves as expected.
Checklist
Community Checklist
UI (if applicable)
License
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
🤖 Generated with Claude Code
Summary by CodeRabbit