Skip to content

Commit b883698

Browse files
committed
refactor: align utility views to NetBox base classes
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)
1 parent fdd958a commit b883698

2 files changed

Lines changed: 48 additions & 45 deletions

File tree

netbox_interface_name_rules/tests/test_views.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,7 @@ def test_csv_import_does_not_raise_key_error(self):
692692
url = reverse("plugins:netbox_interface_name_rules:interfacenamerule_bulk_import")
693693
# If a KeyError is raised the view returns 500; assert it doesn't.
694694
try:
695-
response = self.client.post(url, {"data": csv_data, "format": "csv"})
695+
response = self.client.post(url, {"data": csv_data, "format": "csv", "csv_delimiter": "auto"})
696696
except KeyError as exc:
697697
self.fail(f"CSV import raised KeyError: {exc!r}")
698698
self.assertNotEqual(response.status_code, 500, "CSV import returned a 500 error")
@@ -734,7 +734,7 @@ def test_csv_round_trip_creates_rule(self):
734734

735735
url = reverse("plugins:netbox_interface_name_rules:interfacenamerule_bulk_import")
736736
before_count = InterfaceNameRule.objects.count()
737-
response = self.client.post(url, {"data": csv_data, "format": "csv"})
737+
response = self.client.post(url, {"data": csv_data, "format": "csv", "csv_delimiter": "auto"})
738738
# A successful import redirects (302); failure re-renders the form (200).
739739
self.assertEqual(response.status_code, 302, f"Import did not redirect; status={response.status_code}")
740740
after_count = InterfaceNameRule.objects.count()

netbox_interface_name_rules/views.py

Lines changed: 46 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,12 @@
99
from django.contrib import messages
1010
from django.core.exceptions import PermissionDenied
1111
from django.http import HttpResponse, JsonResponse
12-
from django.shortcuts import get_object_or_404, redirect, render
12+
from django.shortcuts import redirect, render
1313
from django.urls import reverse
1414
from django.utils.http import url_has_allowed_host_and_scheme
15-
from django.views import View
1615
from netbox.views import generic
17-
from utilities.views import ConditionalLoginRequiredMixin, register_model_view
16+
from netbox.views.generic.base import BaseMultiObjectView
17+
from utilities.views import register_model_view
1818

1919
from .filters import InterfaceNameRuleFilterSet
2020
from .forms import InterfaceNameRuleFilterForm, InterfaceNameRuleForm, InterfaceNameRuleImportForm, RuleTestForm
@@ -102,22 +102,30 @@ class InterfaceNameRuleChangeLogView(generic.ObjectChangeLogView):
102102
queryset = InterfaceNameRule.objects.all()
103103

104104

105-
class InterfaceNameRuleDuplicateView(ConditionalLoginRequiredMixin, View):
105+
class InterfaceNameRuleDuplicateView(generic.ObjectView):
106106
"""Redirect to the add view pre-populated with a clone of the given rule."""
107107

108-
def get(self, request, pk):
109-
"""Redirect to the add view pre-populated with fields cloned from rule pk."""
108+
queryset = InterfaceNameRule.objects.all()
109+
110+
def get(self, request, **kwargs):
111+
"""Redirect to the add view pre-populated with fields cloned from the given rule."""
110112
from utilities.querydict import prepare_cloned_fields
111113

112-
rule = get_object_or_404(InterfaceNameRule, pk=pk)
114+
rule = self.get_object(**kwargs)
113115
params = prepare_cloned_fields(rule)
114116
url = reverse("plugins:netbox_interface_name_rules:interfacenamerule_add")
115117
return redirect(f"{url}?{params.urlencode()}")
116118

117119

118-
class InterfaceNameRuleYAMLExportView(ConditionalLoginRequiredMixin, View):
120+
class InterfaceNameRuleYAMLExportView(BaseMultiObjectView):
119121
"""Export selected or all rules as a YAML file importable via BulkImportView."""
120122

123+
queryset = InterfaceNameRule.objects.all()
124+
125+
def get_required_permission(self):
126+
"""Return the permission required to export rules."""
127+
return "netbox_interface_name_rules.view_interfacenamerule"
128+
121129
def _rules_to_yaml_data(self, queryset):
122130
"""Return a list of dicts (one per rule) using import-form field names."""
123131
records = []
@@ -138,9 +146,7 @@ def _build_response(self, queryset):
138146

139147
def get(self, request):
140148
"""Export all rules as YAML."""
141-
if not request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"):
142-
raise PermissionDenied
143-
return self._build_response(InterfaceNameRule.objects.all())
149+
return self._build_response(self.queryset)
144150

145151
def post(self, request):
146152
"""Export selected rules as YAML using NetBox's bulk-select form (pk inputs).
@@ -149,28 +155,26 @@ def post(self, request):
149155
post their PKs as repeated ``pk`` inputs. Falls back to all rules when
150156
nothing is selected.
151157
"""
152-
if not request.user.has_perm("netbox_interface_name_rules.view_interfacenamerule"):
153-
raise PermissionDenied
154158
pk_list = [int(v) for v in request.POST.getlist("pk") if v.isdigit()]
155159
if pk_list:
156-
queryset = InterfaceNameRule.objects.filter(pk__in=pk_list)
160+
queryset = self.queryset.filter(pk__in=pk_list)
157161
else:
158-
queryset = InterfaceNameRule.objects.all()
162+
queryset = self.queryset
159163
return self._build_response(queryset)
160164

161165

162-
class RuleTestView(ConditionalLoginRequiredMixin, View):
166+
class RuleTestView(BaseMultiObjectView):
163167
"""Live-preview a name template with user-supplied variable values and optional DB lookup."""
164168

169+
queryset = InterfaceNameRule.objects.all()
165170
template_name = "netbox_interface_name_rules/rule_test.html"
166171

167-
def _check_permission(self, request):
168-
if not request.user.has_perm("netbox_interface_name_rules.add_interfacenamerule"):
169-
raise PermissionDenied
172+
def get_required_permission(self):
173+
"""Return the permission required to access the rule test tool."""
174+
return "netbox_interface_name_rules.add_interfacenamerule"
170175

171176
def get(self, request):
172177
"""Render the test form, pre-populated from rule_id query param if given."""
173-
self._check_permission(request)
174178
initial = {}
175179
loaded_rule = None
176180
rule_id = request.GET.get("rule_id")
@@ -196,7 +200,6 @@ def get(self, request):
196200

197201
def post(self, request):
198202
"""Evaluate the submitted template and return a preview or redirect to save."""
199-
self._check_permission(request)
200203
form = RuleTestForm(request.POST)
201204
preview_results = None
202205
db_preview = None
@@ -348,59 +351,61 @@ def _fetch_db_preview(self, cd):
348351
return [], 0, f"Unexpected error: {type(exc).__name__}"
349352

350353

351-
class RuleApplyListView(ConditionalLoginRequiredMixin, View):
354+
class RuleApplyListView(BaseMultiObjectView):
352355
"""Display all rules with buttons to preview/apply each one."""
353356

357+
queryset = InterfaceNameRule.objects.all()
354358
template_name = "netbox_interface_name_rules/rule_apply.html"
355359

360+
def get_required_permission(self):
361+
"""Return the permission required to access the apply-rules page."""
362+
return "netbox_interface_name_rules.view_interfacenamerule"
363+
356364
def get(self, request):
357365
"""Render the list of all rules with apply/preview buttons."""
358-
rules = InterfaceNameRule.objects.select_related(
359-
"module_type", "parent_module_type", "device_type", "platform"
360-
).order_by("pk")
366+
rules = self.queryset.select_related("module_type", "parent_module_type", "device_type", "platform").order_by(
367+
"pk"
368+
)
361369
return render(request, self.template_name, {"rules": rules, "batch_limit": APPLY_BATCH_LIMIT})
362370

363371

364-
class RuleApplicableView(ConditionalLoginRequiredMixin, View):
372+
class RuleApplicableView(generic.ObjectView):
365373
"""Return JSON indicating whether a rule would rename at least one interface.
366374
367375
Called on demand from the Apply Rules page — NOT at page load — to avoid
368376
expensive full-scan queries blocking the initial render.
369377
"""
370378

371-
def get(self, request, pk):
379+
queryset = InterfaceNameRule.objects.all()
380+
381+
def get(self, request, **kwargs):
372382
"""Return JSON {"applicable": bool} for the rule identified by pk."""
373383
from .engine import has_applicable_interfaces
374384

375-
rule = get_object_or_404(InterfaceNameRule, pk=pk)
385+
rule = self.get_object(**kwargs)
376386
try:
377387
applicable = has_applicable_interfaces(rule)
378388
except Exception as exc:
379-
logger.exception("applicability scan failed for rule %s", pk)
380-
# Only expose the exception class name to avoid leaking internals
381-
# (SQL, file paths, etc.). Full details are in the server log.
389+
logger.exception("applicability scan failed for rule %s", kwargs.get("pk"))
382390
return JsonResponse(
383391
{"applicable": None, "error": f"scan failed: {type(exc).__name__}"},
384392
status=500,
385393
)
386394
return JsonResponse({"applicable": applicable})
387395

388396

389-
class RuleApplyDetailView(ConditionalLoginRequiredMixin, View):
397+
class RuleApplyDetailView(generic.ObjectView):
390398
"""Show a preview of changes for a specific rule and allow applying them."""
391399

400+
queryset = InterfaceNameRule.objects.all()
392401
template_name = "netbox_interface_name_rules/rule_apply_detail.html"
402+
additional_permissions = ["dcim.change_interface"]
393403

394-
def _check_permission(self, request):
395-
if not request.user.has_perm("dcim.change_interface"):
396-
raise PermissionDenied
397-
398-
def get(self, request, pk):
404+
def get(self, request, **kwargs):
399405
"""Render a preview of all interfaces that would be renamed by this rule."""
400406
from .engine import find_interfaces_for_rule
401407

402-
self._check_permission(request)
403-
rule = get_object_or_404(InterfaceNameRule, pk=pk)
408+
rule = self.get_object(**kwargs)
404409
try:
405410
preview, total_checked = find_interfaces_for_rule(rule, limit=APPLY_BATCH_LIMIT)
406411
except (re.error, ValueError) as exc:
@@ -424,13 +429,11 @@ def get(self, request, pk):
424429
},
425430
)
426431

427-
def post(self, request, pk):
432+
def post(self, request, **kwargs):
428433
"""Apply the rule (foreground batch or background job) and redirect back."""
429434
from .engine import apply_rule_to_existing
430435

431-
self._check_permission(request)
432-
433-
rule = get_object_or_404(InterfaceNameRule, pk=pk)
436+
rule = self.get_object(**kwargs)
434437
action = request.POST.get("action", "apply")
435438

436439
if action == "background":

0 commit comments

Comments
 (0)