Skip to content

Conversation

@Ostap-Zherebetskyi
Copy link
Collaborator

Purpose

update unregistered contributor notifications and email templates

Changes

See diff

QA Notes

N/A

Documentation

N/A

Side Effects

N/A

Ticket

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

Copy link
Collaborator

@cslzchen cslzchen left a comment

Choose a reason for hiding this comment

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

Great work on the fix + the templates updates 🎆 I added a few notes and will review again when unit tests are fixed.

log=True,
save=False,
notification_type=NotificationType.Type.NODE_CONTRIBUTOR_ADDED_DEFAULT
notification_type=False,
Copy link
Collaborator

Choose a reason for hiding this comment

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

False: we don't want user to receive any emails
None: we will just wait for the caller to provide the template

Comment on lines +1598 to +1606
if isinstance(self, AbstractNode):
notification_type = NotificationType.Type.USER_INVITE_DEFAULT
elif isinstance(self, Preprint):
if self.provider.is_default:
notification_type = NotificationType.Type.USER_INVITE_OSF_PREPRINT
else:
notification_type = NotificationType.Type.PROVIDER_USER_INVITE_PREPRINT
elif isinstance(self, DraftRegistration):
notification_type = NotificationType.Type.USER_INVITE_DRAFT_REGISTRATION
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove "unused" comments in the types definition for these ones.

return

claim_url = None
if not contributor.is_confirmed:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note : looks like is_registered , is_confirmed, date_registered and date_confirmed are the same for this check. Let's keep is_confirmed here.

@Ostap-Zherebetskyi Ostap-Zherebetskyi marked this pull request as ready for review November 28, 2025 17:57
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