Preventing completion email from being sent for consent requests

[galvana]

Closes PROD-1634

Description Of Changes

Prevention privacy request completion emails from being sent for consent requests.

Code Changes

• Adding a check for consent requests in request_runner_service.run_privacy_request before sending privacy request completion emails

Steps to Confirm

• set the following values in fides/.env

FIDES__NOTIFICATIONS__NOTIFICATION_SERVICE_TYPE=mailgun
FIDES__NOTIFICATIONS__SEND_REQUEST_COMPLETION_NOTIFICATION=true

• Set "executable": true for the marketing.advertising consentOption in src/fides/data/sample_project/privacy_center/config/config.json
• Create a messaging config via the API (Postman)
  • Post Messaging Config
  • Messaging Config Secrets
• Submit a consent request
• Verify the request in the Admin UI, it should go to complete with no errors

Pre-Merge Checklist

• All CI Pipelines Succeeded
• Issue Requirements are Met
• Update CHANGELOG.md

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
fides-plus-nightly	⬜️ Ignored (Inspect)	Visit Preview		Jan 31, 2024 8:19pm

[cypress]

Passing run #6101 ↗︎

0	4	0	0	0
⚠️ You've recorded test results over your free plan limit. Upgrade your plan to view test results.

Details:

Merge 59f6bff into acc2541...
Project: fides	Commit: 2ec5086845 ℹ️
Status: Passed	Duration: 00:36 💡
Started: Jan 30, 2024 9:39 AM	Ended: Jan 30, 2024 9:40 AM

Review all test suite changes for PR #4578 ↗︎

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (acc2541) 86.81% compared to head (59f6bff) 86.81%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4578   +/-   ##
=======================================
  Coverage   86.81%   86.81%           
=======================================
  Files         330      330           
  Lines       19839    19839           
  Branches     2545     2545           
=======================================
  Hits        17223    17223           
  Misses       2150     2150           
  Partials      466      466           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[adamsachs]

thanks @galvana, this looks like a clean, straightforward fix here. and nice job getting test coverage in place! 👍

if, before merging, you could just double check my refresher/assumption about the policy/rule framework to make sure i'm not going crazy, that would be great :)

[adamsachs]

ok, this took a bit of a refresher for me, but i think i'm remembering now - we basically infer an action type for a given PrivacyRequest based on the Policy, which isn't required to have only rules of a single action type, but practically we expect any Policy to have rules with different action types - right?

if that's the case, then this makes sense. if it's not the case, we'd have an edge case here where a hypothetical policy that does access + consent wouldn't trigger a completion email even though it probably should. but that's just totally unrealistic, it's not like we'd actually have a policy with rules that have both of those action types, right!?

anyway, i think this is all good (if my assumptions are correct) - i just wonder when it'll be time for us to update some of our constraints/assumptions in our policy/rule/general DSR framework to be a bit more explicit and consistent about this expectation. i don't think there's a lot of functional risk at the moment, just makes this a bit harder to keep track of when reviewing code 😅

curious to get your thoughts, this was just a little bit of a rabbit hole i went down before feeling totally good about approving 👍

[galvana]

I hesitated a bit when I wrote this too but it was more around where this check should live. I thought about moving the check to initiate_privacy_request_completion_email and just skip the identity check for consent policies but that still takes us back to your concern of how to reliably determine the action type. As we have it now we have one policy per action type so we should be good for now.