diff --git a/src/review/forms.py b/src/review/forms.py index 99835891f5..b5e567dfbc 100755 --- a/src/review/forms.py +++ b/src/review/forms.py @@ -329,6 +329,35 @@ def check_for_potential_errors(self): return potential_errors +class ReviewerNotificationForm( + core_forms.SettingEmailForm, + core_forms.ConfirmableIfErrorsForm, +): + QUESTION = _( + "The reviewer link in the email body appears to be missing or incorrect. Do you want to send it anyway?" + ) + + def __init__(self, *args, **kwargs): + self.expected_url = kwargs.pop("expected_url", None) + super().__init__(*args, **kwargs) + + def check_for_potential_errors(self): + if not self.expected_url: + return [] + body = self.cleaned_data.get("body", "") + if self.expected_url not in body: + return [ + _( + "The email body does not contain the expected reviewer URL " + "(%(url)s), so the reviewer will not be able to complete the " + "request. This can happen if the body was copy-pasted into " + "the text box from elsewhere." + ) + % {"url": self.expected_url} + ] + return [] + + class GeneratedForm(forms.Form): def __init__(self, *args, **kwargs): review_assignment = kwargs.pop("review_assignment", None) diff --git a/src/review/tests.py b/src/review/tests.py index d51ae293b3..ea1954b1f0 100644 --- a/src/review/tests.py +++ b/src/review/tests.py @@ -1,1065 +1,2 @@ -__copyright__ = "Copyright 2017 Birkbeck, University of London" -__author__ = "Martin Paul Eve & Andy Byers" -__license__ = "AGPL v3" -__maintainer__ = "Birkbeck Centre for Technology and Publishing" - -import os - -from django.core.files.uploadedfile import SimpleUploadedFile -from django.test import TestCase, override_settings -from django.urls import reverse -from django.utils import timezone -from django.conf import settings - -from core import models as core_models, files -from journal import models as journal_models -from production import models as production_models -from review import models as review_models, forms, logic -from submission import models as submission_models -from proofing import models as proofing_models -from press import models as press_models -from utils.install import update_xsl_files, update_settings -from utils import setting_handler -from utils.testing import helpers -from utils.shared import clear_cache - - -# Create your tests here. -class ReviewTests(TestCase): - def setUp(self): - self.files = list() - - @override_settings(URL_CONFIG="domain") - def test_index_view_with_no_questions(self): - """ - If no questions exist, an appropriate message should be displayed. - """ - response = self.client.get("/") - self.assertEqual(response.status_code, 200) - - def test_review_form_can_save(self): - review_field_text = "Here is a review of this paper." - first_form = forms.GeneratedForm( - review_assignment=self.review_assignment, - fields_required=False, - data={ - str(self.review_form_element.pk): review_field_text, - }, - ) - first_form.is_valid() - self.review_assignment.save_review_form(first_form, self.review_assignment) - second_form = forms.GeneratedForm( - review_assignment=self.review_assignment, - fields_required=False, - ) - self.assertEqual( - second_form.fields[str(self.review_form_element.pk)].initial, - review_field_text, - ) - - def test_total_review_count(self): - self.assertEqual( - self.article_review_completed.reviewassignment_set.all().count(), - 3, - ) - - def test_completed_reviews_with_decision_count(self): - self.assertEqual( - self.article_review_completed.completed_reviews_with_decision.count(), - 1, - ) - - def test_review_assignment_form_valid(self): - data = { - "visibility": "double-blind", - "form": self.review_form.pk, - "date_due": "2900-01-01", - "reviewer": self.second_reviewer.pk, - } - form = forms.ReviewAssignmentForm( - journal=self.journal_one, - article=self.article_under_review, - editor=self.editor, - reviewers=logic.get_reviewer_candidates( - self.article_under_review, self.editor - ), - data=data, - ) - self.assertTrue(form.is_valid()) - - def test_review_assignment_form_bad_reviewer(self): - data = { - "visibility": "double-blind", - "form": self.review_form.pk, - "date_due": "2900-01-01", - "reviewer": self.regular_user.pk, - } - form = forms.ReviewAssignmentForm( - journal=self.journal_one, - article=self.article_under_review, - editor=self.editor, - reviewers=logic.get_reviewer_candidates( - self.article_under_review, self.editor - ), - data=data, - ) - self.assertFalse(form.is_valid()) - - def test_reviewer_decision_form_no_decision(self): - article = helpers.create_article( - self.journal_one, - **{ - "owner": self.regular_user, - "title": "Test Article", - "stage": submission_models.STAGE_UNDER_REVIEW, - }, - ) - round, c = review_models.ReviewRound.objects.get_or_create( - article=article, - round_number=1, - ) - - data = { - "reviewer": str(self.regular_user.id), - "form": self.review_form.pk, - "visibility": "double-blind", - "date_due": "2024-03-13", - } - form = forms.ReviewAssignmentForm( - data, - journal=self.journal_one, - article=article, - editor=self.editor, - reviewers=[], - ) - assignment = form.save() - form = forms.ReviewerDecisionForm( - instance=assignment, - decision_required=True, - ) - - self.assertIn('', form.as_p()) - self.assertFalse(form.is_valid()) - - def test_csv_reviewer_import_good(self): - # create a fake request object - request = self.setup_request_object() - - # prep csv - csv_content = "{}\n{}".format( - self.reviewer_csv_header_row, - self.good_reviewer_content_line, - ) - self.test_csv = SimpleUploadedFile( - "test_reviewer_file.csv", bytes(csv_content.encode("utf-8")) - ) - filename, path = files.save_file_to_temp(self.test_csv) - - # prep form with data - details_form = forms.BulkReviewAssignmentForm( - { - "visibility": "double-blind", - "form": self.review_form, - "date_due": "2023-01-01", - } - ) - details_form.is_valid() - - reviewers, error = logic.process_reviewer_csv( - path, - request, - self.article_under_review, - details_form, - ) - self.assertTrue( - isinstance( - reviewers[0].get("review_assignment"), - review_models.ReviewAssignment, - ) - ) - self.assertFalse(error) - os.unlink(path) - - def test_csv_reviewer_import_bad(self): - # create a fake request object - request = self.setup_request_object() - self.test_csv = SimpleUploadedFile( - "test_reviewer_file.csv", bytes(self.empty_reviewer_content_line) - ) - filename, path = files.save_file_to_temp(self.test_csv) - - # prep form with data - details_form = forms.BulkReviewAssignmentForm( - { - "visibility": "double-blind", - "form": self.review_form, - "date_due": "2023-01-01", - } - ) - details_form.is_valid() - reviewers, error = logic.process_reviewer_csv( - path, - request, - self.article_under_review, - details_form, - ) - self.assertFalse(reviewers) - os.unlink(path) - - def test_csv_review_import_uses_existing_user_account(self): - request = self.setup_request_object() - - # prep csv - csv_content = "{}\n{}".format( - self.reviewer_csv_header_row, - self.regular_user_csv_line, - ) - self.test_csv = SimpleUploadedFile( - "test_reviewer_file.csv", bytes(csv_content.encode("utf-8")) - ) - filename, path = files.save_file_to_temp(self.test_csv) - - # prep form with data - details_form = forms.BulkReviewAssignmentForm( - { - "visibility": "double-blind", - "form": self.review_form, - "date_due": "2023-01-01", - } - ) - details_form.is_valid() - reviewers, error = logic.process_reviewer_csv( - path, - request, - self.article_under_review, - details_form, - ) - self.assertTrue( - reviewers[0].get("account"), - self.regular_user, - ) - - def test_csv_doesnt_create_duplicate_assignments(self): - request = self.setup_request_object() - - # prep csv - csv_content = "{}\n{}".format( - self.reviewer_csv_header_row, - self.regular_user_csv_line, - ) - self.test_csv = SimpleUploadedFile( - "test_reviewer_file.csv", bytes(csv_content.encode("utf-8")) - ) - filename, path = files.save_file_to_temp(self.test_csv) - - # prep form with data - details_form = forms.BulkReviewAssignmentForm( - { - "visibility": "double-blind", - "form": self.review_form, - "date_due": "2023-01-01", - } - ) - details_form.is_valid() - reviewers, error = logic.process_reviewer_csv( - path, - request, - self.article_review_completed, - details_form, - ) - self.assertTrue( - reviewers[0].get("review_assignment"), - self.review_assignment_complete, - ) - - def test_incomplete_reviews(self): - args = { - "owner": self.regular_user, - "title": "Test Article", - "stage": submission_models.STAGE_UNDER_REVIEW, - } - article1 = helpers.create_article(self.journal_one, **args) - - article1.correspondence_author = self.regular_user - article1.save() - - round = review_models.ReviewRound.objects.create( - article=article1, - round_number=1, - ) - - assignment = helpers.create_review_assignment( - journal=self.journal_one, - article=article1, - is_complete=False, - review_round=round, - reviewer=self.regular_user, - ) - assignment.decision = None - assignment.save() - - self.client.force_login(self.editor) - decline_url = reverse( - "review_decision", kwargs={"article_id": article1.pk, "decision": "decline"} - ) - response = self.client.get( - decline_url, - SERVER_NAME=self.journal_one.domain, - ) - msg = "The following incomplete reviews will be marked as withdrawn" - self.assertContains(response, msg) - - data = { - "cc": [""], - "bcc": [""], - "subject": ["Article Declined"], - "body": ["Article Declined"], - "attachments": [""], - "skip": ["skip"], - } - response = self.client.post( - decline_url, - data, - SERVER_NAME=self.journal_one.domain, - ) - review_set = article1.reviewassignment_set.all() - self.assertEqual(review_set.filter(is_complete=True).count(), 1) - self.assertEqual(review_set.filter(is_complete=False).count(), 0) - - def test_completed_reviews_shared_setting(self): - # setup data - article_with_completed_reviews = helpers.create_article( - self.journal_one, - **{ - "owner": self.regular_user, - "title": "Test Article", - "stage": submission_models.STAGE_UNDER_REVIEW, - }, - ) - round_one, c = review_models.ReviewRound.objects.get_or_create( - article=article_with_completed_reviews, - round_number=1, - ) - round_two, c = review_models.ReviewRound.objects.get_or_create( - article=article_with_completed_reviews, - round_number=2, - ) - round_one_completed_review = helpers.create_review_assignment( - journal=self.journal_one, - article=article_with_completed_reviews, - is_complete=True, - review_round=round_one, - ) - round_two_active_review = helpers.create_review_assignment( - journal=self.journal_one, - article=article_with_completed_reviews, - is_complete=False, - review_round=round_two, - reviewer=self.regular_user, - ) - - # turn setting on - setting_handler.save_setting( - setting_group_name="general", - setting_name="display_completed_reviews_in_additional_rounds", - journal=self.journal_one, - value="On", - ) - clear_cache() - - # test on - self.client.force_login(self.second_user) - reversed_url = reverse( - "do_review", - kwargs={ - "assignment_id": round_two_active_review.pk, - }, - ) - url_with_access_code = ( - f"{reversed_url}?access_code={round_two_active_review.access_code}" - ) - response = self.client.get( - url_with_access_code, - SERVER_NAME=self.journal_one.domain, - ) - self.assertContains(response, f"View Review #{round_one_completed_review.pk}") - - # turn setting off - setting_handler.save_setting( - setting_group_name="general", - setting_name="display_completed_reviews_in_additional_rounds", - journal=self.journal_one, - value="", - ) - clear_cache() - - # test off - self.client.force_login(self.regular_user) - response = self.client.get( - url_with_access_code, - SERVER_NAME=self.journal_one.domain, - ) - self.assertNotContains( - response, "Please click 'View Review' below to view the peer review report" - ) - - def test_shared_review_download_view(self): - """ - Tests route 1 of the reviewer_shared_review_download view allows and - denies access as required. - """ - article_with_completed_reviews = helpers.create_article( - self.journal_one, - **{ - "owner": self.regular_user, - "title": "Test Article", - "stage": submission_models.STAGE_UNDER_REVIEW, - }, - ) - round_one, c = review_models.ReviewRound.objects.get_or_create( - article=article_with_completed_reviews, - round_number=1, - ) - round_two, c = review_models.ReviewRound.objects.get_or_create( - article=article_with_completed_reviews, - round_number=2, - ) - request = self.setup_request_object() - test_file = SimpleUploadedFile( - "file.txt", - b"content", - ) - file = files.save_file( - request, - test_file, - label="Test", - public=True, - path_parts=("articles", article_with_completed_reviews.pk), - ) - round_one_completed_review = helpers.create_review_assignment( - journal=self.journal_one, - article=article_with_completed_reviews, - is_complete=True, - review_round=round_one, - reviewer=self.second_reviewer, - review_file=file, - ) - round_two_active_review = helpers.create_review_assignment( - journal=self.journal_one, - article=article_with_completed_reviews, - is_complete=False, - review_round=round_two, - reviewer=self.second_user, - ) - - # turn setting on - setting_handler.save_setting( - setting_group_name="general", - setting_name="display_completed_reviews_in_additional_rounds", - journal=self.journal_one, - value="On", - ) - clear_cache() - download_url = reverse( - "reviewer_shared_review_download", - kwargs={ - "article_id": article_with_completed_reviews.pk, - "review_id": round_one_completed_review.pk, - }, - ) - - # in this instance, second_user should have access to download the - self.client.force_login( - self.second_user, - ) - response = self.client.get( - download_url, - SERVER_NAME=self.journal_one.domain, - ) - self.assertEqual( - response.status_code, - 200, - ) - - # in this instance second_reviewer should not have any access. - self.client.force_login( - self.second_reviewer, - ) - response = self.client.get( - download_url, - SERVER_NAME=self.journal_one.domain, - ) - self.assertEquals( - response.status_code, - 404, - ) - - # if we now mark this article as shared second_reviewer should - # be able to access the download - article_with_completed_reviews.reviews_shared = True - article_with_completed_reviews.save() - - self.client.force_login( - self.second_reviewer, - ) - response = self.client.get( - download_url, - SERVER_NAME=self.journal_one.domain, - ) - self.assertEquals( - response.status_code, - 200, - ) - - # test that a regular user cant access the download link - self.client.force_login( - self.regular_user, - ) - response = self.client.get( - download_url, - SERVER_NAME=self.journal_one.domain, - ) - self.assertEquals( - response.status_code, - 403, - ) - - # finally, delete the file from disk - files.delete_file(article_with_completed_reviews, file) - - def setup_request_object(self): - request = helpers.Request() - request.user = self.editor - request.journal = self.journal_one - request.press = self.journal_one.press - request.site_type = self.journal_one - return request - - @staticmethod - def create_user(username, roles=None, journal=None): - """ - Creates a user with the specified permissions. - :return: a user with the specified permissions - """ - # For consistency, outsourced to newer testing helpers - return helpers.create_user( - username, - roles=roles, - journal=journal, - ) - - @staticmethod - def create_roles(roles=None): - """ - Creates the necessary roles for testing. - :return: None - """ - # For consistency, outsourced to newer testing helpers - helpers.create_roles(roles=roles) - - @staticmethod - def create_journals(): - """ - Creates a set of dummy journals for testing - :return: a 2-tuple of two journals - """ - update_settings() - update_xsl_files() - journal_one = journal_models.Journal(code="TST", domain="testserver") - journal_one.save() - - journal_two = journal_models.Journal(code="TSA", domain="journal2.localhost") - journal_two.save() - - return journal_one, journal_two - - def setUp(self): - """ - Setup the test environment. - :return: None - """ - self.journal_one, self.journal_two = self.create_journals() - self.create_roles( - [ - "editor", - "author", - "reviewer", - "proofreader", - "production", - "copyeditor", - "typesetter", - "proofing-manager", - "section-editor", - ] - ) - - self.regular_user = self.create_user("regularuser@martineve.com") - self.regular_user.is_active = True - self.regular_user.save() - - self.second_user = self.create_user( - "seconduser@martineve.com", ["reviewer"], journal=self.journal_one - ) - self.second_user.is_active = True - self.second_user.save() - - self.admin_user = self.create_user("adminuser@martineve.com") - self.admin_user.is_staff = True - self.admin_user.is_active = True - self.admin_user.save() - - self.inactive_user = self.create_user( - "disableduser@martineve.com", - ["editor", "author", "proofreader", "production"], - journal=self.journal_one, - ) - self.inactive_user.is_active = False - self.inactive_user.save() - - self.editor = self.create_user( - "editoruser@martineve.com", ["editor"], journal=self.journal_one - ) - self.editor.is_active = True - self.editor.save() - - self.author = self.create_user( - "authoruser@martineve.com", ["author"], journal=self.journal_one - ) - self.author.is_active = True - self.author.save() - - self.proofreader = self.create_user( - "proofreader@martineve.com", ["proofreader"], journal=self.journal_one - ) - self.proofreader.is_active = True - self.proofreader.save() - - self.proofreader_two = self.create_user( - "proofreader2@martineve.com", ["proofreader"], journal=self.journal_one - ) - self.proofreader_two.is_active = True - self.proofreader_two.save() - - self.production = self.create_user( - "production@martineve.com", ["production"], journal=self.journal_one - ) - self.production.is_active = True - self.production.save() - - self.copyeditor = self.create_user( - "copyeditor@martineve.com", ["copyeditor"], journal=self.journal_one - ) - self.copyeditor.is_active = True - self.copyeditor.save() - - self.typesetter = self.create_user( - "typesetter@martineve.com", ["typesetter"], journal=self.journal_one - ) - self.typesetter.is_active = True - self.typesetter.save() - - self.other_typesetter = self.create_user( - "other_typesetter@martineve.com", ["typesetter"], journal=self.journal_one - ) - self.other_typesetter.is_active = True - self.other_typesetter.save() - - self.proofing_manager = self.create_user( - "proofing_manager@martineve.com", - ["proofing-manager"], - journal=self.journal_one, - ) - self.proofing_manager.is_active = True - self.proofing_manager.save() - - self.other_typesetter.is_active = True - self.other_typesetter.save() - - self.section_editor = self.create_user( - "section_editor@martineve.com", ["section-editor"], journal=self.journal_one - ) - self.section_editor.is_active = True - self.section_editor.save() - - self.second_reviewer = self.create_user( - "second_reviewer@martineve.com", ["reviewer"], journal=self.journal_one - ) - self.second_reviewer.is_active = True - self.second_reviewer.save() - - self.public_file = core_models.File( - mime_type="A/FILE", - original_filename="blah.txt", - uuid_filename="UUID.txt", - label="A file that is public", - description="Oh yes, it's a file", - owner=self.regular_user, - is_galley=False, - privacy="public", - ) - - self.public_file.save() - - self.private_file = core_models.File( - mime_type="A/FILE", - original_filename="blah.txt", - uuid_filename="UUID.txt", - label="A file that is private", - description="Oh yes, it's a file", - owner=self.regular_user, - is_galley=False, - privacy="owner", - ) - - self.private_file.save() - - self.article_in_production = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_TYPESETTING, - journal_id=self.journal_one.id, - ) - self.article_in_production.save() - self.article_in_production.data_figure_files.add(self.public_file) - - self.article_unsubmitted = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_UNSUBMITTED, - journal_id=self.journal_one.id, - ) - self.article_unsubmitted.save() - - self.article_unassigned = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_UNASSIGNED, - journal_id=self.journal_one.id, - ) - self.article_unassigned.save() - - self.article_assigned = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_ASSIGNED, - journal_id=self.journal_one.id, - ) - self.article_assigned.save() - - self.article_under_review = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_UNDER_REVIEW, - journal_id=self.journal_one.id, - ) - self.article_under_review.save() - - self.article_review_completed = submission_models.Article.objects.create( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_ACCEPTED, - journal_id=self.journal_one.id, - date_accepted=timezone.now(), - ) - - self.article_author_is_owner = submission_models.Article.objects.create( - owner=self.author, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_ACCEPTED, - journal_id=self.journal_one.id, - date_accepted=timezone.now(), - ) - - self.editor.snapshot_as_author(self.article_author_is_owner) - self.author.snapshot_as_author(self.article_author_is_owner) - - self.review_form = review_models.ReviewForm( - name="A Form", intro="i", thanks="t", journal=self.journal_one - ) - self.review_form.save() - - self.review_form_element, c = ( - review_models.ReviewFormElement.objects.get_or_create( - name="Review", - kind="text", - order=1, - required=True, - ) - ) - self.review_form.elements.add(self.review_form_element) - self.second_review_form_element, c = ( - review_models.ReviewFormElement.objects.get_or_create( - name="Second Review Form Element", - kind="text", - order=2, - required=True, - ) - ) - self.review_form.elements.add(self.second_review_form_element) - setting_handler.save_setting( - "general", - "enable_save_review_progress", - self.journal_one, - "On", - ) - - self.round_one = review_models.ReviewRound.objects.create( - article=self.article_review_completed, - round_number=1, - ) - self.round_two = review_models.ReviewRound.objects.create( - article=self.article_review_completed, - round_number=2, - ) - - self.review_assignment_complete = review_models.ReviewAssignment( - article=self.article_review_completed, - review_round=self.round_one, - reviewer=self.regular_user, - editor=self.editor, - date_due=timezone.now(), - form=self.review_form, - is_complete=True, - date_complete=timezone.now(), - decision="accept", - ) - - self.review_assignment_complete.save() - - self.review_assignment_withdrawn = ( - review_models.ReviewAssignment.objects.create( - article=self.article_review_completed, - review_round=self.round_two, - reviewer=self.second_reviewer, - editor=self.editor, - date_due=timezone.now(), - form=self.review_form, - is_complete=True, - decision="withdrawn", - ) - ) - - self.review_assignment_declined, created = ( - review_models.ReviewAssignment.objects.get_or_create( - article=self.article_review_completed, - review_round=self.round_two, - reviewer=self.second_reviewer, - editor=self.editor, - date_due=timezone.now(), - date_declined=timezone.now(), - form=self.review_form, - is_complete=False, - ) - ) - - self.review_assignment = review_models.ReviewAssignment( - article=self.article_under_review, - reviewer=self.second_user, - editor=self.editor, - date_due=timezone.now(), - form=self.review_form, - ) - - self.review_assignment.save() - - self.review_assignment_not_in_scope = review_models.ReviewAssignment( - article=self.article_in_production, - reviewer=self.regular_user, - editor=self.editor, - date_due=timezone.now(), - form=self.review_form, - ) - self.review_assignment_not_in_scope.save() - - self.article_under_revision = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_UNDER_REVISION, - journal_id=self.journal_one.id, - ) - self.article_under_revision.save() - - self.article_rejected = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_REJECTED, - journal_id=self.journal_one.id, - ) - self.article_rejected.save() - - self.article_accepted = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_ACCEPTED, - journal_id=self.journal_one.id, - ) - self.article_accepted.save() - - self.section_editor_assignment = review_models.EditorAssignment( - article=self.article_assigned, - editor=self.section_editor, - editor_type="section-editor", - notified=True, - ) - self.section_editor_assignment.save() - - self.article_editor_copyediting = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_EDITOR_COPYEDITING, - journal_id=self.journal_one.id, - ) - self.article_editor_copyediting.save() - - self.article_author_copyediting = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_AUTHOR_COPYEDITING, - journal_id=self.journal_one.id, - ) - self.article_author_copyediting.save() - - self.article_final_copyediting = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_FINAL_COPYEDITING, - journal_id=self.journal_one.id, - ) - self.article_final_copyediting.save() - - self.article_proofing = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_PROOFING, - journal_id=self.journal_one.id, - ) - self.article_proofing.save() - - assigned = production_models.ProductionAssignment( - article=self.article_in_production, production_manager=self.production - ) - assigned.save() - - self.article_published = submission_models.Article( - owner=self.regular_user, - title="A Second Test Article", - abstract="An abstract", - stage=submission_models.STAGE_PUBLISHED, - journal_id=self.journal_one.id, - ) - self.article_published.save() - - assigned = production_models.ProductionAssignment( - article=self.article_published, production_manager=self.production - ) - assigned.save() - - self.article_in_production_inactive = submission_models.Article( - owner=self.regular_user, - title="A Test Article", - abstract="An abstract", - stage=submission_models.STAGE_TYPESETTING, - journal_id=self.journal_one.id, - ) - self.article_in_production_inactive.save() - - self.assigned = production_models.ProductionAssignment( - article=self.article_in_production_inactive, - production_manager=self.inactive_user, - ) - self.assigned.save() - - self.typeset_task = production_models.TypesetTask( - assignment=self.assigned, - typesetter=self.typesetter, - notified=True, - accepted=timezone.now(), - ) - self.typeset_task.save() - - self.other_typeset_task = production_models.TypesetTask( - assignment=self.assigned, - typesetter=self.other_typesetter, - notified=True, - accepted=timezone.now(), - ) - self.other_typeset_task.save() - - self.proofing_assignment = proofing_models.ProofingAssignment( - article=self.article_proofing, - proofing_manager=self.proofing_manager, - notified=True, - ) - self.proofing_assignment.save() - self.proofing_assignment.add_new_proofing_round() - - self.proofing_task = proofing_models.ProofingTask( - round=self.proofing_assignment.current_proofing_round(), - proofreader=self.proofreader, - notified=True, - due=timezone.now(), - accepted=timezone.now(), - task="sdfsdffs", - ) - self.proofing_task.save() - - self.correction_task = proofing_models.TypesetterProofingTask( - proofing_task=self.proofing_task, - typesetter=self.typesetter, - notified=True, - due=timezone.now(), - accepted=timezone.now(), - task="fsddsff", - ) - self.correction_task.save() - - self.journal_one.name = "Journal One" - self.journal_two.name = "Journal Two" - self.press = helpers.create_press() - self.press.save() - update_settings( - self.journal_one, - management_command=False, - ) - self.reviewer_csv_header_row = b"salutation,firstname,middlename,lastname,email_address,department,institution,country,interests,reason" - self.good_reviewer_content_line = b"Mr,Andy,James,Byers,andy@janeway.systems,Open Library of Humanities,Birkbeck,GB,,Test Reason" - self.empty_reviewer_content_line = b" " - self.regular_user_csv_line = b"Mr,Regular,,User,regularuser@martineve.com,Somewhere Dept,Some Inst,GB,,A Reason" - - def test_request_revisions_context(self): - self.client.force_login(self.editor) - response = self.client.get( - reverse( - "review_request_revisions", - kwargs={"article_id": self.article_review_completed.pk}, - ), - SERVER_NAME=self.journal_one.domain, - ) - response.context.get("incomplete") - self.assertEqual( - self.article_review_completed, - response.context.get("article"), - ) - # This test does not cover the revision request form - self.assertEqual( - 0, - response.context.get("pending_approval").count(), - ) - self.assertEqual( - 0, - response.context.get("incomplete").count(), - ) +# Tests have moved to the tests/ package. +# See review/tests/test_views.py and review/tests/test_forms.py. diff --git a/src/review/tests/__init__.py b/src/review/tests/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/review/tests/test_forms.py b/src/review/tests/test_forms.py new file mode 100644 index 0000000000..203b29b726 --- /dev/null +++ b/src/review/tests/test_forms.py @@ -0,0 +1,124 @@ +from django.test import TestCase + +from review.forms import ReviewerNotificationForm +from utils.install import update_settings, update_xsl_files +from utils.testing import helpers +from journal import models as journal_models + + +class ReviewerNotificationFormTests(TestCase): + @classmethod + def setUpTestData(cls): + helpers.create_press() + update_settings() + update_xsl_files() + cls.journal = journal_models.Journal(code="EANF", domain="eanf.testserver") + cls.journal.save() + update_settings(cls.journal, management_command=False) + cls.editor = helpers.create_user( + "eanf_editor@example.com", roles=["editor"], journal=cls.journal + ) + cls.editor.is_active = True + cls.editor.save() + + def _make_request(self): + return helpers.get_request( + user=self.editor, + journal=self.journal, + press=self.journal.press, + ) + + def _form(self, body, expected_url=None, button="confirmable"): + request = self._make_request() + data = { + "cc": "", + "bcc": "", + "subject": "Test", + "body": body, + button: button, + } + return ReviewerNotificationForm( + data, + setting_name="review_assignment", + email_context={ + "article": None, + "editor": self.editor, + "review_assignment": None, + "review_url": "http://example.com/review/requests/1/?access_code=abc", + "article_details": "", + }, + expected_url=expected_url, + request=request, + ) + + def test_no_expected_url_no_potential_errors(self): + form = self._form(body="No URL here at all.", expected_url=None) + form.is_valid() + self.assertEqual(form.check_for_potential_errors(), []) + + def test_url_present_in_body_no_potential_errors(self): + expected_url = "http://example.com/review/requests/1/?access_code=abc" + form = self._form( + body="Please visit: {}".format(expected_url), + expected_url=expected_url, + ) + form.is_valid() + self.assertEqual(form.check_for_potential_errors(), []) + + def test_url_missing_from_body_returns_error(self): + expected_url = "http://example.com/review/requests/1/?access_code=abc" + form = self._form(body="No link here.", expected_url=expected_url) + form.is_valid() + errors = form.check_for_potential_errors() + self.assertEqual(len(errors), 1) + self.assertIn(expected_url, errors[0]) + + def test_url_missing_creates_modal_on_confirmable_submit(self): + expected_url = "http://example.com/review/requests/1/?access_code=abc" + form = self._form( + body="No link here.", + expected_url=expected_url, + button="confirmable", + ) + form.is_valid() + self.assertIsNotNone(form.modal) + + def test_url_present_no_modal_on_confirmable_submit(self): + expected_url = "http://example.com/review/requests/1/?access_code=abc" + form = self._form( + body="Please visit: {}".format(expected_url), + expected_url=expected_url, + button="confirmable", + ) + form.is_valid() + self.assertIsNone(form.modal) + + def test_is_confirmed_true_when_url_present(self): + expected_url = "http://example.com/review/requests/1/?access_code=abc" + form = self._form( + body="Please visit: {}".format(expected_url), + expected_url=expected_url, + button="confirmable", + ) + form.is_valid() + self.assertTrue(form.is_confirmed()) + + def test_is_confirmed_false_when_url_missing_and_not_confirmed(self): + expected_url = "http://example.com/review/requests/1/?access_code=abc" + form = self._form( + body="No link here.", + expected_url=expected_url, + button="confirmable", + ) + form.is_valid() + self.assertFalse(form.is_confirmed()) + + def test_is_confirmed_true_when_confirmed_button_used(self): + expected_url = "http://example.com/review/requests/1/?access_code=abc" + form = self._form( + body="No link here.", + expected_url=expected_url, + button="confirmed", + ) + form.is_valid() + self.assertTrue(form.is_confirmed()) diff --git a/src/review/tests/test_views.py b/src/review/tests/test_views.py new file mode 100644 index 0000000000..6d47cf986a --- /dev/null +++ b/src/review/tests/test_views.py @@ -0,0 +1,1253 @@ +import os + +from django.core import mail +from django.core.files.uploadedfile import SimpleUploadedFile +from django.test import TestCase, override_settings +from django.urls import reverse +from django.utils import timezone +from django.conf import settings + +from core import models as core_models, files +from journal import models as journal_models +from production import models as production_models +from review import models as review_models, forms, logic +from submission import models as submission_models +from proofing import models as proofing_models +from press import models as press_models +from utils.install import update_xsl_files, update_settings +from utils import setting_handler +from utils.testing import helpers +from utils.testing.context_managers import janeway_setting_override +from utils.shared import clear_cache + + +# Create your tests here. +class ReviewTests(TestCase): + def setUp(self): + self.files = list() + + @override_settings(URL_CONFIG="domain") + def test_index_view_with_no_questions(self): + """ + If no questions exist, an appropriate message should be displayed. + """ + response = self.client.get("/") + self.assertEqual(response.status_code, 200) + + def test_review_form_can_save(self): + review_field_text = "Here is a review of this paper." + first_form = forms.GeneratedForm( + review_assignment=self.review_assignment, + fields_required=False, + data={ + str(self.review_form_element.pk): review_field_text, + }, + ) + first_form.is_valid() + self.review_assignment.save_review_form(first_form, self.review_assignment) + second_form = forms.GeneratedForm( + review_assignment=self.review_assignment, + fields_required=False, + ) + self.assertEqual( + second_form.fields[str(self.review_form_element.pk)].initial, + review_field_text, + ) + + def test_total_review_count(self): + self.assertEqual( + self.article_review_completed.reviewassignment_set.all().count(), + 3, + ) + + def test_completed_reviews_with_decision_count(self): + self.assertEqual( + self.article_review_completed.completed_reviews_with_decision.count(), + 1, + ) + + def test_review_assignment_form_valid(self): + data = { + "visibility": "double-blind", + "form": self.review_form.pk, + "date_due": "2900-01-01", + "reviewer": self.second_reviewer.pk, + } + form = forms.ReviewAssignmentForm( + journal=self.journal_one, + article=self.article_under_review, + editor=self.editor, + reviewers=logic.get_reviewer_candidates( + self.article_under_review, self.editor + ), + data=data, + ) + self.assertTrue(form.is_valid()) + + def test_review_assignment_form_bad_reviewer(self): + data = { + "visibility": "double-blind", + "form": self.review_form.pk, + "date_due": "2900-01-01", + "reviewer": self.regular_user.pk, + } + form = forms.ReviewAssignmentForm( + journal=self.journal_one, + article=self.article_under_review, + editor=self.editor, + reviewers=logic.get_reviewer_candidates( + self.article_under_review, self.editor + ), + data=data, + ) + self.assertFalse(form.is_valid()) + + def test_reviewer_decision_form_no_decision(self): + article = helpers.create_article( + self.journal_one, + **{ + "owner": self.regular_user, + "title": "Test Article", + "stage": submission_models.STAGE_UNDER_REVIEW, + }, + ) + round, c = review_models.ReviewRound.objects.get_or_create( + article=article, + round_number=1, + ) + + data = { + "reviewer": str(self.regular_user.id), + "form": self.review_form.pk, + "visibility": "double-blind", + "date_due": "2024-03-13", + } + form = forms.ReviewAssignmentForm( + data, + journal=self.journal_one, + article=article, + editor=self.editor, + reviewers=[], + ) + assignment = form.save() + form = forms.ReviewerDecisionForm( + instance=assignment, + decision_required=True, + ) + + self.assertIn('', form.as_p()) + self.assertFalse(form.is_valid()) + + def test_csv_reviewer_import_good(self): + # create a fake request object + request = self.setup_request_object() + + # prep csv + csv_content = "{}\n{}".format( + self.reviewer_csv_header_row, + self.good_reviewer_content_line, + ) + self.test_csv = SimpleUploadedFile( + "test_reviewer_file.csv", bytes(csv_content.encode("utf-8")) + ) + filename, path = files.save_file_to_temp(self.test_csv) + + # prep form with data + details_form = forms.BulkReviewAssignmentForm( + { + "visibility": "double-blind", + "form": self.review_form, + "date_due": "2023-01-01", + } + ) + details_form.is_valid() + + reviewers, error = logic.process_reviewer_csv( + path, + request, + self.article_under_review, + details_form, + ) + self.assertTrue( + isinstance( + reviewers[0].get("review_assignment"), + review_models.ReviewAssignment, + ) + ) + self.assertFalse(error) + os.unlink(path) + + def test_csv_reviewer_import_bad(self): + # create a fake request object + request = self.setup_request_object() + self.test_csv = SimpleUploadedFile( + "test_reviewer_file.csv", bytes(self.empty_reviewer_content_line) + ) + filename, path = files.save_file_to_temp(self.test_csv) + + # prep form with data + details_form = forms.BulkReviewAssignmentForm( + { + "visibility": "double-blind", + "form": self.review_form, + "date_due": "2023-01-01", + } + ) + details_form.is_valid() + reviewers, error = logic.process_reviewer_csv( + path, + request, + self.article_under_review, + details_form, + ) + self.assertFalse(reviewers) + os.unlink(path) + + def test_csv_review_import_uses_existing_user_account(self): + request = self.setup_request_object() + + # prep csv + csv_content = "{}\n{}".format( + self.reviewer_csv_header_row, + self.regular_user_csv_line, + ) + self.test_csv = SimpleUploadedFile( + "test_reviewer_file.csv", bytes(csv_content.encode("utf-8")) + ) + filename, path = files.save_file_to_temp(self.test_csv) + + # prep form with data + details_form = forms.BulkReviewAssignmentForm( + { + "visibility": "double-blind", + "form": self.review_form, + "date_due": "2023-01-01", + } + ) + details_form.is_valid() + reviewers, error = logic.process_reviewer_csv( + path, + request, + self.article_under_review, + details_form, + ) + self.assertTrue( + reviewers[0].get("account"), + self.regular_user, + ) + + def test_csv_doesnt_create_duplicate_assignments(self): + request = self.setup_request_object() + + # prep csv + csv_content = "{}\n{}".format( + self.reviewer_csv_header_row, + self.regular_user_csv_line, + ) + self.test_csv = SimpleUploadedFile( + "test_reviewer_file.csv", bytes(csv_content.encode("utf-8")) + ) + filename, path = files.save_file_to_temp(self.test_csv) + + # prep form with data + details_form = forms.BulkReviewAssignmentForm( + { + "visibility": "double-blind", + "form": self.review_form, + "date_due": "2023-01-01", + } + ) + details_form.is_valid() + reviewers, error = logic.process_reviewer_csv( + path, + request, + self.article_review_completed, + details_form, + ) + self.assertTrue( + reviewers[0].get("review_assignment"), + self.review_assignment_complete, + ) + + def test_incomplete_reviews(self): + args = { + "owner": self.regular_user, + "title": "Test Article", + "stage": submission_models.STAGE_UNDER_REVIEW, + } + article1 = helpers.create_article(self.journal_one, **args) + + article1.correspondence_author = self.regular_user + article1.save() + + round = review_models.ReviewRound.objects.create( + article=article1, + round_number=1, + ) + + assignment = helpers.create_review_assignment( + journal=self.journal_one, + article=article1, + is_complete=False, + review_round=round, + reviewer=self.regular_user, + ) + assignment.decision = None + assignment.save() + + self.client.force_login(self.editor) + decline_url = reverse( + "review_decision", kwargs={"article_id": article1.pk, "decision": "decline"} + ) + response = self.client.get( + decline_url, + SERVER_NAME=self.journal_one.domain, + ) + msg = "The following incomplete reviews will be marked as withdrawn" + self.assertContains(response, msg) + + data = { + "cc": [""], + "bcc": [""], + "subject": ["Article Declined"], + "body": ["Article Declined"], + "attachments": [""], + "skip": ["skip"], + } + response = self.client.post( + decline_url, + data, + SERVER_NAME=self.journal_one.domain, + ) + review_set = article1.reviewassignment_set.all() + self.assertEqual(review_set.filter(is_complete=True).count(), 1) + self.assertEqual(review_set.filter(is_complete=False).count(), 0) + + def test_completed_reviews_shared_setting(self): + # setup data + article_with_completed_reviews = helpers.create_article( + self.journal_one, + **{ + "owner": self.regular_user, + "title": "Test Article", + "stage": submission_models.STAGE_UNDER_REVIEW, + }, + ) + round_one, c = review_models.ReviewRound.objects.get_or_create( + article=article_with_completed_reviews, + round_number=1, + ) + round_two, c = review_models.ReviewRound.objects.get_or_create( + article=article_with_completed_reviews, + round_number=2, + ) + round_one_completed_review = helpers.create_review_assignment( + journal=self.journal_one, + article=article_with_completed_reviews, + is_complete=True, + review_round=round_one, + ) + round_two_active_review = helpers.create_review_assignment( + journal=self.journal_one, + article=article_with_completed_reviews, + is_complete=False, + review_round=round_two, + reviewer=self.regular_user, + ) + + # turn setting on + setting_handler.save_setting( + setting_group_name="general", + setting_name="display_completed_reviews_in_additional_rounds", + journal=self.journal_one, + value="On", + ) + clear_cache() + + # test on + self.client.force_login(self.second_user) + reversed_url = reverse( + "do_review", + kwargs={ + "assignment_id": round_two_active_review.pk, + }, + ) + url_with_access_code = ( + f"{reversed_url}?access_code={round_two_active_review.access_code}" + ) + response = self.client.get( + url_with_access_code, + SERVER_NAME=self.journal_one.domain, + ) + self.assertContains(response, f"View Review #{round_one_completed_review.pk}") + + # turn setting off + setting_handler.save_setting( + setting_group_name="general", + setting_name="display_completed_reviews_in_additional_rounds", + journal=self.journal_one, + value="", + ) + clear_cache() + + # test off + self.client.force_login(self.regular_user) + response = self.client.get( + url_with_access_code, + SERVER_NAME=self.journal_one.domain, + ) + self.assertNotContains( + response, "Please click 'View Review' below to view the peer review report" + ) + + def test_shared_review_download_view(self): + """ + Tests route 1 of the reviewer_shared_review_download view allows and + denies access as required. + """ + article_with_completed_reviews = helpers.create_article( + self.journal_one, + **{ + "owner": self.regular_user, + "title": "Test Article", + "stage": submission_models.STAGE_UNDER_REVIEW, + }, + ) + round_one, c = review_models.ReviewRound.objects.get_or_create( + article=article_with_completed_reviews, + round_number=1, + ) + round_two, c = review_models.ReviewRound.objects.get_or_create( + article=article_with_completed_reviews, + round_number=2, + ) + request = self.setup_request_object() + test_file = SimpleUploadedFile( + "file.txt", + b"content", + ) + file = files.save_file( + request, + test_file, + label="Test", + public=True, + path_parts=("articles", article_with_completed_reviews.pk), + ) + round_one_completed_review = helpers.create_review_assignment( + journal=self.journal_one, + article=article_with_completed_reviews, + is_complete=True, + review_round=round_one, + reviewer=self.second_reviewer, + review_file=file, + ) + round_two_active_review = helpers.create_review_assignment( + journal=self.journal_one, + article=article_with_completed_reviews, + is_complete=False, + review_round=round_two, + reviewer=self.second_user, + ) + + # turn setting on + setting_handler.save_setting( + setting_group_name="general", + setting_name="display_completed_reviews_in_additional_rounds", + journal=self.journal_one, + value="On", + ) + clear_cache() + download_url = reverse( + "reviewer_shared_review_download", + kwargs={ + "article_id": article_with_completed_reviews.pk, + "review_id": round_one_completed_review.pk, + }, + ) + + # in this instance, second_user should have access to download the + self.client.force_login( + self.second_user, + ) + response = self.client.get( + download_url, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual( + response.status_code, + 200, + ) + + # in this instance second_reviewer should not have any access. + self.client.force_login( + self.second_reviewer, + ) + response = self.client.get( + download_url, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEquals( + response.status_code, + 404, + ) + + # if we now mark this article as shared second_reviewer should + # be able to access the download + article_with_completed_reviews.reviews_shared = True + article_with_completed_reviews.save() + + self.client.force_login( + self.second_reviewer, + ) + response = self.client.get( + download_url, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEquals( + response.status_code, + 200, + ) + + # test that a regular user cant access the download link + self.client.force_login( + self.regular_user, + ) + response = self.client.get( + download_url, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEquals( + response.status_code, + 403, + ) + + # finally, delete the file from disk + files.delete_file(article_with_completed_reviews, file) + + def setup_request_object(self): + request = helpers.Request() + request.user = self.editor + request.journal = self.journal_one + request.press = self.journal_one.press + request.site_type = self.journal_one + return request + + @staticmethod + def create_user(username, roles=None, journal=None): + """ + Creates a user with the specified permissions. + :return: a user with the specified permissions + """ + # For consistency, outsourced to newer testing helpers + return helpers.create_user( + username, + roles=roles, + journal=journal, + ) + + @staticmethod + def create_roles(roles=None): + """ + Creates the necessary roles for testing. + :return: None + """ + # For consistency, outsourced to newer testing helpers + helpers.create_roles(roles=roles) + + @staticmethod + def create_journals(): + """ + Creates a set of dummy journals for testing + :return: a 2-tuple of two journals + """ + update_settings() + update_xsl_files() + journal_one = journal_models.Journal(code="TST", domain="testserver") + journal_one.save() + + journal_two = journal_models.Journal(code="TSA", domain="journal2.localhost") + journal_two.save() + + return journal_one, journal_two + + def setUp(self): + """ + Setup the test environment. + :return: None + """ + self.journal_one, self.journal_two = self.create_journals() + self.create_roles( + [ + "editor", + "author", + "reviewer", + "proofreader", + "production", + "copyeditor", + "typesetter", + "proofing-manager", + "section-editor", + ] + ) + + self.regular_user = self.create_user("regularuser@martineve.com") + self.regular_user.is_active = True + self.regular_user.save() + + self.second_user = self.create_user( + "seconduser@martineve.com", ["reviewer"], journal=self.journal_one + ) + self.second_user.is_active = True + self.second_user.save() + + self.admin_user = self.create_user("adminuser@martineve.com") + self.admin_user.is_staff = True + self.admin_user.is_active = True + self.admin_user.save() + + self.inactive_user = self.create_user( + "disableduser@martineve.com", + ["editor", "author", "proofreader", "production"], + journal=self.journal_one, + ) + self.inactive_user.is_active = False + self.inactive_user.save() + + self.editor = self.create_user( + "editoruser@martineve.com", ["editor"], journal=self.journal_one + ) + self.editor.is_active = True + self.editor.save() + + self.author = self.create_user( + "authoruser@martineve.com", ["author"], journal=self.journal_one + ) + self.author.is_active = True + self.author.save() + + self.proofreader = self.create_user( + "proofreader@martineve.com", ["proofreader"], journal=self.journal_one + ) + self.proofreader.is_active = True + self.proofreader.save() + + self.proofreader_two = self.create_user( + "proofreader2@martineve.com", ["proofreader"], journal=self.journal_one + ) + self.proofreader_two.is_active = True + self.proofreader_two.save() + + self.production = self.create_user( + "production@martineve.com", ["production"], journal=self.journal_one + ) + self.production.is_active = True + self.production.save() + + self.copyeditor = self.create_user( + "copyeditor@martineve.com", ["copyeditor"], journal=self.journal_one + ) + self.copyeditor.is_active = True + self.copyeditor.save() + + self.typesetter = self.create_user( + "typesetter@martineve.com", ["typesetter"], journal=self.journal_one + ) + self.typesetter.is_active = True + self.typesetter.save() + + self.other_typesetter = self.create_user( + "other_typesetter@martineve.com", ["typesetter"], journal=self.journal_one + ) + self.other_typesetter.is_active = True + self.other_typesetter.save() + + self.proofing_manager = self.create_user( + "proofing_manager@martineve.com", + ["proofing-manager"], + journal=self.journal_one, + ) + self.proofing_manager.is_active = True + self.proofing_manager.save() + + self.other_typesetter.is_active = True + self.other_typesetter.save() + + self.section_editor = self.create_user( + "section_editor@martineve.com", ["section-editor"], journal=self.journal_one + ) + self.section_editor.is_active = True + self.section_editor.save() + + self.second_reviewer = self.create_user( + "second_reviewer@martineve.com", ["reviewer"], journal=self.journal_one + ) + self.second_reviewer.is_active = True + self.second_reviewer.save() + + self.public_file = core_models.File( + mime_type="A/FILE", + original_filename="blah.txt", + uuid_filename="UUID.txt", + label="A file that is public", + description="Oh yes, it's a file", + owner=self.regular_user, + is_galley=False, + privacy="public", + ) + + self.public_file.save() + + self.private_file = core_models.File( + mime_type="A/FILE", + original_filename="blah.txt", + uuid_filename="UUID.txt", + label="A file that is private", + description="Oh yes, it's a file", + owner=self.regular_user, + is_galley=False, + privacy="owner", + ) + + self.private_file.save() + + self.article_in_production = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_TYPESETTING, + journal_id=self.journal_one.id, + ) + self.article_in_production.save() + self.article_in_production.data_figure_files.add(self.public_file) + + self.article_unsubmitted = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_UNSUBMITTED, + journal_id=self.journal_one.id, + ) + self.article_unsubmitted.save() + + self.article_unassigned = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_UNASSIGNED, + journal_id=self.journal_one.id, + ) + self.article_unassigned.save() + + self.article_assigned = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_ASSIGNED, + journal_id=self.journal_one.id, + ) + self.article_assigned.save() + + self.article_under_review = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_UNDER_REVIEW, + journal_id=self.journal_one.id, + ) + self.article_under_review.save() + + self.article_review_completed = submission_models.Article.objects.create( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_ACCEPTED, + journal_id=self.journal_one.id, + date_accepted=timezone.now(), + ) + + self.article_author_is_owner = submission_models.Article.objects.create( + owner=self.author, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_ACCEPTED, + journal_id=self.journal_one.id, + date_accepted=timezone.now(), + ) + + self.editor.snapshot_as_author(self.article_author_is_owner) + self.author.snapshot_as_author(self.article_author_is_owner) + + self.review_form = review_models.ReviewForm( + name="A Form", intro="i", thanks="t", journal=self.journal_one + ) + self.review_form.save() + + self.review_form_element, c = ( + review_models.ReviewFormElement.objects.get_or_create( + name="Review", + kind="text", + order=1, + required=True, + ) + ) + self.review_form.elements.add(self.review_form_element) + self.second_review_form_element, c = ( + review_models.ReviewFormElement.objects.get_or_create( + name="Second Review Form Element", + kind="text", + order=2, + required=True, + ) + ) + self.review_form.elements.add(self.second_review_form_element) + setting_handler.save_setting( + "general", + "enable_save_review_progress", + self.journal_one, + "On", + ) + + self.round_one = review_models.ReviewRound.objects.create( + article=self.article_review_completed, + round_number=1, + ) + self.round_two = review_models.ReviewRound.objects.create( + article=self.article_review_completed, + round_number=2, + ) + + self.review_assignment_complete = review_models.ReviewAssignment( + article=self.article_review_completed, + review_round=self.round_one, + reviewer=self.regular_user, + editor=self.editor, + date_due=timezone.now(), + form=self.review_form, + is_complete=True, + date_complete=timezone.now(), + decision="accept", + ) + + self.review_assignment_complete.save() + + self.review_assignment_withdrawn = ( + review_models.ReviewAssignment.objects.create( + article=self.article_review_completed, + review_round=self.round_two, + reviewer=self.second_reviewer, + editor=self.editor, + date_due=timezone.now(), + form=self.review_form, + is_complete=True, + decision="withdrawn", + ) + ) + + self.review_assignment_declined, created = ( + review_models.ReviewAssignment.objects.get_or_create( + article=self.article_review_completed, + review_round=self.round_two, + reviewer=self.second_reviewer, + editor=self.editor, + date_due=timezone.now(), + date_declined=timezone.now(), + form=self.review_form, + is_complete=False, + ) + ) + + self.review_assignment = review_models.ReviewAssignment( + article=self.article_under_review, + reviewer=self.second_user, + editor=self.editor, + date_due=timezone.now(), + form=self.review_form, + ) + + self.review_assignment.save() + + self.review_assignment_not_in_scope = review_models.ReviewAssignment( + article=self.article_in_production, + reviewer=self.regular_user, + editor=self.editor, + date_due=timezone.now(), + form=self.review_form, + ) + self.review_assignment_not_in_scope.save() + + self.article_under_revision = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_UNDER_REVISION, + journal_id=self.journal_one.id, + ) + self.article_under_revision.save() + + self.article_rejected = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_REJECTED, + journal_id=self.journal_one.id, + ) + self.article_rejected.save() + + self.article_accepted = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_ACCEPTED, + journal_id=self.journal_one.id, + ) + self.article_accepted.save() + + self.section_editor_assignment = review_models.EditorAssignment( + article=self.article_assigned, + editor=self.section_editor, + editor_type="section-editor", + notified=True, + ) + self.section_editor_assignment.save() + + self.article_editor_copyediting = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_EDITOR_COPYEDITING, + journal_id=self.journal_one.id, + ) + self.article_editor_copyediting.save() + + self.article_author_copyediting = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_AUTHOR_COPYEDITING, + journal_id=self.journal_one.id, + ) + self.article_author_copyediting.save() + + self.article_final_copyediting = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_FINAL_COPYEDITING, + journal_id=self.journal_one.id, + ) + self.article_final_copyediting.save() + + self.article_proofing = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_PROOFING, + journal_id=self.journal_one.id, + ) + self.article_proofing.save() + + assigned = production_models.ProductionAssignment( + article=self.article_in_production, production_manager=self.production + ) + assigned.save() + + self.article_published = submission_models.Article( + owner=self.regular_user, + title="A Second Test Article", + abstract="An abstract", + stage=submission_models.STAGE_PUBLISHED, + journal_id=self.journal_one.id, + ) + self.article_published.save() + + assigned = production_models.ProductionAssignment( + article=self.article_published, production_manager=self.production + ) + assigned.save() + + self.article_in_production_inactive = submission_models.Article( + owner=self.regular_user, + title="A Test Article", + abstract="An abstract", + stage=submission_models.STAGE_TYPESETTING, + journal_id=self.journal_one.id, + ) + self.article_in_production_inactive.save() + + self.assigned = production_models.ProductionAssignment( + article=self.article_in_production_inactive, + production_manager=self.inactive_user, + ) + self.assigned.save() + + self.typeset_task = production_models.TypesetTask( + assignment=self.assigned, + typesetter=self.typesetter, + notified=True, + accepted=timezone.now(), + ) + self.typeset_task.save() + + self.other_typeset_task = production_models.TypesetTask( + assignment=self.assigned, + typesetter=self.other_typesetter, + notified=True, + accepted=timezone.now(), + ) + self.other_typeset_task.save() + + self.proofing_assignment = proofing_models.ProofingAssignment( + article=self.article_proofing, + proofing_manager=self.proofing_manager, + notified=True, + ) + self.proofing_assignment.save() + self.proofing_assignment.add_new_proofing_round() + + self.proofing_task = proofing_models.ProofingTask( + round=self.proofing_assignment.current_proofing_round(), + proofreader=self.proofreader, + notified=True, + due=timezone.now(), + accepted=timezone.now(), + task="sdfsdffs", + ) + self.proofing_task.save() + + self.correction_task = proofing_models.TypesetterProofingTask( + proofing_task=self.proofing_task, + typesetter=self.typesetter, + notified=True, + due=timezone.now(), + accepted=timezone.now(), + task="fsddsff", + ) + self.correction_task.save() + + self.journal_one.name = "Journal One" + self.journal_two.name = "Journal Two" + self.press = helpers.create_press() + self.press.save() + update_settings( + self.journal_one, + management_command=False, + ) + self.reviewer_csv_header_row = b"salutation,firstname,middlename,lastname,email_address,department,institution,country,interests,reason" + self.good_reviewer_content_line = b"Mr,Andy,James,Byers,andy@janeway.systems,Open Library of Humanities,Birkbeck,GB,,Test Reason" + self.empty_reviewer_content_line = b" " + self.regular_user_csv_line = b"Mr,Regular,,User,regularuser@martineve.com,Somewhere Dept,Some Inst,GB,,A Reason" + + def test_request_revisions_context(self): + self.client.force_login(self.editor) + response = self.client.get( + reverse( + "review_request_revisions", + kwargs={"article_id": self.article_review_completed.pk}, + ), + SERVER_NAME=self.journal_one.domain, + ) + response.context.get("incomplete") + self.assertEqual( + self.article_review_completed, + response.context.get("article"), + ) + # This test does not cover the revision request form + self.assertEqual( + 0, + response.context.get("pending_approval").count(), + ) + self.assertEqual( + 0, + response.context.get("incomplete").count(), + ) + + # assignment_notification tests + + def _make_editor_assignment(self, notified=False): + return review_models.EditorAssignment.objects.create( + article=self.article_unassigned, + editor=self.editor, + editor_type="editor", + notified=notified, + ) + + def _assignment_notification_url(self): + return reverse( + "review_assignment_notification", + kwargs={ + "article_id": self.article_unassigned.pk, + "editor_id": self.editor.pk, + }, + ) + + def test_assignment_notification_get(self): + self._make_editor_assignment() + self.client.force_login(self.editor) + response = self.client.get( + self._assignment_notification_url(), + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual(response.status_code, 200) + + def test_assignment_notification_post_sends(self): + """A valid POST redirects and marks the assignment as notified.""" + assignment = self._make_editor_assignment() + self.client.force_login(self.editor) + response = self.client.post( + self._assignment_notification_url(), + { + "cc": "", + "bcc": "", + "subject": "You have been assigned", + "body": "You have been assigned as an editor.", + "send": "send", + }, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual(response.status_code, 302) + assignment.refresh_from_db() + self.assertTrue(assignment.notified) + + def test_assignment_notification_skip_redirects_and_notifies(self): + """Clicking Skip redirects and marks the assignment as notified without sending email.""" + assignment = self._make_editor_assignment() + self.client.force_login(self.editor) + response = self.client.post( + self._assignment_notification_url(), + { + "cc": "", + "bcc": "", + "subject": "You have been assigned", + "body": "You have been assigned as an editor.", + "skip": "skip", + }, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual(response.status_code, 302) + assignment.refresh_from_db() + self.assertTrue(assignment.notified) + self.assertEqual(len(mail.outbox), 0) + + def test_assignment_notification_send_delivers_email(self): + """Submitting the form sends an email to the assigned editor.""" + self._make_editor_assignment() + self.client.force_login(self.editor) + self.client.post( + self._assignment_notification_url(), + { + "cc": "", + "bcc": "", + "subject": "You have been assigned", + "body": "You have been assigned as an editor.", + "send": "send", + }, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual(len(mail.outbox), 1) + self.assertIn(self.editor.email, mail.outbox[0].to) + + def test_notify_reviewer_post_redirects(self): + """A valid POST redirects when one-click access is off (no URL check).""" + self.client.force_login(self.editor) + with janeway_setting_override( + "general", "enable_one_click_access", self.journal_one, "" + ): + response = self.client.post( + reverse( + "review_notify_reviewer", + kwargs={ + "article_id": self.article_under_review.pk, + "review_id": self.review_assignment.pk, + }, + ), + { + "cc": "", + "bcc": "", + "subject": "Review request", + "body": "Please review this article.", + "send": "send", + }, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual(response.status_code, 302) + + def test_notify_reviewer_skip_redirects_without_email(self): + """Skip bypasses URL validation and redirects without sending an email.""" + self.client.force_login(self.editor) + response = self.client.post( + reverse( + "review_notify_reviewer", + kwargs={ + "article_id": self.article_under_review.pk, + "review_id": self.review_assignment.pk, + }, + ), + { + "cc": "", + "bcc": "", + "subject": "Review request", + "body": "No link here.", + "skip": "skip", + }, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual(response.status_code, 302) + self.assertEqual(len(mail.outbox), 0) + + def test_notify_reviewer_with_correct_url_redirects(self): + """A POST containing the correct reviewer URL redirects when one-click access is on.""" + self.client.force_login(self.editor) + review_url = self.journal_one.site_url( + path=reverse( + "do_review", kwargs={"assignment_id": self.review_assignment.pk} + ) + ) + review_url = "{}?access_code={}".format( + review_url, self.review_assignment.access_code + ) + with janeway_setting_override( + "general", "enable_one_click_access", self.journal_one, "on" + ): + response = self.client.post( + reverse( + "review_notify_reviewer", + kwargs={ + "article_id": self.article_under_review.pk, + "review_id": self.review_assignment.pk, + }, + ), + { + "cc": "", + "bcc": "", + "subject": "Review request", + "body": "Please review at {}".format(review_url), + "send": "send", + }, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual(response.status_code, 302) + + def test_notify_reviewer_missing_url_does_not_redirect(self): + """A POST without the reviewer URL and without skip stays on the page.""" + self.client.force_login(self.editor) + with janeway_setting_override( + "general", "enable_one_click_access", self.journal_one, "on" + ): + response = self.client.post( + reverse( + "review_notify_reviewer", + kwargs={ + "article_id": self.article_under_review.pk, + "review_id": self.review_assignment.pk, + }, + ), + { + "cc": "", + "bcc": "", + "subject": "Review request", + "body": "No link here.", + "send": "send", + }, + SERVER_NAME=self.journal_one.domain, + ) + self.assertEqual(response.status_code, 200) diff --git a/src/review/views.py b/src/review/views.py index 83eaa507f4..b782d19986 100755 --- a/src/review/views.py +++ b/src/review/views.py @@ -4,7 +4,6 @@ __maintainer__ = "Birkbeck Centre for Technology and Publishing" -from uuid import uuid4 from collections import Counter from datetime import timedelta @@ -386,6 +385,7 @@ def assignment_notification(request, article_id, editor_id): ) email_context = logic.get_assignment_context(request, article, editor, assignment) + form = core_forms.SettingEmailForm( setting_name="editor_assignment", email_context=email_context, @@ -400,11 +400,13 @@ def assignment_notification(request, article_id, editor_id): email_context=email_context, request=request, ) - if form.is_valid(): + skip = request.POST.get("skip") + form_valid = form.is_valid() + if skip or form_valid: kwargs = { "editor_assignment": assignment, "request": request, - "skip": request.POST.get("skip"), + "skip": skip, "email_data": form.as_dataclass(), } @@ -412,6 +414,9 @@ def assignment_notification(request, article_id, editor_id): event_logic.Events.ON_EDITOR_MANUALLY_ASSIGNED, **kwargs ) + assignment.notified = True + assignment.save() + if request.GET.get("return", None): return redirect(request.GET.get("return")) else: @@ -749,7 +754,7 @@ def remove_file(request, article_id, round_id, file_id): messages.add_message( request, messages.INFO, - "Cannot remove a file from a closed review round.".format(file.label), + "Cannot remove a file from a closed review round.", ) return redirect(reverse("review_in_review", kwargs={"article_id": article_id})) @@ -1322,24 +1327,30 @@ def notify_reviewer(request, article_id, review_id): email_context = logic.get_reviewer_notification_context( request, article, request.user, review ) + one_click_access = setting_handler.get_setting( + "general", "enable_one_click_access", request.journal + ).value + expected_url = email_context["review_url"] if one_click_access else None - form = core_forms.SettingEmailForm( + form = forms.ReviewerNotificationForm( setting_name="review_assignment", email_context=email_context, + expected_url=expected_url, request=request, ) if request.POST: skip = request.POST.get("skip") - form = core_forms.SettingEmailForm( + form = forms.ReviewerNotificationForm( request.POST, request.FILES, setting_name="review_assignment", email_context=email_context, + expected_url=expected_url, request=request, ) - - if form.is_valid() or skip: + form.is_valid() + if skip or form.is_confirmed(): kwargs = { "email_data": form.as_dataclass(), "review_assignment": review, @@ -1354,7 +1365,26 @@ def notify_reviewer(request, article_id, review_id): review.date_requested = timezone.now() review.save() - return redirect(reverse("review_in_review", kwargs={"article_id": article_id})) + if skip: + messages.add_message( + request, + messages.SUCCESS, + "{} added as a reviewer (notification skipped).".format( + review.reviewer.full_name() + ), + ) + else: + messages.add_message( + request, + messages.SUCCESS, + "{} added as a reviewer and notified.".format( + review.reviewer.full_name() + ), + ) + + return redirect( + reverse("review_in_review", kwargs={"article_id": article_id}) + ) template = "review/notify_reviewer.html" context = { diff --git a/src/templates/admin/elements/email_form.html b/src/templates/admin/elements/email_form.html index 26c42cebd2..0240a03aa3 100644 --- a/src/templates/admin/elements/email_form.html +++ b/src/templates/admin/elements/email_form.html @@ -1,12 +1,12 @@ {% load foundation %} -
+ {% csrf_token %}
{{ form | foundation }}
- + {% if skip %} {% endif %} diff --git a/src/templates/admin/review/notify_reviewer.html b/src/templates/admin/review/notify_reviewer.html index 4b949be2db..6d99a0db22 100644 --- a/src/templates/admin/review/notify_reviewer.html +++ b/src/templates/admin/review/notify_reviewer.html @@ -2,7 +2,6 @@ {% load static %} {% load settings %} - {% block title %}Add Review Assignment{% endblock title %} {% block title-section %}Add Review Assignment{% endblock %} {% block title-sub %}#{{ article.pk }} / {{ article.correspondence_author.last_name }} / {{ article.safe_title }}{% endblock %} @@ -32,7 +31,10 @@

To {{ review.reviewer.full_name }}

From {{ request.user.full_name }}
- {% include "admin/elements/email_form.html" with form=form skip=1 %} + {% include "admin/elements/email_form.html" with form=form skip=1 form_id="notify_reviewer_form" button_name=form.CONFIRMABLE_BUTTON_NAME %} + {% if form.modal %} + {% include "admin/elements/confirm_modal.html" with modal=form.modal form_id="notify_reviewer_form" %} + {% endif %} @@ -44,5 +46,8 @@
From {{ request.user.full_name }}
{{ block.super}} {{ form.media.js }} + {% if form.modal %} + {% include "admin/elements/open_modal.html" with target=form.modal.id %} + {% endif %} {% endblock js %}