fix review comments

netbox/dcim/tests/test_signals.py

@@ -1,6 +1,7 @@
 from types import SimpleNamespace
 from unittest.mock import MagicMock, patch
 
+from django.apps import apps
 from django.contrib.contenttypes.models import ContentType
 from django.db import connection
 from django.test import SimpleTestCase, TestCase
@@ -30,6 +31,7 @@ from dcim.models import (
 from dcim.models.device_components import ComponentModel
 from dcim.models.mixins import CachedScopeMixin
 from ipam.models import Prefix
+from netbox.plugins import PluginConfig
 from virtualization.models import Cluster, ClusterType
 from wireless.models import WirelessLAN
 
@@ -573,11 +575,15 @@ class CableTerminationDenormalizationTriggerTestCase(TestCase):
 
 
 def _concrete_subclasses(base):
-    """Yield every non-abstract model descending from an abstract base model."""
+    """
+    Yield every non-abstract, non-plugin model descending from an abstract base model. Plugin-contributed
+    models are skipped: a plugin that adds a ComponentModel/CachedScopeMixin subclass is responsible for
+    its own trigger migration, and must not fail core's coverage check just by being installed.
+    """
     for subclass in base.__subclasses__():
         if subclass._meta.abstract:
             yield from _concrete_subclasses(subclass)
-        else:
+        elif not isinstance(apps.get_app_config(subclass._meta.app_label), PluginConfig):
             yield subclass
 
 
@@ -589,10 +595,11 @@ def _installed_triggers():
 
 class DenormalizationTriggerCoverageTestCase(TestCase):
     """
-    Guard against a new model silently shipping without its denormalization triggers. The set of
+    Guard against a new core model silently shipping without its denormalization triggers. The set of
     device-component tables and CachedScopeMixin dependents is hand-listed in migrations; this test
     derives those sets from the model layer and asserts the expected triggers are installed, so adding
-    a new component / scoped model without a matching trigger migration fails CI.
+    a new component / scoped model without a matching trigger migration fails CI. Plugin-contributed
+    models are excluded (see _concrete_subclasses).
     """
 
     def test_device_components_have_device_trigger(self):

netbox/utilities/migration.py

@@ -59,6 +59,9 @@ class InstallDenormalizationTrigger(migrations.operations.base.Operation):
             (`(cols) = (SELECT cols FROM table WHERE id = NEW.source_fk)`), so the related row is read once.
             This closes the chain gap when a denormalized column is derived through an intermediate object
             (e.g. a Location's Site change must refresh the dependent's region/site-group, not just its site).
+            If `source_fk` is NULL the subquery returns no row and all its target columns are set to NULL,
+            which is the correct result (the source object has no related object); current callers use a
+            non-nullable `source_fk` (Location.site), so this does not arise in practice.
 
     The trigger fires AFTER UPDATE of the watched source columns (the direct `mappings` sources plus each
     related `source_fk`), and only when at least one of them actually changed. It does not fire on INSERT (a
@@ -120,6 +123,9 @@ class InstallDenormalizationTrigger(migrations.operations.base.Operation):
             END
             $$ LANGUAGE plpgsql;
         ''')
+        # Drop first so the operation is idempotent (re-run / partially-applied migration, or a
+        # trigger pre-installed during testing); CREATE TRIGGER alone errors if one already exists.
+        schema_editor.execute(f'DROP TRIGGER IF EXISTS "{self.trigger_name}" ON "{self.source_table}";')
         schema_editor.execute(f'''
             CREATE TRIGGER "{self.trigger_name}"
                 AFTER UPDATE OF {update_of} ON "{self.source_table}"