Skip to content
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

PC-77 Improvements to sign up & review process. #73

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

antonkatz
Copy link
Contributor

  1. All admins listed in SignUpAdmins group should receive all new pending sign up requests.
  2. Upon approval, confirmation e-mail should be sent to the user.
  3. Upon request rejection an email needs to be sent to the user.

1.	All admins listed in SignUpAdmins group should receive all new pending sign up requests.
2.	Upon approval, confirmation e-mail should be sent to the user.
3.	Upon request rejection an email needs to be sent to the user.
@antonkatz
Copy link
Contributor Author

@allasm @meatcar please review

"userRealName": $realName.trim(),
"userName": $userDoc.name,
"adminName": $mainAdminName,
"adminEmail": $mainAdminEmail,
"platformName": "PhenomeCentral",
Copy link

Choose a reason for hiding this comment

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

Why do we include platformName as a variable in the email template, if we use "PhenomeCentral" without referencing the variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's how it's been. Plus, if it ever changes, we don't have to edit the template.

@@ -130,12 +191,44 @@ The ${platformName} team&lt;/p&gt;</html>
<subject>Your account at PhenomeCentral has been approved</subject>
Copy link
Member

Choose a reason for hiding this comment

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

Can we use platformName here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Interesting question. My gut tells me no, though.

@sdumitriu
Copy link
Member

Valid PR, needs final review.

&lt;p&gt;Best wishes,&lt;br&gt;
The ${platformName} team&lt;/p&gt;</html>
<html>&lt;p&gt;Dear ${userRealName},&lt;/p&gt;
&lt;p&gt;An account has been created for you on ${platformName}. If you forgot your password, please visit &lt;a href="https://phenomecentral.org/XWiki/ResetPassword"&gt;https://phenomecentral.org/XWiki/ResetPassword&lt;/a&gt; and enter your username ${userName}. Please do not hesitate to contact us if you experience any difficulties with your account.&lt;/p&gt;
Copy link

Choose a reason for hiding this comment

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

Any URL pointing to phenomecentral.org should be parametrized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants