Implement newsletter fallback mechanism #2730
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- 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
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements a fallback mechanism to handle cases where the most recent newsletter template from Airtable doesn't exist in the database. When sending newsletters, the system now attempts to use the most recent template first, and if that fails, it falls back to one of the previous two templates before raising an error.
Key changes:
- Fetches up to 3 most recent newsletter template pairs from Airtable instead of just 1
- Iterates through templates with fallback logic, using the most recent available template in the database
- Adds warning logs when fallback templates are used and error logs when no valid templates are found
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/newsletter/rest.py | Implements fallback iteration logic through up to 3 templates with appropriate error handling and logging |
| app/clients/airtable/models.py | Updates get_latest_newsletter_templates to return 3 templates instead of 1 |
| 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.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
5 tasks
- Since airtable doesn't have a native UUID column type. We cannot guarantee that newsletter_template_id_en and fr columns contain only UUIDs. We need to validate the id's when searching backwards for a valid newsletter template_id to use when sending the latest newsletter to new subscribers
This reverts commit 661c25b. As it was meant to be committed in a different branch.......
This reverts commit 87aa01d. Turns out this WAS meant for this branch. Brain is scrambled
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary | Résumé
This PR implements a fallback mechanism for fetching newsletter templates from the DB.
How it works
template_id-> log a warning that we had to use a fallback, and which subscriber we sent that toCurrent newsletter templatesAirtable and likely requires some level of manual intervention.Test instructions | Instructions pour tester la modification
With the most recent template pair being bad data
c3b6603a-15cf-4509-a79f-524da9800da1Send me the most recent newsletterbuttonWithout bad data