-
Notifications
You must be signed in to change notification settings - Fork 12
role update email feature successfully implemented #126
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
base: main
Are you sure you want to change the base?
Conversation
|
|
||
| await user.save(); | ||
|
|
||
| if (req.body.team_role !== undefined && process.env.ENABLE_EMAIL === "true") { |
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.
can be good to add a comment here explaining why we send an email when team role is not defined.
also will the email be sent even if some permissions are updated ?
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.
now every superuser will receive the same email for change in permission
added a few comments
the email will be sent even if only PERMISSIONS are updated
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.
Went through the new commit.
Contrary to your though process, looking superuser with this way will actually fetch all user since no permission criteria is given.
You should rather use the getUsersFromRoleID with role id of superuser (available as environment variable).
dvishal485
left a comment
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.
update superuser fetch logic
No description provided.