We are in the process of upgrading from 2.4.8-2 to 3.1.0-1. Our code is ojs 3.1.0-1, php 5.6.34, apache 2.4.25, postgres 9.6.7. Now we have a test installation, and see the following problem.
Automated reminders are being sent to reviewers even for archived articles years old. Once an article is archived, no automated reminders should be sent to reviewers no matter what incomplete reviewer actions it has. At least three such reminders were sent within hours of going live (and those were just the reviewers who complained).
As one example, searching as editor for one submission in the “Archived Submissions” section shows it as “Declined” but nevertheless “A review is overdue” appears beneath it. This is wrong: an archived submission does not have overdue reviews. We have more than 6400 archived submissions so we can’t look at them individually.
In our case, it was presumably the Acron plugin that sent the reminders, since we don’t have a cron job yet. Now the plugin is turned off and we don’t know how to proceed.
Was this only in a situation where article had unfinished reviews AND the article was archived? I can look into this, but need to be sure about the details. Also did this happen both with published and archived (rejected) articles with unfinished reviews?
I’m suspecting that the problem with
review_round_id that I mentioned in another post could be behind this problem too. If the system can’t locate the information it needs on the status of reviews, it could do anything. So maybe we should hit pause on this question until the other one is resolved.
did not notice this was from you as well. Sounds like the other issue may be causing this, yes.
Too soon to say, but I have a suggestion: either make the Acron plugin disabled by default, or make it obey the
scheduled_tasks configuration option. Neither are true at the moment, which makes it hard to play with test installations without sending spurious messages to reviewers.
(I’m assuming that Acron is the only way a review reminder could be sent within seconds of the journal coming live when
Probably Acron sent messages yes. We actually have a test server where we block all outgoing emails by default.
I do not know if there are any problems in making acron obey scheduled_tasks, you could of course do a feature request about it here at the forum.
After fixing the
review_assignments table as described in my
review_round_id thead, the same reviewer of the same article rejected in 2013 received the same reminder as soon as the journal came up. So there is some other problem. I’ll mention some symptoms that may or may not be relevant on the other thread. Meanwhile I’ll file a feature request and figure out how to disable outgoing mail.
An easy way of doing it is to try doing the upgrades on a local XAMPP server or similar.
Now I am 90% sure that the Acron plugin has a bug (OJS 3.1.0-1). Reminders have been sent to reviewers of articles matching these descriptions:
- Review request was sent but the editor rejected the article without waiting for the review and without cancelling the review request.
- Editor uploaded a review but didn’t enter a corresponding recommendation, then the editor rejected the article.
- Editor uploaded a review but didn’t enter a corresponding recommendation, then the editor accepted the article and it was published.
In my view automatic review reminders should never be sent to reviewers of rejected or accepted articles, and this restriction should override everything else. If I read the code correctly, this restriction is in place already for the scheduled tasks path to sending automatic reminders (please correct me if that’s wrong). Since our interaction with Acron has been solely to stop it from running (not so simple when upgrading 2.x.x-3.x.x), this bug does not affect us. But it might affect others.
Tagging @asmecher if he has an explanation.
The code should check the status of the submission before sending anything. It does that here: https://github.com/pkp/pkp-lib/blob/master/classes/task/ReviewReminder.inc.php#L147
The possible constants there are:
As you can see, if the submission status is anything else than STATUS_QUEUED the code will stop sending the reminder. So both the published and declined cases should not exist.
I was wondering if that code is used by the Acron plugin because I can’t see how rejected or published articles can get past the line you indicate.
The three papers in question were completed before upgrading from 2.4.8 to 3.1.0-1. However, the reminders should not have been sent after the upgrade because the
status field of the
submissions table is set correctly: 4 for the first two articles and 3 for the third.
Ok, so I think I know what is happening there.
Scenario: you have more than one review assignment that is late for the same submission. These are loaded as
$incompleteAssignments and the loop starts on row 136.
With the first assignment:
if on line 141 is filled and on line 143 $submission is set.
- Then on line 147 you have
if ($submission->getStatus() != STATUS_QUEUED) continue; which ends the loop for that assignment
However, with the second assignment for the same submission this happens:
- on line 141 the
if is not true, because the $submission is already set and it is the same for this assigment as well
- so the line 147 with
if ($submission->getStatus() != STATUS_QUEUED) continue; is bypassed and results into reminders being send!
Can you see the same logic here?
The fix is simply to move
if ($submission->getStatus() != STATUS_QUEUED) continue; to line 150 outside the if clause so that it is checked on each loop.
@asmecher if you can confirm this as well, I think this would be worth adding to 3.1.1 release! edit: see Move submission status check outside if clause by ajnyga · Pull Request #3550 · pkp/pkp-lib · GitHub
Great! I was just about to add that the examples I knew about had more than one item in $incompleteAssignments for the same submission, though I hadn’t figured out why that matters. The fix looks fine. I’ll try it soon.
Great spotting, @ajnyga! I’ve merged the PR for release in OJS 3.1.1.
Public Knowledge Project Team