Skip to content

ENG-2797: Add request type selector to policy creation form#7545

Merged
gilluminate merged 5 commits intomainfrom
ENG-2797-policy-request-type-selection
Mar 5, 2026
Merged

ENG-2797: Add request type selector to policy creation form#7545
gilluminate merged 5 commits intomainfrom
ENG-2797-policy-request-type-selection

Conversation

@gilluminate
Copy link
Copy Markdown
Contributor

@gilluminate gilluminate commented Mar 3, 2026

Ticket ENG-2797

Description Of Changes

Adds a required Request type selector (Access / Erasure / Consent) to the Create Policy modal. The selected type is sent as action_type in the PATCH /dsr/policy request, which triggers the existing backend logic to auto-generate a default rule and seed the default data-category targets. For erasure policies, HMAC masking is applied automatically.

The field is not shown when editing an existing policy, since action type is immutable after creation.

Code Changes

  • clients/admin-ui/src/features/policies/policy.slice.ts - Added action_type to PolicyCreateUpdate interface
  • clients/admin-ui/src/features/policies/PolicyFormModal.tsx - Added Request type Select field (create-only, required), updated submit handler to include action_type
  • clients/admin-ui/cypress/e2e/policies/policy-crud.cy.ts - Added tests for type selector visibility, validation, and payload; updated existing submit test to select a type

Steps to Confirm

  1. Navigate to Privacy Request Policies
  2. Click Create policy — confirm a "Request type" dropdown appears between Key and Execution timeframe
  3. Submit without selecting a type — confirm "Request type is required" validation error
  4. Select Access, fill in a name, click Create — confirm the new policy detail page shows an auto-generated access rule under the Rules tab
  5. Repeat for Erasure — confirm the rule has HMAC masking strategy
  6. Open an existing policy and click Edit — confirm the Request type field is absent

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

Adds an action type (Access / Erasure / Consent) selector to the Create
Policy modal. On submit, the selected type is sent as action_type in the
PATCH /dsr/policy request, which auto-generates a default rule and seeds
default data-category targets on the backend.

The field is hidden when editing an existing policy.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown
Contributor

vercel bot commented Mar 3, 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 Mar 5, 2026 7:10pm
1 Skipped Deployment
Project Deployment Actions Updated (UTC)
fides-privacy-center Ignored Ignored Mar 5, 2026 7:10pm

Request Review

gilluminate and others added 2 commits March 2, 2026 19:07
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@gilluminate gilluminate marked this pull request as ready for review March 3, 2026 03:09
@gilluminate gilluminate requested a review from a team as a code owner March 3, 2026 03:09
@gilluminate gilluminate requested review from jpople and removed request for a team March 3, 2026 03:09
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 3, 2026

Greptile Summary

This PR adds a required "Request type" selector (Access / Erasure / Consent) to the Create Policy modal, which sends action_type in the PATCH /dsr/policy request to trigger backend auto-generation of a default rule. The field is correctly hidden on edit since action type is immutable after creation. The implementation is clean and well-tested; all issues found are minor style concerns.

  • capitalize usage: Correctly imports and uses the shared capitalize utility from ~/features/common/utils.ts to generate option labels.
  • A11y lint suppression (PolicyFormModal.tsx:163): Suppresses jsx-a11y/control-has-associated-label on the Select component rather than resolving the underlying concern with an explicit aria-label.
  • Overly broad type (policy.slice.ts:16): action_type is typed ActionType | null in PolicyCreateUpdate, but null is never a valid value to send — the field is either included with a real ActionType on create, or omitted entirely on edit.
  • Magic string in test (policy-crud.cy.ts:49): The assertion value "access" is hardcoded; ActionType.ACCESS could be imported from ~/types/api for type-safety and resilience to enum value changes.

Confidence Score: 4/5

  • This PR is safe to merge; all identified issues are minor style and type-definition concerns with no impact on runtime behavior.
  • The feature is implemented correctly — the new field is conditionally shown, properly validated, and the value is correctly included in the API payload. Test coverage is good. The only issues found are a suppressed a11y lint warning, a slightly too-broad type definition, and one magic string in a test.
  • No files require special attention; minor items are in PolicyFormModal.tsx (lint suppression) and policy.slice.ts (type definition).

Important Files Changed

Filename Overview
clients/admin-ui/src/features/policies/PolicyFormModal.tsx Adds a required "Request type" Select field (create-only) that maps to action_type in the PATCH payload; logic is correct but suppresses an a11y lint warning instead of fixing the root cause.
clients/admin-ui/src/features/policies/policy.slice.ts Adds action_type to PolicyCreateUpdate; types it as `ActionType
clients/admin-ui/cypress/e2e/policies/policy-crud.cy.ts Good test coverage added for selector visibility on create/edit, validation error, and payload assertion; uses a hardcoded magic string for the action_type value instead of importing the enum.
changelog/7545-policy-request-type-selector.yaml Changelog entry correctly added for the new feature.

Last reviewed commit: 42c7c06

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.

4 files reviewed, 3 comments

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.

Tested locally-- ran into a bug where values in the form aren't reset if you close and reopen the modal. Also wondering if it wouldn't be better to still show but just disable the input while editing, like we do with the key?

@gilluminate
Copy link
Copy Markdown
Contributor Author

Good catches! Fixed both:

  • Added form.resetFields() on modal close so values reset properly
  • Request type field is now shown (disabled) when editing, consistent with the key field

- Reset form fields on modal close to fix stale values bug
- Show request type selector as disabled (not hidden) when editing
- Add aria-label to request type select for accessibility
- Remove unnecessary null union from action_type type
- Use ActionType.ACCESS enum instead of magic string in test
- Make key field not required when editing (consistent with request type)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@gilluminate gilluminate added this pull request to the merge queue Mar 5, 2026
Merged via the queue into main with commit 16bfb26 Mar 5, 2026
46 checks passed
@gilluminate gilluminate deleted the ENG-2797-policy-request-type-selection branch March 5, 2026 20:06
mfbrown pushed a commit that referenced this pull request Mar 12, 2026
Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
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