Fix single {module} token rejection at nested module bay depth

[mrmrcoleman]

Fixes: #20474

Summary

Fixes a bug where installing a module type with a single {module} placeholder into a nested module bay (depth > 1) was rejected with: "Cannot install module with placeholder values in a module bay tree 2 in tree but 1 placeholders given."

• A single {module} token now resolves to the immediate parent bay's position regardless of nesting depth
• Multi-token {module} behavior is unchanged (token count must still match tree depth)
• Refactors resolve_name() and resolve_label() into a shared _resolve_module_placeholder() helper to eliminate duplication

Changes

File	Change
dcim/forms/common.py	Single {module} token resolves to parent bay position instead of erroring; multi-token validation unchanged
dcim/models/device_component_templates.py	Consolidated resolve_name()/resolve_label() into shared _resolve_module_placeholder() with same single-token logic
dcim/tests/test_models.py	Regression test: single {module} SFP module installs at depth 2 and resolves to parent position

Testing

All existing dcim model and form tests pass (59 tests). Added one regression test that reproduces the exact scenario from #20474:

1. Create a chassis with a rear card slot (depth 1)
2. Install a rear card module with SFP slots (depth 2)
3. Install an SFP module type with SFP {module} interface template into depth 2
4. Verify the interface resolves to SFP 2 (parent bay position) instead of raising a ValidationError

[arthanson]

@claude /review

[claude]

Claude finished @arthanson's task in 2m 36s —— View job

---

PR Review: Fix single {module} token rejection at nested module bay depth

• Gather context and understand the request
• Read changed files and analyze the diff
• Provide code review feedback

---

Overview

This PR fixes a valid bug (#20474): installing a module type with a single {module} placeholder into a nested module bay (depth > 1) was incorrectly rejected. The root cause was that form validation required the token count to exactly equal the tree depth — with no exception for the common case of a single generic placeholder. The fix makes single-token a special case that always resolves to the immediate parent bay position.

---

Code Correctness

Form fix (forms/common.py) ✅

The new logic is correct:

token_count = template.name.count(MODULE_TOKEN)
if token_count == 1:
    resolved_name = resolved_name.replace(MODULE_TOKEN, module_bays[-1].position)
elif token_count == len(module_bays):
    for mb in module_bays:
        resolved_name = resolved_name.replace(MODULE_TOKEN, mb.position, 1)
else:
    raise forms.ValidationError(...)

module_bays[-1] is the immediate parent bay (the list is built root→leaf), so single-token always resolves to the direct parent. This is the right semantic.

Bonus fix: The old loop used for module_bay in module_bays: which silently shadowed the outer module_bay = self.cleaned_data.get('module_bay'). The new code uses mb and avoids this. Good catch.

Model refactor (device_component_templates.py) ✅ with one concern

The _resolve_module_placeholder() helper cleanly eliminates duplication. However, there is a subtle behavioral change to resolve_name()/resolve_label() for the single-token-at-depth->1 case:

	Old behavior	New behavior
token_count == 1 at depth > 1	Resolved to root bay position (first replacement in list iteration)	Resolves to immediate parent bay position

Since the old form would have rejected this case entirely, this path was only reachable by bypassing the form. The new behavior (immediate parent) is more intuitive and consistent with the form fix. Worth documenting in the commit message or inline comment, since future readers may not realize modules[-1] = immediate parent.

---

Potential Issues

1. Form validation not covered by the regression test ⚠️

The regression test in test_models.py calls Module.objects.create() directly, which bypasses form validation. The actual bug (form raising ValidationError) is not directly exercised by this test. It only verifies that _resolve_module_placeholder() in the model works correctly.

If someone breaks the form fix in forms/common.py, the test would still pass. Consider adding a test that validates the form path — e.g., using ModuleCommonForm or checking that ModuleCommonForm.clean() no longer raises for single-token at depth 2.

2. Inconsistency: model doesn't validate token count for multi-token case (pre-existing, not introduced here)

_resolve_module_placeholder() handles token_count == 1 specially, but for token_count > 1 it just loops through modules without checking that token_count == len(modules). If token_count > len(modules), some {module} tokens will remain unreplaced in the output. The form validates this, so it only matters for code paths bypassing the form. This is pre-existing behavior, just worth noting as a follow-up.

3. Error message doesn't hint at single-token exception (minor UX)

When a user provides 2 tokens at depth 3, they'd see: "Cannot install module with placeholder values in a module bay tree 3 in tree but 2 placeholders given." They might wonder why 1 token would work fine. Mentioning in the error message (or the model field help_text) that a single {module} is always valid regardless of depth would reduce confusion.

---

Test Quality

The test in test_single_module_token_at_nested_depth is well-structured and clearly reproduces the exact scenario from #20474. The @tag('regression') annotation is consistent with the project's convention for bug regression tests. The assertions verify the correct resolved name (SFP 2).

Gaps to consider:

• No test for the form-level validation path (the actual reported failure)
• No test for label resolution with single token at depth > 1 (only name is tested)

---

Summary

The fix is correct and the refactoring is an improvement. The key concern is that the regression test doesn't exercise the actual user-facing failure mode (the form validation path), so the form fix in common.py has no new test coverage. Everything else looks solid.
| Branch: fix/20474-module-token-depth-mismatch

[arthanson]

@mrmrcoleman can you fix the merge conflicts with main, otherwise good to go.

[arthanson]

Need to fix merge conflicts, also this doesn't fix it in the API serializer, just the UI. ModuleSerializer I think needs updating as well, probably should have a common logic routine to do this as it is spread in two parts right now.

[arthanson]

Thanks @mrmrcoleman