Skip to content

Conversation

@DrJfrost
Copy link
Contributor

@DrJfrost DrJfrost commented Dec 2, 2025

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:

  • Filtering System: Added filter UI with support for status filters (All Apps, New Apps, Active, Inactive, Known Vendors, Unknown Vendors, Removed) and vendor filters, with URL query parameter synchronization
  • Data Category Classification: Integrated data category selection component specifically for infrastructure systems, allowing users to assign and manage data categories for discovered applications
  • Promote Functionality: Implemented individual and bulk promote actions to convert discovered IDP applications into System records in the Fides registry
  • Component Isolation: Created separate components for infrastructure systems to avoid affecting existing monitor field components

Technical Implementation:

  • Created new RTK Query endpoints for identity provider monitor promote operations
  • Implemented client-side and server-side filtering logic
  • Added automatic data refresh after promote operations
  • Ensured proper tab change handling with endpoint refetching

Potential Caveats:

  • Ignore functionality for infrastructure systems is not yet implemented (placeholder with TODO)
  • Filter state is synchronized with URL query parameters, which may affect deep linking behavior
  • The bulk promote endpoint expects a direct array in the request body, not an object wrapper

Code Changes

  • Added InfrastructureSystemsFilters.tsx - Filter component with tree-based selection UI for status and vendor filters
  • Added useInfrastructureSystemsFilters.ts - Hook for managing filter state with URL query parameter synchronization
  • Added InfrastructureSystemsFilters.const.ts - Constants and mapping functions for filter labels to API parameters
  • Added InfrastructureClassificationSelect.tsx - Data category selection component specific to infrastructure systems (separate from monitor fields to avoid cross-contamination)
  • Added InfrastructureSystemActionsCell.tsx - Action buttons component for individual system promote/ignore operations
  • Updated InfrastructureSystemListItem.tsx - Integrated classification select and new actions cell component
  • Updated DiscoveredInfrastructureSystemsTable.tsx - Added filter UI, bulk promote functionality, and proper state management
  • Updated useDiscoveredInfrastructureSystemsTable.tsx - Added filter parameter handling, tab change refetching, and exposed refetch function
  • Updated discovery-detection.slice.ts - Added promoteIdentityProviderMonitorResult and bulkPromoteIdentityProviderMonitorResults mutations
  • Updated discovery-detection.slice.ts - Extended IdentityProviderMonitorResultsQueryParams to support diff_status, status, and vendor_id filter parameters

Steps to Confirm

  1. Navigate to the Infrastructure Systems view in Action Center (/data-discovery/action-center/infrastructure/{monitorId})
  2. Filtering:
    • Click the "Filter" button and verify all filter options are available (All Apps, New Apps, Active, Inactive, Known Vendors, Unknown Vendors, Removed)
    • Select various filter combinations and verify the API is called with correct parameters
    • Verify filter state persists in URL query parameters and survives page refresh
    • Test that changing tabs (Attention Required, Activity, Ignored) triggers endpoint refetch even when returning to the same tab
  3. Data Category Classification:
    • Click the "+" button in a system's description area to open the data category selector
    • Select one or more data categories and verify they appear as tags
    • Verify tags with sparkle icons indicate classifier-suggested categories
    • Remove categories by clicking the X on tags
  4. Individual Promote:
    • Click the "Add" button on a single system item
    • Verify success toast appears
    • Verify the system is removed from the list (or moved to appropriate tab)
    • Verify data refreshes automatically after promotion
  5. Bulk Promote:
    • Select multiple systems using checkboxes
    • Click "Actions" dropdown and select "Add"
    • Verify success toast shows correct count of promoted systems
    • Verify selected systems are cleared and data refreshes
    • Verify loading state is shown during bulk operation
  6. Error Handling:
    • Attempt to promote a system without a URN and verify error message
    • Test with invalid filter combinations and verify graceful error handling

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@DrJfrost DrJfrost requested review from a team as code owners December 2, 2025 01:18
@DrJfrost DrJfrost requested review from JadeCara and speaker-ender and removed request for a team December 2, 2025 01:18
@vercel
Copy link

vercel bot commented Dec 2, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Dec 16, 2025 8:34pm
1 Skipped Deployment
Project Deployment Review Updated (UTC)
fides-privacy-center Ignored Ignored Dec 16, 2025 8:34pm

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Dec 2, 2025

Greptile Overview

Greptile Summary

This 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:

  • Added RTK Query endpoints for identity provider monitor operations (promote, mute, unmute - both individual and bulk)
  • Implemented filtering system with URL query parameter synchronization
  • Created infrastructure-specific components to avoid cross-contamination with existing monitor field components
  • Added bulk actions support with selection management
  • Integrated tab-based navigation with automatic data refresh

Issues found:

  • "Known Vendors" filter returns "not_null" string that backend doesn't support yet, which may cause API errors
  • The PR exceeds recommended size limits (1,764 lines across 18 files) per custom rule 32b2f192-9cdf-48f0-be68-5c0eaa04fc70

Confidence Score: 4/5

  • PR is mostly safe to merge with one logical issue requiring verification
  • Score reflects well-structured implementation with proper separation of concerns, cache invalidation, and error handling. However, the "Known Vendors" filter implementation appears incomplete and may cause runtime errors when users select that filter option. The large PR size also increases risk.
  • Pay attention to useDiscoveredInfrastructureSystemsTable.tsx - verify backend supports the vendor_id: "not_null" parameter before the "Known Vendors" filter can be safely used

Important Files Changed

File Analysis

Filename Score Overview
clients/admin-ui/src/features/data-discovery-and-detection/discovery-detection.slice.ts 5/5 Added identity provider monitor endpoints for promote, mute, unmute operations with proper cache invalidation tags
clients/admin-ui/src/features/data-discovery-and-detection/action-center/hooks/useDiscoveredInfrastructureSystemsTable.tsx 4/5 Refactored to add filter support and pagination, includes filter mapping logic for diff_status and metadata status
clients/admin-ui/src/features/data-discovery-and-detection/action-center/fields/InfrastructureSystemsFilters.tsx 5/5 New filter component with tree-based UI, local state management before applying filters
clients/admin-ui/src/features/data-discovery-and-detection/action-center/components/InfrastructureSystemActionsCell.tsx 5/5 Action buttons component for individual system promote/ignore/restore operations with proper loading and error handling
clients/admin-ui/src/features/data-discovery-and-detection/action-center/tables/DiscoveredInfrastructureSystemsTable.tsx 5/5 Main table component with tab navigation, bulk actions, filtering, and pagination

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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

Edit Code Review Agent Settings | Greptile

@lucanovera
Copy link
Contributor

@greptileai review

Copy link
Contributor

@greptile-apps greptile-apps bot left a 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

Edit Code Review Agent Settings | Greptile

Comment on lines +106 to +110
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";
}
Copy link
Contributor

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

Comment on lines +19 to +20
const { getDataUseDisplayNameProps, getDataUses } = useTaxonomies();
const dataUses = getDataUses().filter((use) => use.active);
Copy link
Contributor

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

Copy link
Contributor

@lucanovera lucanovera left a 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.

@lucanovera lucanovera added this pull request to the merge queue Dec 17, 2025
Merged via the queue into main with commit 1f98de5 Dec 17, 2025
46 of 47 checks passed
@lucanovera lucanovera deleted the ENG-2120-flattened branch December 17, 2025 14:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants