-
Notifications
You must be signed in to change notification settings - Fork 13.1k
[FIX] REST endpoints to update user not respecting some settings #11474
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
Conversation
| }); | ||
| } | ||
|
|
||
| if (userData.name && !RocketChat.settings.get('Accounts_AllowEmailChange')) { |
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.
Looks like you made a little copy&paste error here. ;) Should be userData.email I guess.
tests/end-to-end/api/01-users.js
Outdated
| }); | ||
|
|
||
| it('should return an error when trying update user email and it is not allowed', (done) => { | ||
| updateSetting('Accounts_AllowEmailChange', false) |
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 am curious why this test did not find the bug from above. Maybe you need beforeEach to reset settings?
Reordering tests fixed the problem.
Change wrong comparation in saveUser function
AmShaegar13
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.
Looks good to me now. 👍
|
Any reason this PR is not in 0.68 release? |
sampaiodiego
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.
I'm concerned if we're duplicating most of these tests in many places. we need to re-evaluate this in the future.
No description provided.