From a74b29f05984d50aef1dc449eb2f4bfda458755e Mon Sep 17 00:00:00 2001 From: Oliver Date: Mon, 19 Feb 2024 16:04:10 +1100 Subject: [PATCH] Fixes for test result updates (#6514) * Fix ordering of "stock" column in StockItemTable * Handle table.activeFilters - Can be passed undefined value in some cases * Fix legacy test result table - Add in "pass test" button * Improve logic for creating templates - Only look at ancestor parts *above* the existing part * Update migration - Only look above! * Improve matching in template * New data migration - Fixes (probably rare) edge case in previous data migration * Table tweak - Embolden required test templates * Add assertion check to data migration * Update API version - Add filter for "has_results" on the PartTestTemplate API endpoint * Logic fix --- InvenTree/InvenTree/api_version.py | 5 +- InvenTree/part/api.py | 10 ++ .../migrations/0106_auto_20240207_0353.py | 6 +- .../migrations/0108_auto_20240219_0252.py | 114 ++++++++++++++++++ InvenTree/stock/models.py | 4 +- InvenTree/stock/serializers.py | 4 +- InvenTree/templates/js/translated/stock.js | 3 +- .../src/tables/FilterSelectDrawer.tsx | 21 ++-- src/frontend/src/tables/InvenTreeTable.tsx | 12 +- .../src/tables/part/PartTestTemplateTable.tsx | 12 +- .../src/tables/stock/StockItemTable.tsx | 1 + 11 files changed, 173 insertions(+), 19 deletions(-) create mode 100644 InvenTree/stock/migrations/0108_auto_20240219_0252.py diff --git a/InvenTree/InvenTree/api_version.py b/InvenTree/InvenTree/api_version.py index 92048866a0..307c23899d 100644 --- a/InvenTree/InvenTree/api_version.py +++ b/InvenTree/InvenTree/api_version.py @@ -1,11 +1,14 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 169 +INVENTREE_API_VERSION = 170 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v170 -> 2024-02-19 : https://github.com/inventree/InvenTree/pull/6514 + - Adds "has_results" filter to the PartTestTemplate list endpoint + v169 -> 2024-02-14 : https://github.com/inventree/InvenTree/pull/6430 - Adds 'key' field to PartTestTemplate API endpoint - Adds annotated 'results' field to PartTestTemplate API endpoint diff --git a/InvenTree/part/api.py b/InvenTree/part/api.py index ccfa5676b3..b1c4f3cce2 100644 --- a/InvenTree/part/api.py +++ b/InvenTree/part/api.py @@ -398,6 +398,16 @@ class PartTestTemplateFilter(rest_filters.FilterSet): else: return queryset.filter(part=part) + has_results = rest_filters.BooleanFilter( + label=_('Has Results'), method='filter_has_results' + ) + + def filter_has_results(self, queryset, name, value): + """Filter by whether the PartTestTemplate has any associated test results.""" + if str2bool(value): + return queryset.exclude(results=0) + return queryset.filter(results=0) + class PartTestTemplateMixin: """Mixin class for the PartTestTemplate API endpoints.""" diff --git a/InvenTree/stock/migrations/0106_auto_20240207_0353.py b/InvenTree/stock/migrations/0106_auto_20240207_0353.py index 2337963424..c081c68f62 100644 --- a/InvenTree/stock/migrations/0106_auto_20240207_0353.py +++ b/InvenTree/stock/migrations/0106_auto_20240207_0353.py @@ -61,7 +61,11 @@ def set_template(apps, schema_editor): # We have found an existing template for this test pass - elif template := PartTestTemplate.objects.filter(part__tree_id=part.tree_id, test_name__iexact=name).first(): + elif template := PartTestTemplate.objects.filter( + part__tree_id=part.tree_id, + part__lft__lte=part.lft, + part__rght__gte=part.rght, + key=key).first(): # We have found an existing template for this test pass diff --git a/InvenTree/stock/migrations/0108_auto_20240219_0252.py b/InvenTree/stock/migrations/0108_auto_20240219_0252.py new file mode 100644 index 0000000000..60217577c0 --- /dev/null +++ b/InvenTree/stock/migrations/0108_auto_20240219_0252.py @@ -0,0 +1,114 @@ +# Generated by Django 4.2.10 on 2024-02-19 02:52 + +from django.db import migrations +from django.db.models import F, OuterRef, Subquery, IntegerField + + +def update_templates(apps, schema_editor): + """Run data migration to fix potentially mis-applied data migration. + + Ref: https://github.com/inventree/InvenTree/pull/6514 + + The previous data migration (stock.0106_auto_20240207_0353) had a bug, + where it would look for any matching PartTestTemplate objects for a given StockItemTestResult, + as long as the "part tree ID" was the same. + + However, if the template was defined for a part on a different *branch* of the tree, + the wrong template could be applied. + + This is really only the case where the user has a very complex set of nested part variants, + but still there is a potential for a mis-match. + + This data migration will attempt to fix any mis-applied templates. + """ + + PartTestTemplate = apps.get_model('part', 'PartTestTemplate') + StockItemTestResult = apps.get_model('stock', 'StockItemTestResult') + + # Find any StockItemTestResult objects which match a "bad" template + # Here a "bad" template points to a Part which is not *above* the part in the tree + bad_results = StockItemTestResult.objects.exclude( + stock_item__part__tree_id=F('template__part__tree_id'), + stock_item__part__lft__gte=F('template__part__lft'), + stock_item__part__rght__lte=F('template__part__rght'), + ) + + n = bad_results.count() + + if n == 0: + # Escape early - no bad results! + return + + print(f"Found {n} StockItemTestResult objects with bad templates...") + + # For each bad result, attempt to find a matching template + # Here, a matching template must point to a part *above* the part in the tree + # Annotate the queryset with a "mathching template" + + template_query = PartTestTemplate.objects.filter( + part__tree_id=OuterRef('stock_item__part__tree_id'), + part__lft__lte=OuterRef('stock_item__part__lft'), + part__rght__gte=OuterRef('stock_item__part__rght'), + key=OuterRef('template__key') + ).order_by('part__level').values('pk') + + bad_results = bad_results.annotate( + matching_template=Subquery(template_query[:1], output_field=IntegerField()) + ) + + # Update the results for which we have a "good" matching template + matching_results = bad_results.filter(matching_template__isnull=False) + missing_results = bad_results.filter(matching_template__isnull=True) + + results_to_update = [] + + for result in matching_results: + if result.template.pk != result.matching_template: + result.template = PartTestTemplate.objects.get(pk=result.matching_template) + results_to_update.append(result) + + if len(results_to_update) > 0: + # Update any results which point to the wrong template, but have a matching template + print("Updating", len(results_to_update), "matching templates...") + StockItemTestResult.objects.bulk_update(results_to_update, ['template']) + + results_to_update = [] + + # For the remaining results, we need to create a new template + for result in missing_results: + # Check that a template does *not* exist already + if template := PartTestTemplate.objects.filter( + part__tree_id=result.stock_item.part.tree_id, + part__lft__lte=result.stock_item.part.lft, + part__rght__gte=result.stock_item.part.rght, + key=result.template.key + ).first(): + pass + else: + # Create a new template (by copying the old one) + template = result.template + template.part = result.stock_item.part + template.pk = None + template.save() + template.refresh_from_db() + + result.template = template + results_to_update.append(result) + + if len(results_to_update) > 0: + print("Updating", len(results_to_update), "missing templates...") + StockItemTestResult.objects.bulk_update(results_to_update, ['template']) + + # Finall, check that there are no longer any "bad" results + assert(bad_results.order_by('pk').count() == 0) + + +class Migration(migrations.Migration): + + dependencies = [ + ('stock', '0107_remove_stockitemtestresult_test_and_more'), + ] + + operations = [ + migrations.RunPython(update_templates, reverse_code=migrations.RunPython.noop) + ] diff --git a/InvenTree/stock/models.py b/InvenTree/stock/models.py index a894f1a3fe..517345b243 100644 --- a/InvenTree/stock/models.py +++ b/InvenTree/stock/models.py @@ -1577,8 +1577,10 @@ class StockItem( if template is None and test_name is not None: # Attempt to find a matching template + ancestors = self.part.get_ancestors(include_self=True) + template = PartModels.PartTestTemplate.objects.filter( - part__tree_id=self.part.tree_id, key=test_key + part__tree_id=self.part.tree_id, part__in=ancestors, key=test_key ).first() if template is None: diff --git a/InvenTree/stock/serializers.py b/InvenTree/stock/serializers.py index 8864526dce..87b0090e21 100644 --- a/InvenTree/stock/serializers.py +++ b/InvenTree/stock/serializers.py @@ -117,9 +117,11 @@ class StockItemTestResultSerializer(InvenTree.serializers.InvenTreeModelSerializ if not template: test_key = InvenTree.helpers.generateTestKey(test_name) + ancestors = stock_item.part.get_ancestors(include_self=True) + # Find a template based on name if template := part_models.PartTestTemplate.objects.filter( - part__tree_id=stock_item.part.tree_id, key=test_key + part__tree_id=stock_item.part.tree_id, part__in=ancestors, key=test_key ).first(): data['template'] = template diff --git a/InvenTree/templates/js/translated/stock.js b/InvenTree/templates/js/translated/stock.js index 4594bda33b..0b442fecc1 100644 --- a/InvenTree/templates/js/translated/stock.js +++ b/InvenTree/templates/js/translated/stock.js @@ -1433,7 +1433,8 @@ function loadStockTestResultsTable(table, options) { let html = ''; - if (row.parent != parent_node && row.requires_attachment == false && row.requires_value == false && !row.result) { + + if (row.requires_attachment == false && row.requires_value == false && !row.result) { // Enable a "quick tick" option for this test result html += makeIconButton('fa-check-circle icon-green', 'button-test-tick', row.test_name, '{% trans "Pass test" %}'); } diff --git a/src/frontend/src/tables/FilterSelectDrawer.tsx b/src/frontend/src/tables/FilterSelectDrawer.tsx index c95c814094..9af7e8684e 100644 --- a/src/frontend/src/tables/FilterSelectDrawer.tsx +++ b/src/frontend/src/tables/FilterSelectDrawer.tsx @@ -83,7 +83,8 @@ function FilterAddGroup({ availableFilters: TableFilter[]; }) { const filterOptions = useMemo(() => { - let activeFilterNames = tableState.activeFilters.map((flt) => flt.name); + let activeFilterNames = + tableState.activeFilters?.map((flt) => flt.name) ?? []; return availableFilters .filter((flt) => !activeFilterNames.includes(flt.name)) @@ -120,9 +121,10 @@ function FilterAddGroup({ return; } - let filters = tableState.activeFilters.filter( - (flt) => flt.name !== selectedFilter - ); + let filters = + tableState.activeFilters?.filter( + (flt) => flt.name !== selectedFilter + ) ?? []; let newFilter: TableFilter = { ...filter, @@ -188,10 +190,13 @@ export function FilterSelectDrawer({ title={{t`Table Filters`}} > - {tableState.activeFilters.map((f) => ( - - ))} - {tableState.activeFilters.length > 0 && } + {tableState.activeFilters && + tableState.activeFilters.map((f) => ( + + ))} + {tableState.activeFilters && tableState.activeFilters.length > 0 && ( + + )} {addFilter && ( ({ }; // Add custom filters - tableState.activeFilters.forEach( - (flt) => (queryParams[flt.name] = flt.value) - ); + if (tableState.activeFilters) { + tableState.activeFilters.forEach( + (flt) => (queryParams[flt.name] = flt.value) + ); + } // Add custom search term if (tableState.searchTerm) { @@ -560,8 +562,8 @@ export function InvenTreeTable({ {tableProps.enableFilters && filters.length > 0 && ( diff --git a/src/frontend/src/tables/part/PartTestTemplateTable.tsx b/src/frontend/src/tables/part/PartTestTemplateTable.tsx index 3b9a970f48..ed96fb20ae 100644 --- a/src/frontend/src/tables/part/PartTestTemplateTable.tsx +++ b/src/frontend/src/tables/part/PartTestTemplateTable.tsx @@ -33,7 +33,12 @@ export default function PartTestTemplateTable({ partId }: { partId: number }) { { accessor: 'test_name', switchable: false, - sortable: true + sortable: true, + render: (record: any) => { + return ( + {record.test_name} + ); + } }, { accessor: 'results', @@ -77,6 +82,11 @@ export default function PartTestTemplateTable({ partId }: { partId: number }) { name: 'include_inherited', label: t`Include Inherited`, description: t`Show tests from inherited templates` + }, + { + name: 'has_results', + label: t`Has Results`, + description: t`Show tests which have recorded results` } ]; }, []); diff --git a/src/frontend/src/tables/stock/StockItemTable.tsx b/src/frontend/src/tables/stock/StockItemTable.tsx index b99416ff59..ee477e2f0a 100644 --- a/src/frontend/src/tables/stock/StockItemTable.tsx +++ b/src/frontend/src/tables/stock/StockItemTable.tsx @@ -34,6 +34,7 @@ function stockItemTableColumns(): TableColumn[] { }), { accessor: 'quantity', + ordering: 'stock', sortable: true, title: t`Stock`, render: (record) => {