fixup! ✨(backend) allow masking documents from the list view

Author: sampaccoud

src/backend/core/api/viewsets.py

@@ -1108,10 +1108,7 @@ def favorite(self, request, *args, **kwargs):
             document=document, user=user
         ).delete()
         if deleted:
-            return drf.response.Response(
-                {"detail": "Document unmarked as favorite"},
-                status=drf.status.HTTP_204_NO_CONTENT,
-            )
+            return drf.response.Response(status=drf.status.HTTP_204_NO_CONTENT)
         return drf.response.Response(
             {"detail": "Document was already not marked as favorite"},
             status=drf.status.HTTP_200_OK,
@@ -1124,37 +1121,36 @@ def mask(self, request, *args, **kwargs):
         document = self.get_object()
         user = request.user
 
-        if request.method == "POST":
-            link_trace, created = models.LinkTrace.objects.get_or_create(
-                document=document, user=user, defaults={"is_masked": True}
+        try:
+            link_trace = models.LinkTrace.objects.get(document=document, user=user)
+        except models.LinkTrace.DoesNotExist:
+            return drf.response.Response(
+                {"detail": "User never accessed this document before."},
+                status=status.HTTP_400_BAD_REQUEST,
             )
-            if not created and link_trace.is_masked:
+
+        if request.method == "POST":
+            if link_trace.is_masked:
                 return drf.response.Response(
                     {"detail": "Document was already masked"},
                     status=drf.status.HTTP_200_OK,
                 )
             link_trace.is_masked = True
-            link_trace.save()
+            link_trace.save(update_fields=["is_masked"])
             return drf.response.Response(
                 {"detail": "Document was masked"},
                 status=drf.status.HTTP_201_CREATED,
             )
 
         # Handle DELETE method to unmask document
-        try:
-            link_trace = models.LinkTrace.objects.get(document=document, user=user)
-        except models.LinkTrace.DoesNotExist:
+        if not link_trace.is_masked:
             return drf.response.Response(
                 {"detail": "Document was already not masked"},
                 status=drf.status.HTTP_200_OK,
             )
-
         link_trace.is_masked = False
-        link_trace.save()
-        return drf.response.Response(
-            {"detail": "Document was unmasked"},
-            status=drf.status.HTTP_204_NO_CONTENT,
-        )
+        link_trace.save(update_fields=["is_masked"])
+        return drf.response.Response(status=drf.status.HTTP_204_NO_CONTENT)
 
     @drf.decorators.action(detail=True, methods=["post"], url_path="attachment-upload")
     def attachment_upload(self, request, *args, **kwargs):

src/backend/core/tests/documents/test_api_documents_favorite_list.py

@@ -41,8 +41,8 @@ def test_api_document_favorite_list_authenticated_with_favorite():
     client = APIClient()
     client.force_login(user)
 
-    # User don't have access to this document (e.g the user had access and this
-    # access was removed. It should not be in the favorite list anymore.
+    # If the user doesn't have access to this document (e.g the user had access
+    # and this access was removed), it should not be in the favorite list anymore.
     factories.DocumentFactory(favorited_by=[user])
 
     document = factories.UserDocumentAccessFactory(

src/backend/core/tests/documents/test_api_documents_mask.py

@@ -54,6 +54,13 @@ def test_api_document_mask_authenticated_post_allowed(reach, has_role):
     if has_role:
         models.DocumentAccess.objects.create(document=document, user=user)
 
+    # Try masking the document without a link trace
+    response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/")
+    assert response.status_code == 400
+    assert response.json() == {"detail": "User never accessed this document before."}
+    assert not models.LinkTrace.objects.filter(document=document, user=user).exists()
+
+    models.LinkTrace.objects.create(document=document, user=user)
     # Mask document
     response = client.post(f"/api/v1.0/documents/{document.id!s}/mask/")
 
@@ -211,6 +218,10 @@ def test_api_document_mask_authenticated_delete_allowed(reach, has_role):
     response = client.delete(f"/api/v1.0/documents/{document.id!s}/mask/")
 
     assert response.status_code == 204
+    assert response.content == b""  # No body
+    assert response.text == ""  # Empty decoded text
+    assert "Content-Type" not in response.headers  # No Content-Type for 204
+
     assert models.LinkTrace.objects.filter(
         document=document, user=user, is_masked=False
     ).exists()
@@ -259,14 +270,21 @@ def test_api_document_mask_authenticated_delete_not_masked_allowed(reach, has_ro
     if has_role:
         models.DocumentAccess.objects.create(document=document, user=user)
 
-    # Try to unmask when no entry exists
+    # Try unmasking the document without a link trace
+    response = client.delete(f"/api/v1.0/documents/{document.id!s}/mask/")
+    assert response.status_code == 400
+    assert response.json() == {"detail": "User never accessed this document before."}
+    assert not models.LinkTrace.objects.filter(document=document, user=user).exists()
+
+    models.LinkTrace.objects.create(document=document, user=user, is_masked=False)
+    # Unmask document
     response = client.delete(f"/api/v1.0/documents/{document.id!s}/mask/")
 
     assert response.status_code == 200
     assert response.json() == {"detail": "Document was already not masked"}
-    assert (
-        models.LinkTrace.objects.filter(document=document, user=user).exists() is False
-    )
+    assert models.LinkTrace.objects.filter(
+        document=document, user=user, is_masked=False
+    ).exists()
 
 
 def test_api_document_mask_authenticated_delete_not_masked_forbidden():
@@ -310,6 +328,7 @@ def test_api_document_mask_authenticated_post_unmark_then_mark_again_allowed(
     document = factories.DocumentFactory(link_reach=reach)
     if has_role:
         models.DocumentAccess.objects.create(document=document, user=user)
+    models.LinkTrace.objects.create(document=document, user=user, is_masked=False)
 
     url = f"/api/v1.0/documents/{document.id!s}/mask/"
 
@@ -320,6 +339,9 @@ def test_api_document_mask_authenticated_post_unmark_then_mark_again_allowed(
     # Unmask document
     response = client.delete(url)
     assert response.status_code == 204
+    assert response.content == b""  # No body
+    assert response.text == ""  # Empty decoded text
+    assert "Content-Type" not in response.headers  # No Content-Type for 204
 
     # Mask document again
     response = client.post(url)