Skip to content

ENG-2597: FE condition builder location updates#7495

Merged
gilluminate merged 6 commits intomainfrom
gill/ENG-2597/fe-location-updates-2
Feb 26, 2026
Merged

ENG-2597: FE condition builder location updates#7495
gilluminate merged 6 commits intomainfrom
gill/ENG-2597/fe-location-updates-2

Conversation

@gilluminate
Copy link
Copy Markdown
Contributor

@gilluminate gilluminate commented Feb 25, 2026

Ticket ENG-2597

Description Of Changes

Enhance the policy condition builder with multiselect support for location and country fields, refactor field address constants into a centralized enum, and polish the policy conditions UI.

Key changes:

  • Add multiselect support for location and location_country fields when operator is LIST_CONTAINS (single-select remains for location_groups and location_regulations since their operator is always fixed)
  • Introduce PrivacyRequestField constant object to replace hardcoded field address strings throughout the codebase
  • Consolidate FieldValue type into shared types.ts to resolve import cycle between ConditionValueSelector and utils
  • Update location field labels in the policy condition form ("State/Province", "Country/Territory", "Groups", "Regulation")
  • Use Ant Design Empty component for empty states in conditions/rules lists
  • Add operator-change value reset to prevent stale array values from multiselect being submitted with non-list operators
  • Disable operator select when no field is selected in policy condition form

Code Changes

  • clients/admin-ui/src/features/integrations/configure-tasks/utils.ts - Refactored field address strings into PrivacyRequestField const, replaced if/else chain with FIELD_TYPE_MAP lookup, updated parseConditionValue/parseStoredValueForForm to support array values
  • clients/admin-ui/src/features/integrations/configure-tasks/types.ts - Added FieldValue type (moved from ConditionValueSelector to break import cycle)
  • clients/admin-ui/src/features/integrations/configure-tasks/components/ConditionValueSelector.tsx - Added multiselect support for location and location_country fields, kept location_groups/location_regulations as single-select
  • clients/admin-ui/src/features/integrations/configure-tasks/AddConditionForm.tsx - Added operator-change value reset, pass operator prop to ConditionValueSelector, support string[] in form values
  • clients/admin-ui/src/features/integrations/configure-tasks/components/PrivacyRequestFieldPicker.tsx - Use PrivacyRequestField.CUSTOM_FIELDS_PREFIX constant
  • clients/admin-ui/src/features/integrations/configure-tasks/hooks/useCustomFieldMetadata.ts - Use PrivacyRequestField.CUSTOM_FIELDS_PREFIX constant
  • clients/admin-ui/src/features/policies/conditions/PolicyConditionForm.tsx - Updated field labels, added operator-change value reset, disable operator when no field selected, pass operator to ConditionValueSelector
  • clients/admin-ui/src/features/policies/conditions/PolicyConditionsTab.tsx - Use Ant Empty component for empty state, re-throw save errors
  • clients/admin-ui/src/features/policies/rules/RulesTab.tsx - Use Empty.PRESENTED_IMAGE_SIMPLE
  • clients/admin-ui/cypress/e2e/policies/policy-detail.cy.ts - Update test to use new "Country/Territory" label

Steps to Confirm

Note: Policy conditions can currently only be tested on the Default policies (e.g. Default Access Policy, Default Erasure Policy) because conditions require rules to be present, and the UI does not yet support adding rules to newly created policies.

  1. Enable DSR Policies Beta Flag in About page
  2. Navigate to a Default policy detail page → Conditions tab
  3. Click "Add condition" → select "State/Province" field → select "List contains" operator → verify value input shows a multiselect location picker
  4. Select "Country/Territory" field → select "List contains" → verify multiselect country picker
  5. Navigate to integration configure-tasks → add a condition → verify same multiselect behavior for location fields

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

Made with Cursor

@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Feb 25, 2026

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

Project Deployment Actions Updated (UTC)
fides-plus-nightly Ready Ready Preview, Comment Feb 26, 2026 0:38am
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Feb 26, 2026 0:38am

Request Review

@gilluminate gilluminate force-pushed the gill/ENG-2597/fe-location-updates-2 branch from c430e42 to b2adf2a Compare February 26, 2026 00:03
@gilluminate gilluminate requested a review from jpople February 26, 2026 00:25
@gilluminate gilluminate marked this pull request as ready for review February 26, 2026 00:25
@gilluminate gilluminate requested a review from a team as a code owner February 26, 2026 00:25
@gilluminate gilluminate removed the request for review from a team February 26, 2026 00:27
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Feb 26, 2026

Greptile Summary

This PR enhances the policy condition builder with multiselect support for location fields and refactors field constants for better maintainability.

Key improvements:

  • Added multiselect support for location and location_country fields when using LIST_CONTAINS operator
  • Introduced PrivacyRequestField constant object to eliminate hardcoded field address strings throughout the codebase
  • Resolved import cycle by moving FieldValue type to shared types.ts
  • Added operator-change value reset logic to prevent stale array values from being submitted with non-list operators
  • Updated location field labels in PolicyConditionForm ("State/Province", "Country/Territory", "Groups", "Regulation")
  • Improved empty states by using Ant Design's Empty component consistently

Code quality:

  • Refactored getFieldType() to use FIELD_TYPE_MAP lookup instead of if/else chain for better readability
  • Properly handles array values in parseConditionValue() and parseStoredValueForForm()
  • Error re-throwing in PolicyConditionsTab ensures modal stays open on save failures
  • Type assertions used appropriately to work around auto-generated type limitations for array values

Minor note:

  • TODO comment exists about label inconsistency between form and list views (form: "Country/Territory", list: "Location country")

Confidence Score: 5/5

  • This PR is safe to merge with no critical issues
  • The implementation is solid with proper type handling, defensive programming, and good refactoring practices. The multiselect logic correctly switches between single and multi modes based on operator. Value reset logic prevents data integrity issues. The only minor note is a TODO about label consistency, which is already documented and doesn't affect functionality.
  • No files require special attention

Important Files Changed

Filename Overview
clients/admin-ui/src/features/integrations/configure-tasks/utils.ts Refactored to use PrivacyRequestField constant object, replaced if/else chain with FIELD_TYPE_MAP lookup, added array value handling in parseConditionValue and parseStoredValueForForm
clients/admin-ui/src/features/integrations/configure-tasks/components/ConditionValueSelector.tsx Added multiselect support for location/country fields based on operator, properly handles single vs multiselect modes
clients/admin-ui/src/features/integrations/configure-tasks/AddConditionForm.tsx Added operator change value reset, passes operator prop to ConditionValueSelector, supports string[] in form values
clients/admin-ui/src/features/policies/conditions/PolicyConditionForm.tsx Updated field labels, added operator change value reset, disables operator when no field selected; contains TODO about label inconsistency with list view
clients/admin-ui/src/features/policies/conditions/PolicyConditionsTab.tsx Uses Ant Empty component for empty state, re-throws errors to prevent modal close on save failure

Last reviewed commit: b2adf2a

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.

11 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

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.

Functionality looks good, tested in Vercel and didn't run into any issues. Non-blocking UX feedback, but "List contains" rules are a bit confusing to parse in the list view IMO. If it's possible to render in the opposite order (e.g. "GB, US" "contains" "country") I think that'd be clearer, and I think using the same "Country/Territory" labels we do in the select would be better as well.

Image Image

@gilluminate gilluminate added this pull request to the merge queue Feb 26, 2026
Merged via the queue into main with commit 24993a7 Feb 26, 2026
46 checks passed
@gilluminate gilluminate deleted the gill/ENG-2597/fe-location-updates-2 branch February 26, 2026 23:17
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