Initial data validation support on object create / update#84
Open
radiophysicist wants to merge 3 commits intofastapi-admin:devfrom
Open
Initial data validation support on object create / update#84radiophysicist wants to merge 3 commits intofastapi-admin:devfrom
radiophysicist wants to merge 3 commits intofastapi-admin:devfrom
Conversation
Contributor
radiophysicist
commented
Sep 9, 2021
- In recourse model class method for data validation is extracted
- More error handling added in create / update request handlers
- Error message rendering added into create / update page templates
b4cf8e1 to
0cda518
Compare
0cda518 to
c7de145
Compare
|
Looks great! |
|
Is there a possibility we can merge this? |
laggron42
reviewed
Jul 27, 2023
Comment on lines
+93
to
+113
| if not error: | ||
| async with in_transaction() as conn: | ||
| obj = ( | ||
| await model.filter(pk=pk) | ||
| .using_db(conn) | ||
| .select_for_update() | ||
| .get() | ||
| .prefetch_related(*model_resource.get_m2m_field()) | ||
| ) | ||
| await obj.update_from_dict(data).save(using_db=conn) | ||
| for k, items in m2m_data.items(): | ||
| m2m_obj = getattr(obj, k) | ||
| await m2m_obj.clear() | ||
| if items: | ||
| await m2m_obj.add(*items) | ||
| obj = ( | ||
| await model.filter(pk=pk) | ||
| .using_db(conn) | ||
| .get() | ||
| .prefetch_related(*model_resource.get_m2m_field()) | ||
| ) |
Contributor
There was a problem hiding this comment.
I added this on my end to also pass Tortoise errors on edition and not just on creation
Suggested change
| if not error: | |
| async with in_transaction() as conn: | |
| obj = ( | |
| await model.filter(pk=pk) | |
| .using_db(conn) | |
| .select_for_update() | |
| .get() | |
| .prefetch_related(*model_resource.get_m2m_field()) | |
| ) | |
| await obj.update_from_dict(data).save(using_db=conn) | |
| for k, items in m2m_data.items(): | |
| m2m_obj = getattr(obj, k) | |
| await m2m_obj.clear() | |
| if items: | |
| await m2m_obj.add(*items) | |
| obj = ( | |
| await model.filter(pk=pk) | |
| .using_db(conn) | |
| .get() | |
| .prefetch_related(*model_resource.get_m2m_field()) | |
| ) | |
| if not error: | |
| try: | |
| async with in_transaction() as conn: | |
| obj = ( | |
| await model.filter(pk=pk) | |
| .using_db(conn) | |
| .select_for_update() | |
| .get() | |
| .prefetch_related(*model_resource.get_m2m_field()) | |
| ) | |
| await obj.update_from_dict(data).save(using_db=conn) | |
| for k, items in m2m_data.items(): | |
| m2m_obj = getattr(obj, k) | |
| await m2m_obj.clear() | |
| if items: | |
| await m2m_obj.add(*items) | |
| obj = ( | |
| await model.filter(pk=pk) | |
| .using_db(conn) | |
| .get() | |
| .prefetch_related(*model_resource.get_m2m_field()) | |
| ) | |
| except BaseORMException as exc: | |
| error = str(exc) |
laggron42
suggested changes
Jul 27, 2023
Contributor
There was a problem hiding this comment.
Hey, I tested your PR and, after solving a few conflicts, turns out great!
I copied logic from create over update to also pass validation errors to object editions.
Edit: @long2ice I have rebased and solved conflicts on this branch if you're interested in this PR, currently running in prod just fine!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.