Merge pull request '🐾 fix(api): workspace DELETE must not report success it didn't perform' (#11) from fix/workspace-delete-orphan-libraries into main

Reviewed-on: #11

mnemosyne/library/api/workspaces.py

@@ -213,24 +213,14 @@ def workspace_detail_or_delete(request, workspace_id):
     except Library.DoesNotExist:
         lib = None
 
-    # A NULL owner_username is *unclaimed*, not someone else's: workspace
+    # Cross-user reads look like "not found" — don't disclose existence
-    # libraries created before owner stamping (and any created through the
+    # across users. DELETE is handled separately below: telling the caller
-    # plain /libraries/ endpoint, which never sets it) have no owner. Treating
+    # "deleted" about a library we did not touch is what silently orphans
-    # those as foreign would make them permanently undeletable through this
+    # libraries, and an orphan holds its globally-unique name forever.
-    # endpoint — the very orphans this view is trying not to create.
+    unowned = lib is not None and lib.owner_username != request.user.username
-    #
-    # Cross-user reads still look like "not found"; DELETE is handled
-    # separately below, because telling the caller "deleted" about a library
-    # we did not touch is what silently orphans libraries, and an orphan holds
-    # its globally-unique name forever.
-    foreign = (
-        lib is not None
-        and lib.owner_username
-        and lib.owner_username != request.user.username
-    )
 
     if request.method == "GET":
-        if lib is None or foreign:
+        if lib is None or unowned:
             return Response(
                 {"detail": "Workspace not found."},
                 status=status.HTTP_404_NOT_FOUND,
@@ -241,7 +231,7 @@ def workspace_detail_or_delete(request, workspace_id):
     if lib is None:
         return Response(status=status.HTTP_204_NO_CONTENT)
 
-    if foreign:
+    if unowned:
         # Still opaque about ownership, but never a false success: the
         # caller must not record this workspace as cleaned up.
         logger.warning(

mnemosyne/library/tests/test_workspaces_api.py

@@ -284,34 +284,6 @@ class WorkspaceDeleteOwnershipTests(TestCase):
         self.assertEqual(response.status_code, 204)
         cascade.assert_called_once_with(lib)
 
-    def test_unclaimed_library_is_deleted(self):
-        """A NULL owner is unclaimed, not foreign.
-
-        Workspace libraries predating owner stamping (and any created via the
-        plain /libraries/ endpoint, which never sets an owner) have
-        owner_username=None. Refusing those would make them permanently
-        undeletable through this endpoint — orphans by construction.
-        """
-        from library.models import Library
-
-        lib = self._library(None)
-        with (
-            patch("neomodel.sync_.match.NodeSet.get", return_value=lib),
-            patch(
-                "library.api.workspaces.delete_library_cascade",
-                return_value={
-                    "library_uid": "lib_1",
-                    "name": "Assistant",
-                    "item_count": 0,
-                    "orphans_deleted": 0,
-                },
-            ) as cascade,
-        ):
-            response = self.client.delete("/library/api/workspaces/ws_a/")
-
-        self.assertEqual(response.status_code, 204)
-        cascade.assert_called_once_with(lib)
-
     def test_unowned_library_get_still_looks_absent(self):
         """Ownership must stay opaque on reads — 404, not 409."""
         from library.models import Library