Skip to content

Conversation

@amandazhuyilan
Copy link
Contributor

@amandazhuyilan amandazhuyilan commented Nov 26, 2025

Description

Improve email notification debugging capabilities via adding sender information in db.

Changes

  • Introduced email_settings.py so a single DEFAULT_EMAIL_SENDER (overridable via env, currently amanda@biocommons.org.au) is shared by the backend.
  • a new from_address added in EmailNotification, with the field indexed and defaulting to the shared sender so existing code keeps working.
  • services/email_queue.py accepts an optional from_address when queuing mail and falls back to the shared default.
  • updated tests

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have added unit / integration tests that prove my fix is effective or that my feature works
  • I have run all tests locally and they pass
  • I have updated the documentation (if applicable)
  • For any new secrets, I have updated the shared spreadsheet and the GitHub Secrets.

How to Test Manually (if necessary)

<Describe how reviewers can manually, if necessary, test the changes>

Copy link
Collaborator

@marius-mather marius-mather left a comment

Choose a reason for hiding this comment

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

A few small things to look at here - particularly the way we set the config. Would be nice if we could stick to using pydantic-settings and .env for consistentcy

Copy link
Collaborator

@marius-mather marius-mather left a comment

Choose a reason for hiding this comment

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

Looks good to me

@amandazhuyilan amandazhuyilan merged commit 040b783 into main Nov 26, 2025
4 checks passed
@amandazhuyilan amandazhuyilan deleted the fix-add-email-sender-in-emailnotification branch November 26, 2025 04:24
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