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/243 add route to allow user to change username #623

Conversation

kr7pt0
Copy link

@kr7pt0 kr7pt0 commented Feb 5, 2020

No description provided.

naldokan added 2 commits February 5, 2020 10:19
…me, add a parameter in api to change username value
Copy link
Member

@shnizzedy shnizzedy left a comment

Choose a reason for hiding this comment

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

The code changes to girderformindlogger/web_client are unnecessary (see #635), and we definitely don't need the built files in the repo; they should be built after downloading.


login = cherrypy.request.body.params['login']
user['firstName'] = firstName if firstName is not None else ""
user['login'] = login if login is not None else user['login']
Copy link
Member

Choose a reason for hiding this comment

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

We're phasing out userId visibility, so rather than adding this functionality to an already deprecated function that requires userId, we should make or update a call that relies on a user being logged in and modifies only that user. See #635.

@@ -812,19 +812,21 @@ def getUsersDetails(self):
.errorResponse(('You do not have write access for this user.',
'Must be an admin to create an admin.'), 403)
)

Copy link
Member

Choose a reason for hiding this comment

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

This (already deprecated) API call doesn't include the newly added login in the parameters for the interactive documentation.

displayName="",
email="",
admin=False,
status=None,
firstName=None,
lastName=None
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if any frontends are still using the lastName parameter of this deprecated call.

@@ -9,7 +9,8 @@ replica_set = None

[server]
# Set to "production" or "development"
mode = "production"
mode = "development"
# mode = "production"
Copy link
Member

Choose a reason for hiding this comment

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

I don't think we want to commit this change to master

displayName
) else firstName if firstName is not None else ""

login = cherrypy.request.body.params['login']
Copy link
Member

Choose a reason for hiding this comment

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

If we're adding a param to a call, we should in its decorator rather than picking it out of the request like this.

@devbtech devbtech closed this Mar 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants