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

Incorrect File Size Calculation During Upload for Compressed Files in Birth Registration #7840

Open
nifaliana opened this issue Oct 24, 2024 · 5 comments
Assignees
Labels
Milestone

Comments

@nifaliana
Copy link

Describe the bug
During the birth registration process (tested in OpenCRVS-Madagascar), files are compressed before being uploaded. However, the system calculates the total upload size based on the original, uncompressed file sizes, which results in an incorrect size calculation for enforcing the 20MB limit.

I should be able to upload multiple compressed files as long as their total size remains under the 20MB limit. However, the system currently restricts me to only three files because it incorrectly calculates the total size based on the original file sizes rather than the compressed sizes.

Which feature of OpenCRVS your bug concerns?

This bug concerns the File Upload feature during birth registration. Specifically, it involves the compression process and the enforcement of the 20MB file size limit.

To Reproduce
Steps to reproduce the behavior:

  • Log in as a system admin or registrar.
  • Go to the birth registration form where files can be uploaded.
  • Upload multiple files.
  • Compression will occur during the upload process.
  • The total file size is calculated based on the original files instead of the compressed versions.
  • See error where the upload may be allowed or rejected inaccurately due to the wrong size calculation.

Expected behavior
The total size of the uploaded files should be calculated based on the compressed versions of the files. The system should ensure that the total size of the compressed files does not exceed the 20MB limit, preventing any inaccurate size limit enforcement.

Actual behavior
Currently, the system calculates the total size based on the original file sizes before compression. This causes the size limit enforcement to be incorrect, potentially blocking uploads even when the compressed total is below 20MB.

Screenshots
Image

OpenCRVS Core Version:
v1.6.0 (Git branch: master / release-v1.6.0)

Country Configuration Version:
v1.6.0 (Git branch: master / release-v1.6.0)

Desktop (please complete the following information):
OS: Ubuntu 24
Browser: Chrome
Version: recent version 130.0.6723.58

Possible fixes
Modify the file size calculation logic to only include the sizes of the compressed files instead of the original file sizes.
This can be done post-compression during the upload process.

Reproducible demo
N/A (If available, provide a demo link or project for reproduction.)

@nifaliana
Copy link
Author

I made a PR for this one here : #7840
Feel free to make a review.
Thanks

@rikukissa
Copy link
Member

Hi Rija, thanks for reporting!

During the birth registration process (tested in OpenCRVS-Madagascar), files are compressed before being uploaded. However, the system calculates the total upload size based on the original, uncompressed file sizes, which results in an incorrect size calculation for enforcing the 20MB limit. I should be able to upload multiple compressed files as long as their total size remains under the 20MB limit. However, the system currently restricts me to only three files because it incorrectly calculates the total size based on the original file sizes rather than the compressed sizes.

I think what causes confusion and the weird experience is the fact that we allow countries to define compressImagesToSizeMB to which all files are compressed to but the total maximum file size is controlled by maxSizeMB, calculated from the original file sizes.

The way to solve the issue is either

  • Allow countries define compressImagesToSizeMB and maximumNumberOfFiles. This would allow you to understand the required disk space for each file upload by calculating compressImagesToSizeMB * maximumNumberOfFiles. Users could then be only restricted from uploading more than maximumNumberOfFiles files. This approach is problematic as in the future we want to upload PDFs for instance which we cannot reliably compress.
  • Define maximum total size for original files. Inform user if this limit is exceeded. In this approach we can compress the files the best we can after the user is finished uploading, but the user is not affected by the outcome. User can understand how many files they can upload based on the file sizes in their file system.

@jpye-finch @Alta-Nel what are your views on this? Think we need to consider the most user-friendly option here.

@manitra
Copy link

manitra commented Nov 8, 2024

Hello @rikukissa,

By the way, can country integrators customize the maxSizeMB value ?
I saw a kind of default value of 20MB in the code.

@euanmillar euanmillar modified the milestones: w IET Candidates, v1.6.1 Nov 11, 2024
@rikukissa rikukissa moved this from Backlog to Completed in OpenCRVS Core Nov 14, 2024
@rikukissa
Copy link
Member

Total file size calculation fixed in #7961
Solution tested by Madagascar team. Merged to 1.6.1 and will be released by it.

@nifaliana
Copy link
Author

Thanks @rikukissa and @tahmidrahman-dsi 🙏 .
Could you let us know the tag once it has been deployed ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Completed
Development

No branches or pull requests

5 participants