Mentorship-backend: Use HTTPStatus instead of hard coded status codes in resources/mentorship_relation.py file

Created on 8 Sep 2020  路  15Comments  路  Source: anitab-org/mentorship-backend

Describe the bug
mentorship_relation file has few routes that uses hardcoded status codes instead of HTTPStatus.

To Reproduce

Steps to reproduce the behavior:

  1. Go to https://mentorship-backend-temp.herokuapp.com/
  2. Click on Mentorship Relation
  3. Scroll down to /mentorship_relation/{request_id}
  4. See error

Expected behavior
HTTPStatus.XYZ should have been displayed instad of raw status codes

Screenshots
image
The above image shows hardcoded status codes. It should show the status in the below format:-
image

Additional Context
Also, find whether any such hard coded status codes are used for other routes and fix them.

Coding First Timers Only

Most helpful comment

Assigning @sreelekha901 since she is not assigned to any issue and asked first.

All 15 comments

Can I work on this issue?

currently i have no issue assigned...i would like to work on this

I would like to work on this

Can I work on this?

Assigning @sreelekha901 since she is not assigned to any issue and asked first.

I need some help to proceed with this issue

What do you need help with @sreelekha901 ?

@PrashanthPuneriya can you please update the issue title and description to fix all hard coded status codes instead of mentorship_relation file

@PrashanthPuneriya @isabelcosta can I take this up?

I don't think that this issue is valid anymore. Can you please reproduce it first @Aaishpra ?

Just a comment, actually, the value being shown on Swagger UI should be in number format: so 200 instead of HTTPStatus.OK. In the code it should use the HTTPStatus enum, but it should show the real number in the Swagger UI. I think this issue is not about that, but it is instead about if is there numbers hardcoded in the code (instead of enums). so this should still be checked how @devkapilbansal said. Thank you for coming with this @devkapilbansal

We should additionally have an issue to make sure the SwaggerUI shows the values and not the enum representation. that is an implementation detail. the solution might be something like using str(HTTPStatus.OK) or HTTPStatus.OK.value, not sure 馃

@devkapilbansal you are right this issue is not valid anymore, I reproduced it in the Heroku app and the status codes are displayed correctly for mentorship relations, #924 already solved it.

Just a comment, actually, the value being shown on Swagger UI should be in number format: so 200 instead of HTTPStatus.OK. In the code it should use the HTTPStatus enum, but it should show the real number in the Swagger UI. I think this issue is not about that, but it is instead about if is there numbers hardcoded in the code (instead of enums). so this should still be checked how @devkapilbansal said. Thank you for coming with this @devkapilbansal

We should additionally have an issue to make sure the SwaggerUI shows the values and not the enum representation. that is an implementation detail. the solution might be something like using str(HTTPStatus.OK) or HTTPStatus.OK.value, not sure thinking

@isabelcosta @devkapilbansal shall I open the issue and work on it according to as @isabelcosta suggested.

Ok so if this is not valid anymore I will close this @devkapilbansal @Aaishpra

@Aaishpra yes, you can create the issue for it and work on it :) Try to create an issue for a module of the code. For example, you can create an issue to do what I mentioned for Admin API or Mentorship Relation API or User API. So that the file changes in the code review are "small" and easier to review. Makes sense?

You can start with an issue for showing the values on Admin API, and then if the others are still available you continue those. Just let me know once you create the issue so we can assign it to you :)

Ok so if this is not valid anymore I will close this @devkapilbansal @Aaishpra

@Aaishpra yes, you can create the issue for it and work on it :) Try to create an issue for a module of the code. For example, you can create an issue to do what I mentioned for Admin API or Mentorship Relation API or User API. So that the file changes in the code review are "small" and easier to review. Makes sense?

You can start with an issue for showing the values on Admin API, and then if the others are still available you continue those. Just let me know once you create the issue so we can assign it to you :)

Thanks, @isabelcosta got your point will create an issue for the mentorship_relation Api first and work on it and after that we can create more for the remaining APIs.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

isabelcosta picture isabelcosta  路  47Comments

PrashanthPuneriya picture PrashanthPuneriya  路  45Comments

paritoshsinghrahar picture paritoshsinghrahar  路  24Comments

Shivansh2407 picture Shivansh2407  路  25Comments

epicadk picture epicadk  路  19Comments