From 0bfbd45cece8ae7e68b077d61438b70c26ab5a12 Mon Sep 17 00:00:00 2001 From: Oliver Date: Wed, 21 Feb 2024 23:34:20 +1100 Subject: [PATCH] API Tree filters (#6536) * PartCategoryList: Add FilterSet class * Update PartCategory filters - Migrate all custom filtering to the FilterSet * Logic updates * Revert deleted code * Implement similar filters for StockLocation list API * Update API docs * Unit test updates * More fix * Fix for StockLocation filterclass * Fix PUI tables * Cleanup CUI tables * Updated unit tests --- InvenTree/InvenTree/api_version.py | 6 +- InvenTree/part/api.py | 199 +++++++++++------- InvenTree/part/templates/part/category.html | 2 - InvenTree/part/test_api.py | 54 ++--- InvenTree/stock/api.py | 143 +++++++------ InvenTree/stock/serializers.py | 2 +- InvenTree/stock/templates/stock/location.html | 2 - InvenTree/stock/test_api.py | 53 ++--- .../src/tables/part/PartCategoryTable.tsx | 2 +- .../src/tables/stock/StockLocationTable.tsx | 2 +- 10 files changed, 243 insertions(+), 222 deletions(-) diff --git a/InvenTree/InvenTree/api_version.py b/InvenTree/InvenTree/api_version.py index e6e7e8dc9b..5f47313629 100644 --- a/InvenTree/InvenTree/api_version.py +++ b/InvenTree/InvenTree/api_version.py @@ -1,11 +1,15 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 173 +INVENTREE_API_VERSION = 174 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v174 - 2024-02-21 : https://github.com/inventree/InvenTree/pull/6536 + - Expose PartCategory filters to the API documentation + - Expose StockLocation filters to the API documentation + v173 - 2024-02-20 : https://github.com/inventree/InvenTree/pull/6483 - Adds "merge_items" to the PurchaseOrderLine create API endpoint - Adds "auto_pricing" to the PurchaseOrderLine create/update API endpoint diff --git a/InvenTree/part/api.py b/InvenTree/part/api.py index 6f4f612481..7ecec1a67b 100644 --- a/InvenTree/part/api.py +++ b/InvenTree/part/api.py @@ -105,6 +105,126 @@ class CategoryMixin: return ctx +class CategoryFilter(rest_filters.FilterSet): + """Custom filterset class for the PartCategoryList endpoint.""" + + class Meta: + """Metaclass options for this filterset.""" + + model = PartCategory + fields = ['name', 'structural'] + + starred = rest_filters.BooleanFilter( + label=_('Starred'), + method='filter_starred', + help_text=_('Filter by starred categories'), + ) + + def filter_starred(self, queryset, name, value): + """Filter by whether the PartCategory is starred by the current user.""" + user = self.request.user + + starred_categories = [ + star.category.pk for star in user.starred_categories.all() + ] + + if str2bool(value): + return queryset.filter(pk__in=starred_categories) + + return queryset.exclude(pk__in=starred_categories) + + depth = rest_filters.NumberFilter( + label=_('Depth'), method='filter_depth', help_text=_('Filter by category depth') + ) + + def filter_depth(self, queryset, name, value): + """Filter by the "depth" of the PartCategory. + + - This filter is used to limit the depth of the category tree + - If the "parent" filter is also provided, the depth is calculated from the parent category + """ + parent = self.data.get('parent', None) + + # Only filter if the parent filter is *not* provided + if not parent: + queryset = queryset.filter(level__lte=value) + + return queryset + + cascade = rest_filters.BooleanFilter( + label=_('Cascade'), + method='filter_cascade', + help_text=_('Include sub-categories in filtered results'), + ) + + def filter_cascade(self, queryset, name, value): + """Filter by whether to include sub-categories in the filtered results. + + Note: If the "parent" filter is provided, we offload the logic to that method. + """ + parent = self.data.get('parent', None) + + # If the parent is *not* provided, update the results based on the "cascade" value + if not parent: + if not value: + # If "cascade" is False, only return top-level categories + queryset = queryset.filter(parent=None) + + return queryset + + parent = rest_filters.ModelChoiceFilter( + queryset=PartCategory.objects.all(), + label=_('Parent'), + method='filter_parent', + help_text=_('Filter by parent category'), + ) + + def filter_parent(self, queryset, name, value): + """Filter by parent category. + + Note that the filtering behaviour here varies, + depending on whether the 'cascade' value is set. + + So, we have to check the "cascade" value here. + """ + parent = value + depth = self.data.get('depth', None) + cascade = str2bool(self.data.get('cascade', False)) + + if cascade: + # Return recursive subcategories + queryset = queryset.filter( + parent__in=parent.get_descendants(include_self=True) + ) + else: + # Return only direct children + queryset = queryset.filter(parent=parent) + + if depth is not None: + # Filter by depth from parent + depth = int(depth) + queryset = queryset.filter(level__lte=parent.level + depth) + + return queryset + + exclude_tree = rest_filters.ModelChoiceFilter( + queryset=PartCategory.objects.all(), + label=_('Exclude Tree'), + method='filter_exclude_tree', + help_text=_('Exclude sub-categories under the specified category'), + ) + + def filter_exclude_tree(self, queryset, name, value): + """Exclude all sub-categories under the specified category.""" + # Exclude the specified category + queryset = queryset.exclude(pk=value.pk) + + # Exclude any sub-categories also + queryset = queryset.exclude(parent__in=value.get_descendants(include_self=True)) + + return queryset + + class CategoryList(CategoryMixin, APIDownloadMixin, ListCreateAPI): """API endpoint for accessing a list of PartCategory objects. @@ -112,6 +232,8 @@ class CategoryList(CategoryMixin, APIDownloadMixin, ListCreateAPI): - POST: Create a new PartCategory object """ + filterset_class = CategoryFilter + def download_queryset(self, queryset, export_format): """Download the filtered queryset as a data file.""" dataset = PartCategoryResource().export(queryset=queryset) @@ -120,85 +242,8 @@ class CategoryList(CategoryMixin, APIDownloadMixin, ListCreateAPI): return DownloadFile(filedata, filename) - def filter_queryset(self, queryset): - """Custom filtering. - - Rules: - - Allow filtering by "null" parent to retrieve top-level part categories - """ - queryset = super().filter_queryset(queryset) - - params = self.request.query_params - - cat_id = params.get('parent', None) - - cascade = str2bool(params.get('cascade', False)) - - depth = str2int(params.get('depth', None)) - - # Do not filter by category - if cat_id is None: - pass - # Look for top-level categories - elif isNull(cat_id): - if not cascade: - queryset = queryset.filter(parent=None) - - if cascade and depth is not None: - queryset = queryset.filter(level__lte=depth) - - else: - try: - category = PartCategory.objects.get(pk=cat_id) - - if cascade: - parents = category.get_descendants(include_self=True) - if depth is not None: - parents = parents.filter(level__lte=category.level + depth) - - parent_ids = [p.id for p in parents] - - queryset = queryset.filter(parent__in=parent_ids) - else: - queryset = queryset.filter(parent=category) - - except (ValueError, PartCategory.DoesNotExist): - pass - - # Exclude PartCategory tree - exclude_tree = params.get('exclude_tree', None) - - if exclude_tree is not None: - try: - cat = PartCategory.objects.get(pk=exclude_tree) - - queryset = queryset.exclude( - pk__in=[c.pk for c in cat.get_descendants(include_self=True)] - ) - - except (ValueError, PartCategory.DoesNotExist): - pass - - # Filter by "starred" status - starred = params.get('starred', None) - - if starred is not None: - starred = str2bool(starred) - starred_categories = [ - star.category.pk for star in self.request.user.starred_categories.all() - ] - - if starred: - queryset = queryset.filter(pk__in=starred_categories) - else: - queryset = queryset.exclude(pk__in=starred_categories) - - return queryset - filter_backends = SEARCH_ORDER_FILTER - filterset_fields = ['name', 'description', 'structural'] - ordering_fields = ['name', 'pathstring', 'level', 'tree_id', 'lft', 'part_count'] # Use hierarchical ordering by default diff --git a/InvenTree/part/templates/part/category.html b/InvenTree/part/templates/part/category.html index cfa1f17023..734e3ae98b 100644 --- a/InvenTree/part/templates/part/category.html +++ b/InvenTree/part/templates/part/category.html @@ -317,8 +317,6 @@ params: { {% if category %} parent: {{ category.pk }}, - {% else %} - parent: null, {% endif %} }, allowTreeView: true, diff --git a/InvenTree/part/test_api.py b/InvenTree/part/test_api.py index 5cc16e74c9..a9146b2f57 100644 --- a/InvenTree/part/test_api.py +++ b/InvenTree/part/test_api.py @@ -77,59 +77,28 @@ class PartCategoryAPITest(InvenTreeAPITestCase): ({}, 8, 'no parameters'), ({'parent': 1, 'cascade': False}, 3, 'Filter by parent, no cascading'), ({'parent': 1, 'cascade': True}, 5, 'Filter by parent, cascading'), - ({'cascade': True, 'depth': 0}, 8, 'Cascade with no parent, depth=0'), - ({'cascade': False, 'depth': 10}, 8, 'Cascade with no parent, depth=0'), + ({'cascade': True, 'depth': 0}, 2, 'Cascade with no parent, depth=0'), + ({'cascade': False, 'depth': 10}, 2, 'Cascade with no parent, depth=10'), ( - {'parent': 'null', 'cascade': True, 'depth': 0}, - 2, - 'Cascade with null parent, depth=0', - ), - ( - {'parent': 'null', 'cascade': True, 'depth': 10}, + {'cascade': True, 'depth': 10}, 8, 'Cascade with null parent and bigger depth', ), ( - {'parent': 'null', 'cascade': False, 'depth': 10}, - 2, - 'No cascade even with depth specified with null parent', - ), - ( - {'parent': 1, 'cascade': False, 'depth': 0}, + {'parent': 1, 'cascade': False, 'depth': 1}, 3, 'Dont cascade with depth=0 and parent', ), ( {'parent': 1, 'cascade': True, 'depth': 0}, - 3, + 0, 'Cascade with depth=0 and parent', ), ( - {'parent': 1, 'cascade': False, 'depth': 1}, - 3, - 'Dont cascade even with depth=1 specified with parent', - ), - ( - {'parent': 1, 'cascade': True, 'depth': 1}, + {'parent': 1, 'cascade': True, 'depth': 2}, 5, 'Cascade with depth=1 with parent', ), - ( - {'parent': 1, 'cascade': True, 'depth': 'abcdefg'}, - 5, - 'Cascade with invalid depth and parent', - ), - ({'parent': 42}, 8, 'Should return everything if parent_pk is not valid'), - ( - {'parent': 'null', 'exclude_tree': 1, 'cascade': True}, - 2, - 'Should return everything from except tree with pk=1', - ), - ( - {'parent': 'null', 'exclude_tree': 42, 'cascade': True}, - 8, - 'Should return everything because exclude_tree=42 is no valid pk', - ), ( {'parent': 1, 'starred': True, 'cascade': True}, 2, @@ -146,6 +115,15 @@ class PartCategoryAPITest(InvenTreeAPITestCase): response = self.get(url, params, expected_code=200) self.assertEqual(len(response.data), res_len, description) + # The following tests should fail (return 400 code) + test_cases = [ + {'parent': 1, 'cascade': True, 'depth': 'abcdefg'}, # Invalid depth value + {'parent': -42}, # Invalid parent + ] + + for params in test_cases: + response = self.get(url, params, expected_code=400) + # Check that the required fields are present fields = [ 'pk', @@ -631,7 +609,7 @@ class PartAPITest(PartAPITestBase): self.assertEqual(len(response.data), 8) # Request top-level part categories only - response = self.get(url, {'parent': 'null'}) + response = self.get(url, {'cascade': False}) self.assertEqual(len(response.data), 2) diff --git a/InvenTree/stock/api.py b/InvenTree/stock/api.py index a6bb0c9c41..459c5bf1f5 100644 --- a/InvenTree/stock/api.py +++ b/InvenTree/stock/api.py @@ -257,6 +257,12 @@ class StockMerge(CreateAPI): class StockLocationFilter(rest_filters.FilterSet): """Base class for custom API filters for the StockLocation endpoint.""" + class Meta: + """Meta class options for this filterset.""" + + model = StockLocation + fields = ['name', 'structural', 'external'] + location_type = rest_filters.ModelChoiceFilter( queryset=StockLocationType.objects.all(), field_name='location_type' ) @@ -271,6 +277,80 @@ class StockLocationFilter(rest_filters.FilterSet): return queryset.exclude(location_type=None) return queryset.filter(location_type=None) + depth = rest_filters.NumberFilter( + label=_('Depth'), method='filter_depth', help_text=_('Filter by location depth') + ) + + def filter_depth(self, queryset, name, value): + """Filter by the "depth" of the StockLocation. + + - This filter is used to limit the depth of the location tree + - If the "parent" filter is also provided, the depth is calculated from the parent location + """ + parent = self.data.get('parent', None) + + # Only filter if the parent filter is *not* provided + if not parent: + queryset = queryset.filter(level__lte=value) + + return queryset + + cascade = rest_filters.BooleanFilter( + label=_('Cascade'), + method='filter_cascade', + help_text=_('Include sub-locations in filtered results'), + ) + + def filter_cascade(self, queryset, name, value): + """Filter by whether to include sub-locations in the filtered results. + + Note: If the "parent" filter is provided, we offload the logic to that method. + """ + parent = self.data.get('parent', None) + + # If the parent is *not* provided, update the results based on the "cascade" value + if not parent: + if not value: + # If "cascade" is False, only return top-level location + queryset = queryset.filter(parent=None) + + return queryset + + parent = rest_filters.ModelChoiceFilter( + queryset=StockLocation.objects.all(), + method='filter_parent', + label=_('Parent Location'), + help_text=_('Filter by parent location'), + ) + + def filter_parent(self, queryset, name, value): + """Filter by parent location. + + Note that the filtering behaviour here varies, + depending on whether the 'cascade' value is set. + + So, we have to check the "cascade" value here. + """ + parent = value + depth = self.data.get('depth', None) + cascade = str2bool(self.data.get('cascade', False)) + + if cascade: + # Return recursive sub-locations + queryset = queryset.filter( + parent__in=parent.get_descendants(include_self=True) + ) + else: + # Return only direct children + queryset = queryset.filter(parent=parent) + + if depth is not None: + # Filter by depth from parent + depth = int(depth) + queryset = queryset.filter(level__lte=parent.level + depth) + + return queryset + class StockLocationList(APIDownloadMixin, ListCreateAPI): """API endpoint for list view of StockLocation objects. @@ -297,71 +377,8 @@ class StockLocationList(APIDownloadMixin, ListCreateAPI): queryset = StockSerializers.LocationSerializer.annotate_queryset(queryset) return queryset - def filter_queryset(self, queryset): - """Custom filtering: - Allow filtering by "null" parent to retrieve top-level stock locations.""" - queryset = super().filter_queryset(queryset) - - params = self.request.query_params - - loc_id = params.get('parent', None) - - cascade = str2bool(params.get('cascade', False)) - - depth = str2int(params.get('depth', None)) - - # Do not filter by location - if loc_id is None: - pass - # Look for top-level locations - elif isNull(loc_id): - # If we allow "cascade" at the top-level, this essentially means *all* locations - if not cascade: - queryset = queryset.filter(parent=None) - - if cascade and depth is not None: - queryset = queryset.filter(level__lte=depth) - - else: - try: - location = StockLocation.objects.get(pk=loc_id) - - # All sub-locations to be returned too? - if cascade: - parents = location.get_descendants(include_self=True) - if depth is not None: - parents = parents.filter(level__lte=location.level + depth) - - parent_ids = [p.id for p in parents] - queryset = queryset.filter(parent__in=parent_ids) - - else: - queryset = queryset.filter(parent=location) - - except (ValueError, StockLocation.DoesNotExist): - pass - - # Exclude StockLocation tree - exclude_tree = params.get('exclude_tree', None) - - if exclude_tree is not None: - try: - loc = StockLocation.objects.get(pk=exclude_tree) - - queryset = queryset.exclude( - pk__in=[ - subloc.pk for subloc in loc.get_descendants(include_self=True) - ] - ) - - except (ValueError, StockLocation.DoesNotExist): - pass - - return queryset - filter_backends = SEARCH_ORDER_FILTER - filterset_fields = ['name', 'structural', 'external', 'tags__name', 'tags__slug'] - search_fields = ['name', 'description', 'tags__name', 'tags__slug'] ordering_fields = ['name', 'pathstring', 'items', 'level', 'tree_id', 'lft'] diff --git a/InvenTree/stock/serializers.py b/InvenTree/stock/serializers.py index 87b0090e21..ffebd78801 100644 --- a/InvenTree/stock/serializers.py +++ b/InvenTree/stock/serializers.py @@ -126,7 +126,7 @@ class StockItemTestResultSerializer(InvenTree.serializers.InvenTreeModelSerializ data['template'] = template else: - logger.info( + logger.debug( "No matching test template found for '%s' - creating a new template", test_name, ) diff --git a/InvenTree/stock/templates/stock/location.html b/InvenTree/stock/templates/stock/location.html index bfe16dce6a..4d36aa6d22 100644 --- a/InvenTree/stock/templates/stock/location.html +++ b/InvenTree/stock/templates/stock/location.html @@ -271,8 +271,6 @@ params: { {% if location %} parent: {{ location.pk }}, - {% else %} - parent: 'null', {% endif %} }, allowTreeView: true, diff --git a/InvenTree/stock/test_api.py b/InvenTree/stock/test_api.py index 187a2f5219..ca237d2029 100644 --- a/InvenTree/stock/test_api.py +++ b/InvenTree/stock/test_api.py @@ -72,33 +72,13 @@ class StockLocationTest(StockAPITestCase): ({}, 8, 'no parameters'), ({'parent': 1, 'cascade': False}, 2, 'Filter by parent, no cascading'), ({'parent': 1, 'cascade': True}, 2, 'Filter by parent, cascading'), - ({'cascade': True, 'depth': 0}, 8, 'Cascade with no parent, depth=0'), - ({'cascade': False, 'depth': 10}, 8, 'Cascade with no parent, depth=0'), - ( - {'parent': 'null', 'cascade': True, 'depth': 0}, - 7, - 'Cascade with null parent, depth=0', - ), - ( - {'parent': 'null', 'cascade': True, 'depth': 10}, - 8, - 'Cascade with null parent and bigger depth', - ), - ( - {'parent': 'null', 'cascade': False, 'depth': 10}, - 3, - 'No cascade even with depth specified with null parent', - ), + ({'cascade': True, 'depth': 0}, 7, 'Cascade with no parent, depth=0'), + ({'cascade': False, 'depth': 10}, 3, 'Cascade with no parent, depth=10'), ( {'parent': 1, 'cascade': False, 'depth': 0}, - 2, + 1, 'Dont cascade with depth=0 and parent', ), - ( - {'parent': 1, 'cascade': True, 'depth': 0}, - 2, - 'Cascade with depth=0 and parent', - ), ( {'parent': 1, 'cascade': False, 'depth': 1}, 2, @@ -110,20 +90,9 @@ class StockLocationTest(StockAPITestCase): 'Cascade with depth=1 with parent', ), ( - {'parent': 1, 'cascade': True, 'depth': 'abcdefg'}, - 2, - 'Cascade with invalid depth and parent', - ), - ({'parent': 42}, 8, 'Should return everything if parent_pk is not valid'), - ( - {'parent': 'null', 'exclude_tree': 1, 'cascade': True}, - 5, - 'Should return everything except tree with pk=1', - ), - ( - {'parent': 'null', 'exclude_tree': 42, 'cascade': True}, + {'exclude_tree': 1, 'cascade': True}, 8, - 'Should return everything because exclude_tree=42 is no valid pk', + 'Should return everything except tree with pk=1', ), ] @@ -453,6 +422,18 @@ class StockLocationTest(StockAPITestCase): self.assertEqual(len(res), 1) self.assertEqual(res[0]['name'], 'Test location wo type') + res = self.get( + self.list_url, + { + 'parent': str(parent_location.pk), + 'has_location_type': '0', + 'cascade': False, + }, + expected_code=200, + ).json() + + self.assertEqual(len(res), 1) + class StockLocationTypeTest(StockAPITestCase): """Tests for the StockLocationType API endpoints.""" diff --git a/src/frontend/src/tables/part/PartCategoryTable.tsx b/src/frontend/src/tables/part/PartCategoryTable.tsx index a620e9c6cd..4c40a569e4 100644 --- a/src/frontend/src/tables/part/PartCategoryTable.tsx +++ b/src/frontend/src/tables/part/PartCategoryTable.tsx @@ -138,7 +138,7 @@ export function PartCategoryTable({ parentId }: { parentId?: any }) { props={{ enableDownload: true, params: { - parent: parentId ?? 'null' + parent: parentId }, tableFilters: tableFilters, tableActions: tableActions, diff --git a/src/frontend/src/tables/stock/StockLocationTable.tsx b/src/frontend/src/tables/stock/StockLocationTable.tsx index 8573207b82..96a2906bd3 100644 --- a/src/frontend/src/tables/stock/StockLocationTable.tsx +++ b/src/frontend/src/tables/stock/StockLocationTable.tsx @@ -148,7 +148,7 @@ export function StockLocationTable({ parentId }: { parentId?: any }) { props={{ enableDownload: true, params: { - parent: parentId ?? 'null' + parent: parentId }, tableFilters: tableFilters, tableActions: tableActions,