Skip to content

Commit

Permalink
Distinct query fix (#5916)
Browse files Browse the repository at this point in the history
* Fix "allocated" query for stock items

* Use distinct() elsewhere

* Add unit test to check distinct query results
  • Loading branch information
SchrodingersGat committed Nov 14, 2023
1 parent 5be6bc8 commit fe0d9c1
Show file tree
Hide file tree
Showing 6 changed files with 119 additions and 16 deletions.
2 changes: 1 addition & 1 deletion InvenTree/build/models.py
Expand Up @@ -1012,7 +1012,7 @@ def stock_sort(item, bom_item, variant_parts):
)

# 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
Expand Down
2 changes: 1 addition & 1 deletion InvenTree/company/api.py
Expand Up @@ -376,7 +376,7 @@ def filter_queryset(self, queryset):
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

Expand Down
4 changes: 2 additions & 2 deletions InvenTree/company/models.py
Expand Up @@ -206,13 +206,13 @@ def get_thumbnail_url(self):
@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):
Expand Down
17 changes: 10 additions & 7 deletions InvenTree/part/api.py
Expand Up @@ -921,7 +921,8 @@ def filter_has_pricing(self, queryset, name, value):

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')

Expand Down Expand Up @@ -1132,7 +1133,7 @@ def filter_queryset(self, queryset):
# 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)
Expand Down Expand Up @@ -1310,8 +1311,7 @@ def filter_queryset(self, queryset):
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
Expand Down Expand Up @@ -1349,7 +1349,8 @@ def filter_has_choices(self, queryset, name, value):
"""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',
Expand All @@ -1360,7 +1361,8 @@ def filter_has_units(self, queryset, name, value):
"""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):
Expand Down Expand Up @@ -1662,7 +1664,8 @@ def filter_has_pricing(self, queryset, name, value):

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:
Expand Down
15 changes: 10 additions & 5 deletions InvenTree/stock/api.py
Expand Up @@ -481,7 +481,7 @@ def filter_allocated(self, queryset, name, value):
"""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))

Expand Down Expand Up @@ -546,7 +546,8 @@ def filter_serialized(self, queryset, name, value):

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')

Expand All @@ -556,7 +557,8 @@ def filter_has_batch(self, queryset, name, value):

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')

Expand All @@ -572,7 +574,8 @@ def filter_tracked(self, queryset, name, value):

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')

Expand Down Expand Up @@ -1056,7 +1059,9 @@ def filter_queryset(self, queryset):
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

Expand Down
95 changes: 95 additions & 0 deletions InvenTree/stock/test_api.py
Expand Up @@ -13,6 +13,7 @@
from djmoney.money import Money
from rest_framework import status

import build.models
import company.models
import part.models
from common.models import InvenTreeSetting
Expand Down Expand Up @@ -639,6 +640,100 @@ def test_export(self):

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."""

Expand Down

0 comments on commit fe0d9c1

Please sign in to comment.