After upgrade OJS2 to OJS3 some editorial decisions don't appear to be logged fully

makouvlei · February 7, 2022, 11:46pm

Upgrading an OJS instance from OJS243 to OJS3307, and encoutering a problem where editorial decisions logged in the event log seem to be overruled by different criteria in the submission list. The question is whether this is default behaviour, or if it is a result of a data problem during migration?

We’ve narrowed the problem down to occurring in the upgrade from OJS 2.4.5-8 to OJS 3.1.1-4. We’ve compared data (e.g. review_assignments, edit_decisions, review_rounds) from the various version databases, but have not been able to identify any missing or broken data after the migration, that could cause this.

Here are some examples:

================================
Submission #92:

OJS2

PKP92-OJS2485-anon

OJS3

PKP92-OJS3213-anon

OJS3 status: REVIEW_ROUND_STATUS_REVIEWS_COMPLETED (9) i.e. “All reviewers have responded and a decision is needed”, but we’re expecting REVIEW_ROUND_STATUS_RESUBMIT_FOR_REVIEW_SUBMITTED (15) i.e. “Submission has been resubmitted for another review round”

================================
and another example … Submission #100:

OJS2:

PKP100-OJS2485-anon

OJS3:

PKP100-OJS3213-anon

OJS3 status: REVIEW_ROUND_STATUS_REVIEWS_OVERDUE (10) i.e. “A review is overdue”, but we’re expecting REVIEW_ROUND_STATUS_REVISIONS_REQUESTED (1) i.e. “Revisions have been requested.”? Could this problem stem from the logic at \lib\pkp\classes\submission\reviewRound\ReviewRound.inc.php which implies that the overdue reviews override any other status?
// Find the correct review round status based on the state of
// the current review assignments. The check order matters: the
// first conditions override the others.
if (empty($reviewAssignments)) {
return REVIEW_ROUND_STATUS_PENDING_REVIEWERS;
} elseif ($anyOverdueReview) {
return REVIEW_ROUND_STATUS_REVIEWS_OVERDUE;
} elseif ($anyUnreadReview) {
return REVIEW_ROUND_STATUS_REVIEWS_READY;
} elseif ($anyIncompletedReview) {
return REVIEW_ROUND_STATUS_PENDING_REVIEWS;
} elseif ($pendingRecommendations) {
return REVIEW_ROUND_STATUS_PENDING_RECOMMENDATIONS;
}

The problem only applies to the recently migrated submissions/decisions, i.e. after the migration is complete and new decisions are logged in OJS3, the new statuses seem to be display correctly in the submissions list.

Regards, Jannie

NateWr · February 14, 2022, 10:47am

Hi @makouvlei,

The relevant code here is pkp-lib/ReviewRound.inc.php at b50da610b79707eefec6698683398cc2d596ab95 · pkp/pkp-lib · GitHub.

Theses lines look for an existing status of revisions requested/submitted, and they occur before the code that checks on review assignments. So if the review round has the correct status in the database, then ReviewRound::determineStatus() should return the correct value. I suspect the status in the database is not correct.

I’m not sure what would cause that to happen, but I’d recommend checking the status in the database before and after upgrade to see if it changes. If it changes to the wrong thing, then there’s probably a bug somewhere in some upgrade code. It would be useful to know what version the change happens, because we can then track it down to a specific upgrade script.

Either way, if the problem is that the review round is incorrect in the database, and we can’t track it down to a specific issue in the upgrade, it may be quicker for you to write a script to fix the status in the database after upgrades. You can see how we determine whether a response to a revision/resubmit decision has been made in EditDecisionDAO::responseExists().

makouvlei · February 23, 2022, 10:22pm

Thanks @NateWr for your response.

It appears that the problem occurs due to the status’ field in the review_rounds table of the OJS2.4.3 database being empty for all records.

Post-upgrade to OJS 3.2.1.3 the status remains null in the case of #92 and #100, however some other review_rounds are assigned status 2,8,9 & 10.

Because of the null $roundStatus after being passed to ReviewRound::determineStatus() :

#92 never gets tested against EditDecisionDAO::responseExists()
and #100 never returns an accurate status, instead it allows $assignmentStatus to return $anyOverdueReview = true;

Do you have any suggestions to script correct values for the review_rounds status column for these submissions that remain null after the upgrade?

NateWr · February 24, 2022, 12:21pm

Hi @makouvlei, great that you were able to track down the source! I’m not familiar with the status info from OJS 2.x, but I will see if someone else in our community might be able to help.

If not, then the best thing to do is to perform your own recalculation of the review round status and assign the correct value. To do this, you’d need to check what assignments, decisions and revision files exist in the round and set the appropriate REVIEW_ROUND_STATUS_***** constant:

github.com

pkp/pkp-lib/blob/stable-3_3_0/classes/submission/reviewRound/ReviewRound.inc.php#L21-L46

      
        
            define('REVIEW_ROUND_STATUS_REVISIONS_REQUESTED', 1);
            define('REVIEW_ROUND_STATUS_RESUBMIT_FOR_REVIEW', 2);
            define('REVIEW_ROUND_STATUS_SENT_TO_EXTERNAL', 3);
            define('REVIEW_ROUND_STATUS_ACCEPTED', 4);
            define('REVIEW_ROUND_STATUS_DECLINED', 5);
            
            
// The following statuses are calculated based on the statuses of ReviewAssignments
            // in this round.
            define('REVIEW_ROUND_STATUS_PENDING_REVIEWERS', 6); // No reviewers have been assigned
            define('REVIEW_ROUND_STATUS_PENDING_REVIEWS', 7); // Waiting for reviews to be submitted by reviewers
            define('REVIEW_ROUND_STATUS_REVIEWS_READY', 8); // One or more reviews is ready for an editor to view
            define('REVIEW_ROUND_STATUS_REVIEWS_COMPLETED', 9); // All assigned reviews have been confirmed by an editor
            define('REVIEW_ROUND_STATUS_REVIEWS_OVERDUE', 10); // One or more reviews is overdue
            // The following status is calculated when the round is in REVIEW_ROUND_STATUS_REVISIONS_REQUESTED and
            // at least one revision file has been uploaded.
            define('REVIEW_ROUND_STATUS_REVISIONS_SUBMITTED', 11);
            
            
// The following statuses are calculated based on the statuses of recommendOnly EditorAssignments
            // and their decisions in this round.
            define('REVIEW_ROUND_STATUS_PENDING_RECOMMENDATIONS', 12); // Waiting for recommendations to be submitted by recommendOnly editors

This file has been truncated. show original

makouvlei · February 25, 2022, 5:54pm

Thanks @NateWr , that’s what I expected. Before we go the route of writing this script, considering it would be a fairly complex task, a couple of questions:

After upgrade to OJS3, should every record in review_rounds have a value in the status column?
I’ve been trying (unsuccessfully) to figure out at what point the upgrade assigned the values 2,8,9 & 10 to some of the review_rounds that previously had null status? Perhaps there is a solution where the upgrade could be amended for the correct status to be assigned to the other review_rounds as well, rather than coming up with a complex script to solve what might otherwise be a fairly simple problem?

makouvlei · March 2, 2022, 7:30pm

It seems during the upgrade, the review_round status gets configured by Installer::removeCancelledReviewAssignments() which is described as For 2.4.x - 3.1.0 upgrade: remove cancelled review assignments.

It calls ReviewAssignmentDAO::deleteById() which updates the status via ReviewAssignmentDAO::updateReviewRoundStatus(). The problem seems to occur when ReviewRoundDAO::updateStatus() attempts to configure the status column for NULL values via ReviewRound::determineStatus() which the same function called when OJS renders the submissions list page, where the incorrect flags are displayed.

It appears the cause is that ReviewRound::determineStatus() is not able to set status to REVIEW_ROUND_STATUS_REVISIONS_REQUESTED (as expected for example #100), nor REVIEW_ROUND_STATUS_RESUBMIT_FOR_REVIEW_SUBMITTED (as expected for example #92)? as these are dependent on values set explicitly by EditorDecisions? i.e. ReviewRound::determineStatus() expects submissions to already have any of the following statuses in place, and is not equipped to deal with NULL status of such review_rounds records during the upgrade.

REVIEW_ROUND_STATUS_REVISIONS_REQUESTED (1)
REVIEW_ROUND_STATUS_RESUBMIT_FOR_REVIEW (2)
REVIEW_ROUND_STATUS_SENT_TO_EXTERNAL (3)
REVIEW_ROUND_STATUS_ACCEPTED (4)
REVIEW_ROUND_STATUS_DECLINED (5)

Currently looking into a solution of using a modified version of ReviewRound::determineStatus() during the upgrade, which will first evaluate a submission’s most recent decision and populate the review_round status column accordingly before executing the remainder of the ReviewRound::determineStatus() function? Can probably modify code from SendReviewsForm::execute() for this purpose?

This seems like a far simpler solution than creating a custom script to perform these tasks post-upgrade.

NateWr · March 7, 2022, 10:35am

Hi @makouvlei,

Yes, fixing this at the source sounds like a sensible approach. Modifying ReviewRound::determineStatus() for the upgrade could work. Just don’t forget to revert your changes after upgrade.

Another option would be to modify Installer::removeCancelledReviewAssignments() so that it set the status on any review rounds with a NULL status before deleting cancelled review assignments. This approach would fix the bug in the upgrade scripts. We could merge this fix to the stable-3_3_0 branch and other people making this upgrade would benefit from it.