-
Notifications
You must be signed in to change notification settings - Fork 84
UI support for Okta monitor #7052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
src/fides/api/schemas/connection_configuration/connection_secrets_okta.py
Fixed
Show fixed
Hide fixed
Greptile OverviewGreptile SummaryThis PR adds comprehensive filtering, data category classification, and promote functionality for infrastructure systems (Okta applications) in the Action Center. The implementation includes a new filter UI with status and vendor filters, data category selection, and individual/bulk promote operations. Major changes:
Issues found:
Confidence Score: 4/5
Important Files ChangedFile Analysis
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
21 files reviewed, 2 comments
...eatures/data-discovery-and-detection/action-center/fields/useInfrastructureSystemsFilters.ts
Outdated
Show resolved
Hide resolved
...data-discovery-and-detection/action-center/hooks/useDiscoveredInfrastructureSystemsTable.tsx
Show resolved
Hide resolved
88d3275 to
9ea4ca3
Compare
9ea4ca3 to
ffbf491
Compare
|
@greptileai review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
18 files reviewed, 2 comments
| if (vendorFilters.includes("known") && !vendorFilters.includes("unknown")) { | ||
| // Backend should support filtering for items WITH vendor_id | ||
| // Using "not_null" as a special indicator (backend implementation required) | ||
| return "not_null"; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
logic: "known" vendor filter implementation is incomplete. Returns "not_null" string which backend doesn't support yet (see line 108 comment). This will either fail silently or cause API errors when users select "Known Vendors" filter without also selecting "Unknown Vendors"
Check that backend actually handles vendor_id: "not_null" parameter, or disable this filter option until backend support is added
| const { getDataUseDisplayNameProps, getDataUses } = useTaxonomies(); | ||
| const dataUses = getDataUses().filter((use) => use.active); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Component uses getDataUses() instead of getDataCategories(), appears to map data uses (purpose) to the classification selector rather than data categories (data types). Verify this is intentional based on the data model
lucanovera
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made changes to match our code standards and hid the braking changes (auth form basically) under the feature flag so it's safe to merge.
Ticket [https://ethyca.atlassian.net/browse/ENG-2120]
Description Of Changes
This PR adds comprehensive filtering, data category classification, and promote functionality to the Infrastructure Systems view in the Action Center. The changes enable users to filter discovered identity provider applications (e.g., Okta apps) by status and vendor, assign data categories to systems, and promote individual or multiple systems to the Fides registry.
Key Features:
Technical Implementation:
Potential Caveats:
Code Changes
InfrastructureSystemsFilters.tsx- Filter component with tree-based selection UI for status and vendor filtersuseInfrastructureSystemsFilters.ts- Hook for managing filter state with URL query parameter synchronizationInfrastructureSystemsFilters.const.ts- Constants and mapping functions for filter labels to API parametersInfrastructureClassificationSelect.tsx- Data category selection component specific to infrastructure systems (separate from monitor fields to avoid cross-contamination)InfrastructureSystemActionsCell.tsx- Action buttons component for individual system promote/ignore operationsInfrastructureSystemListItem.tsx- Integrated classification select and new actions cell componentDiscoveredInfrastructureSystemsTable.tsx- Added filter UI, bulk promote functionality, and proper state managementuseDiscoveredInfrastructureSystemsTable.tsx- Added filter parameter handling, tab change refetching, and exposed refetch functiondiscovery-detection.slice.ts- AddedpromoteIdentityProviderMonitorResultandbulkPromoteIdentityProviderMonitorResultsmutationsdiscovery-detection.slice.ts- ExtendedIdentityProviderMonitorResultsQueryParamsto supportdiff_status,status, andvendor_idfilter parametersSteps to Confirm
/data-discovery/action-center/infrastructure/{monitorId})Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works