tiangolo / full-stack-fastapi-template

Full stack, modern web application template. Using FastAPI, React, SQLModel, PostgreSQL, Docker, GitHub Actions, automatic HTTPS and more.
MIT License
24.9k stars 4.22k forks source link

User update CRUD method only supports updates with password field #318

Open rkrn opened 3 years ago

rkrn commented 3 years ago

This line doesn't seem to be doing its intended purpose.

https://github.com/tiangolo/full-stack-fastapi-postgresql/blob/490c554e23343eec0736b06e59b2108fdd057fdc/%7B%7Bcookiecutter.project_slug%7D%7D/backend/app/app/crud/crud_user.py#L34

With this, if the password key is not set, then it will raise a KeyError, so the only viable code path is within the if statement. At this moment none of the demo update endpoints will run into this error as they happen to only pass UserUpdate objects to this method.

However it seems the intended purpose of the update method is to support any general update to the User database object, including updates to any single field besides the password field with a dict, e.g. obj_in can be {"full_name": "New name"}:

https://github.com/tiangolo/full-stack-fastapi-postgresql/blob/490c554e23343eec0736b06e59b2108fdd057fdc/%7B%7Bcookiecutter.project_slug%7D%7D/backend/app/app/crud/base.py#L50-L56

A simple change to:

if update_data.get("password"):
    ...

will remain compatible while achieving the intended purpose.

thomas-chauvet commented 3 years ago

There is already a MR #346 to fix this.