From 3f7f3f88f3ab8409e7b6c290047a718ef7ed5995 Mon Sep 17 00:00:00 2001
From: Jeremy Stretch <stretch@packetlife.net>
Date: Fri, 9 Aug 2019 16:34:01 -0400
Subject: [PATCH] Fix form field ordering

---
 docs/additional-features/custom-scripts.md | 23 ++++++++++++++++--
 netbox/extras/forms.py                     |  2 +-
 netbox/extras/scripts.py                   | 28 ++++++++++++++++++----
 netbox/templates/extras/script.html        |  2 +-
 netbox/templates/extras/script_list.html   |  2 +-
 5 files changed, 48 insertions(+), 9 deletions(-)

diff --git a/docs/additional-features/custom-scripts.md b/docs/additional-features/custom-scripts.md
index c97137a41..b4e5852e0 100644
--- a/docs/additional-features/custom-scripts.md
+++ b/docs/additional-features/custom-scripts.md
@@ -37,6 +37,24 @@ Defining variables is optional: You may create a script with only a `run()` meth
 
 Returning output from your script is optional. Any raw output generated by the script will be displayed under the "output" tab in the UI.
 
+## Script Attributes
+
+### script_name
+
+This is the human-friendly names of your script. If omitted, the class name will be used.
+
+### script_description
+
+A human-friendly description of what your script does (optional).
+
+### script_fields
+
+The order in which the variable fields should appear. This is optional, however on Python 3.5 and earlier the fields will appear in random order. (Declarative ordering is preserved on Python 3.6 and above.) For example:
+
+```
+script_fields = ['var1', 'var2', 'var3']
+```
+
 ## Logging
 
 The Script object provides a set of convenient functions for recording messages at different severity levels:
@@ -106,8 +124,9 @@ from extras.scripts import Script, IntegerVar, ObjectVar, StringVar
 
 
 class NewBranchScript(Script):
-    name = "New Branch"
-    description = "Provision a new branch site"
+    script_name = "New Branch"
+    script_description = "Provision a new branch site"
+    script_fields = ['site_name', 'switch_count', 'switch_model']
 
     site_name = StringVar(
         description="Name of the new site"
diff --git a/netbox/extras/forms.py b/netbox/extras/forms.py
index fad5a7ac2..15c91a880 100644
--- a/netbox/extras/forms.py
+++ b/netbox/extras/forms.py
@@ -393,5 +393,5 @@ class ScriptForm(BootstrapMixin, forms.Form):
         super().__init__(*args, **kwargs)
 
         # Dynamically populate fields for variables
-        for name, var in vars:
+        for name, var in vars.items():
             self.fields[name] = var.as_field()
diff --git a/netbox/extras/scripts.py b/netbox/extras/scripts.py
index a4900b9a2..7ef3dde2f 100644
--- a/netbox/extras/scripts.py
+++ b/netbox/extras/scripts.py
@@ -10,6 +10,15 @@ from .constants import LOG_DEFAULT, LOG_FAILURE, LOG_INFO, LOG_SUCCESS, LOG_WARN
 from .forms import ScriptForm
 
 
+__all__ = [
+    'Script',
+    'StringVar',
+    'IntegerVar',
+    'BooleanVar',
+    'ObjectVar',
+]
+
+
 class OptionalBooleanField(forms.BooleanField):
     required = False
 
@@ -117,13 +126,24 @@ class Script:
         self.source = inspect.getsource(self.__class__)
 
     def __str__(self):
-        if hasattr(self, 'name'):
-            return self.name
+        if hasattr(self, 'script_name'):
+            return self.script_name
         return self.__class__.__name__
 
     def _get_vars(self):
-        # TODO: This should preserve var ordering
-        return inspect.getmembers(self, is_variable)
+        vars = OrderedDict()
+
+        # Infer order from script_fields (Python 3.5 and lower)
+        if hasattr(self, 'script_fields'):
+            for name in self.script_fields:
+                vars[name] = getattr(self, name)
+
+        # Default to order of declaration on class
+        for name, attr in self.__class__.__dict__.items():
+            if name not in vars and issubclass(attr.__class__, ScriptVariable):
+                vars[name] = attr
+
+        return vars
 
     def run(self, data):
         raise NotImplementedError("The script must define a run() method.")
diff --git a/netbox/templates/extras/script.html b/netbox/templates/extras/script.html
index bbd949098..66beeb852 100644
--- a/netbox/templates/extras/script.html
+++ b/netbox/templates/extras/script.html
@@ -16,7 +16,7 @@
         </div>
     </div>
     <h1>{{ script }}</h1>
-    <p>{{ script.description }}</p>
+    <p>{{ script.script_description }}</p>
     <ul class="nav nav-tabs" role="tablist">
         <li role="presentation" class="active">
             <a href="#run" role="tab" data-toggle="tab" class="active">Run</a>
diff --git a/netbox/templates/extras/script_list.html b/netbox/templates/extras/script_list.html
index 0189ef755..fcda2a5ae 100644
--- a/netbox/templates/extras/script_list.html
+++ b/netbox/templates/extras/script_list.html
@@ -22,7 +22,7 @@
                                     <td>
                                         <a href="{% url 'extras:script' module=module name=class_name %}" name="script.{{ class_name }}"><strong>{{ script }}</strong></a>
                                     </td>
-                                    <td>{{ script.description }}</td>
+                                    <td>{{ script.script_description }}</td>
                                     <td></td>
                                 </tr>
                             {% endfor %}