Skip to content

[ENG-11880] Fixed missed email sends - #11912

Open
ihorsokhanexoft wants to merge 4 commits into
CenterForOpenScience:feature/pbs-26-18from
ihorsokhanexoft:fix/ENG-11880
Open

ihorsokhanexoft wants to merge 4 commits into
CenterForOpenScience:feature/pbs-26-18from
ihorsokhanexoft:fix/ENG-11880

Conversation

@ihorsokhanexoft

@ihorsokhanexoft ihorsokhanexoft commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Ticket

https://openscience.atlassian.net/browse/ENG-11880

Purpose

Initial approval emails are not sent for usual and embargoed registrations
node_pending_embargo_admin and node_pending_registration_admin emails are not sent.

Changes

From my investigation these emails are sent only when archive process is finished.
archive_success calls dst.sanction.ask that picks up appropriate AUTHORIZER_NOTIFY_EMAIL_TYPE depending on the sanction type. It's the only place where these two emails are listed.

However when we create a new registration, we call require_approval and embargo_registration in RegistrationCreateSerializer but don't send emails at all.

@ihorsokhanexoft ihorsokhanexoft changed the title [ENG-11880] Fixed empty generator with admins on approval [ENG-11880] Fixed missed email sends Sep 16, 2026
self.registration_approval.add_authorizer(admin, node=node)
self.registration_approval.save() # Save approval's approval_state
try:
self.registration_approval.ask(admins)

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.

  1. So if we ask() here and in archive_success() we ask again, doesn't that mean the user will receive two identical emails of the same notification?
  2. If we send the notification here, that means it is possible that the user will approve the registration before archiving is done. What happens then if archiving fails?

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