Skip to content

FEATURE: automatically downsize large images #10

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.

Sign up for GitHub

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

Open
ShashankFC wants to merge 1 commit into
base: image-processing-optimization
Choose a base branch
from
Open

FEATURE: automatically downsize large images #10

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions app/assets/javascripts/discourse/lib/utilities.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ Discourse.Utilities = {

// check file size
var fileSizeKB = file.size / 1024;
var maxSizeKB = Discourse.SiteSettings['max_' + type + '_size_kb'];
var maxSizeKB = 10 * 1024; // 10MB

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

Hardcoding the 10MB limit in multiple places (lines 182 and 246) creates a magic number that's duplicated across the codebase. Consider defining this as a constant at the module level to ensure consistency and ease maintenance.

Copilot uses AI. Check for mistakes.
if (fileSizeKB > maxSizeKB) {
bootbox.alert(I18n.t('post.errors.file_too_large', { max_size_kb: maxSizeKB }));
return false;
Expand Down Expand Up @@ -243,7 +243,7 @@ Discourse.Utilities = {

// entity too large, usually returned from the web server
case 413:
var maxSizeKB = Discourse.SiteSettings.max_image_size_kb;
var maxSizeKB = 10 * 1024; // 10 MB
bootbox.alert(I18n.t('post.errors.file_too_large', { max_size_kb: maxSizeKB }));
return;

Expand Down
11 changes: 10 additions & 1 deletion app/controllers/uploads_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,23 @@ def create_upload(type, file, url)
begin
# API can provide a URL
if file.nil? && url.present? && is_api?
tempfile = FileHelper.download(url, SiteSetting.max_image_size_kb.kilobytes, "discourse-upload-#{type}") rescue nil
tempfile = FileHelper.download(url, 10.megabytes, "discourse-upload-#{type}") rescue nil
filename = File.basename(URI.parse(url).path)
else
tempfile = file.tempfile
filename = file.original_filename
content_type = file.content_type
end

# 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

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The magic number 5 for retry attempts lacks explanation. Consider extracting this to a named constant (e.g., MAX_DOWNSIZE_ATTEMPTS = 5) to improve code clarity and maintainability.

Copilot uses AI. Check for mistakes.
while attempt > 0 && tempfile.size > SiteSetting.max_image_size_kb.kilobytes
OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails)
attempt -= 1
end
end

upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type)
Comment on lines +64 to 72

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

The downsizing loop does not refresh tempfile.size after each iteration. Since the method writes to the same file path, tempfile.size will remain unchanged, creating an infinite loop until attempt reaches 0. Consider using File.size(tempfile.path) to get the updated file size after each downsizing operation.

Suggested change
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)
attempt -= 1
end
end
upload = Upload.create_for(current_user.id, tempfile, filename, tempfile.size, content_type: content_type, image_type: type)
current_size = tempfile.size if tempfile
if tempfile && current_size.to_i > 0 && SiteSetting.max_image_size_kb > 0 && FileHelper.is_image?(filename)
attempt = 5
while attempt > 0 && current_size > SiteSetting.max_image_size_kb.kilobytes
OptimizedImage.downsize(tempfile.path, tempfile.path, "80%", allow_animation: SiteSetting.allow_animated_thumbnails)
current_size = File.size(tempfile.path)
attempt -= 1
end
end
upload = Upload.create_for(current_user.id, tempfile, filename, (current_size || tempfile.size), content_type: content_type, image_type: type)

Copilot uses AI. Check for mistakes.

if upload.errors.empty? && current_user.admin?
Expand Down
17 changes: 8 additions & 9 deletions app/models/optimized_image.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,25 +139,24 @@ def self.downsize_instructions_animated(from, to, dimensions, opts={})
end

def self.resize(from, to, width, height, opts={})
optimize("resize", from, to, width, height, opts)
optimize("resize", from, to, "#{width}x#{height}", opts)
end

def self.downsize(from, to, max_width, max_height, opts={})
optimize("downsize", from, to, max_width, max_height, opts)
optimize("downsize", from, to, "#{max_width}x#{max_height}", opts)
end

def self.optimize(operation, from, to, width, height, opts={})
dim = dimensions(width, height)
def self.downsize(from, to, dimensions, opts={})
optimize("downsize", from, to, dimensions, opts)
end
Comment on lines +149 to +151

Copilot AI Jan 30, 2026

Copy link

Choose a reason for hiding this comment

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

Method overloading is not supported in Ruby. Having two downsize methods with different arities will cause the second definition (lines 149-151) to completely replace the first one (lines 145-147). Callers using the 4-parameter signature will encounter ArgumentError. Consider renaming one method or using optional parameters with different default values.

Copilot uses AI. Check for mistakes.

def self.optimize(operation, from, to, dimensions, opts={})
method_name = "#{operation}_instructions"
method_name += "_animated" if !!opts[:allow_animation] && from =~ /\.GIF$/i
instructions = self.send(method_name.to_sym, from, to, dim, opts)
instructions = self.send(method_name.to_sym, from, to, dimensions, opts)
convert_with(instructions, to)
end

def self.dimensions(width, height)
"#{width}x#{height}"
end

def self.convert_with(instructions, to)
`#{instructions.join(" ")} &> /dev/null`
return false if $?.exitstatus != 0
Expand Down