[ENG-12564] Remove Primary Preprint File from draft state

addons/base/views.py

@@ -1134,6 +1134,13 @@ def persistent_file_download(auth, **kwargs):
             referent, _ = Guid.load_referent(id_or_guid)
             file = referent.primary_file
 
+    # Initial and rejected preprints may have no primary file
+    if file is None:
+        raise HTTPError(http_status.HTTP_404_NOT_FOUND, data={
+            'message_short': 'File Not Found',
+            'message_long': 'The requested file could not be found.'
+        })
+
     if not file.is_file:
         raise HTTPError(http_status.HTTP_400_BAD_REQUEST, data={
             'message_long': 'Downloading folders is not permitted.'

addons/osfstorage/models.py

@@ -145,7 +145,8 @@ def current_version_number(self):
         return int(latest_identifier) if latest_identifier else 1
 
     def _check_delete_allowed(self):
-        if self.is_preprint_primary:
+        # Initial and rejected preprints may be left without a primary file, so authors can replace a wrong one
+        if self.is_preprint_primary and self.target.requires_primary_file:
             raise exceptions.FileNodeIsPrimaryFile()
         if self.is_checked_out:
             raise exceptions.FileNodeCheckedOutError()
@@ -439,7 +440,11 @@ def remove_tags(self, tags, auth, save=True):
             self.remove_tag(tag, auth, save)
 
     def delete(self, user=None, **kwargs):
+        preprint = self.target if self.is_preprint_primary else None
         ret = super().delete(user, **kwargs)
+        if preprint:
+            # reachable when the preprint allows having no primary file
+            preprint.remove_primary_file()
         self.update_search()
         return ret
 

addons/osfstorage/tests/test_models.py

@@ -311,6 +311,35 @@ def test_delete_preprint_primary_file(self):
         with pytest.raises(FileNodeIsPrimaryFile):
             file.delete()
 
+    def test_delete_primary_file_of_pending_preprint(self):
+        preprint = PreprintFactory(is_published=False, machine_state='pending')
+        file = preprint.files.get()
+
+        with pytest.raises(FileNodeIsPrimaryFile):
+            file.delete()
+        preprint.reload()
+        assert preprint.primary_file == file
+
+    def test_delete_primary_file_of_initial_or_rejected_preprint(self):
+        for machine_state in ('initial', 'rejected'):
+            preprint = PreprintFactory(is_published=False, machine_state=machine_state)
+            file = preprint.files.get()
+
+            file.delete()
+            preprint.reload()
+            assert preprint.primary_file is None
+            assert preprint.machine_state == machine_state
+            assert models.TrashedFile.load(file._id)
+
+    def test_delete_folder_with_primary_file_of_initial_preprint(self):
+        preprint = PreprintFactory(is_published=False)
+        folder = preprint.root_folder.append_folder('Drafts')
+        preprint.primary_file.move_under(folder)
+
+        folder.delete()
+        preprint.reload()
+        assert preprint.primary_file is None
+
     def test_delete_file_no_guid(self):
         child = self.node_settings.get_root().append_file('Test')
 

addons/osfstorage/tests/test_views.py

@@ -1091,6 +1091,30 @@ def test_attempt_delete_while_preprint(self):
         res = self.delete(self.preprint.primary_file)
         assert res.status_code == 403
 
+    def test_attempt_delete_while_preprint_pending(self):
+        self.node = self.preprint = PreprintFactory(creator=self.user, is_published=False, machine_state='pending')
+        res = self.delete(self.preprint.primary_file)
+        assert res.status_code == 403
+        assert res.json['message_long'] == 'Cannot delete file as it is the primary file of preprint.'
+
+    def test_delete_while_preprint_initial(self):
+        self.node = self.preprint = PreprintFactory(creator=self.user, is_published=False, machine_state='initial')
+        primary_file = self.preprint.primary_file
+        res = self.delete(primary_file)
+        assert res.status_code == 200
+        self.preprint.reload()
+        assert self.preprint.primary_file is None
+        assert models.TrashedFileNode.load(primary_file._id)
+
+    def test_delete_while_preprint_rejected(self):
+        self.node = self.preprint = PreprintFactory(creator=self.user, is_published=False, machine_state='rejected')
+        primary_file = self.preprint.primary_file
+        res = self.delete(primary_file)
+        assert res.status_code == 200
+        self.preprint.reload()
+        assert self.preprint.primary_file is None
+        assert models.TrashedFileNode.load(primary_file._id)
+
     def test_attempt_delete_folder_with_preprint(self):
         folder = self.root_node.append_folder('Fishes')
         file = folder.append_file('Fish')
@@ -1659,6 +1683,19 @@ def test_download_file(self):
         redirect = self.app.get(url, auth=self.user.auth)
         assert redirect.status_code == 400
 
+    def test_download_preprint_without_primary_file(self):
+        self.preprint = PreprintFactory(creator=self.user, is_published=False, machine_state='rejected')
+        res = self.app.get(f'/download/{self.preprint._id}/', auth=self.user.auth)
+        assert res.status_code == 302
+        res = self.app.get(f'/{self.preprint._id}/download/', auth=self.user.auth)
+        assert res.status_code == 302
+
+        self.preprint.remove_primary_file()
+        res = self.app.get(f'/download/{self.preprint._id}/', auth=self.user.auth)
+        assert res.status_code == 404
+        res = self.app.get(f'/{self.preprint._id}/download/', auth=self.user.auth)
+        assert res.status_code == 404
+
     @responses.activate
     @mock.patch('framework.auth.cas.get_client')
     def test_download_file_with_token(self, mock_get_client):

api_tests/actions/views/test_action_list.py

@@ -393,6 +393,40 @@ def test_submit_preprint_without_files_returns_400(self, app, url, preprint, nod
             expect_errors=True
         )
         assert res.status_code == 400
+        assert res.json['errors'][0]['detail'] == 'A primary file is required to submit a preprint for moderation.'
+
+    def test_resubmit_rejected_preprint_without_file_returns_400(self, app, url, preprint, node_admin):
+        preprint.machine_state = 'rejected'
+        preprint.primary_file = None
+        preprint.save()
+
+        res = app.post_json_api(
+            url,
+            self.create_payload(preprint._id, trigger='submit'),
+            auth=node_admin.auth,
+            expect_errors=True
+        )
+        assert res.status_code == 400
+        assert res.json['errors'][0]['detail'] == 'A primary file is required to submit a preprint for moderation.'
+        preprint.refresh_from_db()
+        assert preprint.machine_state == 'rejected'
+        assert not preprint.actions.exists()
+
+    def test_accept_rejected_preprint_without_file_returns_400(self, app, url, preprint, moderator):
+        preprint.machine_state = 'rejected'
+        preprint.primary_file = None
+        preprint.save()
+
+        res = app.post_json_api(
+            url,
+            self.create_payload(preprint._id, trigger='accept', comment='Looks good.'),
+            auth=moderator.auth,
+            expect_errors=True
+        )
+        assert res.status_code == 400
+        preprint.refresh_from_db()
+        assert preprint.machine_state == 'rejected'
+        assert not preprint.is_published
 
     def test_provider_not_reviewed_returns_409(self, app, url, preprint, node_admin):
         preprint.provider = PreprintProviderFactory(reviews_workflow=None)

osf/models/preprint.py

@@ -708,6 +708,12 @@ def is_preprint_orphan(self):
     def has_submitted_preprint(self):
         return self.machine_state != DefaultStates.INITIAL.value
 
+    @property
+    def requires_primary_file(self):
+        # Authors may leave a preprint without a primary file while they are still working on it: before the first
+        # submission or after a moderator rejected it
+        return self.machine_state not in (DefaultStates.INITIAL.value, DefaultStates.REJECTED.value)
+
     @property
     def is_pending_moderation(self):
         if self.machine_state == DefaultStates.INITIAL.value:
@@ -927,6 +933,12 @@ def set_primary_file(self, preprint_file, auth, save=False, **kwargs):
             self.save()
         update_or_enqueue_on_preprint_updated(preprint_id=self._id, saved_fields=['primary_file'])
 
+    def remove_primary_file(self):
+        """Unlink the primary file, after it was deleted"""
+        self.primary_file = None
+        self.save()
+        update_or_enqueue_on_preprint_updated(preprint_id=self._id, saved_fields=['primary_file'])
+
     @require_permission([ADMIN])
     def set_published(self, published, auth, save=False, **kwargs):
         if self.is_published and not published:
@@ -1045,8 +1057,8 @@ def save(self, *args, **kwargs):
 
         if not first_save and ('ever_public' in saved_fields and saved_fields['ever_public']):
             raise ValidationError('Cannot set "ever_public" to False')
-        if self.has_submitted_preprint and not self.primary_file:
-            raise ValidationError('Cannot save non-initial preprint without primary file.')
+        if self.requires_primary_file and not self.primary_file:
+            raise ValidationError('Only initial or rejected preprints can be saved without a primary file.')
 
         ret = super().save(*args, **kwargs)
 
@@ -1697,6 +1709,9 @@ def run_submit(self, user):
         Params:
             user: The user triggering this transition.
         """
+        # Initial and rejected preprints may have no primary file, but one is required to (re)submit for moderation
+        if self.is_preprint_orphan:
+            raise ValueError('A primary file is required to submit a preprint for moderation.')
         ret = super().run_submit(user=user)
         provider = self.provider
         reviews_workflow = provider.reviews_workflow

osf_tests/test_reviewable.py

@@ -1,10 +1,20 @@
 from unittest import mock
 import pytest
 
+from addons.osfstorage.models import OsfStorageFile
+from api_tests.utils import create_test_preprint_file
+from framework.auth import Auth
 from osf.models import Preprint, NotificationTypeEnum
 from osf.utils.workflows import DefaultStates
 from osf_tests.factories import PreprintFactory, AuthUserFactory
 from tests.utils import capture_notifications
+from website.files.exceptions import FileNodeIsPrimaryFile
+
+
+def delete_primary_file(preprint):
+    # Load the file fresh, like WaterButler's delete hook does, so it sees the preprint's current state
+    OsfStorageFile.objects.get(id=preprint.primary_file_id).delete()
+    preprint.refresh_from_db()
 
 
 @pytest.mark.django_db
@@ -61,3 +71,41 @@ def test_reject_resubmission_sends_emails(self):
         assert len(notifications['emits']) == 1
         assert notifications['emits'][0]['type'] == NotificationTypeEnum.PROVIDER_REVIEWS_RESUBMISSION_CONFIRMATION
         assert preprint.machine_state == DefaultStates.PENDING.value
+
+    def test_submit_requires_primary_file(self):
+        user = AuthUserFactory()
+        preprint = PreprintFactory(reviews_workflow='pre-moderation', is_published=False, creator=user)
+        delete_primary_file(preprint)
+        assert preprint.primary_file is None
+
+        with pytest.raises(ValueError, match='A primary file is required'):
+            preprint.run_submit(user)
+        preprint.refresh_from_db()
+        assert preprint.machine_state == DefaultStates.INITIAL.value
+        assert not preprint.actions.exists()
+
+    def test_rejected_preprint_can_replace_primary_file_and_resubmit(self):
+        user = AuthUserFactory()
+        preprint = PreprintFactory(reviews_workflow='pre-moderation', is_published=False, creator=user)
+        with capture_notifications():
+            preprint.run_submit(user)
+        with pytest.raises(FileNodeIsPrimaryFile):
+            delete_primary_file(preprint)
+
+        with capture_notifications():
+            preprint.run_reject(user, 'wrong file')
+        delete_primary_file(preprint)
+        assert preprint.machine_state == DefaultStates.REJECTED.value
+        assert preprint.primary_file is None
+
+        with pytest.raises(ValueError, match='A primary file is required'):
+            preprint.run_submit(user)
+        preprint.refresh_from_db()
+        assert preprint.machine_state == DefaultStates.REJECTED.value
+
+        new_file = create_test_preprint_file(preprint, user, filename='correct_file.pdf')
+        preprint.set_primary_file(new_file, auth=Auth(user), save=True)
+        with capture_notifications():
+            preprint.run_submit(user)
+        assert preprint.machine_state == DefaultStates.PENDING.value
+        assert preprint.primary_file == new_file

tests/test_preprints.py

@@ -1521,6 +1521,26 @@ def test_cant_update_without_file(self):
         with pytest.raises(ValidationError):
             self.preprint.save()
 
+    def test_can_save_rejected_preprint_without_file(self):
+        self.preprint.set_primary_file(self.file, auth=self.auth, save=True)
+        self.preprint.machine_state = DefaultStates.REJECTED.value
+        self.preprint.save()
+
+        self.preprint.remove_primary_file()
+        self.preprint.reload()
+        assert self.preprint.primary_file is None
+        assert self.preprint.is_preprint_orphan
+
+    def test_cant_save_pending_preprint_without_file(self):
+        self.preprint.set_primary_file(self.file, auth=self.auth, save=True)
+        self.preprint.machine_state = DefaultStates.PENDING.value
+        self.preprint.save()
+
+        with pytest.raises(ValidationError):
+            self.preprint.remove_primary_file()
+        self.preprint.reload()
+        assert self.preprint.primary_file == self.file
+
 
 class TestPreprintPermissions(OsfTestCase):
     def setUp(self):

website/views.py

@@ -224,6 +224,9 @@ def resolve_guid_download(guid, provider=None):
                     resource.has_permission(auth.user, permissions.ADMIN)):
                 raise HTTPError(http_status.HTTP_404_NOT_FOUND)
         resource = resource.primary_file
+        # Initial and rejected preprints may have no primary file
+        if resource is None:
+            raise HTTPError(http_status.HTTP_404_NOT_FOUND)
 
     request.args = request.args.copy()
     if 'revision' not in request.args:  # This is to maintain legacy behavior