Mentorship-backend: Bug: HTTPStatus CONFLICT is returned instead of BAD_REQUEST for invalid input POST /register

Created on 28 Jun 2020  Â·  12Comments  Â·  Source: anitab-org/mentorship-backend

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):

  • OS: MacOS
  • Browser: Chrome
  • Version ??

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

BIT Coding

Most helpful comment

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

All 12 comments

@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

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

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. 😉

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

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

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

image

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

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

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

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
.

Assigning you @NenadPantelic
Happy coding!

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

Was this page helpful?
0 / 5 - 0 ratings