Add option to recursively delete part categories (#3435)

* Add option to recursively delete part categories

Fixes #3384

* - Added test (broken ATM)
- Refactored parameters to booleanish

* Fix styling issues reported by flake8

* Working on unit testing

* Added on_commit debugging callback

* Separate the recursive part of the deletion into another method
to make sure that the delete operation is performed in a single transaction

* Trying transactions with @transactions.atomic

* Fix flake8 reported issues

* Removed unused debug callback

* Fixed tests for category recursive deletion

* Fix flake reported issues

* Fix flake reported issues
Again

* Remove unrelated formatting changes

* Fixed a part of review comments
This commit is contained in:
Miklós Márton 2022-11-08 01:57:00 +01:00 committed by GitHub
parent adcb975853
commit f6cfc12343
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 218 additions and 26 deletions

View File

@ -93,11 +93,11 @@ class InvenTreeMetadata(SimpleMetadata):
# Add a 'DELETE' action if we are allowed to delete
if 'DELETE' in view.allowed_methods and check(user, table, 'delete'):
actions['DELETE'] = True
actions['DELETE'] = {}
# Add a 'VIEW' action if we are allowed to view
if 'GET' in view.allowed_methods and check(user, table, 'view'):
actions['GET'] = True
actions['GET'] = {}
metadata['actions'] = actions

View File

@ -1,6 +1,6 @@
"""Mixins for (API) views in the whole project."""
from rest_framework import generics, status
from rest_framework import generics, mixins, status
from rest_framework.response import Response
from InvenTree.helpers import remove_non_printable_characters, strip_html_tags
@ -106,6 +106,45 @@ class RetrieveUpdateAPI(CleanMixin, generics.RetrieveUpdateAPIView):
pass
class CustomDestroyModelMixin:
"""This mixin was created pass the kwargs from the API to the models."""
def destroy(self, request, *args, **kwargs):
"""Custom destroy method to pass kwargs."""
instance = self.get_object()
self.perform_destroy(instance, **kwargs)
return Response(status=status.HTTP_204_NO_CONTENT)
def perform_destroy(self, instance, **kwargs):
"""Custom destroy method to pass kwargs."""
instance.delete(**kwargs)
class CustomRetrieveUpdateDestroyAPIView(mixins.RetrieveModelMixin,
mixins.UpdateModelMixin,
CustomDestroyModelMixin,
generics.GenericAPIView):
"""This APIView was created pass the kwargs from the API to the models."""
def get(self, request, *args, **kwargs):
"""Custom get method to pass kwargs."""
return self.retrieve(request, *args, **kwargs)
def put(self, request, *args, **kwargs):
"""Custom put method to pass kwargs."""
return self.update(request, *args, **kwargs)
def patch(self, request, *args, **kwargs):
"""Custom patch method to pass kwargs."""
return self.partial_update(request, *args, **kwargs)
def delete(self, request, *args, **kwargs):
"""Custom delete method to pass kwargs."""
return self.destroy(request, *args, **kwargs)
class CustomRetrieveUpdateDestroyAPI(CleanMixin, CustomRetrieveUpdateDestroyAPIView):
"""This APIView was created pass the kwargs from the API to the models."""
class RetrieveUpdateDestroyAPI(CleanMixin, generics.RetrieveUpdateDestroyAPIView):
"""View for retrieve, update and destroy API."""

View File

@ -27,7 +27,8 @@ from InvenTree.api import (APIDownloadMixin, AttachmentMixin,
from InvenTree.filters import InvenTreeOrderingFilter
from InvenTree.helpers import (DownloadFile, increment_serial_number, isNull,
str2bool, str2int)
from InvenTree.mixins import (CreateAPI, ListAPI, ListCreateAPI, RetrieveAPI,
from InvenTree.mixins import (CreateAPI, CustomRetrieveUpdateDestroyAPI,
ListAPI, ListCreateAPI, RetrieveAPI,
RetrieveUpdateAPI, RetrieveUpdateDestroyAPI,
UpdateAPI)
from InvenTree.status_codes import (BuildStatus, PurchaseOrderStatus,
@ -179,7 +180,7 @@ class CategoryList(ListCreateAPI):
]
class CategoryDetail(RetrieveUpdateDestroyAPI):
class CategoryDetail(CustomRetrieveUpdateDestroyAPI):
"""API endpoint for detail view of a single PartCategory object."""
serializer_class = part_serializers.CategorySerializer
@ -218,6 +219,16 @@ class CategoryDetail(RetrieveUpdateDestroyAPI):
return response
def destroy(self, request, *args, **kwargs):
"""Delete a Part category instance via the API"""
delete_parts = 'delete_parts' in request.data and request.data['delete_parts'] == '1'
delete_child_categories = 'delete_child_categories' in request.data and request.data['delete_child_categories'] == '1'
return super().destroy(request,
*args,
**dict(kwargs,
delete_parts=delete_parts,
delete_child_categories=delete_child_categories))
class CategoryMetadata(RetrieveUpdateAPI):
"""API endpoint for viewing / updating PartCategory metadata."""

View File

@ -63,31 +63,46 @@ class PartCategory(MetadataMixin, InvenTreeTree):
default_keywords: Default keywords for parts created in this category
"""
def delete_recursive(self, *args, **kwargs):
"""This function handles the recursive deletion of subcategories depending on kwargs contents"""
delete_parts = kwargs.get('delete_parts', False)
parent_category = kwargs.get('parent_category', None)
if parent_category is None:
# First iteration, (no part_category kwargs passed)
parent_category = self.parent
for child_part in self.parts.all():
if delete_parts:
child_part.delete()
else:
child_part.category = parent_category
child_part.save()
for child_category in self.children.all():
if kwargs.get('delete_child_categories', False):
child_category.delete_recursive(**dict(delete_child_categories=True,
delete_parts=delete_parts,
parent_category=parent_category))
else:
child_category.parent = parent_category
child_category.save()
super().delete(*args, **dict())
def delete(self, *args, **kwargs):
"""Custom model deletion routine, which updates any child categories or parts.
This must be handled within a transaction.atomic(), otherwise the tree structure is damaged
"""
with transaction.atomic():
self.delete_recursive(**dict(delete_parts=kwargs.get('delete_parts', False),
delete_child_categories=kwargs.get('delete_child_categories', False),
parent_category=self.parent))
parent = self.parent
tree_id = self.tree_id
# Update each part in this category to point to the parent category
for p in self.parts.all():
p.category = self.parent
p.save()
# Update each child category
for child in self.children.all():
child.parent = self.parent
child.save()
super().delete(*args, **kwargs)
if parent is not None:
if self.parent is not None:
# Partially rebuild the tree (cheaper than a complete rebuild)
PartCategory.objects.partial_rebuild(tree_id)
PartCategory.objects.partial_rebuild(self.tree_id)
else:
PartCategory.objects.rebuild()

View File

@ -1,6 +1,7 @@
"""Unit tests for the various part API endpoints"""
from decimal import Decimal
from enum import IntEnum
from random import randint
from django.urls import reverse
@ -294,6 +295,114 @@ class PartCategoryAPITest(InvenTreeAPITestCase):
self.assertEqual(response.data['description'], 'A part category')
def test_category_delete(self):
"""Test category deletion with different parameters"""
class Target(IntEnum):
move_subcategories_to_parent_move_parts_to_parent = 0,
move_subcategories_to_parent_delete_parts = 1,
delete_subcategories_move_parts_to_parent = 2,
delete_subcategories_delete_parts = 3,
for i in range(4):
delete_child_categories: bool = False
delete_parts: bool = False
if i == Target.move_subcategories_to_parent_delete_parts \
or i == Target.delete_subcategories_delete_parts:
delete_parts = True
if i == Target.delete_subcategories_move_parts_to_parent \
or i == Target.delete_subcategories_delete_parts:
delete_child_categories = True
# Create a parent category
parent_category = PartCategory.objects.create(
name='Parent category',
description='This is the parent category where the child categories and parts are moved to',
parent=None
)
category_count_before = PartCategory.objects.count()
part_count_before = Part.objects.count()
# Create a category to delete
cat_to_delete = PartCategory.objects.create(
name='Category to delete',
description='This is the category to be deleted',
parent=parent_category
)
url = reverse('api-part-category-detail', kwargs={'pk': cat_to_delete.id})
parts = []
# Create parts in the category to be deleted
for jj in range(3):
parts.append(Part.objects.create(
name=f"Part xyz {jj}",
description="Child part of the deleted category",
category=cat_to_delete
))
child_categories = []
child_categories_parts = []
# Create child categories under the category to be deleted
for ii in range(3):
child = PartCategory.objects.create(
name=f"Child parent_cat {ii}",
description="A child category of the deleted category",
parent=cat_to_delete
)
child_categories.append(child)
# Create parts in the child categories
for jj in range(3):
child_categories_parts.append(Part.objects.create(
name=f"Part xyz {jj}",
description="Child part in the child category of the deleted category",
category=child
))
# Delete the created category (sub categories and their parts will be moved under the parent)
params = {}
if delete_parts:
params['delete_parts'] = '1'
if delete_child_categories:
params['delete_child_categories'] = '1'
response = self.delete(
url,
params,
expected_code=204,
)
self.assertEqual(response.status_code, 204)
if delete_parts:
if i == Target.delete_subcategories_delete_parts:
# Check if all parts deleted
self.assertEqual(Part.objects.count(), part_count_before)
elif i == Target.move_subcategories_to_parent_delete_parts:
# Check if all parts deleted
self.assertEqual(Part.objects.count(), part_count_before + len(child_categories_parts))
else:
# parts moved to the parent category
for part in parts:
part.refresh_from_db()
self.assertEqual(part.category, parent_category)
if delete_child_categories:
for part in child_categories_parts:
part.refresh_from_db()
self.assertEqual(part.category, parent_category)
if delete_child_categories:
# Check if all categories are deleted
self.assertEqual(PartCategory.objects.count(), category_count_before)
else:
# Check if all subcategories to parent moved to parent and all parts deleted
for child in child_categories:
child.refresh_from_db()
self.assertEqual(child.parent, parent_category)
class PartOptionsAPITest(InvenTreeAPITestCase):
"""Tests for the various OPTIONS endpoints in the /part/ API.

View File

@ -333,15 +333,33 @@ function deletePartCategory(pk, options={}) {
var html = `
<div class='alert alert-block alert-danger'>
{% trans "Are you sure you want to delete this part category?" %}
<ul>
<li>{% trans "Any child categories will be moved to the parent of this category" %}</li>
<li>{% trans "Any parts in this category will be moved to the parent of this category" %}</li>
</ul>
</div>`;
var subChoices = [
{
value: 0,
display_name: '{% trans "Move to parent category" %}',
},
{
value: 1,
display_name: '{% trans "Delete" %}',
}
];
constructForm(url, {
title: '{% trans "Delete Part Category" %}',
method: 'DELETE',
fields: {
'delete_parts': {
label: '{% trans "Action for parts in this category" %}',
choices: subChoices,
type: 'choice'
},
'delete_child_categories': {
label: '{% trans "Action for child categories" %}',
choices: subChoices,
type: 'choice'
},
},
preFormContent: html,
onSuccess: function(response) {
handleFormSuccess(response, options);