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

[FIX] /me REST endpoint was missing user roles and preferences #10240

Merged
merged 4 commits into from
Mar 29, 2018

Conversation

MarcosSpessatto
Copy link
Member

Add user roles and preferences to /me endpoint and removed the endpoint with problems, /users.roles.

@MarcosSpessatto MarcosSpessatto self-assigned this Mar 27, 2018
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10240 March 27, 2018 19:47 Inactive
@MarcosSpessatto
Copy link
Member Author

@rafaelks, could you take a look, please?

@rafaelks
Copy link
Contributor

Great! Looking good to me (not tested the code running yet).

@rafaelks
Copy link
Contributor

Thank you @MarcosSpessatto

]);

const verifiedEmail = me.emails.find((email) => email.verified);
const preferencesDontSetUpYet = !me.settings || !me.settings.preferences;
Copy link
Contributor

Choose a reason for hiding this comment

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

preferencesArentSetupYet would be a better english name for this 👍

Method: GET
Route: api/v1/user.roles
*/
RocketChat.API.v1.addRoute('user.roles', { authRequired: true }, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of removing this right now, let's deprecate this for three releases and then remove it on the fourth release. Yes I know this endpoint is not working as we would expect, however there might be someone who is using it and as soon as we remove it without warning we will break their system.

So, for deprecation example. If this gets added to v0.64, then it will be removed on v0.67. Because it will be deprecated on v0.64, v0.65, and v0.66, then removed on v0.67.

@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-10240 March 28, 2018 19:46 Inactive
Method: GET
Route: api/v1/user.roles
Method: GET
Route: api/v1/user.roles
*/
RocketChat.API.v1.addRoute('user.roles', { authRequired: true }, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add a console.warn('The endpoint /api/v1/user.roles is deprecated and will be removed after version v0.xx.'). Also, add it to the resulting object but only when we are in a development environment. Which, might need to add a helper method which generates everything I just mentioned with providing the endpoint and the version it will be removed.

We will need to update the deprecation warnings for the real version after it has been merged.

@@ -0,0 +1,10 @@
RocketChat.API.helperMethods.set('deprecationWarning', function _deprecationWarning({ endpoint, versionWillBeRemove, response }) {
const warningMessage = `The endpoint ${ endpoint } is deprecated and will be removed after version ${ versionWillBeRemove }`;
Copy link
Contributor

Choose a reason for hiding this comment

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

I apologize for being so picky, but can the endpoint be surrounded in quotes? The endpoint "${ endpoint }" is..

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course @graywolf336! I'll do it. =)

@rodrigok rodrigok changed the title [FIX] Add user roles and preferences to /me endpoint and remove users.roles endpoint [FIX] /me REST endpoint was missing user roles and preferences Mar 29, 2018
@rodrigok rodrigok merged commit 2d2315d into develop Mar 29, 2018
@rodrigok rodrigok deleted the fix/rest-api-users-roles branch March 29, 2018 18:35
@rodrigok rodrigok mentioned this pull request Apr 4, 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.

5 participants