Skip to content

pkp/pkp-lib#12140 Back port decision lost solution for 3.4 introduced for 3.5/main upgrade #5520

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
touhidurabir wants to merge 2 commits into
base: main
Choose a base branch
from
+164 −134
Open

pkp/pkp-lib#12140 Back port decision lost solution for 3.4 introduced for 3.5/main upgrade #5520

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4bb8ae4
pkp/pkp-lib#12140 Back port decision lost solution for 3.4 introduced…
touhidurabir Apr 28, 2026
abf215d
pkp/pkp-lib#12140 Submodule Update ##touhidurabir/i12140_main##
touhidurabir Apr 28, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
141 changes: 23 additions & 118 deletions classes/migration/upgrade/v3_4_0/I7725_DecisionConstantsUpdate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
];
}
}
83 changes: 68 additions & 15 deletions classes/migration/upgrade/v3_5_0/I11241_MissingDecisionConstantsUpdate.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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],
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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();
}

Expand Down
71 changes: 71 additions & 0 deletions classes/migration/upgrade/v3_5_0/I12357_FixDecisionConstantsMissedStageId.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
<?php

/**
* @file classes/migration/upgrade/v3_5_0/I12357_FixDecisionConstantsMissedStageId.php
*
* Copyright (c) 2025 Simon Fraser University
* Copyright (c) 2025 John Willinsky
* Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
*
* @class I12357_FixDecisionConstantsMissedStageId
*
* @brief Fix ACCEPT decisions at PRODUCTION stage missed by previous migrations.
* OJS 3.3 allowed ACCEPT at any stage via the "change decision" UI,
* but I7725/I11241 only mapped ACCEPT at SUBMISSION, EXTERNAL_REVIEW, and EDITING.
*
* @see https://github.com/pkp/pkp-lib/issues/12357
* @see https://github.com/pkp/pkp-lib/issues/11876
*/

namespace APP\migration\upgrade\v3_5_0;

use APP\core\Application;
use Illuminate\Support\Facades\DB;
use PKP\install\DowngradeNotSupportedException;

class I12357_FixDecisionConstantsMissedStageId extends \PKP\migration\upgrade\v3_4_0\I7725_DecisionConstantsUpdate
{
/**
* Get the decisions constants mappings
*/
public function getDecisionMappings(): array
{
// Only ACCEPT (1→2) is targeted here. This is collision-safe because
// no other OJS migration mapping produces decision=1 as output
// (INTERNAL_REVIEW=1 is OMP-only and not registered in OJS).
return [
['current_value' => 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();
}
}
1 change: 1 addition & 0 deletions dbscripts/xml/upgrade.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@
<migration class="PKP\migration\upgrade\v3_5_0\I11603_AddMissingUserRoleAssignmentInvitationEmail"/>
<migration class="PKP\migration\upgrade\v3_5_0\I11673_AddMissingApprovalToReviewerSuggestion"/>
<migration class="PKP\migration\upgrade\v3_5_0\I11800_AddUserRoleMastheadUpdateEmail"/>
<migration class="APP\migration\upgrade\v3_5_0\I12357_FixDecisionConstantsMissedStageId"/>
</upgrade>

<upgrade minversion="3.5.0.0" maxversion="3.5.9.9">
Expand Down
2 changes: 1 addition & 1 deletion lib/pkp
Open in desktop
Submodule pkp updated 1 files
+7 −2 classes/submission/maps/Schema.php
Loading