Fixes #22301: Avoid reverse relation name collision among tagged models (#22323)

netbox/extras/managers.py

@@ -1,10 +1,11 @@
 from django.db import router
 from django.db.models import signals
-from taggit.managers import _TaggableManager
+from taggit.managers import TaggableManager, _TaggableManager
 from taggit.utils import require_instance_manager
 
 __all__ = (
     'NetBoxTaggableManager',
+    'NetBoxTaggableManagerField',
 )
 
 
@@ -65,3 +66,29 @@ class NetBoxTaggableManager(_TaggableManager):
             pk_set=new_ids,
             using=db,
         )
+
+
+class NetBoxTaggableManagerField(TaggableManager):
+    """
+    Subclass of taggit's TaggableManager that interpolates `%(app_label)s` and `%(class)s` in
+    `related_name`. taggit's contribute_to_class() bypasses Django's RelatedField, which is what
+    normally performs this substitution, so without this two taggable models that share a class
+    name (e.g. from different plugins) collide on Tag's reverse accessor.
+    """
+    def contribute_to_class(self, cls, name):
+        super().contribute_to_class(cls, name)
+        if not cls._meta.abstract and self.remote_field.related_name:
+            self.remote_field.related_name = self.remote_field.related_name % {
+                'class': cls.__name__.lower(),
+                'app_label': cls._meta.app_label.lower(),
+            }
+
+    def deconstruct(self):
+        # Emit the upstream taggit path and omit related_name so existing migrations remain
+        # equivalent and no AlterField is produced for every TagsMixin consumer. related_name
+        # has no effect on the database schema; it is reapplied on model load. Only safe while
+        # this subclass adds no field attributes that affect schema — if that changes, restore
+        # the real path so migrations capture the diff.
+        name, _path, args, kwargs = super().deconstruct()
+        kwargs.pop('related_name', None)
+        return name, 'taggit.managers.TaggableManager', args, kwargs

netbox/extras/tests/test_tags.py

@@ -1,7 +1,12 @@
+from django.apps import apps
+from django.db import models
+from django.test import SimpleTestCase
 from django.urls import reverse
 from rest_framework import status
 
 from dcim.models import Site
+from extras.managers import NetBoxTaggableManagerField
+from netbox.models.features import TagsMixin
 from utilities.testing import APITestCase, create_tags
 
 
@@ -77,3 +82,47 @@ class TaggedItemTestCase(APITestCase):
         self.assertEqual(len(response.data['tags']), 0)
         site = Site.objects.get(pk=response.data['id'])
         self.assertEqual(len(site.tags.all()), 0)
+
+
+class TagsMixinCollisionTestCase(SimpleTestCase):
+    """
+    Two TagsMixin-derived models that share a class name (e.g. introduced by separate plugins)
+    must not collide on Tag's reverse accessor. Regression test for #22301.
+    """
+    def _make_taggable_model(self, app_label, name='Foo'):
+        # Build a model dynamically and register a cleanup to remove it from the global app
+        # registry so subsequent test runs (and other tests in this process) don't see ghost
+        # models leak in via apps.get_models().
+        model = type(name, (TagsMixin, models.Model), {
+            '__module__': self.__class__.__module__,
+            'Meta': type('Meta', (), {'app_label': app_label}),
+        })
+        self.addCleanup(apps.all_models[app_label].pop, name.lower(), None)
+        self.addCleanup(apps.clear_cache)
+        return model
+
+    def test_same_named_taggable_models_in_different_apps(self):
+        foo_a = self._make_taggable_model('dcim')
+        foo_b = self._make_taggable_model('ipam')
+
+        # Without the fix, Django's system checks raise fields.E304 on these two models.
+        self.assertEqual(foo_a.check(), [])
+        self.assertEqual(foo_b.check(), [])
+
+        # Each model should resolve to its own unique related_name on the Tag relation.
+        rn_a = foo_a._meta.get_field('tags').remote_field.related_name
+        rn_b = foo_b._meta.get_field('tags').remote_field.related_name
+        self.assertNotEqual(rn_a, rn_b)
+
+    def test_deconstruct_emits_upstream_path(self):
+        """
+        NetBoxTaggableManagerField.deconstruct() must emit the upstream taggit path and drop
+        related_name, so existing migrations remain valid and no AlterField is produced for
+        every TagsMixin consumer.
+        """
+        model = self._make_taggable_model('dcim', name='DeconstructProbe')
+        field = model._meta.get_field('tags')
+        self.assertIsInstance(field, NetBoxTaggableManagerField)
+        _name, path, _args, kwargs = field.deconstruct()
+        self.assertEqual(path, 'taggit.managers.TaggableManager')
+        self.assertNotIn('related_name', kwargs)

netbox/netbox/models/features.py

@@ -9,13 +9,12 @@ from django.db import models
 from django.db.models import Q
 from django.utils import timezone
 from django.utils.translation import gettext_lazy as _
-from taggit.managers import TaggableManager
 
 from core.choices import JobStatusChoices, ObjectChangeActionChoices
 from core.models import ObjectType
 from extras.choices import *
 from extras.constants import CUSTOMFIELD_EMPTY_VALUES
-from extras.managers import NetBoxTaggableManager
+from extras.managers import NetBoxTaggableManager, NetBoxTaggableManagerField
 from extras.utils import is_taggable
 from netbox.config import get_config
 from netbox.constants import CORE_APPS
@@ -489,12 +488,15 @@ class JournalingMixin(models.Model):
 class TagsMixin(models.Model):
     """
     Enables support for tag assignment. Assigned tags can be managed via the `tags` attribute,
-    which is a `NetBoxTaggableManager` instance.
+    which is a `NetBoxTaggableManager` instance. The field is a `NetBoxTaggableManagerField`,
+    which performs `%(app_label)s` / `%(class)s` interpolation on `related_name` to avoid
+    reverse-accessor collisions between same-named models in different apps (e.g. plugins).
     """
-    tags = TaggableManager(
+    tags = NetBoxTaggableManagerField(
         through='extras.TaggedItem',
         ordering=('weight', 'name'),
         manager=NetBoxTaggableManager,
+        related_name='%(app_label)s_%(class)s_tagged+',
     )
 
     class Meta:

netbox/utilities/testing/base.py

@@ -165,7 +165,9 @@ class ModelTestCase(TestCase):
                 continue
 
             # Handle ManyToManyFields
-            if value and type(field) in (ManyToManyField, ManyToManyRel, TaggableManager):
+            if value and (
+                type(field) in (ManyToManyField, ManyToManyRel) or isinstance(field, TaggableManager)
+            ):
                 # Resolve reverse M2M relationships
                 if isinstance(field, ManyToManyRel):
                     value = getattr(instance, field.related_name).all()

netbox/utilities/testing/filtersets.py

@@ -82,7 +82,7 @@ class BaseFilterSetTests:
             return [(f'{filter_name}_id', django_filters.ModelMultipleChoiceFilter)]
 
         # Tag manager
-        if type(field) is TaggableManager:
+        if isinstance(field, TaggableManager):
             return [('tag', TagFilter)]
 
         # Unable to determine the correct filter class