refactor reviews machine and fix validation for reviews machine

api/actions/permissions.py

@@ -7,16 +7,16 @@
 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.utils.workflows import PreprintTriggers
 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',
+ PreprintTriggers.SUBMIT.value: None,
+ PreprintTriggers.ACCEPT.value: 'accept_submissions',
+ PreprintTriggers.REJECT.value: 'reject_submissions',
+ PreprintTriggers.WITHDRAW.value: 'withdraw_submissions',
+ PreprintTriggers.EDIT_COMMENT.value: 'edit_review_comments',
 }
 
 

api/actions/serializers.py

@@ -31,8 +31,8 @@
 from osf.utils.workflows import (
  DefaultStates,
  DefaultTriggers,
- ReviewStates,
- ReviewTriggers,
+ PreprintStates,
+ PreprintTriggers,
  RegistrationModerationTriggers,
  SchemaResponseTriggers,
 )
@@ -167,7 +167,7 @@ class Meta:
  type_ = 'actions'
  abstract = True
 
-class ReviewActionSerializer(BaseActionSerializer):
+class PreprintActionSerializer(BaseActionSerializer):
  class Meta:
  type_ = 'review-actions'
 
@@ -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.choices(), read_only=True)
- to_state = ser.ChoiceField(choices=ReviewStates.choices(), read_only=True)
+ trigger = ser.ChoiceField(choices=PreprintTriggers.choices())
+ from_state = ser.ChoiceField(choices=PreprintStates.choices(), read_only=True)
+ to_state = ser.ChoiceField(choices=PreprintStates.choices(), read_only=True)
 
  provider = RelationshipField(
  read_only=True,
@@ -212,8 +212,8 @@ class Meta:
 
  def create(self, validated_data):
  trigger = validated_data.get('trigger')
- if trigger != ReviewTriggers.WITHDRAW.value:
- return super(ReviewActionSerializer, self).create(validated_data)
+ if trigger != PreprintTriggers.WITHDRAW.value:
+ return super().create(validated_data)
  user = validated_data.pop('user')
  target = validated_data.pop('target')
  comment = validated_data.pop('comment', '')

api/actions/views.py

@@ -7,7 +7,7 @@
 from rest_framework.exceptions import NotFound, PermissionDenied
 
 from api.actions.permissions import ReviewActionPermission
-from api.actions.serializers import NodeRequestActionSerializer, ReviewActionSerializer, PreprintRequestActionSerializer
+from api.actions.serializers import NodeRequestActionSerializer, PreprintActionSerializer, PreprintRequestActionSerializer
 from api.base.exceptions import Conflict
 from api.base.filters import ListFilterMixin
 from api.base.views import JSONAPIBaseView
@@ -78,13 +78,13 @@ class ActionDetail(JSONAPIBaseView, generics.RetrieveAPIView):
  required_read_scopes = [CoreScopes.ACTIONS_READ]
  required_write_scopes = [CoreScopes.ACTIONS_WRITE]
 
- serializer_class = ReviewActionSerializer
+ serializer_class = PreprintActionSerializer
  view_category = 'actions'
  view_name = 'action-detail'
 
  def get_serializer_class(self):
  # Not allowed to view NodeRequestActions yet, making extra logic unnecessary
- return ReviewActionSerializer
+ return PreprintActionSerializer
 
  def get_object(self):
  action = None
@@ -162,7 +162,7 @@ class ReviewActionListCreate(JSONAPIBaseView, generics.ListCreateAPIView, ListFi
  required_write_scopes = [CoreScopes.ACTIONS_WRITE]
 
  parser_classes = (JSONAPIMultipleRelationshipsParser, JSONAPIMultipleRelationshipsParserForRegularJSON,)
- serializer_class = ReviewActionSerializer
+ serializer_class = PreprintActionSerializer
  model_class = ReviewAction
 
  ordering = ('-created',)

api/preprints/views.py

@@ -10,7 +10,7 @@
 from osf.utils.requests import check_select_for_update
 
 from api.actions.permissions import ReviewActionPermission
-from api.actions.serializers import ReviewActionSerializer
+from api.actions.serializers import PreprintActionSerializer
 from api.actions.views import get_review_actions_queryset
 from api.base.pagination import PreprintContributorPagination
 from api.base.exceptions import Conflict
@@ -535,7 +535,7 @@ class PreprintActionList(JSONAPIBaseView, generics.ListCreateAPIView, ListFilter
  required_write_scopes = [CoreScopes.ACTIONS_WRITE]
 
  parser_classes = (JSONAPIMultipleRelationshipsParser, JSONAPIMultipleRelationshipsParserForRegularJSON,)
- serializer_class = ReviewActionSerializer
+ serializer_class = PreprintActionSerializer
  model_class = ReviewAction
 
  ordering = ('-created',)

osf/external/chronos/chronos.py

@@ -6,7 +6,7 @@
 from api.files.serializers import get_file_download_link
 from osf.models import ChronosJournal
 from osf.models import ChronosSubmission
-from osf.utils.workflows import ChronosSubmissionStatus, ReviewStates
+from osf.utils.workflows import ChronosSubmissionStatus, PreprintStates
 from website.settings import (
  CHRONOS_USE_FAKE_FILE, CHRONOS_FAKE_FILE_URL,
  CHRONOS_API_KEY, CHRONOS_USERNAME, CHRONOS_PASSWORD, CHRONOS_HOST, VERIFY_CHRONOS_SSL_CERT
@@ -210,7 +210,7 @@ def submit_manuscript(self, journal, preprint, submitter):
  raise ValueError('Cannot submit because your submission was accepted')
  if submission_qs.filter(status=4).exists():
  raise ValueError('Cannot submit because your submission was published')
- if preprint.machine_state != ReviewStates.ACCEPTED.value:
+ if preprint.machine_state != PreprintStates.ACCEPTED.value:
  raise ValueError('Cannot submit to Chronos if the preprint is not accepted by moderators')
 
  body = ChronosSerializer.serialize_manuscript(journal.journal_id, preprint)

osf/metadata/osf_gathering.py

@@ -436,7 +436,7 @@ def gather_preprint_withdrawal(focus):
  _preprint = focus.dbmodel
  yield (OSF.dateWithdrawn, _preprint.date_withdrawn)
  _withdrawal_request = _preprint.requests.filter(
- machine_state=osfworkflows.ReviewStates.ACCEPTED.value,
+ machine_state=osfworkflows.PreprintStates.ACCEPTED.value,
  request_type=osfworkflows.RequestTypes.WITHDRAWAL.value,
  ).last()
  if _withdrawal_request:

osf/models/action.py

@@ -8,8 +8,8 @@
  ApprovalStates,
  DefaultStates,
  DefaultTriggers,
- ReviewStates,
- ReviewTriggers,
+ PreprintStates,
+ PreprintTriggers,
  RegistrationModerationTriggers,
  RegistrationModerationStates,
  SchemaResponseTriggers,
@@ -43,9 +43,9 @@ def target(self):
 class ReviewAction(BaseAction):
  target = models.ForeignKey('Preprint', related_name='actions', on_delete=models.CASCADE)
 
- trigger = models.CharField(max_length=31, choices=ReviewTriggers.choices())
- from_state = models.CharField(max_length=31, choices=ReviewStates.choices())
- to_state = models.CharField(max_length=31, choices=ReviewStates.choices())
+ trigger = models.CharField(max_length=31, choices=PreprintTriggers.choices())
+ from_state = models.CharField(max_length=31, choices=PreprintStates.choices())
+ to_state = models.CharField(max_length=31, choices=PreprintStates.choices())
 
 
 class NodeRequestAction(BaseAction):

osf/models/mixins.py

@@ -35,7 +35,7 @@
 from osf.utils.fields import NonNaiveDateTimeField
 from osf.utils.datetime_aware_jsonfield import DateTimeAwareJSONField
 from osf.utils.machines import (
- ReviewsMachine,
+ PreprintStateMachine,
  NodeRequestMachine,
  PreprintRequestMachine,
 )
@@ -45,8 +45,8 @@
 from osf.utils.workflows import (
  DefaultStates,
  DefaultTriggers,
- ReviewStates,
- ReviewTriggers,
+ PreprintStates,
+ PreprintTriggers,
 )
 
 from osf.utils.requests import get_request_and_user_id
@@ -874,14 +874,14 @@ class Meta:
 class ReviewableMixin(MachineableMixin):
  """Something that may be included in a reviewed collection and is subject to a reviews workflow.
  """
- TriggersClass = ReviewTriggers
+ TriggersClass = PreprintTriggers
 
- machine_state = models.CharField(max_length=15, db_index=True, choices=ReviewStates.choices(), default=ReviewStates.INITIAL.value)
+ machine_state = models.CharField(max_length=15, db_index=True, choices=PreprintStates.choices(), default=PreprintStates.INITIAL.value)
 
  class Meta:
  abstract = True
 
- MachineClass = ReviewsMachine
+ MachineClass = PreprintStateMachine
 
  @property
  def in_public_reviews_state(self):
@@ -960,7 +960,7 @@ class ReviewProviderMixin(GuardianMixin):
  """
 
  REVIEWABLE_RELATION_NAME = None
- REVIEW_STATES = ReviewStates
+ REVIEW_STATES = PreprintStates
  STATE_FIELD_NAME = 'machine_state'
 
  groups = REVIEW_GROUPS

osf/models/preprint.py

@@ -29,7 +29,7 @@
 from .mixins import ReviewableMixin, Taggable, Loggable, GuardianMixin
 from .validators import validate_doi
 from osf.utils.fields import NonNaiveDateTimeField
-from osf.utils.workflows import DefaultStates, ReviewStates
+from osf.utils.workflows import DefaultStates, PreprintStates
 from osf.utils import sanitize
 from osf.utils.permissions import ADMIN, WRITE
 from osf.utils.requests import get_request_and_user_id, string_type_request_headers
@@ -581,7 +581,7 @@ def set_published(self, published, auth, save=False):
  self.set_privacy('public', log=False, save=False)
 
  # In case this provider is ever set up to use a reviews workflow, put this preprint in a sensible state
- self.machine_state = ReviewStates.ACCEPTED.value
+ self.machine_state = PreprintStates.ACCEPTED.value
  self.date_last_transitioned = self.date_published
 
  # This preprint will have a tombstone page when it's withdrawn.

osf/utils/machines.py

@@ -12,10 +12,10 @@
 from osf.utils.workflows import (
  DefaultStates,
  DefaultTriggers,
- ReviewStates,
+ PreprintStates,
  ApprovalStates,
  DEFAULT_TRANSITIONS,
- REVIEWABLE_TRANSITIONS,
+ PREPRINT_STATE_TRANSITIONS,
  APPROVAL_TRANSITIONS,
  CollectionSubmissionStates,
  COLLECTION_SUBMISSION_TRANSITIONS,
@@ -98,13 +98,12 @@ def update_last_transitioned(self, ev):
  self.machineable.date_last_transitioned = now
 
 
-class ReviewsMachine(BaseMachine):
+class PreprintStateMachine(BaseMachine):
  ActionClass = ReviewAction
- States = ReviewStates
- Transitions = REVIEWABLE_TRANSITIONS
+ States = PreprintStates
+ Transitions = PREPRINT_STATE_TRANSITIONS
 
- def save_changes(self, ev):
- now = self.action.created if self.action is not None else timezone.now()
+ def _validate_transitions(self, ev):
  should_publish = self.machineable.in_public_reviews_state
  if self.machineable.is_retracted:
  pass # Do not alter published state
@@ -115,11 +114,16 @@ def save_changes(self, ev):
  raise ValueError('Preprint provider not specified; cannot publish.')
  if not self.machineable.subjects.exists():
  raise ValueError('Preprint must have at least one subject to be published.')
- self.machineable.date_published = now
- self.machineable.is_published = True
- self.machineable.ever_public = True
- elif not should_publish and self.machineable.is_published:
+
+ def save_changes(self, ev):
+ should_publish = self.machineable.in_public_reviews_state
+ now = self.action.created if self.action is not None else timezone.now()
+ self.machineable.date_published = now
+ self.machineable.is_published = True
+ self.machineable.ever_public = True
+ if not should_publish and self.machineable.is_published:
  self.machineable.is_published = False
+
  self.machineable.save()
 
  def resubmission_allowed(self, ev):

osf/utils/workflows.py

@@ -220,25 +220,25 @@ def db_name(self):
  ('REJECT', 'reject'),
  ('EDIT_COMMENT', 'edit_comment'),
 ]
-REVIEW_STATES = DEFAULT_STATES + [
+PREPRINT_STATES = DEFAULT_STATES + [
  ('WITHDRAWN', 'withdrawn'),
 ]
-REVIEW_TRIGGERS = DEFAULT_TRIGGERS + [
+PREPRINT_TRIGGERS = DEFAULT_TRIGGERS + [
  ('WITHDRAW', 'withdraw')
 ]
 
-REGISTRATION_STATES = REVIEW_STATES + [
+REGISTRATION_STATES = PREPRINT_STATES + [
  ('EMBARGO', 'embargo'),
  ('PENDING_EMBARGO_TERMINATION', 'pending_embargo_termination'),
  ('PENDING_WITHDRAW_REQUEST', 'pending_withdraw_request'),
  ('PENDING_WITHDRAW', 'pending_withdraw'),
 ]
 
 DefaultStates = ChoiceEnum('DefaultStates', DEFAULT_STATES)
-ReviewStates = ChoiceEnum('ReviewStates', REVIEW_STATES)
+PreprintStates = ChoiceEnum('PreprintStates', PREPRINT_STATES)
 RegistrationStates = ChoiceEnum('RegistrationStates', REGISTRATION_STATES)
 DefaultTriggers = ChoiceEnum('DefaultTriggers', DEFAULT_TRIGGERS)
-ReviewTriggers = ChoiceEnum('ReviewTriggers', REVIEW_TRIGGERS)
+PreprintTriggers = ChoiceEnum('PreprintTriggers', PREPRINT_TRIGGERS)
 
 CHRONOS_STATUS_STATES = [
  ('DRAFT', 1),
@@ -256,40 +256,46 @@ def db_name(self):
  'trigger': DefaultTriggers.SUBMIT.value,
  'source': [DefaultStates.INITIAL.value],
  'dest': DefaultStates.PENDING.value,
+ 'before': ['_validate_transitions'],
  'after': ['save_action', 'update_last_transitioned', 'save_changes', 'notify_submit'],
  },
  {
  'trigger': DefaultTriggers.SUBMIT.value,
  'source': [DefaultStates.PENDING.value, DefaultStates.REJECTED.value],
  'conditions': 'resubmission_allowed',
  'dest': DefaultStates.PENDING.value,
+ 'before': ['_validate_transitions'],
  'after': ['save_action', 'update_last_transitioned', 'save_changes', 'notify_resubmit'],
  },
  {
  'trigger': DefaultTriggers.ACCEPT.value,
  'source': [DefaultStates.PENDING.value, DefaultStates.REJECTED.value],
  'dest': DefaultStates.ACCEPTED.value,
+ 'before': ['_validate_transitions'],
  'after': ['save_action', 'update_last_transitioned', 'save_changes', 'notify_accept_reject'],
  },
  {
  'trigger': DefaultTriggers.REJECT.value,
  'source': [DefaultStates.PENDING.value, DefaultStates.ACCEPTED.value],
  'dest': DefaultStates.REJECTED.value,
+ 'before': ['_validate_transitions'],
  'after': ['save_action', 'update_last_transitioned', 'save_changes', 'notify_accept_reject'],
  },
  {
  'trigger': DefaultTriggers.EDIT_COMMENT.value,
  'source': [DefaultStates.PENDING.value, DefaultStates.REJECTED.value, DefaultStates.ACCEPTED.value],
  'dest': '=',
+ 'before': ['_validate_transitions'],
  'after': ['save_action', 'save_changes', 'notify_edit_comment'],
  },
 ]
 
-REVIEWABLE_TRANSITIONS = DEFAULT_TRANSITIONS + [
+PREPRINT_STATE_TRANSITIONS = DEFAULT_TRANSITIONS + [
  {
- 'trigger': ReviewTriggers.WITHDRAW.value,
- 'source': [ReviewStates.PENDING.value, ReviewStates.ACCEPTED.value],
- 'dest': ReviewStates.WITHDRAWN.value,
+ 'trigger': PreprintTriggers.WITHDRAW.value,
+ 'source': [PreprintStates.PENDING.value, PreprintStates.ACCEPTED.value],
+ 'dest': PreprintStates.WITHDRAWN.value,
+ 'before': ['_validate_transitions'],
  'after': ['save_action', 'update_last_transitioned', 'perform_withdraw', 'save_changes', 'notify_withdraw']
  }
 ]

osf_tests/metadata/test_osf_gathering.py

@@ -709,7 +709,7 @@ def test_gather_preprint_withdrawal(self):
  # withdrawn via PreprintRequest
  _withdrawal_request = factories.PreprintRequestFactory(
  target=self.preprint,
- machine_state=workflows.ReviewStates.ACCEPTED.value,
+ machine_state=workflows.PreprintStates.ACCEPTED.value,
  request_type=workflows.RequestTypes.WITHDRAWAL.value,
  creator=self.user__admin,
  comment='request unprint',