From 80802a4357b89d65df0faf7ae7ca2024e1cbad88 Mon Sep 17 00:00:00 2001 From: Arthur Date: Thu, 14 Mar 2024 15:06:26 -0700 Subject: [PATCH] 15353 add better script error message --- netbox/extras/models/scripts.py | 2 + netbox/extras/views.py | 33 +++++++++-- netbox/templates/extras/script.html | 65 ++++++++++++---------- netbox/templates/extras/script/source.html | 12 +++- 4 files changed, 74 insertions(+), 38 deletions(-) diff --git a/netbox/extras/models/scripts.py b/netbox/extras/models/scripts.py index e857e59b7..7e24b7353 100644 --- a/netbox/extras/models/scripts.py +++ b/netbox/extras/models/scripts.py @@ -96,6 +96,7 @@ class ScriptModule(PythonModuleMixin, JobsMixin, ManagedFile): Proxy model for script module files. """ objects = ScriptModuleManager() + error = None class Meta: proxy = True @@ -118,6 +119,7 @@ class ScriptModule(PythonModuleMixin, JobsMixin, ManagedFile): try: module = self.get_module() except Exception as e: + self.error = e logger.debug(f"Failed to load script: {self.python_name} error: {e}") module = None diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 1fa2a30aa..3195fabd4 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -1040,12 +1040,27 @@ class ScriptListView(ContentTypePermissionRequiredMixin, View): }) -class ScriptView(generic.ObjectView): +class BaseScriptView(generic.ObjectView): queryset = Script.objects.all() + def _get_script_class(self, script): + script_class = script.python_class + if script_class: + script_class = script_class() + + return script_class + + +class ScriptView(BaseScriptView): + def get(self, request, **kwargs): script = self.get_object(**kwargs) - script_class = script.python_class() + script_class = self._get_script_class(script) + if not script_class: + return render(request, 'extras/script.html', { + 'script': script, + }) + form = script_class.as_form(initial=normalize_querydict(request.GET)) return render(request, 'extras/script.html', { @@ -1057,11 +1072,16 @@ class ScriptView(generic.ObjectView): def post(self, request, **kwargs): script = self.get_object(**kwargs) - script_class = script.python_class() if not request.user.has_perm('extras.run_script', obj=script): return HttpResponseForbidden() + script_class = self._get_script_class(script) + if not script_class: + return render(request, 'extras/script.html', { + 'script': script, + }) + form = script_class.as_form(request.POST, request.FILES) # Allow execution only if RQ worker process is running @@ -1091,21 +1111,22 @@ class ScriptView(generic.ObjectView): }) -class ScriptSourceView(generic.ObjectView): +class ScriptSourceView(BaseScriptView): queryset = Script.objects.all() def get(self, request, **kwargs): script = self.get_object(**kwargs) + script_class = self._get_script_class(script) return render(request, 'extras/script/source.html', { 'script': script, - 'script_class': script.python_class(), + 'script_class': script_class, 'job_count': script.jobs.count(), 'tab': 'source', }) -class ScriptJobsView(generic.ObjectView): +class ScriptJobsView(BaseScriptView): queryset = Script.objects.all() def get(self, request, **kwargs): diff --git a/netbox/templates/extras/script.html b/netbox/templates/extras/script.html index 8a4f86579..fe616dc8a 100644 --- a/netbox/templates/extras/script.html +++ b/netbox/templates/extras/script.html @@ -14,38 +14,43 @@ {% trans "You do not have permission to run scripts" %}. {% endif %} -
- {% csrf_token %} -
- {# Render grouped fields according to declared fieldsets #} - {% for group, fields in script_class.get_fieldsets %} - {% if fields %} -
-
-
{{ group }}
+ {% if form %} + + {% csrf_token %} +
+ {# Render grouped fields according to declared fieldsets #} + {% for group, fields in script_class.get_fieldsets %} + {% if fields %} +
+
+
{{ group }}
+
+ {% for name in fields %} + {% with field=form|getfield:name %} + {% render_field field %} + {% endwith %} + {% endfor %}
- {% for name in fields %} - {% with field=form|getfield:name %} - {% render_field field %} - {% endwith %} - {% endfor %} -
+ {% endif %} + {% endfor %} +
+
+ {% trans "Cancel" %} + {% if not request.user|can_run:script or not script.is_executable %} + + {% else %} + {% endif %} - {% endfor %} -
-
- {% trans "Cancel" %} - {% if not request.user|can_run:script or not script.is_executable %} - - {% else %} - - {% endif %} -
- +
+ + {% else %} +

{% trans "Error loading script" %}.

+
{{ script.module.error }}
+ {% endif %}
{% endblock content %} diff --git a/netbox/templates/extras/script/source.html b/netbox/templates/extras/script/source.html index 96362c7ef..ffb6694c5 100644 --- a/netbox/templates/extras/script/source.html +++ b/netbox/templates/extras/script/source.html @@ -1,6 +1,14 @@ {% extends 'extras/script/base.html' %} +{% load i18n %} {% block content %} - {{ script_class.filename }} -
{{ script_class.source }}
+ + {% if script_class %} + {{ script_class.filename }} +
{{ script_class.source }}
+ {% else %} +

{% trans "Error loading script" %}.

+
{{ script.module.error }}
+ {% endif %} + {% endblock %}