Fix CSVModelMultipleChoiceField's own export/import round trip; docs; hardening

- CSVModelMultipleChoiceField.clean() split on a bare comma with no
  whitespace stripping, but ManyToManyColumn's default CSV export
  separator is ", " (comma + space) -- so re-importing NetBox's own CSV
  export of any multi-value column using this field (module_bay_types
  among others, since this is a shared utility field) failed with
  "Object not found:  <value>" on every value after the first. Verified
  directly against ModuleTypeTable's actual export value before fixing.
  Also cast to str() before splitting: a YAML-bound caller (as opposed to
  a CSV cell, always a string) can pass a non-string scalar, which
  previously raised an unhandled AttributeError instead of a form error.

- Docs for module bay type resolution still described the pre-a3b5e4b
  fallback ("then any remaining candidate"); updated to describe the
  refusal behavior that replaced it. Added a matching note to
  modulebay.md, which had none.

- dedupe_module_bay_types_by_manufacturer() collapses candidates by pk
  within each name group before computing preference, so a caller
  passing a duplicate row in a raw list -- the signature accepts "an
  iterable," not just a queryset -- can't manufacture a same-manufacturer
  tie that would then crash on None.manufacturer.name. Unreachable via
  the three current callers today (each resolves from a queryset,
  which can't contain a row twice), but cheap to make the helper safe
  standalone.

- Fixed a stale test docstring contrasting the two import forms' field
  types by a distinction (plain vs. CSV multiple-choice field) that no
  longer exists since both were aligned to CSVModelMultipleChoiceField.

- Added ambiguity-refusal coverage at the other two call sites
  (ModuleBayTemplateImportForm, ModuleBayImportForm) -- previously only
  ModuleTypeImportForm was covered for this path.

Also found independently while verifying the above: ModuleTypeListView
.export_yaml() prefetched modulebaytemplates__module_bay_types, but
ModuleType.to_yaml() -- unlike DeviceType.to_yaml() -- never reads
self.modulebaytemplates at all (a separate, pre-existing, out-of-scope
gap: ModuleType.to_yaml() doesn't export a nested module-bays section).
That prefetch was dead weight, adding a query with no corresponding
saving. Removed it, and with it the now-meaningless "bay count doesn't
affect query count" test (nothing in ModuleType.to_yaml() ever varied
with bay count to begin with), replacing it with an exact-delta
assertion isolating what the one relevant prefetch (module_bay_types
on the module type itself) actually saves.

docs/models/dcim/modulebay.md

@@ -34,6 +34,8 @@ The numeric position in which this module bay is situated. For example, this wou
 
 Zero or more [module bay types](./modulebaytype.md) assigned to this bay. When at least one bay type is set, only module types that share a common bay type may be installed. Leave empty to allow any module type.
 
+Bay types are importable via CSV, referenced by name, with the same manufacturer-based resolution described for [module bay templates](./modulebaytemplate.md) -- scoped to the manufacturer of the module bay's own device.
+
 ### Enabled
 
 Whether this module bay is enabled. Disabled module bays are not available for installation.

docs/models/dcim/modulebaytemplate.md

@@ -4,4 +4,4 @@ A template for a module bay that will be created on all instantiations of the pa
 
 [Bay types](./modulebaytype.md) assigned to a module bay template are copied to each instantiated module bay, so constraints defined on the device type propagate automatically to all devices of that type.
 
-Bay types are importable and exportable as part of a device type's YAML definition (`module-bays[].module_bay_types`), referenced by name. A bay type belonging to a manufacturer other than the device type's own may be referenced; this mirrors the UI and REST API, which likewise place no manufacturer restriction on the assignment. Since a bay type's uniqueness is scoped to `(manufacturer, name)` rather than name alone, more than one bay type may share a name; import prefers, in order, an exact match on the device type's own manufacturer, then a global (manufacturer-less) bay type, then any remaining candidate.
+Bay types are importable and exportable as part of a device type's YAML definition (`module-bays[].module_bay_types`), referenced by name. A bay type belonging to a manufacturer other than the device type's own may be referenced; this mirrors the UI and REST API, which likewise place no manufacturer restriction on the assignment. Since a bay type's uniqueness is scoped to `(manufacturer, name)` rather than name alone, more than one bay type may share a name; import prefers, in order, an exact match on the device type's own manufacturer, then a global (manufacturer-less) bay type. If a name instead matches two or more bay types belonging to *other* manufacturers, with neither the device type's own manufacturer nor a global type available to break the tie, the import is rejected rather than resolved to an arbitrary one.

docs/models/dcim/moduletype.md

@@ -91,7 +91,7 @@ The assigned [profile](./moduletypeprofile.md) for the type of module. Profiles
 
 Zero or more [module bay types](./modulebaytype.md) that this module type is compatible with. When at least one bay type is set, the module type may only be installed into bays that share a common type. Leave empty to allow installation into any bay.
 
-Bay types are importable and exportable as part of a module type's YAML definition (`module_bay_types`), referenced by name. A bay type belonging to a manufacturer other than the module type's own may be referenced -- e.g. a third-party module declaring compatibility with another manufacturer's proprietary bay type -- mirroring the UI and REST API, which likewise place no manufacturer restriction on the assignment. Since a bay type's uniqueness is scoped to `(manufacturer, name)` rather than name alone, more than one bay type may share a name; import prefers, in order, an exact match on the module type's own manufacturer, then a global (manufacturer-less) bay type, then any remaining candidate.
+Bay types are importable and exportable as part of a module type's YAML definition (`module_bay_types`), referenced by name. A bay type belonging to a manufacturer other than the module type's own may be referenced -- e.g. a third-party module declaring compatibility with another manufacturer's proprietary bay type -- mirroring the UI and REST API, which likewise place no manufacturer restriction on the assignment. Since a bay type's uniqueness is scoped to `(manufacturer, name)` rather than name alone, more than one bay type may share a name; import prefers, in order, an exact match on the module type's own manufacturer, then a global (manufacturer-less) bay type. If a name instead matches two or more bay types belonging to *other* manufacturers, with neither the module type's own manufacturer nor a global type available to break the tie, the import is rejected rather than resolved to an arbitrary one.
 
 ### Attributes
 

netbox/dcim/tests/test_forms.py

@@ -17,6 +17,7 @@ from dcim.choices import (
 )
 from dcim.forms import *
 from dcim.models import *
+from dcim.tables.modules import ModuleTypeTable
 from dcim.tests.test_module_moves import fail_after
 from ipam.models import ASN, RIR, VLAN
 from utilities.exceptions import AbortRequest
@@ -395,6 +396,24 @@ class ModuleBayTemplateImportFormTestCase(TestCase):
         self.assertTrue(reimport_form.is_valid(), reimport_form.errors)
         self.assertEqual(list(reimport_form.save().module_bay_types.all()), [cisco_bay_type])
 
+    def test_module_bay_types_rejects_ambiguous_name_across_two_foreign_manufacturers(self):
+        juniper = Manufacturer.objects.create(name='Juniper', slug='juniper')
+        cisco = Manufacturer.objects.create(name='Cisco', slug='cisco')
+        arista = Manufacturer.objects.create(name='Arista', slug='arista')
+        ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
+        ModuleBayType.objects.create(name='SFP28', slug='sfp28-arista', manufacturer=arista)
+        device_type = DeviceType.objects.create(
+            manufacturer=juniper, model='Juniper Device Type 2', slug='juniper-device-type-2',
+        )
+
+        form = ModuleBayTemplateImportForm({
+            'device_type': device_type.pk,
+            'name': 'Module Bay 1',
+            'module_bay_types': ['SFP28'],
+        })
+        self.assertFalse(form.is_valid())
+        self.assertIn('module_bay_types', form.errors)
+
 
 class ModuleBayImportFormTestCase(TestCase):
     """
@@ -431,6 +450,21 @@ class ModuleBayImportFormTestCase(TestCase):
         self.assertTrue(form.is_valid(), form.errors)
         self.assertEqual(list(form.save().module_bay_types.all()), [own_type])
 
+    def test_module_bay_types_rejects_ambiguous_name_across_two_foreign_manufacturers(self):
+        device = create_test_device('Module Bay Import Device')
+        cisco = Manufacturer.objects.create(name='Cisco', slug='cisco')
+        arista = Manufacturer.objects.create(name='Arista', slug='arista')
+        ModuleBayType.objects.create(name='SFP28', slug='sfp28-cisco', manufacturer=cisco)
+        ModuleBayType.objects.create(name='SFP28', slug='sfp28-arista', manufacturer=arista)
+
+        form = ModuleBayImportForm({
+            'device': device.name,
+            'name': 'Bay 1',
+            'module_bay_types': 'SFP28',
+        })
+        self.assertFalse(form.is_valid())
+        self.assertIn('module_bay_types', form.errors)
+
 
 class ModuleTypeImportFormTestCase(TestCase):
 
@@ -469,10 +503,9 @@ class ModuleTypeImportFormTestCase(TestCase):
 
     def test_module_bay_types_accepts_csv_comma_separated_string(self):
         """
-        Unlike ModuleBayTemplateImportForm.module_bay_types (a plain ModelMultipleChoiceField,
-        bound only from YAML-parsed lists), this form's module_bay_types is a
-        CSVModelMultipleChoiceField because ModuleTypeImportForm also serves plain CSV bulk
-        import, where the cell value arrives as a comma-separated string rather than a list.
+        module_bay_types is a CSVModelMultipleChoiceField because ModuleTypeImportForm also
+        serves plain CSV bulk import, where the cell value arrives as a comma-separated
+        string rather than a list.
         """
         manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
         bay_type_a = ModuleBayType.objects.create(name='SFP28', slug='sfp28')
@@ -502,6 +535,51 @@ class ModuleTypeImportFormTestCase(TestCase):
         self.assertTrue(form.is_valid(), form.errors)
         self.assertFalse(form.save().module_bay_types.exists())
 
+    def test_module_bay_types_round_trips_through_the_table_column_export_value(self):
+        """
+        ManyToManyColumn's export separator is ", " (comma + space, django-tables2's
+        default), not the bare "," this field's CSVModelMultipleChoiceField splits on -- so
+        re-importing NetBox's own CSV export of a multi-value module_bay_types column must
+        not fail on the leading space of every value after the first.
+        """
+        manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
+        bay_type_a = ModuleBayType.objects.create(name='SFP28', slug='sfp28')
+        bay_type_b = ModuleBayType.objects.create(name='QSFP28', slug='qsfp28')
+        original = ModuleType.objects.create(manufacturer=manufacturer, model='Module Type 1')
+        original.module_bay_types.set([bay_type_a, bay_type_b])
+
+        table = ModuleTypeTable([original])
+        exported_value = table.columns['module_bay_types'].column.value(original.module_bay_types.all())
+        self.assertEqual(exported_value, 'QSFP28, SFP28')
+
+        form = ModuleTypeImportForm({
+            'manufacturer': manufacturer.name,
+            'model': 'Module Type 2',
+            'module_bay_types': exported_value,
+        })
+        self.assertTrue(form.is_valid(), form.errors)
+        self.assertEqual(
+            set(form.save().module_bay_types.values_list('name', flat=True)),
+            {bay_type_a.name, bay_type_b.name},
+        )
+
+    def test_module_bay_types_non_string_scalar_is_a_validation_error_not_a_crash(self):
+        """
+        A CSV cell is always a string, but this field is also bound from YAML-parsed data
+        (e.g. via ModuleBayTemplateImportForm), where a scalar column can arrive as a
+        non-string (an int or bool). .split() on that would raise AttributeError -- an
+        unhandled 500 rather than a form error.
+        """
+        manufacturer = Manufacturer.objects.create(name='Manufacturer 1', slug='manufacturer-1')
+
+        form = ModuleTypeImportForm({
+            'manufacturer': manufacturer.name,
+            'model': 'Module Type 1',
+            'module_bay_types': 100,
+        })
+        self.assertFalse(form.is_valid())
+        self.assertIn('module_bay_types', form.errors)
+
     def test_module_bay_types_permits_a_different_manufacturers_type(self):
         """
         The UI (ModuleTypeForm) and REST API place no manufacturer restriction on

netbox/dcim/tests/test_views.py

@@ -1796,48 +1796,18 @@ module-bays:
         self.assertEqual(mb1.position, '1')
         self.assertEqual(list(mb1.module_bay_types.values_list('name', flat=True)), ['SFP28'])
 
-    def test_bulk_yaml_export_module_bay_types_query_count_is_constant(self):
-        """
-        ModuleTypeListView.export_yaml() prefetches both module_bay_types (on the module type
-        itself) and modulebaytemplates__module_bay_types (on its nested module bay templates),
-        so neither adds a query per module bay template as the number of bays grows.
-        """
-        manufacturer = Manufacturer.objects.create(name='Export Query Manufacturer', slug='export-query-mfr')
-        bay_type = ModuleBayType.objects.create(name='Export Query SFP28', slug='export-query-sfp28')
-
-        def make_module_type(model_name, bay_count):
-            module_type = ModuleType.objects.create(manufacturer=manufacturer, model=model_name)
-            module_type.module_bay_types.set([bay_type])
-            for i in range(bay_count):
-                bay = ModuleBayTemplate.objects.create(module_type=module_type, name=f'Bay {i}')
-                bay.module_bay_types.set([bay_type])
-            return module_type
-
-        one_bay_module_type = make_module_type('Export Query MT One Bay', 1)
-        five_bay_module_type = make_module_type('Export Query MT Five Bays', 5)
-
-        view = ModuleTypeListView()
-        view.queryset = ModuleType.objects.filter(pk=one_bay_module_type.pk)
-        with CaptureQueriesContext(connection) as one_bay_queries:
-            view.export_yaml()
-
-        view.queryset = ModuleType.objects.filter(pk=five_bay_module_type.pk)
-        with CaptureQueriesContext(connection) as five_bay_queries:
-            view.export_yaml()
-
-        self.assertEqual(len(one_bay_queries), len(five_bay_queries))
-
     def test_bulk_yaml_export_prefetches_module_bay_types_on_the_module_type_itself(self):
         """
-        Companion to test_..._is_constant above: that test holds the module type count fixed
-        at 1 and varies bay count, so it can't detect a regression in the module_bay_types
-        prefetch on ModuleType itself (which saves one query per module TYPE row, not per
-        bay) -- a 1-row queryset can't show a per-row saving. Comparing module-type COUNTS
-        (e.g. 1 vs. 5) doesn't work either: to_yaml() touches several other per-instance
-        relations (manufacturer, port_mappings, ...) that legitimately scale with row count
-        regardless of this fix, which would swamp the signal. Instead, compare the *same*
-        5-row queryset with and without the module_bay_types prefetch, isolating exactly what
-        it saves.
+        Comparing module-type COUNTS (e.g. 1 vs. 5) to detect a per-row prefetch saving
+        doesn't work: to_yaml() touches several other per-instance relations (manufacturer,
+        port_mappings, ...) that legitimately scale with row count regardless of this fix,
+        which would swamp the signal. Instead, compare the *same* 5-row queryset with and
+        without the module_bay_types prefetch, isolating exactly what it saves.
+
+        Unlike DeviceType.to_yaml(), ModuleType.to_yaml() does not export a nested
+        module-bays section at all (a separate, pre-existing gap, out of scope here), so
+        module_bay_types is the only relation ModuleTypeListView.export_yaml() needs to
+        prefetch -- confirmed by this test's exact-delta assertion below.
         """
         manufacturer = Manufacturer.objects.create(name='Export Query MT Manufacturer', slug='export-query-mt-mfr')
         bay_type = ModuleBayType.objects.create(name='Export Query MT SFP28', slug='export-query-mt-sfp28')
@@ -1858,12 +1828,8 @@ module-bays:
             view.export_yaml()
 
         # Without the prefetch, each of the 5 module types issues its own module_bay_types
-        # query. The exact delta isn't asserted -- prefetching modulebaytemplates (even when
-        # empty, as here) also lets to_yaml()'s .exists() check on that relation short-circuit
-        # from the prefetch cache instead of querying, so the totals reflect more than just
-        # module_bay_types -- but dropping the module_bay_types prefetch can only narrow this
-        # gap, never widen it, so a strict inequality still catches that regression.
-        self.assertGreater(len(unprefetched), len(prefetched))
+        # query; with it, exactly one query serves all 5.
+        self.assertEqual(len(unprefetched) - len(prefetched), 4)
 
     @override_settings(STREAMING_EXPORTS=True)
     def test_export_objects(self):

netbox/dcim/utils.py

@@ -37,12 +37,16 @@ def dedupe_module_bay_types_by_manufacturer(module_bay_types, manufacturer=None)
             return 1
         return 2
 
-    by_name = defaultdict(list)
+    by_name = defaultdict(dict)
     for module_bay_type in module_bay_types:
-        by_name[module_bay_type.name].append(module_bay_type)
+        # Keyed by pk within each name group so a caller passing the same row twice (the
+        # three current callers never do, since each resolves from a queryset, but the
+        # signature accepts any iterable) can't manufacture a same-manufacturer "tie" below.
+        by_name[module_bay_type.name][module_bay_type.pk] = module_bay_type
 
     resolved = []
-    for name, candidates in by_name.items():
+    for name, candidates_by_pk in by_name.items():
+        candidates = list(candidates_by_pk.values())
         best_rank = min(preference(c) for c in candidates)
         best = [c for c in candidates if preference(c) == best_rank]
         if len(best) > 1:

netbox/dcim/views.py

@@ -1912,15 +1912,13 @@ class ModuleTypeListView(generic.ObjectListView):
     table = tables.ModuleTypeTable
 
     def export_yaml(self):
-        # to_yaml() reads module_bay_types directly, plus each nested module bay template's
-        # own module_bay_types -- prefetch both so these relations don't add a query per
-        # module type/module bay template across the whole queryset. to_yaml()'s other
-        # component-template relations (interfaces, ports, etc.) are unprefetched here as
-        # they were before these relations existed, and remain their own N+1 across a large
-        # export.
-        self.queryset = self.queryset.prefetch_related(
-            'module_bay_types', 'modulebaytemplates__module_bay_types',
-        )
+        # to_yaml() reads module_bay_types directly -- unlike DeviceType.to_yaml(), it does
+        # not export a nested module-bays section at all (a separate, pre-existing gap, out
+        # of scope here), so there is no modulebaytemplates relation to prefetch alongside
+        # it. to_yaml()'s other component-template relations (interfaces, ports, etc.) are
+        # unprefetched here as they were before this relation existed, and remain their own
+        # N+1 across a large export.
+        self.queryset = self.queryset.prefetch_related('module_bay_types')
         return super().export_yaml()
 
 

netbox/utilities/forms/fields/csv.py

@@ -100,7 +100,13 @@ class CSVModelMultipleChoiceField(forms.ModelMultipleChoiceField):
 
     def clean(self, value):
         if not isinstance(value, list):
-            value = value.split(',') if value else []
+            # str(value): a caller may bind this field from parsed YAML/JSON rather than a
+            # CSV cell, where a scalar column can arrive as a non-string (e.g. an int or
+            # bool) -- .split() would raise AttributeError otherwise. .strip() each piece:
+            # a table's default ManyToManyColumn export separator is ", " (comma space), so
+            # re-importing NetBox's own CSV export of a multi-value column would otherwise
+            # fail to match on the leading space.
+            value = [v.strip() for v in str(value).split(',')] if value else []
         return super().clean(value)