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

Feat/provide list of sending domains #2018

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

andrewleith
Copy link
Member

@andrewleith andrewleith commented Dec 12, 2024

Summary | Résumé

This PR updates the sending domain admin page to use a list of verified sending domains from AWS instead of a drop down list which could allow users to enter invalid domains into.

Since this lists rarely changes the request to SES is cached for 5 minutes so we don't send too many requests.

Related cards

Before - textbox

image

After - dropdown

image

Test instructions | Instructions pour tester la modification

  • Login as an administrator
  • Navigate to a service
  • Go to Settings
  • Go to the Sending domain row and click Change
    • Ensure the list of domains are in a dropdown
  • Select a different domain
  • Save your setting
  • Send yourself a test email
    • Ensure the sending domain is the one you selected

Copy link

@sastels
Copy link
Collaborator

sastels commented Dec 12, 2024

Is it easy to clear this from the cache, ie when we add a new one to AWS? (waiting 5 minutes is fine if this isn't already baked in somehow)

Copy link
Contributor

@jzbahrai jzbahrai left a comment

Choose a reason for hiding this comment

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

I tested this locally and it worked, it doesn't work on admin preview though.

@andrewleith
Copy link
Member Author

Is it easy to clear this from the cache, ie when we add a new one to AWS? (waiting 5 minutes is fine if this isn't already baked in somehow)

Hmm not really - I can add a function to clear it though. FYI it isn't using redis, its using flask_cache.memoize

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants