From 9db5205f79c9f88ea9e276546a883be860d3eb5c Mon Sep 17 00:00:00 2001 From: Oliver Date: Mon, 17 Mar 2025 09:21:43 +1100 Subject: [PATCH] Bulk update mixin (#9313) * Refactor BulkDeleteMixin * Implement BulkUpdateMixin class * Refactor NotificationsTable - Use common bulkdelete operation * Update successMessage * Update metadata constructs * Add bulk-edit support for PartList endpoint * Implement set-category for part table * Cleanup old endpoint * Improve form error handling * Simplify translated text * Add playwright tests * Bump API version * Fix unit tests * Further test updates --- src/backend/InvenTree/InvenTree/api.py | 215 ++++++++++++------ .../InvenTree/InvenTree/api_version.py | 6 +- src/backend/InvenTree/InvenTree/metadata.py | 6 +- src/backend/InvenTree/InvenTree/test_api.py | 10 +- src/backend/InvenTree/part/api.py | 16 +- src/backend/InvenTree/part/serializers.py | 51 ----- src/frontend/src/components/forms/ApiForm.tsx | 21 +- src/frontend/src/hooks/UseForm.tsx | 37 ++- src/frontend/src/pages/Notifications.tsx | 30 +-- .../src/tables/InvenTreeTableHeader.tsx | 13 ++ .../notifications/NotificationTable.tsx | 1 + src/frontend/src/tables/part/PartTable.tsx | 27 ++- src/frontend/tests/pages/pui_part.spec.ts | 20 ++ 13 files changed, 275 insertions(+), 178 deletions(-) diff --git a/src/backend/InvenTree/InvenTree/api.py b/src/backend/InvenTree/InvenTree/api.py index 6b9b9c7056..01a59b55dd 100644 --- a/src/backend/InvenTree/InvenTree/api.py +++ b/src/backend/InvenTree/InvenTree/api.py @@ -373,15 +373,141 @@ class NotFoundView(APIView): return self.not_found(request) -class BulkDeleteMixin: +class BulkOperationMixin: + """Mixin class for handling bulk data operations. + + Bulk operations are implemented for two major reasons: + - Speed (single API call vs multiple API calls) + - Atomicity (guaranteed that either *all* items are updated, or *none*) + """ + + def get_bulk_queryset(self, request): + """Return a queryset based on the selection made in the request. + + Selection can be made by providing either: + + - items: A list of primary key values + - filters: A dictionary of filter values + """ + model = self.serializer_class.Meta.model + + items = request.data.pop('items', None) + filters = request.data.pop('filters', None) + + queryset = model.objects.all() + + if not items and not filters: + raise ValidationError({ + 'non_field_errors': _( + 'List of items or filters must be provided for bulk operation' + ) + }) + + if items: + if type(items) is not list: + raise ValidationError({ + 'non_field_errors': _('Items must be provided as a list') + }) + + # Filter by primary key + try: + queryset = queryset.filter(pk__in=items) + except Exception: + raise ValidationError({ + 'non_field_errors': _('Invalid items list provided') + }) + + if filters: + if type(filters) is not dict: + raise ValidationError({ + 'non_field_errors': _('Filters must be provided as a dict') + }) + + try: + queryset = queryset.filter(**filters) + except Exception: + raise ValidationError({ + 'non_field_errors': _('Invalid filters provided') + }) + + if queryset.count() == 0: + raise ValidationError({ + 'non_field_errors': _('No items match the provided criteria') + }) + + return queryset + + +class BulkUpdateMixin(BulkOperationMixin): + """Mixin class for enabling 'bulk update' operations for various models. + + Bulk update allows for multiple items to be updated in a single API query, + rather than using multiple API calls to the various detail endpoints. + """ + + def validate_update(self, queryset, request) -> None: + """Perform validation right before updating. + + Arguments: + queryset: The queryset to be updated + request: The request object + + Returns: + None + + Raises: + ValidationError: If the update should not proceed + """ + # Default implementation does nothing + + def filter_update_queryset(self, queryset, request): + """Provide custom filtering for the queryset *before* it is updated. + + The default implementation does nothing, just returns the queryset. + """ + return queryset + + def put(self, request, *args, **kwargs): + """Perform a PUT operation against this list endpoint. + + Simply redirects to the PATCH method. + """ + return self.patch(request, *args, **kwargs) + + def patch(self, request, *args, **kwargs): + """Perform a PATCH operation against this list endpoint. + + Note that the typical DRF list endpoint does not support PATCH, + so this method is provided as a custom implementation. + """ + queryset = self.get_bulk_queryset(request) + queryset = self.filter_update_queryset(queryset, request) + + self.validate_update(queryset, request) + + # Perform the update operation + data = request.data + + n = queryset.count() + + with transaction.atomic(): + # Perform object update + # Note that we do not perform a bulk-update operation here, + # as we want to trigger any custom post_save methods on the model + for instance in queryset: + serializer = self.get_serializer(instance, data=data, partial=True) + + serializer.is_valid(raise_exception=True) + serializer.save() + + return Response({'success': f'Updated {n} items'}, status=200) + + +class BulkDeleteMixin(BulkOperationMixin): """Mixin class for enabling 'bulk delete' operations for various models. Bulk delete allows for multiple items to be deleted in a single API query, rather than using multiple API calls to the various detail endpoints. - - This is implemented for two major reasons: - - Atomicity (guaranteed that either *all* items are deleted, or *none*) - - Speed (single API call and DB query) """ def validate_delete(self, queryset, request) -> None: @@ -397,6 +523,7 @@ class BulkDeleteMixin: Raises: ValidationError: If the deletion should not proceed """ + # Default implementation does nothing def filter_delete_queryset(self, queryset, request): """Provide custom filtering for the queryset *before* it is deleted. @@ -408,79 +535,23 @@ class BulkDeleteMixin: def delete(self, request, *args, **kwargs): """Perform a DELETE operation against this list endpoint. - We expect a list of primary-key (ID) values to be supplied as a JSON object, e.g. - { - items: [4, 8, 15, 16, 23, 42] - } - + Note that the typical DRF list endpoint does not support DELETE, + so this method is provided as a custom implementation. """ - model = self.serializer_class.Meta.model + queryset = self.get_bulk_queryset(request) + queryset = self.filter_delete_queryset(queryset, request) - # Extract the items from the request body - try: - items = request.data.getlist('items', None) - except AttributeError: - items = request.data.get('items', None) - - # Extract the filters from the request body - try: - filters = request.data.getlist('filters', None) - except AttributeError: - filters = request.data.get('filters', None) - - if not items and not filters: - raise ValidationError({ - 'non_field_errors': [ - 'List of items or filters must be provided for bulk deletion' - ] - }) - - if items and type(items) is not list: - raise ValidationError({ - 'items': ["'items' must be supplied as a list object"] - }) - - if filters and type(filters) is not dict: - raise ValidationError({ - 'filters': ["'filters' must be supplied as a dict object"] - }) + self.validate_delete(queryset, request) # Keep track of how many items we deleted - n_deleted = 0 + n_deleted = queryset.count() with transaction.atomic(): - # Start with *all* models and perform basic filtering - queryset = model.objects.all() - queryset = self.filter_delete_queryset(queryset, request) - - # Filter by provided item ID values - if items: - try: - queryset = queryset.filter(id__in=items) - except Exception: - raise ValidationError({ - 'non_field_errors': _('Invalid items list provided') - }) - - # Filter by provided filters - if filters: - try: - queryset = queryset.filter(**filters) - except Exception: - raise ValidationError({ - 'non_field_errors': _('Invalid filters provided') - }) - - if queryset.count() == 0: - raise ValidationError({ - 'non_field_errors': _('No items found to delete') - }) - - # Run a final validation step (should raise an error if the deletion should not proceed) - self.validate_delete(queryset, request) - - n_deleted = queryset.count() - queryset.delete() + # Perform object deletion + # Note that we do not perform a bulk-delete operation here, + # as we want to trigger any custom post_delete methods on the model + for item in queryset: + item.delete() return Response({'success': f'Deleted {n_deleted} items'}, status=204) diff --git a/src/backend/InvenTree/InvenTree/api_version.py b/src/backend/InvenTree/InvenTree/api_version.py index cf06a17cfb..1a46da67e7 100644 --- a/src/backend/InvenTree/InvenTree/api_version.py +++ b/src/backend/InvenTree/InvenTree/api_version.py @@ -1,13 +1,17 @@ """InvenTree API version information.""" # InvenTree API version -INVENTREE_API_VERSION = 322 +INVENTREE_API_VERSION = 323 """Increment this API version number whenever there is a significant change to the API that any clients need to know about.""" INVENTREE_API_TEXT = """ +v323 - 2025-03-17 : https://github.com/inventree/InvenTree/pull/9313 + - Adds BulkUpdate support to the Part API endpoint + - Remove legacy API endpoint to set part category for multiple parts + v322 - 2025-03-16 : https://github.com/inventree/InvenTree/pull/8933 - Add min_date and max_date query filters for orders, for use in calendar views diff --git a/src/backend/InvenTree/InvenTree/metadata.py b/src/backend/InvenTree/InvenTree/metadata.py index f06cd29e8c..de0995df68 100644 --- a/src/backend/InvenTree/InvenTree/metadata.py +++ b/src/backend/InvenTree/InvenTree/metadata.py @@ -44,6 +44,8 @@ class InvenTreeMetadata(SimpleMetadata): See SimpleMetadata.determine_actions for more information. """ + from InvenTree.api import BulkUpdateMixin + actions = {} for method in {'PUT', 'POST', 'GET'} & set(view.allowed_methods): @@ -54,7 +56,9 @@ class InvenTreeMetadata(SimpleMetadata): view.check_permissions(view.request) # Test object permissions if method == 'PUT' and hasattr(view, 'get_object'): - view.get_object() + if not issubclass(view.__class__, BulkUpdateMixin): + # Bypass the get_object method for the BulkUpdateMixin + view.get_object() except (exceptions.APIException, PermissionDenied, Http404): pass else: diff --git a/src/backend/InvenTree/InvenTree/test_api.py b/src/backend/InvenTree/InvenTree/test_api.py index a7c5128577..f6defd64e5 100644 --- a/src/backend/InvenTree/InvenTree/test_api.py +++ b/src/backend/InvenTree/InvenTree/test_api.py @@ -217,9 +217,11 @@ class ApiAccessTests(InvenTreeAPITestCase): actions = self.getActions(url) - self.assertEqual(len(actions), 2) + self.assertEqual(len(actions), 3) + self.assertIn('POST', actions) self.assertIn('GET', actions) + self.assertIn('PUT', actions) # Fancy bulk-update action def test_detail_endpoint_actions(self): """Tests for detail API endpoint actions.""" @@ -268,19 +270,19 @@ class BulkDeleteTests(InvenTreeAPITestCase): response = self.delete(url, {}, expected_code=400) self.assertIn( - 'List of items or filters must be provided for bulk deletion', + 'List of items or filters must be provided for bulk operation', str(response.data), ) # DELETE with invalid 'items' response = self.delete(url, {'items': {'hello': 'world'}}, expected_code=400) - self.assertIn("'items' must be supplied as a list object", str(response.data)) + self.assertIn('Items must be provided as a list', str(response.data)) # DELETE with invalid 'filters' response = self.delete(url, {'filters': [1, 2, 3]}, expected_code=400) - self.assertIn("'filters' must be supplied as a dict object", str(response.data)) + self.assertIn('Filters must be provided as a dict', str(response.data)) class SearchTests(InvenTreeAPITestCase): diff --git a/src/backend/InvenTree/part/api.py b/src/backend/InvenTree/part/api.py index d7acfe8dbf..22094273b3 100644 --- a/src/backend/InvenTree/part/api.py +++ b/src/backend/InvenTree/part/api.py @@ -20,7 +20,7 @@ import part.filters from build.models import Build, BuildItem from build.status_codes import BuildStatusGroups from importer.mixins import DataExportViewMixin -from InvenTree.api import ListCreateDestroyAPIView, MetadataView +from InvenTree.api import BulkUpdateMixin, ListCreateDestroyAPIView, MetadataView from InvenTree.filters import ( ORDER_FILTER, ORDER_FILTER_ALIAS, @@ -1239,7 +1239,7 @@ class PartMixin: return context -class PartList(PartMixin, DataExportViewMixin, ListCreateAPI): +class PartList(PartMixin, BulkUpdateMixin, DataExportViewMixin, ListCreateAPI): """API endpoint for accessing a list of Part objects, or creating a new Part instance.""" filterset_class = PartFilter @@ -1407,13 +1407,6 @@ class PartList(PartMixin, DataExportViewMixin, ListCreateAPI): ] -class PartChangeCategory(CreateAPI): - """API endpoint to change the location of multiple parts in bulk.""" - - serializer_class = part_serializers.PartSetCategorySerializer - queryset = Part.objects.none() - - class PartDetail(PartMixin, RetrieveUpdateDestroyAPI): """API endpoint for detail view of a single Part object.""" @@ -2231,11 +2224,6 @@ part_api_urls = [ path('', PartDetail.as_view(), name='api-part-detail'), ]), ), - path( - 'change_category/', - PartChangeCategory.as_view(), - name='api-part-change-category', - ), path('', PartList.as_view(), name='api-part-list'), ] diff --git a/src/backend/InvenTree/part/serializers.py b/src/backend/InvenTree/part/serializers.py index 08d77c2e64..ad906f6ffc 100644 --- a/src/backend/InvenTree/part/serializers.py +++ b/src/backend/InvenTree/part/serializers.py @@ -465,57 +465,6 @@ class PartParameterSerializer( ) -class PartSetCategorySerializer(serializers.Serializer): - """Serializer for changing PartCategory for multiple Part objects.""" - - class Meta: - """Metaclass options.""" - - fields = ['parts', 'category'] - - parts = serializers.PrimaryKeyRelatedField( - queryset=Part.objects.all(), - many=True, - required=True, - allow_null=False, - label=_('Parts'), - ) - - def validate_parts(self, parts): - """Validate the selected parts.""" - if len(parts) == 0: - raise serializers.ValidationError(_('No parts selected')) - - return parts - - category = serializers.PrimaryKeyRelatedField( - queryset=PartCategory.objects.filter(structural=False), - many=False, - required=True, - allow_null=False, - label=_('Category'), - help_text=_('Select category'), - ) - - @transaction.atomic - def save(self): - """Save the serializer to change the location of the selected parts.""" - data = self.validated_data - parts = data['parts'] - category = data['category'] - - parts_to_save = [] - - for p in parts: - if p.category == category: - continue - - p.category = category - parts_to_save.append(p) - - Part.objects.bulk_update(parts_to_save, ['category']) - - class DuplicatePartSerializer(serializers.Serializer): """Serializer for specifying options when duplicating a Part. diff --git a/src/frontend/src/components/forms/ApiForm.tsx b/src/frontend/src/components/forms/ApiForm.tsx index 096f8739d1..9ee6a4a60e 100644 --- a/src/frontend/src/components/forms/ApiForm.tsx +++ b/src/frontend/src/components/forms/ApiForm.tsx @@ -505,7 +505,22 @@ export function ApiForm({ case 400: // Data validation errors const _nonFieldErrors: string[] = []; + const processErrors = (errors: any, _path?: string) => { + // Handle an array of errors + if (Array.isArray(errors)) { + errors.forEach((error: any) => { + _nonFieldErrors.push(error.toString()); + }); + return; + } + + // Handle simple string + if (typeof errors === 'string') { + _nonFieldErrors.push(errors); + return; + } + for (const [k, v] of Object.entries(errors)) { const path = _path ? `${_path}.${k}` : k; @@ -513,10 +528,8 @@ export function ApiForm({ const field = fields[k]; const valid = field && !field.hidden; - if (!valid || k === 'non_field_errors' || k === '__all__') { - if (Array.isArray(v)) { - _nonFieldErrors.push(...v); - } + if (!valid || k == 'non_field_errors' || k == '__all__') { + processErrors(v); continue; } diff --git a/src/frontend/src/hooks/UseForm.tsx b/src/frontend/src/hooks/UseForm.tsx index abf97034dc..0a8e8c7cb4 100644 --- a/src/frontend/src/hooks/UseForm.tsx +++ b/src/frontend/src/hooks/UseForm.tsx @@ -88,7 +88,7 @@ export function useCreateApiFormModal(props: ApiFormModalProps) { props.successMessage === null ? null : (props.successMessage ?? t`Item Created`), - method: 'POST' + method: props.method ?? 'POST' }), [props] ); @@ -116,6 +116,41 @@ export function useEditApiFormModal(props: ApiFormModalProps) { return useApiFormModal(editProps); } +interface BulkEditApiFormModalProps extends ApiFormModalProps { + items: number[]; +} + +export function useBulkEditApiFormModal({ + items, + ...props +}: BulkEditApiFormModalProps) { + const bulkEditProps = useMemo( + () => ({ + ...props, + method: 'PATCH', + submitText: props.submitText ?? t`Update`, + successMessage: + props.successMessage === null + ? null + : (props.successMessage ?? t`Items Updated`), + preFormContent: props.preFormContent ?? ( + {t`Update multiple items`} + ), + fields: { + ...props.fields, + items: { + hidden: true, + field_type: 'number', + value: items + } + } + }), + [props, items] + ); + + return useApiFormModal(bulkEditProps); +} + /** * Open a modal form to delete a model instance */ diff --git a/src/frontend/src/pages/Notifications.tsx b/src/frontend/src/pages/Notifications.tsx index 4bc99f34c7..e25fc6ed75 100644 --- a/src/frontend/src/pages/Notifications.tsx +++ b/src/frontend/src/pages/Notifications.tsx @@ -1,6 +1,5 @@ import { t } from '@lingui/macro'; import { Stack } from '@mantine/core'; -import { modals } from '@mantine/modals'; import { IconBellCheck, IconBellExclamation, @@ -39,26 +38,6 @@ export default function NotificationsPage() { .catch((_error) => {}); }, []); - const deleteNotifications = useCallback(() => { - modals.openConfirmModal({ - title: t`Delete Notifications`, - onConfirm: () => { - api - .delete(apiUrl(ApiEndpoints.notifications_list), { - data: { - filters: { - read: true - } - } - }) - .then((_response) => { - readTable.refreshTable(); - }) - .catch((_error) => {}); - } - }); - }, []); - const notificationPanels = useMemo(() => { return [ { @@ -139,14 +118,7 @@ export default function NotificationsPage() { } } ]} - tableActions={[ - } - tooltip={t`Delete notifications`} - onClick={deleteNotifications} - /> - ]} + tableActions={[]} /> ) } diff --git a/src/frontend/src/tables/InvenTreeTableHeader.tsx b/src/frontend/src/tables/InvenTreeTableHeader.tsx index 19daef8a32..f9bce56344 100644 --- a/src/frontend/src/tables/InvenTreeTableHeader.tsx +++ b/src/frontend/src/tables/InvenTreeTableHeader.tsx @@ -9,6 +9,7 @@ import { } from '@mantine/core'; import { IconBarcode, + IconExclamationCircle, IconFilter, IconRefresh, IconTrash @@ -16,6 +17,7 @@ import { import { useMemo, useState } from 'react'; import { Fragment } from 'react/jsx-runtime'; +import { showNotification } from '@mantine/notifications'; import { Boundary } from '../components/Boundary'; import { ActionButton } from '../components/buttons/ActionButton'; import { ButtonMenu } from '../components/buttons/ButtonMenu'; @@ -112,6 +114,17 @@ export default function InvenTreeTableHeader({ hidden: true } }, + successMessage: t`Items deleted`, + onFormError: (response) => { + showNotification({ + id: 'bulk-delete-error', + title: t`Error`, + message: t`Failed to delete items`, + color: 'red', + icon: , + autoClose: 5000 + }); + }, onFormSuccess: () => { tableState.clearSelectedRecords(); tableState.refreshTable(); diff --git a/src/frontend/src/tables/notifications/NotificationTable.tsx b/src/frontend/src/tables/notifications/NotificationTable.tsx index 34af987a10..9703b1f9a1 100644 --- a/src/frontend/src/tables/notifications/NotificationTable.tsx +++ b/src/frontend/src/tables/notifications/NotificationTable.tsx @@ -52,6 +52,7 @@ export function NotificationTable({ rowActions: actions, tableActions: tableActions, enableSelection: true, + enableBulkDelete: true, params: params }} /> diff --git a/src/frontend/src/tables/part/PartTable.tsx b/src/frontend/src/tables/part/PartTable.tsx index 3d037475b4..74626ad56c 100644 --- a/src/frontend/src/tables/part/PartTable.tsx +++ b/src/frontend/src/tables/part/PartTable.tsx @@ -12,7 +12,10 @@ import { ModelType } from '../../enums/ModelType'; import { UserRoles } from '../../enums/Roles'; import { usePartFields } from '../../forms/PartForms'; import { InvenTreeIcon } from '../../functions/icons'; -import { useCreateApiFormModal } from '../../hooks/UseForm'; +import { + useBulkEditApiFormModal, + useCreateApiFormModal +} from '../../hooks/UseForm'; import { useTable } from '../../hooks/UseTable'; import { apiUrl } from '../../states/ApiState'; import { useUserState } from '../../states/UserState'; @@ -341,6 +344,16 @@ export function PartListTable({ modelType: ModelType.part }); + const setCategory = useBulkEditApiFormModal({ + url: ApiEndpoints.part_list, + items: table.selectedIds, + title: t`Set Category`, + fields: { + category: {} + }, + onFormSuccess: table.refreshTable + }); + const orderPartsWizard = OrderPartsWizard({ parts: table.selectedRecords }); const tableActions = useMemo(() => { @@ -350,10 +363,21 @@ export function PartListTable({ icon={} disabled={!table.hasSelectedRecords} actions={[ + { + name: t`Set Category`, + icon: , + tooltip: t`Set category for selected parts`, + hidden: !user.hasChangeRole(UserRoles.part), + disabled: !table.hasSelectedRecords, + onClick: () => { + setCategory.open(); + } + }, { name: t`Order Parts`, icon: , tooltip: t`Order selected parts`, + hidden: !user.hasAddRole(UserRoles.purchase_order), onClick: () => { orderPartsWizard.openWizard(); } @@ -372,6 +396,7 @@ export function PartListTable({ return ( <> {newPart.modal} + {setCategory.modal} {orderPartsWizard.wizard} { await page.waitForURL('**/platform/part/101/**'); await page.getByText('Select Part Revision').waitFor(); }); + +test('Parts - Bulk Edit', async ({ page }) => { + await doQuickLogin(page); + + await navigate(page, 'part/category/index/parts'); + + // Edit the category for multiple parts + await page.getByLabel('Select record 1', { exact: true }).click(); + await page.getByLabel('Select record 2', { exact: true }).click(); + await page.getByLabel('action-menu-part-actions').click(); + await page.getByLabel('action-menu-part-actions-set-category').click(); + await page.getByLabel('related-field-category').fill('rnitu'); + await page + .getByRole('option', { name: '- Furniture/Chairs' }) + .getByRole('paragraph') + .click(); + + await page.getByRole('button', { name: 'Update' }).click(); + await page.getByText('Items Updated').waitFor(); +});