From aed37815d79b366534ddf70584cf5db988615380 Mon Sep 17 00:00:00 2001 From: Touhidur Rahman Date: Tue, 17 Feb 2026 10:38:31 +0600 Subject: [PATCH 1/4] pkp/pkp-lib#12347 Proper log history for submission file revision upload --- classes/submissionFile/Repository.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/classes/submissionFile/Repository.php b/classes/submissionFile/Repository.php index 707694762b7..a8e8c02e879 100644 --- a/classes/submissionFile/Repository.php +++ b/classes/submissionFile/Repository.php @@ -367,9 +367,12 @@ public function edit( $this->dao->update($newSubmissionFile); + // Get the updated submission file after saving to ensure all data is up to date + $newSubmissionFile = $this->get($newSubmissionFile->getId()); + $newFileUploaded = !empty($params['fileId']) && $params['fileId'] !== $submissionFile->getData('fileId'); - $logData = $this->getSubmissionFileLogData($submissionFile); + $logData = $this->getSubmissionFileLogData($newSubmissionFile); $logEntry = Repo::eventLog()->newDataObject(array_merge( $logData, [ From 3ed2062c7f3c29d5d1d06c5a6e073f7262f43494 Mon Sep 17 00:00:00 2001 From: Touhidur Rahman Date: Mon, 23 Feb 2026 16:26:44 +0600 Subject: [PATCH 2/4] pkp/pkp-lib#12347 migration to fix existing log submission file revision log history --- .../I12347_FixRevisionUploadLogData.php | 230 ++++++++++++++++++ 1 file changed, 230 insertions(+) create mode 100644 classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php diff --git a/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php b/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php new file mode 100644 index 00000000000..268139dd945 --- /dev/null +++ b/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php @@ -0,0 +1,230 @@ +fixRevisionUploadLogData(); + } + + /** + * Reverse the migration. + */ + public function down(): void + { + throw new DowngradeNotSupportedException(); + } + + /** + * Find all submission files with REVISION_UPLOAD log entries and fix + * their fileId and filename settings. + */ + private function fixRevisionUploadLogData(): void + { + $submissionFileIds = DB::table('event_log') + ->where('event_type', self::EVENT_TYPE_REVISION_UPLOAD) + ->where('assoc_type', self::ASSOC_TYPE_SUBMISSION_FILE) + ->distinct() + ->pluck('assoc_id'); + + foreach ($submissionFileIds as $submissionFileId) { + $this->fixLogsForSubmissionFile((int) $submissionFileId); + } + } + + /** + * Fix fileId and filename for all REVISION_UPLOAD log entries + * belonging to a single submission file. + * + * The bug caused each revision upload log to capture the PREVIOUS file's + * data instead of the newly uploaded file's data. This creates an off-by-one + * pattern in the revision chain: + * + * Revision chain: [file_86, file_87, file_88] + * Wrong log data: log1.fileId=86 (should be 87), log2.fileId=87 (should be 88) + */ + private function fixLogsForSubmissionFile(int $submissionFileId): void + { + // Get the revision chain for this submission file + $revisionChain = DB::table('submission_file_revisions') + ->where('submission_file_id', $submissionFileId) + ->orderBy('revision_id') + ->pluck('file_id') + // ->map(fn ($id) => (int) $id) + ->all(); + + // If there is only one entry, means there has been no revision upload being uploaded + // for this submission file and does not need any correction via migration + if (count($revisionChain) < 2) { + return; + } + + // Get all REVISION_UPLOAD log entries with their fileId settings + $logs = DB::table('event_log AS el') + ->join('event_log_settings AS els', function ($join) { + $join->on('els.log_id', '=', 'el.log_id') + ->where('els.setting_name', '=', 'fileId'); + }) + ->where('el.event_type', self::EVENT_TYPE_REVISION_UPLOAD) + ->where('el.assoc_type', self::ASSOC_TYPE_SUBMISSION_FILE) + ->where('el.assoc_id', $submissionFileId) + ->select([ + 'el.log_id', + 'el.date_logged', + 'els.setting_value AS logged_file_id', + ]) + ->orderBy('el.date_logged') + ->get(); + + if ($logs->isEmpty()) { + return; + } + + // Build a map: file_id => next_file_id in the revision chain + $nextFileIdMap = []; + for ($i = 0; $i < count($revisionChain) - 1; $i++) { + $nextFileIdMap[$revisionChain[$i]] = $revisionChain[$i + 1]; + } + + // Identify which log entries have the off-by-one bug + $wrongLogs = []; + foreach ($logs as $log) { + $loggedFileId = (int) $log->logged_file_id; + if (isset($nextFileIdMap[$loggedFileId])) { + $wrongLogs[] = $log; + } + } + + if (empty($wrongLogs)) { + return; + } + + // Fix fileId for all wrong entries + foreach ($wrongLogs as $log) { + $correctFileId = $nextFileIdMap[(int) $log->logged_file_id]; + + DB::table('event_log_settings') + ->where('log_id', $log->log_id) + ->where('setting_name', 'fileId') + ->update(['setting_value' => (string) $correctFileId]); + } + + // Fix filenames using the shifted-chain approach. + // Only safe when ALL log entries in this group are wrong (consistent pattern). + $this->fixFilenamesForSubmissionFile($submissionFileId, $logs, $wrongLogs); + } + + /** + * Fix the filename settings for REVISION_UPLOAD log entries using the + * shifted-chain approach. + * + * Since each log captured the PREVIOUS revision's filename: + * - log[i]'s correct filename = log[i+1]'s current (wrong) filename + * - Last log's correct filename = current submission_file_settings.name + * + * This only runs when ALL logs in the group are confirmed wrong. + */ + private function fixFilenamesForSubmissionFile(int $submissionFileId, Collection $allLogs, array $wrongLogs): void + { + if (count($wrongLogs) !== $allLogs->count()) { + return; + } + + $logsArray = $allLogs->values()->all(); + $logCount = count($logsArray); + + // Collect current (wrong) filenames for each log entry, per locale + $filenamesByLogId = []; + foreach ($logsArray as $log) { + $filenamesByLogId[$log->log_id] = DB::table('event_log_settings') + ->where('log_id', $log->log_id) + ->where('setting_name', 'filename') + ->pluck('setting_value', 'locale') + ->all(); + } + + // Get current submission_file_settings name for the last log entry's correction + $currentNames = DB::table('submission_file_settings') + ->where('submission_file_id', $submissionFileId) + ->where('setting_name', 'name') + ->pluck('setting_value', 'locale') + ->all(); + + for ($i = 0; $i < $logCount; $i++) { + $logId = $logsArray[$i]->log_id; + + if ($i < $logCount - 1) { + // Correct filename = next log entry's current (wrong) filename + $correctNames = $filenamesByLogId[$logsArray[$i + 1]->log_id]; + } else { + // Last log: correct filename = current submission_file_settings name + $correctNames = $currentNames; + } + + if (empty($correctNames)) { + continue; + } + + // Update each locale's filename + foreach ($correctNames as $locale => $name) { + DB::table('event_log_settings') + ->updateOrInsert( + [ + 'log_id' => $logId, + 'setting_name' => 'filename', + 'locale' => $locale, + ], + ['setting_value' => $name] + ); + } + + // Remove locale entries that exist in the old data but not in the correct data + $oldLocales = array_keys($filenamesByLogId[$logId] ?? []); + $removedLocales = array_diff($oldLocales, array_keys($correctNames)); + + if (!empty($removedLocales)) { + DB::table('event_log_settings') + ->where('log_id', $logId) + ->where('setting_name', 'filename') + ->whereIn('locale', $removedLocales) + ->delete(); + } + } + } +} From fbf020515befa0f927d760897245b6dbe8b7b9e6 Mon Sep 17 00:00:00 2001 From: Touhidur Rahman Date: Tue, 24 Feb 2026 15:24:07 +0600 Subject: [PATCH 3/4] pkp/pkp-lib#12347 optimised query at migration update --- .../I12347_FixRevisionUploadLogData.php | 340 +++++++++++------- classes/submissionFile/Repository.php | 3 +- 2 files changed, 215 insertions(+), 128 deletions(-) diff --git a/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php b/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php index 268139dd945..838581d4a8e 100644 --- a/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php +++ b/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php @@ -17,9 +17,8 @@ namespace PKP\migration\upgrade\v3_6_0; -use Illuminate\Support\Facades\DB; use Illuminate\Support\Collection; - +use Illuminate\Support\Facades\DB; use PKP\install\DowngradeNotSupportedException; use PKP\migration\Migration; @@ -52,179 +51,266 @@ public function down(): void } /** - * Find all submission files with REVISION_UPLOAD log entries and fix - * their fileId and filename settings. - */ - private function fixRevisionUploadLogData(): void - { - $submissionFileIds = DB::table('event_log') - ->where('event_type', self::EVENT_TYPE_REVISION_UPLOAD) - ->where('assoc_type', self::ASSOC_TYPE_SUBMISSION_FILE) - ->distinct() - ->pluck('assoc_id'); - - foreach ($submissionFileIds as $submissionFileId) { - $this->fixLogsForSubmissionFile((int) $submissionFileId); - } - } - - /** - * Fix fileId and filename for all REVISION_UPLOAD log entries - * belonging to a single submission file. + * Fix all REVISION_UPLOAD log entries using a bulk read → process → batch write approach. * - * The bug caused each revision upload log to capture the PREVIOUS file's - * data instead of the newly uploaded file's data. This creates an off-by-one + * Previously we have each revision upload log to capture the PREVIOUS file's + * data instead of the newly uploaded file's data which creates an off-by-one * pattern in the revision chain: * * Revision chain: [file_86, file_87, file_88] * Wrong log data: log1.fileId=86 (should be 87), log2.fileId=87 (should be 88) */ - private function fixLogsForSubmissionFile(int $submissionFileId): void + private function fixRevisionUploadLogData(): void { - // Get the revision chain for this submission file - $revisionChain = DB::table('submission_file_revisions') - ->where('submission_file_id', $submissionFileId) - ->orderBy('revision_id') - ->pluck('file_id') - // ->map(fn ($id) => (int) $id) - ->all(); + // Phase 1: Bulk read all needed data (4 queries total) + $allLogs = $this->fetchAllRevisionUploadLogs(); - // If there is only one entry, means there has been no revision upload being uploaded - // for this submission file and does not need any correction via migration - if (count($revisionChain) < 2) { + if ($allLogs->isEmpty()) { return; } - // Get all REVISION_UPLOAD log entries with their fileId settings - $logs = DB::table('event_log AS el') + $submissionFileIds = $allLogs->pluck('submission_file_id')->unique()->values(); + $logIds = $allLogs->pluck('log_id')->unique()->values(); + + $allRevisions = $this->fetchAllRevisionChains($submissionFileIds); + $allFilenames = $this->fetchAllLogFilenames($logIds); + $allCurrentNames = $this->fetchAllCurrentNames($submissionFileIds); + + // Phase 2: Compute all corrections in PHP (0 DB calls) + [$fileIdUpdates, $filenameLogIdsToDelete, $filenameInserts] = $this->computeCorrections( + $allLogs, + $allRevisions, + $allFilenames, + $allCurrentNames + ); + + // Phase 3: Batch write corrections (minimal queries) + $this->batchUpdateFileIds($fileIdUpdates); + $this->batchReplaceFilenames($filenameLogIdsToDelete, $filenameInserts); + } + + /** + * Fetch all REVISION_UPLOAD log entries with their fileId settings. + */ + private function fetchAllRevisionUploadLogs(): Collection + { + return DB::table('event_log AS el') ->join('event_log_settings AS els', function ($join) { $join->on('els.log_id', '=', 'el.log_id') ->where('els.setting_name', '=', 'fileId'); }) ->where('el.event_type', self::EVENT_TYPE_REVISION_UPLOAD) ->where('el.assoc_type', self::ASSOC_TYPE_SUBMISSION_FILE) - ->where('el.assoc_id', $submissionFileId) ->select([ 'el.log_id', + 'el.assoc_id AS submission_file_id', 'el.date_logged', 'els.setting_value AS logged_file_id', ]) + ->orderBy('el.assoc_id') ->orderBy('el.date_logged') ->get(); + } - if ($logs->isEmpty()) { - return; - } + /** + * Fetch all revision chains for the given submission file IDs. + * + * @return Collection Keyed by submission_file_id + */ + private function fetchAllRevisionChains(Collection $submissionFileIds): Collection + { + return DB::table('submission_file_revisions') + ->whereIn('submission_file_id', $submissionFileIds) + ->orderBy('submission_file_id') + ->orderBy('revision_id') + ->get(['submission_file_id', 'file_id']) + ->groupBy('submission_file_id'); + } - // Build a map: file_id => next_file_id in the revision chain - $nextFileIdMap = []; - for ($i = 0; $i < count($revisionChain) - 1; $i++) { - $nextFileIdMap[$revisionChain[$i]] = $revisionChain[$i + 1]; - } + /** + * Fetch all filename settings for the given log IDs. + * + * @return Collection Keyed by log_id + */ + private function fetchAllLogFilenames(Collection $logIds): Collection + { + return DB::table('event_log_settings') + ->whereIn('log_id', $logIds) + ->where('setting_name', 'filename') + ->get(['log_id', 'locale', 'setting_value']) + ->groupBy('log_id'); + } + + /** + * Fetch all current submission_file_settings names for the given submission file IDs. + * + * @return Collection Keyed by submission_file_id + */ + private function fetchAllCurrentNames(Collection $submissionFileIds): Collection + { + return DB::table('submission_file_settings') + ->whereIn('submission_file_id', $submissionFileIds) + ->where('setting_name', 'name') + ->get(['submission_file_id', 'locale', 'setting_value']) + ->groupBy('submission_file_id'); + } + + /** + * Compute all corrections needed by processing the pre-fetched data + * + * @return array{0: array, 1: array, 2: array} + * [0] fileIdUpdates: [log_id => correct_file_id] + * [1] filenameLogIdsToDelete: log_ids whose filename entries should be replaced + * [2] filenameInserts: rows to insert as correct filename entries + */ + private function computeCorrections( + Collection $allLogs, + Collection $allRevisions, + Collection $allFilenames, + Collection $allCurrentNames + ): array { + $fileIdUpdates = []; + $filenameLogIdsToDelete = []; + $filenameInserts = []; - // Identify which log entries have the off-by-one bug - $wrongLogs = []; - foreach ($logs as $log) { - $loggedFileId = (int) $log->logged_file_id; - if (isset($nextFileIdMap[$loggedFileId])) { - $wrongLogs[] = $log; + $logsBySubmissionFile = $allLogs->groupBy('submission_file_id'); + + foreach ($logsBySubmissionFile as $submissionFileId => $logs) { + $revisions = $allRevisions->get($submissionFileId); + + // if we have no revision or there is been only one revision, + // means there is just one initial file upload for which no correction needed + if (!$revisions || $revisions->count() < 2) { + continue; } - } - if (empty($wrongLogs)) { - return; - } + $revisionChain = $revisions->pluck('file_id')->all(); - // Fix fileId for all wrong entries - foreach ($wrongLogs as $log) { - $correctFileId = $nextFileIdMap[(int) $log->logged_file_id]; + // Build a map: file_id => next_file_id in the revision chain + $nextFileIdMap = []; + for ($i = 0; $i < count($revisionChain) - 1; $i++) { + $nextFileIdMap[$revisionChain[$i]] = $revisionChain[$i + 1]; + } - DB::table('event_log_settings') - ->where('log_id', $log->log_id) - ->where('setting_name', 'fileId') - ->update(['setting_value' => (string) $correctFileId]); + // Identify which log entries have the off-by-one bug + $wrongLogs = []; + foreach ($logs as $log) { + $loggedFileId = (int) $log->logged_file_id; + if (isset($nextFileIdMap[$loggedFileId])) { + $wrongLogs[] = $log; + $fileIdUpdates[$log->log_id] = $nextFileIdMap[$loggedFileId]; + } + } + + if (empty($wrongLogs)) { + continue; + } + + // Fix filenames only when ALL log entries in this group are wrong (consistent pattern) + if (count($wrongLogs) !== $logs->count()) { + continue; + } + + // Compute filename corrections using the shifted-chain approach: + // log[i]'s correct filename = log[i+1]'s current (wrong) filename + // Last log's correct filename = current submission_file_settings name + $logsArray = $logs->values()->all(); + $logCount = count($logsArray); + + $filenamesByLogId = []; + foreach ($logsArray as $log) { + $logFilenames = $allFilenames->get($log->log_id); + $filenamesByLogId[$log->log_id] = $logFilenames + ? $logFilenames->pluck('setting_value', 'locale')->all() + : []; + } + + $currentNames = $allCurrentNames->get($submissionFileId); + $currentNamesMap = $currentNames + ? $currentNames->pluck('setting_value', 'locale')->all() + : []; + + for ($i = 0; $i < $logCount; $i++) { + $logId = $logsArray[$i]->log_id; + + if ($i < $logCount - 1) { + $correctNames = $filenamesByLogId[$logsArray[$i + 1]->log_id]; + } else { + $correctNames = $currentNamesMap; + } + + if (empty($correctNames)) { + continue; + } + + $filenameLogIdsToDelete[] = $logId; + + foreach ($correctNames as $locale => $name) { + $filenameInserts[] = [ + 'log_id' => $logId, + 'setting_name' => 'filename', + 'locale' => $locale, + 'setting_value' => $name, + ]; + } + } } - // Fix filenames using the shifted-chain approach. - // Only safe when ALL log entries in this group are wrong (consistent pattern). - $this->fixFilenamesForSubmissionFile($submissionFileId, $logs, $wrongLogs); + return [$fileIdUpdates, $filenameLogIdsToDelete, $filenameInserts]; } /** - * Fix the filename settings for REVISION_UPLOAD log entries using the - * shifted-chain approach. - * - * Since each log captured the PREVIOUS revision's filename: - * - log[i]'s correct filename = log[i+1]'s current (wrong) filename - * - Last log's correct filename = current submission_file_settings.name + * Batch update fileId settings using chunked CASE/WHEN SQL. * - * This only runs when ALL logs in the group are confirmed wrong. + * @param array $fileIdUpdates [log_id => correct_file_id] */ - private function fixFilenamesForSubmissionFile(int $submissionFileId, Collection $allLogs, array $wrongLogs): void + private function batchUpdateFileIds(array $fileIdUpdates): void { - if (count($wrongLogs) !== $allLogs->count()) { + if (empty($fileIdUpdates)) { return; } - $logsArray = $allLogs->values()->all(); - $logCount = count($logsArray); - - // Collect current (wrong) filenames for each log entry, per locale - $filenamesByLogId = []; - foreach ($logsArray as $log) { - $filenamesByLogId[$log->log_id] = DB::table('event_log_settings') - ->where('log_id', $log->log_id) - ->where('setting_name', 'filename') - ->pluck('setting_value', 'locale') - ->all(); - } + foreach (array_chunk($fileIdUpdates, 1000, true) as $chunk) { + $cases = []; + $logIds = []; - // Get current submission_file_settings name for the last log entry's correction - $currentNames = DB::table('submission_file_settings') - ->where('submission_file_id', $submissionFileId) - ->where('setting_name', 'name') - ->pluck('setting_value', 'locale') - ->all(); - - for ($i = 0; $i < $logCount; $i++) { - $logId = $logsArray[$i]->log_id; - - if ($i < $logCount - 1) { - // Correct filename = next log entry's current (wrong) filename - $correctNames = $filenamesByLogId[$logsArray[$i + 1]->log_id]; - } else { - // Last log: correct filename = current submission_file_settings name - $correctNames = $currentNames; + foreach ($chunk as $logId => $correctFileId) { + $cases[] = sprintf('WHEN %d THEN \'%d\'', (int) $logId, (int) $correctFileId); + $logIds[] = (int) $logId; } - if (empty($correctNames)) { - continue; - } + $caseSql = implode(' ', $cases); + $logIdList = implode(',', $logIds); - // Update each locale's filename - foreach ($correctNames as $locale => $name) { - DB::table('event_log_settings') - ->updateOrInsert( - [ - 'log_id' => $logId, - 'setting_name' => 'filename', - 'locale' => $locale, - ], - ['setting_value' => $name] - ); - } + DB::statement( + "UPDATE event_log_settings SET setting_value = CASE log_id {$caseSql} END WHERE log_id IN ({$logIdList}) AND setting_name = 'fileId'" + ); + } + } - // Remove locale entries that exist in the old data but not in the correct data - $oldLocales = array_keys($filenamesByLogId[$logId] ?? []); - $removedLocales = array_diff($oldLocales, array_keys($correctNames)); + /** + * Replace filename settings by deleting old entries and inserting correct ones. + * + * @param array $logIdsToDelete Log IDs whose filename entries should be replaced + * @param array $inserts Rows to insert as correct filename entries + */ + private function batchReplaceFilenames(array $logIdsToDelete, array $inserts): void + { + if (empty($logIdsToDelete)) { + return; + } - if (!empty($removedLocales)) { - DB::table('event_log_settings') - ->where('log_id', $logId) - ->where('setting_name', 'filename') - ->whereIn('locale', $removedLocales) - ->delete(); - } + // Delete old filename entries for affected logs + foreach (array_chunk(array_unique($logIdsToDelete), 1000) as $chunk) { + DB::table('event_log_settings') + ->whereIn('log_id', $chunk) + ->where('setting_name', 'filename') + ->delete(); + } + + // Insert correct filename entries + foreach (array_chunk($inserts, 500) as $chunk) { + DB::table('event_log_settings')->insert($chunk); } } } diff --git a/classes/submissionFile/Repository.php b/classes/submissionFile/Repository.php index a8e8c02e879..d6e1152b0ba 100644 --- a/classes/submissionFile/Repository.php +++ b/classes/submissionFile/Repository.php @@ -388,7 +388,7 @@ public function edit( $submission = Repo::submission()->get($submissionFile->getData('submissionId')); - Repo::eventLog()->newDataObject(array_merge( + $submissionLogEntry = Repo::eventLog()->newDataObject(array_merge( $logData, [ 'assocType' => PKPApplication::ASSOC_TYPE_SUBMISSION, @@ -399,6 +399,7 @@ public function edit( 'dateLogged' => Core::getCurrentDate(), ] )); + Repo::eventLog()->add($submissionLogEntry); } /** From 6e3068df913aa0bb10bc7a1253f34cc559e8b514 Mon Sep 17 00:00:00 2001 From: Touhidur Rahman Date: Fri, 27 Feb 2026 17:35:01 +0600 Subject: [PATCH 4/4] pkp/pkp-lib#12347 migration update data filter based on 3.4.0-x or a above installed date --- .../I12347_FixRevisionUploadLogData.php | 47 +++++++++++++++++-- 1 file changed, 44 insertions(+), 3 deletions(-) diff --git a/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php b/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php index 838581d4a8e..2d35dc7e411 100644 --- a/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php +++ b/classes/migration/upgrade/v3_6_0/I12347_FixRevisionUploadLogData.php @@ -17,6 +17,7 @@ namespace PKP\migration\upgrade\v3_6_0; +use APP\core\Application; use Illuminate\Support\Collection; use Illuminate\Support\Facades\DB; use PKP\install\DowngradeNotSupportedException; @@ -62,8 +63,20 @@ public function down(): void */ private function fixRevisionUploadLogData(): void { + // The off-by-one bug in Repository::edit() was introduced in version 3.4.0. + // Data created before that version is correct and must not be modified. + $cutoffDate = $this->getBugIntroductionDate(); + + if ($cutoffDate === null) { + // No version >= 3.4.0 found in history. This happens on direct + // pre-3.4.0 → 3.6.0 upgrades where the buggy code never ran. + // So we can safely assume as this is an upgrade from 3.3.0-x or below + // no need to update any data in that case and nothing to fix. + return; + } + // Phase 1: Bulk read all needed data (4 queries total) - $allLogs = $this->fetchAllRevisionUploadLogs(); + $allLogs = $this->fetchAllRevisionUploadLogs($cutoffDate); if ($allLogs->isEmpty()) { return; @@ -90,9 +103,36 @@ private function fixRevisionUploadLogData(): void } /** - * Fetch all REVISION_UPLOAD log entries with their fileId settings. + * Determine the date when the buggy code (>= 3.4.0) was first installed. + * + * The off-by-one bug in PKP\submissionFile\Repository::edit() was introduced + * in version 3.4.0. Data created before this version was installed is correct + * and must not be modified by this migration. + * + * @return ?string The date_installed of the earliest version >= 3.4.0, or null if + * no such version exists (meaning the buggy code never ran). + */ + private function getBugIntroductionDate(): ?string + { + $product = Application::get()->getName(); + + return DB::table('versions') + ->where('product_type', 'core') + ->where('product', $product) + ->whereRaw('major * 1000 + minor * 100 + revision * 10 + build >= ?', [3400]) + ->orderByRaw('major * 1000 + minor * 100 + revision * 10 + build ASC') + ->limit(1) + ->value('date_installed'); + } + + /** + * Fetch all REVISION_UPLOAD log entries with their fileId settings, + * limited to entries created on or after the cutoff date. + * + * @param string $cutoffDate Only include logs with date_logged >= this value. + * This filters out correct pre-3.4.0 data. */ - private function fetchAllRevisionUploadLogs(): Collection + private function fetchAllRevisionUploadLogs(string $cutoffDate): Collection { return DB::table('event_log AS el') ->join('event_log_settings AS els', function ($join) { @@ -101,6 +141,7 @@ private function fetchAllRevisionUploadLogs(): Collection }) ->where('el.event_type', self::EVENT_TYPE_REVISION_UPLOAD) ->where('el.assoc_type', self::ASSOC_TYPE_SUBMISSION_FILE) + ->where('el.date_logged', '>=', $cutoffDate) ->select([ 'el.log_id', 'el.assoc_id AS submission_file_id',