Closes #18663: Replace assertions with proper error handling (#22344)

netbox/dcim/forms/bulk_import.py

@@ -1583,7 +1583,8 @@ class CableImportForm(PrimaryModelImportForm):
 
         :param side: 'a' or 'b'
         """
-        assert side in 'ab', f"Invalid side designation: {side}"
+        if side not in ('a', 'b'):
+            raise ValueError(_("Invalid side designation: {side}").format(side=side))
 
         device = self.cleaned_data.get(f'side_{side}_device')
         power_panel = self.cleaned_data.get(f'side_{side}_power_panel')

netbox/dcim/models/cables.py

@@ -675,7 +675,10 @@ class CableTermination(ChangeLoggedModel):
         Cache objects related to the termination (e.g. device, rack, site) directly on the object to
         enable efficient filtering.
         """
-        assert self.termination is not None
+        if self.termination is None:
+            raise ValueError(
+                _("Invalid cable termination: the assigned termination object does not exist.")
+            )
 
         # Device components
         if getattr(self.termination, 'device', None):

netbox/dcim/tests/test_forms.py

@@ -503,6 +503,15 @@ class InterfaceTestCase(TestCase):
         self.assertNotIn('qinq_svlan', form.cleaned_data.keys())
 
 
+class CableTestCase(TestCase):
+
+    def test_invalid_side_designation_raises_value_error(self):
+        """_clean_side rejects a side other than 'a' or 'b' with ValueError."""
+        form = CableImportForm.__new__(CableImportForm)
+        with self.assertRaisesMessage(ValueError, "Invalid side designation: c"):
+            form._clean_side('c')
+
+
 class SiteFormTestCase(TestCase):
     """
     Tests for M2MAddRemoveFields using Site ASN assignments as the test case.

netbox/dcim/tests/test_models.py

@@ -2189,6 +2189,18 @@ class CableTestCase(TestCase):
         self.assertIsNone(interface.path)
 
 
+class CableTerminationTestCase(TestCase):
+
+    def test_cache_related_objects_requires_resolvable_termination(self):
+        """cache_related_objects raises ValueError when the termination cannot be resolved."""
+        cable_termination = CableTermination(
+            termination_type=ObjectType.objects.get_for_model(Interface),
+            termination_id=0,
+        )
+        with self.assertRaises(ValueError):
+            cable_termination.cache_related_objects()
+
+
 class VirtualDeviceContextTestCase(TestCase):
 
     @classmethod

netbox/extras/events.py

@@ -125,7 +125,13 @@ def enqueue_event(queue, instance, request, event_type):
     app_label = instance._meta.app_label
     model_name = instance._meta.model_name
 
-    assert instance.pk is not None
+    if instance.pk is None:
+        raise ValueError(
+            _("Cannot enqueue an event for an unsaved {app_label}.{model} instance.").format(
+                app_label=app_label,
+                model=model_name,
+            )
+        )
     key = f'{app_label}.{model_name}:{instance.pk}'
 
     if key in queue:

netbox/extras/tests/test_event_rules.py

@@ -39,6 +39,16 @@ class EventRuleTestCase(APITestCase):
         # Clear the queue so leftover jobs do not leak to the next test suite
         self.queue.empty()
 
+    def test_enqueue_event_requires_saved_instance(self):
+        """enqueue_event raises ValueError for an unsaved instance."""
+        request = RequestFactory().get('/')
+        request.id = uuid.uuid4()
+        request.user = self.user
+        site = Site(name='Site 1', slug='site-1')
+        with patch('extras.events.has_feature', return_value=True):
+            with self.assertRaises(ValueError):
+                enqueue_event({}, site, request, OBJECT_CREATED)
+
     @classmethod
     def setUpTestData(cls):
 

netbox/netbox/models/deletion.py

@@ -3,6 +3,7 @@ import logging
 from django.contrib.contenttypes.fields import GenericRelation
 from django.db import router
 from django.db.models.deletion import CASCADE, Collector
+from django.utils.translation import gettext as _
 
 logger = logging.getLogger("netbox.models.deletion")
 
@@ -45,7 +46,7 @@ class CustomCollector(Collector):
 
         # Add GenericRelations to the dependency graph
         processed_relations = set()
-        for _, instances in list(self.data.items()):
+        for _model, instances in list(self.data.items()):
             for instance in instances:
                 # Get all GenericRelations for this model
                 for field in instance._meta.private_fields:
@@ -70,10 +71,13 @@ class DeleteMixin:
         Override delete to use our custom collector.
         """
         using = using or router.db_for_write(self.__class__, instance=self)
-        assert self._get_pk_val() is not None, (
+        if self._get_pk_val() is None:
-            f"{self._meta.object_name} object can't be deleted because its "
+            raise ValueError(
-            f"{self._meta.pk.attname} attribute is set to None."
+                _("{object_name} object can't be deleted because its {pk_attname} attribute is set to None.").format(
-        )
+                    object_name=self._meta.object_name,
+                    pk_attname=self._meta.pk.attname,
+                )
+            )
 
         collector = CustomCollector(using=using)
         collector.collect([self], keep_parents=keep_parents)

netbox/netbox/tests/test_models.py

@@ -4,6 +4,7 @@ from django.conf import settings
 from django.test import TestCase
 
 from core.models import ObjectChange
+from dcim.models import Site
 from netbox.tests.dummy_plugin.models import DummyNetBoxModel
 
 
@@ -21,3 +22,12 @@ class ModelTestCase(TestCase):
         m.pk = 123
 
         self.assertEqual(m.get_absolute_url(), f'/plugins/dummy-plugin/netboxmodel/{m.pk}/')
+
+
+class DeleteMixinTestCase(TestCase):
+
+    def test_delete_unsaved_instance_raises_value_error(self):
+        """Deleting an instance with no primary key raises ValueError."""
+        site = Site(name='Site 1', slug='site-1')
+        with self.assertRaises(ValueError):
+            site.delete()

netbox/netbox/views/generic/bulk_views.py

@@ -7,7 +7,7 @@ from types import SimpleNamespace
 from django.conf import settings
 from django.contrib import messages
 from django.contrib.contenttypes.fields import GenericForeignKey, GenericRel
-from django.core.exceptions import FieldDoesNotExist, ObjectDoesNotExist, ValidationError
+from django.core.exceptions import FieldDoesNotExist, ImproperlyConfigured, ObjectDoesNotExist, ValidationError
 from django.db import IntegrityError, router, transaction
 from django.db.models import ManyToManyField, ProtectedError, RestrictedError
 from django.db.models.fields.reverse_related import ManyToManyRel
@@ -731,7 +731,10 @@ class BulkEditView(GetReturnURLMixin, BaseMultiObjectView):
 
             # Update custom fields
             for name, customfield in custom_fields.items():
-                assert name.startswith('cf_')
+                if not name.startswith('cf_'):
+                    raise ImproperlyConfigured(
+                        _("Custom field form field name must begin with 'cf_': {name}").format(name=name)
+                    )
                 cf_name = name[3:]  # Strip cf_ prefix
                 if name in form.nullable_fields and name in nullified_fields:
                     obj.custom_field_data[cf_name] = None

netbox/utilities/choices.py

@@ -1,6 +1,7 @@
 import enum
 
 from django.conf import settings
+from django.core.exceptions import ImproperlyConfigured
 from django.utils.translation import gettext_lazy as _
 
 from utilities.data import get_config_value_ci
@@ -20,9 +21,10 @@ class ChoiceSetMeta(type):
 
         # Extend static choices with any configured choices
         if key := attrs.get('key'):
-            assert type(attrs['CHOICES']) is list, _(
+            if type(attrs['CHOICES']) is not list:
-                "{name} has a key defined but CHOICES is not a list"
+                raise ImproperlyConfigured(
-            ).format(name=name)
+                    _("{name} has a key defined but CHOICES is not a list").format(name=name)
+                )
             app = attrs['__module__'].split('.', 1)[0]
             replace_key = f'{app}.{key}'
             replace_choices = get_config_value_ci(settings.FIELD_CHOICES, replace_key)

netbox/utilities/tests/test_choices.py

@@ -1,3 +1,4 @@
+from django.core.exceptions import ImproperlyConfigured
 from django.test import TestCase, override_settings
 
 from utilities.choices import ChoiceSet
@@ -31,6 +32,19 @@ class ChoiceSetTestCase(TestCase):
     def test_values(self):
         self.assertListEqual(ExampleChoices.values(), ['a', 'b', 'c', 1, 2, 3])
 
+    def test_key_with_non_list_choices_raises(self):
+        """A ChoiceSet declaring a key must define CHOICES as a list."""
+        with self.assertRaises(ImproperlyConfigured):
+            type(
+                'InvalidChoices',
+                (ChoiceSet,),
+                {
+                    '__module__': __name__,
+                    'key': 'invalid_choices',
+                    'CHOICES': (('foo', 'Foo'),),
+                },
+            )
+
 
 class FieldChoicesCaseInsensitiveTestCase(TestCase):
     """