Skip to content

Commit b734f67

Browse files
committed
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
1 parent e9b823c commit b734f67

4 files changed

Lines changed: 10 additions & 191 deletions

File tree

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,6 @@ site/
6060

6161
# direnv
6262
.envrc
63+
64+
# Brainstorming / design specs (local only, not for version control)
65+
docs/superpowers/

docs/superpowers/specs/2026-03-30-remove-module-path-plus-fixes-design.md

Lines changed: 0 additions & 178 deletions
This file was deleted.

netbox_interface_name_rules/tests/test_views.py

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -725,13 +725,12 @@ class YAMLExportViewTest(ViewTestBase):
725725

726726
YAML_URL = "/plugins/interface-name-rules/rules/export/yaml/"
727727

728-
def test_yaml_export_requires_login(self):
729-
"""Unauthenticated request behaviour depends on LOGIN_REQUIRED setting.
728+
def test_yaml_export_unauthenticated_responds(self):
729+
"""Unauthenticated GET must not return a server error (4xx or 2xx is acceptable).
730730
731-
ConditionalLoginRequiredMixin only enforces authentication when
732-
LOGIN_REQUIRED=True (the default in production). In the test environment
733-
LOGIN_REQUIRED may be False, so we just assert the view responds without
734-
a server error rather than asserting a redirect.
731+
ViewTestBase does not log in by default — this exercises anonymous access.
732+
ConditionalLoginRequiredMixin enforces auth only when LOGIN_REQUIRED=True;
733+
in the test environment that setting may be False, so 200 is also valid.
735734
"""
736735
response = self.client.get(self.YAML_URL)
737736
self.assertIn(response.status_code, [200, 301, 302])

netbox_interface_name_rules/views.py

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -124,20 +124,15 @@ def _rules_to_yaml_data(self, queryset):
124124
for rule in queryset.select_related("module_type", "parent_module_type", "device_type", "platform"):
125125
entry = {}
126126
for header, value in zip(InterfaceNameRule.csv_headers, rule.to_csv()):
127-
# Omit empty/falsy optional fields to keep output readable,
128-
# but always include required fields.
129-
required = {"name_template"}
130-
if value != "" and value is not None:
131-
entry[header] = value
132-
elif header in required:
127+
if value != "" and value is not None or header in {"name_template"}:
133128
entry[header] = value
134129
records.append(entry)
135130
return records
136131

137132
def _build_response(self, queryset):
138133
data = self._rules_to_yaml_data(queryset)
139134
content = yaml.dump(data, default_flow_style=False, allow_unicode=True, sort_keys=False)
140-
response = HttpResponse(content, content_type="application/x-yaml; charset=utf-8")
135+
response = HttpResponse(content, content_type="application/yaml; charset=utf-8")
141136
response["Content-Disposition"] = 'attachment; filename="interface_name_rules.yaml"'
142137
return response
143138

0 commit comments

Comments
 (0)