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

Add subscribed/unsub_reason fields to waitlist #707

Merged
merged 15 commits into from
Jul 6, 2023

Conversation

leplatrem
Copy link
Contributor

@leplatrem leplatrem commented Jun 19, 2023

This will be necessary for an efficient implementation of #561 . Without that, we cannot synchronize unsubscriptions efficiently on Acoustic. Without a trace of records with subscribed=False, the only way to keep to waitlist subscriptions in sync in Acoustic is todelete all entries of the waitlist relational table for a given contact, and re-add their subscriptions.

With this PR, the waitlist implementation will follow what we want for newsletters in #562

  • Add fields to model, and DB migration
  • Set subscribed to false instead of deleting waitlist records
  • Add dedicated tests with unsub_reason

@leplatrem leplatrem changed the title Add subscribed field waitlist Add subscribed/unsub_reason fields to waitlist Jun 19, 2023
@leplatrem leplatrem added the enhancement New feature or request label Jun 19, 2023
@leplatrem leplatrem force-pushed the add-subscribed-field-waitlist branch from 8c5506e to 5e18525 Compare June 20, 2023 09:10
@leplatrem leplatrem marked this pull request as ready for review June 20, 2023 09:14
@leplatrem leplatrem requested a review from a team as a code owner June 20, 2023 09:14
Copy link
Contributor

@grahamalama grahamalama left a comment

Choose a reason for hiding this comment

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

Without a trace of records with subscribed=False, the only way to keep to waitlist subscriptions in sync in Acoustic is todelete all entries of the waitlist relational table for a given contact, and re-add their subscriptions.

Not necessarily. In #571 we discussed the idea of using some to_delete queue. That discussion was focused on deleting entire contacts, but a similar design might also be useful for removing people from waitlists.

If it's a requirement to capture unsub_reason though, (i.e. if marketing will use this column for some reason) then having this "soft delete" approach would make sense.

bin/integration-test.sh Outdated Show resolved Hide resolved
ctms/models.py Outdated Show resolved Hide resolved
ctms/schemas/waitlist.py Outdated Show resolved Hide resolved
Co-authored-by: grahamalama <gbeckley@mozilla.com>
@leplatrem leplatrem marked this pull request as draft June 28, 2023 14:25
@leplatrem leplatrem marked this pull request as ready for review June 29, 2023 14:36
ctms/crud.py Show resolved Hide resolved
@leplatrem leplatrem merged commit 09c0b56 into main Jul 6, 2023
5 checks passed
@leplatrem leplatrem deleted the add-subscribed-field-waitlist branch July 6, 2023 06:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants