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: Fixed the error shown in PUT/user Updates user profile #855

Closed

Conversation

pallavisavant
Copy link
Contributor

@pallavisavant pallavisavant commented Sep 9, 2020

Description

Removed 400:Bad Request shown when user uses same username while updating users profile and changed it to 200:user was updated successfully ,allowing users to change the details even if they use same username and update other details removing bug in the current logic which makes it compulsory for user to change their username on each update . No 400:ERROR will be shown but the changed details will be reflected in users updated profile.

PS:If user can even change username on update along with other details or they can also use the already existing username and update other details.

Fixes #596

Type of Change:

  • Code
  • Documentation

Code/Quality Assurance Only

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Before this commit we would get the following result that is 400:Bad Request when user uses already existing username

Before_commit

  • After this commit it shows 200:users updated successfully even if the user uses already existing username and changes other details

After_commit

Checklist:

  • My PR follows the style guidelines of this project
  • I have performed a self-review of my own code or materials
  • I have commented my code or provided relevant documentation, particularly in hard-to-understand areas
  • Update Swagger documentation and the exported file at /docs folder

Code/Quality Assurance Only

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

…ers\test_update_username_already_taken() that proves my fix is effective
…est_update_username_already_taken() that proves my fix is effective
@codecov
Copy link

codecov bot commented Sep 9, 2020

Codecov Report

Merging #855 into develop will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #855      +/-   ##
===========================================
- Coverage    95.84%   95.84%   -0.01%     
===========================================
  Files           95       95              
  Lines         5199     5198       -1     
===========================================
- Hits          4983     4982       -1     
  Misses         216      216              
Impacted Files Coverage Δ
app/api/dao/user.py 85.02% <100.00%> (-0.07%) ⬇️
tests/users/test_api_update_user.py 98.98% <100.00%> (ø)

@pallavisavant
Copy link
Contributor Author

@vj-codes why are the checks failing over here🤔can anyone please review and let me know if i need to make any changes.....Thank you😊.

@mtreacy002 mtreacy002 added Category: Coding Changes to code base or refactored code that doesn't fix a bug. Status: Needs Review PR needs an additional review or a maintainer's review. labels Oct 14, 2020
@mtreacy002 mtreacy002 requested review from isabelcosta, SanketDG, a team and sunjunkie and removed request for a team October 14, 2020 12:40
@vj-codes vj-codes added First Timers Only Good for newcomers. and removed Category: Coding Changes to code base or refactored code that doesn't fix a bug. labels Nov 4, 2020
@devkapilbansal
Copy link
Member

@pallavisavant please resolve the merge conflicts

@vj-codes
Copy link
Member

@pallavisavant hey any updates abt resolving merge conflicts?

@vj-codes vj-codes added Status: Changes Requested Changes are required to be done by the PR author. and removed First Timers Only Good for newcomers. Status: Needs Review PR needs an additional review or a maintainer's review. labels Jan 10, 2021
@pallavisavant
Copy link
Contributor Author

@pallavisavant hey any updates abt resolving merge conflicts?

Hey hi sry for delayed reply, I rgt now don't have the setup on my system will have to set it up again, if someone who already has it can resolve the conflicts it would be g8, I'll see if I can set up then I'll resolve 🙂

@devkapilbansal
Copy link
Member

@pallavisavant any updates??

@vj-codes
Copy link
Member

@pallavisavant I will close this PR and make the issue available due to inactivity , thank you for your contribution :)

@vj-codes vj-codes closed this Feb 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Changes Requested Changes are required to be done by the PR author.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update the user with existing name and username.
4 participants