Skip to content
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

Consent SaaS Config: Allow definition of multiple opt-in or opt-out requests #2315

Merged
merged 3 commits into from
Jan 23, 2023

Conversation

pattisdr
Copy link
Contributor

@pattisdr pattisdr commented Jan 21, 2023

Unticketed

Code Changes

  • Make the consent_requests section on the saas config have opt_in and opt_out types of Union[SaaSRequest, List[SaaSRequest]] = [], so the dev can define a single request or a list of requests. Validators turn everything into a list of requests
  • Adjust the SaaSConnector to fire either the list of opt in or the list of opt out requests, instead of the single request
  • Adjust MailchimpTransactional connector to fire two opt-out requests - the current requests being fired I don't think have the desired effect when opting out.

https://mailchimp.com/developer/transactional/api/allowlists/remove-email-from-allowlist/

POST
/allowlists/delete
Removes an email address from the allowlist.

https://mailchimp.com/developer/transactional/api/rejects/add-email-to-denylist/

Add email to denylist
POST
/rejects/add
Adds an email to your email rejection denylist. Addresses that you add manually will never expire and there is no reputation penalty for removing them from your denylist. Attempting to denylist an address that has been added to the allowlist will have no effect.

Note that opt-in, adding the user to the allowlist, still requires just one request:
https://mailchimp.com/developer/transactional/api/allowlists/add-email-to-allowlist/

Add email to allowlist
Collapse
POST
/allowlists/add
Adds an email to your email rejection allowlist. If the address is currently on your denylist, that denylist entry will be removed automatically.

Steps to Confirm

  • list any manual steps for reviewers to confirm the changes

Pre-Merge Checklist

Description Of Changes

The current consent_requests saas config section was set up to allow one opt-out or one opt-in request to be defined, but I think we're already hitting a scenario where a string of requests may need to be fired.

Allow optionally multiple opt in or opt out requests to be defined, that will be fired in sequence for consent requests.

  • Adjust the Mailchimp Transactional connector to first remove the user from the allow list, before adding them to the denylist when the user opts out. On a closer read-through of the docs, I think we need to fire two requests.

…hat will be fired in sequence for consent requests.

- Adjust the Mailchimp Transactional connector to first remove the user from the allow list, before adding them to the denylist when the user opts out.
@pattisdr pattisdr added the run unsafe ci checks Runs fides-related CI checks that require sensitive credentials label Jan 21, 2023
@pattisdr pattisdr marked this pull request as ready for review January 23, 2023 14:24
@pattisdr pattisdr requested a review from seanpreston January 23, 2023 14:27
Copy link
Contributor

@seanpreston seanpreston left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor suggestion — good to go either way, thanks @pattisdr

@pattisdr
Copy link
Contributor Author

Mailchimp transactional opt in and opt out requests both verified locally, integration tests passing locally -

@pattisdr pattisdr merged commit 49efe9a into consent-propagation Jan 23, 2023
@pattisdr pattisdr deleted the multiple-opt-ins branch January 23, 2023 17:59
@pattisdr pattisdr mentioned this pull request Jan 23, 2023
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run unsafe ci checks Runs fides-related CI checks that require sensitive credentials
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants