Skip to content

Commit 39e0537

Browse files
antkrytadlius
andcommitted
[ENG-9157] Add atomic ability to remove contributors from children projects in API (CenterForOpenScience#11425)
* add include_children to delete contributors * atomic contributor remove; add tests * remove from children only for nodes * fix tests --------- Co-authored-by: Yuhuai Liu <yuhuai@cos.io>
1 parent 95c5169 commit 39e0537

3 files changed

Lines changed: 2 additions & 39 deletions

File tree

api/nodes/views.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@
161161
Preprint,
162162
Collection,
163163
Contributor,
164-
NotificationTypeEnum,
164+
NotificationType,
165165
)
166166
from addons.osfstorage.models import Region
167167
from osf.utils.permissions import ADMIN, WRITE_NODE
@@ -572,8 +572,6 @@ def perform_destroy(self, instance):
572572
)
573573
with transaction.atomic():
574574
for descendant in targets:
575-
if not descendant.has_permission(auth.user, osf_permissions.ADMIN):
576-
raise PermissionDenied(f'Must have admin permission on {descendant._id} to remove contributor.')
577575
removed = descendant.remove_contributor(instance, auth)
578576
if not removed:
579577
raise ValidationError(f'{descendant._id} must have at least one registered admin contributor')

api_tests/draft_registrations/views/test_draft_registration_contributor_detail.py

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -277,19 +277,6 @@ def test_remove_contributor_include_children_is_atomic_on_violation(self, app, u
277277
project.reload()
278278
assert user_write_contrib not in project.contributors
279279

280-
def test_remove_contributor_include_children_forbidden_if_unauthorized_child(self, app, user, user_write_contrib, project):
281-
# Draft registrations have no child components, so include_children does not
282-
# trigger any descendant permission checks; the contributor is simply removed.
283-
assert user_write_contrib in project.contributors
284-
285-
url = f'/{API_BASE}draft_registrations/{project._id}/contributors/{user_write_contrib._id}/?include_children=true'
286-
with disconnected_from_listeners(contributor_removed):
287-
res = app.delete(url, auth=user.auth)
288-
assert res.status_code == 204
289-
290-
project.reload()
291-
assert user_write_contrib not in project.contributors
292-
293280

294281
@pytest.mark.django_db
295282
class TestDraftBibliographicContributorDetail():

api_tests/nodes/views/test_node_contributors_detail.py

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -493,29 +493,7 @@ def test_remove_contributor_include_children_is_atomic_on_violation(self, app, u
493493
with disconnected_from_listeners(contributor_removed):
494494
res = app.delete(url, auth=user.auth, expect_errors=True)
495495

496-
assert res.status_code == 403
497-
498-
project.reload()
499-
child.reload()
500-
501-
assert user_write_contrib in project.contributors
502-
assert user_write_contrib in child.contributors
503-
504-
def test_remove_contributor_include_children_forbidden_if_unauthorized_child(self, app, user, user_write_contrib, project):
505-
other_admin = AuthUserFactory()
506-
child = ProjectFactory(parent=project, creator=other_admin)
507-
child.add_contributor(user_write_contrib, permissions=permissions.WRITE, visible=True, save=True)
508-
509-
assert user in project.contributors
510-
assert user not in child.contributors
511-
assert other_admin in child.contributors
512-
assert user_write_contrib in project.contributors
513-
assert user_write_contrib in child.contributors
514-
515-
url = f'/{API_BASE}nodes/{project._id}/contributors/{user_write_contrib._id}/?include_children=true'
516-
with disconnected_from_listeners(contributor_removed):
517-
res = app.delete(url, auth=user.auth, expect_errors=True)
518-
assert res.status_code == 403
496+
assert res.status_code == 400
519497

520498
project.reload()
521499
child.reload()

0 commit comments

Comments
 (0)