diff --git a/InvenTree/InvenTree/api.py b/InvenTree/InvenTree/api.py index 974d518e6a..a9f55b23a6 100644 --- a/InvenTree/InvenTree/api.py +++ b/InvenTree/InvenTree/api.py @@ -1,11 +1,14 @@ """Main JSON interface views.""" from django.conf import settings +from django.db import transaction from django.http import JsonResponse from django.utils.translation import gettext_lazy as _ from django_filters.rest_framework import DjangoFilterBackend -from rest_framework import filters, permissions +from rest_framework import filters, generics, permissions +from rest_framework.response import Response +from rest_framework.serializers import ValidationError from .status import is_worker_running from .version import (inventreeApiVersion, inventreeInstanceName, @@ -50,6 +53,60 @@ class NotFoundView(AjaxView): return JsonResponse(data, status=404) +class BulkDeleteMixin: + """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 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] + } + + """ + model = self.serializer_class.Meta.model + + # Extract the items from the request body + try: + items = request.data.getlist('items', None) + except AttributeError: + items = request.data.get('items', None) + + if items is None or type(items) is not list or not items: + raise ValidationError({ + "non_field_errors": ["List of items must be provided for bulk deletion"] + }) + + # Keep track of how many items we deleted + n_deleted = 0 + + with transaction.atomic(): + objects = model.objects.filter(id__in=items) + n_deleted = objects.count() + objects.delete() + + return Response( + { + 'success': f"Deleted {n_deleted} items", + }, + status=204 + ) + + +class ListCreateDestroyAPIView(BulkDeleteMixin, generics.ListCreateAPIView): + """Custom API endpoint which provides BulkDelete functionality in addition to List and Create""" + ... + + class APIDownloadMixin: """Mixin for enabling a LIST endpoint to be downloaded a file. diff --git a/InvenTree/InvenTree/api_tester.py b/InvenTree/InvenTree/api_tester.py index 08952a7df1..ef1007c66b 100644 --- a/InvenTree/InvenTree/api_tester.py +++ b/InvenTree/InvenTree/api_tester.py @@ -123,18 +123,30 @@ class InvenTreeAPITestCase(UserMixin, APITestCase): return response - def post(self, url, data, expected_code=None, format='json'): + def post(self, url, data=None, expected_code=None, format='json'): """Issue a POST request.""" response = self.client.post(url, data=data, format=format) + if data is None: + data = {} + if expected_code is not None: + + if response.status_code != expected_code: + print(f"Unexpected response at '{url}':") + print(response.data) + self.assertEqual(response.status_code, expected_code) return response - def delete(self, url, expected_code=None): + def delete(self, url, data=None, expected_code=None, format='json'): """Issue a DELETE request.""" - response = self.client.delete(url) + + if data is None: + data = {} + + response = self.client.delete(url, data=data, foramt=format) if expected_code is not None: self.assertEqual(response.status_code, expected_code) @@ -155,6 +167,11 @@ class InvenTreeAPITestCase(UserMixin, APITestCase): response = self.client.put(url, data=data, format=format) if expected_code is not None: + + if response.status_code != expected_code: + print(f"Unexpected response at '{url}':") + print(response.data) + self.assertEqual(response.status_code, expected_code) return response diff --git a/InvenTree/InvenTree/api_version.py b/InvenTree/InvenTree/api_version.py index 1ba4f259fb..7bb41bd8c5 100644 --- a/InvenTree/InvenTree/api_version.py +++ b/InvenTree/InvenTree/api_version.py @@ -2,11 +2,14 @@ # InvenTree API version -INVENTREE_API_VERSION = 57 +INVENTREE_API_VERSION = 58 """ Increment this API version number whenever there is a significant change to the API that any clients need to know about +v58 -> 2022-06-06 : https://github.com/inventree/InvenTree/pull/3146 + - Adds a BulkDelete API mixin class for fast, safe deletion of multiple objects with a single API request + v57 -> 2022-06-05 : https://github.com/inventree/InvenTree/pull/3130 - Transfer PartCategoryTemplateParameter actions to the API diff --git a/InvenTree/build/api.py b/InvenTree/build/api.py index cd6c6ba82a..89e929256c 100644 --- a/InvenTree/build/api.py +++ b/InvenTree/build/api.py @@ -7,7 +7,7 @@ from rest_framework import filters, generics from django_filters.rest_framework import DjangoFilterBackend from django_filters import rest_framework as rest_filters -from InvenTree.api import AttachmentMixin, APIDownloadMixin +from InvenTree.api import AttachmentMixin, APIDownloadMixin, ListCreateDestroyAPIView from InvenTree.helpers import str2bool, isNull, DownloadFile from InvenTree.filters import InvenTreeOrderingFilter from InvenTree.status_codes import BuildStatus @@ -413,7 +413,7 @@ class BuildItemList(generics.ListCreateAPIView): ] -class BuildAttachmentList(AttachmentMixin, generics.ListCreateAPIView): +class BuildAttachmentList(AttachmentMixin, ListCreateDestroyAPIView): """API endpoint for listing (and creating) BuildOrderAttachment objects.""" queryset = BuildOrderAttachment.objects.all() diff --git a/InvenTree/company/api.py b/InvenTree/company/api.py index c0455abcca..2e8544002b 100644 --- a/InvenTree/company/api.py +++ b/InvenTree/company/api.py @@ -7,7 +7,7 @@ from django_filters import rest_framework as rest_filters from django_filters.rest_framework import DjangoFilterBackend from rest_framework import filters, generics -from InvenTree.api import AttachmentMixin +from InvenTree.api import AttachmentMixin, ListCreateDestroyAPIView from InvenTree.helpers import str2bool from .models import (Company, ManufacturerPart, ManufacturerPartAttachment, @@ -98,7 +98,7 @@ class ManufacturerPartFilter(rest_filters.FilterSet): active = rest_filters.BooleanFilter(field_name='part__active') -class ManufacturerPartList(generics.ListCreateAPIView): +class ManufacturerPartList(ListCreateDestroyAPIView): """API endpoint for list view of ManufacturerPart object. - GET: Return list of ManufacturerPart objects @@ -158,7 +158,7 @@ class ManufacturerPartDetail(generics.RetrieveUpdateDestroyAPIView): serializer_class = ManufacturerPartSerializer -class ManufacturerPartAttachmentList(AttachmentMixin, generics.ListCreateAPIView): +class ManufacturerPartAttachmentList(AttachmentMixin, ListCreateDestroyAPIView): """API endpoint for listing (and creating) a ManufacturerPartAttachment (file upload).""" queryset = ManufacturerPartAttachment.objects.all() @@ -180,7 +180,7 @@ class ManufacturerPartAttachmentDetail(AttachmentMixin, generics.RetrieveUpdateD serializer_class = ManufacturerPartAttachmentSerializer -class ManufacturerPartParameterList(generics.ListCreateAPIView): +class ManufacturerPartParameterList(ListCreateDestroyAPIView): """API endpoint for list view of ManufacturerPartParamater model.""" queryset = ManufacturerPartParameter.objects.all() @@ -253,7 +253,7 @@ class ManufacturerPartParameterDetail(generics.RetrieveUpdateDestroyAPIView): serializer_class = ManufacturerPartParameterSerializer -class SupplierPartList(generics.ListCreateAPIView): +class SupplierPartList(ListCreateDestroyAPIView): """API endpoint for list view of SupplierPart object. - GET: Return list of SupplierPart objects diff --git a/InvenTree/order/api.py b/InvenTree/order/api.py index 9ad123577d..0826b9fdb0 100644 --- a/InvenTree/order/api.py +++ b/InvenTree/order/api.py @@ -10,7 +10,8 @@ from rest_framework.response import Response import order.models as models import order.serializers as serializers from company.models import SupplierPart -from InvenTree.api import APIDownloadMixin, AttachmentMixin +from InvenTree.api import (APIDownloadMixin, AttachmentMixin, + ListCreateDestroyAPIView) from InvenTree.filters import InvenTreeOrderingFilter from InvenTree.helpers import DownloadFile, str2bool from InvenTree.status_codes import PurchaseOrderStatus, SalesOrderStatus @@ -527,7 +528,7 @@ class PurchaseOrderExtraLineDetail(generics.RetrieveUpdateDestroyAPIView): serializer_class = serializers.PurchaseOrderExtraLineSerializer -class SalesOrderAttachmentList(AttachmentMixin, generics.ListCreateAPIView): +class SalesOrderAttachmentList(AttachmentMixin, ListCreateDestroyAPIView): """API endpoint for listing (and creating) a SalesOrderAttachment (file upload)""" queryset = models.SalesOrderAttachment.objects.all() @@ -1056,7 +1057,7 @@ class SalesOrderShipmentComplete(generics.CreateAPIView): return ctx -class PurchaseOrderAttachmentList(AttachmentMixin, generics.ListCreateAPIView): +class PurchaseOrderAttachmentList(AttachmentMixin, ListCreateDestroyAPIView): """API endpoint for listing (and creating) a PurchaseOrderAttachment (file upload)""" queryset = models.PurchaseOrderAttachment.objects.all() diff --git a/InvenTree/part/api.py b/InvenTree/part/api.py index 31ae1adca5..23b2955744 100644 --- a/InvenTree/part/api.py +++ b/InvenTree/part/api.py @@ -22,7 +22,8 @@ import order.models from build.models import Build, BuildItem from common.models import InvenTreeSetting from company.models import Company, ManufacturerPart, SupplierPart -from InvenTree.api import APIDownloadMixin, AttachmentMixin +from InvenTree.api import (APIDownloadMixin, AttachmentMixin, + ListCreateDestroyAPIView) from InvenTree.helpers import DownloadFile, increment, isNull, str2bool from InvenTree.status_codes import (BuildStatus, PurchaseOrderStatus, SalesOrderStatus) @@ -302,7 +303,7 @@ class PartInternalPriceList(generics.ListCreateAPIView): ] -class PartAttachmentList(AttachmentMixin, generics.ListCreateAPIView): +class PartAttachmentList(AttachmentMixin, ListCreateDestroyAPIView): """API endpoint for listing (and creating) a PartAttachment (file upload).""" queryset = PartAttachment.objects.all() @@ -1522,7 +1523,7 @@ class BomFilter(rest_filters.FilterSet): return queryset -class BomList(generics.ListCreateAPIView): +class BomList(ListCreateDestroyAPIView): """API endpoint for accessing a list of BomItem objects. - GET: Return list of BomItem objects diff --git a/InvenTree/stock/api.py b/InvenTree/stock/api.py index d22c0c37dc..13b3c0219e 100644 --- a/InvenTree/stock/api.py +++ b/InvenTree/stock/api.py @@ -22,7 +22,8 @@ import stock.serializers as StockSerializers from build.models import Build from company.models import Company, SupplierPart from company.serializers import CompanySerializer, SupplierPartSerializer -from InvenTree.api import APIDownloadMixin, AttachmentMixin +from InvenTree.api import (APIDownloadMixin, AttachmentMixin, + ListCreateDestroyAPIView) from InvenTree.filters import InvenTreeOrderingFilter from InvenTree.helpers import (DownloadFile, extract_serial_numbers, isNull, str2bool) @@ -465,7 +466,7 @@ class StockFilter(rest_filters.FilterSet): updated_after = rest_filters.DateFilter(label='Updated after', field_name='updated', lookup_expr='gte') -class StockList(APIDownloadMixin, generics.ListCreateAPIView): +class StockList(APIDownloadMixin, ListCreateDestroyAPIView): """API endpoint for list view of Stock objects. - GET: Return a list of all StockItem objects (with optional query filters) @@ -1043,7 +1044,7 @@ class StockList(APIDownloadMixin, generics.ListCreateAPIView): ] -class StockAttachmentList(AttachmentMixin, generics.ListCreateAPIView): +class StockAttachmentList(AttachmentMixin, ListCreateDestroyAPIView): """API endpoint for listing (and creating) a StockItemAttachment (file upload).""" queryset = StockItemAttachment.objects.all() @@ -1074,7 +1075,7 @@ class StockItemTestResultDetail(generics.RetrieveUpdateDestroyAPIView): serializer_class = StockSerializers.StockItemTestResultSerializer -class StockItemTestResultList(generics.ListCreateAPIView): +class StockItemTestResultList(ListCreateDestroyAPIView): """API endpoint for listing (and creating) a StockItemTestResult object.""" queryset = StockItemTestResult.objects.all() diff --git a/InvenTree/stock/templates/stock/item.html b/InvenTree/stock/templates/stock/item.html index e951428378..c7c6019ddb 100644 --- a/InvenTree/stock/templates/stock/item.html +++ b/InvenTree/stock/templates/stock/item.html @@ -276,12 +276,12 @@ { success: function(response) { - var results = []; + var items = []; // Ensure that we are only deleting the correct test results - response.forEach(function(item) { - if (item.stock_item == {{ item.pk }}) { - results.push(item); + response.forEach(function(result) { + if (result.stock_item == {{ item.pk }}) { + items.push(result.pk); } }); @@ -290,22 +290,14 @@ {% trans "Delete all test results for this stock item" %} `; - constructFormBody({}, { + constructForm(url, { + form_data: { + items: items, + }, method: 'DELETE', title: '{% trans "Delete Test Data" %}', preFormContent: html, - onSubmit: function(fields, opts) { - inventreeMultiDelete( - url, - results, - { - modal: opts.modal, - success: function() { - reloadTable(); - } - } - ) - } + onSuccess: reloadTable, }); } } diff --git a/InvenTree/stock/test_api.py b/InvenTree/stock/test_api.py index 4de9d47031..b0213e5a0b 100644 --- a/InvenTree/stock/test_api.py +++ b/InvenTree/stock/test_api.py @@ -15,7 +15,7 @@ import part.models from common.models import InvenTreeSetting from InvenTree.api_tester import InvenTreeAPITestCase from InvenTree.status_codes import StockStatus -from stock.models import StockItem, StockLocation +from stock.models import StockItem, StockItemTestResult, StockLocation class StockAPITestCase(InvenTreeAPITestCase): @@ -934,6 +934,50 @@ class StockTestResultTest(StockAPITestCase): # Check that an attachment has been uploaded self.assertIsNotNone(response.data['attachment']) + def test_bulk_delete(self): + """Test that the BulkDelete endpoint works for this model""" + + n = StockItemTestResult.objects.count() + + tests = [] + + url = reverse('api-stock-test-result-list') + + # Create some objects (via the API) + for _ii in range(50): + response = self.post( + url, + { + 'stock_item': 1, + 'test': f"Some test {_ii}", + 'result': True, + 'value': 'Test result value' + }, + expected_code=201 + ) + + tests.append(response.data['pk']) + + self.assertEqual(StockItemTestResult.objects.count(), n + 50) + + # Attempt a delete without providing items + self.delete( + url, + {}, + expected_code=400, + ) + + # Now, let's delete all the newly created items with a single API request + response = self.delete( + url, + { + 'items': tests, + }, + expected_code=204 + ) + + self.assertEqual(StockItemTestResult.objects.count(), n) + class StockAssignTest(StockAPITestCase): """Unit tests for the stock assignment API endpoint, where stock items are manually assigned to a customer.""" diff --git a/InvenTree/templates/js/translated/api.js b/InvenTree/templates/js/translated/api.js index eda93984f6..0bb8b2b2ea 100644 --- a/InvenTree/templates/js/translated/api.js +++ b/InvenTree/templates/js/translated/api.js @@ -7,7 +7,6 @@ /* exported inventreeGet, inventreeDelete, - inventreeMultiDelete, inventreeFormDataUpload, showApiError, */ @@ -160,59 +159,20 @@ function inventreePut(url, data={}, options={}) { } +/* + * Performs a DELETE API call to the server + */ function inventreeDelete(url, options={}) { - /* - * Delete a record - */ options = options || {}; options.method = 'DELETE'; - return inventreePut(url, {}, options); -} - - -/* - * Perform a 'multi delete' operation: - * - * - Items are deleted sequentially from the database, rather than simultaneous requests - * - This prevents potential overload / transaction issues in the DB backend - * - * Notes: - * - Assumes that each item in the 'items' list has a parameter 'pk' - */ -function inventreeMultiDelete(url, items, options={}) { - - if (!url.endsWith('/')) { - url += '/'; - } - - function doNextDelete() { - if (items.length > 0) { - var item = items.shift(); - - inventreeDelete(`${url}${item.pk}/`, { - complete: doNextDelete - }); - } else { - if (options.modal) { - $(options.modal).modal('hide'); - } - - if (options.success) { - options.success(); - } - } - } - - if (options.modal) { - showModalSpinner(options.modal); - } - - // Initiate the process - doNextDelete(); - + return inventreePut( + url, + options.data || {}, + options + ); } diff --git a/InvenTree/templates/js/translated/attachment.js b/InvenTree/templates/js/translated/attachment.js index 7d89725a65..fcf89fe7e2 100644 --- a/InvenTree/templates/js/translated/attachment.js +++ b/InvenTree/templates/js/translated/attachment.js @@ -86,9 +86,11 @@ function deleteAttachments(attachments, url, options={}) { } var rows = ''; + var ids = []; attachments.forEach(function(att) { rows += renderAttachment(att); + ids.push(att.pk); }); var html = ` @@ -105,22 +107,16 @@ function deleteAttachments(attachments, url, options={}) { `; - constructFormBody({}, { + constructForm(url, { method: 'DELETE', title: '{% trans "Delete Attachments" %}', preFormContent: html, - onSubmit: function(fields, opts) { - inventreeMultiDelete( - url, - attachments, - { - modal: opts.modal, - success: function() { - // Refresh the table once all attachments are deleted - $('#attachment-table').bootstrapTable('refresh'); - } - } - ); + form_data: { + items: ids, + }, + onSuccess: function() { + // Refresh the table once all attachments are deleted + $('#attachment-table').bootstrapTable('refresh'); } }); } diff --git a/InvenTree/templates/js/translated/bom.js b/InvenTree/templates/js/translated/bom.js index c62e29bdf5..b0f0999f35 100644 --- a/InvenTree/templates/js/translated/bom.js +++ b/InvenTree/templates/js/translated/bom.js @@ -672,9 +672,11 @@ function deleteBomItems(items, options={}) { } var rows = ''; + var ids = []; items.forEach(function(item) { rows += renderItem(item); + ids.push(item.pk); }); var html = ` @@ -692,22 +694,14 @@ function deleteBomItems(items, options={}) { `; - constructFormBody({}, { + constructForm('{% url "api-bom-list" %}', { method: 'DELETE', title: '{% trans "Delete selected BOM items?" %}', - fields: {}, - preFormContent: html, - onSubmit: function(fields, opts) { - - inventreeMultiDelete( - '{% url "api-bom-list" %}', - items, - { - modal: opts.modal, - success: options.success, - } - ); + form_data: { + items: ids, }, + preFormContent: html, + onSuccess: options.success, }); } diff --git a/InvenTree/templates/js/translated/company.js b/InvenTree/templates/js/translated/company.js index e560d6a72f..7254fe461b 100644 --- a/InvenTree/templates/js/translated/company.js +++ b/InvenTree/templates/js/translated/company.js @@ -3,7 +3,6 @@ /* globals constructForm, imageHoverIcon, - inventreeMultiDelete, loadTableFilters, makeIconButton, renderLink, @@ -238,9 +237,11 @@ function deleteSupplierParts(parts, options={}) { } var rows = ''; + var ids = []; parts.forEach(function(sup_part) { rows += renderPart(sup_part); + ids.push(sup_part.pk); }); var html = ` @@ -258,21 +259,14 @@ function deleteSupplierParts(parts, options={}) { `; - constructFormBody({}, { + constructForm('{% url "api-supplier-part-list" %}', { method: 'DELETE', title: '{% trans "Delete Supplier Parts" %}', preFormContent: html, - onSubmit: function(fields, opts) { - - inventreeMultiDelete( - '{% url "api-supplier-part-list" %}', - parts, - { - modal: opts.modal, - success: options.success - } - ); - } + form_data: { + items: ids, + }, + onSuccess: options.success, }); } @@ -472,9 +466,11 @@ function deleteManufacturerParts(selections, options={}) { } var rows = ''; + var ids = []; selections.forEach(function(man_part) { rows += renderPart(man_part); + ids.push(man_part.pk); }); var html = ` @@ -491,21 +487,14 @@ function deleteManufacturerParts(selections, options={}) { `; - constructFormBody({}, { + constructForm('{% url "api-manufacturer-part-list" %}', { method: 'DELETE', title: '{% trans "Delete Manufacturer Parts" %}', preFormContent: html, - onSubmit: function(fields, opts) { - - inventreeMultiDelete( - '{% url "api-manufacturer-part-list" %}', - selections, - { - modal: opts.modal, - success: options.success, - } - ); - } + form_data: { + items: ids, + }, + onSuccess: options.success, }); } @@ -525,9 +514,11 @@ function deleteManufacturerPartParameters(selections, options={}) { } var rows = ''; + var ids = []; selections.forEach(function(param) { rows += renderParam(param); + ids.push(param.pk); }); var html = ` @@ -543,20 +534,14 @@ function deleteManufacturerPartParameters(selections, options={}) { `; - constructFormBody({}, { + constructForm('{% url "api-manufacturer-part-parameter-list" %}', { method: 'DELETE', title: '{% trans "Delete Parameters" %}', preFormContent: html, - onSubmit: function(fields, opts) { - inventreeMultiDelete( - '{% url "api-manufacturer-part-parameter-list" %}', - selections, - { - modal: opts.modal, - success: options.success, - } - ); - } + form_data: { + items: ids, + }, + onSuccess: options.success, }); } diff --git a/InvenTree/templates/js/translated/forms.js b/InvenTree/templates/js/translated/forms.js index 794c282e6e..9f536fd548 100644 --- a/InvenTree/templates/js/translated/forms.js +++ b/InvenTree/templates/js/translated/forms.js @@ -725,7 +725,8 @@ function submitFormData(fields, options) { // Only used if file / image upload is required var form_data = new FormData(); - var data = {}; + // We can (optionally) provide a "starting point" for the submitted data + var data = options.form_data || {}; var has_files = false; diff --git a/InvenTree/templates/js/translated/stock.js b/InvenTree/templates/js/translated/stock.js index b31eb704a9..ec5d74f1de 100644 --- a/InvenTree/templates/js/translated/stock.js +++ b/InvenTree/templates/js/translated/stock.js @@ -12,7 +12,6 @@ handleFormErrors, imageHoverIcon, inventreeGet, - inventreeMultiDelete, inventreePut, launchModalForm, linkButtonsToSelection, @@ -1107,12 +1106,23 @@ function adjustStock(action, items, options={}) { // Delete action is handled differently if (action == 'delete') { - inventreeMultiDelete( + var ids = []; + + items.forEach(function(item) { + ids.push(item.pk); + }); + + showModalSpinner(opts.modal, true); + inventreeDelete( '{% url "api-stock-list" %}', - items, { - modal: opts.modal, - success: options.success, + data: { + items: ids, + }, + success: function(response) { + $(opts.modal).modal('hide'); + options.success(response); + } } );