openlibhums / janeway

A web-based platform for publishing journals, preprints, conference proceedings, and books
https://janeway.systems/
GNU Affero General Public License v3.0
177 stars 65 forks source link

Error when accepting draft decision to reject paper #3510

Open pauldmccarthy opened 1 year ago

pauldmccarthy commented 1 year ago

Describe the bug Howdy, I experience the following error when a draft decision to reject a paper is accepted:

File "/usr/local/lib/python3.8/site-packages/django/core/handlers/exception.py" in inner
  41.             response = get_response(request)
File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py" in _legacy_get_response
  249.             response = self._get_response(request)
File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py" in _get_response
  187.                 response = self.process_exception_by_middleware(e, request)
File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py" in _get_response
  185.                 response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/vol/janeway/src/security/decorators.py" in wrapper
  89.         return func(request, *args, **kwargs)
File "/vol/janeway/src/security/decorators.py" in wrapper
  64.             return func(request, *args, **kwargs)
File "/vol/janeway/src/security/decorators.py" in wrapper
  190.             return func(request, *args, **kwargs)
File "/vol/janeway/src/review/views.py" in manage_draft
  2267.         decision_action = logic.handle_decision_action(article, draft, request)
File "/vol/janeway/src/review/logic.py" in handle_decision_action
  346.         event_logic.Events.raise_event(
File "/vol/janeway/src/events/logic.py" in raise_event
  294.             event_return = [func(**kwargs) for func in Events._hooks[event_name]]
File "/vol/janeway/src/events/logic.py" in <listcomp>
  294.             event_return = [func(**kwargs) for func in Events._hooks[event_name]]
File "/vol/janeway/src/utils/transactional_emails.py" in send_article_decision
  449.             subject,
Exception Type: UnboundLocalError at /review/article/20/decision/draft/9/action/
Exception Value: local variable 'subject' referenced before assignment

The utils.transactional_emails.send_article_decision function expects to be given a decision argument with a value of accept, decline or undecline. However, the review.logic.handle_decision_action function is setting decision='reject'.

I'm afraid I'm not familiar enough with the code to understand the semantics that differentiate 'decline' and 'reject', so am unable to suggest a fix. I have worked around it in my installation by making the following change to src/utils/transactional_emails.py:

diff --git a/src/utils/transactional_emails.py b/src/utils/transactional_emails.py
index 88b94390..2b6f0b04 100644
--- a/src/utils/transactional_emails.py
+++ b/src/utils/transactional_emails.py
@@ -437,7 +437,7 @@ def send_article_decision(**kwargs):

     if decision == 'accept':
         subject = 'subject_review_decision_accept'
-    elif decision == 'decline':
+    elif decision in ('decline', 'reject'):
         subject = 'subject_review_decision_decline'
     elif decision == 'undecline':
         subject = 'subject_review_decision_undecline'

Janeway version

v1.4.4 (but it looks like this issue is still present in master).

To Reproduce Steps to reproduce the behavior:

  1. Log in as author, and submit an article
  2. Log in as editor, assign the article to self
  3. Move to review, assign a reviewer
  4. Log in as reviewer, submit a review
  5. Log in as editor, navigate to article page, click on "Make a decision"
  6. Click on "Draft decisions", submit a draft decision to reject
  7. Accept the draft

Expected behavior

The error should not occur

Screenshots If applicable, add screenshots to help explain your problem.

Front-end Issues

If the issue is front-end specific please add the following details:

Desktop (please complete the following information):

Smartphone (please complete the following information):

Additional context Add any other context about the problem here.

ajrbyers commented 1 year ago

Thanks for this @pauldmccarthy, this is going to be addressed in release candidate three of v1.5

pauldmccarthy commented 1 year ago

Hi @ajrbyers, great - thanks for the incredibly speedy reply!

pauldmccarthy commented 1 year ago

Btw, would you have any idea as to whether my (obviously hacky and insufficient) work-around would be safe to use in the short term?

ajrbyers commented 1 year ago

It should be ok - I think its was just a typo originally. We have some confusion around "Reject" and "Decline" in some places that will need to be cleared up!