-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add register and token actions, improve message content and format #13
Conversation
Use Study rather study_id to build the message body. Fix missing domain argument. Bump npg-python-lib from 0.3.2 to 0.3.4
"\n" | ||
f"{path}\n" | ||
"\n" | ||
"This is an automated email from NPG. You are receiving it because you are registered\n" | ||
"as a contact for one or more of the Studies listed below:\n" | ||
"This is an automated email from NPG. You are receiving it because you are registered as a contact for one or more of the Studies listed below:\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Notifications for Illumina and PacBio platforms have a standard footer. Do we really want ONT notifications to be different?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one?
npg_notifications/src/npg_notify/mail.py
Lines 152 to 156 in 67a9abf
"If you have any questions or need further assistance, please feel " | |
+ "free to reach out to a Scientific Service Representative at " | |
+ f"dnap-ssr@{domain}.", | |
"", | |
"NPG on behalf of DNA Pipelines\n", |
Yes, it's good for it to be the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. Lines 152-154 were added on request by SSRs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm also expecting to add a few other details in the week or two after it's live - listing the plate barcodes and tag names/sequences.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Footer added
Use Study rather study_id to build the message body.
Fix missing domain argument.
Bump npg-python-lib from 0.3.2 to 0.3.4