Closes #20152: Add support for disabling Device and Module bays

[pheus]

Fixes: #20152

This PR adds an enabled boolean field to ModuleBay, ModuleBayTemplate, DeviceBay, and DeviceBayTemplate.

The intent is to model bay availability as a simple on/off state, which feels like the best fit for this feature and keeps the implementation lightweight. Disabled bays are now reflected in the UI and cannot be used for new device or module installation. Forms, filters, serializers, and tables have also been updated so the new field is supported consistently across NetBox.

[pheus]

@claude /review

[claude]

Claude finished @pheus's task in 3m 35s —— View job

---

Code Review: Add support for disabling Device and Module bays

• Gathering PR context
• Reading changed files
• Analyzing code quality and correctness
• Checking test coverage
• Posting review feedback

---

Overview

This PR adds an enabled boolean field to ModuleBay, ModuleBayTemplate, DeviceBay, and DeviceBayTemplate. The implementation is consistently applied across models, serializers, filtersets, forms, tables, and GraphQL filters. Overall the approach is clean and follows NetBox's existing conventions well. A few things worth addressing:

---

Issues

1. Missing model-level validation tests

The PR adds validation logic in DeviceBay.clean() (device_components.py:1339–1351) and Module.clean() (modules.py:262–267) to prevent installation into disabled bays, but no tests cover these code paths. The existing test_models.py has a ModuleBayTestCase class that tests recursion/validation — the new validation rules should be exercised there.

Suggested tests:

• DeviceBay: assert ValidationError is raised when installed_device is set and enabled=False (new record)
• DeviceBay: assert ValidationError is not raised when disabling a bay that already has a device installed (same installed_device_id)
• Module: assert ValidationError is raised when installing into a disabled ModuleBay

Fix this →

2. Missing documentation updates

docs/models/dcim/devicebay.md and docs/models/dcim/modulebay.md both have a ## Fields section but neither mentions the new enabled field. This is needed for user-facing documentation.

Fix this →

---

Minor Observations

3. _occupied property logic for ModuleBay

# device_components.py:1307–1308
@property
def _occupied(self):
    return bool(not self.enabled or hasattr(self, 'installed_module'))

Using hasattr(self, 'installed_module') is correct here (the reverse OneToOne raises RelatedObjectDoesNotExist when nothing is installed, so hasattr returns False), but it triggers a DB query unless the relation is already cached. Compare to DeviceBay._occupied which uses self.installed_device (a nullable FK that avoids an extra query). Since Module has module_bay = OneToOneField(...), consider using hasattr(self, 'installed_module_id') or annotating querysets with select_related where this is used in list views — but this is a pre-existing pattern issue, not unique to this PR.

4. Redundant clean_enabled() methods in import forms

# forms/bulk_import.py
def clean_enabled(self):
    # Make sure enabled is True when it's not included in the uploaded data
    if 'enabled' not in self.data:
        return True
    return self.cleaned_data['enabled']

This is duplicated identically for both ModuleBayImportForm and DeviceBayImportForm. Since the model field defaults to True, the form field (derived from the model) should also default to True when empty. However, CSV form processing can result in an empty string being submitted for missing columns, so the override may be justified. The comment explains the intent, though a shared mixin or helper would avoid duplication. Also self.data is a QueryDict, so 'enabled' not in self.data checks the raw POST data — this is correct for CSV handling.

5. Migration missing verbose_name

The migration (0229_devicebay_modulebay_enabled.py) uses models.BooleanField(default=True) but the actual model fields include verbose_name=_('enabled'). Django doesn't require the migration to match exactly, but consistency helps when reviewing migrations. This is cosmetic.

6. Commit message typo

The commit message reads "Add enabled field to Module and Ddevice bays" — "Ddevice" has a double capital D. Minor, but worth correcting before merge.

7. replication_fields includes enabled

# forms/bulk_create.py
replication_fields = ('name', 'label', 'position', 'enabled')

Including enabled in replication_fields for ModuleBayBulkCreateForm means when bulk-creating bays from a name pattern like Bay[1-5], the enabled state is replicated across all created bays. This is correct behavior — all bays in a bulk-create operation should share the same enabled state.

---

Summary

Category	Status
Model fields	✅ Correct, consistent across all 4 models
Migration	✅ Valid (minor cosmetic nit on verbose_name)
Serializers	✅ Follows existing patterns
Filtersets	✅ Correct
Forms (model/bulk/import)	✅ Correct
Tables & template code	✅ UI correctly gates install button on enabled
GraphQL filters	✅ Correct
Validation logic	✅ Correct semantics; ⚠️ missing tests
Documentation	⚠️ Missing enabled field description in docs
Test coverage	⚠️ Filterset/API tests updated; model validation tests absent

[pheus]

@claude /review

1. has been addressed
2. has been addressed
3. can't be implemented, ModuleBay is on the reverse side of the relation (Module points to ModuleBay) → there's no installed_module_id column on ModuleBay
4. is correct
5. no issue
6. has been addressed
7. no issue

[claude]

Claude finished @pheus's task in 1m 1s —— View job

---

Updated Review: Add support for disabling Device and Module bays

Thanks for the clarifications and updates. Here's the revised status for each point:

---

1. Missing model-level validation tests → ✅ Addressed

Three thorough tests added to DeviceBayTestCase and one to ModuleBayTestCase in test_models.py:

• test_cannot_install_device_in_disabled_bay — asserts ValidationError on new install into disabled bay ✓
• test_can_disable_bay_with_existing_device — asserts no error when disabling a bay that keeps its existing device ✓
• test_cannot_change_installed_device_in_disabled_bay — asserts ValidationError when swapping installed device in a disabled bay ✓
• test_cannot_install_module_in_disabled_bay — asserts ValidationError when installing into a disabled ModuleBay ✓

2. Missing documentation → ✅ Addressed

Both docs/models/dcim/devicebay.md and docs/models/dcim/modulebay.md now include a ### Enabled section with a clear description.

3. _occupied using hasattr(self, 'installed_module') → ✅ Acknowledged — not applicable

The user is correct: Module holds the FK to ModuleBay (OneToOneField), so there's no installed_module_id column on ModuleBay itself. Using hasattr on the reverse accessor is the only option here. The pre-existing pattern stands.

4. Redundant clean_enabled() in import forms → ✅ Correct as-is

The override is justified because CSV form processing submits empty strings for missing columns, which the model's default doesn't handle automatically.

5–7. Cosmetic/minor observations → ✅ Non-issues or already addressed

---

Summary

All substantive issues from the original review have been addressed. The implementation is clean, consistent with NetBox conventions, and now has proper test coverage for the new validation logic. The PR looks good to merge.