#1617 do_revision bugfix - now validates for manageable files

src/core/files.py

@@ -476,9 +476,9 @@ def serve_pdf_galley_to_browser(request, file, article):
         raise Http404
 
 
-def delete_file(article_object, file_object):
+def unassociate_file(article_object, file_object):
     """ Deletes a file. Note: the actual file is not deleted, this just removes the association of the file with an
-    article.
+    article.  The history remains.
 
     :param article_object: the article associated with the file
     :param file_object: the file object to delete

src/review/logic.py

@@ -413,7 +413,7 @@ def get_draft_email_message(request, article):
     return render_template.get_message_content(request, email_context, 'draft_message')
 
 
-def group_files(article, reviews):
+def group_files(article, reviews, manageable=False):
     files = list()
 
     for file in article.manuscript_files.all():
@@ -422,10 +422,11 @@ def group_files(article, reviews):
     for file in article.data_figure_files.all():
         files.append(file)
 
-    for review in reviews:
-        if review.for_author_consumption and review.display_review_file:
-            files.append(review.review_file)
-
+    if not manageable:
+        for review in reviews:
+            if review.for_author_consumption and review.display_review_file:
+                files.append(review.review_file)
+
     return files
 
 

src/review/tests.py

@@ -516,7 +516,7 @@ def test_shared_review_download_view(self):
         )
 
         # finally, delete the file from disk
-        files.delete_file(article_with_completed_reviews, file)
+        files.unassociate_file(article_with_completed_reviews, file)
 
 
     def setup_request_object(self):

src/review/views.py

@@ -2034,7 +2034,8 @@ def do_revisions(request, article_id, revision_id):
     ).exclude(decision='withdrawn')
 
     form = forms.DoRevisions(instance=revision_request)
-    revision_files = logic.group_files(revision_request.article, reviews)
+    manageable_files = logic.group_files(revision_request.article, reviews, manageable=True)
+    downloadable_files = logic.group_files(revision_request.article, reviews)
 
     if request.POST:
         post_redirect = reverse(
@@ -2047,15 +2048,22 @@ def do_revisions(request, article_id, revision_id):
         if 'delete' in request.POST:
             file_id = request.POST.get('delete')
             file = get_object_or_404(core_models.File, pk=file_id)
-            files.delete_file(revision_request.article, file)
-            logic.log_revision_event(
-                'File {0} ({1}) deleted.'.format(
-                    file.id,
-                    file.original_filename
-                ),
-                request.user,
-                revision_request,
-            )
+            if file in manageable_files:
+                files.unassociate_file(revision_request.article, file)
+                logic.log_revision_event(
+                    'File {0} ({1}) deleted.'.format(
+                        file.id,
+                        file.original_filename
+                    ),
+                    request.user,
+                    revision_request,
+                )
+            else:
+                messages.add_message(
+                    request,
+                    messages.WARNING,
+                    _('Given file ID not found in article files.')
+                )
             return redirect(post_redirect)
 
         elif 'save' in request.POST:
@@ -2101,7 +2109,7 @@ def do_revisions(request, article_id, revision_id):
     if request.GET.get('file_id', None):
         file_id = request.GET.get('file_id')
         file = get_object_or_404(core_models.File, pk=file_id)
-        if file in revision_files:
+        if file in downloadable_files:
             logic.log_revision_event(
                 'Downloaded file {0} ({1}).'.format(
                     file.label,