mirror of
				https://github.com/inventree/InvenTree.git
				synced 2025-10-31 13:15:43 +00:00 
			
		
		
		
	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
This commit is contained in:
		| @@ -1,11 +1,14 @@ | |||||||
| """InvenTree API version information.""" | """InvenTree API version information.""" | ||||||
|  |  | ||||||
| # InvenTree API version | # 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.""" | """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" | ||||||
|  |  | ||||||
| INVENTREE_API_TEXT = """ | 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 | v169 -> 2024-02-14 : https://github.com/inventree/InvenTree/pull/6430 | ||||||
|     - Adds 'key' field to PartTestTemplate API endpoint |     - Adds 'key' field to PartTestTemplate API endpoint | ||||||
|     - Adds annotated 'results' field to PartTestTemplate API endpoint |     - Adds annotated 'results' field to PartTestTemplate API endpoint | ||||||
|   | |||||||
| @@ -398,6 +398,16 @@ class PartTestTemplateFilter(rest_filters.FilterSet): | |||||||
|         else: |         else: | ||||||
|             return queryset.filter(part=part) |             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: | class PartTestTemplateMixin: | ||||||
|     """Mixin class for the PartTestTemplate API endpoints.""" |     """Mixin class for the PartTestTemplate API endpoints.""" | ||||||
|   | |||||||
| @@ -61,7 +61,11 @@ def set_template(apps, schema_editor): | |||||||
|                 # We have found an existing template for this test |                 # We have found an existing template for this test | ||||||
|                 pass |                 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 |                 # We have found an existing template for this test | ||||||
|                 pass |                 pass | ||||||
|  |  | ||||||
|   | |||||||
							
								
								
									
										114
									
								
								InvenTree/stock/migrations/0108_auto_20240219_0252.py
									
									
									
									
									
										Normal file
									
								
							
							
						
						
									
										114
									
								
								InvenTree/stock/migrations/0108_auto_20240219_0252.py
									
									
									
									
									
										Normal file
									
								
							| @@ -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) | ||||||
|  |     ] | ||||||
| @@ -1577,8 +1577,10 @@ class StockItem( | |||||||
|         if template is None and test_name is not None: |         if template is None and test_name is not None: | ||||||
|             # Attempt to find a matching template |             # Attempt to find a matching template | ||||||
|  |  | ||||||
|  |             ancestors = self.part.get_ancestors(include_self=True) | ||||||
|  |  | ||||||
|             template = PartModels.PartTestTemplate.objects.filter( |             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() |             ).first() | ||||||
|  |  | ||||||
|             if template is None: |             if template is None: | ||||||
|   | |||||||
| @@ -117,9 +117,11 @@ class StockItemTestResultSerializer(InvenTree.serializers.InvenTreeModelSerializ | |||||||
|         if not template: |         if not template: | ||||||
|             test_key = InvenTree.helpers.generateTestKey(test_name) |             test_key = InvenTree.helpers.generateTestKey(test_name) | ||||||
|  |  | ||||||
|  |             ancestors = stock_item.part.get_ancestors(include_self=True) | ||||||
|  |  | ||||||
|             # Find a template based on name |             # Find a template based on name | ||||||
|             if template := part_models.PartTestTemplate.objects.filter( |             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(): |             ).first(): | ||||||
|                 data['template'] = template |                 data['template'] = template | ||||||
|  |  | ||||||
|   | |||||||
| @@ -1433,7 +1433,8 @@ function loadStockTestResultsTable(table, options) { | |||||||
|  |  | ||||||
|         let html = ''; |         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 |             // 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" %}'); |             html += makeIconButton('fa-check-circle icon-green', 'button-test-tick', row.test_name, '{% trans "Pass test" %}'); | ||||||
|         } |         } | ||||||
|   | |||||||
| @@ -83,7 +83,8 @@ function FilterAddGroup({ | |||||||
|   availableFilters: TableFilter[]; |   availableFilters: TableFilter[]; | ||||||
| }) { | }) { | ||||||
|   const filterOptions = useMemo(() => { |   const filterOptions = useMemo(() => { | ||||||
|     let activeFilterNames = tableState.activeFilters.map((flt) => flt.name); |     let activeFilterNames = | ||||||
|  |       tableState.activeFilters?.map((flt) => flt.name) ?? []; | ||||||
|  |  | ||||||
|     return availableFilters |     return availableFilters | ||||||
|       .filter((flt) => !activeFilterNames.includes(flt.name)) |       .filter((flt) => !activeFilterNames.includes(flt.name)) | ||||||
| @@ -120,9 +121,10 @@ function FilterAddGroup({ | |||||||
|         return; |         return; | ||||||
|       } |       } | ||||||
|  |  | ||||||
|       let filters = tableState.activeFilters.filter( |       let filters = | ||||||
|  |         tableState.activeFilters?.filter( | ||||||
|           (flt) => flt.name !== selectedFilter |           (flt) => flt.name !== selectedFilter | ||||||
|       ); |         ) ?? []; | ||||||
|  |  | ||||||
|       let newFilter: TableFilter = { |       let newFilter: TableFilter = { | ||||||
|         ...filter, |         ...filter, | ||||||
| @@ -188,10 +190,13 @@ export function FilterSelectDrawer({ | |||||||
|       title={<StylishText size="lg">{t`Table Filters`}</StylishText>} |       title={<StylishText size="lg">{t`Table Filters`}</StylishText>} | ||||||
|     > |     > | ||||||
|       <Stack spacing="xs"> |       <Stack spacing="xs"> | ||||||
|         {tableState.activeFilters.map((f) => ( |         {tableState.activeFilters && | ||||||
|  |           tableState.activeFilters.map((f) => ( | ||||||
|             <FilterItem key={f.name} flt={f} tableState={tableState} /> |             <FilterItem key={f.name} flt={f} tableState={tableState} /> | ||||||
|           ))} |           ))} | ||||||
|         {tableState.activeFilters.length > 0 && <Divider />} |         {tableState.activeFilters && tableState.activeFilters.length > 0 && ( | ||||||
|  |           <Divider /> | ||||||
|  |         )} | ||||||
|         {addFilter && ( |         {addFilter && ( | ||||||
|           <Stack spacing="xs"> |           <Stack spacing="xs"> | ||||||
|             <FilterAddGroup |             <FilterAddGroup | ||||||
|   | |||||||
| @@ -278,9 +278,11 @@ export function InvenTreeTable<T = any>({ | |||||||
|     }; |     }; | ||||||
|  |  | ||||||
|     // Add custom filters |     // Add custom filters | ||||||
|  |     if (tableState.activeFilters) { | ||||||
|       tableState.activeFilters.forEach( |       tableState.activeFilters.forEach( | ||||||
|         (flt) => (queryParams[flt.name] = flt.value) |         (flt) => (queryParams[flt.name] = flt.value) | ||||||
|       ); |       ); | ||||||
|  |     } | ||||||
|  |  | ||||||
|     // Add custom search term |     // Add custom search term | ||||||
|     if (tableState.searchTerm) { |     if (tableState.searchTerm) { | ||||||
| @@ -560,8 +562,8 @@ export function InvenTreeTable<T = any>({ | |||||||
|             {tableProps.enableFilters && filters.length > 0 && ( |             {tableProps.enableFilters && filters.length > 0 && ( | ||||||
|               <Indicator |               <Indicator | ||||||
|                 size="xs" |                 size="xs" | ||||||
|                 label={tableState.activeFilters.length} |                 label={tableState.activeFilters?.length ?? 0} | ||||||
|                 disabled={tableState.activeFilters.length == 0} |                 disabled={tableState.activeFilters?.length == 0} | ||||||
|               > |               > | ||||||
|                 <ActionIcon> |                 <ActionIcon> | ||||||
|                   <Tooltip label={t`Table filters`}> |                   <Tooltip label={t`Table filters`}> | ||||||
|   | |||||||
| @@ -33,7 +33,12 @@ export default function PartTestTemplateTable({ partId }: { partId: number }) { | |||||||
|       { |       { | ||||||
|         accessor: 'test_name', |         accessor: 'test_name', | ||||||
|         switchable: false, |         switchable: false, | ||||||
|         sortable: true |         sortable: true, | ||||||
|  |         render: (record: any) => { | ||||||
|  |           return ( | ||||||
|  |             <Text weight={record.required && 700}>{record.test_name}</Text> | ||||||
|  |           ); | ||||||
|  |         } | ||||||
|       }, |       }, | ||||||
|       { |       { | ||||||
|         accessor: 'results', |         accessor: 'results', | ||||||
| @@ -77,6 +82,11 @@ export default function PartTestTemplateTable({ partId }: { partId: number }) { | |||||||
|         name: 'include_inherited', |         name: 'include_inherited', | ||||||
|         label: t`Include Inherited`, |         label: t`Include Inherited`, | ||||||
|         description: t`Show tests from inherited templates` |         description: t`Show tests from inherited templates` | ||||||
|  |       }, | ||||||
|  |       { | ||||||
|  |         name: 'has_results', | ||||||
|  |         label: t`Has Results`, | ||||||
|  |         description: t`Show tests which have recorded results` | ||||||
|       } |       } | ||||||
|     ]; |     ]; | ||||||
|   }, []); |   }, []); | ||||||
|   | |||||||
| @@ -34,6 +34,7 @@ function stockItemTableColumns(): TableColumn[] { | |||||||
|     }), |     }), | ||||||
|     { |     { | ||||||
|       accessor: 'quantity', |       accessor: 'quantity', | ||||||
|  |       ordering: 'stock', | ||||||
|       sortable: true, |       sortable: true, | ||||||
|       title: t`Stock`, |       title: t`Stock`, | ||||||
|       render: (record) => { |       render: (record) => { | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user