FEATURE: automatically downsize large images#1
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements automatic downsizing of large images during upload, allowing users to upload images larger than the configured limit which will be automatically reduced to an acceptable size.
- Modifies the
OptimizedImageclass to accept percentage-based dimensions for downsizing operations - Adds automatic image downsizing logic in the uploads controller with retry mechanism
- Updates JavaScript file size validation to allow larger uploads (10MB) on the client side
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| app/models/optimized_image.rb | Refactors dimension handling to support percentage-based downsizing and removes duplicate method |
| app/controllers/uploads_controller.rb | Adds automatic image downsizing loop with 80% reduction per attempt |
| app/assets/javascripts/discourse/lib/utilities.js | Increases client-side file size limit to 10MB for upload validation |
|
|
||
| def self.optimize(operation, from, to, width, height, opts={}) | ||
| dim = dimensions(width, height) | ||
| def self.downsize(from, to, dimensions, opts={}) |
There was a problem hiding this comment.
Method downsize is duplicated. There are now two methods with the same name but different parameter signatures, which will cause the second definition to override the first one.
| if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) | ||
| attempt = 5 | ||
| while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes | ||
| OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) |
There was a problem hiding this comment.
The downsize method call is missing the opts parameter. Based on the method signature, it should be OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", { allow_animation: SiteSetting.allow_animated_thumbnails }).
| OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) | |
| OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", { allow_animation: SiteSetting.allow_animated_thumbnails }) |
|
|
||
| # allow users to upload large images that will be automatically reduced to allowed size | ||
| if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) | ||
| attempt = 5 |
There was a problem hiding this comment.
[nitpick] The magic number 5 should be extracted to a constant or configuration setting to make the retry limit configurable and self-documenting.
| attempt = 5 | |
| attempt = MAX_RETRY_ATTEMPTS |
| if tempfile && tempfile.size > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename) | ||
| attempt = 5 | ||
| while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes | ||
| OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) |
There was a problem hiding this comment.
[nitpick] The percentage value "80%" is hardcoded. Consider making this configurable as different image types or use cases might benefit from different downsizing ratios.
| OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails) | |
| OptimizedImage.downsize(tempfile.path, tempfile.path, SiteSetting.image_downsize_ratio, allow_animation: SiteSetting.allow_animated_thumbnails) |
|
This pull request has been automatically marked as stale because it has been open for 60 days with no activity. To keep it open, remove the stale tag, push code, or add a comment. Otherwise, it will be closed in 14 days. |
Test 1