From fe0d9c19230013471a2100fbc1f41c7694f318fa Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 15 Nov 2023 10:18:26 +1100 Subject: [PATCH] Distinct query fix (#5916) * Fix "allocated" query for stock items * Use distinct() elsewhere * Add unit test to check distinct query results --- InvenTree/build/models.py | 2 +- InvenTree/company/api.py | 2 +- InvenTree/company/models.py | 4 +- InvenTree/part/api.py | 17 ++++--- InvenTree/stock/api.py | 15 ++++-- InvenTree/stock/test_api.py | 95 +++++++++++++++++++++++++++++++++++++ 6 files changed, 119 insertions(+), 16 deletions(-) diff --git a/InvenTree/build/models.py b/InvenTree/build/models.py index edb2d665a3..737f60f7fa 100644 --- a/InvenTree/build/models.py +++ b/InvenTree/build/models.py @@ -1012,7 +1012,7 @@ class Build(MPTTModel, InvenTree.mixins.DiffMixin, InvenTree.models.InvenTreeBar ) # Filter out "serialized" stock items, these cannot be auto-allocated - available_stock = available_stock.filter(Q(serial=None) | Q(serial='')) + available_stock = available_stock.filter(Q(serial=None) | Q(serial='')).distinct() if location: # Filter only stock items located "below" the specified location diff --git a/InvenTree/company/api.py b/InvenTree/company/api.py index 88f8a875ac..51c27f2279 100644 --- a/InvenTree/company/api.py +++ b/InvenTree/company/api.py @@ -376,7 +376,7 @@ class SupplierPartList(ListCreateDestroyAPIView): company = params.get('company', None) if company is not None: - queryset = queryset.filter(Q(manufacturer_part__manufacturer=company) | Q(supplier=company)) + queryset = queryset.filter(Q(manufacturer_part__manufacturer=company) | Q(supplier=company)).distinct() return queryset diff --git a/InvenTree/company/models.py b/InvenTree/company/models.py index f5eef23b3a..465df22125 100644 --- a/InvenTree/company/models.py +++ b/InvenTree/company/models.py @@ -206,13 +206,13 @@ class Company(InvenTreeNotesMixin, MetadataMixin, models.Model): @property def parts(self): """Return SupplierPart objects which are supplied or manufactured by this company.""" - return SupplierPart.objects.filter(Q(supplier=self.id) | Q(manufacturer_part__manufacturer=self.id)) + return SupplierPart.objects.filter(Q(supplier=self.id) | Q(manufacturer_part__manufacturer=self.id)).distinct() @property def stock_items(self): """Return a list of all stock items supplied or manufactured by this company.""" stock = apps.get_model('stock', 'StockItem') - return stock.objects.filter(Q(supplier_part__supplier=self.id) | Q(supplier_part__manufacturer_part__manufacturer=self.id)).all() + return stock.objects.filter(Q(supplier_part__supplier=self.id) | Q(supplier_part__manufacturer_part__manufacturer=self.id)).distinct() class CompanyAttachment(InvenTreeAttachment): diff --git a/InvenTree/part/api.py b/InvenTree/part/api.py index 64ac4d1f7a..ab725d3b76 100644 --- a/InvenTree/part/api.py +++ b/InvenTree/part/api.py @@ -921,7 +921,8 @@ class PartFilter(rest_filters.FilterSet): if str2bool(value): return queryset.exclude(q_a | q_b) - return queryset.filter(q_a | q_b) + + return queryset.filter(q_a | q_b).distinct() stocktake = rest_filters.BooleanFilter(label="Has stocktake", method='filter_has_stocktake') @@ -1132,7 +1133,7 @@ class PartList(PartMixin, APIDownloadMixin, ListCreateAPI): # Return any relationship which points to the part in question relation_filter = Q(part_1=related_part) | Q(part_2=related_part) - for relation in PartRelated.objects.filter(relation_filter): + for relation in PartRelated.objects.filter(relation_filter).distinct(): if relation.part_1.pk != pk: part_ids.add(relation.part_1.pk) @@ -1310,8 +1311,7 @@ class PartRelatedList(ListCreateAPI): if part is not None: try: part = Part.objects.get(pk=part) - - queryset = queryset.filter(Q(part_1=part) | Q(part_2=part)) + queryset = queryset.filter(Q(part_1=part) | Q(part_2=part)).distinct() except (ValueError, Part.DoesNotExist): pass @@ -1349,7 +1349,8 @@ class PartParameterTemplateFilter(rest_filters.FilterSet): """Filter queryset to include only PartParameterTemplates with choices.""" if str2bool(value): return queryset.exclude(Q(choices=None) | Q(choices='')) - return queryset.filter(Q(choices=None) | Q(choices='')) + + return queryset.filter(Q(choices=None) | Q(choices='')).distinct() has_units = rest_filters.BooleanFilter( method='filter_has_units', @@ -1360,7 +1361,8 @@ class PartParameterTemplateFilter(rest_filters.FilterSet): """Filter queryset to include only PartParameterTemplates with units.""" if str2bool(value): return queryset.exclude(Q(units=None) | Q(units='')) - return queryset.filter(Q(units=None) | Q(units='')) + + return queryset.filter(Q(units=None) | Q(units='')).distinct() class PartParameterTemplateList(ListCreateAPI): @@ -1662,7 +1664,8 @@ class BomFilter(rest_filters.FilterSet): if str2bool(value): return queryset.exclude(q_a | q_b) - return queryset.filter(q_a | q_b) + + return queryset.filter(q_a | q_b).distinct() class BomMixin: diff --git a/InvenTree/stock/api.py b/InvenTree/stock/api.py index 4482f85130..52632dbcd3 100644 --- a/InvenTree/stock/api.py +++ b/InvenTree/stock/api.py @@ -481,7 +481,7 @@ class StockFilter(rest_filters.FilterSet): """Filter by whether or not the stock item is 'allocated'""" if str2bool(value): # Filter StockItem with either build allocations or sales order allocations - return queryset.filter(Q(sales_order_allocations__isnull=False) | Q(allocations__isnull=False)) + return queryset.filter(Q(sales_order_allocations__isnull=False) | Q(allocations__isnull=False)).distinct() # Filter StockItem without build allocations or sales order allocations return queryset.filter(Q(sales_order_allocations__isnull=True) & Q(allocations__isnull=True)) @@ -546,7 +546,8 @@ class StockFilter(rest_filters.FilterSet): if str2bool(value): return queryset.exclude(q) - return queryset.filter(q) + + return queryset.filter(q).distinct() has_batch = rest_filters.BooleanFilter(label='Has batch code', method='filter_has_batch') @@ -556,7 +557,8 @@ class StockFilter(rest_filters.FilterSet): if str2bool(value): return queryset.exclude(q) - return queryset.filter(q) + + return queryset.filter(q).distinct() tracked = rest_filters.BooleanFilter(label='Tracked', method='filter_tracked') @@ -572,7 +574,8 @@ class StockFilter(rest_filters.FilterSet): if str2bool(value): return queryset.exclude(q_batch & q_serial) - return queryset.filter(q_batch & q_serial) + + return queryset.filter(q_batch).filter(q_serial).distinct() installed = rest_filters.BooleanFilter(label='Installed in other stock item', method='filter_installed') @@ -1056,7 +1059,9 @@ class StockList(APIDownloadMixin, ListCreateDestroyAPIView): company = params.get('company', None) if company is not None: - queryset = queryset.filter(Q(supplier_part__supplier=company) | Q(supplier_part__manufacturer_part__manufacturer=company)) + queryset = queryset.filter( + Q(supplier_part__supplier=company) | Q(supplier_part__manufacturer_part__manufacturer=company).distinct() + ) return queryset diff --git a/InvenTree/stock/test_api.py b/InvenTree/stock/test_api.py index 9d518179b3..46ee953f4c 100644 --- a/InvenTree/stock/test_api.py +++ b/InvenTree/stock/test_api.py @@ -13,6 +13,7 @@ import tablib from djmoney.money import Money from rest_framework import status +import build.models import company.models import part.models from common.models import InvenTreeSetting @@ -639,6 +640,100 @@ class StockItemListTest(StockAPITestCase): self.assertEqual(len(dataset), 17) + def test_filter_by_allocated(self): + """Test that we can filter by "allocated" status: + + - Only return stock items which are 'allocated' + - Either to a build order or sales order + - Test that the results are "distinct" (no duplicated results) + - Ref: https://github.com/inventree/InvenTree/pull/5916 + """ + + # Create a build order to allocate to + assembly = part.models.Part.objects.create(name='F Assembly', description='Assembly for filter test', assembly=True) + component = part.models.Part.objects.create(name='F Component', description='Component for filter test', component=True) + bom_item = part.models.BomItem.objects.create(part=assembly, sub_part=component, quantity=10) + + # Create two build orders + bo_1 = build.models.Build.objects.create(part=assembly, quantity=10) + bo_2 = build.models.Build.objects.create(part=assembly, quantity=20) + + # Test that two distinct build line items are created automatically + self.assertEqual(bo_1.build_lines.count(), 1) + self.assertEqual(bo_2.build_lines.count(), 1) + self.assertEqual(build.models.BuildLine.objects.filter(bom_item=bom_item).count(), 2) + + build_line_1 = bo_1.build_lines.first() + build_line_2 = bo_2.build_lines.first() + + # Allocate stock + location = StockLocation.objects.first() + stock_1 = StockItem.objects.create(part=component, quantity=100, location=location) + stock_2 = StockItem.objects.create(part=component, quantity=100, location=location) + stock_3 = StockItem.objects.create(part=component, quantity=100, location=location) + + # Allocate stock_1 to two build orders + build.models.BuildItem.objects.create( + stock_item=stock_1, + build_line=build_line_1, + quantity=5 + ) + + build.models.BuildItem.objects.create( + stock_item=stock_1, + build_line=build_line_2, + quantity=5 + ) + + # Allocate stock_2 to 1 build orders + build.models.BuildItem.objects.create( + stock_item=stock_2, + build_line=build_line_1, + quantity=5 + ) + + url = reverse('api-stock-list') + + # 3 items when just filtering by part + response = self.get( + url, + { + "part": component.pk, + "in_stock": True + }, + expected_code=200 + ) + self.assertEqual(len(response.data), 3) + + # 1 item when filtering by "not allocated" + response = self.get( + url, + { + "part": component.pk, + "in_stock": True, + "allocated": False, + }, + expected_code=200 + ) + + self.assertEqual(len(response.data), 1) + self.assertEqual(response.data[0]["pk"], stock_3.pk) + + # 2 items when filtering by "allocated" + response = self.get( + url, + { + "part": component.pk, + "in_stock": True, + "allocated": True, + }, + expected_code=200 + ) + + self.assertEqual(len(response.data), 2) + self.assertEqual(response.data[0]["pk"], stock_1.pk) + self.assertEqual(response.data[1]["pk"], stock_2.pk) + def test_query_count(self): """Test that the number of queries required to fetch stock items is reasonable."""