From 7b36621e2fccfd9640698e7423ebede5627b1db6 Mon Sep 17 00:00:00 2001 From: Andy Byers Date: Tue, 31 Mar 2026 12:11:38 +0100 Subject: [PATCH 1/2] fix: #1027 accept copyedit now uses EmailForm and dedicated view, includes skip --- src/copyediting/admin.py | 7 +- src/copyediting/logic.py | 15 ++--- src/copyediting/tests.py | 49 +++++++++++++- src/copyediting/urls.py | 5 ++ src/copyediting/views.py | 65 +++++++++++++++---- .../admin/copyediting/accept_copyedit.html | 38 +++++++++++ .../admin/copyediting/editor_review.html | 22 +------ src/utils/transactional_emails.py | 10 +-- 8 files changed, 159 insertions(+), 52 deletions(-) create mode 100644 src/templates/admin/copyediting/accept_copyedit.html diff --git a/src/copyediting/admin.py b/src/copyediting/admin.py index 679ef23c94..2957b5b946 100755 --- a/src/copyediting/admin.py +++ b/src/copyediting/admin.py @@ -39,8 +39,7 @@ class CopyeditAdmin(admin_utils.ArticleFKModelAdmin): "editor_note", "copyeditor_note", ) - raw_id_fields = ("article", "copyeditor", "editor") - filter_horizontal = ("files_for_copyediting", "copyeditor_files") + raw_id_fields = ("article", "copyeditor", "editor", "files_for_copyediting", "copyeditor_files") inlines = [admin_utils.AuthorReviewInline] @@ -62,9 +61,7 @@ class AuthorAdmin(admin.ModelAdmin): "assignment__editor_note", "assignment__copyeditor_note", ) - raw_id_fields = ("author", "assignment") - filter_horizontal = ("files_updated",) - exclude = ("files_updated",) + raw_id_fields = ("author", "assignment", "files_updated") def _journal(self, obj): return obj.assignment.article.journal.code if obj else "" diff --git a/src/copyediting/logic.py b/src/copyediting/logic.py index 29302654c8..f7a9a4f0ac 100755 --- a/src/copyediting/logic.py +++ b/src/copyediting/logic.py @@ -145,31 +145,28 @@ def handle_file_post(request, copyedit): return errors -def accept_copyedit(copyedit, article, request): +def accept_copyedit(copyedit, article, request, email_data=None, skip=False): """ Raises an event when a copyedit is accepted :param copyedit: CopyeditAssignment object :param article: Article object :param request: HttpRequest object + :param email_data: EmailData dataclass instance + :param skip: bool, if True the notification email is skipped :return: None """ - user_message_content = request.POST.get("accept_note") - kwargs = { "copyedit_assignment": copyedit, "article": article, - "user_message_content": user_message_content, + "email_data": email_data, "request": request, - "skip": True if "skip" in request.POST else False, + "skip": skip, } event_logic.Events.raise_event(event_logic.Events.ON_COPYEDIT_ACKNOWLEDGE, **kwargs) copyedit.copyedit_accepted = timezone.now() - - if "skip" not in request.POST: - copyedit.copyedit_acknowledged = True - + copyedit.copyedit_acknowledged = True copyedit.save() diff --git a/src/copyediting/tests.py b/src/copyediting/tests.py index ed0d5475bb..ebf59671b8 100644 --- a/src/copyediting/tests.py +++ b/src/copyediting/tests.py @@ -16,8 +16,9 @@ def setUpTestData(cls): "typesetter@janeway.systems", ["editor"], journal=cls.journal_one, - atrrs={"is_active": True}, ) + cls.editor.is_active = True + cls.editor.save() cls.copyeditor = helpers.create_user( "copyeditor@janeway.systems", ["copyeditor"], @@ -130,3 +131,49 @@ def test_active_article_review_task_200s(self): response.status_code, 200, ) + + def test_accept_copyedit_sends_email_and_marks_acknowledged(self): + """Accepting with send sets copyedit_accepted and copyedit_acknowledged.""" + task = models.CopyeditAssignment.objects.create( + article=self.active_article, + copyeditor=self.copyeditor, + editor=self.editor, + copyeditor_completed=timezone.now(), + ) + self.client.force_login(self.editor) + self.client.post( + reverse( + "accept_copyedit", + kwargs={ + "article_id": self.active_article.pk, + "copyedit_id": task.pk, + }, + ), + {"subject": "Thank you", "body": "Thank you for your work."}, + ) + task.refresh_from_db() + self.assertIsNotNone(task.copyedit_accepted) + self.assertTrue(task.copyedit_acknowledged) + + def test_accept_copyedit_skip_marks_acknowledged_without_email(self): + """Accepting with skip sets copyedit_accepted and copyedit_acknowledged without sending email.""" + task = models.CopyeditAssignment.objects.create( + article=self.active_article, + copyeditor=self.copyeditor, + editor=self.editor, + copyeditor_completed=timezone.now(), + ) + self.client.force_login(self.editor) + self.client.post( + reverse( + "accept_copyedit", + kwargs={ + "article_id": self.active_article.pk, + "copyedit_id": task.pk, + }, + ), + {"skip": "True"}, + ) + task.refresh_from_db() + self.assertIsNotNone(task.copyedit_accepted) + self.assertTrue(task.copyedit_acknowledged) diff --git a/src/copyediting/urls.py b/src/copyediting/urls.py index 79c1b64754..bea1b108c8 100755 --- a/src/copyediting/urls.py +++ b/src/copyediting/urls.py @@ -37,6 +37,11 @@ views.editor_review, name="editor_review", ), + re_path( + r"^article/(?P\d+)/assignment/(?P\d+)/accept/$", + views.accept_copyedit, + name="accept_copyedit", + ), re_path( r"^article/(?P\d+)/assignment/(?P\d+)/author_review/(?P\d+)/$", views.request_author_copyedit, diff --git a/src/copyediting/views.py b/src/copyediting/views.py index bdc7940243..b6b3b72c39 100755 --- a/src/copyediting/views.py +++ b/src/copyediting/views.py @@ -547,14 +547,7 @@ def editor_review(request, article_id, copyedit_id): ) if request.POST: - if "accept_note" in request.POST: - logic.accept_copyedit(copyedit, article, request) - - return redirect( - reverse("article_copyediting", kwargs={"article_id": article.id}) - ) - - elif ( + if ( "author_review" in request.POST or author_review_form.CONFIRMED_BUTTON_NAME in request.POST ): @@ -591,9 +584,6 @@ def editor_review(request, article_id, copyedit_id): context = { "article": article, "copyedit": copyedit, - "accept_message": logic.get_copyedit_message( - request, article, copyedit, "copyeditor_ack" - ), "reopen_message": logic.get_copyedit_message( request, article, copyedit, "copyeditor_reopen_task" ), @@ -603,6 +593,59 @@ def editor_review(request, article_id, copyedit_id): return render(request, template, context) +@editor_user_required +def accept_copyedit(request, article_id, copyedit_id): + """ + Allows an editor to accept a completed copyedit and optionally notify the copyeditor. + """ + article = get_object_or_404( + submission_models.Article, + pk=article_id, + journal=request.journal, + ) + copyedit = get_object_or_404( + models.CopyeditAssignment, + pk=copyedit_id, + article=article, + ) + email_context = logic.get_copyeditor_notification_context(request, article, copyedit) + form = core_forms.SettingEmailForm( + setting_name="copyeditor_ack", + email_context=email_context, + request=request, + ) + + if request.POST: + skip = "skip" in request.POST + form = core_forms.SettingEmailForm( + request.POST, + request.FILES, + setting_name="copyeditor_ack", + email_context=email_context, + request=request, + ) + if form.is_valid() or skip: + logic.accept_copyedit( + copyedit, + article, + request, + email_data=form.as_dataclass() if form.is_valid() else None, + skip=skip, + ) + return redirect( + reverse("article_copyediting", kwargs={"article_id": article.id}) + ) + + template = "copyediting/accept_copyedit.html" + context = { + "article": article, + "copyedit": copyedit, + "form": form, + } + + return render(request, template, context) + + @editor_user_required def request_author_copyedit(request, article_id, copyedit_id, author_review_id): """ diff --git a/src/templates/admin/copyediting/accept_copyedit.html b/src/templates/admin/copyediting/accept_copyedit.html new file mode 100644 index 0000000000..3432c656f7 --- /dev/null +++ b/src/templates/admin/copyediting/accept_copyedit.html @@ -0,0 +1,38 @@ +{% extends "admin/core/base.html" %} +{% load static %} + +{% block title %}Accept Copyedit{% endblock %} +{% block title-section %}Accept Copyedit{% endblock %} +{% block title-sub %}#{{ article.pk }} / {{ article.correspondence_author.last_name }} / {{ article.safe_title }}{% endblock %} + +{% block breadcrumbs %} + {{ block.super }} + {% include "elements/breadcrumbs/copyediting_base.html" with subpage="yes" %} +
  • Review Copyediting
    • +{% endblock breadcrumbs %} + +{% block body %} + +
    +
    +

    Accept Copyedit

    +
    +
    +

    You can write a note to the copyeditor thanking them for their time, or skip the email.

    +
    +
    +

    To {{ copyedit.copyeditor.full_name }}

    +
    From {{ request.user.full_name }}
    +
    +
    + {% url 'editor_review' article.pk copyedit.pk as cancel_url %} + {% include 'admin/elements/email_form.html' with form=form skip=1 cancel_url=cancel_url %} +
    +
    + +{% endblock %} + +{% block js %} + {{ block.super }} + {{ form.media.js }} +{% endblock js %} diff --git a/src/templates/admin/copyediting/editor_review.html b/src/templates/admin/copyediting/editor_review.html index 65f4e46548..f4f80487b7 100644 --- a/src/templates/admin/copyediting/editor_review.html +++ b/src/templates/admin/copyediting/editor_review.html @@ -149,7 +149,7 @@

    Actions

    {% if copyedit.actions_available %}
    -
    -
    -
    -

     Accept Copyedit

    -
    -
    -

    You can write a note to the copyeditor thanking them for their time.

    - - {% csrf_token %} - - - -
    -
    - -
    diff --git a/src/utils/transactional_emails.py b/src/utils/transactional_emails.py index aa7e37fabe..725e52d0be 100644 --- a/src/utils/transactional_emails.py +++ b/src/utils/transactional_emails.py @@ -921,7 +921,7 @@ def send_author_copyedit_deleted(**kwargs): def send_copyedit_ack(**kwargs): request = kwargs["request"] copyedit_assignment = kwargs["copyedit_assignment"] - user_message_content = kwargs["user_message_content"] + email_data = kwargs.get("email_data") skip = kwargs.get("skip", False) description = "{0} has acknowledged copyediting for {1}".format( @@ -937,11 +937,11 @@ def send_copyedit_ack(**kwargs): "target": copyedit_assignment.article, } - notify_helpers.send_email_with_body_from_user( + core_email.send_email( + copyedit_assignment.copyeditor, + email_data, request, - "subject_copyeditor_ack", - copyedit_assignment.copyeditor.email, - user_message_content, + article=copyedit_assignment.article, log_dict=log_dict, ) notify_helpers.send_slack(request, description, ["slack_editors"]) From 8ec275ebee6ccc51f117f44166c3b53d6c19b3a8 Mon Sep 17 00:00:00 2001 From: Andy Byers Date: Tue, 31 Mar 2026 12:12:20 +0100 Subject: [PATCH 2/2] chore: ruff formatting --- src/copyediting/views.py | 3 --- src/utils/transactional_emails.py | 4 +--- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/copyediting/views.py b/src/copyediting/views.py index b6b3b72c39..2df9bcf930 100755 --- a/src/copyediting/views.py +++ b/src/copyediting/views.py @@ -10,7 +10,6 @@ from django.http import Http404 from django.shortcuts import render, get_object_or_404, redirect from django.utils import timezone -from django.utils.translation import gettext as _ from copyediting import models, logic, forms from core import ( @@ -21,12 +20,10 @@ ) from events import logic as event_logic from security.decorators import ( - production_user_or_editor_required, copyeditor_user_required, copyeditor_for_copyedit_required, article_author_required, editor_user_required, - senior_editor_user_required, any_editor_user_required, ) diff --git a/src/utils/transactional_emails.py b/src/utils/transactional_emails.py index 725e52d0be..095c719172 100644 --- a/src/utils/transactional_emails.py +++ b/src/utils/transactional_emails.py @@ -18,7 +18,6 @@ models as core_models, ) from review import logic as review_logic -from review.const import EditorialDecisions as ED def send_reviewer_withdrawl_notice(**kwargs): @@ -1924,8 +1923,7 @@ def preprint_new_version(**kwargs): description = "{author} has submitted a new {obj} version.".format( author=request.user.full_name(), obj=request.repository.object_name, - title=preprint.title, - ) + ) log_dict = { "level": "Info", "action_text": description,