-
Notifications
You must be signed in to change notification settings - Fork 449
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
Update the user with existing name and username. #596
Comments
I added the Status: On Hold since I thought of including this issue for the upcoming Open Source Hack. |
I would like to work on this issue for OSH. |
Hi @alapan-sau, this issue will be made available once Open Source Hack starts on September 1st, 2020. Till then you can set up the development environment for and familiarise yourself with mentorship-backend. If you've any doubts feel free to ask them on Zulip. |
As OSH is open today, May I work on this issue? |
Hi @alapan-sau, you've already been assigned issue #746, once you're done with sending a PR for the same, you can work on available issues. |
I would like to work on this issue... Can you please assign this to me for Open-Source Hack? |
Sure @faznan3nazer, here you go |
@faznan3nazer making it available as you are already assigned #606 |
can i work on this issue ...Thanks. |
Assigning you @pallavisavant |
@vj-codes I am facing one issue while reproducing this....we get the access token by giving username and password in login section rgt,and after that we are supposed to use this token for updating ...it shows invalid token wherever i am using it.Can anyone help ..Thanks. |
Assigning you @epicadk |
Should I also handle race conditions? @isabelcosta @faznan3nazer |
@epicadk which race conditions? |
Currently , we first check if the username is available or not and then proceed to updating it . However this isn't how it should be implemented because what happens when two users try to register with the same username and almost the same time? It would lead to an unexpected error. So we shouldn't query the database to check if the username if available or not but rather just update the username and handle the exception if a user with that username already exists. |
@isabelcosta I can't seem to find the link to the resource where I read about this but I can try to explain here. So consider 2 users user1 and user2. Suppose both of them want to ( in this case) change their existing username to the new same username. So what'll happen is first user1's request will check if the username is already taken and it will find that the username does not exist in the database, at the same time user2's request can also query the database to check if the username exists and since user1's data hasn't been updated yet user2 will also find that the username is available. Consider User1's data is now updated however when user2 attempts to update their data an exception will be thrown and the app will crash if this is not handled. |
I'm sorry that probably doesn't explain it well enough maybe I can try in the mentorship open session. |
No problem @epicadk we'll discuss in the meeting :) |
Actually this is working just fine. The documentation is not clear enough maybe you only have to send the data you want to change. So say you only want to change location from Perth to Sydney then you would only send |
Un-assigning myself as the required behavior needs to be discussed before changes are made. The endpoint does what is claims to do. If we do expect all the user data to be present then the documentation needs to be changed as well. Adding this to the mentorship system agenda. cc @isabelcosta |
I think remember this being the issue of PUT vs PATCH 🤔 just putting here so I don't forget later |
Can I work on this? @isabelcosta |
Hi, is anyone working on this? Could I be assigned to work on it if not? @vj-codes |
Assigning you @hirshrm |
When you are updating the user with existing name and username, you are getting an error message "A user with that username already exists."
To Reproduce
Steps to reproduce the behavior:
Current username: zxcvbnm
Updating username to: zxcvbnm,
Current name: zxcvbnm
Updating name to: zxcvbnm,
"name": "zxcvbnm",
"username": "zxcvbnm",
"bio": "string",
"location": "string",
"occupation": "string",
"organization": "string",
"slack_username": "string",
"social_media_links": "string",
"skills": "string",
"interests": "string",
"resume_url": "string",
"photo_url": "string",
"need_mentoring": true,
"available_to_mentor": true
}
Expected behavior
It should either be successfully updated message or a message like "no fields have been updated". Also the error message being shown now is not listed under responses.
Desktop (please complete the following information):
The text was updated successfully, but these errors were encountered: