From 2fdd46f64cdd7791169990918d662ec9a77cddb3 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Tue, 21 Oct 2025 11:16:56 -0500 Subject: [PATCH 1/2] Fixes #19872: Display form validation errors for script execution When script form validation fails (e.g., required fields excluded from fieldsets), display error messages via Django's message framework instead of failing silently. Error format: "field: error1, error2; field2: error". --- netbox/extras/tests/test_views.py | 45 +++++++++++++++++++++++++++++++ netbox/extras/views.py | 5 ++++ 2 files changed, 50 insertions(+) diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index 9da6f047a..145606393 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -1,11 +1,14 @@ from django.contrib.contenttypes.models import ContentType from django.urls import reverse +from django.test import tag +from core.choices import ManagedFileRootPathChoices from core.events import * from core.models import ObjectType from dcim.models import DeviceType, Manufacturer, Site from extras.choices import * from extras.models import * +from extras.scripts import Script as PythonClass, IntegerVar, BooleanVar from users.models import Group, User from utilities.testing import ViewTestCases, TestCase @@ -897,3 +900,45 @@ class ScriptListViewTest(TestCase): response = self.client.get(url, {'embedded': 'true'}) self.assertEqual(response.status_code, 200) self.assertTemplateUsed(response, 'extras/inc/script_list_content.html') + + +class ScriptValidationErrorTest(TestCase): + user_permissions = ['extras.view_script', 'extras.run_script'] + + class TestScriptMixin: + bar = IntegerVar(min_value=0, max_value=30, default=30) + + class TestScriptClass(TestScriptMixin, PythonClass): + class Meta: + name = 'Test script' + commit_default = False + fieldsets = (("Logging", ("debug_mode",)),) + + debug_mode = BooleanVar(default=False) + + def run(self, data, commit): + return "Complete" + + @classmethod + def setUpTestData(cls): + module = ScriptModule.objects.create(file_root=ManagedFileRootPathChoices.SCRIPTS, file_path='test_script.py') + cls.script = Script.objects.create(module=module, name='Test script', is_executable=True) + + def setUp(self): + super().setUp() + Script.python_class = property(lambda self: ScriptValidationErrorTest.TestScriptClass) + + @tag('regression') + def test_script_validation_error_displays_message(self): + """Test that form validation errors are displayed to the user""" + from unittest.mock import patch + + url = reverse('extras:script', kwargs={'pk': self.script.pk}) + + with patch('extras.views.get_workers_for_queue', return_value=['worker']): + response = self.client.post(url, {'debug_mode': 'true', '_commit': 'true'}) + + self.assertEqual(response.status_code, 200) + messages = list(response.context['messages']) + self.assertEqual(len(messages), 1) + self.assertEqual(str(messages[0]), "bar: This field is required.") diff --git a/netbox/extras/views.py b/netbox/extras/views.py index 32d19674b..b3eb435d6 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -1485,6 +1485,11 @@ class ScriptView(BaseScriptView): ) return redirect('extras:script_result', job_pk=job.pk) + else: + messages.error( + request, + '; '.join(f"{field}: {', '.join(errors)}" for field, errors in form.errors.items()) + ) return render(request, 'extras/script.html', { 'object': script, From 5fbae8407e408da2e7ed7074e87b0424e4969284 Mon Sep 17 00:00:00 2001 From: Jason Novinger Date: Tue, 21 Oct 2025 11:54:46 -0500 Subject: [PATCH 2/2] Only show non-rendered field errors in toast When script form validation fails, display error messages for fields not in fieldsets. Fields in fieldsets show inline errors only; hidden fields show toast notifications to provide feedback instead of failing silently. --- netbox/extras/tests/test_views.py | 27 ++++++++++++++++++++++++++- netbox/extras/views.py | 12 ++++++++---- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/netbox/extras/tests/test_views.py b/netbox/extras/tests/test_views.py index 145606393..91444e2ce 100644 --- a/netbox/extras/tests/test_views.py +++ b/netbox/extras/tests/test_views.py @@ -930,7 +930,6 @@ class ScriptValidationErrorTest(TestCase): @tag('regression') def test_script_validation_error_displays_message(self): - """Test that form validation errors are displayed to the user""" from unittest.mock import patch url = reverse('extras:script', kwargs={'pk': self.script.pk}) @@ -942,3 +941,29 @@ class ScriptValidationErrorTest(TestCase): messages = list(response.context['messages']) self.assertEqual(len(messages), 1) self.assertEqual(str(messages[0]), "bar: This field is required.") + + @tag('regression') + def test_script_validation_error_no_toast_for_fieldset_fields(self): + from unittest.mock import patch, PropertyMock + + class FieldsetScript(PythonClass): + class Meta: + name = 'Fieldset test' + commit_default = False + fieldsets = (("Fields", ("required_field",)),) + + required_field = IntegerVar(min_value=10) + + def run(self, data, commit): + return "Complete" + + url = reverse('extras:script', kwargs={'pk': self.script.pk}) + + with patch.object(Script, 'python_class', new_callable=PropertyMock) as mock_python_class: + mock_python_class.return_value = FieldsetScript + with patch('extras.views.get_workers_for_queue', return_value=['worker']): + response = self.client.post(url, {'required_field': '5', '_commit': 'true'}) + + self.assertEqual(response.status_code, 200) + messages = list(response.context['messages']) + self.assertEqual(len(messages), 0) diff --git a/netbox/extras/views.py b/netbox/extras/views.py index b3eb435d6..32f87fb97 100644 --- a/netbox/extras/views.py +++ b/netbox/extras/views.py @@ -1486,10 +1486,14 @@ class ScriptView(BaseScriptView): return redirect('extras:script_result', job_pk=job.pk) else: - messages.error( - request, - '; '.join(f"{field}: {', '.join(errors)}" for field, errors in form.errors.items()) - ) + fieldset_fields = {field for _, fields in script_class.get_fieldsets() for field in fields} + hidden_errors = { + field: errors for field, errors in form.errors.items() + if field not in fieldset_fields + } + if hidden_errors: + error_msg = '; '.join(f"{field}: {', '.join(errors)}" for field, errors in hidden_errors.items()) + messages.error(request, error_msg) return render(request, 'extras/script.html', { 'object': script,