From d0bd1ad25b168555725b1f53b4bace0a09cbe413 Mon Sep 17 00:00:00 2001 From: dansheps Date: Mon, 24 Feb 2020 10:18:19 -0600 Subject: [PATCH 1/6] Fixes: #4255 - Add new script variable types based on dynamic model fields --- netbox/extras/scripts.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index 57cbc8149c4..68a1a5ba4df 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -18,6 +18,7 @@ from ipam.validators import MaxPrefixLengthValidator, MinPrefixLengthValidator, prefix_validator from .constants import LOG_DEFAULT, LOG_FAILURE, LOG_INFO, LOG_SUCCESS, LOG_WARNING from utilities.exceptions import AbortTransaction +from utilities.forms import DynamicModelChoiceField, DynamicModelMultipleChoiceField from .forms import ScriptForm from .signals import purge_changelog @@ -197,6 +198,20 @@ def __init__(self, queryset, *args, **kwargs): self.form_field = TreeNodeMultipleChoiceField +class DynamicObjectVar(ObjectVar): + """ + A dynamic netbox object variable. APISelect will determine the available choices + """ + form_field = DynamicModelChoiceField + + +class DynamicMultiObjectVar(MultiObjectVar): + """ + A multiple choice version of DynamicObjectVar + """ + form_field = DynamicModelMultipleChoiceField + + class FileVar(ScriptVariable): """ An uploaded file. From a5853427d44aa56abb93c74401753a787d59a467 Mon Sep 17 00:00:00 2001 From: dansheps Date: Mon, 24 Feb 2020 10:21:17 -0600 Subject: [PATCH 2/6] Update __all__ for #4255 --- netbox/extras/scripts.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index 68a1a5ba4df..eb796cf322b 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -26,6 +26,8 @@ 'BaseScript', 'BooleanVar', 'ChoiceVar', + 'DynamicObjectVar', + 'DynamicMultiObjectVar', 'FileVar', 'IntegerVar', 'IPAddressVar', From 8ed0d0400f5dcd5c784fac1fb41b1ec2a0245faf Mon Sep 17 00:00:00 2001 From: dansheps Date: Mon, 24 Feb 2020 10:29:07 -0600 Subject: [PATCH 3/6] Add tests --- netbox/extras/tests/test_scripts.py | 50 +++++++++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/netbox/extras/tests/test_scripts.py b/netbox/extras/tests/test_scripts.py index 6237d1d952a..c926a0b2903 100644 --- a/netbox/extras/tests/test_scripts.py +++ b/netbox/extras/tests/test_scripts.py @@ -145,6 +145,30 @@ class TestScript(Script): self.assertTrue(form.is_valid()) self.assertEqual(form.cleaned_data['var1'].pk, data['var1']) + def test_dynamicobjectvar(self): + """ + Test dynamic version of the objectvar + """ + + class TestScript(Script): + + var1 = DynamicObjectVar( + queryset=DeviceRole.objects.all() + ) + + # Populate some objects + for i in range(1, 6): + DeviceRole( + name='Device Role {}'.format(i), + slug='device-role-{}'.format(i) + ).save() + + # Validate valid data + data = {'var1': DeviceRole.objects.first().pk} + form = TestScript().as_form(data, None) + self.assertTrue(form.is_valid()) + self.assertEqual(form.cleaned_data['var1'].pk, data['var1']) + def test_multiobjectvar(self): class TestScript(Script): @@ -168,6 +192,32 @@ class TestScript(Script): self.assertEqual(form.cleaned_data['var1'][1].pk, data['var1'][1]) self.assertEqual(form.cleaned_data['var1'][2].pk, data['var1'][2]) + def test_dynamicmultiobjectvar(self): + """ + Test dynamic version of the multiobjectvar + """ + + class TestScript(Script): + + var1 = DynamicMultiObjectVar( + queryset=DeviceRole.objects.all() + ) + + # Populate some objects + for i in range(1, 6): + DeviceRole( + name='Device Role {}'.format(i), + slug='device-role-{}'.format(i) + ).save() + + # Validate valid data + data = {'var1': [role.pk for role in DeviceRole.objects.all()[:3]]} + form = TestScript().as_form(data, None) + self.assertTrue(form.is_valid()) + self.assertEqual(form.cleaned_data['var1'][0].pk, data['var1'][0]) + self.assertEqual(form.cleaned_data['var1'][1].pk, data['var1'][1]) + self.assertEqual(form.cleaned_data['var1'][2].pk, data['var1'][2]) + def test_filevar(self): class TestScript(Script): From 27e3b6f377afb8c88e2e79ca805c335d15cff11a Mon Sep 17 00:00:00 2001 From: dansheps Date: Thu, 27 Feb 2020 07:45:11 -0600 Subject: [PATCH 4/6] Remove second variables, make widget mandatory on ObjectVar and MultiObjectVar --- netbox/extras/scripts.py | 28 +++----------- netbox/extras/tests/test_scripts.py | 57 +++-------------------------- 2 files changed, 11 insertions(+), 74 deletions(-) diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index eb796cf322b..fdde58a834c 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -26,8 +26,6 @@ 'BaseScript', 'BooleanVar', 'ChoiceVar', - 'DynamicObjectVar', - 'DynamicMultiObjectVar', 'FileVar', 'IntegerVar', 'IPAddressVar', @@ -170,10 +168,10 @@ class ObjectVar(ScriptVariable): """ NetBox object representation. The provided QuerySet will determine the choices available. """ - form_field = forms.ModelChoiceField + form_field = DynamicModelChoiceField - def __init__(self, queryset, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, queryset, widget, *args, **kwargs): + super().__init__(widget=widget, *args, **kwargs) # Queryset for field choices self.field_attrs['queryset'] = queryset @@ -187,10 +185,10 @@ class MultiObjectVar(ScriptVariable): """ Like ObjectVar, but can represent one or more objects. """ - form_field = forms.ModelMultipleChoiceField + form_field = DynamicModelMultipleChoiceField - def __init__(self, queryset, *args, **kwargs): - super().__init__(*args, **kwargs) + def __init__(self, queryset, widget, *args, **kwargs): + super().__init__(widget=widget, *args, **kwargs) # Queryset for field choices self.field_attrs['queryset'] = queryset @@ -200,20 +198,6 @@ def __init__(self, queryset, *args, **kwargs): self.form_field = TreeNodeMultipleChoiceField -class DynamicObjectVar(ObjectVar): - """ - A dynamic netbox object variable. APISelect will determine the available choices - """ - form_field = DynamicModelChoiceField - - -class DynamicMultiObjectVar(MultiObjectVar): - """ - A multiple choice version of DynamicObjectVar - """ - form_field = DynamicModelMultipleChoiceField - - class FileVar(ScriptVariable): """ An uploaded file. diff --git a/netbox/extras/tests/test_scripts.py b/netbox/extras/tests/test_scripts.py index c926a0b2903..2d4d5b3fd8a 100644 --- a/netbox/extras/tests/test_scripts.py +++ b/netbox/extras/tests/test_scripts.py @@ -4,6 +4,7 @@ from dcim.models import DeviceRole from extras.scripts import * +from utilities.forms import APISelect, APISelectMultiple class ScriptVariablesTest(TestCase): @@ -129,31 +130,8 @@ def test_objectvar(self): class TestScript(Script): var1 = ObjectVar( - queryset=DeviceRole.objects.all() - ) - - # Populate some objects - for i in range(1, 6): - DeviceRole( - name='Device Role {}'.format(i), - slug='device-role-{}'.format(i) - ).save() - - # Validate valid data - data = {'var1': DeviceRole.objects.first().pk} - form = TestScript().as_form(data, None) - self.assertTrue(form.is_valid()) - self.assertEqual(form.cleaned_data['var1'].pk, data['var1']) - - def test_dynamicobjectvar(self): - """ - Test dynamic version of the objectvar - """ - - class TestScript(Script): - - var1 = DynamicObjectVar( - queryset=DeviceRole.objects.all() + queryset=DeviceRole.objects.all(), + widget=APISelect(api_url='/api/dcim/device-roles/') ) # Populate some objects @@ -174,33 +152,8 @@ def test_multiobjectvar(self): class TestScript(Script): var1 = MultiObjectVar( - queryset=DeviceRole.objects.all() - ) - - # Populate some objects - for i in range(1, 6): - DeviceRole( - name='Device Role {}'.format(i), - slug='device-role-{}'.format(i) - ).save() - - # Validate valid data - data = {'var1': [role.pk for role in DeviceRole.objects.all()[:3]]} - form = TestScript().as_form(data, None) - self.assertTrue(form.is_valid()) - self.assertEqual(form.cleaned_data['var1'][0].pk, data['var1'][0]) - self.assertEqual(form.cleaned_data['var1'][1].pk, data['var1'][1]) - self.assertEqual(form.cleaned_data['var1'][2].pk, data['var1'][2]) - - def test_dynamicmultiobjectvar(self): - """ - Test dynamic version of the multiobjectvar - """ - - class TestScript(Script): - - var1 = DynamicMultiObjectVar( - queryset=DeviceRole.objects.all() + queryset=DeviceRole.objects.all(), + widget=APISelectMultiple(api_url='/api/dcim/device-roles/') ) # Populate some objects From 0995e10d8720efe1af7c08cfd189ff6fafc47416 Mon Sep 17 00:00:00 2001 From: dansheps Date: Thu, 19 Mar 2020 08:09:31 -0500 Subject: [PATCH 5/6] Modify script ObjectVars to use DynamicModelChoiceFields --- netbox/extras/scripts.py | 8 ++++---- netbox/extras/tests/test_scripts.py | 6 ++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py index fdde58a834c..55d53700f74 100644 --- a/netbox/extras/scripts.py +++ b/netbox/extras/scripts.py @@ -170,8 +170,8 @@ class ObjectVar(ScriptVariable): """ form_field = DynamicModelChoiceField - def __init__(self, queryset, widget, *args, **kwargs): - super().__init__(widget=widget, *args, **kwargs) + def __init__(self, queryset, *args, **kwargs): + super().__init__(*args, **kwargs) # Queryset for field choices self.field_attrs['queryset'] = queryset @@ -187,8 +187,8 @@ class MultiObjectVar(ScriptVariable): """ form_field = DynamicModelMultipleChoiceField - def __init__(self, queryset, widget, *args, **kwargs): - super().__init__(widget=widget, *args, **kwargs) + def __init__(self, queryset, *args, **kwargs): + super().__init__(*args, **kwargs) # Queryset for field choices self.field_attrs['queryset'] = queryset diff --git a/netbox/extras/tests/test_scripts.py b/netbox/extras/tests/test_scripts.py index 2d4d5b3fd8a..ea2888bd8ff 100644 --- a/netbox/extras/tests/test_scripts.py +++ b/netbox/extras/tests/test_scripts.py @@ -130,8 +130,7 @@ def test_objectvar(self): class TestScript(Script): var1 = ObjectVar( - queryset=DeviceRole.objects.all(), - widget=APISelect(api_url='/api/dcim/device-roles/') + queryset=DeviceRole.objects.all() ) # Populate some objects @@ -152,8 +151,7 @@ def test_multiobjectvar(self): class TestScript(Script): var1 = MultiObjectVar( - queryset=DeviceRole.objects.all(), - widget=APISelectMultiple(api_url='/api/dcim/device-roles/') + queryset=DeviceRole.objects.all() ) # Populate some objects From fa1548f3cea7bca17ad3a82f79ee37ae93a8b3fb Mon Sep 17 00:00:00 2001 From: dansheps Date: Thu, 19 Mar 2020 08:11:14 -0500 Subject: [PATCH 6/6] Remove extraneous import --- netbox/extras/tests/test_scripts.py | 1 - 1 file changed, 1 deletion(-) diff --git a/netbox/extras/tests/test_scripts.py b/netbox/extras/tests/test_scripts.py index ea2888bd8ff..6237d1d952a 100644 --- a/netbox/extras/tests/test_scripts.py +++ b/netbox/extras/tests/test_scripts.py @@ -4,7 +4,6 @@ from dcim.models import DeviceRole from extras.scripts import * -from utilities.forms import APISelect, APISelectMultiple class ScriptVariablesTest(TestCase):