search

jazzband / django-taggit

Simple tagging for django
https://django-taggit.readthedocs.io
BSD 3-Clause "New" or "Revised" License
3.33k stars 622 forks source link

TaggedItemBase related_name not filling in %(app_label)s_%(class)s_items since Django 4.2 LTS? #861

Closed robvdl closed 1 year ago

robvdl commented 1 year ago

I upgraded a Wagtail site recently and because of that ended up on Django 4.2 LTS.

I noticed it detected changes in my models when it shouldn't have. Upon closer inspection the issue was caused by the tag field in TaggedItemBase in models.py. The tag field has related_name="%(app_label)s_%(class)s_items" and in my initial migration of my app this gets resolved to related_name='blog_blogpostpagetag_items'.

However in the new version of Django 4.2 LTS, it seems to no longer be filling in %(app_label)s and %(class)s and it wants to change related_name='blog_blogpostpagetag_items' to related_name="%(app_label)s_%(class)s_items" which doesn't seem right to me.

What I'm going to do in the mean time at least is hardcode the related_name until this is resolved.

I have not yet reported this with Wagtail as it is in this project really, not Wagtail.

robvdl commented 1 year ago

I'm just wanting to know if this is actually a problem. I have a feeling it isn't actually a problem but the migrations just need updating.

I guess one test would be to check if that backref is still working, and if it is then it's probably just a matter of updating the migrations.

robvdl commented 1 year ago

I tested if the backref is still working and it is still working just fine using the old name.

I tried some_tag.blog_blogpostpagetag_items.all() and it worked the same, so this is a non-issue.

So perhaps this is just how Django now represents this in migrations and I will update my migrations, considering this is a non-issue.

I'm happy to close this issue, or perhaps someone else will run into this.

rtpg commented 1 year ago

Thanks for the report! This indeed sounds like an issue with django-taggit. Going to attempt a reproduction of this issue

robvdl commented 1 year ago

I don't think there is anything you can do about it other than to update the migration.

It's a non-issue as per my last comment and it isn't Taggit, it's Django 4.2+ that does this

robvdl commented 1 year ago

I'm happy to close this issue as I long moved on by just updating my initial migration.

rtpg commented 1 year ago

At the very least this is something where I would have wanted to put in documentation to confirm that this was indeed a thing that happens. Having to manage migrations on library upgrades, I'm often having to go in and figure out why something has changed or not.

It seems like this change happened in the 3.2 -> 4.0 move (how I figured this out: installing various Djangos, then running PYTHON_PATH=$(pwd) DJANGO_SETTINGS='tests' django-admin makemigrations --dry-run --check to find where we started to add a bunch of stuff. You can also see the difference by checking Through2.tag.field.deconstruct() and seeing that related_name is filled in in 3.2, and is an untouched template in 4.0).

The related Django change is in the 4.0 release notes:

As a side-effect, running makemigrations might generate no-op AlterField operations for ManyToManyField and ForeignKey fields in some cases.

Reading through this though, in practice it sounds like something where we would notice the issue while upgrading to Django 4 on the "real" project.

I'm going to set up a tests that tests for changes to the test file migrations, so we can at least signal this sort of stuff in the release notes (even if it's redundant)