Skip to content

Commit 7814453

Browse files
authored
[ENG-7750] Preprint creator cannot remove bibliographic citation status (#11087)
## Purpose fix preprint creator cannot remove bibliographic citation status ## Changes - move restriction logic to serializer field validator to validate only permission field during any updates: PATCH, PUT - add more tests ## Ticket https://openscience.atlassian.net/browse/ENG-7750
1 parent af28216 commit 7814453

File tree

5 files changed

+94
-78
lines changed

5 files changed

+94
-78
lines changed

api/preprints/serializers.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
NodeLicense,
5050
)
5151
from osf.utils import permissions as osf_permissions
52+
from osf.utils.workflows import DefaultStates
5253

5354

5455
class PrimaryFileRelationshipField(RelationshipField):
@@ -625,6 +626,20 @@ class PreprintContributorDetailSerializer(NodeContributorDetailSerializer, Prepr
625626
id = IDField(required=True, source='_id')
626627
index = ser.IntegerField(required=False, read_only=False, source='_order')
627628

629+
def validate_permission(self, value):
630+
preprint = self.context.get('resource')
631+
user = self.context.get('user')
632+
if (
633+
user # if user is None then probably we're trying to make bulk update and this validation is not relevant
634+
and preprint.machine_state == DefaultStates.INITIAL.value
635+
and preprint.creator_id == user.id
636+
):
637+
raise ValidationError(
638+
'You cannot change your permission setting at this time. '
639+
'Have another admin contributor edit your permission after you’ve submitted your preprint',
640+
)
641+
return value
642+
628643

629644
class PreprintStorageProviderSerializer(NodeStorageProviderSerializer):
630645
node = HideIfPreprint(ser.CharField(source='node_id', read_only=True))

api/preprints/views.py

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -77,10 +77,10 @@ class PreprintOldVersionsImmutableMixin:
7777

7878
@staticmethod
7979
def is_edit_allowed(preprint):
80-
if preprint.is_latest_version or preprint.machine_state == 'initial':
80+
if preprint.is_latest_version or preprint.machine_state == DefaultStates.INITIAL.value:
8181
return True
8282
if preprint.provider.reviews_workflow == Workflows.PRE_MODERATION.value:
83-
if preprint.machine_state == 'pending' or preprint.machine_state == 'rejected':
83+
if preprint.machine_state == DefaultStates.PENDING.value or preprint.machine_state == DefaultStates.REJECTED.value:
8484
return True
8585
return False
8686

@@ -535,19 +535,10 @@ def get_object(self):
535535
def get_serializer_context(self):
536536
context = JSONAPIBaseView.get_serializer_context(self)
537537
context['resource'] = self.get_preprint()
538+
context['user'] = self.get_user()
538539
context['default_email'] = 'preprint'
539540
return context
540541

541-
def patch(self, *args, **kwargs):
542-
preprint = self.get_resource()
543-
user = self.get_user()
544-
if preprint.machine_state == DefaultStates.INITIAL.value and preprint.creator_id == user.id:
545-
raise ValidationError(
546-
'You cannot change your permission setting at this time. '
547-
'Have another admin contributor edit your permission after you’ve submitted your preprint',
548-
)
549-
return super().patch(*args, **kwargs)
550-
551542
def perform_destroy(self, instance):
552543
preprint = self.get_resource()
553544
auth = get_user_auth(self.request)

api_tests/preprints/views/test_preprint_contributors_detail.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,33 @@ def test_patch_permission_only(self, app, user, preprint):
10221022
assert preprint.get_permissions(user_read_contrib) == [permissions.READ]
10231023
assert not preprint.get_visible(user_read_contrib)
10241024

1025+
def test_patch_admin_self_initial_preprint(self, app, user, preprint):
1026+
new_version = PreprintFactory.create_version(
1027+
create_from=preprint,
1028+
creator=user,
1029+
final_machine_state='initial',
1030+
is_published=False,
1031+
set_doi=False
1032+
)
1033+
1034+
contrib_id = f'{new_version._id}-{user._id}'
1035+
data = {
1036+
'data': {
1037+
'id': contrib_id,
1038+
'type': 'contributors',
1039+
'attributes': {
1040+
'permission': permissions.WRITE,
1041+
'bibliographic': True
1042+
}
1043+
}
1044+
}
1045+
url = f'/{API_BASE}preprints/{new_version._id}/contributors/{user._id}/'
1046+
res = app.patch_json_api(url, data, auth=user.auth, expect_errors=True)
1047+
assert res.status_code == 400
1048+
1049+
new_version.reload()
1050+
assert permissions.ADMIN in new_version.get_permissions(user)
1051+
10251052

10261053
@pytest.mark.django_db
10271054
class TestPreprintContributorDelete:
@@ -1190,6 +1217,28 @@ def test_remove_self_contributor_not_unique_admin(
11901217
preprint.reload()
11911218
assert user not in preprint.contributors
11921219

1220+
def test_remove_self_creator_not_unique_admin_initial_preprint(
1221+
self, app, user, user_write_contrib, preprint):
1222+
new_version = PreprintFactory.create_version(
1223+
create_from=preprint,
1224+
creator=user,
1225+
final_machine_state='initial',
1226+
is_published=False,
1227+
set_doi=False
1228+
)
1229+
1230+
new_version.add_permission(
1231+
user_write_contrib,
1232+
permissions.ADMIN,
1233+
save=True
1234+
)
1235+
assert len(new_version.contributors) == 2
1236+
assert new_version.creator_id == user.id
1237+
1238+
url = f'/{API_BASE}preprints/{new_version._id}/contributors/{user._id}/'
1239+
res = app.delete(url, auth=user.auth, expect_errors=True)
1240+
assert res.status_code == 400
1241+
11931242
# @assert_logs(PreprintLog.CONTRIB_REMOVED, 'preprint')
11941243
def test_can_remove_self_as_contributor_not_unique_admin(
11951244
self, app, user_write_contrib, preprint, url_user_write_contrib):

osf_tests/test_draft_registration.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,33 @@ def test_update_contributor(self, draft_registration, auth):
571571
assert draft_registration.get_permissions(new_contrib) == [READ]
572572
assert draft_registration.get_visible(new_contrib) is False
573573

574+
def test_remove_admin_permission(self, draft_registration, auth):
575+
admin_user = auth.user
576+
assert len(draft_registration.contributors) == 1
577+
assert draft_registration.has_permission(admin_user, ADMIN) is True
578+
579+
with pytest.raises(DraftRegistrationStateError) as exc_info:
580+
draft_registration.update_contributor(
581+
admin_user,
582+
WRITE,
583+
draft_registration.get_visible(admin_user),
584+
auth=auth
585+
)
586+
assert str(exc_info.value) == f"{admin_user.fullname} is the only admin."
587+
588+
# user should be able to remove their own admin permission if there're 2+ admin contributors
589+
new_contrib = factories.AuthUserFactory()
590+
draft_registration.add_contributor(new_contrib, permissions=ADMIN, auth=auth)
591+
assert draft_registration.has_permission(new_contrib, ADMIN) is True
592+
593+
draft_registration.update_contributor(
594+
admin_user,
595+
WRITE,
596+
draft_registration.get_visible(admin_user),
597+
auth=auth,
598+
)
599+
assert draft_registration.has_permission(admin_user, ADMIN) is False
600+
574601
def test_update_contributor_non_admin_raises_error(self, draft_registration, auth):
575602
non_admin = factories.AuthUserFactory()
576603
draft_registration.add_contributor(

tests/test_preprints.py

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -2739,69 +2739,3 @@ def test_ember_redirect_to_versioned_guid(self):
27392739
location_with_no_guid = res.location.replace(guid_with_version, '')
27402740
# check if location has not wrong format https://osf.io/preprints/socarxiv/3rhyz/3rhyz_v1
27412741
assert location_with_no_guid == location_with_no_guid.replace(guid_with_no_version, '')
2742-
2743-
@pytest.mark.django_db
2744-
class TestPreprintCreatorStateChangings:
2745-
2746-
@pytest.fixture()
2747-
def creator(self):
2748-
return AuthUserFactory()
2749-
2750-
@pytest.fixture()
2751-
def unpublished_preprint_pre_mod(self):
2752-
return PreprintFactory(reviews_workflow='pre-moderation', is_published=False)
2753-
2754-
def test_preprint_initial_creator_removal(self, creator, unpublished_preprint_pre_mod):
2755-
new_version = PreprintFactory.create_version(
2756-
create_from=unpublished_preprint_pre_mod,
2757-
creator=creator,
2758-
final_machine_state='initial',
2759-
is_published=False,
2760-
set_doi=False
2761-
)
2762-
request = RequestFactory().delete('/fake_path')
2763-
request.user = creator
2764-
request.query_params = {}
2765-
request.parser_context = {
2766-
'kwargs': {
2767-
'preprint_id': new_version._id,
2768-
'user_id': creator._id
2769-
},
2770-
}
2771-
view = PreprintContributorDetail()
2772-
view = setup_view(view, request, preprint_id=new_version._id, user_id=creator._id)
2773-
try:
2774-
view.perform_destroy(request)
2775-
except Exception as error:
2776-
assert error.args[0] == ('You cannot delete yourself at this time. '
2777-
'Have another admin contributor do that after you’ve submitted your preprint')
2778-
else:
2779-
assert False
2780-
2781-
def test_preprint_initial_creator_update(self, creator, unpublished_preprint_pre_mod):
2782-
new_version = PreprintFactory.create_version(
2783-
create_from=unpublished_preprint_pre_mod,
2784-
creator=creator,
2785-
final_machine_state='initial',
2786-
is_published=False,
2787-
set_doi=False
2788-
)
2789-
request = RequestFactory().patch('/fake_path')
2790-
request.user = creator
2791-
request.query_params = {}
2792-
request.parser_context = {
2793-
'kwargs': {
2794-
'preprint_id': new_version._id,
2795-
'user_id': creator._id
2796-
},
2797-
}
2798-
view = PreprintContributorDetail()
2799-
view = setup_view(view, request, preprint_id=new_version._id, user_id=creator._id)
2800-
try:
2801-
view.patch(request)
2802-
except Exception as error:
2803-
assert error.args[0] == ('You cannot change your permission setting at this time. '
2804-
'Have another admin contributor edit your permission '
2805-
'after you’ve submitted your preprint')
2806-
else:
2807-
assert False

0 commit comments

Comments
 (0)