From 04b3f022a0f074a16ead2a6cc2e232042581206c Mon Sep 17 00:00:00 2001
From: Oliver <oliver.henry.walters@gmail.com>
Date: Tue, 20 Sep 2022 19:02:08 +1000
Subject: [PATCH] Path name bug fix (#3694)

* bug fix: Ensure that child tree items update their pathstring when requried

* Add unit tests for fix

* Unit test fix

* Tweak for unit test
---
 InvenTree/InvenTree/helpers.py  | 22 +---------
 InvenTree/InvenTree/models.py   | 12 +++++-
 InvenTree/part/test_category.py |  2 +-
 InvenTree/stock/tests.py        | 76 +++++++++++++++++++++++++++++++++
 4 files changed, 90 insertions(+), 22 deletions(-)

diff --git a/InvenTree/InvenTree/helpers.py b/InvenTree/InvenTree/helpers.py
index 52befbff8f..0b34dee851 100644
--- a/InvenTree/InvenTree/helpers.py
+++ b/InvenTree/InvenTree/helpers.py
@@ -68,28 +68,10 @@ def constructPathString(path, max_chars=250):
 
     pathstring = '/'.join(path)
 
-    idx = 0
-
     # Replace middle elements to limit the pathstring
     if len(pathstring) > max_chars:
-        mid = len(path) // 2
-        path_l = path[0:mid]
-        path_r = path[mid:]
-
-        # Ensure the pathstring length is limited
-        while len(pathstring) > max_chars:
-
-            # Remove an element from the list
-            if idx % 2 == 0:
-                path_l = path_l[:-1]
-            else:
-                path_r = path_r[1:]
-
-            subpath = path_l + ['...'] + path_r
-
-            pathstring = '/'.join(subpath)
-
-            idx += 1
+        n = int(max_chars / 2 - 2)
+        pathstring = pathstring[:n] + "..." + pathstring[-n:]
 
     return pathstring
 
diff --git a/InvenTree/InvenTree/models.py b/InvenTree/InvenTree/models.py
index 61fab96f40..5a7491b610 100644
--- a/InvenTree/InvenTree/models.py
+++ b/InvenTree/InvenTree/models.py
@@ -516,8 +516,18 @@ class InvenTreeTree(MPTTModel):
         )
 
         if pathstring != self.pathstring:
+
+            if 'force_insert' in kwargs:
+                del kwargs['force_insert']
+
+            kwargs['force_update'] = True
+
             self.pathstring = pathstring
-            super().save(force_update=True)
+            super().save(*args, **kwargs)
+
+            # Ensure that the pathstring changes are propagated down the tree also
+            for child in self.get_children():
+                child.save(*args, **kwargs)
 
     class Meta:
         """Metaclass defines extra model properties."""
diff --git a/InvenTree/part/test_category.py b/InvenTree/part/test_category.py
index edd85b4f95..c02aa1b6bd 100644
--- a/InvenTree/part/test_category.py
+++ b/InvenTree/part/test_category.py
@@ -118,7 +118,7 @@ class CategoryTest(TestCase):
         self.assertTrue(len(child.path), 26)
         self.assertEqual(
             child.pathstring,
-            "Cat/AAAAAAAAAA/BBBBBBBBBB/CCCCCCCCCC/DDDDDDDDDD/EEEEEEEEEE/FFFFFFFFFF/GGGGGGGGGG/HHHHHHHHHH/IIIIIIIIII/JJJJJJJJJJ/.../OOOOOOOOOO/PPPPPPPPPP/QQQQQQQQQQ/RRRRRRRRRR/SSSSSSSSSS/TTTTTTTTTT/UUUUUUUUUU/VVVVVVVVVV/WWWWWWWWWW/XXXXXXXXXX/YYYYYYYYYY/ZZZZZZZZZZ"
+            "Cat/AAAAAAAAAA/BBBBBBBBBB/CCCCCCCCCC/DDDDDDDDDD/EEEEEEEEEE/FFFFFFFFFF/GGGGGGGGGG/HHHHHHHHHH/IIIIIIIIII/JJJJJJJJJJ/KKKKKKKKK...OO/PPPPPPPPPP/QQQQQQQQQQ/RRRRRRRRRR/SSSSSSSSSS/TTTTTTTTTT/UUUUUUUUUU/VVVVVVVVVV/WWWWWWWWWW/XXXXXXXXXX/YYYYYYYYYY/ZZZZZZZZZZ"
         )
         self.assertTrue(len(child.pathstring) <= 250)
 
diff --git a/InvenTree/stock/tests.py b/InvenTree/stock/tests.py
index 5e2fd72761..cf0773b246 100644
--- a/InvenTree/stock/tests.py
+++ b/InvenTree/stock/tests.py
@@ -48,6 +48,82 @@ class StockTestBase(InvenTreeTestCase):
 class StockTest(StockTestBase):
     """Tests to ensure that the stock location tree functions correcly."""
 
+    def test_pathstring(self):
+        """Check that pathstring updates occur as expected"""
+
+        a = StockLocation.objects.create(name="A")
+        b = StockLocation.objects.create(name="B", parent=a)
+        c = StockLocation.objects.create(name="C", parent=b)
+        d = StockLocation.objects.create(name="D", parent=c)
+
+        def refresh():
+            a.refresh_from_db()
+            b.refresh_from_db()
+            c.refresh_from_db()
+            d.refresh_from_db()
+
+        # Initial checks
+        self.assertEqual(a.pathstring, "A")
+        self.assertEqual(b.pathstring, "A/B")
+        self.assertEqual(c.pathstring, "A/B/C")
+        self.assertEqual(d.pathstring, "A/B/C/D")
+
+        c.name = "Cc"
+        c.save()
+
+        refresh()
+        self.assertEqual(a.pathstring, "A")
+        self.assertEqual(b.pathstring, "A/B")
+        self.assertEqual(c.pathstring, "A/B/Cc")
+        self.assertEqual(d.pathstring, "A/B/Cc/D")
+
+        b.name = "Bb"
+        b.save()
+
+        refresh()
+        self.assertEqual(a.pathstring, "A")
+        self.assertEqual(b.pathstring, "A/Bb")
+        self.assertEqual(c.pathstring, "A/Bb/Cc")
+        self.assertEqual(d.pathstring, "A/Bb/Cc/D")
+
+        a.name = "Aa"
+        a.save()
+
+        refresh()
+        self.assertEqual(a.pathstring, "Aa")
+        self.assertEqual(b.pathstring, "Aa/Bb")
+        self.assertEqual(c.pathstring, "Aa/Bb/Cc")
+        self.assertEqual(d.pathstring, "Aa/Bb/Cc/D")
+
+        d.name = "Dd"
+        d.save()
+
+        refresh()
+        self.assertEqual(a.pathstring, "Aa")
+        self.assertEqual(b.pathstring, "Aa/Bb")
+        self.assertEqual(c.pathstring, "Aa/Bb/Cc")
+        self.assertEqual(d.pathstring, "Aa/Bb/Cc/Dd")
+
+        # Test a really long name
+        # (it will be clipped to < 250 characters)
+        a.name = "A" * 100
+        a.save()
+        b.name = "B" * 100
+        b.save()
+        c.name = "C" * 100
+        c.save()
+        d.name = "D" * 100
+        d.save()
+
+        refresh()
+        self.assertEqual(len(a.pathstring), 100)
+        self.assertEqual(len(b.pathstring), 201)
+        self.assertEqual(len(c.pathstring), 249)
+        self.assertEqual(len(d.pathstring), 249)
+
+        self.assertTrue(d.pathstring.startswith("AAAAAAAA"))
+        self.assertTrue(d.pathstring.endswith("DDDDDDDD"))
+
     def test_link(self):
         """Test the link URL field validation"""