Skip to content

Commit

Permalink
#10660 fix:cancelling revision file deletes original
Browse files Browse the repository at this point in the history
  • Loading branch information
taslangraham committed Jan 30, 2025
1 parent 0d02655 commit 4d3308e
Show file tree
Hide file tree
Showing 6 changed files with 199 additions and 17 deletions.
2 changes: 1 addition & 1 deletion classes/migration/SubmissionFilesMigration.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public function up() {
$table->bigInteger('submission_file_id')->unsigned();
$table->bigInteger('file_id')->unsigned();
$table->foreign('submission_file_id')->references('submission_file_id')->on('submission_files');
$table->foreign('file_id')->references('file_id')->on('files');
$table->foreign('file_id')->references('file_id')->on('files')->onDelete('cascade')->onUpdate('cascade');
});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
<?php
/**
* @file classes/migration/upgrade/I8933_FixSubmissionFileRevisionsFileIdForeignKeyConstraint.inc.php
*
* Copyright (c) 2024 Simon Fraser University
* Copyright (c) 2024 John Willinsky
* Distributed under the GNU GPL v3. For full terms see the file docs/COPYING.
*
* @class I8933_FixSubmissionFileRevisionsFileIdForeignKeyConstraint
* @brief Cascade delete and update for `fileId` in submission_file_revisions table.
*/

use Illuminate\Database\Capsule\Manager as Capsule;
use Illuminate\Database\Migrations\Migration;
use Illuminate\Database\Schema\Blueprint;

class I8933_FixSubmissionFileRevisionsFileIdForeignKeyConstraint extends Migration
{
/**
* Run the migration.
*/
public function up()
{
Capsule::schema()->table('submission_file_revisions', function (Blueprint $table) {
$table->dropForeign(['file_id']);
$table->foreign('file_id')->references('file_id')->on('files')->onDelete('cascade')->onUpdate('cascade');
});
}

/**
* Reverse the migration.
*/
public function down()
{
Capsule::schema()->table('submission_file_revisions', function (Blueprint $table) {
$table->dropForeign(['file_id']);
$table->foreign('file_id')->references('file_id')->on('files');
});
}
}
105 changes: 104 additions & 1 deletion controllers/api/file/PKPManageFileApiHandler.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ function __construct() {
parent::__construct();
$this->addRoleAssignment(
array(ROLE_ID_MANAGER, ROLE_ID_SUB_EDITOR, ROLE_ID_ASSISTANT, ROLE_ID_REVIEWER, ROLE_ID_AUTHOR),
array('deleteFile', 'editMetadata', 'editMetadataTab', 'saveMetadata')
array('deleteFile', 'editMetadata', 'editMetadataTab', 'saveMetadata', 'cancelFileUpload')
);
// Load submission-specific translations
AppLocale::requireComponents(LOCALE_COMPONENT_PKP_SUBMISSION);
Expand Down Expand Up @@ -170,6 +170,109 @@ protected function getUpdateNotifications() {
return array(NOTIFICATION_TYPE_PENDING_EXTERNAL_REVISIONS);
}

/**
* Restore original file when cancelling the upload wizard
*/
public function cancelFileUpload(array $args, Request $request): JSONMessage
{
if (!$request->checkCSRF()) {
return new JSONMessage(false);
}

$submissionFile = $this->getAuthorizedContextObject(ASSOC_TYPE_SUBMISSION_FILE); /**@var SubmissionFile $submissionFile */
$originalFile = $request->getUserVar('originalFile') ? (array)$request->getUserVar('originalFile') : null;
$revisedFileId = $request->getUserVar('fileId') ? (int)$request->getUserVar('fileId') : null;

$submissionFileDao = DAORegistry::getDAO('SubmissionFileDAO'); /* @var $submissionFileDao SubmissionFileDAO */

// Get revisions and check file IDs
$revisions = $submissionFileDao->getRevisions($submissionFile->getId());
$revisionIds = [];
foreach ($revisions as $revision) {
$revisionIds[] = $revision->fileId;
}

if (!$revisedFileId || !in_array($revisedFileId, $revisionIds)) {
return new JSONMessage(false);
}

if (!empty($originalFile)) {
if (!isset($originalFile['fileId']) || !in_array($originalFile['fileId'], $revisionIds)) {
return new JSONMessage(false);
}

$originalFileId = (int)$originalFile['fileId'];

// Get the file name and uploader user ID
$originalUserId = $originalFile['uploaderUserId'] ? (int)$originalFile['uploaderUserId'] : null;
$originalFileName = $originalFile['name'] ? (array)$originalFile['name'] : null;
if (!$originalUserId || !$originalFileName) {
return new JSONMessage(false);
}

$userDao = DAORegistry::getDAO('UserDAO'); /* @var $userDao UserDAO */

$originalUser = $userDao->getById($originalUserId);
if (!$originalUser) {
return new JSONMessage(false);
}

$originalUsername = $originalUser->getUsername();
$matchedLogEntry = $this->findMatchedLogEntry($submissionFile, $originalFileId, $originalUsername, $originalFileName);
if (!$matchedLogEntry) {
return new JSONMessage(false);
}

// Restore original submission file
$matchedLogEntryParams = $matchedLogEntry->getParams();
$submissionFile->setData('fileId', $matchedLogEntryParams['fileId']);
$submissionFile->setData('name', $originalFileName);
$submissionFile->setData('uploaderUserId', $userDao->getByUsername($matchedLogEntryParams['username'])->getId());
$submissionFileDao->updateObject($submissionFile);
}

// Remove uploaded file
Services::get('file')->delete($revisedFileId);
$this->setupTemplate($request);
return DAO::getDataChangedEvent();
}

/**
* Compare user supplied data when cancelling file upload with saved in the event log;
* assuming we found the right entry if they match
*/
protected function findMatchedLogEntry(
SubmissionFile $submissionFile,
int $originalFileId,
string $originalUsername,
array $originalFileName
): ?EventLogEntry
{
$submissionFileEventLogDao = DAORegistry::getDAO('SubmissionFileEventLogDAO'); /* @var $submissionFileEventLogDao SubmissionFileEventLogDAO */

$logEntries = $submissionFileEventLogDao->getBySubmissionFileId($submissionFile->getId());
$match = null;
foreach ($logEntries->toIterator() as $logEntry) {
$params = $logEntry->getParams();
$loggedUsername = $params['username'];
$loggedFileName = $params['originalFileName'];
$loggedFileId = $params['fileId'];
if (!$loggedUsername || !$loggedFileName || !$loggedFileId) {
continue;
}

if (
$loggedUsername === $originalUsername &&
in_array($loggedFileName, $originalFileName) &&
$loggedFileId === $originalFileId
) {
$match = $logEntry;
break;
}
}

return $match;
}
}


26 changes: 21 additions & 5 deletions controllers/wizard/fileUpload/FileUploadWizardHandler.inc.php
Original file line number Diff line number Diff line change
Expand Up @@ -367,14 +367,20 @@ function uploadFile($args, $request) {
return new JSONMessage(true, $uploadForm->fetch($request));
}

$submissionFileId = $uploadForm->getRevisedFileId();
// Store the data of the submission file before it's replaced by the revised file
if ($submissionFileId) {
$submissionFileDao = DAORegistry::getDAO('SubmissionFileDAO'); /* @var $submissionFileDao SubmissionFileDAO */
$originalFile = $submissionFileDao->getById($submissionFileId);
}

$uploadedFile = $uploadForm->execute(); /* @var $uploadedFile SubmissionFile */
if (!is_a($uploadedFile, 'SubmissionFile')) {
return new JSONMessage(false, __('common.uploadFailed'));
}

// Retrieve file info to be used in a JSON response.
$uploadedFileInfo = $this->_getUploadedFileInfo($uploadedFile);
$reviewRound = $this->getReviewRound();
$uploadedFileInfo = $this->_getUploadedFileInfo($uploadedFile, $originalFile ?? null);

// Advance to the next step (i.e. meta-data editing).
return new JSONMessage(true, '', '0', $uploadedFileInfo);
Expand All @@ -398,7 +404,7 @@ function editMetadata($args, $request) {
import('lib.pkp.controllers.wizard.fileUpload.form.SubmissionFilesMetadataForm');
$form = new SubmissionFilesMetadataForm($submissionFile, $this->getStageId(), $this->getReviewRound());
$form->initData();

return new JSONMessage(true, $form->fetch($request));
}

Expand Down Expand Up @@ -462,14 +468,24 @@ function _onlyNumbersDiffer($a, $b) {
* @param SubmissionFile $uploadedFile
* @return array
*/
function _getUploadedFileInfo($uploadedFile) {
return array(
function _getUploadedFileInfo($uploadedFile, $originalFile = null) {
$uploadedFile = array(
'uploadedFile' => array(
'id' => $uploadedFile->getId(),
'fileId' => $uploadedFile->getData('fileId'),
'name' => $uploadedFile->getLocalizedData('name'),
'genreId' => $uploadedFile->getGenreId(),
)
);

if ($originalFile) {
$uploadedFile['uploadedFile']['originalFile'] = [
'fileId' => $originalFile->getData('fileId'),
'name' => $originalFile->getData('name'),
'uploaderUserId' => $originalFile->getData('uploaderUserId'),
];
}

return $uploadedFile;
}
}
40 changes: 31 additions & 9 deletions js/controllers/wizard/fileUpload/FileUploadWizardHandler.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
this.deleteUrl_ = options.deleteUrl;
this.metadataUrl_ = options.metadataUrl;
this.finishUrl_ = options.finishUrl;
this.cancelUrl_ = options.cancelUrl;

// Bind events of the nested upload forms.
this.bind('fileUploaded', this.handleFileUploaded);
Expand All @@ -65,7 +66,7 @@


/**
* The URL to be called when a cancel event occurs.
* The URL to be called when a delete event occurs.
* @private
* @type {string}
*/
Expand All @@ -90,7 +91,14 @@
$.pkp.controllers.wizard.fileUpload.FileUploadWizardHandler.
prototype.finishUrl_ = '';


/**
* The URL to be called when a cancel event occurs.
* @private
* @type {string}
*/
$.pkp.controllers.wizard.fileUpload.FileUploadWizardHandler.
prototype.cancelUrl_ = '';

/**
* Information about the uploaded file (once there is one).
* @private
Expand All @@ -100,6 +108,14 @@
prototype.uploadedFile_ = null;


/**
* Information about the file being revised.
* @private
* @type {{fileId: number, name: string, uploaderUserId: number}}
*/
$.pkp.controllers.wizard.fileUpload.FileUploadWizardHandler.
prototype.originalFile_ = null;

//
// Public methods
//
Expand Down Expand Up @@ -243,7 +259,8 @@
this.uploadedFile_.csrfToken = this.csrfToken_;
// Authorization policy expects to find the submissionFileId para
this.uploadedFile_.submissionFileId = this.uploadedFile_.id;
$.post(this.deleteUrl_, this.uploadedFile_,
this.uploadedFile_.originalFile = this.originalFile_;
$.post(this.cancelUrl_, this.uploadedFile_,
$.pkp.classes.Helper.curry(this.wizardCancelSuccess, this,
wizardElement, event), 'json');

Expand Down Expand Up @@ -297,9 +314,15 @@
*/
$.pkp.controllers.wizard.fileUpload.FileUploadWizardHandler.
prototype.handleFileUploaded = function(callingForm, event, uploadedFile) {
// Keep the original file data to restore if the wizard is canceled
if (this.originalFile_ === null) {
this.originalFile_ = uploadedFile.originalFile;
}

delete uploadedFile.originalFile;

// Save the uploaded file information.
this.uploadedFile_ = uploadedFile;
// Save the uploaded file information.
this.uploadedFile_ = uploadedFile;
};


Expand Down Expand Up @@ -347,11 +370,10 @@
*/
$.pkp.controllers.wizard.fileUpload.FileUploadWizardHandler.
prototype.startWizard = function() {
// Reset the uploaded and original file.
this.uploadedFile_ = this.originalFile_ = null;

// Reset the uploaded file.
this.uploadedFile_ = null;

this.parent('startWizard');
this.parent('startWizard');
};


Expand Down
3 changes: 2 additions & 1 deletion templates/controllers/wizard/fileUpload/fileUploadWizard.tpl
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@
finishButtonText: {translate|json_encode key="common.complete"},
deleteUrl: {url|json_encode component="api.file.ManageFileApiHandler" op="deleteFile" submissionId=$submissionId stageId=$stageId fileStage=$fileStage suppressNotification=true escape=false},
metadataUrl: {url|json_encode op="editMetadata" submissionId=$submissionId stageId=$stageId reviewRoundId=$reviewRoundId fileStage=$fileStage assocType=$assocType assocId=$assocId queryId=$queryId escape=false},
finishUrl: {url|json_encode op="finishFileSubmission" submissionId=$submissionId stageId=$stageId reviewRoundId=$reviewRoundId fileStage=$fileStage assocType=$assocType assocId=$assocId queryId=$queryId escape=false}
finishUrl: {url|json_encode op="finishFileSubmission" submissionId=$submissionId stageId=$stageId reviewRoundId=$reviewRoundId fileStage=$fileStage assocType=$assocType assocId=$assocId queryId=$queryId escape=false},
cancelUrl: {url|json_encode component="api.file.ManageFileApiHandler" op="cancelFileUpload" submissionId=$submissionId stageId=$stageId fileStage=$fileStage escape=false}
{rdelim}
);
{rdelim});
Expand Down

0 comments on commit 4d3308e

Please sign in to comment.