diff --git a/admin_tests/preprints/test_views.py b/admin_tests/preprints/test_views.py index 357ff643a06..5731c5c9aac 100644 --- a/admin_tests/preprints/test_views.py +++ b/admin_tests/preprints/test_views.py @@ -23,7 +23,6 @@ from osf.models.spam import SpamStatus from osf.utils.workflows import DefaultStates, RequestTypes from osf.utils.permissions import ADMIN -from framework.auth import Auth from admin_tests.utilities import setup_view, setup_log_view, handle_post_view_request diff --git a/api/nodes/views.py b/api/nodes/views.py index 8ae38017b17..9bf2404d806 100644 --- a/api/nodes/views.py +++ b/api/nodes/views.py @@ -9,7 +9,8 @@ from osf import features from packaging.version import Version from django.apps import apps -from django.db.models import F, Max, Q, Subquery +from django.db.models import F, Max, Q, Subquery, Exists, OuterRef +from django.db import transaction from django.utils import timezone from django.contrib.contenttypes.models import ContentType from rest_framework import generics, permissions as drf_permissions, exceptions @@ -154,6 +155,7 @@ Folder, CedarMetadataRecord, Preprint, Collection, + Contributor, ) from addons.osfstorage.models import Region from osf.utils.permissions import ADMIN, WRITE_NODE @@ -549,9 +551,28 @@ def perform_destroy(self, instance): auth = get_user_auth(self.request) if node.visible_contributors.count() == 1 and instance.visible: raise ValidationError('Must have at least one visible contributor') - removed = node.remove_contributor(instance, auth) - if not removed: - raise ValidationError('Must have at least one registered admin contributor') + + include_children = is_truthy(self.request.query_params.get('include_children', False)) + + if include_children and isinstance(node, Node): + hierarchy = Node.objects.get_children(node, active=True, include_root=True) + targets = hierarchy.filter( + Exists( + Contributor.objects.filter( + node=OuterRef('pk'), + user=instance.user, + ), + ), + ) + with transaction.atomic(): + for descendant in targets: + removed = descendant.remove_contributor(instance, auth) + if not removed: + raise ValidationError(f'{descendant._id} must have at least one registered admin contributor') + else: + removed = node.remove_contributor(instance, auth) + if not removed: + raise ValidationError('Must have at least one registered admin contributor') class NodeImplicitContributorsList(JSONAPIBaseView, generics.ListAPIView, ListFilterMixin, NodeMixin): diff --git a/api_tests/nodes/views/test_node_contributors_detail.py b/api_tests/nodes/views/test_node_contributors_detail.py index 57f7e41444f..004a591cc2c 100644 --- a/api_tests/nodes/views/test_node_contributors_detail.py +++ b/api_tests/nodes/views/test_node_contributors_detail.py @@ -459,3 +459,46 @@ def test_can_remove_self_as_contributor_not_unique_admin(self, app, user_write_c ) assert res.status_code == 204 assert user_write_contrib not in project.contributors + + def test_remove_contributor_include_children_removes_descendants(self, app, user, user_write_contrib, project): + child1 = ProjectFactory(parent=project, creator=user) + child2 = ProjectFactory(parent=project, creator=user) + child1.add_contributor(user_write_contrib, permissions=permissions.WRITE, visible=True, save=True) + child2.add_contributor(user_write_contrib, permissions=permissions.WRITE, visible=True, save=True) + + assert user_write_contrib in project.contributors + assert user_write_contrib in child1.contributors + assert user_write_contrib in child2.contributors + + url = f'/{API_BASE}nodes/{project._id}/contributors/{user_write_contrib._id}/?include_children=true' + with disconnected_from_listeners(contributor_removed): + res = app.delete(url, auth=user.auth) + assert res.status_code == 204 + + project.reload() + child1.reload() + child2.reload() + + assert user_write_contrib not in project.contributors + assert user_write_contrib not in child1.contributors + assert user_write_contrib not in child2.contributors + + def test_remove_contributor_include_children_is_atomic_on_violation(self, app, user, user_write_contrib, project): + child = ProjectFactory(parent=project, creator=user) + child.add_contributor(user_write_contrib, permissions=permissions.ADMIN, visible=True, save=True) + child.set_permissions(user, permissions.READ, save=True) + + assert user_write_contrib in project.contributors + assert user_write_contrib in child.contributors + + url = f'/{API_BASE}nodes/{project._id}/contributors/{user_write_contrib._id}/?include_children=true' + with disconnected_from_listeners(contributor_removed): + res = app.delete(url, auth=user.auth, expect_errors=True) + + assert res.status_code == 400 + + project.reload() + child.reload() + + assert user_write_contrib in project.contributors + assert user_write_contrib in child.contributors