Fix FileFields in deeply nested inlines #127
Conversation
Codecov Report
@@ Coverage Diff @@
## master #127 +/- ##
==========================================
+ Coverage 78.48% 78.56% +0.08%
==========================================
Files 17 17
Lines 2031 2053 +22
==========================================
+ Hits 1594 1613 +19
- Misses 437 440 +3
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #127 +/- ##
==========================================
+ Coverage 78.48% 78.57% +0.08%
==========================================
Files 17 17
Lines 2031 2058 +27
==========================================
+ Hits 1594 1617 +23
- Misses 437 441 +4
Continue to review full report at Codecov.
|
The 'move_by_offset' method has been used in a single instance, it was called with parameters: (0, -1), but the resulting move was: (0, -16). This commit inlines 'move_by_offset' and deletes the method.
Updates offset of '_match_helper_with_target' from (0, -16) to (0, -15). Without this, adding a new element to NestedStackedInline would fail test suite.
Django 2.1+ implemented 'has_file_field', this value no longer is hardcoded to 'True', but depends on 'is_multipart()'. Django relies on 'has_file_field' to set form's enctype to multipart. With false negatives in 'has_file_field', FileField changes are not saved. This commit adds support for nested_formsets to 'is_multipart()'.
|
This is really phenomenal! I hope you'll excuse the fact that I made a few tweaks to your patch. In particular, I saw your commit for django/django@7c3a8b9 and it occurred to me that we could include the check for nested formsets in a custom |
|
Yours is the first pull request to ever include tests, and I can't express how much I appreciate that you spent the time writing that. |
|
Excellent. Thank you @btknu very much for that. |
|
I'm happy to see that it was helpful, thanks for the review, tweaks and the merge! |
Fixes #111 .
Some highlights:
Django 2.1+ implemented
has_file_field, this value no longer is hardcoded toTrue, but depends onis_multipart(). Django relies onhas_file_fieldto set form's enctype to multipart.With false negatives in
has_file_field, FileField changes are not saved. This commit adds support for nested_formsets tois_multipart().Django 2.1 and 2.2 use Form's
is_multipartto calculatehas_file_field. This could be improved, there's already a dedicated FormSet'sis_multipartwhich handles a more general case. Django 3 should already use FormSet (see latest master). This PR includes code to handle both scenarios.The variable name
_has_multipart_nested_formsetfeels a bit strange; I picked it because it's similar tohas_file_fieldandis_multipart, I'm open to other suggestions and I can change it if needed.Updated after forced push:
In the early version of this PR the tests used to be in the
tests.two_deep.tests.InlineAdminTestCaseMixin, this way they were run for both Stacked and Tabular inlines. I've moved them toTestStackedInlineAdminonly; TestTabularInlineAdmin test cases were failing due to elements becoming wider and overlapping UI buttons. However, the nesting style doesn't seem to matter in this case.I've added a minor cleanup to the
DragAndDropAction, more details in commit messages.