Skip to content

Commit

Permalink
continue refactor and cleanup
Browse files Browse the repository at this point in the history
  • Loading branch information
John Tordoff committed Jul 15, 2024
1 parent 6fbb21b commit d40e594
Show file tree
Hide file tree
Showing 38 changed files with 171 additions and 170 deletions.
16 changes: 8 additions & 8 deletions admin_tests/preprints/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -506,13 +506,13 @@ def withdrawal_request(self, preprint, submitter):
creator=submitter,
target=preprint,
request_type=RequestTypes.WITHDRAWAL.value,
machine_state=DefaultStates.INITIAL.value,
machine_state=DefaultStates.INITIAL.db_name,
)
withdrawal_request.run_submit(submitter)
return withdrawal_request

def test_can_approve_withdrawal_request(self, withdrawal_request, submitter, preprint, admin):
assert withdrawal_request.machine_state == DefaultStates.PENDING.value
assert withdrawal_request.machine_state == DefaultStates.PENDING.db_name
original_comment = withdrawal_request.comment

request = RequestFactory().post(reverse('preprints:approve-withdrawal', kwargs={'guid': preprint._id}))
Expand All @@ -524,11 +524,11 @@ def test_can_approve_withdrawal_request(self, withdrawal_request, submitter, pre

withdrawal_request.refresh_from_db()
withdrawal_request.target.refresh_from_db()
assert withdrawal_request.machine_state == DefaultStates.ACCEPTED.value
assert withdrawal_request.machine_state == DefaultStates.ACCEPTED.db_name
assert original_comment == withdrawal_request.target.withdrawal_justification

def test_can_reject_withdrawal_request(self, withdrawal_request, admin, preprint):
assert withdrawal_request.machine_state == DefaultStates.PENDING.value
assert withdrawal_request.machine_state == DefaultStates.PENDING.db_name

request = RequestFactory().post(reverse('preprints:reject-withdrawal', kwargs={'guid': preprint._id}))
request.POST = {'action': 'reject'}
Expand All @@ -539,7 +539,7 @@ def test_can_reject_withdrawal_request(self, withdrawal_request, admin, preprint

withdrawal_request.refresh_from_db()
withdrawal_request.target.refresh_from_db()
assert withdrawal_request.machine_state == DefaultStates.REJECTED.value
assert withdrawal_request.machine_state == DefaultStates.REJECTED.db_name
assert not withdrawal_request.target.withdrawal_justification

def test_permissions_errors(self, user, submitter):
Expand Down Expand Up @@ -569,10 +569,10 @@ def test_osf_admin_has_correct_view_permissions(self, withdrawal_request, admin)
assert response.status_code == 200

@pytest.mark.parametrize('action, final_state', [
('approve', DefaultStates.ACCEPTED.value),
('reject', DefaultStates.REJECTED.value)])
('approve', DefaultStates.ACCEPTED.db_name),
('reject', DefaultStates.REJECTED.db_name)])
def test_approve_reject_on_list_view(self, withdrawal_request, admin, action, final_state):
assert withdrawal_request.machine_state == DefaultStates.PENDING.value
assert withdrawal_request.machine_state == DefaultStates.PENDING.db_name
original_comment = withdrawal_request.comment
request = RequestFactory().post(reverse('preprints:withdrawal-requests'), {'action': action, withdrawal_request.id: ['on']})
request.user = admin
Expand Down
16 changes: 8 additions & 8 deletions api/actions/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@
from api.base.utils import get_user_auth
from osf.models import CollectionSubmissionAction
from osf.models.action import BaseAction
from osf.models.mixins import ReviewableMixin, ReviewProviderMixin
from osf.utils.workflows import ReviewTriggers
from osf.models.mixins import PreprintStateMachineMixin, ReviewProviderMixin
from osf.utils.workflows import PreprintStateTriggers
from osf.utils import permissions as osf_permissions

# Required permission to perform each action. `None` means no permissions required.
TRIGGER_PERMISSIONS = {
ReviewTriggers.SUBMIT.value: None,
ReviewTriggers.ACCEPT.value: 'accept_submissions',
ReviewTriggers.REJECT.value: 'reject_submissions',
ReviewTriggers.WITHDRAW.value: 'withdraw_submissions',
ReviewTriggers.EDIT_COMMENT.value: 'edit_review_comments',
PreprintStateTriggers.SUBMIT.db_name: None,
PreprintStateTriggers.ACCEPT.db_name: 'accept_submissions',
PreprintStateTriggers.REJECT.db_name: 'reject_submissions',
PreprintStateTriggers.WITHDRAW.db_name: 'withdraw_submissions',
PreprintStateTriggers.EDIT_COMMENT.db_name: 'edit_review_comments',
}


Expand All @@ -30,7 +30,7 @@ def has_object_permission(self, request, view, obj):
if isinstance(obj, tuple(BaseAction.__subclasses__())):
target = obj.target
provider = target.provider
elif isinstance(obj, ReviewableMixin):
elif isinstance(obj, PreprintStateMachineMixin):
target = obj
provider = target.provider
elif isinstance(obj, ReviewProviderMixin):
Expand Down
16 changes: 8 additions & 8 deletions api/actions/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@
from osf.utils.workflows import (
DefaultStates,
DefaultTriggers,
ReviewStates,
ReviewTriggers,
PreprintStates,
PreprintStateTriggers,
RegistrationModerationTriggers,
SchemaResponseTriggers,
)
Expand Down Expand Up @@ -110,7 +110,7 @@ class BaseActionSerializer(JSONAPISerializer):

id = ser.CharField(source='_id', read_only=True)

trigger = ser.ChoiceField(choices=DefaultTriggers.choices())
trigger = ser.ChoiceField(choices=DefaultTriggers.char_field_choices())

comment = ser.CharField(max_length=65535, required=False, allow_blank=True, allow_null=True)

Expand Down Expand Up @@ -147,7 +147,7 @@ def create(self, validated_data):
comment = validated_data.get('comment', '')
permissions = validated_data.get('permissions', '')
visible = validated_data.get('visible', '')

print('cdsds', comment, trigger, DefaultTriggers.EDIT_COMMENT.db_name)
try:
if trigger == DefaultTriggers.ACCEPT.db_name:
return target.run_accept(user=user, comment=comment, permissions=permissions, visible=visible)
Expand Down Expand Up @@ -183,9 +183,9 @@ class Meta:
])

comment = HideIfProviderCommentsPrivate(ser.CharField(max_length=65535, required=False))
trigger = ser.ChoiceField(choices=ReviewTriggers.choices())
from_state = ser.ChoiceField(choices=ReviewStates.char_field_choices(), read_only=True)
to_state = ser.ChoiceField(choices=ReviewStates.char_field_choices(), read_only=True)
trigger = ser.ChoiceField(choices=PreprintStateTriggers.char_field_choices())
from_state = ser.ChoiceField(choices=PreprintStates.char_field_choices(), read_only=True)
to_state = ser.ChoiceField(choices=PreprintStates.char_field_choices(), read_only=True)

provider = RelationshipField(
read_only=True,
Expand All @@ -212,7 +212,7 @@ class Meta:

def create(self, validated_data):
trigger = validated_data.get('trigger')
if trigger != ReviewTriggers.WITHDRAW.db_name:
if trigger != PreprintStateTriggers.WITHDRAW.db_name:
return super(ReviewActionSerializer, self).create(validated_data)
user = validated_data.pop('user')
target = validated_data.pop('target')
Expand Down
2 changes: 1 addition & 1 deletion api/files/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class IsPreprintFile(PreprintPublishedOrAdmin):
def has_object_permission(self, request, view, obj):
assert isinstance(obj, BaseFileNode), 'obj must be a BaseFileNode, got {}'.format(obj)
if (hasattr(obj.target, 'primary_file') and obj.target.primary_file == obj):
if request.method == 'DELETE' and obj.target.machine_state != DefaultStates.INITIAL.value:
if request.method == 'DELETE' and obj.target.machine_state != DefaultStates.INITIAL.db_name:
return False

if obj.target.is_retracted and request.method in permissions.SAFE_METHODS:
Expand Down
2 changes: 1 addition & 1 deletion api/preprints/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def has_object_permission(self, request, view, obj):
obj.verified_publishable or
(obj.is_public and auth.user.has_perm('view_submissions', obj.provider)) or
obj.has_permission(auth.user, osf_permissions.ADMIN) or
(obj.is_contributor(auth.user) and obj.machine_state != DefaultStates.INITIAL.value)
(obj.is_contributor(auth.user) and obj.machine_state != DefaultStates.INITIAL.db_name)
)
return user_has_permissions
else:
Expand Down
3 changes: 2 additions & 1 deletion api/providers/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -481,8 +481,9 @@ def get_queryset(self):

# overrides APIView
def get_renderer_context(self):
context = super(PreprintProviderPreprintList, self).get_renderer_context()
context = super().get_renderer_context()
show_counts = is_truthy(self.request.query_params.get('meta[reviews_state_counts]', False))

if show_counts:
# TODO don't duplicate the above
auth = get_user_auth(self.request)
Expand Down
20 changes: 10 additions & 10 deletions api/requests/permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,17 @@ def has_object_permission(self, request, view, obj):
target = obj
node = obj.target
# Creating a Request is "submitting"
trigger = request.data.get('trigger', DefaultTriggers.SUBMIT.value if request.method not in drf_permissions.SAFE_METHODS else None)
trigger = request.data.get('trigger', DefaultTriggers.SUBMIT.db_name if request.method not in drf_permissions.SAFE_METHODS else None)
elif isinstance(obj, Node):
node = obj
trigger = DefaultTriggers.SUBMIT.value if request.method not in drf_permissions.SAFE_METHODS else None
trigger = DefaultTriggers.SUBMIT.db_name if request.method not in drf_permissions.SAFE_METHODS else None
else:
raise ValueError('Not a request-related model: {}'.format(obj))

if not node.access_requests_enabled:
return False

is_requester = target is not None and target.creator == auth.user or trigger == DefaultTriggers.SUBMIT.value
is_requester = target is not None and target.creator == auth.user or trigger == DefaultTriggers.SUBMIT.db_name
is_node_admin = node.has_permission(auth.user, osf_permissions.ADMIN)
has_view_permission = is_requester or is_node_admin

Expand All @@ -48,10 +48,10 @@ def has_object_permission(self, request, view, obj):
if not has_view_permission:
return False

if trigger in [DefaultTriggers.ACCEPT.value, DefaultTriggers.REJECT.value]:
if trigger in [DefaultTriggers.ACCEPT.db_name, DefaultTriggers.REJECT.db_name]:
# Node admins can only approve or reject requests
return is_node_admin
if trigger in [DefaultTriggers.EDIT_COMMENT.value, DefaultTriggers.SUBMIT.value]:
if trigger in [DefaultTriggers.EDIT_COMMENT.db_name, DefaultTriggers.SUBMIT.db_name]:
# Requesters may not be contributors
# Requesters may edit their comment or submit their request
return is_requester and auth.user not in node.contributors
Expand All @@ -73,14 +73,14 @@ def has_object_permission(self, request, view, obj):
target = obj
preprint = obj.target
# Creating a Request is "submitting"
trigger = request.data.get('trigger', DefaultTriggers.SUBMIT.value if request.method not in drf_permissions.SAFE_METHODS else None)
trigger = request.data.get('trigger', DefaultTriggers.SUBMIT.db_name if request.method not in drf_permissions.SAFE_METHODS else None)
elif isinstance(obj, Preprint):
preprint = obj
trigger = DefaultTriggers.SUBMIT.value if request.method not in drf_permissions.SAFE_METHODS else None
trigger = DefaultTriggers.SUBMIT.db_name if request.method not in drf_permissions.SAFE_METHODS else None
else:
raise ValueError('Not a request-related model: {}'.format(obj))

is_requester = target is not None and target.creator == auth.user or trigger == DefaultTriggers.SUBMIT.value
is_requester = target is not None and target.creator == auth.user or trigger == DefaultTriggers.SUBMIT.db_name
is_preprint_admin = preprint.has_permission(auth.user, osf_permissions.ADMIN)
is_moderator = auth.user.has_perm('withdraw_submissions', preprint.provider)
has_view_permission = is_requester or is_preprint_admin or is_moderator
Expand All @@ -92,10 +92,10 @@ def has_object_permission(self, request, view, obj):
if not has_view_permission:
return False

if trigger in [DefaultTriggers.ACCEPT.value, DefaultTriggers.REJECT.value]:
if trigger in [DefaultTriggers.ACCEPT.db_name, DefaultTriggers.REJECT.db_name]:
# Only moderators can approve or reject requests
return is_moderator
if trigger in [DefaultTriggers.EDIT_COMMENT.value, DefaultTriggers.SUBMIT.value]:
if trigger in [DefaultTriggers.EDIT_COMMENT.db_name, DefaultTriggers.SUBMIT.db_name]:
# Requesters may edit their comment or submit their request
return is_requester
return False
5 changes: 3 additions & 2 deletions api/requests/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ def create(self, validated_data):
target=node,
creator=auth.user,
comment=comment,
machine_state=DefaultStates.INITIAL.value,
machine_state=DefaultStates.INITIAL.db_name,
request_type=request_type,
)
node_request.save()
Expand Down Expand Up @@ -145,6 +145,7 @@ class Meta:
def get_target_url(self, obj):
return absolute_reverse('preprints:preprint-detail', kwargs={'preprint_id': obj.target._id, 'version': self.context['request'].parser_context['kwargs']['version']})


class PreprintRequestCreateSerializer(PreprintRequestSerializer):
request_type = ser.ChoiceField(required=True, choices=RequestTypes.choices())

Expand All @@ -171,7 +172,7 @@ def create(self, validated_data):
target=preprint,
creator=auth.user,
comment=comment,
machine_state=DefaultStates.INITIAL.value,
machine_state=DefaultStates.INITIAL.db_name,
request_type=request_type,
)
preprint_request.save()
Expand Down
2 changes: 1 addition & 1 deletion api_tests/files/views/test_file_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -890,7 +890,7 @@ def test_deleted_preprint_file(self, app, file_url, preprint, user, other_user):
assert res.status_code == 410

def test_abandoned_preprint_file(self, app, file_url, preprint, user, other_user):
preprint.machine_state = DefaultStates.INITIAL.value
preprint.machine_state = DefaultStates.INITIAL.db_name
preprint.save()

# Unauthenticated
Expand Down
2 changes: 1 addition & 1 deletion api_tests/identifiers/views/test_identifier_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ def test_identifier_preprint_detail_abandoned(
self, app, preprint, user, identifier_preprint, noncontrib
):
url = '/{}identifiers/{}/'.format(API_BASE, identifier_preprint._id)
preprint.machine_state = DefaultStates.INITIAL.value
preprint.machine_state = DefaultStates.INITIAL.db_name
preprint.save()

# test_abandoned_preprint_identifier_unauthenticated
Expand Down
2 changes: 1 addition & 1 deletion api_tests/identifiers/views/test_identifier_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ def test_preprint_identifier_list_permissions_deleted(

def test_preprint_identifier_list_permissions_abandoned(
self, app, user, data_preprint_identifier, preprint, url_preprint_identifier):
preprint.machine_state = DefaultStates.INITIAL.value
preprint.machine_state = DefaultStates.INITIAL.db_name
preprint.save()

# test_unpublished_preprint_identifier_unauthenticated
Expand Down
4 changes: 2 additions & 2 deletions api_tests/nodes/views/test_node_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -1118,7 +1118,7 @@ def test_abandonded_preprint_in_preprint_true_filter_results(
project=ProjectFactory(creator=user_one)
)
abandoned.node.add_contributor(user_two, save=True)
abandoned.machine_state = DefaultStates.INITIAL.value
abandoned.machine_state = DefaultStates.INITIAL.db_name
abandoned.save()

url = '/{}nodes/?filter[preprint]=true'.format(API_BASE)
Expand Down Expand Up @@ -1150,7 +1150,7 @@ def test_abandonded_preprint_in_preprint_false_filter_results(
project=ProjectFactory(creator=user_one)
)
abandoned.node.add_contributor(user_two, save=True)
abandoned.machine_state = DefaultStates.INITIAL.value
abandoned.machine_state = DefaultStates.INITIAL.db_name
abandoned.save()

url = '/{}nodes/?filter[preprint]=false'.format(API_BASE)
Expand Down
4 changes: 2 additions & 2 deletions api_tests/preprints/views/test_preprint_citations.py
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ def test_deleted_preprint_citations(self):
assert_equal(res.status_code, 404)

def test_abandoned_preprint_citations(self):
self.published_preprint.machine_state = DefaultStates.INITIAL.value
self.published_preprint.machine_state = DefaultStates.INITIAL.db_name
self.published_preprint.save()

# Unauthenticated
Expand Down Expand Up @@ -267,7 +267,7 @@ def test_deleted_preprint_citations(self):
assert_equal(res.status_code, 404)

def test_abandoned_preprint_citations(self):
self.published_preprint.machine_state = DefaultStates.INITIAL.value
self.published_preprint.machine_state = DefaultStates.INITIAL.db_name
self.published_preprint.save()

# Unauthenticated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def test_preprint_contributor_private(

def test_preprint_contributor_abandoned(
self, app, user, preprint_published, url_published):
preprint_published.machine_state = DefaultStates.INITIAL.value
preprint_published.machine_state = DefaultStates.INITIAL.db_name
preprint_published.save()

# Unauthenticated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ def test_return_preprint_contributors_deleted_preprint(

def test_return_preprint_contributors_abandoned_preprint(
self, app, user, user_two, preprint_published, url_published):
preprint_published.machine_state = DefaultStates.INITIAL.value
preprint_published.machine_state = DefaultStates.INITIAL.db_name
preprint_published.save()

# test_abandoned_preprint_contributors_logged_out
Expand Down
6 changes: 3 additions & 3 deletions api_tests/preprints/views/test_preprint_detail.py
Original file line number Diff line number Diff line change
Expand Up @@ -1936,7 +1936,7 @@ def unpublished_reviews_preprint(
provider=reviews_provider,
subjects=[[subject._id]],
is_published=False,
machine_state=DefaultStates.PENDING.value)
machine_state=DefaultStates.PENDING.db_name)
preprint.add_contributor(write_contrib, permissions=WRITE)
preprint.save()
return preprint
Expand All @@ -1950,7 +1950,7 @@ def unpublished_reviews_initial_preprint(
provider=reviews_provider,
subjects=[[subject._id]],
is_published=False,
machine_state=DefaultStates.INITIAL.value)
machine_state=DefaultStates.INITIAL.db_name)

@pytest.fixture()
def private_reviews_preprint(
Expand All @@ -1962,7 +1962,7 @@ def private_reviews_preprint(
subjects=[[subject._id]],
is_published=False,
is_public=False,
machine_state=DefaultStates.PENDING.value)
machine_state=DefaultStates.PENDING.db_name)
preprint.add_contributor(write_contrib, permissions=WRITE)
return preprint

Expand Down
4 changes: 2 additions & 2 deletions api_tests/preprints/views/test_preprint_files_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ def test_private_preprint_files(self):
assert res.status_code == 200

def test_abandoned_preprint_files(self):
self.preprint.machine_state = DefaultStates.INITIAL.value
self.preprint.machine_state = DefaultStates.INITIAL.db_name
self.preprint.save()

# Unauthenticated
Expand Down Expand Up @@ -273,7 +273,7 @@ def test_private_preprint_files(self):
assert res.status_code == 200

def test_abandoned_preprint_files(self):
self.preprint.machine_state = DefaultStates.INITIAL.value
self.preprint.machine_state = DefaultStates.INITIAL.db_name
self.preprint.save()

# Unauthenticated
Expand Down
Loading

0 comments on commit d40e594

Please sign in to comment.