Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 26 additions & 2 deletions src/backend/core/api/viewsets.py
Original file line number Diff line number Diff line change
Expand Up @@ -947,8 +947,8 @@ def move(self, request, *args, **kwargs):
"as a child to this target document."
)
elif target_document.is_root():
owner_accesses = document.get_root().accesses.filter(
role=models.RoleChoices.OWNER
owner_accesses = list(
document.get_root().accesses.filter(role=models.RoleChoices.OWNER)
)
elif not target_document.get_parent().get_abilities(user).get("move"):
message = (
Expand All @@ -970,6 +970,30 @@ def move(self, request, *args, **kwargs):
status=status.HTTP_400_BAD_REQUEST,
)

# A move changes the document's permission scope in any of these cases:
# - it is currently a root (it carries its own scope),
# - it is moving into a different tree (different current root than target's),
# - it is being promoted to root as a sibling of its own current root.
# In all these cases, direct accesses and pending invitations must be wiped so
# the document inherits the new scope. Deletions and the move share the same
# atomic transaction, so a failure rolls everything back.
becomes_sibling_root = (
position
not in [
enums.MoveNodePositionChoices.FIRST_CHILD,
enums.MoveNodePositionChoices.LAST_CHILD,
]
and target_document.is_root()
)
scope_changes = (
document.is_root()
or becomes_sibling_root
or document.get_root() != target_document.get_root()
)
if scope_changes:
document.accesses.all().delete()
document.invitations.all().delete()

# Make sure we have at least one owner
if (
owner_accesses
Expand Down
276 changes: 272 additions & 4 deletions src/backend/core/tests/documents/test_api_documents_move.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,10 @@ def test_api_documents_move_authenticated_no_owner_user_and_team():

document.refresh_from_db()
assert list(document.get_children()) == [child]
assert document.accesses.count() == 3

assert document.accesses.count() == 2
assert document.accesses.get(user__isnull=False, role="owner").user == parent_owner
assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite"
assert document.accesses.get(role="administrator").user == user


def test_api_documents_move_authenticated_no_owner_same_user():
Expand Down Expand Up @@ -304,8 +304,10 @@ def test_api_documents_move_authenticated_no_owner_same_team():

document.refresh_from_db()
assert list(document.get_children()) == [child]
assert document.accesses.count() == 2
assert document.accesses.get(user__isnull=False, role="administrator").user == user

# The user's direct administrator access was wiped during the cross-tree move;
# only the "lasuite" team remains, re-added as owner from the previous root.
assert document.accesses.count() == 1
assert document.accesses.get(user__isnull=True, role="owner").team == "lasuite"


Expand Down Expand Up @@ -527,3 +529,269 @@ def test_api_documents_move_to_self(position):
# Ensure document has not moved
document.refresh_from_db()
assert document.is_root() is True


def test_api_documents_move_root_deletes_accesses_and_invitations():
"""
Moving a root document should automatically delete all its direct accesses and
invitations so it inherits the target's permissions.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)

other_user = factories.UserFactory()
document = factories.DocumentFactory(
users=[(user, "owner"), (other_user, "editor")]
)
factories.InvitationFactory(document=document)
factories.InvitationFactory(document=document)

target = factories.DocumentFactory(users=[(user, "owner")])

assert document.is_root()
assert document.accesses.count() == 2
assert document.invitations.count() == 2

response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": "first-child"},
)

assert response.status_code == 200
assert response.json() == {"message": "Document moved successfully."}

document.refresh_from_db()
assert document.is_child_of(target)
assert document.accesses.count() == 0
assert document.invitations.count() == 0


def test_api_documents_move_cross_tree_deletes_accesses_and_invitations():
"""
Moving a non-root document to a different tree should delete its direct accesses
and invitations because it is leaving its current permission scope.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)

other_user = factories.UserFactory()
source_root = factories.DocumentFactory(users=[(user, "owner")])
document = factories.DocumentFactory(
parent=source_root, users=[(user, "owner"), (other_user, "editor")]
)
factories.InvitationFactory(document=document)

target_root = factories.DocumentFactory(users=[(user, "owner")])
target = factories.DocumentFactory(parent=target_root, users=[(user, "owner")])

assert not document.is_root()
assert document.get_root() != target.get_root()
assert document.accesses.count() == 2
assert document.invitations.count() == 1

response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": "first-child"},
)

assert response.status_code == 200
assert response.json() == {"message": "Document moved successfully."}

document.refresh_from_db()
assert document.is_child_of(target)
assert document.accesses.count() == 0
assert document.invitations.count() == 0


def test_api_documents_move_same_tree_keeps_accesses_and_invitations():
"""
Moving a non-root document within the same tree should preserve its direct
accesses and invitations since it stays in the same permission scope.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)

other_user = factories.UserFactory()
root = factories.DocumentFactory(users=[(user, "owner")])
document = factories.DocumentFactory(
parent=root, users=[(user, "owner"), (other_user, "editor")]
)
factories.InvitationFactory(document=document)
target = factories.DocumentFactory(parent=root, users=[(user, "owner")])

assert not document.is_root()
assert document.get_root() == target.get_root()
assert document.accesses.count() == 2
assert document.invitations.count() == 1

response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": "first-child"},
)

assert response.status_code == 200
assert response.json() == {"message": "Document moved successfully."}

document.refresh_from_db()
assert document.is_child_of(target)
assert document.accesses.count() == 2
assert document.invitations.count() == 1


@pytest.mark.parametrize("position", ["first-sibling", "last-sibling", "left", "right"])
def test_api_documents_move_sub_document_to_root_deletes_accesses_and_invitations(
position,
):
"""
Moving a sub-document to root level (as a sibling of a root document) changes its
permission scope. Its direct accesses and invitations must be deleted.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)

other_user = factories.UserFactory()
parent = factories.DocumentFactory(users=[(user, "owner")])
document = factories.DocumentFactory(
parent=parent, users=[(user, "owner"), (other_user, "editor")]
)
factories.InvitationFactory(document=document)

# target is a root document; moving as its sibling promotes document to root level
target = factories.DocumentFactory(users=[(user, "owner")])

assert not document.is_root()
assert document.accesses.count() == 2
assert document.invitations.count() == 1

response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": position},
)

assert response.status_code == 200
document.refresh_from_db()
assert document.is_root()
# Direct accesses and invitations are wiped; the backend then ensures at least one
# owner exists by inheriting the owner(s) from the previous root.
assert document.accesses.count() == 1
assert document.accesses.filter(role="owner").exists()
assert document.invitations.count() == 0


@pytest.mark.parametrize("position", ["first-sibling", "last-sibling", "left", "right"])
def test_api_documents_move_sub_document_as_sibling_of_its_own_root_deletes_accesses(
position,
):
"""
Moving a sub-document as a sibling of its current root promotes it to a
new root. Even though before the move both share the same root, the document is
leaving its permission scope and its direct accesses/invitations must be deleted.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)

other_user = factories.UserFactory()
root = factories.DocumentFactory(users=[(user, "owner")])
document = factories.DocumentFactory(
parent=root, users=[(user, "owner"), (other_user, "editor")]
)
factories.InvitationFactory(document=document)

assert not document.is_root()
assert document.get_root() == root
assert document.accesses.count() == 2
assert document.invitations.count() == 1

response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(root.id), "position": position},
)

assert response.status_code == 200
document.refresh_from_db()
assert document.is_root()
# Original accesses wiped; owner re-inherited from previous root ensures non-orphaned.
assert document.accesses.count() == 1
assert document.accesses.filter(role="owner").exists()
assert document.invitations.count() == 0


@pytest.mark.parametrize("position", ["first-sibling", "last-sibling", "left", "right"])
def test_api_documents_move_root_as_sibling_of_root_preserves_owner(position):
"""
Moving a root document as sibling of another root, the owners
collected from the previous root (which is the document itself) must survive the
pre-move access deletion, so the document keeps at least one owner afterwards.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)

other_user = factories.UserFactory()
document = factories.DocumentFactory(
users=[(user, "owner"), (other_user, "editor")]
)
factories.InvitationFactory(document=document)

target = factories.DocumentFactory(users=[(user, "owner")])

assert document.is_root()
assert target.is_root()

response = client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": position},
)

assert response.status_code == 200
document.refresh_from_db()
assert document.is_root()
# The original editor access and invitation are wiped; the previous root owner
# is re-added so the document still has at least one owner.
assert document.accesses.count() == 1
assert document.accesses.get(role="owner").user == user
assert document.invitations.count() == 0


def test_api_documents_move_scope_change_deletion_is_atomic(monkeypatch):
"""
When accesses/invitations are to be deleted (root or cross-tree move), both
deletions and the tree move are atomic: if the move fails, deletions roll back.
"""
user = factories.UserFactory()
client = APIClient()
client.force_login(user)

other_user = factories.UserFactory()
document = factories.DocumentFactory(
users=[(user, "owner"), (other_user, "editor")]
)
factories.InvitationFactory(document=document)
target = factories.DocumentFactory(users=[(user, "owner")])

assert document.is_root()
assert document.accesses.count() == 2
assert document.invitations.count() == 1

# Force Document.move to fail *after* the deletion block has already run,
# so we actually exercise the rollback path rather than bailing out earlier.
def failing_move(self, *args, **kwargs):
raise RuntimeError("Simulated move failure for atomicity test")

monkeypatch.setattr(models.Document, "move", failing_move)

with pytest.raises(RuntimeError, match="Simulated move failure"):
client.post(
f"/api/v1.0/documents/{document.id!s}/move/",
data={"target_document_id": str(target.id), "position": "first-sibling"},
)

# Accesses and invitations must still exist due to transaction rollback
document.refresh_from_db()
assert document.accesses.count() == 2
assert document.invitations.count() == 1
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@ import { TreeViewMoveModeEnum } from '@gouvfr-lasuite/ui-kit';
import { useMutation, useQueryClient } from '@tanstack/react-query';

import { APIError, errorCauses, fetchAPI } from '@/api';
import {
getDocAccesses,
getDocInvitations,
useDeleteDocAccess,
useDeleteDocInvitation,
} from '@/docs/doc-share';

import { KEY_LIST_DOC } from './useDocs';

Expand Down Expand Up @@ -37,46 +31,15 @@ export const moveDoc = async ({
return response.json() as Promise<void>;
};

export function useMoveDoc(deleteAccessOnMove = false) {
export function useMoveDoc() {
const queryClient = useQueryClient();
const { mutate: handleDeleteInvitation } = useDeleteDocInvitation();
const { mutate: handleDeleteAccess } = useDeleteDocAccess();

return useMutation<void, APIError, MoveDocParam>({
mutationFn: moveDoc,
async onSuccess(_data, variables, _onMutateResult, _context) {
if (!deleteAccessOnMove) {
return;
}

onSuccess() {
void queryClient.invalidateQueries({
queryKey: [KEY_LIST_DOC],
});
const accesses = await getDocAccesses({
docId: variables.sourceDocumentId,
});

const invitationsResponse = await getDocInvitations({
docId: variables.sourceDocumentId,
page: 1,
});

const invitations = invitationsResponse.results;

await Promise.all([
...invitations.map((invitation) =>
handleDeleteInvitation({
docId: variables.sourceDocumentId,
invitationId: invitation.id,
}),
),
...accesses.map((access) =>
handleDeleteAccess({
docId: variables.sourceDocumentId,
accessId: access.id,
}),
),
]);
},
});
}
Loading
Loading