Fixes #406: Resolve correct to_field_name for CSV/YAML/JSON bulk import

netbox_custom_objects/field_types.py

@@ -44,6 +44,58 @@
 # PostgreSQL's hard limit for identifier names is 63 bytes.
 _PG_MAX_IDENTIFIER_LEN = 63
 
+# Ordered list of field names tried when resolving the natural text identifier for a
+# model during CSV/YAML/JSON bulk import.  The first name that exists on the model
+# wins.  This handles models like ModuleType that use 'model' rather than 'name'.
+_CSV_IDENTIFIER_FIELD_PRECEDENCE = ('name', 'slug', 'model', 'identifier')
+
+
+def _csv_import_to_field_name(model_class, explicit=None):
+    """Return the field name to use as ``to_field_name`` for CSV import lookups.
+
+    If *explicit* is provided (e.g. from a stored field configuration), it is
+    validated against the model and returned if the field exists.  If the stored
+    value is stale (field was renamed on the target model), a warning is logged
+    and the function falls through to the probe loop.
+
+    The probe loop iterates ``_CSV_IDENTIFIER_FIELD_PRECEDENCE`` and returns the
+    first candidate that exists **and is unique** on the model, guaranteeing that
+    ``CSVModelChoiceField`` can resolve a single record.  If no unique match is
+    found the first existing candidate (unique or not) is used as a fallback.
+    Falls back to ``'pk'`` when none of the candidates exist at all.
+    """
+    if explicit is not None:
+        try:
+            model_class._meta.get_field(explicit)
+            return explicit
+        except FieldDoesNotExist:
+            logger.warning(
+                'Stored to_field_name %r not found on %s; probing for a natural identifier.',
+                explicit, model_class.__name__,
+            )
+
+    first_match = None  # best non-unique candidate, used only if no unique field found
+    for candidate in _CSV_IDENTIFIER_FIELD_PRECEDENCE:
+        try:
+            field_obj = model_class._meta.get_field(candidate)
+            if getattr(field_obj, 'unique', False):
+                return candidate
+            if first_match is None:
+                first_match = candidate
+        except FieldDoesNotExist:
+            pass
+
+    if first_match is not None:
+        return first_match
+
+    logger.warning(
+        'No natural identifier field found on %s for CSV import '
+        '(tried %s); falling back to pk.',
+        model_class.__name__,
+        ', '.join(_CSV_IDENTIFIER_FIELD_PRECEDENCE),
+    )
+    return 'pk'
+
 
 def _safe_pg_identifier(full_name: str) -> str:
     """
@@ -721,12 +773,10 @@ def get_form_field(self, field, for_csv_import=False, **kwargs):
 
         if for_csv_import:
             field_class = CSVModelChoiceField
-            # For CSV import, determine to_field_name from the field configuration
-            to_field_name = getattr(field, 'to_field_name', None) or 'name'
+            to_field_name = _csv_import_to_field_name(model, explicit=getattr(field, 'to_field_name', None))
             return field_class(
                 queryset=model.objects.all(),
                 required=field.required,
-                # Remove initial=field.default to allow Django to handle instance data properly
                 to_field_name=to_field_name,
             )
         else:
@@ -1239,8 +1289,7 @@ def get_form_field(self, field, for_csv_import=False, **kwargs):
 
         if for_csv_import:
             field_class = CSVModelMultipleChoiceField
-            # For CSV import, determine to_field_name from the field configuration
-            to_field_name = getattr(field, 'to_field_name', None) or 'name'
+            to_field_name = _csv_import_to_field_name(model, explicit=getattr(field, 'to_field_name', None))
             return field_class(
                 queryset=model.objects.all(),
                 required=field.required,

netbox_custom_objects/tests/test_field_types.py

@@ -5,15 +5,17 @@
 from unittest.mock import Mock
 from datetime import date, datetime
 from decimal import Decimal
-from django.core.exceptions import ValidationError
+from django.core.exceptions import FieldDoesNotExist, ValidationError
 from django.test import TestCase
 
 from core.models import ObjectType
+from dcim.models import Device, DeviceType, ModuleType
 from netbox_custom_objects.field_types import (
     MultiObjectFieldType,
     MultiSelectFieldType,
     ObjectFieldType,
     SelectFieldType,
+    _csv_import_to_field_name,
 )
 from netbox_custom_objects.models import CustomObjectType, CustomObjectTypeField
 from .base import CustomObjectsTestCase
@@ -802,6 +804,61 @@ def test_object_field_model_generation(self):
         instance = model.objects.create(name="Test", device=device)
         self.assertEqual(instance.device, device)
 
+    def test_csv_import_field_uses_name_for_models_with_name(self):
+        """get_form_field(for_csv_import=True) uses 'name' as to_field_name for models
+        that have a name field (e.g. Device)."""
+        self.assertEqual(_csv_import_to_field_name(Device), 'name')
+
+    def test_csv_import_field_uses_model_for_module_type(self):
+        """get_form_field(for_csv_import=True) uses 'model' as to_field_name for
+        ModuleType, which has no 'name' field — regression for issue #406."""
+        self.assertEqual(_csv_import_to_field_name(ModuleType), 'model')
+
+    def test_csv_import_field_uses_slug_before_model_for_device_type(self):
+        """DeviceType has both 'slug' and 'model'; slug takes precedence per the
+        _CSV_IDENTIFIER_FIELD_PRECEDENCE ordering, matching NetBox core behaviour."""
+        self.assertEqual(_csv_import_to_field_name(DeviceType), 'slug')
+
+    def test_csv_import_form_field_for_module_type_uses_model_field(self):
+        """get_form_field(for_csv_import=True) on an Object field referencing ModuleType
+        produces a CSVModelChoiceField with to_field_name='model', not 'name'."""
+        module_type_ot = ObjectType.objects.get(app_label='dcim', model='moduletype')
+        cotf = self.create_custom_object_type_field(
+            self.custom_object_type,
+            name="module_type",
+            label="Module Type",
+            type="object",
+            related_object_type=module_type_ot,
+        )
+        form_field = ObjectFieldType().get_form_field(cotf, for_csv_import=True)
+        self.assertEqual(form_field.to_field_name, 'model',
+                         "ModuleType CSV import must use 'model', not 'name'")
+
+    def test_csv_import_to_field_name_explicit_returned_when_valid(self):
+        """An explicit to_field_name that exists on the model is returned directly."""
+        self.assertEqual(_csv_import_to_field_name(Device, explicit='id'), 'id')
+
+    def test_csv_import_to_field_name_explicit_falls_through_when_stale(self):
+        """A stale explicit to_field_name (field no longer exists) triggers a warning
+        and falls through to the probe loop."""
+        import logging
+        with self.assertLogs('netbox_custom_objects.field_types', level=logging.WARNING):
+            result = _csv_import_to_field_name(Device, explicit='nonexistent_field_xyz')
+        # Falls through to probe loop — Device has 'name'
+        self.assertEqual(result, 'name')
+
+    def test_csv_import_to_field_name_pk_fallback_when_no_candidate_exists(self):
+        """Falls back to 'pk' and emits a warning when none of the candidate fields
+        exist on the model."""
+        import logging
+        from unittest.mock import MagicMock
+        mock_model = MagicMock()
+        mock_model.__name__ = 'FakeModel'
+        mock_model._meta.get_field.side_effect = FieldDoesNotExist
+        with self.assertLogs('netbox_custom_objects.field_types', level=logging.WARNING):
+            result = _csv_import_to_field_name(mock_model)
+        self.assertEqual(result, 'pk')
+
 
 class MultiObjectFieldTypeTestCase(FieldTypeTestCase):
     """Test cases for multiobject field type."""
@@ -881,6 +938,21 @@ def test_multiobject_field_model_generation(self):
         self.assertIn(device1, instance.devices.all())
         self.assertIn(device2, instance.devices.all())
 
+    def test_csv_import_form_field_for_module_type_uses_model_field(self):
+        """MultiObjectFieldType.get_form_field(for_csv_import=True) on a field
+        referencing ModuleType produces to_field_name='model', not 'name'."""
+        module_type_ot = ObjectType.objects.get(app_label='dcim', model='moduletype')
+        cotf = self.create_custom_object_type_field(
+            self.custom_object_type,
+            name="module_types",
+            label="Module Types",
+            type="multiobject",
+            related_object_type=module_type_ot,
+        )
+        form_field = MultiObjectFieldType().get_form_field(cotf, for_csv_import=True)
+        self.assertEqual(form_field.to_field_name, 'model',
+                         "ModuleType multi-object CSV import must use 'model', not 'name'")
+
 
 class SelfReferentialFieldTestCase(FieldTypeTestCase):
     """Test cases for self-referential object fields.