(backend/frontend) move access/invitation cleanup from frontend to backend on document move

[maboukerfa]

Purpose

Move the access and invitation cleanup logic from the frontend to the backend during document move operations.

Problems with the current approach:

1. No atomicity: The frontend deletes accesses and invitations via separate HTTP requests after the move succeeds. If the user loses their connection mid-process, the move completes but the old permissions remain, leaving the document in
   an inconsistent state.
2. API consumers bypass cleanup entirely: A direct API call to POST /documents/{id}/move/ moves the document without touching its permissions. This creates sub-documents with their own direct accesses that differ from their parent's,
   breaking the inheritance model.
3. Race condition on deletion: After the move, the frontend fetches the document's accesses to delete them. But since the document has already moved to a new parent, the permissions it retrieves are the inherited ones from the new tree not the old direct accesses. Attempting to delete these returns 404s.

Screen.Recording.2026-04-16.at.17.29.32.mov

Proposal

• Backend: When a document leaves its current permission scope (root document being moved, cross-tree move, or sub-document promoted to root), its direct accesses and pending invitations are deleted within the same @transaction.atomic. block as the move. If the move fails, everything rolls back.
• Frontend: The useMoveDoc hook is simplified to a plain move call, no more manual fetch-and-delete logic.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 9f4969de-3c4b-46ce-8498-6fc9a1aaeecf

📥 Commits

Reviewing files that changed from the base of the PR and between 0c0f025 and 99bfe78.

📒 Files selected for processing (2)

• src/backend/core/api/viewsets.py
• src/backend/core/tests/documents/test_api_documents_move.py

🚧 Files skipped from review as they are similar to previous changes (1)

• src/backend/core/tests/documents/test_api_documents_move.py

---

Walkthrough

The backend DocumentViewSet.move now detects permission-scope changes (root status, sibling-root placement, or root-different moves), converts owner_accesses to a list when target is root, and—when scope changes—deletes the document's direct accesses and invitations inside the same @transaction.atomic before calling document.move. Frontend useMoveDoc dropped the deleteAccessOnMove parameter and all invitation/access deletion side effects; its call sites were updated accordingly. Tests were expanded to cover root moves, cross-tree moves, same-tree moves, promotions to root, sibling-root moves, owner preservation, and atomicity on move failure.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: moving access/invitation cleanup logic from frontend to backend during document move operations.
Description check	✅ Passed	The description is directly related to the changeset, explaining the purpose, problems addressed, and the proposal for the access/invitation cleanup logic relocation.
Docstring Coverage	✅ Passed	Docstring coverage is 92.31% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/backend/core/api/viewsets.py`:
- Around line 965-971: Materialize the owner_accesses queryset before deleting
direct accesses: before calling document.accesses.all().delete() (and
document.invitations.all().delete()), evaluate owner_accesses into a concrete
list (e.g. owner_accesses = list(owner_accesses)) so the later owner-recreation
loop can iterate over the preserved items; update the code that computes
owner_accesses to materialize it and then proceed with the deletes and the owner
recreation logic.
- Around line 969-971: The cleanup condition incorrectly only compares current
roots; instead compute the prospective root after the move and use that to
decide whether to drop direct accesses/invitations. Change the logic around
document.is_root(), document.get_root(), and target_document.get_root() to
determine the new root post-move (e.g., if moving as a child use
target_document.get_root(), if moving as a sibling use
target_document.get_parent() and its root or None if that parent is root) and
then run document.accesses.all().delete() and
document.invitations.all().delete() when document.is_root() or
document.get_root() != prospective_root (i.e., when the root membership changes
after the move).

In `@src/backend/core/tests/documents/test_api_documents_move.py`:
- Around line 594-620: The test currently fails before the deletion block
because the target lookup uses a random UUID; change it to use a real target
document and force Document.move to raise after the deletes so rollback can be
observed. Concretely: create a real target_document (e.g. via
factories.DocumentFactory()) and pass its id in the POST, and monkeypatch or
temporarily stub Document.move (the method on the Document model) to raise an
exception when called; then call client.post on the move endpoint and assert the
response indicates failure and that document.accesses.count() and
document.invitations.count() remain unchanged (verifying the deletion roll
back).

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 56acc976-7a4f-4750-b14f-c62dbde5d92e

📥 Commits

Reviewing files that changed from the base of the PR and between e59d8a4 and 0c0f025.

📒 Files selected for processing (5)

• src/backend/core/api/viewsets.py
• src/backend/core/tests/documents/test_api_documents_move.py
• src/frontend/apps/impress/src/features/docs/doc-management/api/useMoveDoc.tsx
• src/frontend/apps/impress/src/features/docs/docs-grid/components/DocGridContentList.tsx
• src/frontend/apps/impress/src/features/docs/docs-grid/components/DocMoveModal.tsx

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

This rollback test never reaches the new deletion path.

With a random UUID, the view returns from the target lookup error branch before the document.accesses.all().delete() / document.invitations.all().delete() block runs. This still passes even if the transaction wrapper is removed. To verify rollback, force a failure after those deletes instead, e.g. by making Document.move(...) raise.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/core/tests/documents/test_api_documents_move.py` around lines 594
- 620, The test currently fails before the deletion block because the target
lookup uses a random UUID; change it to use a real target document and force
Document.move to raise after the deletes so rollback can be observed.
Concretely: create a real target_document (e.g. via factories.DocumentFactory())
and pass its id in the POST, and monkeypatch or temporarily stub Document.move
(the method on the Document model) to raise an exception when called; then call
client.post on the move endpoint and assert the response indicates failure and
that document.accesses.count() and document.invitations.count() remain unchanged
(verifying the deletion roll back).