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

Limit allowed file size to 2MB for image uploads #178

Merged
merged 2 commits into from
Nov 21, 2024

Conversation

lkeegan
Copy link
Member

@lkeegan lkeegan commented Nov 20, 2024

  • admin-frontend
    • limit upload image file sizes to 2 MB for Milestone and MilestoneGroup images
      • display warning modal and ignore files if too large
  • frontend
    • add WarningModal and ImageFileUpload components
  • deployment
    • set client_max_body_size to 2MB in nginx config
  • backend
    • remove exif orientation tag if present to avoid unexpected rotation of uploaded images in some cases
    • use ImageOps.contain instead of resizing the image
  • resolves Image upload file size limits #165

@codecov-commenter
Copy link

codecov-commenter commented Nov 20, 2024

Codecov Report

Attention: Patch coverage is 21.42857% with 33 lines in your changes missing coverage. Please review.

Project coverage is 29.96%. Comparing base (d7717bd) to head (4440830).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...rc/lib/components/DataInput/ImageFileUpload.svelte 0.00% 23 Missing and 1 partial ⚠️
...ib/components/Admin/EditMilestoneGroupModal.svelte 0.00% 3 Missing ⚠️
...src/lib/components/Admin/EditMilestoneModal.svelte 0.00% 3 Missing ⚠️
frontend/src/lib/components/WarningModal.svelte 0.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
- Coverage   30.04%   29.96%   -0.08%     
==========================================
  Files          99      101       +2     
  Lines        3245     3267      +22     
  Branches       87       89       +2     
==========================================
+ Hits          975      979       +4     
- Misses       2198     2214      +16     
- Partials       72       74       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

- admin-frontend
  - limit upload image file sizes to 2 MB for Milestone and MilestoneGroup images
    - display warning modal and ignore files if too large
- frontend
  - add WarningModal and ImageFileUpload components
- deployment
  - set client_max_body_size to 2MB in nginx config
- backend
  - remove exif orientation tag if present to avoid unexpected rotation of uploaded images in some cases
  - use ImageOps.contain instead of resizing the image
- resolves #165
@lkeegan lkeegan force-pushed the fix_165_image_upload_file_size_limit branch from 0d7743c to f9cbeb6 Compare November 20, 2024 10:44
@lkeegan lkeegan requested a review from MaHaWo November 20, 2024 10:47
Copy link
Collaborator

@MaHaWo MaHaWo left a comment

Choose a reason for hiding this comment

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

lgtm. One minor comment wrt usability. I'm not sure if we should fix such things as they appear, or if we should collect them in a meta issue and work through them in a more focused way?

frontend/src/lib/components/WarningModal.svelte Outdated Show resolved Hide resolved
@lkeegan lkeegan merged commit 579ef10 into main Nov 21, 2024
6 checks passed
@lkeegan lkeegan deleted the fix_165_image_upload_file_size_limit branch November 21, 2024 12:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image upload file size limits
3 participants