Skip to content

Conversation

@sinisaos
Copy link
Member

Enabled user creation and editing in Piccolo Admin

@codecov-commenter
Copy link

codecov-commenter commented Jun 25, 2022

Codecov Report

Merging #158 (b643e95) into master (ef105f7) will increase coverage by 0.12%.
The diff coverage is 89.47%.

@@            Coverage Diff             @@
##           master     #158      +/-   ##
==========================================
+ Coverage   92.10%   92.23%   +0.12%     
==========================================
  Files          32       32              
  Lines        1774     1789      +15     
==========================================
+ Hits         1634     1650      +16     
+ Misses        140      139       -1     
Impacted Files Coverage Δ
piccolo_api/crud/endpoints.py 95.37% <89.47%> (+0.40%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Comment on lines 811 to 818
self.table._validate_password(password=cleaned_data["password"])
cleaned_data["password"] = self.table.hash_password(
cleaned_data["password"]
)
model = self.pydantic_model(**cleaned_data)
user_row = self.table(**model.dict())
await user_row.save().run()
return Response(status_code=200)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your idea. You might be able to do this instead:

user = await self.table.create_user(**model.dict())
json = dump_json({'id': user.id})
return CustomJSONResponse(json, status_code=201)

It still validates the password and hashes it. Also, it returns the data in the same format as other tables.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works perfectly and is much simpler. Thanks.

@sinisaos
Copy link
Member Author

@dantownsend I think we don't need this and this anymore.

@dantownsend
Copy link
Member

@sinisaos Yes, they can be removed - don't think they'll be used.

@dantownsend
Copy link
Member

@sinisaos I'm a bit confused why MyPy is failing in GitHub Actions. It works OK for me locally. Is it the same for you?

@dantownsend
Copy link
Member

The MyPy errors are when the latest version of fastapi is installed.

@dantownsend dantownsend merged commit d0fe8ee into piccolo-orm:master Sep 26, 2022
@dantownsend
Copy link
Member

@sinisaos Sorry for the delay on this - I thought we already merged it in.

@sinisaos
Copy link
Member Author

@dantownsend No worries. When you have time, look at the other PRs in Piccolo Api, and also in Piccolo Admin, where we have several stale PRs.

@sinisaos sinisaos deleted the user_actions_in_admin branch September 27, 2022 04:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants