[ENG-12060] Rejected registrations disappearing for registration providers

api/nodes/views.py

@@ -240,11 +240,11 @@ def get_draft(self, draft_id=None, check_object_permissions=True):
         self.check_branched_from(draft)
 
         if self.request.method not in drf_permissions.SAFE_METHODS:
-            if draft.registered_node and not draft.registered_node.is_deleted:
+            if draft.has_active_registration:
                 raise PermissionDenied('This draft has already been registered and cannot be modified.')
 
         else:
-            if draft.registered_node and not draft.registered_node.is_deleted:
+            if draft.has_active_registration:
                 redirect_url = draft.registered_node.absolute_api_v2_url
                 self.headers['location'] = redirect_url
                 raise PermanentlyMovedError(detail='Draft has already been registered')

api/schema_responses/permissions.py

@@ -11,7 +11,11 @@
 from osf.utils.workflows import ApprovalStates
 
 
-MODERATOR_VISIBLE_STATES = [ApprovalStates.PENDING_MODERATION, ApprovalStates.APPROVED]
+MODERATOR_VISIBLE_STATES = [
+    ApprovalStates.PENDING_MODERATION,
+    ApprovalStates.APPROVED,
+    ApprovalStates.MODERATOR_REJECTED,
+]
 
 
 class SchemaResponseParentPermission:
@@ -20,7 +24,7 @@ class SchemaResponseParentPermission:
     To GET a SchemaResponse (or a subpath), one of three conditions must be met:
       *  The user must have "read" permissions on the parent resource
       *  The user must be a moderator on the parent resource's Provider and the
-         SchemaResponse must be in an APPROVED or PENDING_MODERATION state
+         SchemaResponse must be in an APPROVED, PENDING_MODERATION or MODERATOR_REJECTED state
       *  The SchemaResponse must be APPROVED and the parent resource must be public
 
     For DELETE/PATCH/POST/PUT, the required permission should be added in the

api_tests/actions/views/test_schema_response_action_detail.py

@@ -110,10 +110,14 @@ def get_status_code_for_preconditions(
         if role == 'non-contributor':
             return 403
 
-        # Moderators can GET PENDING_MODERATION and APPROVED SchemaResponses on
+        # Moderators can GET PENDING_MODERATION, APPROVED and MODERATOR_REJECTED SchemaResponses on
         # public or private registrations that are part of a moderated registry
         if role == 'moderator':
-            moderator_visible_states = [ApprovalStates.PENDING_MODERATION, ApprovalStates.APPROVED]
+            moderator_visible_states = [
+                ApprovalStates.PENDING_MODERATION,
+                ApprovalStates.APPROVED,
+                ApprovalStates.MODERATOR_REJECTED,
+            ]
             if schema_response_state in moderator_visible_states and reviews_workflow is not None:
                 return 200
             else:

api_tests/actions/views/test_schema_response_action_list.py

@@ -146,10 +146,14 @@ def get_status_code_for_preconditions(
         if role == 'non-contributor':
             return 403
 
-        # Moderators can GET PENDING_MODERATION and APPROVED SchemaResponses on
+        # Moderators can GET PENDING_MODERATION, APPROVED and MODERATOR_REJECTED SchemaResponses on
         # public or private registrations that are part of a moderated registry
         if role == 'moderator':
-            moderator_visible_states = [ApprovalStates.PENDING_MODERATION, ApprovalStates.APPROVED]
+            moderator_visible_states = [
+                ApprovalStates.PENDING_MODERATION,
+                ApprovalStates.APPROVED,
+                ApprovalStates.MODERATOR_REJECTED,
+            ]
             if schema_response_state in moderator_visible_states and reviews_workflow is not None:
                 return 200
             else:

api_tests/registries_moderation/test_submissions.py

@@ -1,7 +1,9 @@
 import pytest
 import datetime
+from unittest import mock
 
 from api.base.settings.defaults import API_BASE
+from framework.auth import Auth
 
 from api.providers.workflows import Workflows
 from osf.utils.workflows import NodeRequestTypes, RegistrationModerationTriggers, RegistrationModerationStates
@@ -14,7 +16,8 @@
     NodeRequestFactory,
     EmbargoFactory,
     RetractionFactory,
-    ProjectFactory
+    ProjectFactory,
+    SubjectFactory,
 )
 
 
@@ -351,6 +354,58 @@ def test_registries_moderation_post_reject_moderator(self, app, registration, re
         assert resp.json['data']['attributes']['trigger'] == RegistrationModerationTriggers.REJECT_SUBMISSION.db_name
         registration.refresh_from_db()
         assert registration.moderation_state == RegistrationModerationStates.REJECTED.db_name
+        assert not registration.is_deleted
+
+    def test_moderator_can_view_rejected_registration(self, app, registration, moderator, moderator_wrong_provider, registration_actions_url, actions_payload_base):
+        registration.require_approval(user=registration.creator)
+        with capture_notifications():
+            registration.registration_approval.accept()
+
+        actions_payload_base['data']['attributes']['trigger'] = RegistrationModerationTriggers.REJECT_SUBMISSION.db_name
+        actions_payload_base['data']['relationships']['target']['data']['id'] = registration._id
+        with capture_notifications():
+            resp = app.post_json_api(registration_actions_url, actions_payload_base, auth=moderator.auth)
+        assert resp.status_code == 201
+
+        registration_url = f'/{API_BASE}registrations/{registration._id}/'
+        assert app.get(registration_url, auth=moderator.auth).status_code == 200
+        resp = app.get(f'{registration_url}schema_responses/', auth=moderator.auth)
+        assert [entry['id'] for entry in resp.json['data']] == [registration.schema_responses.last()._id]
+
+        # Rejected registrations stay private for everyone else
+        assert app.get(registration_url, auth=moderator_wrong_provider.auth, expect_errors=True).status_code == 403
+        assert app.get(registration_url, auth=AuthUserFactory().auth, expect_errors=True).status_code == 403
+        assert app.get(registration_url, expect_errors=True).status_code == 401
+
+    def test_rejected_submission_draft_is_returned_for_resubmission(self, app, registration, reg_creator, moderator, registration_actions_url, actions_payload_base):
+        registration.require_approval(user=registration.creator)
+        with capture_notifications():
+            registration.registration_approval.accept()
+
+        actions_payload_base['data']['attributes']['trigger'] = RegistrationModerationTriggers.REJECT_SUBMISSION.db_name
+        actions_payload_base['data']['relationships']['target']['data']['id'] = registration._id
+        with capture_notifications():
+            resp = app.post_json_api(registration_actions_url, actions_payload_base, auth=moderator.auth)
+        assert resp.status_code == 201
+
+        # The draft is back in the author's drafts and can be edited
+        draft = registration.draft_registration.get()
+        assert draft in reg_creator.draft_registrations_active
+        draft_url = f'/{API_BASE}draft_registrations/{draft._id}/'
+        assert app.get(draft_url, auth=reg_creator.auth).status_code == 200
+        payload = {'data': {'type': 'draft_registrations', 'id': draft._id, 'attributes': {'title': 'Fixed title'}}}
+        assert app.patch_json_api(draft_url, payload, auth=reg_creator.auth).status_code == 200
+
+        # Resubmitting creates a new registration, the rejected one stays as it is
+        draft.subjects.add(SubjectFactory())
+        with mock.patch('framework.celery_tasks.handlers.enqueue_task'):
+            resubmitted = draft.register(Auth(reg_creator), save=True)
+        draft.refresh_from_db()
+        registration.refresh_from_db()
+        assert draft.registered_node == resubmitted
+        assert draft not in reg_creator.draft_registrations_active
+        assert registration.moderation_state == RegistrationModerationStates.REJECTED.db_name
+        assert not registration.is_deleted
 
     def test_registries_moderation_post_embargo(self, app, embargo_registration, moderator, provider, embargo_registration_actions_url, actions_payload_base, reg_creator):
         assert embargo_registration.moderation_state == RegistrationModerationStates.INITIAL.db_name
@@ -387,6 +442,7 @@ def test_registries_moderation_post_embargo_reject(self, app, embargo_registrati
         assert resp.json['data']['attributes']['trigger'] == RegistrationModerationTriggers.REJECT_SUBMISSION.db_name
         embargo_registration.refresh_from_db()
         assert embargo_registration.moderation_state == RegistrationModerationStates.REJECTED.db_name
+        assert not embargo_registration.is_deleted
 
     @pytest.mark.usefixtures('mock_gravy_valet_get_verified_links')
     def test_registries_moderation_post_withdraw_accept(self, app, retract_registration, moderator, retract_registration_actions_url, actions_payload_base, provider):
@@ -627,6 +683,7 @@ def test_rejected_submission_doesnt_break_ham_functionality(self, app, registrat
         assert resp.json['data']['attributes']['trigger'] == RegistrationModerationTriggers.REJECT_SUBMISSION.db_name
         registration.refresh_from_db()
         assert registration.moderation_state == RegistrationModerationStates.REJECTED.db_name
+        assert not registration.is_deleted
 
         # ham the project creator
         user.confirm_ham(save=True)

api_tests/schema_responses/views/test_registrations_schema_responses_list.py

@@ -186,8 +186,8 @@ class TestRegistrationSchemaResponseListGETBehavior:
     Contributors on the base Registration should be able to see all SchemaResponses
     on a registration, whether approved or not, for both public and private Registrations.
 
-    Moderators should be able to see both PENDING_MODERATION and APPROVED SchemaResponses
-    on Registrations that are part of the moderated provider.
+    Moderators should be able to see PENDING_MODERATION, APPROVED and MODERATOR_REJECTED
+    SchemaResponses on Registrations that are part of the moderated provider.
 
     Non-contributors should only see APPROVED SchemaResponses on Public registrations
     (permissions tests verify 403/401 response for non-contributors on a private registration).
@@ -242,7 +242,12 @@ def test_GET__moderated_registration_responses_as_moderator(
 
         # Always expect the APPROVED response
         expected_ids = {updated_response.previous_response._id}
-        if response_state in [ApprovalStates.PENDING_MODERATION, ApprovalStates.APPROVED]:
+        moderator_visible_states = [
+            ApprovalStates.PENDING_MODERATION,
+            ApprovalStates.APPROVED,
+            ApprovalStates.MODERATOR_REJECTED,
+        ]
+        if response_state in moderator_visible_states:
             expected_ids.add(updated_response._id)
         encountered_ids = {entry['id'] for entry in resp.json['data']}
         assert encountered_ids == expected_ids

api_tests/schema_responses/views/test_schema_response_detail.py

@@ -136,10 +136,14 @@ def get_status_code_for_preconditions(
         if role == 'non-contributor':
             return 403
 
-        # Moderators can GET PENDING_MODERATION and APPROVED SchemaResponses on
+        # Moderators can GET PENDING_MODERATION, APPROVED and MODERATOR_REJECTED SchemaResponses on
         # public or private registrations that are part of a moderated registry
         if role == 'moderator':
-            moderator_visible_states = [ApprovalStates.PENDING_MODERATION, ApprovalStates.APPROVED]
+            moderator_visible_states = [
+                ApprovalStates.PENDING_MODERATION,
+                ApprovalStates.APPROVED,
+                ApprovalStates.MODERATOR_REJECTED,
+            ]
             if schema_response_state in moderator_visible_states and reviews_workflow is not None:
                 return 200
             else:

osf/models/node.py

@@ -59,7 +59,7 @@
 from osf.utils.datetime_aware_jsonfield import DateTimeAwareJSONField
 from osf.utils.fields import NonNaiveDateTimeField
 from osf.utils.requests import get_request_and_user_id, string_type_request_headers, get_current_request
-from osf.utils.workflows import CollectionSubmissionStates
+from osf.utils.workflows import CollectionSubmissionStates, RegistrationModerationStates
 from osf.utils import sanitize
 from website import language, settings
 from website.citations.utils import datetime_to_csl
@@ -680,7 +680,11 @@ def draft_registrations_active(self):
         return DraftRegistration.objects.filter(
             models.Q(branched_from=self) &
             models.Q(deleted__isnull=True) &
-            (models.Q(registered_node=None) | models.Q(registered_node__deleted__isnull=False)),
+            (
+                models.Q(registered_node=None)
+                | models.Q(registered_node__deleted__isnull=False)
+                | models.Q(registered_node__moderation_state=RegistrationModerationStates.REJECTED.db_name)
+            ),
         )
 
     @property

osf/models/registrations.py

@@ -1161,6 +1161,25 @@ def branched_from_type(self):
     def has_project(self):
         return isinstance(self.branched_from, Node)
 
+    @property
+    def has_active_registration(self):
+        """
+        Check whether this draft has an active registration, so it can no longer be edited.
+        A registration that was deleted or rejected by a moderator is not active: its draft is
+        returned to the contributors for resubmission.
+        """
+        registration = self.registered_node
+        if registration is None:
+            # Not registered yet
+            return False
+        if registration.is_deleted:
+            # Rejected by a contributor or failed to archive
+            return False
+        if registration.moderation_state == RegistrationModerationStates.REJECTED.db_name:
+            # Rejected by a moderator
+            return False
+        return True
+
     @property
     def url(self):
         return self.URL_TEMPLATE.format(

osf/models/sanctions.py

@@ -687,6 +687,9 @@ def _on_reject(self, event_data):
                 'embargo_id': self._id,
             },
             auth=Auth(user) if user else Auth(self.initiated_by))
+        # Registrations rejected by a moderator are kept in the REJECTED moderation state
+        if self.approval_stage is ApprovalStates.MODERATOR_REJECTED:
+            return
         # Remove backref to parent project if embargo was for a new registration
         if not self.for_existing_registration:
             parent_registration.delete_registration_tree(save=True)
@@ -1052,7 +1055,9 @@ def _on_reject(self, event_data):
         NodeLog = apps.get_model('osf.NodeLog')
 
         registered_from = self.target_registration.registered_from
-        self.target_registration.delete_registration_tree(save=True)
+        # Registrations rejected by a moderator are kept in the REJECTED moderation state
+        if self.approval_stage is not ApprovalStates.MODERATOR_REJECTED:
+            self.target_registration.delete_registration_tree(save=True)
         registered_from.add_log(
             action=NodeLog.REGISTRATION_APPROVAL_CANCELLED,
             params={

osf/models/user.py

@@ -65,6 +65,7 @@
 from osf.utils.names import impute_names
 from osf.utils.requests import check_select_for_update
 from osf.utils.permissions import API_CONTRIBUTOR_PERMISSIONS, MANAGER, MEMBER, ADMIN
+from osf.utils.workflows import RegistrationModerationStates
 from website import settings as website_settings
 from website import filters
 from website.project import new_bookmark_collection
@@ -971,7 +972,11 @@ def draft_registrations_active(self):
         """
 
         return self.draft_registrations.filter(
-            (models.Q(registered_node__isnull=True) | models.Q(registered_node__deleted__isnull=False)),
+            (
+                models.Q(registered_node__isnull=True)
+                | models.Q(registered_node__deleted__isnull=False)
+                | models.Q(registered_node__moderation_state=RegistrationModerationStates.REJECTED.db_name)
+            ),
             branched_from__deleted__isnull=True,
             deleted__isnull=True,
         )

osf/utils/workflows.py

@@ -128,6 +128,7 @@ def in_moderation_states(cls):
             cls.EMBARGO.db_name,
             cls.PENDING_EMBARGO_TERMINATION.db_name,
             cls.PENDING_WITHDRAW.db_name,
+            cls.REJECTED.db_name,
         ]
 
 

osf_tests/test_draft_registration.py

@@ -6,6 +6,7 @@
 from osf.exceptions import UserNotAffiliatedError, DraftRegistrationStateError, NodeStateError
 from osf.models import RegistrationSchema, DraftRegistration, DraftRegistrationContributor, NodeLicense, Node, NodeLog
 from osf.utils.permissions import ADMIN, READ, WRITE
+from osf.utils.workflows import RegistrationModerationStates
 from osf_tests.test_node import TestNodeEditableFieldsMixin, TestTagging, TestNodeSubjects
 from osf_tests.test_node_license import TestNodeLicenses
 from django.utils import timezone
@@ -192,6 +193,36 @@ def test_draft_registrations_active(self):
         assert draft2 in project.draft_registrations_active.all()
         assert finished_draft not in project.draft_registrations_active.all()
 
+    def test_draft_registrations_includes_drafts_of_moderator_rejected_registrations(self):
+        project = factories.ProjectFactory()
+        rejected_registration = factories.RegistrationFactory(project=project)
+        rejected_registration.moderation_state = RegistrationModerationStates.REJECTED.db_name
+        rejected_registration.save()
+
+        draft = factories.DraftRegistrationFactory(branched_from=project, user=project.creator)
+        draft.registered_node = rejected_registration
+        draft.save()
+
+        assert draft in project.draft_registrations_active.all()
+        assert draft in project.creator.draft_registrations_active.all()
+
+    def test_has_active_registration(self):
+        project = factories.ProjectFactory()
+        draft = factories.DraftRegistrationFactory(branched_from=project)
+        assert not draft.has_active_registration
+
+        registration = factories.RegistrationFactory(project=project)
+        draft.registered_node = registration
+        assert draft.has_active_registration
+
+        registration.moderation_state = RegistrationModerationStates.REJECTED.db_name
+        assert not draft.has_active_registration
+
+        registration.moderation_state = RegistrationModerationStates.ACCEPTED.db_name
+        registration.is_deleted = True
+        registration.deleted = timezone.now()
+        assert not draft.has_active_registration
+
     def test_draft_registration_url(self):
         project = factories.ProjectFactory()
         draft = factories.DraftRegistrationFactory(branched_from=project)

tests/test_registrations/test_review_flows.py

@@ -355,6 +355,7 @@ def test_moderator_rejection_flow(
 
         registration.refresh_from_db()
         assert registration.moderation_state == end_state.db_name
+        assert not registration.is_deleted
 
     @pytest.mark.parametrize('sanction_object', [registration_approval, embargo, retraction])
     def test_admin_cannot_give_moderator_approval(self, sanction_object, provider):

website/project/views/drafts.py

@@ -97,8 +97,7 @@ def validate_registration_choice(registration_choice):
         )
 
 def check_draft_state(draft):
-    registered_and_deleted = draft.registered_node and draft.registered_node.is_deleted
-    if draft.registered_node and not registered_and_deleted:
+    if draft.has_active_registration:
         raise HTTPError(http_status.HTTP_403_FORBIDDEN, data={
             'message_short': 'This draft has already been registered',
             'message_long': 'This draft has already been registered and cannot be modified.'
@@ -192,7 +191,7 @@ def delete_draft_registration(auth, node, draft, *args, **kwargs):
     :return: None
     :rtype: NoneType
     """
-    if draft.registered_node and not draft.registered_node.is_deleted:
+    if draft.has_active_registration:
         raise HTTPError(
             http_status.HTTP_403_FORBIDDEN,
             data={