Updates to tests and code

nautobot_design_builder/contrib/ext.py

@@ -322,7 +322,7 @@ class NextPrefixExtension(AttributeExtension):
 
     tag = "next_prefix"
 
-    def attribute(self, attr="prefix", value: dict = None, model_instance: ModelInstance = None) -> None:
+    def attribute(self, *args, value: dict = None, model_instance: ModelInstance = None) -> None:
         """Provides the `!next_prefix` attribute that will calculate the next available prefix.
 
         Args:
@@ -393,6 +393,7 @@ def attribute(self, attr="prefix", value: dict = None, model_instance: ModelInst
             query = Q(**value) & reduce(operator.or_, prefix_q)
 
         prefixes = Prefix.objects.filter(query)
+        attr = args[0] if args else "prefix"
         return attr, self._get_next(prefixes, length)
 
     @staticmethod
@@ -419,7 +420,7 @@ class ChildPrefixExtension(AttributeExtension):
 
     tag = "child_prefix"
 
-    def attribute(self, attr: str = "prefix", value: dict = None, model_instance=None) -> None:
+    def attribute(self, *args, value: dict = None, model_instance=None) -> None:
         """Provides the `!child_prefix` attribute.
 
         !child_prefix calculates a child prefix using a parent prefix
@@ -476,7 +477,7 @@ def attribute(self, attr: str = "prefix", value: dict = None, model_instance=Non
             raise DesignImplementationError("the child_prefix tag requires an offset")
         if not isinstance(offset, str):
             raise DesignImplementationError("offset must be string")
-
+        attr = args[0] if args else "prefix"
         return attr, network_offset(parent, offset)
 
 

nautobot_design_builder/design_job.py

@@ -299,10 +299,11 @@ def _run_in_transaction(self, **kwargs):  # pylint: disable=too-many-branches, t
             if previous_journal:
                 deleted_object_ids = previous_journal - journal
                 if deleted_object_ids:
-                    journal.design_instance.decommission(*deleted_object_ids, local_logger=self.logger)
-                    self.post_implementation(context, self.environment)
+                    self.log_info(f"Decommissioning {deleted_object_ids}")
+                    journal.design_instance.decommission(*deleted_object_ids, local_logger=self.environment.logger)
 
             if commit:
+                self.post_implementation(context, self.environment)
                 # The Journal stores the design (with Nautobot identifiers from post_implementation)
                 # for future operations (e.g., updates)
                 journal.design_instance.status = Status.objects.get(

nautobot_design_builder/migrations/0007_auto_20240430_1235.py

@@ -0,0 +1,27 @@
+# Generated by Django 3.2.20 on 2024-04-30 12:35
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("nautobot_design_builder", "0006_alter_designinstance_status"),
+    ]
+
+    operations = [
+        migrations.AlterModelOptions(
+            name="journal",
+            options={"ordering": ["-last_updated"]},
+        ),
+        migrations.RemoveField(
+            model_name="journal",
+            name="builder_output",
+        ),
+        migrations.AddField(
+            model_name="journalentry",
+            name="index",
+            field=models.IntegerField(default=0),
+            preserve_default=False,
+        ),
+    ]

nautobot_design_builder/models.py

@@ -304,9 +304,13 @@ class Journal(PrimaryModel):
         related_name="journals",
     )
     job_result = models.ForeignKey(to=JobResult, on_delete=models.PROTECT, editable=False)
-    builder_output = models.JSONField(encoder=NautobotKombuJSONEncoder, editable=False, null=True, blank=True)
     active = models.BooleanField(editable=False, default=True)
 
+    class Meta:
+        """Set the default query ordering."""
+
+        ordering = ["-last_updated"]
+
     def get_absolute_url(self):
         """Return detail view for design instances."""
         return reverse("plugins:nautobot_design_builder:journal", args=[self.pk])
@@ -327,6 +331,18 @@ def user_input(self):
         job = self.design_instance.design.job
         return job.job_class.deserialize_data(user_input)
 
+    def _next_index(self):
+        # The hokey getting/setting here is to make pylint happy
+        # and not complain about `no-member`
+        index = getattr(self, "_index", None)
+        if index is None:
+            index = self.entries.aggregate(index=models.Max("index"))["index"]
+            if index is None:
+                index = -1
+        index += 1
+        setattr(self, "_index", index)
+        return index
+
     def log(self, model_instance):
         """Log changes to a model instance.
 
@@ -357,6 +373,7 @@ def log(self, model_instance):
                 _design_object_id=instance.id,
                 changes=model_instance.get_changes(),
                 full_control=model_instance.metadata.created,
+                index=self._next_index(),
             )
         return entry
 
@@ -372,7 +389,7 @@ def revert(self, *object_ids, local_logger: logging.Logger = logger):
         # Without a design object we cannot have changes, right? I suppose if the
         # object has been deleted since the change was made then it wouldn't exist,
         # but I think we need to discuss the implications of this further.
-        entries = self.entries.order_by("-last_updated").exclude(_design_object_id=None).exclude(active=False)
+        entries = self.entries.order_by("-index").exclude(_design_object_id=None).exclude(active=False)
         if not object_ids:
             local_logger.info("Reverting journal", extra={"obj": self})
         else:
@@ -410,7 +427,7 @@ def __sub__(self, other: "Journal"):
         other_ids = other.entries.values_list("_design_object_id")
 
         return (
-            self.entries.order_by("-last_updated")
+            self.entries.order_by("-index")
             .exclude(_design_object_id__in=other_ids)
             .values_list("_design_object_id", flat=True)
         )
@@ -423,16 +440,20 @@ def exclude_decommissioned(self):
         """Returns JournalEntry which the related DesignInstance is not decommissioned."""
         return self.exclude(journal__design_instance__status__name=choices.DesignInstanceStatusChoices.DECOMMISSIONED)
 
-    def filter_related(self, entry: "JournalEntry"):
-        """Returns JournalEntries which have the same object ID but excluding itself."""
-        return self.filter(_design_object_id=entry._design_object_id).exclude(  # pylint: disable=protected-access
-            id=entry.id
-        )
+    def filter_related(self, entry):
+        """Returns other JournalEntries which have the same object ID but are in different designs.
+
+        Args:
+            entry (JournalEntry): The JournalEntry to use as reference.
 
-    def filter_same_parent_design_instance(self, entry: "JournalEntry"):
-        """Returns JournalEntries which have the same parent design instance."""
-        return self.filter(_design_object_id=entry._design_object_id).exclude(  # pylint: disable=protected-access
-            journal__design_instance__id=entry.journal.design_instance.id
+        Returns:
+            QuerySet: The queryset that matches other journal entries with the same design object ID. This
+            excludes matching entries in the same design.
+        """
+        return (
+            self.filter(active=True)
+            .filter(_design_object_id=entry._design_object_id)  # pylint:disable=protected-access
+            .exclude(journal__design_instance_id=entry.journal.design_instance_id)
         )
 
     def filter_by_instance(self, design_instance: "DesignInstance", model=None):
@@ -474,6 +495,8 @@ class JournalEntry(BaseModel):
         related_name="entries",
     )
 
+    index = models.IntegerField(null=False, blank=False)
+
     _design_object_type = models.ForeignKey(
         to=ContentType,
         on_delete=models.PROTECT,
@@ -537,14 +560,9 @@ def revert(self, local_logger: logging.Logger = logger):  # pylint: disable=too-
         local_logger.info("Reverting journal entry", extra={"obj": self.design_object})
         # local_logger.info("Reverting journal entry for %s %s", object_type, object_str, extra={"obj": self})
         if self.full_control:
-            related_entries = (
-                JournalEntry.objects.filter(active=True)
-                .filter_related(self)
-                .filter_same_parent_design_instance(self)
-                .exclude_decommissioned()
-            )
+            related_entries = list(JournalEntry.objects.filter_related(self).values_list("id", flat=True))
             if related_entries:
-                active_journal_ids = ",".join([str(j.id) for j in related_entries])
+                active_journal_ids = ",".join(map(str, related_entries))
                 raise DesignValidationError(f"This object is referenced by other active Journals: {active_journal_ids}")
 
             self.design_object._current_design = self.journal.design_instance  # pylint: disable=protected-access

nautobot_design_builder/tests/test_data_protection.py

@@ -50,6 +50,7 @@ def setUp(self):
             full_control=True,
             changes=calculate_changes(self.manufacturer_from_design),
             journal=self.journal,
+            index=self.journal._next_index(),  # pylint:disable=protected-access
         )
 
         self.client = Client()

nautobot_design_builder/tests/test_decommissioning_job.py

@@ -125,7 +125,10 @@ def test_basic_decommission_run_with_full_control(self):
         self.assertEqual(1, Secret.objects.count())
 
         journal_entry = models.JournalEntry.objects.create(
-            journal=self.journal1, design_object=self.secret, full_control=True
+            journal=self.journal1,
+            design_object=self.secret,
+            full_control=True,
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry.validated_save()
 
@@ -137,13 +140,22 @@ def test_decommission_run_with_dependencies(self):
         self.assertEqual(1, Secret.objects.count())
 
         journal_entry_1 = models.JournalEntry.objects.create(
-            journal=self.journal1, design_object=self.secret, full_control=True
+            journal=self.journal1,
+            design_object=self.secret,
+            full_control=True,
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
 
         journal_entry_1.validated_save()
 
         journal_entry_2 = models.JournalEntry.objects.create(
-            journal=self.journal2, design_object=self.secret, full_control=False, changes={"differences": {}}
+            journal=self.journal2,
+            design_object=self.secret,
+            full_control=False,
+            changes={
+                "differences": {},
+            },
+            index=self.journal2._next_index(),  # pylint:disable=protected-access
         )
         journal_entry_2.validated_save()
 
@@ -160,20 +172,24 @@ def test_decommission_run_with_dependencies_but_decommissioned(self):
         self.assertEqual(1, Secret.objects.count())
 
         journal_entry_1 = models.JournalEntry.objects.create(
-            journal=self.journal1, design_object=self.secret, full_control=True
+            journal=self.journal1,
+            design_object=self.secret,
+            full_control=True,
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
 
         journal_entry_1.validated_save()
 
         journal_entry_2 = models.JournalEntry.objects.create(
-            journal=self.journal2, design_object=self.secret, full_control=False, changes={"differences": {}}
+            journal=self.journal2,
+            design_object=self.secret,
+            full_control=False,
+            changes={"differences": {}},
+            index=self.journal2._next_index(),  # pylint:disable=protected-access
         )
         journal_entry_2.validated_save()
 
-        self.design_instance_2.status = Status.objects.get(
-            content_types=self.content_type, name=choices.DesignInstanceStatusChoices.DECOMMISSIONED
-        )
-        self.design_instance_2.validated_save()
+        self.design_instance_2.decommission()
 
         self.job.run(data={"design_instances": [self.design_instance]}, commit=True)
 
@@ -183,7 +199,11 @@ def test_basic_decommission_run_without_full_control(self):
         self.assertEqual(1, Secret.objects.count())
 
         journal_entry_1 = models.JournalEntry.objects.create(
-            journal=self.journal1, design_object=self.secret, full_control=False, changes={"differences": {}}
+            journal=self.journal1,
+            design_object=self.secret,
+            full_control=False,
+            changes={"differences": {}},
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry_1.validated_save()
 
@@ -205,6 +225,7 @@ def test_decommission_run_without_full_control_string_value(self):
                     "removed": {"description": "previous description"},
                 }
             },
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry.validated_save()
 
@@ -224,6 +245,7 @@ def test_decommission_run_without_full_control_dict_value_with_overlap(self):
                     "removed": {"parameters": self.initial_params},
                 }
             },
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry.validated_save()
 
@@ -245,6 +267,7 @@ def test_decommission_run_without_full_control_dict_value_without_overlap(self):
                     "removed": {"parameters": self.initial_params},
                 }
             },
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry.validated_save()
 
@@ -270,6 +293,7 @@ def test_decommission_run_without_full_control_dict_value_with_new_values_and_ol
                     "removed": {"parameters": self.initial_params},
                 }
             },
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry.validated_save()
 
@@ -282,7 +306,10 @@ def test_decommission_run_with_pre_hook_pass(self):
         self.assertEqual(1, Secret.objects.count())
 
         journal_entry_1 = models.JournalEntry.objects.create(
-            journal=self.journal1, design_object=self.secret, full_control=True
+            journal=self.journal1,
+            design_object=self.secret,
+            full_control=True,
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry_1.validated_save()
 
@@ -295,7 +322,10 @@ def test_decommission_run_with_pre_hook_fail(self):
         models.DesignInstance.pre_decommission.connect(fake_ko)
         self.assertEqual(1, Secret.objects.count())
         journal_entry_1 = models.JournalEntry.objects.create(
-            journal=self.journal1, design_object=self.secret, full_control=True
+            journal=self.journal1,
+            design_object=self.secret,
+            full_control=True,
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry_1.validated_save()
 
@@ -311,7 +341,10 @@ def test_decommission_run_with_pre_hook_fail(self):
 
     def test_decommission_run_multiple_design_instance(self):
         journal_entry = models.JournalEntry.objects.create(
-            journal=self.journal1, design_object=self.secret, full_control=True
+            journal=self.journal1,
+            design_object=self.secret,
+            full_control=True,
+            index=self.journal1._next_index(),  # pylint:disable=protected-access
         )
         journal_entry.validated_save()
 
@@ -323,7 +356,10 @@ def test_decommission_run_multiple_design_instance(self):
         secret_2.validated_save()
 
         journal_entry_2 = models.JournalEntry.objects.create(
-            journal=self.journal2, design_object=secret_2, full_control=True
+            journal=self.journal2,
+            design_object=secret_2,
+            full_control=True,
+            index=self.journal2._next_index(),  # pylint:disable=protected-access
         )
         journal_entry_2.validated_save()