From b2b476b570b89c5705dea5a3bc934dcfdd4354f3 Mon Sep 17 00:00:00 2001 From: Oliver Date: Sun, 31 May 2026 13:38:18 +1000 Subject: [PATCH] [API] Fix 1 + N query for custom status fields (#12055) * Add CustomStatusSerializerMixin - Caches custom status values * Add unit test for PurchaseOrder model * Implement changes for StockItemSerializer * Implement changes for BuildOrder * Fix unit test * Bump API version * Add serializer field type hint --- .../InvenTree/InvenTree/api_version.py | 5 +- .../InvenTree/InvenTree/serializers.py | 50 ++++++++++ src/backend/InvenTree/build/serializers.py | 4 +- src/backend/InvenTree/build/test_api.py | 91 +++++++++++++++++++ src/backend/InvenTree/order/serializers.py | 9 +- src/backend/InvenTree/order/test_api.py | 67 +++++++++++++- src/backend/InvenTree/stock/serializers.py | 6 +- src/backend/InvenTree/stock/test_api.py | 76 ++++++++++++++++ 8 files changed, 295 insertions(+), 13 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/api_version.py b/src/backend/InvenTree/InvenTree/api_version.py index 82d1c6dba3..f13f695f0c 100644 --- a/src/backend/InvenTree/InvenTree/api_version.py +++ b/src/backend/InvenTree/InvenTree/api_version.py @@ -1,11 +1,14 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 497 +INVENTREE_API_VERSION = 498 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v498 -> 2026-05-31 : https://github.com/inventree/InvenTree/pull/12055 + - Updates the "status_text" field for models which support custom status values + v497 -> 2026-05-27 : https://github.com/inventree/InvenTree/pull/12019 - Adds "location" field to StockCount API endpoint diff --git a/src/backend/InvenTree/InvenTree/serializers.py b/src/backend/InvenTree/InvenTree/serializers.py index 3df76e9e3c..fc9073dd0b 100644 --- a/src/backend/InvenTree/InvenTree/serializers.py +++ b/src/backend/InvenTree/InvenTree/serializers.py @@ -764,6 +764,56 @@ class InvenTreeDecimalField(serializers.FloatField): raise serializers.ValidationError(_('Invalid value')) +class CustomStatusSerializerMixin(serializers.Serializer): + """Serializer mixin for models that support custom status values. + + Provides a `status_text` SerializerMethodField that resolves custom + status labels with a single database query per model per serializer + context (i.e. one query for a whole list page) rather than one query per + object (N+1). + """ + + status_text = serializers.SerializerMethodField() + + @extend_schema_field(serializers.CharField(allow_null=True)) + def get_status_text(self, instance) -> Optional[str]: + """Return the human-readable status text for the instance. + + Uses a per-context cache keyed by model name so that all objects in a + single serialization pass share one DB hit for custom label lookup. + + During write operations DRF may call to_representation on the raw + validated_data dict rather than a model instance (e.g. when building + response headers). Return None in that case — the response body is + always produced from a real instance via a separate serializer call. + """ + if not hasattr(instance, 'get_custom_status'): + return None + + custom_key = instance.get_custom_status() + + if custom_key is None: + return instance.status_class.label(instance.get_status()) + + model_name = instance._meta.model_name + cache_key = f'_custom_status_labels_{model_name}' + + # Cache a dict of custom status labels for this model, if not already cached + if cache_key not in self.context: + from common.models import InvenTreeCustomUserStateModel + + self.context[cache_key] = { + obj.key: obj.label + for obj in InvenTreeCustomUserStateModel.objects.filter( + model__model=model_name + ) + } + + return self.context[cache_key].get( + custom_key, instance.status_class.label(instance.get_status()) + ) + + class NotesFieldMixin: """Serializer mixin for handling 'notes' fields. diff --git a/src/backend/InvenTree/build/serializers.py b/src/backend/InvenTree/build/serializers.py index 888ad883bd..0db1cc0b1b 100644 --- a/src/backend/InvenTree/build/serializers.py +++ b/src/backend/InvenTree/build/serializers.py @@ -32,6 +32,7 @@ from common.settings import get_global_setting from generic.states.fields import InvenTreeCustomStatusSerializerMixin from InvenTree.mixins import DataImportExportSerializerMixin from InvenTree.serializers import ( + CustomStatusSerializerMixin, FilterableSerializerMixin, InvenTreeDecimalField, InvenTreeModelSerializer, @@ -53,6 +54,7 @@ from .status_codes import BuildStatus class BuildSerializer( + CustomStatusSerializerMixin, FilterableSerializerMixin, NotesFieldMixin, DataImportExportSerializerMixin, @@ -116,8 +118,6 @@ class BuildSerializer( level = serializers.IntegerField(label=_('Build Level'), read_only=True) - status_text = serializers.CharField(source='get_status_display', read_only=True) - part_detail = OptionalField( serializer_class=part_serializers.PartBriefSerializer, serializer_kwargs={'source': 'part', 'many': False, 'read_only': True}, diff --git a/src/backend/InvenTree/build/test_api.py b/src/backend/InvenTree/build/test_api.py index 5ba8824b3a..6050774e80 100644 --- a/src/backend/InvenTree/build/test_api.py +++ b/src/backend/InvenTree/build/test_api.py @@ -1817,6 +1817,97 @@ class BuildConsumeTest(BuildAPITest): self.assertEqual(line.consumed, 100) +class BuildCustomStatusTest(BuildAPITest): + """Tests for custom status values on Build orders.""" + + url = reverse('api-build-list') + + def test_custom_status_query_count(self): + """Test that listing Build orders with custom statuses does not cause N+1 queries. + + Ensures that resolving 'status_text' for custom status values is O(1) + in database queries, not O(N) relative to the number of results. + """ + from django.contrib.contenttypes.models import ContentType + + from common.models import InvenTreeCustomUserStateModel + + build_content_type = ContentType.objects.get_for_model(Build) + + # 10 custom status values - different keys, labels, and logical_keys + logical_keys = [ + BuildStatus.PENDING.value, + BuildStatus.PRODUCTION.value, + BuildStatus.ON_HOLD.value, + BuildStatus.CANCELLED.value, + BuildStatus.COMPLETE.value, + BuildStatus.PENDING.value, + BuildStatus.PRODUCTION.value, + BuildStatus.ON_HOLD.value, + BuildStatus.CANCELLED.value, + BuildStatus.COMPLETE.value, + ] + + custom_statuses = [ + InvenTreeCustomUserStateModel.objects.create( + key=3000 + i, + name=f'BuildCustomStatus{i}', + label=f'Build Custom Status Label {i}', + color='secondary', + logical_key=logical_keys[i], + model=build_content_type, + reference_status='BuildStatus', + ) + for i in range(10) + ] + + part = Part.objects.filter(assembly=True).first() + + # Build is an MPTT tree model; bulk_create requires tree fields to be + # populated manually. All new orders are root nodes (no parent) so each + # gets its own unique tree_id. + from django.db.models import Max + + next_tree_id = (Build.objects.aggregate(m=Max('tree_id'))['m'] or 0) + 1 + + # Create 100 build orders, cycling through the 10 custom statuses + Build.objects.bulk_create([ + Build( + part=part, + reference=f'BO-QTEST-{i}', + quantity=1, + status=custom_statuses[i % 10].logical_key, + status_custom_key=custom_statuses[i % 10].key, + lft=1, + rght=2, + level=0, + tree_id=next_tree_id + i, + ) + for i in range(100) + ]) + + # Lookup: custom_key -> custom_status_object, for quick per-row assertions + custom_lookup = {cs.key: cs for cs in custom_statuses} + + # Query count must stay below the fixed threshold regardless of limit. + # An N+1 bug would push limit=50 or limit=100 well over the threshold. + for limit in [1, 10, 50, 100]: + response = self.get( + self.url, data={'limit': limit}, expected_code=200, max_query_count=50 + ) + + for result in response.data['results']: + cs = custom_lookup.get(result['status_custom_key']) + + if cs is None: + # Build from fixtures - no custom status assigned + continue + + self.assertEqual(result['status'], cs.logical_key) + self.assertEqual(result['status_custom_key'], cs.key) + self.assertEqual(result['status_text'], cs.label) + + class BuildAutoAllocateAPITest(InvenTreeAPITestCase): """API integration tests for BuildAutoAllocate endpoint back-ports (stock_sort_by, build_lines).""" diff --git a/src/backend/InvenTree/order/serializers.py b/src/backend/InvenTree/order/serializers.py index 81c2a95fec..91b0c9d2d2 100644 --- a/src/backend/InvenTree/order/serializers.py +++ b/src/backend/InvenTree/order/serializers.py @@ -31,6 +31,7 @@ from importer.registry import register_importer from InvenTree.helpers import extract_serial_numbers, hash_barcode, normalize, str2bool from InvenTree.mixins import DataImportExportSerializerMixin from InvenTree.serializers import ( + CustomStatusSerializerMixin, FilterableSerializerMixin, InvenTreeCurrencySerializer, InvenTreeDecimalField, @@ -100,7 +101,10 @@ class DuplicateOrderSerializer(serializers.Serializer): class AbstractOrderSerializer( - DataImportExportSerializerMixin, FilterableSerializerMixin, serializers.Serializer + CustomStatusSerializerMixin, + DataImportExportSerializerMixin, + FilterableSerializerMixin, + serializers.Serializer, ): """Abstract serializer class which provides fields common to all order types.""" @@ -118,9 +122,6 @@ class AbstractOrderSerializer( read_only=True, allow_null=True, label=_('Completed Lines') ) - # Human-readable status text (read-only) - status_text = serializers.CharField(source='get_status_display', read_only=True) - # status field cannot be set directly status = serializers.IntegerField(read_only=True, label=_('Order Status')) diff --git a/src/backend/InvenTree/order/test_api.py b/src/backend/InvenTree/order/test_api.py index 5f686d6b5f..508d1849ee 100644 --- a/src/backend/InvenTree/order/test_api.py +++ b/src/backend/InvenTree/order/test_api.py @@ -16,7 +16,7 @@ from icalendar import Calendar from rest_framework import status from common.currency import currency_codes -from common.models import InvenTreeSetting +from common.models import InvenTreeCustomUserStateModel, InvenTreeSetting from common.settings import set_global_setting from company.models import Company, SupplierPart, SupplierPriceBreak from InvenTree.unit_test import InvenTreeAPITestCase @@ -737,6 +737,69 @@ class PurchaseOrderTest(OrderTest): ) self.assertEqual(response.status_code, 200) + def test_po_custom_status_query_count(self): + """Test that listing PurchaseOrders with custom statuses does not cause N+1 queries. + + Ensures that resolving the 'status_text' field for custom status values + is O(1) in database queries, not O(N) relative to the number of results. + """ + from django.contrib.contenttypes.models import ContentType + + po_content_type = ContentType.objects.get_for_model(models.PurchaseOrder) + + # 10 custom status values - different keys, labels, and logical_keys + logical_keys = [ + PurchaseOrderStatus.PENDING.value, + PurchaseOrderStatus.PLACED.value, + PurchaseOrderStatus.ON_HOLD.value, + PurchaseOrderStatus.COMPLETE.value, + PurchaseOrderStatus.CANCELLED.value, + PurchaseOrderStatus.LOST.value, + PurchaseOrderStatus.RETURNED.value, + PurchaseOrderStatus.PENDING.value, + PurchaseOrderStatus.PLACED.value, + PurchaseOrderStatus.ON_HOLD.value, + ] + + custom_statuses = [ + InvenTreeCustomUserStateModel.objects.create( + key=1000 + i, + name=f'PoCustomStatus{i}', + label=f'PO Custom Status Label {i}', + color='secondary', + logical_key=logical_keys[i], + model=po_content_type, + reference_status='PurchaseOrderStatus', + ) + for i in range(10) + ] + + # Create 100 purchase orders, cycling through the custom statuses + supplier = Company.objects.filter(is_supplier=True).first() + models.PurchaseOrder.objects.bulk_create([ + models.PurchaseOrder( + supplier=supplier, + reference=f'PO-QTEST-{i}', + status=custom_statuses[i % 10].logical_key, + status_custom_key=custom_statuses[i % 10].key, + ) + for i in range(100) + ]) + + # Query count must stay below the fixed threshold for all limit values. + # An N+1 bug would push limit=50 or limit=100 well over the threshold. + for limit in [1, 5, 10, 25, 50, 100]: + response = self.get( + self.LIST_URL, + data={'limit': limit}, + expected_code=200, + max_query_count=50, + ) + + for result in response.data['results']: + self.assertIn('status_text', result) + self.assertIsNotNone(result['status_text']) + class PurchaseOrderLineItemTest(OrderTest): """Unit tests for PurchaseOrderLineItems.""" @@ -771,7 +834,7 @@ class PurchaseOrderLineItemTest(OrderTest): # Try to delete a set of line items via their IDs self.delete(url, {'items': [1, 2]}, expected_code=200) - # We should have 2 less PurchaseOrderLineItems after deletign them + # We should have 2 less PurchaseOrderLineItems after deleting them self.assertEqual(models.PurchaseOrderLineItem.objects.count(), n - 2) def test_po_line_merge_pricing(self): diff --git a/src/backend/InvenTree/stock/serializers.py b/src/backend/InvenTree/stock/serializers.py index 31b400e18c..cefe76ab4f 100644 --- a/src/backend/InvenTree/stock/serializers.py +++ b/src/backend/InvenTree/stock/serializers.py @@ -33,6 +33,7 @@ from generic.states.fields import InvenTreeCustomStatusSerializerMixin from importer.registry import register_importer from InvenTree.mixins import DataImportExportSerializerMixin from InvenTree.serializers import ( + CustomStatusSerializerMixin, InvenTreeCurrencySerializer, InvenTreeDecimalField, OptionalField, @@ -308,6 +309,7 @@ class StockItemTestResultSerializer( @register_importer() class StockItemSerializer( + CustomStatusSerializerMixin, InvenTree.serializers.FilterableSerializerMixin, DataImportExportSerializerMixin, InvenTreeCustomStatusSerializerMixin, @@ -565,10 +567,6 @@ class StockItemSerializer( return queryset - status_text = serializers.CharField( - source='get_status_display', read_only=True, label=_('Status') - ) - SKU = serializers.CharField( source='supplier_part.SKU', read_only=True, diff --git a/src/backend/InvenTree/stock/test_api.py b/src/backend/InvenTree/stock/test_api.py index b8e1000f63..4299cfa83c 100644 --- a/src/backend/InvenTree/stock/test_api.py +++ b/src/backend/InvenTree/stock/test_api.py @@ -1411,6 +1411,82 @@ class CustomStockItemStatusTest(StockAPITestCase): self.assertEqual(status['value'], self.status.key) self.assertEqual(status['display_name'], self.status.label) + def test_custom_status_query_count(self): + """Test that listing StockItems with custom statuses does not cause N+1 queries. + + Ensures that resolving 'status_text' for custom status values is O(1) + in database queries, not O(N) relative to the number of results. + """ + stock_content_type = ContentType.objects.get(model='stockitem') + + # 10 custom status values - different keys, labels, and logical_keys + logical_keys = [ + StockStatus.OK.value, + StockStatus.ATTENTION.value, + StockStatus.DAMAGED.value, + StockStatus.DESTROYED.value, + StockStatus.REJECTED.value, + StockStatus.LOST.value, + StockStatus.QUARANTINED.value, + StockStatus.RETURNED.value, + StockStatus.OK.value, + StockStatus.ATTENTION.value, + ] + + custom_statuses = [ + InvenTreeCustomUserStateModel.objects.create( + key=2000 + i, + name=f'StockCustomStatus{i}', + label=f'Stock Custom Status Label {i}', + color='secondary', + logical_key=logical_keys[i], + model=stock_content_type, + reference_status='StockStatus', + ) + for i in range(10) + ] + + part = Part.objects.filter(active=True, virtual=False).first() + + # Create 500 stock items, cycling through the 10 custom statuses + StockItem.objects.bulk_create([ + StockItem( + part=part, + quantity=1, + level=0, + tree_id=0, + lft=0, + rght=0, + status=custom_statuses[i % 10].logical_key, + status_custom_key=custom_statuses[i % 10].key, + ) + for i in range(500) + ]) + + # Lookup: custom_key -> custom_status_object, for quick per-row assertions + custom_lookup = {cs.key: cs for cs in custom_statuses} + + # Query count must stay below the fixed threshold regardless of limit. + # An N+1 bug would push limit=100 or limit=500 well over the threshold. + for limit in [50, 100, 500]: + response = self.get( + self.list_url, + data={'limit': limit}, + expected_code=200, + max_query_count=50, + ) + + for result in response.data['results']: + cs = custom_lookup.get(result['status_custom_key']) + + if cs is None: + # Item from fixtures - no custom status assigned + continue + + self.assertEqual(result['status'], cs.logical_key) + self.assertEqual(result['status_custom_key'], cs.key) + self.assertEqual(result['status_text'], cs.label) + class StockItemTest(StockAPITestCase): """Series of API tests for the StockItem API."""