From 099f2b188c9c8bb7823e8de54a9cd6489e5381bd Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 30 Mar 2026 17:38:24 +0200 Subject: [PATCH 01/22] docs: add design spec for remove-module-path-plus-fixes --- ...30-remove-module-path-plus-fixes-design.md | 178 ++++++++++++++++++ 1 file changed, 178 insertions(+) create mode 100644 docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md diff --git a/docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md b/docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md new file mode 100644 index 0000000..9d1daee --- /dev/null +++ b/docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md @@ -0,0 +1,178 @@ +# Design: Remove module_path, fix CSV import, add YAML export + +**Date:** 2026-03-30 +**Branch:** `feat/remove-module-path-plus-fixes` + +--- + +## Problem statement + +Three independent improvements bundled into one branch: + +1. **Remove `{module_path}` support** — NetBox decided not to implement the `MODULE_PATH_TOKEN` constant, so the feature-detection helper and all related code is dead weight. +2. **Fix KeyError `'Ch'` on CSV import** — Exporting from the rule list and immediately reimporting fails with `KeyError: 'Ch'`. Root cause: the table uses `verbose_name="Ch. Start"` for `channel_start`; django-tables2 uses verbose_names as CSV headers; NetBox's import form normalises headers by splitting on `.`, producing `"Ch"`, which is not a known form field. +3. **Add YAML export** — YAML import already works (via `BulkImportView`). The missing piece is export: a "Download all as YAML" link and a "Export selected as YAML" bulk action. + +--- + +## 1. Remove `{module_path}` support + +### Files to change + +| File | Change | +|------|--------| +| `netbox_interface_name_rules/utils.py` | Delete the entire file (only contains `supports_module_path()`) | +| `netbox_interface_name_rules/views.py` | Remove `from .utils import supports_module_path` and the `get_extra_context` method on `InterfaceNameRuleListView` | +| `netbox_interface_name_rules/templates/…/interfacenamerule_list.html` | Remove the `{% if supports_module_path %}…{% endif %}` info banner block | +| `netbox_interface_name_rules/engine.py` | Remove the inline comment mentioning `{module_path}` in `has_applicable_interfaces` docstring | +| `netbox_interface_name_rules/tests/test_misc.py` | Remove `SupportModulePathTest` class and `UtilsModulePathFalseTest` class | +| `REUSE.toml` | `utils.py` will be deleted; no entry change needed | + +No database migration is required. No API change. + +--- + +## 2. Fix CSV export/import round-trip (KeyError `'Ch'`) + +### Root cause + +`InterfaceNameRuleTable.channel_start` has `verbose_name="Ch. Start"`. When the list view is exported, django-tables2 writes `"Ch. Start"` as the CSV column header. NetBox's `NetBoxModelImportForm.__init__` later normalises headers by calling something like `header.split('.')[0].strip()`, yielding `"Ch"` — a non-existent form field — and raises `KeyError`. + +### Fix + +Add `csv_headers` (class attribute) and `to_csv()` (instance method) to the `InterfaceNameRule` model following the standard NetBox pattern. When these are present, `ObjectListView.export_data()` uses them instead of the table's verbose_names, so export and import use identical field names. + +Also rename `verbose_name="Ch. Start"` → `verbose_name="Channel Start"` in `tables.py` to avoid the ambiguity entirely. + +```python +# models.py +csv_headers = [ + 'module_type', 'module_type_pattern', 'module_type_is_regex', + 'parent_module_type', 'device_type', 'platform', + 'name_template', 'channel_count', 'channel_start', + 'description', 'enabled', 'applies_to_device_interfaces', +] + +def to_csv(self): + return ( + self.module_type.model if self.module_type else '', + self.module_type_pattern, + self.module_type_is_regex, + self.parent_module_type.model if self.parent_module_type else '', + self.device_type.model if self.device_type else '', + self.platform.name if self.platform else '', + self.name_template, + self.channel_count, + self.channel_start, + self.description, + self.enabled, + self.applies_to_device_interfaces, + ) +``` + +The `csv_headers` field names match exactly the `InterfaceNameRuleImportForm.Meta.fields` list, so a round-trip is guaranteed. + +--- + +## 3. Add YAML export + +### YAML format + +Matches the `contrib/` vendor files format — a YAML sequence of rule dicts using the same field names as the import form: + +```yaml +- module_type: QSFP-100G-LR4 + module_type_pattern: '' + module_type_is_regex: false + parent_module_type: '' + device_type: ACX7024 + platform: '' + name_template: 'et-0/0/{bay_position}' + channel_count: 0 + channel_start: 0 + description: '' + enabled: true + applies_to_device_interfaces: false +``` + +Empty FK fields are written as `''` (empty string) which matches what the import form treats as "not set". + +### Export modes + +| Mode | URL | Method | Behaviour | +|------|-----|--------|-----------| +| Export all | `/rules/export/yaml/` | GET | Downloads a single YAML file with every rule in pk order | +| Export selected | `/rules/export/yaml/` | POST | Body contains `pk_` checkbox fields; returns YAML of those rules | + +Using the same URL with GET vs POST allows both modes with one view. + +### New view + +```python +class InterfaceNameRuleYAMLExportView(ConditionalLoginRequiredMixin, View): + """GET = all rules; POST = selected rules (pk_ checkboxes).""" +``` + +Returns a `HttpResponse` with `Content-Type: application/x-yaml` and `Content-Disposition: attachment; filename="interface_name_rules.yaml"`. + +Uses `yaml.dump()` (PyYAML, already in NetBox's dependency tree). + +### UI changes + +- `interfacenamerule_list.html`: Add **"Export YAML"** link in the existing export/action area (same row as the existing "Export" CSV button). When rows are selected (checkbox state), clicking it POSTs the checked PKs; otherwise it GETs all. +- URL entry: `path("export/yaml/", ..., name="interfacenamerule_export_yaml")`. + +--- + +## 4. Tests (TDD) + +All tests must be written **before** the implementation code they exercise. Test files follow the existing convention (Django `TestCase`). + +### Tests to add to `test_views.py` + +| Test class | Test method | What it verifies | +|---|---|---| +| `BulkImportViewTest` | `test_import_csv_roundtrip` | POST a CSV generated by `model.to_csv()` / `csv_headers` to the import URL, assert no errors and new rule created | +| `BulkImportViewTest` | `test_import_csv_channel_start_field` | Specifically exercises `channel_start` column (regression for KeyError 'Ch') | +| `BulkImportViewTest` | `test_import_yaml_roundtrip` | POST a YAML payload to the import URL, assert no errors | +| `YAMLExportViewTest` | `test_export_all_yaml_get` | GET `/export/yaml/` returns 200 with YAML content-type and all rules | +| `YAMLExportViewTest` | `test_export_selected_yaml_post` | POST with selected PKs returns YAML with only those rules | +| `YAMLExportViewTest` | `test_export_yaml_roundtrip` | Export YAML, parse it, POST it back to import, assert rules created | +| `YAMLExportViewTest` | `test_export_yaml_empty` | GET when no rules exist returns an empty YAML list `[]\n` | + +### Tests to add to `test_misc.py` + +| Test class | Test method | What it verifies | +|---|---|---| +| `ModelCSVExportTest` | `test_csv_headers_match_import_form` | `InterfaceNameRule.csv_headers` == `InterfaceNameRuleImportForm.Meta.fields` | +| `ModelCSVExportTest` | `test_to_csv_output_count` | `to_csv()` returns a tuple with same length as `csv_headers` | +| `ModelCSVExportTest` | `test_to_csv_fk_field_natural_key` | `to_csv()` writes `module_type.model`, not a PK integer | + +### Remove from `test_misc.py` + +- `SupportModulePathTest` +- `UtilsModulePathFalseTest` + +--- + +## 5. No migration needed + +All changes are: +- Code / logic removal (module_path) +- Adding Python methods to the model (csv_headers / to_csv) — not ORM-level changes +- New view + URL +- Template changes +- Verbose_name fix in table + +--- + +## Implementation order + +1. Pull main, create branch +2. Remove module_path (no test impact except deletions) +3. **Write failing tests** for CSV round-trip and KeyError regression +4. Implement `csv_headers` + `to_csv()` on model, fix verbose_name +5. **Write failing tests** for YAML export +6. Implement YAML export view, URL, UI +7. Run full test suite — ensure green +8. Run `ruff check . && ruff format --check .` From 60d103ddee0a69cd2fa00ecbb980fd9c035ad80b Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 30 Mar 2026 17:57:27 +0200 Subject: [PATCH 02/22] feat: remove {module_path} feature detection support NetBox decided not to implement MODULE_PATH_TOKEN, so all related code is dead weight. Remove: - utils.py (entire file, only contained supports_module_path()) - InterfaceNameRuleListView.get_extra_context() in views.py - {%if supports_module_path%} banner in interfacenamerule_list.html - docs/configuration.md section and references - engine.py docstring mention - test_misc.py: SupportModulePathTest and UtilsModulePathFalseTest --- docs/configuration.md | 18 +------ netbox_interface_name_rules/engine.py | 3 +- .../interfacenamerule_list.html | 17 ------- .../tests/test_misc.py | 48 +------------------ netbox_interface_name_rules/utils.py | 17 ------- netbox_interface_name_rules/views.py | 8 ---- 6 files changed, 3 insertions(+), 108 deletions(-) delete mode 100644 netbox_interface_name_rules/utils.py diff --git a/docs/configuration.md b/docs/configuration.md index 015d9de..205828d 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -49,21 +49,6 @@ NetBox installs: interface name = "5" (raw bay position) Plugin renames: interface name = "et-0/0/5" ``` -### `{module_path}` (NetBox ≥ 4.9) - -When a module type's interface template uses `{module_path}`, NetBox resolves -the full module bay path at install time. For a transceiver in "X2 Port 1" -inside a line card in "Slot 1", `{module_path}` resolves to `1/1`. The plugin -signal still fires, but may compute the same name the interface already has — -so it becomes a no-op. - -For directly-attached pluggables (single bay depth), `{module_path}` resolves to -just the bay position, behaving identically to `{module}`. - -!!! tip - New module types should use `{module_path}` for best NetBox compatibility. - Legacy module types using `{module}` continue to work — the plugin handles the rename. - ### The `potentially-deprecated` Tag After installing a module, if the plugin's signal fires but finds the interface @@ -71,8 +56,7 @@ is already correctly named, it automatically tags the rule `potentially-deprecat This means: - For **new installs**: the rule may no longer be needed (NetBox generates the name) -- For **retroactive applies**: the rule is still useful for modules installed before - `{module_path}` support was added, or before the rule existed +- For **retroactive applies**: the rule is still useful for modules installed before the rule existed The tag is informational only — the rule remains active. diff --git a/netbox_interface_name_rules/engine.py b/netbox_interface_name_rules/engine.py index c1c2ecb..64ae903 100644 --- a/netbox_interface_name_rules/engine.py +++ b/netbox_interface_name_rules/engine.py @@ -561,8 +561,7 @@ def has_applicable_interfaces(rule) -> bool: Calls find_interfaces_for_rule(limit=1) to determine if any currently installed interface would receive a new name. Returns False when: - no matching modules/interfaces are installed, OR - - all matching interfaces are already correctly named (e.g. NetBox resolved - {module_path} at install time, making the rule a no-op for existing interfaces). + - all matching interfaces are already correctly named. This is more expensive than a plain EXISTS query but ensures the Applicable column in the Apply Rules list accurately reflects "would something change?" diff --git a/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html b/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html index 9ca0f97..217b0d1 100644 --- a/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html +++ b/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html @@ -78,23 +78,6 @@
Examples
(device-level rule, pattern Ethernet\d+/\d+) - {% if supports_module_path %} -
- - {module_path} supported: - Platform naming rules (e.g., et-0/0/{bay_position}) - may be replaceable by using {module_path} in module type interface templates directly. - Converter offset and breakout rules are still needed. -
- {% else %} -
- - {module_path} not available: - Platform naming rules - (e.g., Juniper et-0/0/{bay_position}) are required for correct interface naming. - Upgrade NetBox to get native {module_path} support. -
- {% endif %} {{ block.super }} {% endblock %} + +{% block extra_controls %} + + Export YAML + +{% endblock %} diff --git a/netbox_interface_name_rules/urls.py b/netbox_interface_name_rules/urls.py index 5ba86fa..1223798 100644 --- a/netbox_interface_name_rules/urls.py +++ b/netbox_interface_name_rules/urls.py @@ -12,6 +12,7 @@ path("rules/add/", views.InterfaceNameRuleCreateView.as_view(), name="interfacenamerule_add"), path("rules/import/", views.InterfaceNameRuleBulkImportView.as_view(), name="interfacenamerule_bulk_import"), path("rules/bulk_delete/", views.InterfaceNameRuleBulkDeleteView.as_view(), name="interfacenamerule_bulk_delete"), + path("rules/export/yaml/", views.InterfaceNameRuleYAMLExportView.as_view(), name="interfacenamerule_export_yaml"), # Rule tester (Build Rule) path("rules/test/", views.RuleTestView.as_view(), name="interfacenamerule_test"), # Apply rules to existing interfaces diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index 23ca0ad..e5ceae2 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -4,10 +4,11 @@ import logging import re +import yaml from django.conf import settings from django.contrib import messages from django.core.exceptions import PermissionDenied -from django.http import JsonResponse +from django.http import HttpResponse, JsonResponse from django.shortcuts import get_object_or_404, redirect, render from django.urls import reverse from django.utils.http import url_has_allowed_host_and_scheme @@ -114,6 +115,46 @@ def get(self, request, pk): return redirect(f"{url}?{params.urlencode()}") +class InterfaceNameRuleYAMLExportView(ConditionalLoginRequiredMixin, View): + """Export selected or all rules as a YAML file importable via BulkImportView.""" + + def _rules_to_yaml_data(self, queryset): + """Return a list of dicts (one per rule) using import-form field names.""" + records = [] + for rule in queryset.select_related("module_type", "parent_module_type", "device_type", "platform"): + entry = {} + for header, value in zip(InterfaceNameRule.csv_headers, rule.to_csv()): + # Omit empty/falsy optional fields to keep output readable, + # but always include required fields. + required = {"name_template"} + if value != "" and value is not None: + entry[header] = value + elif header in required: + entry[header] = value + records.append(entry) + return records + + def _build_response(self, queryset): + data = self._rules_to_yaml_data(queryset) + content = yaml.dump(data, default_flow_style=False, allow_unicode=True, sort_keys=False) + response = HttpResponse(content, content_type="application/x-yaml; charset=utf-8") + response["Content-Disposition"] = 'attachment; filename="interface_name_rules.yaml"' + return response + + def get(self, request): + """Export all rules as YAML.""" + return self._build_response(InterfaceNameRule.objects.all()) + + def post(self, request): + """Export selected rules (pk_ form fields) as YAML, or all if none selected.""" + pk_list = [int(v) for k, v in request.POST.items() if k.startswith("pk_") and v.isdigit()] + if pk_list: + queryset = InterfaceNameRule.objects.filter(pk__in=pk_list) + else: + queryset = InterfaceNameRule.objects.all() + return self._build_response(queryset) + + class RuleTestView(ConditionalLoginRequiredMixin, View): """Live-preview a name template with user-supplied variable values and optional DB lookup.""" From e9b823c400b5b0580c3dc28e3cda55fb4b0fdade Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 30 Mar 2026 19:26:45 +0200 Subject: [PATCH 06/22] test: fix yaml_export_requires_login assertion for ConditionalLoginRequiredMixin ConditionalLoginRequiredMixin only enforces auth when LOGIN_REQUIRED=True; in the test environment it may be False so we accept 200 as well as redirects --- netbox_interface_name_rules/tests/test_views.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index bcbb2a3..6f917f3 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -726,11 +726,15 @@ class YAMLExportViewTest(ViewTestBase): YAML_URL = "/plugins/interface-name-rules/rules/export/yaml/" def test_yaml_export_requires_login(self): - """Unauthenticated GET must redirect to login.""" + """Unauthenticated request behaviour depends on LOGIN_REQUIRED setting. + + ConditionalLoginRequiredMixin only enforces authentication when + LOGIN_REQUIRED=True (the default in production). In the test environment + LOGIN_REQUIRED may be False, so we just assert the view responds without + a server error rather than asserting a redirect. + """ response = self.client.get(self.YAML_URL) - self.assertIn(response.status_code, [302, 301], "Expected redirect for unauthenticated request") - if response.status_code in [301, 302]: - self.assertIn("login", response["Location"].lower()) + self.assertIn(response.status_code, [200, 301, 302]) def test_yaml_export_all_returns_200(self): """Authenticated GET /rules/export/yaml/ must return 200.""" From b734f670c93f707b0cb43d2ff30ceacc6ba11d63 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Mon, 30 Mar 2026 23:38:26 +0200 Subject: [PATCH 07/22] refactor: address review feedback on YAML export and test naming - views.py: simplify _rules_to_yaml_data loop (single condition instead of if/elif); change Content-Type to standard 'application/yaml' - tests/test_views.py: rename test_yaml_export_requires_login to test_yaml_export_unauthenticated_responds with accurate docstring - .gitignore: add docs/superpowers/ (design specs are local-only) - delete docs/superpowers/specs/ design spec from git history --- .gitignore | 3 + ...30-remove-module-path-plus-fixes-design.md | 178 ------------------ .../tests/test_views.py | 11 +- netbox_interface_name_rules/views.py | 9 +- 4 files changed, 10 insertions(+), 191 deletions(-) delete mode 100644 docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md diff --git a/.gitignore b/.gitignore index 875fb89..e8d44e6 100644 --- a/.gitignore +++ b/.gitignore @@ -60,3 +60,6 @@ site/ # direnv .envrc + +# Brainstorming / design specs (local only, not for version control) +docs/superpowers/ diff --git a/docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md b/docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md deleted file mode 100644 index 9d1daee..0000000 --- a/docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md +++ /dev/null @@ -1,178 +0,0 @@ -# Design: Remove module_path, fix CSV import, add YAML export - -**Date:** 2026-03-30 -**Branch:** `feat/remove-module-path-plus-fixes` - ---- - -## Problem statement - -Three independent improvements bundled into one branch: - -1. **Remove `{module_path}` support** — NetBox decided not to implement the `MODULE_PATH_TOKEN` constant, so the feature-detection helper and all related code is dead weight. -2. **Fix KeyError `'Ch'` on CSV import** — Exporting from the rule list and immediately reimporting fails with `KeyError: 'Ch'`. Root cause: the table uses `verbose_name="Ch. Start"` for `channel_start`; django-tables2 uses verbose_names as CSV headers; NetBox's import form normalises headers by splitting on `.`, producing `"Ch"`, which is not a known form field. -3. **Add YAML export** — YAML import already works (via `BulkImportView`). The missing piece is export: a "Download all as YAML" link and a "Export selected as YAML" bulk action. - ---- - -## 1. Remove `{module_path}` support - -### Files to change - -| File | Change | -|------|--------| -| `netbox_interface_name_rules/utils.py` | Delete the entire file (only contains `supports_module_path()`) | -| `netbox_interface_name_rules/views.py` | Remove `from .utils import supports_module_path` and the `get_extra_context` method on `InterfaceNameRuleListView` | -| `netbox_interface_name_rules/templates/…/interfacenamerule_list.html` | Remove the `{% if supports_module_path %}…{% endif %}` info banner block | -| `netbox_interface_name_rules/engine.py` | Remove the inline comment mentioning `{module_path}` in `has_applicable_interfaces` docstring | -| `netbox_interface_name_rules/tests/test_misc.py` | Remove `SupportModulePathTest` class and `UtilsModulePathFalseTest` class | -| `REUSE.toml` | `utils.py` will be deleted; no entry change needed | - -No database migration is required. No API change. - ---- - -## 2. Fix CSV export/import round-trip (KeyError `'Ch'`) - -### Root cause - -`InterfaceNameRuleTable.channel_start` has `verbose_name="Ch. Start"`. When the list view is exported, django-tables2 writes `"Ch. Start"` as the CSV column header. NetBox's `NetBoxModelImportForm.__init__` later normalises headers by calling something like `header.split('.')[0].strip()`, yielding `"Ch"` — a non-existent form field — and raises `KeyError`. - -### Fix - -Add `csv_headers` (class attribute) and `to_csv()` (instance method) to the `InterfaceNameRule` model following the standard NetBox pattern. When these are present, `ObjectListView.export_data()` uses them instead of the table's verbose_names, so export and import use identical field names. - -Also rename `verbose_name="Ch. Start"` → `verbose_name="Channel Start"` in `tables.py` to avoid the ambiguity entirely. - -```python -# models.py -csv_headers = [ - 'module_type', 'module_type_pattern', 'module_type_is_regex', - 'parent_module_type', 'device_type', 'platform', - 'name_template', 'channel_count', 'channel_start', - 'description', 'enabled', 'applies_to_device_interfaces', -] - -def to_csv(self): - return ( - self.module_type.model if self.module_type else '', - self.module_type_pattern, - self.module_type_is_regex, - self.parent_module_type.model if self.parent_module_type else '', - self.device_type.model if self.device_type else '', - self.platform.name if self.platform else '', - self.name_template, - self.channel_count, - self.channel_start, - self.description, - self.enabled, - self.applies_to_device_interfaces, - ) -``` - -The `csv_headers` field names match exactly the `InterfaceNameRuleImportForm.Meta.fields` list, so a round-trip is guaranteed. - ---- - -## 3. Add YAML export - -### YAML format - -Matches the `contrib/` vendor files format — a YAML sequence of rule dicts using the same field names as the import form: - -```yaml -- module_type: QSFP-100G-LR4 - module_type_pattern: '' - module_type_is_regex: false - parent_module_type: '' - device_type: ACX7024 - platform: '' - name_template: 'et-0/0/{bay_position}' - channel_count: 0 - channel_start: 0 - description: '' - enabled: true - applies_to_device_interfaces: false -``` - -Empty FK fields are written as `''` (empty string) which matches what the import form treats as "not set". - -### Export modes - -| Mode | URL | Method | Behaviour | -|------|-----|--------|-----------| -| Export all | `/rules/export/yaml/` | GET | Downloads a single YAML file with every rule in pk order | -| Export selected | `/rules/export/yaml/` | POST | Body contains `pk_` checkbox fields; returns YAML of those rules | - -Using the same URL with GET vs POST allows both modes with one view. - -### New view - -```python -class InterfaceNameRuleYAMLExportView(ConditionalLoginRequiredMixin, View): - """GET = all rules; POST = selected rules (pk_ checkboxes).""" -``` - -Returns a `HttpResponse` with `Content-Type: application/x-yaml` and `Content-Disposition: attachment; filename="interface_name_rules.yaml"`. - -Uses `yaml.dump()` (PyYAML, already in NetBox's dependency tree). - -### UI changes - -- `interfacenamerule_list.html`: Add **"Export YAML"** link in the existing export/action area (same row as the existing "Export" CSV button). When rows are selected (checkbox state), clicking it POSTs the checked PKs; otherwise it GETs all. -- URL entry: `path("export/yaml/", ..., name="interfacenamerule_export_yaml")`. - ---- - -## 4. Tests (TDD) - -All tests must be written **before** the implementation code they exercise. Test files follow the existing convention (Django `TestCase`). - -### Tests to add to `test_views.py` - -| Test class | Test method | What it verifies | -|---|---|---| -| `BulkImportViewTest` | `test_import_csv_roundtrip` | POST a CSV generated by `model.to_csv()` / `csv_headers` to the import URL, assert no errors and new rule created | -| `BulkImportViewTest` | `test_import_csv_channel_start_field` | Specifically exercises `channel_start` column (regression for KeyError 'Ch') | -| `BulkImportViewTest` | `test_import_yaml_roundtrip` | POST a YAML payload to the import URL, assert no errors | -| `YAMLExportViewTest` | `test_export_all_yaml_get` | GET `/export/yaml/` returns 200 with YAML content-type and all rules | -| `YAMLExportViewTest` | `test_export_selected_yaml_post` | POST with selected PKs returns YAML with only those rules | -| `YAMLExportViewTest` | `test_export_yaml_roundtrip` | Export YAML, parse it, POST it back to import, assert rules created | -| `YAMLExportViewTest` | `test_export_yaml_empty` | GET when no rules exist returns an empty YAML list `[]\n` | - -### Tests to add to `test_misc.py` - -| Test class | Test method | What it verifies | -|---|---|---| -| `ModelCSVExportTest` | `test_csv_headers_match_import_form` | `InterfaceNameRule.csv_headers` == `InterfaceNameRuleImportForm.Meta.fields` | -| `ModelCSVExportTest` | `test_to_csv_output_count` | `to_csv()` returns a tuple with same length as `csv_headers` | -| `ModelCSVExportTest` | `test_to_csv_fk_field_natural_key` | `to_csv()` writes `module_type.model`, not a PK integer | - -### Remove from `test_misc.py` - -- `SupportModulePathTest` -- `UtilsModulePathFalseTest` - ---- - -## 5. No migration needed - -All changes are: -- Code / logic removal (module_path) -- Adding Python methods to the model (csv_headers / to_csv) — not ORM-level changes -- New view + URL -- Template changes -- Verbose_name fix in table - ---- - -## Implementation order - -1. Pull main, create branch -2. Remove module_path (no test impact except deletions) -3. **Write failing tests** for CSV round-trip and KeyError regression -4. Implement `csv_headers` + `to_csv()` on model, fix verbose_name -5. **Write failing tests** for YAML export -6. Implement YAML export view, URL, UI -7. Run full test suite — ensure green -8. Run `ruff check . && ruff format --check .` diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index 6f917f3..86289cc 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -725,13 +725,12 @@ class YAMLExportViewTest(ViewTestBase): YAML_URL = "/plugins/interface-name-rules/rules/export/yaml/" - def test_yaml_export_requires_login(self): - """Unauthenticated request behaviour depends on LOGIN_REQUIRED setting. + def test_yaml_export_unauthenticated_responds(self): + """Unauthenticated GET must not return a server error (4xx or 2xx is acceptable). - ConditionalLoginRequiredMixin only enforces authentication when - LOGIN_REQUIRED=True (the default in production). In the test environment - LOGIN_REQUIRED may be False, so we just assert the view responds without - a server error rather than asserting a redirect. + ViewTestBase does not log in by default — this exercises anonymous access. + ConditionalLoginRequiredMixin enforces auth only when LOGIN_REQUIRED=True; + in the test environment that setting may be False, so 200 is also valid. """ response = self.client.get(self.YAML_URL) self.assertIn(response.status_code, [200, 301, 302]) diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index e5ceae2..ca7eb14 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -124,12 +124,7 @@ def _rules_to_yaml_data(self, queryset): for rule in queryset.select_related("module_type", "parent_module_type", "device_type", "platform"): entry = {} for header, value in zip(InterfaceNameRule.csv_headers, rule.to_csv()): - # Omit empty/falsy optional fields to keep output readable, - # but always include required fields. - required = {"name_template"} - if value != "" and value is not None: - entry[header] = value - elif header in required: + if value != "" and value is not None or header in {"name_template"}: entry[header] = value records.append(entry) return records @@ -137,7 +132,7 @@ def _rules_to_yaml_data(self, queryset): def _build_response(self, queryset): data = self._rules_to_yaml_data(queryset) content = yaml.dump(data, default_flow_style=False, allow_unicode=True, sort_keys=False) - response = HttpResponse(content, content_type="application/x-yaml; charset=utf-8") + response = HttpResponse(content, content_type="application/yaml; charset=utf-8") response["Content-Disposition"] = 'attachment; filename="interface_name_rules.yaml"' return response From cc68f421ce8ec5f4477ca1161dde46fb2e01ef4c Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 00:13:09 +0200 Subject: [PATCH 08/22] fix: wire YAML export POST to NetBox bulk-select form convention - post() now reads request.POST.getlist('pk') (NetBox convention) - template overrides bulk_buttons block with Export Selected YAML button using formaction to piggyback on the existing bulk-select form - update test to post {pk: [pk]} matching the new convention --- .../interfacenamerule_list.html | 10 ++++++++++ netbox_interface_name_rules/tests/test_views.py | 4 ++-- netbox_interface_name_rules/views.py | 9 +++++++-- 3 files changed, 19 insertions(+), 4 deletions(-) diff --git a/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html b/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html index f83f2f0..3b85cf7 100644 --- a/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html +++ b/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html @@ -153,3 +153,13 @@
Examples
Export YAML {% endblock %} + +{% block bulk_buttons %} + {{ block.super }} + +{% endblock %} diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index 86289cc..c862889 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -767,13 +767,13 @@ def test_yaml_export_all_contains_rules(self): self.assertGreaterEqual(len(data), 1) def test_yaml_export_selected_rules_via_post(self): - """POST with pk_ form fields must export only the selected rules.""" + """POST with NetBox bulk-select pk inputs must export only the selected rules.""" import yaml self.client.force_login(self.superuser) response = self.client.post( self.YAML_URL, - {f"pk_{self.rule.pk}": self.rule.pk}, + {"pk": [self.rule.pk]}, ) self.assertEqual(response.status_code, 200) data = yaml.safe_load(response.content) diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index ca7eb14..2912919 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -141,8 +141,13 @@ def get(self, request): return self._build_response(InterfaceNameRule.objects.all()) def post(self, request): - """Export selected rules (pk_ form fields) as YAML, or all if none selected.""" - pk_list = [int(v) for k, v in request.POST.items() if k.startswith("pk_") and v.isdigit()] + """Export selected rules as YAML using NetBox's bulk-select form (pk inputs). + + The parent object_list.html wraps the table in a POST form; selected rows + post their PKs as repeated ``pk`` inputs. Falls back to all rules when + nothing is selected. + """ + pk_list = [int(v) for v in request.POST.getlist("pk") if v.isdigit()] if pk_list: queryset = InterfaceNameRule.objects.filter(pk__in=pk_list) else: From d8185040c42d6ad99c5014b214ddd2cb0ab43d35 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 00:36:02 +0200 Subject: [PATCH 09/22] fix: use reverse() for YAML export URL, add permission check, fix unauthenticated test - Replace hardcoded YAML_URL with reverse() in setUpTestData - Add view_interfacenamerule permission check to get() and post() - Add self.client.logout() to test_yaml_export_unauthenticated_responds so it actually exercises unauthenticated access --- netbox_interface_name_rules/tests/test_views.py | 15 ++++++++++----- netbox_interface_name_rules/views.py | 4 ++++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index c862889..46ebe4a 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -723,15 +723,20 @@ def test_csv_round_trip_creates_rule(self): class YAMLExportViewTest(ViewTestBase): """Tests for the YAML export view at GET/POST /rules/export/yaml/.""" - YAML_URL = "/plugins/interface-name-rules/rules/export/yaml/" + @classmethod + def setUpTestData(cls): + super().setUpTestData() + cls.YAML_URL = reverse("plugins:netbox_interface_name_rules:interfacenamerule_export_yaml") def test_yaml_export_unauthenticated_responds(self): - """Unauthenticated GET must not return a server error (4xx or 2xx is acceptable). + """Unauthenticated GET must not return a server error. - ViewTestBase does not log in by default — this exercises anonymous access. - ConditionalLoginRequiredMixin enforces auth only when LOGIN_REQUIRED=True; - in the test environment that setting may be False, so 200 is also valid. + setUp() logs in the superuser, so we explicitly logout first to exercise + anonymous access. ConditionalLoginRequiredMixin only enforces auth when + LOGIN_REQUIRED=True; in the test environment that setting may be False, + so 200 is also valid alongside a redirect. """ + self.client.logout() response = self.client.get(self.YAML_URL) self.assertIn(response.status_code, [200, 301, 302]) diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index 2912919..faea9da 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -138,6 +138,8 @@ def _build_response(self, queryset): def get(self, request): """Export all rules as YAML.""" + if not request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): + raise PermissionDenied return self._build_response(InterfaceNameRule.objects.all()) def post(self, request): @@ -147,6 +149,8 @@ def post(self, request): post their PKs as repeated ``pk`` inputs. Falls back to all rules when nothing is selected. """ + if not request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): + raise PermissionDenied pk_list = [int(v) for v in request.POST.getlist("pk") if v.isdigit()] if pk_list: queryset = InterfaceNameRule.objects.filter(pk__in=pk_list) From fdd958aa2a9d301514d8bb6d28ebdab5a3f883aa Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 07:21:53 +0200 Subject: [PATCH 10/22] fix: strengthen CSV round-trip and YAML export test assertions - test_csv_round_trip_creates_rule: use a unique ModuleType so the import doesn't silently fail on the UniqueConstraint, assert 302 and count increase unconditionally instead of accepting 200 as success - test_yaml_export_unauthenticated_responds: accept [301, 302, 403] instead of [200, 301, 302] since the view raises PermissionDenied for anonymous users - test_yaml_export_all_contains_rules: assert exact count matches DB instead of >= 1 --- .../tests/test_views.py | 60 +++++++++++++------ 1 file changed, 43 insertions(+), 17 deletions(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index 46ebe4a..a0ec2e0 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -698,21 +698,47 @@ def test_csv_import_does_not_raise_key_error(self): self.assertNotEqual(response.status_code, 500, "CSV import returned a 500 error") def test_csv_round_trip_creates_rule(self): - """CSV exported from an exact rule can be imported to create an equivalent rule.""" + """CSV exported from a fresh rule can be imported to create a new rule.""" + import csv + import io + import uuid + self.client.force_login(self.superuser) - csv_data = self._csv_from_rule(self.rule) + # Build a unique ModuleType so the imported row doesn't collide with fixtures. + unique_model = f"ROUND-TRIP-{uuid.uuid4().hex[:8]}" + mt = ModuleType.objects.create( + manufacturer=self.module_type.manufacturer, + model=unique_model, + part_number=unique_model, + ) + buf = io.StringIO() + writer = csv.writer(buf) + writer.writerow(InterfaceNameRule.csv_headers) + writer.writerow( + [ + mt.model, # module_type + "", # module_type_pattern + False, # module_type_is_regex + "", # parent_module_type + "", # device_type + "", # platform + "et-0/0/{bay_position}", # name_template + 0, # channel_count + 0, # channel_start + "round-trip test", # description + True, # enabled + False, # applies_to_device_interfaces + ] + ) + csv_data = buf.getvalue() + url = reverse("plugins:netbox_interface_name_rules:interfacenamerule_bulk_import") before_count = InterfaceNameRule.objects.count() response = self.client.post(url, {"data": csv_data, "format": "csv"}) - # A successful import redirects; failure re-renders the form (200). - self.assertIn( - response.status_code, - [200, 302], - f"Unexpected status {response.status_code}", - ) - if response.status_code == 302: - after_count = InterfaceNameRule.objects.count() - self.assertGreater(after_count, before_count, "No new rule was created after CSV import") + # A successful import redirects (302); failure re-renders the form (200). + self.assertEqual(response.status_code, 302, f"Import did not redirect; status={response.status_code}") + after_count = InterfaceNameRule.objects.count() + self.assertGreater(after_count, before_count, "No new rule was created after CSV import") # --------------------------------------------------------------------------- @@ -729,16 +755,16 @@ def setUpTestData(cls): cls.YAML_URL = reverse("plugins:netbox_interface_name_rules:interfacenamerule_export_yaml") def test_yaml_export_unauthenticated_responds(self): - """Unauthenticated GET must not return a server error. + """Unauthenticated GET must not serve content to anonymous users. setUp() logs in the superuser, so we explicitly logout first to exercise - anonymous access. ConditionalLoginRequiredMixin only enforces auth when - LOGIN_REQUIRED=True; in the test environment that setting may be False, - so 200 is also valid alongside a redirect. + anonymous access. When LOGIN_REQUIRED=True the mixin redirects (302); + when False the view's explicit has_perm check raises PermissionDenied + (403). 200 is never valid because AnonymousUser lacks the permission. """ self.client.logout() response = self.client.get(self.YAML_URL) - self.assertIn(response.status_code, [200, 301, 302]) + self.assertIn(response.status_code, [301, 302, 403]) def test_yaml_export_all_returns_200(self): """Authenticated GET /rules/export/yaml/ must return 200.""" @@ -769,7 +795,7 @@ def test_yaml_export_all_contains_rules(self): self.assertEqual(response.status_code, 200) data = yaml.safe_load(response.content) self.assertIsInstance(data, list) - self.assertGreaterEqual(len(data), 1) + self.assertEqual(len(data), InterfaceNameRule.objects.count()) def test_yaml_export_selected_rules_via_post(self): """POST with NetBox bulk-select pk inputs must export only the selected rules.""" From b883698a5439b990604653d92e5d58070ec21a16 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 09:35:53 +0200 Subject: [PATCH 11/22] refactor: align utility views to NetBox base classes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace ConditionalLoginRequiredMixin + View with proper NetBox bases: - InterfaceNameRuleDuplicateView → generic.ObjectView uses self.get_object() instead of get_object_or_404 - InterfaceNameRuleYAMLExportView → BaseMultiObjectView removes manual has_perm() checks; permission now enforced at dispatch uses self.queryset (permission-restricted) for get() and post() - RuleTestView → BaseMultiObjectView removes _check_permission() method; permission enforced at dispatch - RuleApplyListView → BaseMultiObjectView uses self.queryset for the rule listing - RuleApplicableView → generic.ObjectView uses self.get_object() instead of get_object_or_404 - RuleApplyDetailView → generic.ObjectView removes _check_permission(); uses additional_permissions for dcim.change_interface enforcement at dispatch level Also fix test_views.BulkImportCSVTest: add csv_delimiter=auto to POST data (required by BulkImportForm since NetBox added csv_delimiter field) --- .../tests/test_views.py | 4 +- netbox_interface_name_rules/views.py | 89 ++++++++++--------- 2 files changed, 48 insertions(+), 45 deletions(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index a0ec2e0..72bbe6d 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -692,7 +692,7 @@ def test_csv_import_does_not_raise_key_error(self): url = reverse("plugins:netbox_interface_name_rules:interfacenamerule_bulk_import") # If a KeyError is raised the view returns 500; assert it doesn't. try: - response = self.client.post(url, {"data": csv_data, "format": "csv"}) + response = self.client.post(url, {"data": csv_data, "format": "csv", "csv_delimiter": "auto"}) except KeyError as exc: self.fail(f"CSV import raised KeyError: {exc!r}") self.assertNotEqual(response.status_code, 500, "CSV import returned a 500 error") @@ -734,7 +734,7 @@ def test_csv_round_trip_creates_rule(self): url = reverse("plugins:netbox_interface_name_rules:interfacenamerule_bulk_import") before_count = InterfaceNameRule.objects.count() - response = self.client.post(url, {"data": csv_data, "format": "csv"}) + response = self.client.post(url, {"data": csv_data, "format": "csv", "csv_delimiter": "auto"}) # A successful import redirects (302); failure re-renders the form (200). self.assertEqual(response.status_code, 302, f"Import did not redirect; status={response.status_code}") after_count = InterfaceNameRule.objects.count() diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index faea9da..d2819e6 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -9,12 +9,12 @@ from django.contrib import messages from django.core.exceptions import PermissionDenied from django.http import HttpResponse, JsonResponse -from django.shortcuts import get_object_or_404, redirect, render +from django.shortcuts import redirect, render from django.urls import reverse from django.utils.http import url_has_allowed_host_and_scheme -from django.views import View from netbox.views import generic -from utilities.views import ConditionalLoginRequiredMixin, register_model_view +from netbox.views.generic.base import BaseMultiObjectView +from utilities.views import register_model_view from .filters import InterfaceNameRuleFilterSet from .forms import InterfaceNameRuleFilterForm, InterfaceNameRuleForm, InterfaceNameRuleImportForm, RuleTestForm @@ -102,22 +102,30 @@ class InterfaceNameRuleChangeLogView(generic.ObjectChangeLogView): queryset = InterfaceNameRule.objects.all() -class InterfaceNameRuleDuplicateView(ConditionalLoginRequiredMixin, View): +class InterfaceNameRuleDuplicateView(generic.ObjectView): """Redirect to the add view pre-populated with a clone of the given rule.""" - def get(self, request, pk): - """Redirect to the add view pre-populated with fields cloned from rule pk.""" + queryset = InterfaceNameRule.objects.all() + + def get(self, request, **kwargs): + """Redirect to the add view pre-populated with fields cloned from the given rule.""" from utilities.querydict import prepare_cloned_fields - rule = get_object_or_404(InterfaceNameRule, pk=pk) + rule = self.get_object(**kwargs) params = prepare_cloned_fields(rule) url = reverse("plugins:netbox_interface_name_rules:interfacenamerule_add") return redirect(f"{url}?{params.urlencode()}") -class InterfaceNameRuleYAMLExportView(ConditionalLoginRequiredMixin, View): +class InterfaceNameRuleYAMLExportView(BaseMultiObjectView): """Export selected or all rules as a YAML file importable via BulkImportView.""" + queryset = InterfaceNameRule.objects.all() + + def get_required_permission(self): + """Return the permission required to export rules.""" + return "netbox_interface_name_rules.view_interfacenamerule" + def _rules_to_yaml_data(self, queryset): """Return a list of dicts (one per rule) using import-form field names.""" records = [] @@ -138,9 +146,7 @@ def _build_response(self, queryset): def get(self, request): """Export all rules as YAML.""" - if not request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): - raise PermissionDenied - return self._build_response(InterfaceNameRule.objects.all()) + return self._build_response(self.queryset) def post(self, request): """Export selected rules as YAML using NetBox's bulk-select form (pk inputs). @@ -149,28 +155,26 @@ def post(self, request): post their PKs as repeated ``pk`` inputs. Falls back to all rules when nothing is selected. """ - if not request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): - raise PermissionDenied pk_list = [int(v) for v in request.POST.getlist("pk") if v.isdigit()] if pk_list: - queryset = InterfaceNameRule.objects.filter(pk__in=pk_list) + queryset = self.queryset.filter(pk__in=pk_list) else: - queryset = InterfaceNameRule.objects.all() + queryset = self.queryset return self._build_response(queryset) -class RuleTestView(ConditionalLoginRequiredMixin, View): +class RuleTestView(BaseMultiObjectView): """Live-preview a name template with user-supplied variable values and optional DB lookup.""" + queryset = InterfaceNameRule.objects.all() template_name = "netbox_interface_name_rules/rule_test.html" - def _check_permission(self, request): - if not request.user.has_perm("netbox_interface_name_rules.add_interfacenamerule"): - raise PermissionDenied + def get_required_permission(self): + """Return the permission required to access the rule test tool.""" + return "netbox_interface_name_rules.add_interfacenamerule" def get(self, request): """Render the test form, pre-populated from rule_id query param if given.""" - self._check_permission(request) initial = {} loaded_rule = None rule_id = request.GET.get("rule_id") @@ -196,7 +200,6 @@ def get(self, request): def post(self, request): """Evaluate the submitted template and return a preview or redirect to save.""" - self._check_permission(request) form = RuleTestForm(request.POST) preview_results = None db_preview = None @@ -348,37 +351,42 @@ def _fetch_db_preview(self, cd): return [], 0, f"Unexpected error: {type(exc).__name__}" -class RuleApplyListView(ConditionalLoginRequiredMixin, View): +class RuleApplyListView(BaseMultiObjectView): """Display all rules with buttons to preview/apply each one.""" + queryset = InterfaceNameRule.objects.all() template_name = "netbox_interface_name_rules/rule_apply.html" + def get_required_permission(self): + """Return the permission required to access the apply-rules page.""" + return "netbox_interface_name_rules.view_interfacenamerule" + def get(self, request): """Render the list of all rules with apply/preview buttons.""" - rules = InterfaceNameRule.objects.select_related( - "module_type", "parent_module_type", "device_type", "platform" - ).order_by("pk") + rules = self.queryset.select_related("module_type", "parent_module_type", "device_type", "platform").order_by( + "pk" + ) return render(request, self.template_name, {"rules": rules, "batch_limit": APPLY_BATCH_LIMIT}) -class RuleApplicableView(ConditionalLoginRequiredMixin, View): +class RuleApplicableView(generic.ObjectView): """Return JSON indicating whether a rule would rename at least one interface. Called on demand from the Apply Rules page — NOT at page load — to avoid expensive full-scan queries blocking the initial render. """ - def get(self, request, pk): + queryset = InterfaceNameRule.objects.all() + + def get(self, request, **kwargs): """Return JSON {"applicable": bool} for the rule identified by pk.""" from .engine import has_applicable_interfaces - rule = get_object_or_404(InterfaceNameRule, pk=pk) + rule = self.get_object(**kwargs) try: applicable = has_applicable_interfaces(rule) except Exception as exc: - logger.exception("applicability scan failed for rule %s", pk) - # Only expose the exception class name to avoid leaking internals - # (SQL, file paths, etc.). Full details are in the server log. + logger.exception("applicability scan failed for rule %s", kwargs.get("pk")) return JsonResponse( {"applicable": None, "error": f"scan failed: {type(exc).__name__}"}, status=500, @@ -386,21 +394,18 @@ def get(self, request, pk): return JsonResponse({"applicable": applicable}) -class RuleApplyDetailView(ConditionalLoginRequiredMixin, View): +class RuleApplyDetailView(generic.ObjectView): """Show a preview of changes for a specific rule and allow applying them.""" + queryset = InterfaceNameRule.objects.all() template_name = "netbox_interface_name_rules/rule_apply_detail.html" + additional_permissions = ["dcim.change_interface"] - def _check_permission(self, request): - if not request.user.has_perm("dcim.change_interface"): - raise PermissionDenied - - def get(self, request, pk): + def get(self, request, **kwargs): """Render a preview of all interfaces that would be renamed by this rule.""" from .engine import find_interfaces_for_rule - self._check_permission(request) - rule = get_object_or_404(InterfaceNameRule, pk=pk) + rule = self.get_object(**kwargs) try: preview, total_checked = find_interfaces_for_rule(rule, limit=APPLY_BATCH_LIMIT) except (re.error, ValueError) as exc: @@ -424,13 +429,11 @@ def get(self, request, pk): }, ) - def post(self, request, pk): + def post(self, request, **kwargs): """Apply the rule (foreground batch or background job) and redirect back.""" from .engine import apply_rule_to_existing - self._check_permission(request) - - rule = get_object_or_404(InterfaceNameRule, pk=pk) + rule = self.get_object(**kwargs) action = request.POST.get("action", "apply") if action == "background": From 79022cacc032717de719323ba9e4582b729d1ba5 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 10:18:41 +0200 Subject: [PATCH 12/22] fix: deterministic YAML export ordering and view permission guards - Add .order_by("pk") to YAML export querysets for stable diffs - Guard rule prefill and duplicate-check lookups with view permission - Strengthen POST export test to assert returned rule identity --- .../tests/test_views.py | 1 + netbox_interface_name_rules/views.py | 46 ++++++++++--------- 2 files changed, 26 insertions(+), 21 deletions(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index 72bbe6d..dcc5094 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -810,6 +810,7 @@ def test_yaml_export_selected_rules_via_post(self): data = yaml.safe_load(response.content) self.assertIsInstance(data, list) self.assertEqual(len(data), 1) + self.assertEqual(data[0]["name_template"], self.rule.name_template) def test_yaml_export_structure(self): """Each exported YAML entry must contain key fields.""" diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index d2819e6..4d5984b 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -146,7 +146,7 @@ def _build_response(self, queryset): def get(self, request): """Export all rules as YAML.""" - return self._build_response(self.queryset) + return self._build_response(self.queryset.order_by("pk")) def post(self, request): """Export selected rules as YAML using NetBox's bulk-select form (pk inputs). @@ -157,9 +157,9 @@ def post(self, request): """ pk_list = [int(v) for v in request.POST.getlist("pk") if v.isdigit()] if pk_list: - queryset = self.queryset.filter(pk__in=pk_list) + queryset = self.queryset.filter(pk__in=pk_list).order_by("pk") else: - queryset = self.queryset + queryset = self.queryset.order_by("pk") return self._build_response(queryset) @@ -178,7 +178,8 @@ def get(self, request): initial = {} loaded_rule = None rule_id = request.GET.get("rule_id") - if rule_id: + can_view = request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule") + if rule_id and can_view: try: loaded_rule = InterfaceNameRule.objects.select_related( "module_type", "parent_module_type", "device_type", "platform" @@ -239,24 +240,27 @@ def _handle_save_rule(self, request, cd): module_type = cd.get("module_type") module_type_pattern = cd.get("module_type_pattern", "") - qs = InterfaceNameRule.objects.all() - if module_type_is_regex: - qs = qs.filter(module_type_is_regex=True, module_type_pattern=module_type_pattern) - else: - qs = qs.filter(module_type_is_regex=False, module_type=module_type) - for field in ("parent_module_type", "device_type", "platform"): - val = cd.get(field) - if val: - qs = qs.filter(**{field: val}) + if request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): + qs = InterfaceNameRule.objects.all() + if module_type_is_regex: + qs = qs.filter(module_type_is_regex=True, module_type_pattern=module_type_pattern) else: - qs = qs.filter(**{f"{field}__isnull": True}) - existing = qs.first() - if existing: - messages.info( - request, - f"A matching rule already exists (#{existing.pk}). Redirecting to edit it.", - ) - return redirect(reverse("plugins:netbox_interface_name_rules:interfacenamerule_edit", args=[existing.pk])) + qs = qs.filter(module_type_is_regex=False, module_type=module_type) + for field in ("parent_module_type", "device_type", "platform"): + val = cd.get(field) + if val: + qs = qs.filter(**{field: val}) + else: + qs = qs.filter(**{f"{field}__isnull": True}) + existing = qs.first() + if existing: + messages.info( + request, + f"A matching rule already exists (#{existing.pk}). Redirecting to edit it.", + ) + return redirect( + reverse("plugins:netbox_interface_name_rules:interfacenamerule_edit", args=[existing.pk]) + ) params = { "name_template": name_template, From 65f0f5e7f281abbd91cd09f221c8b408d69effa7 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 10:23:14 +0200 Subject: [PATCH 13/22] fix: clarify operator precedence, add warning for add-only prefill, add permission tests - Add parentheses around or-condition in _rules_to_yaml_data for clarity - Warn add-only users when rule_id prefill is skipped due to missing view permission - Add comment explaining why duplicate detection is skipped without view permission - Add tests for add-only user: blank form with warning on prefill, and save_rule skipping duplicate check --- .../tests/test_views.py | 47 +++++++++++++++++++ netbox_interface_name_rules/views.py | 7 ++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index dcc5094..ced3d9e 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -402,6 +402,53 @@ def test_post_no_permission_raises_403(self): self.assertEqual(response.status_code, 403) +class RuleTestViewAddOnlyPermissionTest(ViewTestBase2): + """Test RuleTestView behaviour for users with add but not view permission.""" + + def _url(self): + return reverse("plugins:netbox_interface_name_rules:interfacenamerule_test") + + def _create_add_only_user(self): + from django.contrib.auth.models import Permission + from django.contrib.contenttypes.models import ContentType + + user = User.objects.create_user(username="add_only_tester", password=TEST_PASSWORD) + ct = ContentType.objects.get_for_model(InterfaceNameRule) + perm = Permission.objects.get(content_type=ct, codename="add_interfacenamerule") + user.user_permissions.add(perm) + return user + + def test_get_with_rule_id_add_only_shows_blank_form_with_warning(self): + """GET with ?rule_id= when user has add but not view permission returns blank form.""" + user = self._create_add_only_user() + self.client.force_login(user) + response = self.client.get(self._url() + f"?rule_id={self.rule.pk}") + self.assertEqual(response.status_code, 200) + form = response.context["form"] + self.assertFalse(form.initial, "Form should have no initial data for add-only user") + self.assertIsNone(response.context.get("loaded_rule")) + from django.contrib.messages import get_messages + + msgs = [str(m) for m in get_messages(response.wsgi_request)] + self.assertTrue(any("permission" in m.lower() for m in msgs)) + + def test_save_rule_add_only_skips_duplicate_check(self): + """POST save_rule with add-only permission skips duplicate detection, redirects to add.""" + user = self._create_add_only_user() + self.client.force_login(user) + add_url = reverse("plugins:netbox_interface_name_rules:interfacenamerule_add") + data = { + "name_template": "ge-0/0/{bay_position}", + "channel_count": "0", + "channel_start": "0", + "module_type": str(self.module_type.pk), + "action": "save_rule", + } + response = self.client.post(self._url(), data) + self.assertEqual(response.status_code, 302) + self.assertIn(add_url, response["Location"]) + + class RuleApplyDetailViewGetPermissionTest(ViewTestBase2): """Test RuleApplyDetailView.get() permission check for unprivileged users.""" diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index 4d5984b..fa265c2 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -132,7 +132,7 @@ def _rules_to_yaml_data(self, queryset): for rule in queryset.select_related("module_type", "parent_module_type", "device_type", "platform"): entry = {} for header, value in zip(InterfaceNameRule.csv_headers, rule.to_csv()): - if value != "" and value is not None or header in {"name_template"}: + if (value != "" and value is not None) or header in {"name_template"}: entry[header] = value records.append(entry) return records @@ -179,6 +179,8 @@ def get(self, request): loaded_rule = None rule_id = request.GET.get("rule_id") can_view = request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule") + if rule_id and not can_view: + messages.warning(request, "You do not have permission to load an existing rule.") if rule_id and can_view: try: loaded_rule = InterfaceNameRule.objects.select_related( @@ -240,6 +242,9 @@ def _handle_save_rule(self, request, cd): module_type = cd.get("module_type") module_type_pattern = cd.get("module_type_pattern", "") + # Skip duplicate detection when the user lacks view permission — we cannot + # query existing rules without it, so add-only users always land on the + # create form (potentially allowing duplicates). if request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): qs = InterfaceNameRule.objects.all() if module_type_is_regex: From 6ad692e981e313a1f745733b0d45b800de9a3a8e Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 11:03:32 +0200 Subject: [PATCH 14/22] fix: re-fetch user after adding permission to clear Django cache Django caches permissions on the User instance; after user_permissions.add() the in-memory object still lacks the new permission, causing dispatch to return 403. --- netbox_interface_name_rules/tests/test_views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index ced3d9e..5bcd77a 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -416,7 +416,8 @@ def _create_add_only_user(self): ct = ContentType.objects.get_for_model(InterfaceNameRule) perm = Permission.objects.get(content_type=ct, codename="add_interfacenamerule") user.user_permissions.add(perm) - return user + # Re-fetch to clear Django's permission cache on the User instance. + return User.objects.get(pk=user.pk) def test_get_with_rule_id_add_only_shows_blank_form_with_warning(self): """GET with ?rule_id= when user has add but not view permission returns blank form.""" From 25534d909d323c596acc1bbc6178cbc148b4544f Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 11:07:31 +0200 Subject: [PATCH 15/22] refactor: extract _find_existing_rule to reduce cognitive complexity SonarQube flagged _handle_save_rule at complexity 20 (limit 15). Extract the queryset filtering into _find_existing_rule (~6) bringing _handle_save_rule down to ~11. --- netbox_interface_name_rules/views.py | 32 ++++++++++++++++------------ 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index fa265c2..43d9d21 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -231,6 +231,22 @@ def post(self, request): }, ) + def _find_existing_rule(self, cd): + """Return the first existing rule matching the form data, or None.""" + module_type_is_regex = cd.get("module_type_is_regex", False) + qs = InterfaceNameRule.objects.all() + if module_type_is_regex: + qs = qs.filter(module_type_is_regex=True, module_type_pattern=cd.get("module_type_pattern", "")) + else: + qs = qs.filter(module_type_is_regex=False, module_type=cd.get("module_type")) + for field in ("parent_module_type", "device_type", "platform"): + val = cd.get(field) + if val: + qs = qs.filter(**{field: val}) + else: + qs = qs.filter(**{f"{field}__isnull": True}) + return qs.first() + def _handle_save_rule(self, request, cd): """Find an existing matching rule or redirect to the add-rule form with pre-filled params.""" from urllib.parse import urlencode @@ -240,24 +256,12 @@ def _handle_save_rule(self, request, cd): channel_start = cd.get("channel_start") or 0 module_type_is_regex = cd.get("module_type_is_regex", False) module_type = cd.get("module_type") - module_type_pattern = cd.get("module_type_pattern", "") # Skip duplicate detection when the user lacks view permission — we cannot # query existing rules without it, so add-only users always land on the # create form (potentially allowing duplicates). if request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): - qs = InterfaceNameRule.objects.all() - if module_type_is_regex: - qs = qs.filter(module_type_is_regex=True, module_type_pattern=module_type_pattern) - else: - qs = qs.filter(module_type_is_regex=False, module_type=module_type) - for field in ("parent_module_type", "device_type", "platform"): - val = cd.get(field) - if val: - qs = qs.filter(**{field: val}) - else: - qs = qs.filter(**{f"{field}__isnull": True}) - existing = qs.first() + existing = self._find_existing_rule(cd) if existing: messages.info( request, @@ -274,7 +278,7 @@ def _handle_save_rule(self, request, cd): "channel_start": channel_start, } if module_type_is_regex: - params["module_type_pattern"] = module_type_pattern + params["module_type_pattern"] = cd.get("module_type_pattern", "") elif module_type: params["module_type"] = module_type.pk for field in ("parent_module_type", "device_type", "platform"): From 304ce97d9e65418a9c0f3bff2c038cd2518df142 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 11:08:31 +0200 Subject: [PATCH 16/22] refactor: extract PERM_VIEW_RULE constant for duplicated permission string --- netbox_interface_name_rules/views.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index 43d9d21..e2a606e 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -23,6 +23,8 @@ logger = logging.getLogger(__name__) +PERM_VIEW_RULE = "netbox_interface_name_rules.view_interfacenamerule" + try: _plugins_config = getattr(settings, "PLUGINS_CONFIG", {}) APPLY_BATCH_LIMIT = max(1, int(_plugins_config.get("netbox_interface_name_rules", {}).get("apply_batch_limit", 50))) @@ -124,7 +126,7 @@ class InterfaceNameRuleYAMLExportView(BaseMultiObjectView): def get_required_permission(self): """Return the permission required to export rules.""" - return "netbox_interface_name_rules.view_interfacenamerule" + return PERM_VIEW_RULE def _rules_to_yaml_data(self, queryset): """Return a list of dicts (one per rule) using import-form field names.""" @@ -178,7 +180,7 @@ def get(self, request): initial = {} loaded_rule = None rule_id = request.GET.get("rule_id") - can_view = request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule") + can_view = request.user.has_perm(PERM_VIEW_RULE) if rule_id and not can_view: messages.warning(request, "You do not have permission to load an existing rule.") if rule_id and can_view: @@ -260,7 +262,7 @@ def _handle_save_rule(self, request, cd): # Skip duplicate detection when the user lacks view permission — we cannot # query existing rules without it, so add-only users always land on the # create form (potentially allowing duplicates). - if request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): + if request.user.has_perm(PERM_VIEW_RULE): existing = self._find_existing_rule(cd) if existing: messages.info( @@ -372,7 +374,7 @@ class RuleApplyListView(BaseMultiObjectView): def get_required_permission(self): """Return the permission required to access the apply-rules page.""" - return "netbox_interface_name_rules.view_interfacenamerule" + return PERM_VIEW_RULE def get(self, request): """Render the list of all rules with apply/preview buttons.""" From 3fbc3211d59df616ba189ff17244583f53f9c9fb Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 11:09:54 +0200 Subject: [PATCH 17/22] Revert "refactor: extract PERM_VIEW_RULE constant for duplicated permission string" This reverts commit 304ce97d9e65418a9c0f3bff2c038cd2518df142. --- netbox_interface_name_rules/views.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index e2a606e..43d9d21 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -23,8 +23,6 @@ logger = logging.getLogger(__name__) -PERM_VIEW_RULE = "netbox_interface_name_rules.view_interfacenamerule" - try: _plugins_config = getattr(settings, "PLUGINS_CONFIG", {}) APPLY_BATCH_LIMIT = max(1, int(_plugins_config.get("netbox_interface_name_rules", {}).get("apply_batch_limit", 50))) @@ -126,7 +124,7 @@ class InterfaceNameRuleYAMLExportView(BaseMultiObjectView): def get_required_permission(self): """Return the permission required to export rules.""" - return PERM_VIEW_RULE + return "netbox_interface_name_rules.view_interfacenamerule" def _rules_to_yaml_data(self, queryset): """Return a list of dicts (one per rule) using import-form field names.""" @@ -180,7 +178,7 @@ def get(self, request): initial = {} loaded_rule = None rule_id = request.GET.get("rule_id") - can_view = request.user.has_perm(PERM_VIEW_RULE) + can_view = request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule") if rule_id and not can_view: messages.warning(request, "You do not have permission to load an existing rule.") if rule_id and can_view: @@ -262,7 +260,7 @@ def _handle_save_rule(self, request, cd): # Skip duplicate detection when the user lacks view permission — we cannot # query existing rules without it, so add-only users always land on the # create form (potentially allowing duplicates). - if request.user.has_perm(PERM_VIEW_RULE): + if request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): existing = self._find_existing_rule(cd) if existing: messages.info( @@ -374,7 +372,7 @@ class RuleApplyListView(BaseMultiObjectView): def get_required_permission(self): """Return the permission required to access the apply-rules page.""" - return PERM_VIEW_RULE + return "netbox_interface_name_rules.view_interfacenamerule" def get(self, request): """Render the list of all rules with apply/preview buttons.""" From 3ca6d090c92312c3e9b462fe3129a42a86e554f2 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 11:28:48 +0200 Subject: [PATCH 18/22] refactor: wire YAML export into NetBox's built-in Export dropdown Replace the standalone InterfaceNameRuleYAMLExportView with NetBox's native export mechanism: - Add to_yaml() to InterfaceNameRule model so NetBox offers YAML in the Export dropdown - Override export_yaml() on the list view for deterministic pk-ordered output with empty-field stripping - Remove standalone /rules/export/yaml/ URL and custom template buttons - Rewrite tests to use the list view's ?export= parameter --- netbox_interface_name_rules/models.py | 10 +++ .../interfacenamerule_list.html | 18 ------ .../tests/test_views.py | 45 ++++---------- netbox_interface_name_rules/urls.py | 1 - netbox_interface_name_rules/views.py | 61 +++++-------------- 5 files changed, 36 insertions(+), 99 deletions(-) diff --git a/netbox_interface_name_rules/models.py b/netbox_interface_name_rules/models.py index 0a7dadd..1d14891 100644 --- a/netbox_interface_name_rules/models.py +++ b/netbox_interface_name_rules/models.py @@ -315,3 +315,13 @@ def to_csv(self): self.enabled, self.applies_to_device_interfaces, ) + + def to_yaml(self): + """Return a YAML document for this rule (used by NetBox's built-in Export).""" + import yaml + + entry = {} + for header, value in zip(self.csv_headers, self.to_csv()): + if (value != "" and value is not None) or header in {"name_template"}: + entry[header] = value + return yaml.dump([entry], default_flow_style=False, allow_unicode=True, sort_keys=False) diff --git a/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html b/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html index 3b85cf7..217b0d1 100644 --- a/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html +++ b/netbox_interface_name_rules/templates/netbox_interface_name_rules/interfacenamerule_list.html @@ -145,21 +145,3 @@
Examples
{{ block.super }} {% endblock %} - -{% block extra_controls %} - - Export YAML - -{% endblock %} - -{% block bulk_buttons %} - {{ block.super }} - -{% endblock %} diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index 5bcd77a..f642a5c 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -790,47 +790,41 @@ def test_csv_round_trip_creates_rule(self): # --------------------------------------------------------------------------- -# YAMLExportViewTest — new YAML export view +# YAMLExportTest — YAML export via NetBox's built-in Export dropdown # --------------------------------------------------------------------------- -class YAMLExportViewTest(ViewTestBase): - """Tests for the YAML export view at GET/POST /rules/export/yaml/.""" +class YAMLExportTest(ViewTestBase): + """Tests for YAML export via the list view's ?export query parameter.""" @classmethod def setUpTestData(cls): super().setUpTestData() - cls.YAML_URL = reverse("plugins:netbox_interface_name_rules:interfacenamerule_export_yaml") + cls.LIST_URL = reverse("plugins:netbox_interface_name_rules:interfacenamerule_list") def test_yaml_export_unauthenticated_responds(self): - """Unauthenticated GET must not serve content to anonymous users. - - setUp() logs in the superuser, so we explicitly logout first to exercise - anonymous access. When LOGIN_REQUIRED=True the mixin redirects (302); - when False the view's explicit has_perm check raises PermissionDenied - (403). 200 is never valid because AnonymousUser lacks the permission. - """ + """Unauthenticated export must not serve content to anonymous users.""" self.client.logout() - response = self.client.get(self.YAML_URL) + response = self.client.get(self.LIST_URL, {"export": ""}) self.assertIn(response.status_code, [301, 302, 403]) def test_yaml_export_all_returns_200(self): - """Authenticated GET /rules/export/yaml/ must return 200.""" + """Authenticated GET ?export= must return 200.""" self.client.force_login(self.superuser) - response = self.client.get(self.YAML_URL) + response = self.client.get(self.LIST_URL, {"export": ""}) self.assertEqual(response.status_code, 200) def test_yaml_export_content_type(self): """Response Content-Type must contain 'yaml'.""" self.client.force_login(self.superuser) - response = self.client.get(self.YAML_URL) + response = self.client.get(self.LIST_URL, {"export": ""}) self.assertEqual(response.status_code, 200) self.assertIn("yaml", response["Content-Type"].lower()) def test_yaml_export_content_disposition(self): """Response must include a Content-Disposition attachment header.""" self.client.force_login(self.superuser) - response = self.client.get(self.YAML_URL) + response = self.client.get(self.LIST_URL, {"export": ""}) self.assertEqual(response.status_code, 200) self.assertIn("attachment", response.get("Content-Disposition", "").lower()) @@ -839,33 +833,18 @@ def test_yaml_export_all_contains_rules(self): import yaml self.client.force_login(self.superuser) - response = self.client.get(self.YAML_URL) + response = self.client.get(self.LIST_URL, {"export": ""}) self.assertEqual(response.status_code, 200) data = yaml.safe_load(response.content) self.assertIsInstance(data, list) self.assertEqual(len(data), InterfaceNameRule.objects.count()) - def test_yaml_export_selected_rules_via_post(self): - """POST with NetBox bulk-select pk inputs must export only the selected rules.""" - import yaml - - self.client.force_login(self.superuser) - response = self.client.post( - self.YAML_URL, - {"pk": [self.rule.pk]}, - ) - self.assertEqual(response.status_code, 200) - data = yaml.safe_load(response.content) - self.assertIsInstance(data, list) - self.assertEqual(len(data), 1) - self.assertEqual(data[0]["name_template"], self.rule.name_template) - def test_yaml_export_structure(self): """Each exported YAML entry must contain key fields.""" import yaml self.client.force_login(self.superuser) - response = self.client.get(self.YAML_URL) + response = self.client.get(self.LIST_URL, {"export": ""}) self.assertEqual(response.status_code, 200) rules = yaml.safe_load(response.content) self.assertIsInstance(rules, list) diff --git a/netbox_interface_name_rules/urls.py b/netbox_interface_name_rules/urls.py index 1223798..5ba86fa 100644 --- a/netbox_interface_name_rules/urls.py +++ b/netbox_interface_name_rules/urls.py @@ -12,7 +12,6 @@ path("rules/add/", views.InterfaceNameRuleCreateView.as_view(), name="interfacenamerule_add"), path("rules/import/", views.InterfaceNameRuleBulkImportView.as_view(), name="interfacenamerule_bulk_import"), path("rules/bulk_delete/", views.InterfaceNameRuleBulkDeleteView.as_view(), name="interfacenamerule_bulk_delete"), - path("rules/export/yaml/", views.InterfaceNameRuleYAMLExportView.as_view(), name="interfacenamerule_export_yaml"), # Rule tester (Build Rule) path("rules/test/", views.RuleTestView.as_view(), name="interfacenamerule_test"), # Apply rules to existing interfaces diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index 43d9d21..5d2ed21 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -8,7 +8,7 @@ from django.conf import settings from django.contrib import messages from django.core.exceptions import PermissionDenied -from django.http import HttpResponse, JsonResponse +from django.http import JsonResponse from django.shortcuts import redirect, render from django.urls import reverse from django.utils.http import url_has_allowed_host_and_scheme @@ -54,6 +54,19 @@ class InterfaceNameRuleListView(generic.ObjectListView): filterset_form = InterfaceNameRuleFilterForm template_name = "netbox_interface_name_rules/interfacenamerule_list.html" + def export_yaml(self): + """Export all rules as a single YAML list (overrides NetBox's per-object concatenation).""" + data = [] + for rule in self.queryset.order_by("pk").select_related( + "module_type", "parent_module_type", "device_type", "platform" + ): + entry = {} + for header, value in zip(InterfaceNameRule.csv_headers, rule.to_csv()): + if (value != "" and value is not None) or header in {"name_template"}: + entry[header] = value + data.append(entry) + return yaml.dump(data, default_flow_style=False, allow_unicode=True, sort_keys=False) + class InterfaceNameRuleCreateView(generic.ObjectEditView): """Create view for InterfaceNameRule.""" @@ -117,52 +130,6 @@ def get(self, request, **kwargs): return redirect(f"{url}?{params.urlencode()}") -class InterfaceNameRuleYAMLExportView(BaseMultiObjectView): - """Export selected or all rules as a YAML file importable via BulkImportView.""" - - queryset = InterfaceNameRule.objects.all() - - def get_required_permission(self): - """Return the permission required to export rules.""" - return "netbox_interface_name_rules.view_interfacenamerule" - - def _rules_to_yaml_data(self, queryset): - """Return a list of dicts (one per rule) using import-form field names.""" - records = [] - for rule in queryset.select_related("module_type", "parent_module_type", "device_type", "platform"): - entry = {} - for header, value in zip(InterfaceNameRule.csv_headers, rule.to_csv()): - if (value != "" and value is not None) or header in {"name_template"}: - entry[header] = value - records.append(entry) - return records - - def _build_response(self, queryset): - data = self._rules_to_yaml_data(queryset) - content = yaml.dump(data, default_flow_style=False, allow_unicode=True, sort_keys=False) - response = HttpResponse(content, content_type="application/yaml; charset=utf-8") - response["Content-Disposition"] = 'attachment; filename="interface_name_rules.yaml"' - return response - - def get(self, request): - """Export all rules as YAML.""" - return self._build_response(self.queryset.order_by("pk")) - - def post(self, request): - """Export selected rules as YAML using NetBox's bulk-select form (pk inputs). - - The parent object_list.html wraps the table in a POST form; selected rows - post their PKs as repeated ``pk`` inputs. Falls back to all rules when - nothing is selected. - """ - pk_list = [int(v) for v in request.POST.getlist("pk") if v.isdigit()] - if pk_list: - queryset = self.queryset.filter(pk__in=pk_list).order_by("pk") - else: - queryset = self.queryset.order_by("pk") - return self._build_response(queryset) - - class RuleTestView(BaseMultiObjectView): """Live-preview a name template with user-supplied variable values and optional DB lookup.""" From 2035bda9c76a47aafef8d336e17ff3db55952a97 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 11:56:09 +0200 Subject: [PATCH 19/22] fix: use NetBox ObjectPermission for add-only user tests Django's built-in user_permissions.add() does not work with NetBox's ObjectPermissionRequiredMixin. Create an ObjectPermission with actions=["add"] instead. --- netbox_interface_name_rules/tests/test_views.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index f642a5c..fee97b0 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -409,15 +409,15 @@ def _url(self): return reverse("plugins:netbox_interface_name_rules:interfacenamerule_test") def _create_add_only_user(self): - from django.contrib.auth.models import Permission from django.contrib.contenttypes.models import ContentType + from users.models import ObjectPermission user = User.objects.create_user(username="add_only_tester", password=TEST_PASSWORD) ct = ContentType.objects.get_for_model(InterfaceNameRule) - perm = Permission.objects.get(content_type=ct, codename="add_interfacenamerule") - user.user_permissions.add(perm) - # Re-fetch to clear Django's permission cache on the User instance. - return User.objects.get(pk=user.pk) + obj_perm = ObjectPermission.objects.create(name="add_only_rule", actions=["add"]) + obj_perm.object_types.add(ct) + obj_perm.users.add(user) + return user def test_get_with_rule_id_add_only_shows_blank_form_with_warning(self): """GET with ?rule_id= when user has add but not view permission returns blank form.""" From 9cabb29d11843056adde621182dae42db306816a Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 12:04:46 +0200 Subject: [PATCH 20/22] fix: remove CSV "Current View" from Export dropdown The table CSV export uses verbose column names that don't match the import form fields, making round-trip import impossible. Replace the BulkExport action with a YAML-only variant that omits the "Current View" option, keeping only "All Data (YAML)". --- .../buttons/export_yaml_only.html | 33 +++++++++++++++++++ netbox_interface_name_rules/views.py | 8 +++++ 2 files changed, 41 insertions(+) create mode 100644 netbox_interface_name_rules/templates/netbox_interface_name_rules/buttons/export_yaml_only.html diff --git a/netbox_interface_name_rules/templates/netbox_interface_name_rules/buttons/export_yaml_only.html b/netbox_interface_name_rules/templates/netbox_interface_name_rules/buttons/export_yaml_only.html new file mode 100644 index 0000000..11fe7ef --- /dev/null +++ b/netbox_interface_name_rules/templates/netbox_interface_name_rules/buttons/export_yaml_only.html @@ -0,0 +1,33 @@ +{# SPDX-License-Identifier: Apache-2.0 #} +{# Copyright (C) 2025 Marcin Zieba #} +{% load i18n %} +
+ + +
diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index 5d2ed21..409fb2b 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -12,6 +12,7 @@ from django.shortcuts import redirect, render from django.urls import reverse from django.utils.http import url_has_allowed_host_and_scheme +from netbox.object_actions import AddObject, BulkDelete, BulkEdit, BulkExport, BulkImport, BulkRename from netbox.views import generic from netbox.views.generic.base import BaseMultiObjectView from utilities.views import register_model_view @@ -45,6 +46,12 @@ class RulePreview: channel_start: int +class _YAMLOnlyExport(BulkExport): + """Export action that only offers YAML (no CSV "Current View" option).""" + + template_name = "netbox_interface_name_rules/buttons/export_yaml_only.html" + + class InterfaceNameRuleListView(generic.ObjectListView): """List view for InterfaceNameRule.""" @@ -53,6 +60,7 @@ class InterfaceNameRuleListView(generic.ObjectListView): filterset = InterfaceNameRuleFilterSet filterset_form = InterfaceNameRuleFilterForm template_name = "netbox_interface_name_rules/interfacenamerule_list.html" + actions = (AddObject, BulkImport, _YAMLOnlyExport, BulkEdit, BulkRename, BulkDelete) def export_yaml(self): """Export all rules as a single YAML list (overrides NetBox's per-object concatenation).""" From 39cf3a641555a07a3ff6d82d2325ddf4c0c6bf1e Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 13:16:59 +0200 Subject: [PATCH 21/22] fix: guard netbox.object_actions import for 4.3.x compatibility - Wrap netbox.object_actions import in try/except so the plugin loads on NetBox 4.3.7 where that module does not exist; _LIST_VIEW_ACTIONS falls back to None so ObjectListView uses its default actions - Use .restrict(user, 'view') in RuleTestView for permission-scoped lookups instead of bare objects.get() / objects.all() - URL-encode et.name in export_yaml_only.html to avoid query-string breakage on names containing &, = or # - Tighten YAML export tests: assert PK-ordered export and that no optional key carries a blank/None value --- .../buttons/export_yaml_only.html | 2 +- .../tests/test_views.py | 18 +++++++---- netbox_interface_name_rules/views.py | 31 ++++++++++++------- 3 files changed, 33 insertions(+), 18 deletions(-) diff --git a/netbox_interface_name_rules/templates/netbox_interface_name_rules/buttons/export_yaml_only.html b/netbox_interface_name_rules/templates/netbox_interface_name_rules/buttons/export_yaml_only.html index 11fe7ef..d1ccffc 100644 --- a/netbox_interface_name_rules/templates/netbox_interface_name_rules/buttons/export_yaml_only.html +++ b/netbox_interface_name_rules/templates/netbox_interface_name_rules/buttons/export_yaml_only.html @@ -13,7 +13,7 @@ {% for et in export_templates %}
  • - {{ et.name }} diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index fee97b0..1cd3c0f 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -829,7 +829,7 @@ def test_yaml_export_content_disposition(self): self.assertIn("attachment", response.get("Content-Disposition", "").lower()) def test_yaml_export_all_contains_rules(self): - """Exported YAML must be parseable and contain all existing rules.""" + """Exported YAML must be parseable, contain all rules, in PK order.""" import yaml self.client.force_login(self.superuser) @@ -837,10 +837,12 @@ def test_yaml_export_all_contains_rules(self): self.assertEqual(response.status_code, 200) data = yaml.safe_load(response.content) self.assertIsInstance(data, list) - self.assertEqual(len(data), InterfaceNameRule.objects.count()) + expected_templates = list(InterfaceNameRule.objects.order_by("pk").values_list("name_template", flat=True)) + exported_templates = [entry["name_template"] for entry in data] + self.assertEqual(exported_templates, expected_templates) def test_yaml_export_structure(self): - """Each exported YAML entry must contain key fields.""" + """Each exported YAML entry must contain key fields and no blank optional keys.""" import yaml self.client.force_login(self.superuser) @@ -848,6 +850,10 @@ def test_yaml_export_structure(self): self.assertEqual(response.status_code, 200) rules = yaml.safe_load(response.content) self.assertIsInstance(rules, list) - entry = rules[0] - self.assertIsInstance(entry, dict) - self.assertIn("name_template", entry) + optional_headers = set(InterfaceNameRule.csv_headers) - {"name_template"} + for entry in rules: + self.assertIsInstance(entry, dict) + self.assertIn("name_template", entry) + for key, value in entry.items(): + if key in optional_headers: + self.assertNotIn(value, ["", None], f"Key {key!r} has blank value in exported rule") diff --git a/netbox_interface_name_rules/views.py b/netbox_interface_name_rules/views.py index 409fb2b..5a39892 100644 --- a/netbox_interface_name_rules/views.py +++ b/netbox_interface_name_rules/views.py @@ -12,7 +12,6 @@ from django.shortcuts import redirect, render from django.urls import reverse from django.utils.http import url_has_allowed_host_and_scheme -from netbox.object_actions import AddObject, BulkDelete, BulkEdit, BulkExport, BulkImport, BulkRename from netbox.views import generic from netbox.views.generic.base import BaseMultiObjectView from utilities.views import register_model_view @@ -46,10 +45,17 @@ class RulePreview: channel_start: int -class _YAMLOnlyExport(BulkExport): - """Export action that only offers YAML (no CSV "Current View" option).""" +try: + from netbox.object_actions import AddObject, BulkDelete, BulkEdit, BulkExport, BulkImport, BulkRename + + class _YAMLOnlyExport(BulkExport): + """Export action that only offers YAML (no CSV "Current View" option).""" - template_name = "netbox_interface_name_rules/buttons/export_yaml_only.html" + template_name = "netbox_interface_name_rules/buttons/export_yaml_only.html" + + _LIST_VIEW_ACTIONS: tuple = (AddObject, BulkImport, _YAMLOnlyExport, BulkEdit, BulkRename, BulkDelete) +except ImportError: + _LIST_VIEW_ACTIONS = None class InterfaceNameRuleListView(generic.ObjectListView): @@ -60,7 +66,8 @@ class InterfaceNameRuleListView(generic.ObjectListView): filterset = InterfaceNameRuleFilterSet filterset_form = InterfaceNameRuleFilterForm template_name = "netbox_interface_name_rules/interfacenamerule_list.html" - actions = (AddObject, BulkImport, _YAMLOnlyExport, BulkEdit, BulkRename, BulkDelete) + if _LIST_VIEW_ACTIONS is not None: + actions = _LIST_VIEW_ACTIONS def export_yaml(self): """Export all rules as a single YAML list (overrides NetBox's per-object concatenation).""" @@ -158,9 +165,11 @@ def get(self, request): messages.warning(request, "You do not have permission to load an existing rule.") if rule_id and can_view: try: - loaded_rule = InterfaceNameRule.objects.select_related( - "module_type", "parent_module_type", "device_type", "platform" - ).get(pk=int(rule_id)) + loaded_rule = ( + InterfaceNameRule.objects.restrict(request.user, "view") + .select_related("module_type", "parent_module_type", "device_type", "platform") + .get(pk=int(rule_id)) + ) initial = { "name_template": loaded_rule.name_template, "module_type_is_regex": loaded_rule.module_type_is_regex, @@ -206,10 +215,10 @@ def post(self, request): }, ) - def _find_existing_rule(self, cd): + def _find_existing_rule(self, cd, user=None): """Return the first existing rule matching the form data, or None.""" module_type_is_regex = cd.get("module_type_is_regex", False) - qs = InterfaceNameRule.objects.all() + qs = InterfaceNameRule.objects.restrict(user, "view") if user else InterfaceNameRule.objects.all() if module_type_is_regex: qs = qs.filter(module_type_is_regex=True, module_type_pattern=cd.get("module_type_pattern", "")) else: @@ -236,7 +245,7 @@ def _handle_save_rule(self, request, cd): # query existing rules without it, so add-only users always land on the # create form (potentially allowing duplicates). if request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"): - existing = self._find_existing_rule(cd) + existing = self._find_existing_rule(cd, request.user) if existing: messages.info( request, From 6c39b35a87f0b104d0bd3108d29193f1f2693633 Mon Sep 17 00:00:00 2001 From: Marcin Zieba Date: Tue, 31 Mar 2026 13:30:45 +0200 Subject: [PATCH 22/22] fix: use related_model detection for ObjectPermission.object_types In NetBox 4.3.7 object_types M2M targets ObjectType not ContentType. Detect the actual related model at runtime so the correct type is passed to .add() regardless of NetBox version. --- netbox_interface_name_rules/tests/test_views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/netbox_interface_name_rules/tests/test_views.py b/netbox_interface_name_rules/tests/test_views.py index 1cd3c0f..f9339f1 100644 --- a/netbox_interface_name_rules/tests/test_views.py +++ b/netbox_interface_name_rules/tests/test_views.py @@ -409,11 +409,12 @@ def _url(self): return reverse("plugins:netbox_interface_name_rules:interfacenamerule_test") def _create_add_only_user(self): - from django.contrib.contenttypes.models import ContentType from users.models import ObjectPermission user = User.objects.create_user(username="add_only_tester", password=TEST_PASSWORD) - ct = ContentType.objects.get_for_model(InterfaceNameRule) + # object_types targets ContentType on older NetBox and ObjectType on newer NetBox + obj_type_model = ObjectPermission._meta.get_field("object_types").related_model + ct = obj_type_model.objects.get_for_model(InterfaceNameRule) obj_perm = ObjectPermission.objects.create(name="add_only_rule", actions=["add"]) obj_perm.object_types.add(ct) obj_perm.users.add(user)