Full clean builtin #141
Merged
Full clean builtin #141
Conversation
This file contains 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
At least, for Binder models this is the case. All models managed by Binder views should inherit from BinderModel, and if not, the user should code validation error checking in _store(). In all our projects we use django-fullclean because being able to accidentally save invalid models is a huge mind fuck. Thus, it is pointless to call full_clean() in our views, as it means we'll be running potentially expensive validations twice. For example, relation fields check that the foreign relation exists, which requires a query per relation field. It is technically a breaking change, but for most projects this will be a non-issue. The test suite of Boekestijn passed except for one test which was specifically for its User view, which uses a non-Binder model. This was relatively easy to convert to use a full_clean() in its _store() method.
Codecov Report
@@ Coverage Diff @@
## master #141 +/- ##
==========================================
- Coverage 78.19% 78.12% -0.08%
==========================================
Files 24 24
Lines 3252 3264 +12
==========================================
+ Hits 2543 2550 +7
- Misses 709 714 +5
Continue to review full report at Codecov.
|
sjamaan
force-pushed
the
full-clean-builtin
branch
from
da93764 to
b5ec096
Compare
sjamaan
pushed a commit
that referenced
this pull request
Sep 1, 2020
This leverages our custom full_clean implementation (#141) in combination with the LoadedValues mixin. This allows us to know when a value did not change and avoid validating the foreign key just before save, because the foreign key was fine as it was stored, so it cannot suddenly become invalid. For complex models with many relations or which are saved a lot, this makes a big difference, as the reduced number of queries can make things quite a bit more efficient.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This implements #128 by calling
full_cleaninBinderModel'ssave()method.