14438 review changes

netbox/extras/api/serializers.py

@@ -83,11 +83,8 @@ class EventRuleSerializer(NetBoxModelSerializer):
         # We need to manually instantiate the serializer for scripts
         if instance.action_type == EventRuleActionChoices.SCRIPT:
             script = instance.action_object
-            script_name = script.name
+            instance = script.python_class() if script.python_class else None
-            if script.python_class:
+            return NestedScriptSerializer(instance, context=context).data
-                return NestedScriptSerializer(script.python_class(), context=context).data
-            else:
-                return NestedScriptSerializer(None, context=context).data
         else:
             serializer = get_serializer_for_model(
                 model=instance.action_object_type.model_class(),
@@ -525,7 +522,7 @@ class ScriptSerializer(ValidatedModelSerializer):
     class Meta:
         model = Script
         fields = [
-            'id', 'url', 'module', 'name', 'description', 'vars', 'result', 'display', 'is_valid',
+            'id', 'url', 'module', 'name', 'description', 'vars', 'result', 'display', 'is_executable',
         ]
 
     @extend_schema_field(serializers.JSONField(allow_null=True))

netbox/extras/forms/model_forms.py

@@ -300,7 +300,7 @@ class EventRuleForm(NetBoxModelForm):
         choices = []
         for module in ScriptModule.objects.all():
             script_list = []
-            for script in module.scripts.all():
+            for script in module.scripts.all().prefetch_related('scripts'):
                 name = f"{str(script.pk)}:{script.name}"
                 script_list.append((name, script.name))
             if script_list:

netbox/extras/migrations/0108_script_models.py

@@ -57,16 +57,12 @@ class Migration(migrations.Migration):
                 ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False)),
                 ('name', models.CharField(max_length=79)),
                 ('module', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='scripts', to='extras.scriptmodule')),
+                ('is_executable', models.BooleanField(default=True))
             ],
             options={
-                'ordering': ('name', 'pk'),
+                'ordering': ('module', 'name'),
             },
         ),
-        migrations.AddField(
-            model_name='script',
-            name='is_valid',
-            field=models.BooleanField(default=True),
-        ),
         migrations.AddConstraint(
             model_name='script',
             constraint=models.UniqueConstraint(fields=('name', 'module'), name='extras_script_unique_name_module'),

netbox/extras/models/scripts.py

@@ -33,8 +33,9 @@ class Script(EventRulesMixin, JobsMixin, models.Model):
         on_delete=models.CASCADE,
         related_name='scripts'
     )
-    is_valid = models.BooleanField(
+    is_executable = models.BooleanField(
-        default=True
+        default=True,
+        verbose_name=_('is executable')
     )
     events = GenericRelation(
         'extras.EventRule',
@@ -46,7 +47,7 @@ class Script(EventRulesMixin, JobsMixin, models.Model):
         return self.name
 
     class Meta:
-        ordering = ('name', 'pk')
+        ordering = ('module', 'name')
         constraints = (
             models.UniqueConstraint(
                 fields=('name', 'module'),

netbox/extras/tests/test_api.py

@@ -774,7 +774,7 @@ class ScriptTest(APITestCase):
         Script.objects.create(
             module=module,
             name="Test script",
-            is_valid=True,
+            is_executable=True,
         )
 
     def python_class(self):

netbox/templates/extras/script/base.html

@@ -26,10 +26,10 @@
 {% block tabs %}
   <ul class="nav nav-tabs">
     <li class="nav-item" role="presentation">
-      <a class="nav-link{% if not tab %} active{% endif %}{% if not script.is_valid %} disabled{% endif %}" href="{% url 'extras:script' script.id %}">{% trans "Script" %}</a>
+      <a class="nav-link{% if not tab %} active{% endif %}{% if not script.is_executable %} disabled{% endif %}" href="{{ script.get_absolute_url }}">{% trans "Script" %}</a>
     </li>
     <li class="nav-item" role="presentation">
-      <a class="nav-link{% if tab == 'source' %} active{% endif %}{% if not script.is_valid %} disabled{% endif %}" href="{% url 'extras:script_source' script.id %}">{% trans "Source" %}</a>
+      <a class="nav-link{% if tab == 'source' %} active{% endif %}{% if not script.is_executable %} disabled{% endif %}" href="{% url 'extras:script_source' script.id %}">{% trans "Source" %}</a>
     </li>
     <li class="nav-item" role="presentation">
       <a class="nav-link{% if tab == 'jobs' %} active{% endif %}" href="{% url 'extras:script_jobs' script.id %}">

netbox/templates/extras/script/jobs.html

@@ -8,7 +8,7 @@
       <div class="card">
         <div class="card-body table-responsive">
 
-          {% if not script.is_valid %}
+          {% if not script.is_executable %}
             <div class="alert alert-warning" role="alert">
               <div class="d-flex justify-content-between">
                 <div>

netbox/templates/extras/script_list.html

@@ -48,7 +48,7 @@
               {% with last_job=script.get_latest_jobs|get_key:script.name %}
                 <tr>
                   <td>
-                    {% if script.is_valid %}
+                    {% if script.is_executable %}
                       <a href="{% url 'extras:script' script.pk %}" id="{{ script.module }}.{{ script.class_name }}">{{ script.name }}</a>
                     {% else %}
                       <a href="{% url 'extras:script_jobs' script.pk %}" id="{{ script.module }}.{{ script.class_name }}">{{ script.name }}</a>

netbox/templates/extras/script_result.html

@@ -16,7 +16,7 @@
       <ol class="breadcrumb">
         <li class="breadcrumb-item"><a href="{% url 'extras:script_list' %}">{% trans "Scripts" %}</a></li>
         <li class="breadcrumb-item"><a href="{% url 'extras:script_list' %}#module.{{ script.module }}">{{ script.module|bettertitle }}</a></li>
-        <li class="breadcrumb-item"><a href="{% url 'extras:script' script.id %}">{{ script }}</a></li>
+        <li class="breadcrumb-item"><a href="{{ script.get_absolute_url }}">{{ script }}</a></li>
         <li class="breadcrumb-item">{{ job.created|annotated_date }}</li>
       </ol>
     </nav>