-
Notifications
You must be signed in to change notification settings - Fork 72
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Submit privacy requests from admin UI #4738
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Passing run #6867 ↗︎
Details:
Review all test suite changes for PR #4738 ↗︎ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is looking good! I just called out some minor misses on the acceptance criteria.
Outside of the code, I reached out to @mfbrown on the Figma designs to potentially change "user" to "subject" for the new modal https://www.figma.com/file/wg7cLWnmVgAOh5POHNhn2K?type=design&node-id=20-33619&mode=design#747437392
clients/admin-ui/src/features/privacy-requests/privacy-requests.slice.ts
Outdated
Show resolved
Hide resolved
...rc/types/api/models/fides__api__schemas__privacy_center_config__CustomPrivacyRequestField.ts
Show resolved
Hide resolved
@@ -15,7 +15,7 @@ class IdentityInputs(FidesSchema): | |||
|
|||
class CustomPrivacyRequestField(FidesSchema): | |||
label: str | |||
required: Optional[bool] = False | |||
required: Optional[bool] = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updating this to default required
to True
if not provided to match the current Privacy Center behavior.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4738 +/- ##
=======================================
Coverage 86.64% 86.64%
=======================================
Files 338 338
Lines 19993 19993
Branches 2555 2555
=======================================
Hits 17323 17323
Misses 2201 2201
Partials 469 469 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making the changes! Michael pushed back on my proposal to change "user" to "subject" so you should be all set
clients/admin-ui/src/features/privacy-requests/PrivacyRequestsContainer.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to take back the approval, I just realized we should only show the new button to users that have the PRIVACY_REQUEST_CREATE
scope
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the change, I still see a // TEMP -- do not merge
comment in your Cypress tests but I'm not going to hold this up any more, approved!
…thub.com/ethyca/fides into jpople/prod-1789/submit-privacy-requests
Closes PROD-1789
Description Of Changes
Adds a form to the privacy request table that allows users to submit privacy requests, using the fields configured for the privacy center.
Code Changes
Steps to Confirm
Add the following to
.fides/fides.toml
, then run Fidesplus backend:/privacy-request
/privacy-center-config
with a new config, such as the one in/cypress/fixtures/privacy-requests/privacy-center-config.json
/privacy-request
Pre-Merge Checklist
CHANGELOG.md