From a5f035acdcc2bfc435e9554b4e6659f96a95af60 Mon Sep 17 00:00:00 2001
From: Leo Kirchner <leo.kirchner@networktocode.com>
Date: Wed, 15 Nov 2023 08:50:05 +0100
Subject: [PATCH] fixes natural deletion order flag

Prior to this change, if natural deletion order was set on a parent
model and that model had no changes, children would not be recursed
through.
---
 diffsync/helpers.py                     |  4 +-
 tests/unit/test_diffsync_model_flags.py | 64 +++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 1 deletion(-)

diff --git a/diffsync/helpers.py b/diffsync/helpers.py
index 96882bae..c4e64c53 100644
--- a/diffsync/helpers.py
+++ b/diffsync/helpers.py
@@ -369,11 +369,13 @@ def sync_diff_element(self, element: DiffElement, parent_model: Optional["DiffSy
             natural_deletion_order = bool(dst_model.model_flags & DiffSyncModelFlags.NATURAL_DELETION_ORDER)
             skip_children = bool(dst_model.model_flags & DiffSyncModelFlags.SKIP_CHILDREN_ON_DELETE)
 
+        # Recurse through children to delete if we are supposed to delete the current diff element
         changed = False
         if natural_deletion_order and self.action == DiffSyncActions.DELETE and not skip_children:
             for child in element.get_children():
                 changed |= self.sync_diff_element(child, parent_model=dst_model)
 
+        # Sync the current model - this will delete the current model if self.action is DELETE
         changed, modified_model = self.sync_model(src_model=src_model, dst_model=dst_model, ids=ids, attrs=attrs)
         dst_model = modified_model or dst_model
 
@@ -396,7 +398,7 @@ def sync_diff_element(self, element: DiffElement, parent_model: Optional["DiffSy
 
         self.incr_elements_processed()
 
-        if not natural_deletion_order:
+        if not natural_deletion_order or self.action is not DiffSyncActions.DELETE:
             for child in element.get_children():
                 changed |= self.sync_diff_element(child, parent_model=dst_model)
 
diff --git a/tests/unit/test_diffsync_model_flags.py b/tests/unit/test_diffsync_model_flags.py
index 76457ccf..8c10499d 100644
--- a/tests/unit/test_diffsync_model_flags.py
+++ b/tests/unit/test_diffsync_model_flags.py
@@ -162,3 +162,67 @@ def load(self):
     source.remove(source.get("parent", {"name": "Test-Parent"}), remove_children=True)
     source.sync_to(destination)
     assert call_order == ["Test-Child", "Test-Parent"]
+
+
+def test_natural_deletion_order_with_noop_parent():
+    """Test whether children are recursed through when natural deletion order is set and the parent has no changes."""
+    call_order = []
+
+    class ChildModel(DiffSyncModel):
+        """Test child model that reports when its update method is called."""
+
+        _modelname = "child"
+        _identifiers = ("name",)
+        _attributes = ("attribute",)
+
+        name: str
+        attribute: str
+
+        def update(self, attrs):
+            call_order.append("Update on child")
+            return super().update(attrs)
+
+    class ParentModel(DiffSyncModel):
+        """Test parent model."""
+
+        _modelname = "parent"
+        _identifiers = ("name",)
+        _attributes = ("attribute",)
+        _children = {"child": "children"}
+
+        name: str
+        attribute: str
+        children: List[ChildModel] = []
+
+    class Adapter(DiffSync):
+        """Test adapter."""
+
+        top_level = ["parent"]
+
+        parent = ParentModel
+        child = ChildModel
+
+        def load(self, is_source=False) -> None:
+            """Test load method. Generate a difference with the is_source parameter."""
+            parent = self.parent(name="Test Parent", attribute="This doesn't change")
+            parent.model_flags |= DiffSyncModelFlags.NATURAL_DELETION_ORDER
+            self.add(parent)
+            if is_source:
+                child = self.child(name="Test Child", attribute="Attribute from source")
+                child.model_flags |= DiffSyncModelFlags.NATURAL_DELETION_ORDER
+                parent.add_child(child)
+                self.add(child)
+            else:
+                child = self.child(name="Test Child", attribute="Attribute from destination")
+                child.model_flags |= DiffSyncModelFlags.NATURAL_DELETION_ORDER
+                parent.add_child(child)
+                self.add(child)
+
+    source_adapter = Adapter()
+    source_adapter.load(is_source=True)
+    destination_adapter = Adapter()
+    destination_adapter.load()
+
+    source_adapter.sync_to(destination_adapter)
+
+    assert "Update on child" in call_order