🏷️ Add mypy to the GitHub Action for tests and fixed types in the whole project #655
Conversation
estebanx64
changed the title
There was a problem hiding this comment.
Cool! I updated a couple of things and it's ready now.
I reverted the types in alembic, as that code is autogenerated and will keep increasing, without types. 🥲 ...and it's also excluded from mypy, so it's fine to leave it as is.
I updated the delete_user endpoint to move the errors to the top and put the main code outside of an if block.
I updated a bit the settings, there's a couple extra things to do but it's fine for now. I updated it to read from the .env file.
backend/app/api/routes/users.py
Outdated
| session.delete(user) | ||
| session.commit() | ||
| return Message(message="User deleted successfully") | ||
| elif user == current_user and current_user.is_superuser: | ||
| raise HTTPException( | ||
| status_code=403, detail="Super users are not allowed to delete themselves" | ||
| ) | ||
|
|
||
| return Message(message="User does not match the conditions to be deleted") |
There was a problem hiding this comment.
I was checking this part, and I see it would be better to move the errors to the top, error out early, that way the rest of the "happy path" of the code can run outside of any if statements.
This also handles that this last Message would be returned as a 200 OK, but it should be an error... although this line would really never be reached. By refactoring the code this way, there's no need to do tricks to convince mypy that it's all good.
There was a problem hiding this comment.
Oh yes, sorry I forgot to add the status code 400 at that response
There was a problem hiding this comment.
It was a lot better the refactor you made, thanks!
|
Thanks! 🚀 ☕ |
…le project (fastapi#655) Co-authored-by: Sebastián Ramírez <[email protected]>
No description provided.