mirror of
https://github.com/suitenumerique/docs.git
synced 2026-09-10 03:37:52 +02:00
✨(backend) delete accesses and invitations on move when scope changes
When a document is moved outside its current permission scope (root document, cross-tree move, or promotion to root), its direct accesses and pending invitations are now deleted server-side within the same atomic transaction as the move itself. This ensures consistency: if the move fails, deletions are rolled back. Signed-off-by: Mohamed El Amine BOUKERFA <boukerfa.ma@gmail.com>
This commit is contained in:
@@ -949,8 +949,8 @@ class DocumentViewSet(
|
||||
"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 = (
|
||||
@@ -972,6 +972,30 @@ class DocumentViewSet(
|
||||
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
|
||||
|
||||
@@ -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():
|
||||
@@ -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"
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user