Fix similar_objects() in multi inheritance contexts (KeyError) #712
Conversation
Codecov Report
@@ Coverage Diff @@
## master #712 +/- ##
==========================================
+ Coverage 91.43% 93.07% +1.64%
==========================================
Files 3 8 +5
Lines 432 621 +189
Branches 78 93 +15
==========================================
+ Hits 395 578 +183
- Misses 23 25 +2
- Partials 14 18 +4
Continue to review full report at Codecov.
|
|
At first glance it looks like a problem with the test scripts for 3.6 and 3.7 and not an issue with this PR. Can a JazzBand maintainer confirm? |
|
Test scripts for 3.6 and 3.7 are attempting to use a version of Django that no longer supports python 3.6 and 3.7. Looks like these test builds could be dropped, or they need to be edited to specify an older version of Django. |
| ) | ||
|
|
||
| class Meta: | ||
| unique_together = [["content_object", "tag"]] |
There was a problem hiding this comment.
instead of unique_together you could try UniqueContraint
There was a problem hiding this comment.
There was a problem hiding this comment.
There are about a half dozen occurrences of unique_together in this file, and none of them have been switched over to UniqueConstraint yet. I think for clarity, as this would be an unrelated change, it would be better to change all the unique-together to UniqueConstraint with its own issue / PR.
There was a problem hiding this comment.
UniqueConstraint was introduced in Django 2.2, so if I'm not mistaken, this would also mean the min version of Django would have to be bumped to 2.2 and version 1 support would have to be dropped. This is probably fine at this point, but it is also beyond the scope of this PR.
|
Django 4.0 has dropped support for python 3.6,3.7. |
tests/migrations/0001_initial.py
Outdated
| ("title", models.CharField(max_length=100)), | ||
| ], | ||
| ), | ||
| migrations.CreateModel( |
There was a problem hiding this comment.
did you created the migrations yourselves? are they going to break the downward migrations?
There was a problem hiding this comment.
did you created the migrations yourselves?
Yes.
are they going to break the downward migrations?
Please review them. The tests are passing.
|
@auvipy, please let me know if there is anything else I can do to help get this fix in. If there is some other way to demonstrate the migrations I added are correct, let me know and I'll try to do that over the weekend. |
|
#599 can also be closed when this is merged in. |
|
@auvipy I'm no longer actively using or maintaining django-taggit. Please feel free to review, merge, and release as you see fit. |
4 participants
This addresses issues wagtail users have had using
similar_objects()withClusterTaggableManagerThis PR would resolve both #424 and #508 and close issue #80.
It is an updated and rebased version of the commits made in #424 by nickhudkins.
It finishes the work in PR #424 in that drops the workarounds for older versions of Django as requested by @jdufresne in #424.