Skip to content

Implement a newsletter template fallback system #2728

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.

Sign up for GitHub

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

Jump to bottom
Closed
whabanks wants to merge 6 commits into from
Closed

Implement a newsletter template fallback system #2728

whabanks wants to merge 6 commits into from

Conversation

@whabanks
Copy link
Contributor

@whabanks whabanks commented Dec 10, 2025 •
edited
Loading

Summary | Résumé

This PR implements a fallback mechanism for fetching newsletter templates from the DB.

How it works

  1. Get the last 3 pairs of newsletter template_ids from Airtable
  2. Iterate through them, starting at the most recent pair
  3. Check if it exists in the DB -> if it does, we use this template to send the user the latest newsletter
  4. Otherwise, continue iterating backwards through the newsletter instalments until we find one that exists in the DB.
    • When we have to use a past template_id -> log a warning that we had to use a fallback, and which subscriber we sent that to
  5. If the previous 3 template_ids all do not exist in the DB, we raise a 500 error up to admin as this indicates there's something very wrong with the Current newsletter templates Airtable and likely requires some level of manual intervention.

Additional fix:

We're also implementing a fix to ensure that the from_email() method checks for the existence of the mailing list table before executing a formula query against it.

Related Issues | Cartes liées

Test instructions | Instructions pour tester la modification

With the most recent template pair being bad data

  1. In Airtable Add a pair of non-existent template_ids to the table, use this for both EN and FR: c3b6603a-15cf-4509-a79f-524da9800da1
  2. Sign up for and confirm your subscription to the newsletter
  3. Click the Send me the most recent newsletter button
  4. Note you receive the newsletter
  5. Note the application logs contain a warning log:
WARNING None "Most recent template from Airtable: c3b6603a-15cf-4509-a79f-524da9800da1 not found in database. Using fallback template: c3a0273c-ea55-4de4-a688-018ab909795d for subscriber: `your_sub_id`

Without bad data

  1. Delete the row you added above
  2. Send yourself the most recent newsletter again
  3. Note you received the newsletter
  4. Note the absence of the warning logging

Testing the from_email() fix

  1. In Airtable, rename, or delete the DEV - Mailing List and DEV - Latest newsletter templates tables
  2. Sign up for the newsletter
  3. Note that it worked and generated the mailing list table
  4. Confirm your subscription and send yourself the latest newsletter
  5. Note that you were routed back to the sign up page. This is because the DEV - Latest newsletter templates is missing the Created at column so the endpoint is throwing back an error. (see note in release instructions for why this happens)
  6. Go back to airtable -> note that the DEV - Latest newsletter templates table was created
  7. Navigate to the table and add a new column called Created at (case sensitive)
    • Date format : ISO format
    • Include Time: True
    • Time format: 24 hours
  8. Add a new row to the table
    • EN template id: c3a0273c-ea55-4de4-a688-018ab909795d (when doing this in prod use the real newsletter template_ids)
    • FR template id: d6545dba-6cac-48bb-94ea-95dcd5c621f6
  9. Send yourself the latest newsletter
  10. Note that you successfully received the newsletter

Release Instructions | Instructions pour le déploiement

Go through the Testing the from_email() fix steps again right after releasing to prod. This will ensure that:

  • The PROD - Mailing List table gets generated
  • The PROD - Latest newsletter templates table gets generated
    • Manual creation of the Created at column is performed
    • Manual addition of the first EN and FR newsletter templates are added to the table

Unfortunately Airtable doesn't support creation of the Created Time field type via their API. So we have to manually do that ourselves, making this process very much a PITA as we now have to remember this restriction in the event that something happens to the tables in the future.

Reviewer checklist | Liste de vérification du réviseur

  • This PR does not break existing functionality.
  • This PR does not violate GCNotify's privacy policies.
  • This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
  • This PR does not significantly alter performance.
  • Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

@whabanks
Implement a newsletter template fallback system
8a406c0 
- If the latest newsletter template_id in airtable is unable to be found
  in our DB, then we will fallback to the most recent template_id that
we can find in our DB. Currently this looks back at the last 3
newsletter installments to find a template_id that exists in the db
Copilot AI review requested due to automatic review settings December 10, 2025 20:25
@whabanks whabanks requested a review from jimleroyer as a code owner December 10, 2025 20:25
Copilot AI reviewed Dec 10, 2025
View reviewed changes
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements a fallback mechanism for newsletter template retrieval by iterating through up to 3 most recent template pairs from Airtable when the latest template is not found in the database. If a template exists in the DB, it is used to send newsletters to subscribers; otherwise, the system falls back to older templates with appropriate logging, and raises a 500 error if none of the 3 templates are found.

Key changes:

  • Modified Airtable query to fetch 3 most recent newsletter template pairs instead of 1
  • Added iterative fallback logic with logging when newer templates are unavailable
  • Enhanced error handling to distinguish between template retrieval failures and missing templates

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
app/newsletter/rest.py Implements core fallback iteration logic through 3 most recent templates with SQLAlchemyError handling and warning logs
app/clients/airtable/models.py Changes Airtable query from fetching 1 to 3 most recent newsletter template records
tests/app/newsletter/test_rest.py Adds test coverage for fallback behavior and all-templates-not-found scenario

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

whabanks and others added 4 commits December 10, 2025 16:26
@whabanks
Update app/newsletter/rest.py
7176b1a 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@whabanks
Add defensive check on first_template_id
2b83fda
@whabanks
Add check and create mailing list table
4edb8cf 
- Add check for and create mailing list logic to the `from_email()`
  method
@whabanks
Fix model tests
d0fa498
whabanks
whabanks commented Dec 10, 2025
View reviewed changes
app/newsletter/rest.py

# Iterate through the 3 latest newsletter templates from Airtable and use the most recent
# template that currently exists in the DB
for index, newsletter_template in enumerate(latest_newsletter_templates):
Copy link
Contributor Author

@whabanks whabanks Dec 10, 2025

Choose a reason for hiding this comment

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

Shame on my non-pythonic use of index here.

smcmurtry
smcmurtry requested changes Dec 10, 2025
View reviewed changes
Copy link
Contributor

@smcmurtry smcmurtry left a comment

Choose a reason for hiding this comment

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

I'm able to get the DEV tables set up again after deleting them using this code.

It's not passing the test I'm doing when adding bad template ids though:

  1. add a new row to the bottom of "DEV - current newsletter templates" with bad ids like "abc" etc
  2. add a 2nd bad row
  3. click the "send me the latest newsletter" button
  4. I am redirected to the signup page

@whabanks
Copy link
Contributor Author

whabanks commented Dec 10, 2025

Closing because we decided to split this up into two PRs:
#2730
#2729

@whabanks whabanks closed this Dec 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments

@smcmurtry smcmurtry smcmurtry requested changes

@jimleroyer jimleroyer Awaiting requested review from jimleroyer jimleroyer is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@whabanks @smcmurtry