Skip to content
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.

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

Cancelling revision upload removes the original file #8933

Closed
Vitaliy-1 opened this issue Apr 19, 2023 · 12 comments
Closed

Cancelling revision upload removes the original file #8933

Vitaliy-1 opened this issue Apr 19, 2023 · 12 comments
Assignees
Labels
Bug:2:Moderate A bug that is causing problems for a substantial minority of users.

Comments

@Vitaliy-1
Copy link
Collaborator

Vitaliy-1 commented Apr 19, 2023

Describe the bug
From the forum: https://forum.pkp.sfu.ca/t/bug-when-canceling-file-revision-in-omp/77900
Related to file upload wizard in general. When uploading a new submission file revision, there is an option to cancel the upload. If the file is already uploaded, pressing Cancel button deletes also original file
fileUploadWizard

To Reproduce
Steps to reproduce the behavior:

  1. Make s submission, upload a submission file
  2. On the submission stage, under Submission Files click on Upload File
  3. Select that this file is a revision of an existing submission file
  4. Click on Upload File and select file from the system to upload
  5. After upland press Cancel button
  6. See that the original file is removed

What application are you using?
OJS, OMP and OPS 3.4/stable 3.3.0

Additional information
When revision is uploaded it replaces the previous submission file:

. Create a new submission file instead?

Pressing Cancel triggers deleteFile operation on the correspondent submission file, which also removes both entries in files table, original and a new (revised) one

@Vitaliy-1 Vitaliy-1 added the Bug:2:Moderate A bug that is causing problems for a substantial minority of users. label Apr 19, 2023
@Vitaliy-1
Copy link
Collaborator Author

Vitaliy-1 commented Apr 19, 2023

The logic behind file revisions handling in the file upload wizard works as per documentation: 1) when a new revision is uploaded, new entry is added in the files table and the previous submission file is updated in the submission_files table; 2) During submissionFile removal all orphaned files not referenced in the submission_files are also removed, they are considered to be revisions, see:

// Delete all files that are not referenced by other submission files

The solution (considering the stage of the dev cycle) could be passing additional parameter to PKPManageFileApiHandler::deleteFile() from the old JS handler where the post request is made to cancel file upload, with the data from the original submissionFile so it could be restored

Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Apr 24, 2023
@Vitaliy-1
Copy link
Collaborator Author

Vitaliy-1 commented Apr 24, 2023

PRs
pkp-lib: #8941
OJS: pkp/ojs#3883
OMP: pkp/omp#1417
OPS: pkp/ops#539

Also requires backporting to the stable branch

Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Apr 24, 2023
@Vitaliy-1
Copy link
Collaborator Author

@NateWr, can you take a look? According to my tests on cancelling file upload, this PR does the trick. Cancelling the wizard updates the submission file, so it points to the original entry in the files table and the uploaded file is removed.

Do I need additional steps to validate the data regarding original file when passing it to the new endpoint in PKPManageFileApiHandler?

@Vitaliy-1
Copy link
Collaborator Author

Watch out for: #8389 as it also touches event log, either PR should be modified depending on what is merged first

Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 17, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 17, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 17, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 18, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 18, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 18, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 18, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 18, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 18, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 19, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 19, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 20, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 20, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue May 20, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/pkp-lib that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/ojs that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/ojs that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/ops that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/omp that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to Vitaliy-1/omp that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to pkp/omp that referenced this issue Jun 2, 2023
Vitaliy-1 added a commit to pkp/ops that referenced this issue Jun 2, 2023
@Vitaliy-1
Copy link
Collaborator Author

Merged

@asmecher
Copy link
Member

asmecher commented Jun 2, 2023

🎆 🎆 🎆 Thanks, @Vitaliy-1!

@asmecher
Copy link
Member

asmecher commented Jun 2, 2023

@Vitaliy-1, heads-up that the is_translated column is nullable on install in the event_log table but not nullable on upgrade. This was causing upgrade failures with PostgreSQL (see e.g. https://app.travis-ci.com/github/pkp/ops/builds/263527519). I'm not sure the column is actually supposed to be nullable but at least this gets the tests back to working (hopefully).

@Vitaliy-1
Copy link
Collaborator Author

Thanks! Forgot to revert changes on that line. Ideally, isTranslated column shouldn't be nullable or should be removed but I don't have enough information on how it was use before 3.0 release, so decided to leave it for now.

@ajnyga
Copy link
Collaborator

ajnyga commented Feb 22, 2024

Hi,
Would it be possible that these changes affected how submissionfilesuploadform::validate works?

In my allowedUploads plugin the hook is used and the plugin tries to add an error here:
https://github.com/ajnyga/allowedUploads/blob/master/AllowedUploadsPlugin.inc.php#L155

However, this leads to a weird error where I see a javascript in a dialog box and the upload forms opens again inside itself. This is in 3.4 stable. I think when I released the current version of the plugin back in early 2023 this worked so trying to figure out what changed.

The validation method is identical with https://github.com/pkp/controlPublicFiles/blob/main/ControlPublicFilesPlugin.php#L218

edit: added to dev channel as well...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug:2:Moderate A bug that is causing problems for a substantial minority of users.
Projects
None yet
Development

No branches or pull requests

4 participants