Skip to content

feat: data steward filtering [ENG-2074]#7254

Merged
speaker-ender merged 2 commits intomainfrom
feat/data-steward-monitor-connfig--filtering
Jan 28, 2026
Merged

feat: data steward filtering [ENG-2074]#7254
speaker-ender merged 2 commits intomainfrom
feat/data-steward-monitor-connfig--filtering

Conversation

@speaker-ender
Copy link
Copy Markdown
Contributor

@speaker-ender speaker-ender commented Jan 27, 2026

Ticket ENG-2074

Description Of Changes

Adding a filter to the monitor list that is linked to the url to filter by stewards assigned to a monitor.
Filter defaults to current user.

Code Changes

  • Starting to add a generic useSearchFilter hook to manage forms intended to filter search results on a screen
  • Implementing a new MonitorListSearchForm component to handle searching in the monitor screen

Steps to Confirm

  1. Add some test users via the user management screen
  2. Assign users as stewards to some monitors
  3. Visit the action center
  4. Verify that the Data steward filter is visible and is pre-populated with your user value and the results are filtered by that value
  5. Verify that you can choose from a list of users to view monitors that assigned to a steward.
  6. Verify that changing stewards updates the url

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

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Jan 27, 2026

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

Project Deployment Review Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Jan 28, 2026 2:57am
1 Skipped Deployment
Project Deployment Review Updated (UTC)
fides-privacy-center Ignored Ignored Jan 28, 2026 2:57am

Request Review

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Jan 27, 2026

Greptile Overview

Greptile Summary

This PR adds data steward filtering to the monitor list screen, allowing users to filter monitors by assigned stewards. The filter defaults to the current user and updates the URL state.

Key Changes:

  • Added new useSearchForm hook to manage search and filter state with URL synchronization
  • Created MonitorListSearchForm component with steward dropdown and search input
  • Updated MonitorList.tsx to use the new form and pass steward_user_id filter to the API
  • Added steward avatars display in MonitorResult.tsx
  • Backend API now supports steward_user_id array parameter for filtering

Issues Found:

  • The new useSearchForm hook duplicates functionality from the existing useSearch hook (clients/admin-ui/src/features/common/hooks/useSearch.ts), violating the custom rule about using existing hooks for search functionality
  • Hardcoded 100-user limit in the steward dropdown will cause issues for large organizations where stewards beyond that limit won't be filterable
  • Commented-out hideOnSinglePage pagination logic should either be removed or uncommented
  • Direct palette import bypasses design system abstractions

Confidence Score: 3/5

  • This PR is mostly safe but has architectural concerns that should be addressed
  • The feature implementation is functional and follows existing patterns for the most part. However, it introduces code duplication by creating a new hook instead of using the existing useSearch hook, and has a scalability issue with the hardcoded 100-user limit that will cause problems for large organizations. The commented-out code also suggests incomplete implementation decisions.
  • Pay special attention to useSearchForm.ts (duplicates existing functionality) and MonitorListSearchForm.tsx (hardcoded user limit)

Important Files Changed

Filename Overview
clients/admin-ui/src/features/data-discovery-and-detection/action-center/hooks/useSearchForm.ts New hook that duplicates existing useSearch hook functionality. Custom rule d22f4a9c-ba37-4061-9f69-b649706d166a violated.
clients/admin-ui/src/features/data-discovery-and-detection/action-center/forms/MonitorListSearchForm.tsx New form component for filtering monitors by steward and search. Hardcodes 100 user limit which may be insufficient for large orgs.
clients/admin-ui/src/features/data-discovery-and-detection/action-center/MonitorList.tsx Replaced search with new useSearchForm hook; pagination hideOnSinglePage logic commented out without explanation.
clients/admin-ui/src/features/data-discovery-and-detection/action-center/action-center.slice.ts Added steward_user_id array parameter to monitor results query. Clean implementation following existing patterns.
clients/admin-ui/src/features/data-discovery-and-detection/action-center/MonitorResult.tsx Added steward avatars display. Direct palette import may cause maintainability issues if design tokens change.

Copy link
Copy Markdown
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.

5 files reviewed, 5 comments

Edit Code Review Agent Settings | Greptile


useEffect(() => {
resetPagination();
// eslint-disable-next-line react-hooks/exhaustive-deps
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using JSON.stringify(requestData) in the dependency array can cause unnecessary re-renders if the object reference changes but the values remain the same. Consider using a more stable dependency or memoizing requestData.

chore: api update

chore: updating api binding

chore: type fixes

chore: wip fixes

fix: all the types

feat: actually adding steward

fix: correct types for table

feat: default initial values

chore: type and linting fixes

chore: wip

chore: just more work

wip: i hate these form utilities

wip: neeed better form handling

chore: good enough

chore: update api definition

feat: adding stewards to monitor list items

fix: missing type definition

fix: type issue

fix: pagination

chore: adding changelog entry

chore: ignore initial user if root
Copy link
Copy Markdown
Contributor

@jpople jpople left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally, working as expected. Nice approach on the search hook.

@speaker-ender speaker-ender added this pull request to the merge queue Jan 28, 2026
Merged via the queue into main with commit ccd2620 Jan 28, 2026
46 checks passed
@speaker-ender speaker-ender deleted the feat/data-steward-monitor-connfig--filtering branch January 28, 2026 13:10
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.

2 participants