Skip to content

Conversation

@MarcosSpessatto
Copy link
Contributor

No description provided.

@MarcosSpessatto MarcosSpessatto added this to the 0.67.0 milestone Jul 16, 2018
@MarcosSpessatto MarcosSpessatto self-assigned this Jul 16, 2018
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11474 July 16, 2018 17:03 Inactive
});
}

if (userData.name && !RocketChat.settings.get('Accounts_AllowEmailChange')) {
Copy link
Contributor

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.

});

it('should return an error when trying update user email and it is not allowed', (done) => {
updateSetting('Accounts_AllowEmailChange', false)
Copy link
Contributor

@AmShaegar13 AmShaegar13 Jul 17, 2018

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
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11474 July 17, 2018 16:14 Inactive
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-11474 July 17, 2018 16:19 Inactive
AmShaegar13
AmShaegar13 previously approved these changes Jul 17, 2018
Copy link
Contributor

@AmShaegar13 AmShaegar13 left a 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. 👍

@rafaelks
Copy link
Contributor

Any reason this PR is not in 0.68 release?

@theorenck theorenck modified the milestones: 0.68.0, Short-term Jul 31, 2018
@MarcosSpessatto MarcosSpessatto modified the milestones: Short-term, 0.69.0 Aug 8, 2018
@sampaiodiego sampaiodiego temporarily deployed to rocket-chat-pr-11474 August 17, 2018 11:57 Inactive
@sampaiodiego sampaiodiego changed the title [FIX] Fix rest users.update, users.updateOwnBasicInfo that were bypassing some settings [FIX] REST endpoints to update user not respecting some settings Aug 17, 2018
Copy link
Member

@sampaiodiego sampaiodiego left a 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.

@sampaiodiego sampaiodiego merged commit a9d41be into develop Aug 17, 2018
@sampaiodiego sampaiodiego deleted the fix-rest-users-update-bypass-settings branch August 17, 2018 13:09
@sampaiodiego sampaiodiego mentioned this pull request Aug 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants