Skip to content

Email approvers when we have a new user request to join orgs#33

Merged
marius-mather merged 7 commits into
mainfrom
AAI-127-email-approvers
Jul 1, 2025
Merged

Email approvers when we have a new user request to join orgs#33
marius-mather merged 7 commits into
mainfrom
AAI-127-email-approvers

Conversation

@amandazhuyilan

@amandazhuyilan amandazhuyilan commented Jun 27, 2025

Copy link
Copy Markdown
Contributor

Description

AAI-127: Email approvers when we have a new user request to join orgs

Changes

-Send email to approvers on new BPA user registration

  • Add EmailService using AWS SES
  • Mocks email sending in tests
  • Approver email made configurable
  • AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY added as Github action secrets

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)

How to Test Manually (if necessary)

All unit tests shall pass.

Sample email here:
Screenshot 2025-07-01 at 12 32 19 PM

@amandazhuyilan amandazhuyilan changed the title make ruff happy Email approvers when we have a new user request to join orgs Jun 27, 2025
@amandazhuyilan amandazhuyilan requested review from marius-mather and minh-biocommons and removed request for marius-mather June 27, 2025 10:05

@marius-mather marius-mather left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good overall but I think the actual email sending should occur in a separate function, and ideally a background task, rather than as part of the main registration function.

Comment thread routers/bpa_register.py
Comment thread tests/test_ses.py Outdated
Comment thread .env.example Outdated
@amandazhuyilan amandazhuyilan force-pushed the AAI-127-email-approvers branch from 825062f to 093e1d3 Compare July 1, 2025 02:09

@marius-mather marius-mather left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looking really good overall - just one suggestion re: making sure we move all the email-related work into the background task.

Comment thread routers/bpa_register.py Outdated
Comment thread routers/bpa_register.py Outdated
@marius-mather marius-mather self-requested a review July 1, 2025 03:44

@marius-mather marius-mather left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great, thanks!

@marius-mather marius-mather merged commit 42efd0b into main Jul 1, 2025
1 check passed
@amandazhuyilan amandazhuyilan deleted the AAI-127-email-approvers branch September 16, 2025 00:10
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.

2 participants