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

Prevent upload of empty submission corrections #552

Merged
merged 10 commits into from
Oct 31, 2023

Conversation

Splines
Copy link
Member

@Splines Splines commented Oct 29, 2023

This is a temporary fix for #513 by preventing empty file uploads.

  • During correction upload
  • During bulk correction upload
  • Not checked: size during submission uploads by users

Where in the codebase

To test it in the UI, search for the string":type option"in the codebase. Once you've found the comment, set the size to 0 manually by adding

data.metadata.size = 0

above. Then, go ahead and upload any file in respective view. You should see a warning and not be able to complete upload.

Backend

Note: this is only a UI fix, not checking this on the backend, users could still upload empty files via POST. We might still want to ship this to prevent basic errors in the frontend during upload and to maybe find the reason for #513.
We are also checking for the minimum file size in the backend. To test it, change

if data.metadata.size == 0

to

if false

Then, users are able to upload the empty file in the frontend, but when they click on "Save", it will fail (with the same error message), as the backend validation fails.

Dummy file

Use the following dummy file as an empty file to test with: empty-file.pdf

@Splines Splines changed the base branch from main to mampf-next October 29, 2023 23:17
@Splines Splines self-assigned this Oct 29, 2023
@Splines Splines added the bug label Oct 29, 2023
@Splines Splines marked this pull request as ready for review October 29, 2023 23:17
@codecov
Copy link

codecov bot commented Oct 29, 2023

Codecov Report

Merging #552 (4af005a) into mampf-next (3289e75) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@              Coverage Diff               @@
##           mampf-next     #552      +/-   ##
==============================================
+ Coverage       66.49%   66.51%   +0.02%     
==============================================
  Files             311      311              
  Lines            9417     9423       +6     
==============================================
+ Hits             6262     6268       +6     
  Misses           3155     3155              
Files Coverage Δ
app/uploaders/correction_uploader.rb 100.00% <100.00%> (ø)
app/uploaders/submission_uploader.rb 100.00% <100.00%> (ø)

fosterfarrell9
fosterfarrell9 previously approved these changes Oct 30, 2023
@fosterfarrell9 fosterfarrell9 self-requested a review October 31, 2023 11:13
Copy link
Collaborator

@fosterfarrell9 fosterfarrell9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did some testing in my local setup, and it seems to be working nicely.

@Splines Splines merged commit 0ff42a1 into mampf-next Oct 31, 2023
5 of 7 checks passed
@Splines Splines deleted the fix/empty-submissions branch October 31, 2023 16:14
@Splines Splines mentioned this pull request Nov 1, 2023
Splines added a commit that referenced this pull request Nov 1, 2023
* Prevent upload of empty files

* Add console log for empty file

* Add more specific error message for empty files

* Validate file size in backend

* Improve locale strings

* Undo automatic linting

* Undo unwanted changes

* Reset whitespaces

* Fix multiple files stats for bulk upload

* Provide info about which file is empty to user
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants