Outstanding editorial task automatic email content going wrong

[yoruyenturk]

Describe the bug
Dear @asmecher, @touhidurabir

The Editorial Reminder task email does not return the correct list. Is it possible to update this using the determineStatus function?

To Reproduce
Steps to reproduce the behavior:

1. EditorialReminder.php prepares the task content using the review round status, but prepares it incorrectly.
2. In the article shown in Picture 1, 1 reviewer has completed the evaluation and 1 reviewer is overdue. However, the editor of the article does not receive an overdue message in the Outstanding e-mail. Review Round table status 7 appears in the database (see Picture 2).
3. Similarly, there is an article with round status 8 in the database, but it is overdue in the backend but not in the mail (see Picture 3). You can see the database image from Picture 4.
4. The status statuses that appear in the system backend are not sent in the outstanding task e-mail. While the system shows notification using REVIEW_ASSIGNMENT_STATUS_XXX, the outstanding task email is sent using REVIEW_ROUND_STATUS_XXX.
5. It would be very helpful if jobs/email/EditorialReminder.php could be updated to provide a correct list.

What application are you using?
OJS version 3.4.0-5

Additional information
Picture 1.

Picture 2.

Picture 3.

Picture 4.

PRs

Stable 3.4.0

pkp-lib --> #10332
ojs --> pkp/ojs#4407 [TEST ONLY]

main (upcoming 3.5)

pkp-lib --> #10753
ojs --> pkp/ojs#4570 [TEST ONLY]

[Vitaliy-1]

I cannot reproduce the bug described here. Just to confirm, the editorial reminder is sent but it doesn't contain the "A review is overdue" message or the whole editorial reminder email isn't sent?

[yoruyenturk]

I cannot reproduce the bug described here. Just to confirm, the editorial reminder is sent but it doesn't contain the "A review is overdue" message or the whole editorial reminder email isn't sent?

Hi @Vitaliy-1 ,

“A review is overdue” is going. However, cases with a status of 7 or 8 in the database and one or more reviewers are overdue are not included in the e-mail.

[yoruyenturk]

For example, in the article in Picture 1, a referee is displayed as overdue. However, it is not included in the editorial task mail. Because in Picture 2, the status in the database is 7. Editorial Reminder only includes those with status 10 in the list.

[Vitaliy-1]

Hmm, review round status 7 and 8 shouldn't be assigned if the review is overdue...

[yoruyenturk]

Hmm, review round status 7 and 8 shouldn't be assigned if the review is overdue...

You are right, but I can see a similar situation in many articles.

When I updated the code in jobs/email/EditorialReminder.php as determineStatus instead of getStatus, it worked. It may be a bug that the status stays at 7 and 8.

[touhidurabir]

Probably our implementation approach has some issue here. Here we are making decision based on last review round's status but we probably need to check the any active review assignment associated with that review round and make decision based on those review assignments' status . Another approach can be replace the getStatus with determineStatus for checking as @yoruyenturk has described but not possible to confirm without some testing .

About the review round get the status 7 or 8, if I am not mistaken , it will be updated to those value for a review round when any one the review assignment in that review round is pending or any review is submitted for editor to look at . Seems like this checking is bit confusing .

[touhidurabir]

I am able to reproduce the issue where getStatus and determineStatus return value . When a reviewer get assigned and accepted the review request , then both the getStatus and determineStatus return REVIEW_ROUND_STATUS_PENDING_REVIEWS ==> 7 that is Waiting for reviews to be submitted by reviewers . But once the review submission due date exceed , the getStatus still returns REVIEW_ROUND_STATUS_PENDING_REVIEWS ==> 7 but the determineStatus return REVIEW_ROUND_STATUS_REVIEWS_OVERDUE ==> 10 . This mainly happens as we do not have any schedule/cron to automatically update the review round status once the due date exceed . However as the determineStatus get to check based on review assignment at run time , it returns different result .

Also in the EditorialReminder job , we are deciding if outstanding based on the following statuses of review round

1. REVIEW_ROUND_STATUS_PENDING_REVIEWERS --> 6
2. REVIEW_ROUND_STATUS_REVIEWS_COMPLETED --> 9
3. REVIEW_ROUND_STATUS_REVIEWS_OVERDUE --> 10
4. REVIEW_ROUND_STATUS_REVISIONS_SUBMITTED --> 11

so it does not consider the REVIEW_ROUND_STATUS_PENDING_REVIEWS ==> 7 (Waiting for reviews to be submitted by reviewers) and REVIEW_ROUND_STATUS_REVIEWS_READY --> 8 (One or more reviews is ready for an editor to view) .

One way to solve it to include the REVIEW_ROUND_STATUS_PENDING_REVIEWS ==> 7 and REVIEW_ROUND_STATUS_REVIEWS_READY --> 8 in the list of status based on which the outstanding get calculated . And that seems easy one .

Another approach can be to have a scheduler to update the review round status to REVIEW_ROUND_STATUS_REVIEWS_OVERDUE --> 10 when due dates exceeds .

@Vitaliy-1 what do you think ?

[Vitaliy-1]

Hmm, it's a more general problem than I thought before. One of the solutions could be creating new job every time due dates are updated to run in a specified time (and to double check when it runs if the date isn't edited). But I would vote to not try figuring out the best approach so late in the development cycle. It's better just to use determineStatus here and open an issue with a description of the problem with 3.6 milestone.

As for other status, it makes sense to add them to the EditorialReminder

[touhidurabir]

@Vitaliy-1 as per my finding at #10214 (comment) , it is possible to only add those 2 status (REVIEW_ROUND_STATUS_PENDING_REVIEWS ==> 7 and REVIEW_ROUND_STATUS_REVIEWS_READY --> 8) we can have a solution without depending on determineStatus . may be we should really not include the determineStatus and go with this solution for now and have a issue created for 3.6 that will have a better implementation as we both agree that call to determineStatus is an expensive call .

[Vitaliy-1]

I'm ok to use determineStatus in this job (once) but we have the same problem in the Submission Schema, for the submissions list, where we have this call for each review assignment associated with the submissions in the list.

it is possible to only add those 2 status (REVIEW_ROUND_STATUS_PENDING_REVIEWS ==> 7 and REVIEW_ROUND_STATUS_REVIEWS_READY --> 8) we can have a solution without depending on determineStatus

But it doesn't tell us if review is overdue, right?

[touhidurabir]

But it doesn't tell us if review is overdue, right?

@Vitaliy-1 you are right about that . and in that case our only option is to depend on determineStatus until we have a schedule task that update due date periodically .

[touhidurabir]

@Vitaliy-1 updated. If all ok, then I will forward port it main .