bug 1331643, 1330357: Avoid duplicate tags with TAGGIT_CASE_INSENSITIVE=True #4103
Conversation
|
I've opened PR jazzband/django-taggit#461 to fix the issue upstream, and will apply the fixes to this branch with the assumption that a future version of |
jwhitlock
force-pushed
the
taggit_ci_1331643
branch
from
3eb3cfe
to
a390cce
Compare
Current coverage is 86.20% (diff: 100%)@@ master #4103 diff @@
==========================================
Files 143 143
Lines 8887 8886 -1
Methods 0 0
Messages 0 0
Branches 1192 1192
==========================================
- Hits 7661 7660 -1
Misses 989 989
Partials 237 237
|
jwhitlock
force-pushed
the
taggit_ci_1331643
branch
2 times, most recently
from
826cf93
to
9d5a7a3
Compare
|
Need to update |
| django-taggit==0.18.0 \ | ||
| --hash=sha256:7010a1f7597d954aaf8b8174563f2e3455520db949d9c5d360dd9e4f906dd32a \ | ||
| --hash=sha256:4ddd61928a4fc6ba7ccb1145d8d3a6d5c2829de8d8738eea49924ade8969f0ce | ||
| django-taggit==0.21.3 \ |
There was a problem hiding this comment.
with @jwhitlock patch, 0.21.5 has been released
There was a problem hiding this comment.
Not released: https://pypi.python.org/pypi/django-taggit
jwhitlock
force-pushed
the
taggit_ci_1331643
branch
from
9d5a7a3
to
fc73328
Compare
|
Unfortunately, I introduced a bug in django-taggit 0.21.5. I've opened an issue (jazzband/django-taggit#463) and a PR (jazzband/django-taggit#464) to fix it. Meanwhile, the manual tag creation continues to bypass the buggy code, so leaving it in. I'd like to merge and deploy this PR, fixing the ISE bugs, and follow up with a new PR when django-taggit 0.21.6 is released. |
|
Its better to use your fork until the patch is merged rather than using custom code for manual tag creating |
|
Upstream patch is merged and published |
Tell django-taggit to use case insensitive lookups for existing tags. We're using MySQL with a case insensitive collation, so this is already done at the database level, but this may help the code work correctly with the database.
Add tests for bug 1331643, where Revisions with existing "duplicate" tags (equal after adjusting for case insensitivity) set the Document tags.
Test bug 1330357, where Kuma code lowercases profile tags and tries to add a tag that collides with the mixed case tag.
django-taggit 0.18.0->0.21.5: Many changes, including: * Deprecate old and add new Django version support * Implement m2m_changed signal * Improvements for use in Django admin * Fix case-insensitive duplicates when creating tags This fixes the xfailed tests.
jwhitlock
force-pushed
the
taggit_ci_1331643
branch
from
fc73328
to
1d0f303
Compare
|
@frewsxcv thanks for the quick merge and publish! Rebased on current master, removed manual tag creation, crossing fingers and waiting for TravisCI 🤞 Update: Tests pass! |
With bug 1293749 fixed (Add missing
UNIQUE KEYindexes in production), some new issues have appeared:Both appear to be caused by an issue with django-taggit and MySQL with a case-insensitive collation and a unique index. Adding two "case similar" tags, like "WEB" and "web", will result in attempting to add both, and raise an IntegrityError.
The issue is handled in this PR by querying the database for existing tags with "case similar" names, and creating new ones as needed, and then passing the unique set of Tag instances to taggit to create the many-to-many relations, rather than passing a list of names.
Additionally, user profile interests and expertise were restricted to lower-case tags to avoid triggering the creation of duplicate tags. This removes the restriction and UI changes, so that non-lowercase interests, like "HTML" and "CSS", can be added. This change allows mixed-case profile tags, but existing tags will need to be updated in the database as desired.
This is an alternate for PR #4100 that uses
TAGGIT_CASE_INSENSITIVE=True, and also avoids django-taggit issue jazzband/django-taggit#460.