Eng 2185 finalization and email for consent tasks#7102
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7102 +/- ##
==========================================
- Coverage 87.30% 87.29% -0.02%
==========================================
Files 533 533
Lines 35090 35121 +31
Branches 4067 4073 +6
==========================================
+ Hits 30635 30658 +23
- Misses 3570 3576 +6
- Partials 885 887 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile OverviewGreptile SummaryThis PR adds consent request completion email functionality, mirroring the existing access/erasure completion email workflow. The implementation correctly prioritizes access/erasure emails over consent emails when a privacy request contains multiple action types. Key Changes
Implementation DetailsThe email sending logic follows this priority:
The implementation is consistent with existing patterns for erasure completion emails (no body params required, simple template rendering). Confidence Score: 5/5
Important Files ChangedFile Analysis
|
src/fides/api/service/privacy_request/request_runner_service.py
Outdated
Show resolved
Hide resolved
|
@greptile please review |
| and config_proxy.execution.consent_request_finalization_required | ||
| ) | ||
| ) | ||
| if requires_finalization: |
There was a problem hiding this comment.
There are some cases where we wouldn't want to pause the request for finalization even if the setting is enabled. I know this is at least the case for requests with a source of consent_webhook. These requests are system-generated and should just continue processing. You might want to check with @mfbrown if there are other sources that we want to exclude from this finalization (like Fides.js or Janus SDK).
There was a problem hiding this comment.
What would the impact of manualizing those be? I can imagine that if a notice was used on a website it might be too many DSRs to manually approve. But is there a technical reason beyond human workload?
There was a problem hiding this comment.
Do we want to maybe add a specific type etc to the env var that way a user would turn on the finalization for the types they wanted explicitly?
There was a problem hiding this comment.
If we have to add this, that would be better yeah. My concern being that source should be more expansive than it currently is and I'd like some future wiggle room to define more of them.
There was a problem hiding this comment.
I do think we should add a config to specify which sources require finalization. To the consent point, it would add a lot of manual work to a process that that could process many thousands of webhook responses, so I don't think we can put that work suddenly on customers for all requests.
I agree with @tvandort that we should add the ability to define more sources, more easily.
There was a problem hiding this comment.
OK - making an update here. I did a little work on this over the last week. I have several PRs lined up to make Consent Manual Tasks a thing. I will update this PR to handle the email updates and remove the other elements.
The PR chain starts here The first couple are marked ready for review :)
|
This PR now only does the messaging portion - The consent tasks are being handled in other PRs |
|
@greptile please review |
|
TODO: Need to fix the privacy center save button.
|
Co-authored-by: Jade Wibbels <jade@ethyca.com>
Co-authored-by: Jade Wibbels <jade@ethyca.com>
Ticket ENG-2185
Description Of Changes
🎯 Add email for consent task completion.
In this PR added a consent finalization email template which is not enabled by default and must be turned on by the user.
Code Changes
clients/admin-ui/src/features/messaging-templates/CustomizableMessagingTemplatesEnum.tsclients/admin-ui/src/features/messaging-templates/CustomizableMessagingTemplatesLabelEnum.tsclients/admin-ui/src/features/messaging-templates/CustomizableMessagingTemplatesLabelEnum.tsclients/admin-ui/src/types/api/models/MessagingActionType.tsclients/privacy-center/types/api/models/MessagingActionType.tssrc/fides/api/models/messaging_template.pysrc/fides/api/schemas/messaging/messaging.pysrc/fides/api/service/privacy_request/request_runner_service.pywith new messaging callSteps to Confirm
Now you can make that consent request. It will take a second because it also uses celery workers but it should come through in the Request Manager (http://localhost:3000/new-privacy-requests)
Then check your emails - you can also test that updating the template works here too.
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works