Merge pull request #296 from avantifellows/video-update-warning

Adds support for deleting items in bulks + deleting linked image if question is deleted

plio/signals.py

@@ -28,3 +28,15 @@ def item_update_cache(sender, instance, **kwargs):
 def question_update_cache(sender, instance, **kwargs):
     # invalidate saved cache for the plio
     invalidate_cache_for_instance(instance.item.plio)
+
+
+@receiver([post_save], sender=Question)
+def delete_linked_image_on_question_deletion(sender, instance, **kwargs):
+    # since we are using soft deletion, the `post_delete` signal is never called
+    # also, the way deletion works under the hood when using soft delete is that it
+    # just sets the `deleted` attribute and updates the instance. so, by default,
+    # the deleted attribute is None. when we execute soft_delete, that attribute gets set
+    if instance.deleted is not None and instance.image is not None:
+        # if any image is linked to the question instance,
+        # (soft) delete that image as well
+        instance.image.delete()

plio/tests.py

@@ -1296,6 +1296,53 @@ def test_updating_item_updates_linked_plio_instance_cache(self):
         # check plio cache with the update item time
         self.assertEqual(cache.get(cache_key_name)["items"][0]["time"], item_new_time)
 
+    def test_bulk_delete_fails_without_id(self):
+        response = self.client.delete("/api/v1/items/bulk_delete/")
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+        self.assertEqual(response.data["detail"], "item id(s) not provided")
+
+    def test_bulk_delete_fails_with_non_list_id(self):
+        response = self.client.delete("/api/v1/items/bulk_delete/", {"id": 1})
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+        self.assertEqual(
+            response.data["detail"],
+            "id should contain a list of item ids to be deleted",
+        )
+
+    def test_bulk_delete_fails_with_non_existing_item_ids(self):
+        response = self.client.delete(
+            "/api/v1/items/bulk_delete/",
+            json.dumps({"id": [self.item.id, 100]}),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        self.assertEqual(
+            response.data["detail"], "one or more of the ids provided do not exist"
+        )
+
+    def test_bulk_delete(self):
+        # create a few more items and associate with different plios
+        item_2 = Item.objects.create(type="question", plio=self.plio, time=10)
+        plio = Plio.objects.create(
+            name="Plio 2", video=self.video, created_by=self.user
+        )
+        item_3 = Item.objects.create(type="question", plio=plio, time=20)
+
+        item_ids = [self.item.id, item_2.id, item_3.id]
+
+        response = self.client.delete(
+            "/api/v1/items/bulk_delete/",
+            json.dumps({"id": item_ids}),
+            content_type="application/json",
+        )
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        for item_id in item_ids:
+            response = self.client.get(f"/api/v1/items/{item_id}/")
+            self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
 
 class QuestionTestCase(BaseTestCase):
     def setUp(self):
@@ -1497,6 +1544,27 @@ def test_updating_question_updates_linked_plio_instance_cache(self):
             cache.get(cache_key_name)["items"][0]["details"]["text"], question_new_text
         )
 
+    def test_deleting_question_deletes_linked_image(self):
+        # upload a test image and retrieve the id
+        with open("plio/static/plio/test_image.jpeg", "rb") as img:
+            response = self.client.post(
+                reverse("images-list"), {"url": img, "alt_text": "test image"}
+            )
+        image_id = response.json()["id"]
+
+        # attach image id to question
+        response = self.client.put(
+            reverse("questions-detail", args=[self.question.id]),
+            {"item": self.item.id, "image": image_id},
+        )
+
+        # delete question
+        response = self.client.delete(f"/api/v1/questions/{self.question.id}/")
+
+        # the image should be deleted as well
+        response = self.client.get(f"/api/v1/images/{image_id}/")
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+
 
 class ImageTestCase(BaseTestCase):
     """Tests the Image CRUD functionality."""

plio/views.py

@@ -240,7 +240,6 @@ def play(self, request, uuid):
     @action(
         methods=["post"],
         detail=True,
-        permission_classes=[IsAuthenticated, PlioPermission],
     )
     def duplicate(self, request, uuid):
         """Creates a clone of the plio with the given uuid"""
@@ -257,7 +256,6 @@ def duplicate(self, request, uuid):
     @action(
         methods=["post"],
         detail=True,
-        permission_classes=[IsAuthenticated, PlioPermission],
     )
     def copy(self, request, uuid):
         """Copies the given plio to another workspace"""
@@ -355,7 +353,6 @@ def copy(self, request, uuid):
     @action(
         methods=["get"],
         detail=True,
-        permission_classes=[IsAuthenticated, PlioPermission],
     )
     def metrics(self, request, uuid):
         """Returns usage metrics for the plio"""
@@ -518,7 +515,6 @@ def is_answer_correct(row):
     @action(
         methods=["get"],
         detail=True,
-        permission_classes=[IsAuthenticated, PlioPermission],
     )
     def download_data(self, request, uuid):
         """
@@ -686,7 +682,7 @@ def get_queryset(self):
             queryset = queryset.filter(plio__uuid=plio_uuid).order_by("time")
         return queryset
 
-    @action(methods=["post"], detail=True, permission_classes=[IsAuthenticated])
+    @action(methods=["post"], detail=True)
     def duplicate(self, request, pk):
         """
         Creates a clone of the item with the given pk and links it to the plio
@@ -711,6 +707,34 @@ def duplicate(self, request, pk):
         item.save()
         return Response(self.get_serializer(item).data)
 
+    @action(methods=["delete"], detail=False)
+    def bulk_delete(self, request):
+        """deletes items whose ids have been provided"""
+        if "id" not in request.data:
+            return Response(
+                {"detail": "item id(s) not provided"},
+                status=status.HTTP_400_BAD_REQUEST,
+            )
+
+        ids_to_delete = request.data["id"]
+
+        # ensure that a list of ids has been provided
+        if not isinstance(ids_to_delete, list):
+            return Response(
+                {"detail": "id should contain a list of item ids to be deleted"},
+                status=status.HTTP_400_BAD_REQUEST,
+            )
+
+        items_to_delete = Item.objects.filter(pk__in=ids_to_delete)
+        if len(items_to_delete) != len(ids_to_delete):
+            return Response(
+                {"detail": "one or more of the ids provided do not exist"},
+                status=status.HTTP_404_NOT_FOUND,
+            )
+
+        items_to_delete.delete()
+        return Response("deletion successful")
+
 
 class QuestionViewSet(viewsets.ModelViewSet):
     """
@@ -728,7 +752,7 @@ class QuestionViewSet(viewsets.ModelViewSet):
     serializer_class = QuestionSerializer
     permission_classes = [IsAuthenticated, PlioPermission]
 
-    @action(methods=["post"], detail=True, permission_classes=[IsAuthenticated])
+    @action(methods=["post"], detail=True)
     def duplicate(self, request, pk):
         """
         Creates a clone of the question with the given pk and links it to the item
@@ -774,8 +798,9 @@ class ImageViewSet(viewsets.ModelViewSet):
 
     queryset = Image.objects.all()
     serializer_class = ImageSerializer
+    permission_classes = [IsAuthenticated]
 
-    @action(methods=["post"], detail=True, permission_classes=[IsAuthenticated])
+    @action(methods=["post"], detail=True)
     def duplicate(self, request, pk):
         """
         Creates a clone of the image with the given pk

users/views.py

@@ -46,7 +46,6 @@ class UserViewSet(viewsets.ModelViewSet):
     @action(
         detail=True,
         methods=["patch", "get"],
-        permission_classes=[IsAuthenticated, UserPermission],
     )
     def config(self, request, pk):
         user = self.get_object()