From 2908ad0721d8a5d91aa16d989fadd4391805d1b6 Mon Sep 17 00:00:00 2001 From: Oliver Date: Tue, 31 Oct 2023 22:58:43 +1100 Subject: [PATCH] Recursive delete fix fix (#5819) * Enable mysql checks as part of PR * Add debug for CI * Add delete_nodes method - Ensure that the "parent" field is set to None before delete - This means that we do not violate any ForeignKey constraints due to undefined order of operations --- .github/workflows/qc_checks.yaml | 1 - InvenTree/InvenTree/models.py | 22 ++++++++++++++++++++-- InvenTree/part/test_category.py | 2 ++ 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/.github/workflows/qc_checks.yaml b/.github/workflows/qc_checks.yaml index 1dad674423..fc008768a7 100644 --- a/.github/workflows/qc_checks.yaml +++ b/.github/workflows/qc_checks.yaml @@ -274,7 +274,6 @@ jobs: runs-on: ubuntu-20.04 needs: [ 'pep_style', 'pre-commit' ] - if: github.event_name == 'push' env: # Database backend configuration diff --git a/InvenTree/InvenTree/models.py b/InvenTree/InvenTree/models.py index 640fa23306..44bcca5d9d 100644 --- a/InvenTree/InvenTree/models.py +++ b/InvenTree/InvenTree/models.py @@ -660,12 +660,14 @@ class InvenTreeTree(MPTTModel): D) delete_children = False and delete_items = False """ + child_nodes = self.get_descendants(include_self=False) + # Case A: Delete all child items, and all child nodes. # - Delete all items at any lower level # - Delete all descendant nodes if delete_children and delete_items: self.get_items(cascade=True).delete() - self.get_descendants(include_self=False).delete() + self.delete_nodes(child_nodes) # Case B: Delete all child nodes, but move all child items up to the parent # - Move all items at any lower level to the parent of this item @@ -674,7 +676,8 @@ class InvenTreeTree(MPTTModel): self.get_items(cascade=True).update(**{ self.ITEM_PARENT_KEY: self.parent }) - self.get_descendants(include_self=False).delete() + + self.delete_nodes(child_nodes) # Case C: Delete all child items, but keep all child nodes # - Remove all items directly associated with this node @@ -692,6 +695,21 @@ class InvenTreeTree(MPTTModel): }) self.get_children().update(parent=self.parent) + def delete_nodes(self, nodes): + """Delete a set of nodes from the tree. + + 1. First, set the "parent" value for selected nodes to None + 2. Then, perform bulk deletion of selected nodes + + Step 1. is required because we cannot guarantee the order-of-operations in the db backend + + Arguments: + nodes: A queryset of nodes to delete + """ + + nodes.update(parent=None) + nodes.delete() + def validate_unique(self, exclude=None): """Validate that this tree instance satisfies our uniqueness requirements. diff --git a/InvenTree/part/test_category.py b/InvenTree/part/test_category.py index 4e5400c2fd..714c192449 100644 --- a/InvenTree/part/test_category.py +++ b/InvenTree/part/test_category.py @@ -226,6 +226,8 @@ class CategoryTest(TestCase): # Clear out any existing parts Part.objects.all().delete() + PartCategory.objects.rebuild() + # First, create a structured tree of part categories A = PartCategory.objects.create( name='A',