diff --git a/classes/migration/upgrade/v3_4_0/I7725_DecisionConstantsUpdate.php b/classes/migration/upgrade/v3_4_0/I7725_DecisionConstantsUpdate.php index 127ab2520c9..e2a9c2cbea5 100644 --- a/classes/migration/upgrade/v3_4_0/I7725_DecisionConstantsUpdate.php +++ b/classes/migration/upgrade/v3_4_0/I7725_DecisionConstantsUpdate.php @@ -24,125 +24,30 @@ class I7725_DecisionConstantsUpdate extends \PKP\migration\upgrade\v3_4_0\I7725_ */ public function getDecisionMappings(): array { + // stage_id filtering removed: all old OJS 3.3 decision values are unique, + // and the parent class's updated_at tracking mechanism prevents collisions + // between sequential mappings (e.g., 1→2 then 2→4). + // OJS 3.3 had no validation on which decisions could be recorded at which + // stages, so decisions can exist at any stage in legacy data. + // See https://github.com/pkp/pkp-lib/issues/12357 return [ - // \PKP\decision\Decision::ACCEPT - [ - 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION, WORKFLOW_STAGE_ID_EXTERNAL_REVIEW, WORKFLOW_STAGE_ID_EDITING], - 'current_value' => 1, - 'updated_value' => 2, - ], - - // \PKP\decision\Decision::EXTERNAL_REVIEW - [ - 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION], - 'current_value' => 8, - 'updated_value' => 3, - ], - - // \PKP\decision\Decision::PENDING_REVISIONS - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 2, - 'updated_value' => 4, - ], - - // \PKP\decision\Decision::RESUBMIT - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 3, - 'updated_value' => 5, - ], - - // \PKP\decision\Decision::DECLINE - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 4, - 'updated_value' => 6, - ], - - // \PKP\decision\Decision::INITIAL_DECLINE - [ - 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION], - 'current_value' => 9, - 'updated_value' => 8, - ], - - // \PKP\decision\Decision::RECOMMEND_ACCEPT - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 11, - 'updated_value' => 9, - ], - - // \PKP\decision\Decision::RECOMMEND_PENDING_REVISIONS - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 12, - 'updated_value' => 10, - ], - - // \PKP\decision\Decision::RECOMMEND_RESUBMIT - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 13, - 'updated_value' => 11, - ], - - // \PKP\decision\Decision::RECOMMEND_DECLINE - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 14, - 'updated_value' => 12, - ], - - // \PKP\decision\Decision::NEW_EXTERNAL_ROUND - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 16, - 'updated_value' => 14, - ], - - // \PKP\decision\Decision::REVERT_DECLINE - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 17, - 'updated_value' => 15, - ], - - // \PKP\decision\Decision::REVERT_INITIAL_DECLINE - [ - 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION], - 'current_value' => 18, - 'updated_value' => 16 - ], - - // \PKP\decision\Decision::SKIP_EXTERNAL_REVIEW - [ - 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION], - 'current_value' => 19, - 'updated_value' => 17, - ], - - // \PKP\decision\Decision::BACK_FROM_PRODUCTION - [ - 'stage_id' => [WORKFLOW_STAGE_ID_PRODUCTION], - 'current_value' => 31, - 'updated_value' => 29, - ], - - // \PKP\decision\Decision::BACK_FROM_COPYEDITING - [ - 'stage_id' => [WORKFLOW_STAGE_ID_EDITING], - 'current_value' => 32, - 'updated_value' => 30, - ], - - // \PKP\decision\Decision::CANCEL_REVIEW_ROUND - [ - 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION, WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], - 'current_value' => 33, - 'updated_value' => 31, - ], + ['current_value' => 1, 'updated_value' => 2], // ACCEPT + ['current_value' => 8, 'updated_value' => 3], // EXTERNAL_REVIEW + ['current_value' => 2, 'updated_value' => 4], // PENDING_REVISIONS + ['current_value' => 3, 'updated_value' => 5], // RESUBMIT + ['current_value' => 4, 'updated_value' => 6], // DECLINE + ['current_value' => 9, 'updated_value' => 8], // INITIAL_DECLINE + ['current_value' => 11, 'updated_value' => 9], // RECOMMEND_ACCEPT + ['current_value' => 12, 'updated_value' => 10], // RECOMMEND_PENDING_REVISIONS + ['current_value' => 13, 'updated_value' => 11], // RECOMMEND_RESUBMIT + ['current_value' => 14, 'updated_value' => 12], // RECOMMEND_DECLINE + ['current_value' => 16, 'updated_value' => 14], // NEW_EXTERNAL_ROUND + ['current_value' => 17, 'updated_value' => 15], // REVERT_DECLINE + ['current_value' => 18, 'updated_value' => 16], // REVERT_INITIAL_DECLINE + ['current_value' => 19, 'updated_value' => 17], // SKIP_EXTERNAL_REVIEW + ['current_value' => 31, 'updated_value' => 29], // BACK_FROM_PRODUCTION + ['current_value' => 32, 'updated_value' => 30], // BACK_FROM_COPYEDITING + ['current_value' => 33, 'updated_value' => 31], // CANCEL_REVIEW_ROUND ]; } } diff --git a/classes/migration/upgrade/v3_5_0/I11241_MissingDecisionConstantsUpdate.php b/classes/migration/upgrade/v3_5_0/I11241_MissingDecisionConstantsUpdate.php index 873cf7a7a70..4c1c8a2bf12 100644 --- a/classes/migration/upgrade/v3_5_0/I11241_MissingDecisionConstantsUpdate.php +++ b/classes/migration/upgrade/v3_5_0/I11241_MissingDecisionConstantsUpdate.php @@ -30,24 +30,14 @@ public function getDecisionMappings(): array { return [ // \PKP\decision\Decision::ACCEPT - /** - * NOTE : Accept of submission can happen at the - * 1. submission stage without going through external review phase - * 2. external review stage after going through external review phase - */ + // PRODUCTION added: OJS 3.3 allowed ACCEPT at any stage via "change decision" UI + // See https://github.com/pkp/pkp-lib/issues/12357 [ - 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION, WORKFLOW_STAGE_ID_EXTERNAL_REVIEW], + 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION, WORKFLOW_STAGE_ID_EXTERNAL_REVIEW, WORKFLOW_STAGE_ID_PRODUCTION], 'current_value' => 1, 'updated_value' => 2, ], - // \PKP\decision\Decision::EXTERNAL_REVIEW - [ - 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION], - 'current_value' => 8, - 'updated_value' => 3, - ], - // \PKP\decision\Decision::SKIP_EXTERNAL_REVIEW [ 'stage_id' => [WORKFLOW_STAGE_ID_SUBMISSION], @@ -101,7 +91,14 @@ public function up(): void return; } - // Upgrading from a 3.4.0-* + // If upgrading from 3.4.0-11 or above, then we have a fixed applied also for it + // at https://github.com/pkp/pkp-lib/issues/12140 + // so nothing to do and return + if ($currentVersion->major == 3 && $currentVersion->minor == 4 && $currentVersion->build >= 11) { + return; + } + + // Upgrading from a 3.4.0-10 or below version // Need to figure out the first installed date of 3.4.0-* // Then need to update the decisions made before the first version of 3.4.0-* installed $firstVersionOf34 = DB::table('versions') @@ -129,7 +126,63 @@ public function up(): void 'updated_at' => Carbon::now(), ]) ); - + + // \PKP\decision\Decision::EXTERNAL_REVIEW (8→3) — special handling + // + // After the buggy I7725 ran, both stranded EXTERNAL_REVIEW rows and + // correctly-migrated INITIAL_DECLINE rows share decision=8, stage_id=1. + // They are indistinguishable from edit_decisions alone. + // + // Two-layer disambiguation: + // 1. whereExists(review_rounds at EXTERNAL_REVIEW stage) — the submission + // must have actually reached external review + // 2. whereNotExists(later decision=8 at same submission/stage) — only + // the MOST RECENT decision=8 per submission is the EXTERNAL_REVIEW; + // all earlier ones were INITIAL_DECLINE + // + // Why "most recent" instead of checking for REVERT_INITIAL_DECLINE: + // OJS 3.3 had a loose workflow that allowed editors to decline a submission + // and then send it to review WITHOUT recording a REVERT_INITIAL_DECLINE. + // The "most recent" check handles ALL cases: with revert, without revert, + // and multiple decline cycles. + + // Collect the IDs of the MOST RECENT decision=8 per submission + // (the EXTERNAL_REVIEW row). Done as a separate SELECT because MySQL + // does not allow referencing the target table in an UPDATE subquery. + $externalReviewIds = DB::table('edit_decisions') + ->where('edit_decisions.stage_id', WORKFLOW_STAGE_ID_SUBMISSION) + ->where('edit_decisions.decision', 8) + ->whereNull('edit_decisions.updated_at') + ->where('edit_decisions.date_decided', '<', $firstVersionOf34->date_installed) + ->whereExists(function ($query) { + $query->select(DB::raw(1)) + ->from('review_rounds') + ->whereColumn('review_rounds.submission_id', 'edit_decisions.submission_id') + ->where('review_rounds.stage_id', WORKFLOW_STAGE_ID_EXTERNAL_REVIEW); + }) + ->whereNotExists(function ($query) use ($firstVersionOf34) { + // If a LATER decision=8 at stage_id=1 exists for the same submission, + // then this row was an earlier INITIAL_DECLINE — not the final + // EXTERNAL_REVIEW that triggered the review + $query->select(DB::raw(1)) + ->from('edit_decisions as later_ed') + ->whereColumn('later_ed.submission_id', 'edit_decisions.submission_id') + ->where('later_ed.decision', 8) + ->where('later_ed.stage_id', WORKFLOW_STAGE_ID_SUBMISSION) + ->whereColumn('later_ed.date_decided', '>', 'edit_decisions.date_decided') + ->where('later_ed.date_decided', '<', $firstVersionOf34->date_installed); + }) + ->pluck('edit_decision_id'); + + if ($externalReviewIds->isNotEmpty()) { + DB::table('edit_decisions') + ->whereIn('edit_decision_id', $externalReviewIds) + ->update([ + 'decision' => 3, + 'updated_at' => Carbon::now(), + ]); + } + $this->removeUpdatedAtColumn(); } diff --git a/classes/migration/upgrade/v3_5_0/I12357_FixDecisionConstantsMissedStageId.php b/classes/migration/upgrade/v3_5_0/I12357_FixDecisionConstantsMissedStageId.php new file mode 100644 index 00000000000..cca01f702f9 --- /dev/null +++ b/classes/migration/upgrade/v3_5_0/I12357_FixDecisionConstantsMissedStageId.php @@ -0,0 +1,71 @@ + 1, 'updated_value' => 2], // ACCEPT + ]; + } + + /** + * Run the migrations. + */ + public function up(): void + { + // If the first installed version is 3.4.0+, no pre-3.4 legacy data exists + $firstInstalledVersion = DB::table('versions') + ->where('product', Application::get()->getName()) + ->where('product_type', 'core') + ->orderBy('date_installed') + ->first(); + + if ($firstInstalledVersion->major > 3 || ($firstInstalledVersion->major == 3 && $firstInstalledVersion->minor >= 4)) { + return; + } + + // Run the parent's up() which uses configureUpdatedAtColumn(), + // iterates mappings with whereNull('updated_at'), and removeUpdatedAtColumn(). + // Already-migrated ACCEPT rows have decision=2 (not 1), so + // WHERE decision=1 only matches stranded rows. + parent::up(); + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + throw new DowngradeNotSupportedException(); + } +} diff --git a/dbscripts/xml/upgrade.xml b/dbscripts/xml/upgrade.xml index 1c25b5aeef9..16d11157c2a 100644 --- a/dbscripts/xml/upgrade.xml +++ b/dbscripts/xml/upgrade.xml @@ -182,6 +182,7 @@ + diff --git a/lib/pkp b/lib/pkp index aee04153f01..3bbaabea4d7 160000 --- a/lib/pkp +++ b/lib/pkp @@ -1 +1 @@ -Subproject commit aee04153f01b75b648269f995dc65572b0e5ee3c +Subproject commit 3bbaabea4d7e2371732191ccdaaac97158bf3596