Merge branch 'debug-fix'

.github/workflows/docker.yaml

@@ -21,6 +21,10 @@ on:
     branches:
       - 'master'
 
+  pull_request:
+    branches:
+      - 'master'
+
 jobs:
 
   # Build the docker image

.gitignore

@@ -42,6 +42,7 @@ dummy_image.*
 _tmp.csv
 inventree/label.pdf
 inventree/label.png
+inventree/my_special*
 
 # Sphinx files
 docs/_build

InvenTree/InvenTree/config.py

@@ -59,6 +59,13 @@ def get_config_file(create=True) -> Path:
 def load_config_data() -> map:
     """Load configuration data from the config file."""
 
+    import sys
+
+    print("load_config_data()")
+    print("- cwd:", os.getcwd())
+    print("- exe:", sys.executable)
+    print("- path:", sys.path)
+
     import yaml
 
     cfg_file = get_config_file()

InvenTree/InvenTree/settings.py

@@ -31,6 +31,16 @@ INVENTREE_NEWS_URL = 'https://inventree.org/news/feed.atom'
 # Determine if we are running in "test" mode e.g. "manage.py test"
 TESTING = 'test' in sys.argv
 
+# Note: The following fix is "required" for docker build workflow
+# Note: 2022-12-12 still unsure why...
+if TESTING:
+    # Ensure that sys.path includes global python libs
+    python_dir = os.path.dirname(sys.executable)
+    python_lib = os.path.join(python_dir, "lib", "site-packages")
+
+    if python_lib not in sys.path:
+        sys.path.append(python_lib)
+
 # Are environment variables manipulated by tests? Needs to be set by testing code
 TESTING_ENV = False
 

InvenTree/InvenTree/views.py

@@ -642,6 +642,9 @@ class CustomLoginView(LoginView):
             # Initiate form
             form = self.get_form_class()(request.GET.dict(), request=request)
 
+            # Validate form data
+            form.is_valid()
+
             # Try to login
             form.full_clean()
             return form.login(request)

InvenTree/company/models.py

@@ -704,7 +704,7 @@ def after_save_supplier_price(sender, instance, created, **kwargs):
     if InvenTree.ready.canAppAccessDatabase() and not InvenTree.ready.isImportingData():
 
         if instance.part and instance.part.part:
-            instance.part.part.pricing.schedule_for_update()
+            instance.part.part.schedule_pricing_update()
 
 
 @receiver(post_delete, sender=SupplierPriceBreak, dispatch_uid='post_delete_supplier_price_break')
@@ -714,4 +714,4 @@ def after_delete_supplier_price(sender, instance, **kwargs):
     if InvenTree.ready.canAppAccessDatabase() and not InvenTree.ready.isImportingData():
 
         if instance.part and instance.part.part:
-            instance.part.part.pricing.schedule_for_update()
+            instance.part.part.schedule_pricing_update()

InvenTree/order/models.py

@@ -390,7 +390,7 @@ class PurchaseOrder(Order):
             # Schedule pricing update for any referenced parts
             for line in self.lines.all():
                 if line.part and line.part.part:
-                    line.part.part.pricing.schedule_for_update()
+                    line.part.part.schedule_pricing_update()
 
             trigger_event('purchaseorder.completed', id=self.pk)
 
@@ -778,7 +778,7 @@ class SalesOrder(Order):
 
         # Schedule pricing update for any referenced parts
         for line in self.lines.all():
-            line.part.pricing.schedule_for_update()
+            line.part.schedule_pricing_update()
 
         trigger_event('salesorder.completed', id=self.pk)
 

InvenTree/part/models.py

@@ -771,6 +771,10 @@ class Part(InvenTreeBarcodeMixin, MetadataMixin, MPTTModel):
                     'IPN': _('Duplicate IPN not allowed in part settings'),
                 })
 
+        # Ensure unique across (Name, revision, IPN) (as specified)
+        if Part.objects.exclude(pk=self.pk).filter(name=self.name, revision=self.revision, IPN=self.IPN).exists():
+            raise ValidationError(_("Part with this Name, IPN and Revision already exists."))
+
     def clean(self):
         """Perform cleaning operations for the Part model.
 
@@ -1699,6 +1703,20 @@ class Part(InvenTreeBarcodeMixin, MetadataMixin, MPTTModel):
 
         return pricing
 
+    def schedule_pricing_update(self):
+        """Helper function to schedule a pricing update.
+
+        Importantly, catches any errors which may occur during deletion of related objects,
+        in particular due to post_delete signals.
+
+        Ref: https://github.com/inventree/InvenTree/pull/3986
+        """
+
+        try:
+            self.pricing.schedule_for_update()
+        except (PartPricing.DoesNotExist, IntegrityError):
+            pass
+
     def get_price_info(self, quantity=1, buy=True, bom=True, internal=False):
         """Return a simplified pricing string for this part.
 
@@ -2293,23 +2311,35 @@ class PartPricing(models.Model):
     def schedule_for_update(self, counter: int = 0):
         """Schedule this pricing to be updated"""
 
-        if self.pk is None:
+        try:
-            self.save()
+            self.refresh_from_db()
+        except (PartPricing.DoesNotExist, IntegrityError):
+            # Error thrown if this PartPricing instance has already been removed
+            return
 
-        self.refresh_from_db()
+        # Ensure that the referenced part still exists in the database
+        try:
+            p = self.part
+            p.refresh_from_db()
+        except IntegrityError:
+            return
 
         if self.scheduled_for_update:
             # Ignore if the pricing is already scheduled to be updated
-            logger.info(f"Pricing for {self.part} already scheduled for update - skipping")
+            logger.info(f"Pricing for {p} already scheduled for update - skipping")
             return
 
         if counter > 25:
             # Prevent infinite recursion / stack depth issues
-            logger.info(counter, f"Skipping pricing update for {self.part} - maximum depth exceeded")
+            logger.info(counter, f"Skipping pricing update for {p} - maximum depth exceeded")
             return
 
-        self.scheduled_for_update = True
+        try:
-        self.save()
+            self.scheduled_for_update = True
+            self.save()
+        except IntegrityError:
+            # An IntegrityError here likely indicates that the referenced part has already been deleted
+            return
 
         import part.tasks as part_tasks
 
@@ -2372,7 +2402,11 @@ class PartPricing(models.Model):
         # Update the currency which was used to perform the calculation
         self.currency = currency_code_default()
 
-        self.update_overall_cost()
+        try:
+            self.update_overall_cost()
+        except IntegrityError:
+            # If something has happened to the Part model, might throw an error
+            pass
 
         super().save(*args, **kwargs)
 
@@ -3557,7 +3591,7 @@ def update_pricing_after_edit(sender, instance, created, **kwargs):
 
     # Update part pricing *unless* we are importing data
     if InvenTree.ready.canAppAccessDatabase() and not InvenTree.ready.isImportingData():
-        instance.part.pricing.schedule_for_update()
+        instance.part.schedule_pricing_update()
 
 
 @receiver(post_delete, sender=BomItem, dispatch_uid='post_delete_bom_item')
@@ -3568,7 +3602,7 @@ def update_pricing_after_delete(sender, instance, **kwargs):
 
     # Update part pricing *unless* we are importing data
     if InvenTree.ready.canAppAccessDatabase() and not InvenTree.ready.isImportingData():
-        instance.part.pricing.schedule_for_update()
+        instance.part.schedule_pricing_update()
 
 
 class BomItemSubstitute(models.Model):

InvenTree/part/test_api.py

@@ -126,7 +126,7 @@ class PartCategoryAPITest(InvenTreeAPITestCase):
             # Create parts in this category
             for jj in range(10):
                 Part.objects.create(
-                    name=f"Part xyz {jj}",
+                    name=f"Part xyz {jj}_{ii}",
                     description="A test part",
                     category=child
                 )
@@ -339,7 +339,7 @@ class PartCategoryAPITest(InvenTreeAPITestCase):
             # Create parts in the category to be deleted
             for jj in range(3):
                 parts.append(Part.objects.create(
-                    name=f"Part xyz {jj}",
+                    name=f"Part xyz {i}_{jj}",
                     description="Child part of the deleted category",
                     category=cat_to_delete
                 ))
@@ -349,7 +349,7 @@ class PartCategoryAPITest(InvenTreeAPITestCase):
             # Create child categories under the category to be deleted
             for ii in range(3):
                 child = PartCategory.objects.create(
-                    name=f"Child parent_cat {ii}",
+                    name=f"Child parent_cat {i}_{ii}",
                     description="A child category of the deleted category",
                     parent=cat_to_delete
                 )
@@ -358,7 +358,7 @@ class PartCategoryAPITest(InvenTreeAPITestCase):
                 # Create parts in the child categories
                 for jj in range(3):
                     child_categories_parts.append(Part.objects.create(
-                        name=f"Part xyz {jj}",
+                        name=f"Part xyz {i}_{jj}_{ii}",
                         description="Child part in the child category of the deleted category",
                         category=child
                     ))
@@ -881,11 +881,15 @@ class PartAPITest(InvenTreeAPITestCase):
         """
         url = reverse('api-part-list')
 
-        response = self.post(url, {
+        response = self.post(
-            'name': 'all defaults',
+            url,
-            'description': 'my test part',
+            {
-            'category': 1,
+                'name': 'all defaults',
-        })
+                'description': 'my test part',
+                'category': 1,
+            },
+            expected_code=201,
+        )
 
         data = response.data
 
@@ -903,23 +907,31 @@ class PartAPITest(InvenTreeAPITestCase):
             self.user
         )
 
-        response = self.post(url, {
+        response = self.post(
-            'name': 'all defaults',
+            url,
-            'description': 'my test part 2',
+            {
-            'category': 1,
+                'name': 'all defaults 2',
-        })
+                'description': 'my test part 2',
+                'category': 1,
+            },
+            expected_code=201,
+        )
 
         # Part should now be purchaseable by default
         self.assertTrue(response.data['purchaseable'])
 
         # "default" values should not be used if the value is specified
-        response = self.post(url, {
+        response = self.post(
-            'name': 'all defaults',
+            url,
-            'description': 'my test part 2',
+            {
-            'category': 1,
+                'name': 'all defaults 3',
-            'active': False,
+                'description': 'my test part 3',
-            'purchaseable': False,
+                'category': 1,
-        })
+                'active': False,
+                'purchaseable': False,
+            },
+            expected_code=201
+        )
 
         self.assertFalse(response.data['active'])
         self.assertFalse(response.data['purchaseable'])
@@ -2752,3 +2764,18 @@ class PartInternalPriceBreakTest(InvenTreeAPITestCase):
                 round(Decimal(data['price']), 4),
                 round(Decimal(p), 4)
             )
+
+        # Now, ensure that we can delete the Part via the API
+        # In particular this test checks that there are no circular post_delete relationships
+        # Ref: https://github.com/inventree/InvenTree/pull/3986
+
+        # First, ensure the part instance can be deleted
+        p = Part.objects.get(pk=1)
+        p.active = False
+        p.save()
+
+        response = self.delete(reverse("api-part-detail", kwargs={"pk": 1}))
+        self.assertEqual(response.status_code, 204)
+
+        with self.assertRaises(Part.DoesNotExist):
+            p.refresh_from_db()

InvenTree/part/test_pricing.py

@@ -328,3 +328,44 @@ class PartPricingTests(InvenTreeTestCase):
 
         self.assertEqual(pricing.purchase_cost_min, Money('1.333333', 'USD'))
         self.assertEqual(pricing.purchase_cost_max, Money('1.764706', 'USD'))
+
+    def test_delete_with_pricing(self):
+        """Test for deleting a part which has pricing information"""
+
+        # Create some pricing data
+        self.create_price_breaks()
+
+        # Check that pricing does exist
+        pricing = self.part.pricing
+
+        pricing.update_pricing()
+        pricing.save()
+
+        self.assertIsNotNone(pricing.overall_min)
+        self.assertIsNotNone(pricing.overall_max)
+
+        self.part.active = False
+        self.part.save()
+
+        # Remove the part from the database
+        self.part.delete()
+
+        # Check that the pricing was removed also
+        with self.assertRaises(part.models.PartPricing.DoesNotExist):
+            pricing.refresh_from_db()
+
+    def test_delete_without_pricing(self):
+        """Test that we can delete a part which does not have pricing information"""
+
+        pricing = self.part.pricing
+
+        self.assertIsNone(pricing.pk)
+
+        self.part.active = False
+        self.part.save()
+
+        self.part.delete()
+
+        # Check that part was actually deleted
+        with self.assertRaises(part.models.Part.DoesNotExist):
+            self.part.refresh_from_db()