Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
rkrn opened this issue Nov 18, 2020 · 1 comment
Open

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

rkrn opened this issue Nov 18, 2020 · 1 comment

Comments

@rkrn
Copy link

rkrn commented Nov 18, 2020

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
Copy link

There is already a MR #346 to fix this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants