anitab-org / mentorship-backend

Mentorship System is an application that matches women in tech to mentor each other, on career development, through 1:1 relations during a certain period of time. This is the backend of this system.
https://mentorship-backend-temp.herokuapp.com/
GNU General Public License v3.0
196 stars 449 forks source link

Bug: HTTPStatus CONFLICT is returned instead of BAD_REQUEST for invalid input POST /register #653

Closed mtreacy002 closed 3 years ago

mtreacy002 commented 4 years ago

Describe the bug

HTTPStatus.CONFLICT is returned for invalid input instead of HTTPStatus.BAD_REQUEST

To Reproduce

Steps to reproduce the behavior:

  1. Run python run.py command
  2. Go to Swagger UI and try to register new user with invalid input
  3. You will see error message with 409 CONFLICT instead of BAD_REQUEST 400 being returned. You can also see the expected response on Swagger UI that have invalid input returned as HTTPStatus.CONFLICT (409)

Expected behavior The error return on invalid inputs need to be HTTPStatus.BAD_REQUEST (400) instead of CONFLICT (409)

Screenshots

Screen Shot 2020-06-28 at 2 03 47 pm

Desktop (please complete the following information):

Additional context This is caused by the recent PR#650 that got merged to develop.

mtreacy002 commented 4 years ago

@anitab-org/bridgeintech-maintainers and @anitab-org/coding-team . Just want to report a bug that is raised by the recent merged PR #650 . The PR was supposed to refactor 400 response to 409 on the events where user inserted existing username or email only. Despite the code being refactored correctly on the app/api/dao/user.py,

Screen Shot 2020-06-28 at 2 17 14 pm

The responses shown on the Swagger UI got some unwanted changes. The invalid token or invalid input should be kept as HTTPStatus.BAD_REQUEST (2 bottom red boxes)

Screen Shot 2020-06-28 at 2 17 18 pm

HTTPStatus.CONFLICT is supposed to be used only for the event when username or email already exist as clearly explained in issue #619. So, line 261-268 on original code need to be separated into 2 responses, HTTPStatus.BAD_REQUEST for line 267-268 (on original) and HTTPStatus.CONFLICT for line 265-266 (on original).

Screen Shot 2020-06-28 at 2 24 33 pm
nandini45 commented 4 years ago

@mtreacy002 testcases have to be added can i do the work?

mtreacy002 commented 4 years ago

Sure, @nandini45 . You can add the test cases in this same issue instead of having them separately. Let me know if you are unclear of the task required here. 😉

mtreacy002 commented 4 years ago

Before working on the test cases. Make sure you make the relevant changes mentioned in the issue description and my comment above.

nandini45 commented 4 years ago

@mtreacy002 i will update my work asap.. sorry for the delay..i am working on it

mtreacy002 commented 4 years ago

No worries, @nandini45 , take your time. Let me know if there's anything you're not sure of.

nandini45 commented 4 years ago

image

i was trying to test the code after making the changes but an error raised. what am i doing wrong? @satya7289

NenadPantelic commented 3 years ago

@mtreacy002 Is this still active? I could work on this :smile:

mtreacy002 commented 3 years ago

@nandini45, can you let us know if you're still working on this issue or is it ok if @NenadPantelic pick it up now?

nandini45 commented 3 years ago

I am not working. Sorry for the inconvenience U can assign it to nenand

On 6 Jan 2021 9:29 a.m., "Maya Treacy" notifications@github.com wrote:

@nandini45 https://github.com/nandini45, can you let us know if you're still working on this issue or is it ok if @NenadPantelic https://github.com/NenadPantelic pick it up now?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/anitab-org/mentorship-backend/issues/653#issuecomment-755062882, or unsubscribe https://github.com/notifications/unsubscribe-auth/AOZMDIJHPUS2FD4UWJOHEDDSYPN3TANCNFSM4OKK2UOA .

vj-codes commented 3 years ago

Assigning you @NenadPantelic Happy coding!

NenadPantelic commented 3 years ago

@mtreacy002 @vjcodes Hi, I made a PR regarding this issue. Can you please review it? Thanks :slightly_smiling_face: