- Document that ModuleType.to_yaml() exports module_bay_types by name but
the field isn't currently importable back through it (no ModuleTypeImportForm
field survived the CSV-import revert).
- modulebaytemplate.md's note covered only the device-type-parented import
path; ModuleBayTemplateImportForm is registered for both DeviceTypeImportView
and ModuleTypeImportView, scoping to whichever parent type's manufacturer
applies. Reworded to cover both, and added the "rejected rather than
resolved" clause for a name matching only some other manufacturer's type.
- Clarified clean_module_bay_types()'s docstring: the "never a cross-manufacturer
collision" guarantee holds only because ModularComponentTemplateModel.clean()
rejects a template with neither device_type nor module_type before this
method's result would ever be saved.
- Fixed a test docstring overstating symmetry between its two comparison arms.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per review, ditch dedupe_module_bay_types_by_manufacturer() and any import
logic that resolves module_bay_types by name alone across manufacturers.
ModuleBayType's unique constraint is (manufacturer, name), not name alone,
so resolving a bare name against an unscoped, potentially cross-manufacturer
pool -- guessing via a preference order, rejecting only on a genuine tie --
is not a reliable way to identify a specific object. CSV import forms have
no way to qualify an M2M reference beyond a bare name, so module_bay_types
is no longer exposed there at all (ModuleTypeImportForm, ModuleBayImportForm
in bulk_import.py): it's acceptable not to support this rather than resolve
it unreliably. This also reverts the netbox/tables/columns.py export_transform
API addition and the three tables' use of it, which existed only to make the
CSV round trip work.
The one import path that survives is ModuleBayTemplateImportForm (the YAML
device/module type "Import Components" flow), because it can reliably scope
module_bay_types' queryset to the parent device/module type's own
manufacturer plus global (manufacturer-less) types *before* resolving by
name -- so a name collision is never cross-manufacturer, only "this
manufacturer's own type vs. a global one of the same name," which
ModuleBayType's own uniqueness constraint makes unambiguous. A name matching
only some other manufacturer's type doesn't resolve at all, rather than
being coerced to an arbitrary guess.
Kept: the ModuleBayTemplateImportForm.enabled field/clean_enabled() fix
(default=True was previously lost on YAML re-import; unrelated to the
above), and to_yaml()'s export of module_bay_types on both ModuleType and
ModuleBayTemplate, plus the export_yaml() prefetch optimizations -- none of
these involve resolving an object's identity from an ambiguous attribute.
Trimmed the model docs to match: the modulebay.md and moduletype.md
paragraphs described capabilities (CSV import, cross-manufacturer YAML
import) that no longer exist and are removed; modulebaytemplate.md's note
is rewritten to describe the actual (manufacturer-or-global-scoped)
resolution behavior.
- 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.
The manufacturer-or-null queryset scoping added to disambiguate a name shared
by a global and a manufacturer-scoped ModuleBayType went further than
intended: it also excluded a *different* manufacturer's bay type entirely.
The UI (ModuleTypeForm/ModuleBayTemplateForm) and REST API place no such
restriction -- a third-party module may legitimately declare compatibility
with another manufacturer's proprietary bay type. Confirmed the regression
directly: creating that assignment via ModuleTypeForm succeeds, but
exporting it and re-importing the same YAML failed with
"Object not found: SFP28", making valid existing data unimportable -- worse
than the bug this feature exists to fix.
Remove the queryset scoping entirely and instead make
dedupe_module_bay_types_by_manufacturer() manufacturer-aware: given the
target manufacturer, it now prefers (in order) an exact match, then a global
type, then any remaining candidate, resolved from an unscoped queryset in
clean() rather than a sibling clean_<field>() mutating the field's queryset.
This also drops the Meta.fields-ordering dependency those methods required.
Also, from the same review round:
- Test asserting Django's literal English error string now asserts the
error code instead, so it survives wording changes/translation.
- The prefetch query-count test moved from test_models.py (which doesn't
otherwise touch views) to test_views.py, and strengthened from "prefetch
saves at least one query" to "query count is constant regardless of bay
count" -- the actual invariant. Added equivalent coverage for
ModuleTypeListView, which the prior version didn't test at all.
- Corrected the export_yaml() prefetch comments to not imply the other
~11 relations to_yaml() touches are also covered -- they aren't, and
weren't before this feature either.
- Updated the model docs to describe the new (permissive, cross-manufacturer
allowed) behavior instead of the old (restrictive) one they described a
commit ago.
Adds regression tests importing a bay type belonging to a different
manufacturer than the importing device/module type, through both
ModuleBayTemplateImportForm and ModuleTypeImportForm.
- clean_module_bay_types()'s two ValidationErrors were unreachable:
ModelMultipleChoiceField.clean() already raises before the clean_<name>
hook runs on a non-list or an unresolvable name, per Django's
BaseForm._clean_fields(). Simplify to dedupe from cleaned_data (already
scoped and validated) via a shared dedupe_module_bay_types_by_manufacturer()
helper in dcim/utils.py, used by both ModuleBayTemplateImportForm and the
new ModuleTypeImportForm.module_bay_types below. This also drops the
self.data access that ignored the form prefix, broke on a QueryDict, and
re-queried once per name.
- ModuleType.module_bay_types (the module's own side of the bay/module
compatibility intersection) was still missing from the YAML round trip.
Add it to ModuleType.to_yaml() and ModuleTypeImportForm, mirroring
ModuleBayTemplateImportForm's manufacturer-scoping and dedup.
- ModuleBayTemplate.to_yaml() emitted enabled but the import form didn't
accept it, so it silently reset to False (not the model's default=True)
on any dict-bound re-import. Add it with the same
clean_enabled()-defaults-to-True pattern already used by
ModuleBayImportForm's CSV import.
- Prefetch module_bay_types in DeviceTypeListView/ModuleTypeListView's
export_yaml() so bulk YAML export doesn't add one query per module bay
template across the exported queryset.
- Document the manufacturer-preference precedence rule in the model docs,
since export emits a bare name and import can resolve a colliding one to
either a global or manufacturer-specific type.
Adds regression tests for the module_type-scoped path, the enabled
round trip, an export/import round trip, export ordering, the new
ModuleTypeImportForm coverage, and the prefetch fix.
Jeremy Stretch
Brian Tiemann
bctiemann
jeremystretch