Skip to content

Conversation

@lucanovera
Copy link
Contributor

@lucanovera lucanovera commented Nov 7, 2025

Ticket ENG-1805

Description Of Changes

Adds a section to the privacy request settings page to enable/disable duplicate detection and to set the detection period.

Code Changes

  • Adds related types
  • Adds new component to handle duplicate settings
  • Modifies titles on the page to fit 2 sections (existing redaction patterns and duplicate detection)

Steps to Confirm

  1. Login to admin-ui and go to Settings > Privacy Request or click here for the preview
  2. Check there are now 2 sections for Redaction patterns and Duplicate detection
  3. Try enabling/disabling duplicate detection and changing the period
  4. Check that the changes are saved correctly after hitting 'Save' by reloading the page

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

@lucanovera lucanovera requested a review from a team as a code owner November 7, 2025 15:47
@lucanovera lucanovera requested review from speaker-ender and removed request for a team November 7, 2025 15:47
@vercel
Copy link

vercel bot commented Nov 7, 2025

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

Project Deployment Preview Comments Updated (UTC)
fides-plus-nightly Ready Ready Preview Comment Nov 10, 2025 8:02pm
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
fides-privacy-center Ignored Ignored Nov 10, 2025 8:02pm

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.

Greptile Overview

Greptile Summary

This PR adds UI controls for duplicate privacy request detection settings. The changes reorganize the privacy request settings page into two sections: "Redaction patterns" (existing) and "Duplicate detection" (new).

Key Changes:

  • Added new PrivacyRequestDuplicateDetectionSettings component with toggle for enabling/disabling duplicate detection and input for configuring the detection time window (1-3650 days)
  • Updated page title from "Privacy request redaction patterns" to "Privacy request settings" to reflect multiple sections
  • Added Redux selector for duplicate detection settings following existing patterns
  • Auto-generated TypeScript types for new API models

Implementation Details:

  • Uses Ant Design Form with proper validation (time window must be between 1-3650 days)
  • Conditionally shows time window input only when duplicate detection is enabled
  • Follows existing code patterns for config settings (selector structure, form handling, error/success alerts)
  • Uses Tailwind CSS classes consistently with custom instruction guidelines

Confidence Score: 4/5

  • This PR is safe to merge with minimal risk
  • Clean implementation that follows existing patterns and coding standards. The component properly handles form validation, loading states, and error handling. One minor issue: the match_identity_fields field exists in the API type but isn't exposed in the UI, which could lead to inconsistency if that field is set elsewhere.
  • clients/admin-ui/src/features/settings/PrivacyRequestDuplicateDetectionSettings.tsx - verify whether match_identity_fields should be configurable through the UI

Important Files Changed

File Analysis

Filename Score Overview
clients/admin-ui/src/features/settings/PrivacyRequestDuplicateDetectionSettings.tsx 4/5 New component for duplicate detection settings. Clean implementation using Ant Design Form with proper validation, but doesn't handle the match_identity_fields from the API type.
clients/admin-ui/src/features/config-settings/config-settings.slice.ts 5/5 Adds selector for duplicate detection settings with proper defaults. Implementation follows existing patterns in the file.
clients/admin-ui/src/pages/settings/privacy-requests/index.tsx 5/5 Updates page to display both redaction patterns and duplicate detection settings. Uses Ant Design Flex component properly.

8 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

Copy link
Contributor

@speaker-ender speaker-ender left a comment

Choose a reason for hiding this comment

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

When updating setting, the switch first goes back to the previously selected value and then updates to the new value.

Screen.Recording.2025-11-10.at.9.50.58.AM.mov

@lucanovera
Copy link
Contributor Author

@speaker-ender Thanks for testing and nice catch with the loading states. It should be fixed now, I added an skeleton for the initial loading state and improved the way initialValues are handled.

@speaker-ender
Copy link
Contributor

Nit UX but I think we should start developing patterns for async interactions that look really intentional.
Depending on the load time, the button might look like it's glitching.

Screen.Recording.2025-11-10.at.1.34.29.PM.mov

I think having an inline success indicator that persists would keep it from bouncing back + give more direct feedback for the action.

Screen.Recording.2025-11-10.at.2.22.30.PM.mov

This would also give us the opportunity to "reset" the success state when a user makes a change instead of always showing the submit button in the enabled state.

I think there is also a skeleton specifically for inputs that might be more appropriate for the initial loading state

@lucanovera
Copy link
Contributor Author

Nit UX but I think we should start developing patterns for async interactions that look really intentional. Depending on the load time, the button might look like it's glitching.

Screen.Recording.2025-11-10.at.1.34.29.PM.mov
I think having an inline success indicator that persists would keep it from bouncing back + give more direct feedback for the action.

Screen.Recording.2025-11-10.at.2.22.30.PM.mov
This would also give us the opportunity to "reset" the success state when a user makes a change instead of always showing the submit button in the enabled state.

I think there is also a skeleton specifically for inputs that might be more appropriate for the initial loading state

I've replaced the skeleton with input skeleton. I agree with the patterns for async interactions 💯 , and the example you show looks very good.
I've added it as a topic for the next design system meeting, but IMO we should definitely start developing these patterns. As far as implementation, I think we should add that UX support in a CustomButton wrapper with an interface that is easy to reuse, and that it should be implemented separate from this PR.

@lucanovera lucanovera added this pull request to the merge queue Nov 10, 2025
Merged via the queue into main with commit a060cb6 Nov 10, 2025
47 checks passed
@lucanovera lucanovera deleted the ENG-1805-FE-Support-duplicate-detection-config branch November 10, 2025 23:09
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