Enhancement: bulk delete objects (#5688)

This commit is contained in:
shamoon
2024-02-08 10:13:15 -08:00
committed by GitHub
parent b60e16fe33
commit b643a68fa3
15 changed files with 535 additions and 221 deletions

View File

@@ -1281,7 +1281,7 @@ class ShareLinkSerializer(OwnedObjectSerializer):
return super().create(validated_data)
class BulkEditObjectPermissionsSerializer(serializers.Serializer, SetPermissionsMixin):
class BulkEditObjectsSerializer(serializers.Serializer, SetPermissionsMixin):
objects = serializers.ListField(
required=True,
allow_empty=False,
@@ -1301,6 +1301,16 @@ class BulkEditObjectPermissionsSerializer(serializers.Serializer, SetPermissions
write_only=True,
)
operation = serializers.ChoiceField(
choices=[
"set_permissions",
"delete",
],
label="Operation",
required=True,
write_only=True,
)
owner = serializers.PrimaryKeyRelatedField(
queryset=User.objects.all(),
required=False,
@@ -1353,11 +1363,14 @@ class BulkEditObjectPermissionsSerializer(serializers.Serializer, SetPermissions
def validate(self, attrs):
object_type = attrs["object_type"]
objects = attrs["objects"]
permissions = attrs.get("permissions")
operation = attrs.get("operation")
self._validate_objects(objects, object_type)
if permissions is not None:
self._validate_permissions(permissions)
if operation == "set_permissions":
permissions = attrs.get("permissions")
if permissions is not None:
self._validate_permissions(permissions)
return attrs

View File

@@ -222,3 +222,118 @@ class TestApiStoragePaths(DirectoriesMixin, APITestCase):
args, _ = bulk_update_mock.call_args
self.assertCountEqual([document.pk], args[0])
class TestBulkEditObjects(APITestCase):
# See test_api_permissions.py for bulk tests on permissions
def setUp(self):
super().setUp()
self.temp_admin = User.objects.create_superuser(username="temp_admin")
self.client.force_authenticate(user=self.temp_admin)
self.t1 = Tag.objects.create(name="t1")
self.t2 = Tag.objects.create(name="t2")
self.c1 = Correspondent.objects.create(name="c1")
self.dt1 = DocumentType.objects.create(name="dt1")
self.sp1 = StoragePath.objects.create(name="sp1")
self.user1 = User.objects.create(username="user1")
self.user2 = User.objects.create(username="user2")
self.user3 = User.objects.create(username="user3")
def test_bulk_objects_delete(self):
"""
GIVEN:
- Existing objects
WHEN:
- bulk_edit_objects API endpoint is called with delete operation
THEN:
- Objects are deleted
"""
response = self.client.post(
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"operation": "delete",
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(Tag.objects.count(), 0)
response = self.client.post(
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.c1.id],
"object_type": "correspondents",
"operation": "delete",
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(Correspondent.objects.count(), 0)
response = self.client.post(
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.dt1.id],
"object_type": "document_types",
"operation": "delete",
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(DocumentType.objects.count(), 0)
response = self.client.post(
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.sp1.id],
"object_type": "storage_paths",
"operation": "delete",
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_200_OK)
self.assertEqual(StoragePath.objects.count(), 0)
def test_bulk_edit_object_permissions_insufficient_perms(self):
"""
GIVEN:
- Objects owned by user other than logged in user
WHEN:
- bulk_edit_objects API endpoint is called with delete operation
THEN:
- User is not able to delete objects
"""
self.t1.owner = User.objects.get(username="temp_admin")
self.t1.save()
self.client.force_authenticate(user=self.user1)
response = self.client.post(
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"operation": "delete",
},
),
content_type="application/json",
)
self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
self.assertEqual(response.content, b"Insufficient permissions")

View File

@@ -717,7 +717,7 @@ class TestBulkEditObjectPermissions(APITestCase):
GIVEN:
- Existing objects
WHEN:
- bulk_edit_object_perms API endpoint is called
- bulk_edit_objects API endpoint is called with set_permissions operation
THEN:
- Permissions and / or owner are changed
"""
@@ -733,11 +733,12 @@ class TestBulkEditObjectPermissions(APITestCase):
}
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"operation": "set_permissions",
"permissions": permissions,
},
),
@@ -748,11 +749,12 @@ class TestBulkEditObjectPermissions(APITestCase):
self.assertIn(self.user1, get_users_with_perms(self.t1))
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.c1.id],
"object_type": "correspondents",
"operation": "set_permissions",
"permissions": permissions,
},
),
@@ -763,11 +765,12 @@ class TestBulkEditObjectPermissions(APITestCase):
self.assertIn(self.user1, get_users_with_perms(self.c1))
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.dt1.id],
"object_type": "document_types",
"operation": "set_permissions",
"permissions": permissions,
},
),
@@ -778,11 +781,12 @@ class TestBulkEditObjectPermissions(APITestCase):
self.assertIn(self.user1, get_users_with_perms(self.dt1))
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.sp1.id],
"object_type": "storage_paths",
"operation": "set_permissions",
"permissions": permissions,
},
),
@@ -793,11 +797,12 @@ class TestBulkEditObjectPermissions(APITestCase):
self.assertIn(self.user1, get_users_with_perms(self.sp1))
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"operation": "set_permissions",
"owner": self.user3.id,
},
),
@@ -808,11 +813,12 @@ class TestBulkEditObjectPermissions(APITestCase):
self.assertEqual(Tag.objects.get(pk=self.t2.id).owner, self.user3)
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.sp1.id],
"object_type": "storage_paths",
"operation": "set_permissions",
"owner": self.user3.id,
},
),
@@ -827,7 +833,7 @@ class TestBulkEditObjectPermissions(APITestCase):
GIVEN:
- Existing objects
WHEN:
- bulk_edit_object_perms API endpoint is called with merge=True or merge=False (default)
- bulk_edit_objects API endpoint is called with set_permissions operation with merge=True or merge=False (default)
THEN:
- Permissions and / or owner are replaced or merged, depending on the merge flag
"""
@@ -848,13 +854,14 @@ class TestBulkEditObjectPermissions(APITestCase):
# merge=True
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"owner": self.user1.id,
"permissions": permissions,
"operation": "set_permissions",
"merge": True,
},
),
@@ -877,12 +884,13 @@ class TestBulkEditObjectPermissions(APITestCase):
# merge=False (default)
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"permissions": permissions,
"operation": "set_permissions",
"merge": False,
},
),
@@ -900,7 +908,7 @@ class TestBulkEditObjectPermissions(APITestCase):
GIVEN:
- Objects owned by user other than logged in user
WHEN:
- bulk_edit_object_perms API endpoint is called
- bulk_edit_objects API endpoint is called with set_permissions operation
THEN:
- User is not able to change permissions
"""
@@ -909,11 +917,12 @@ class TestBulkEditObjectPermissions(APITestCase):
self.client.force_authenticate(user=self.user1)
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id],
"object_type": "tags",
"operation": "set_permissions",
"owner": self.user1.id,
},
),
@@ -928,17 +937,18 @@ class TestBulkEditObjectPermissions(APITestCase):
GIVEN:
- Existing objects
WHEN:
- bulk_edit_object_perms API endpoint is called with invalid params
- bulk_edit_objects API endpoint is called with set_permissions operation with invalid params
THEN:
- Validation fails
"""
# not a list
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": self.t1.id,
"object_type": "tags",
"operation": "set_permissions",
"owner": self.user1.id,
},
),
@@ -949,7 +959,7 @@ class TestBulkEditObjectPermissions(APITestCase):
# not a list of ints
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": ["one"],
@@ -964,11 +974,12 @@ class TestBulkEditObjectPermissions(APITestCase):
# duplicates
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [self.t1.id, self.t2.id, self.t1.id],
"object_type": "tags",
"operation": "set_permissions",
"owner": self.user1.id,
},
),
@@ -979,11 +990,12 @@ class TestBulkEditObjectPermissions(APITestCase):
# not a valid object type
response = self.client.post(
"/api/bulk_edit_object_perms/",
"/api/bulk_edit_objects/",
json.dumps(
{
"objects": [1],
"object_type": "madeup",
"operation": "set_permissions",
"owner": self.user1.id,
},
),

View File

@@ -115,7 +115,7 @@ from documents.permissions import has_perms_owner_aware
from documents.permissions import set_permissions_for_object
from documents.serialisers import AcknowledgeTasksViewSerializer
from documents.serialisers import BulkDownloadSerializer
from documents.serialisers import BulkEditObjectPermissionsSerializer
from documents.serialisers import BulkEditObjectsSerializer
from documents.serialisers import BulkEditSerializer
from documents.serialisers import CorrespondentSerializer
from documents.serialisers import CustomFieldSerializer
@@ -1401,9 +1401,9 @@ def serve_file(doc: Document, use_archive: bool, disposition: str):
return response
class BulkEditObjectPermissionsView(GenericAPIView, PassUserMixin):
class BulkEditObjectsView(GenericAPIView, PassUserMixin):
permission_classes = (IsAuthenticated,)
serializer_class = BulkEditObjectPermissionsSerializer
serializer_class = BulkEditObjectsSerializer
parser_classes = (parsers.JSONParser,)
def post(self, request, *args, **kwargs):
@@ -1414,42 +1414,52 @@ class BulkEditObjectPermissionsView(GenericAPIView, PassUserMixin):
object_type = serializer.validated_data.get("object_type")
object_ids = serializer.validated_data.get("objects")
object_class = serializer.get_object_class(object_type)
permissions = serializer.validated_data.get("permissions")
owner = serializer.validated_data.get("owner")
merge = serializer.validated_data.get("merge")
operation = serializer.validated_data.get("operation")
objs = object_class.objects.filter(pk__in=object_ids)
if not user.is_superuser:
objs = object_class.objects.filter(pk__in=object_ids)
has_perms = all((obj.owner == user or obj.owner is None) for obj in objs)
if not has_perms:
return HttpResponseForbidden("Insufficient permissions")
try:
qs = object_class.objects.filter(id__in=object_ids)
if operation == "set_permissions":
permissions = serializer.validated_data.get("permissions")
owner = serializer.validated_data.get("owner")
merge = serializer.validated_data.get("merge")
# if merge is true, we dont want to remove the owner
if "owner" in serializer.validated_data and (
not merge or (merge and owner is not None)
):
# if merge is true, we dont want to overwrite the owner
qs_owner_update = qs.filter(owner__isnull=True) if merge else qs
qs_owner_update.update(owner=owner)
try:
qs = object_class.objects.filter(id__in=object_ids)
if "permissions" in serializer.validated_data:
for obj in qs:
set_permissions_for_object(
permissions=permissions,
object=obj,
merge=merge,
)
# if merge is true, we dont want to remove the owner
if "owner" in serializer.validated_data and (
not merge or (merge and owner is not None)
):
# if merge is true, we dont want to overwrite the owner
qs_owner_update = qs.filter(owner__isnull=True) if merge else qs
qs_owner_update.update(owner=owner)
return Response({"result": "OK"})
except Exception as e:
logger.warning(f"An error occurred performing bulk permissions edit: {e!s}")
return HttpResponseBadRequest(
"Error performing bulk permissions edit, check logs for more detail.",
)
if "permissions" in serializer.validated_data:
for obj in qs:
set_permissions_for_object(
permissions=permissions,
object=obj,
merge=merge,
)
except Exception as e:
logger.warning(
f"An error occurred performing bulk permissions edit: {e!s}",
)
return HttpResponseBadRequest(
"Error performing bulk permissions edit, check logs for more detail.",
)
elif operation == "delete":
objs.delete()
return Response({"result": "OK"})
class WorkflowTriggerViewSet(ModelViewSet):

View File

@@ -322,7 +322,7 @@ REST_FRAMEWORK = {
"DEFAULT_VERSION": "1",
# Make sure these are ordered and that the most recent version appears
# last
"ALLOWED_VERSIONS": ["1", "2", "3", "4"],
"ALLOWED_VERSIONS": ["1", "2", "3", "4", "5"],
}
if DEBUG:

View File

@@ -16,7 +16,7 @@ from rest_framework.routers import DefaultRouter
from documents.views import AcknowledgeTasksView
from documents.views import BulkDownloadView
from documents.views import BulkEditObjectPermissionsView
from documents.views import BulkEditObjectsView
from documents.views import BulkEditView
from documents.views import CorrespondentViewSet
from documents.views import CustomFieldViewSet
@@ -129,9 +129,9 @@ urlpatterns = [
),
path("token/", views.obtain_auth_token),
re_path(
"^bulk_edit_object_perms/",
BulkEditObjectPermissionsView.as_view(),
name="bulk_edit_object_permissions",
"^bulk_edit_objects/",
BulkEditObjectsView.as_view(),
name="bulk_edit_objects",
),
path("profile/generate_auth_token/", GenerateAuthTokenView.as_view()),
path(