FEATURE: Humanize file size error messages (#14398)

The file size error messages for max_image_size_kb and
max_attachment_size_kb are shown to the user in the KB
format, regardless of how large the limit is. Since we
are going to support uploading much larger files soon,
this KB-based limit soon becomes unfriendly to the end
user.

For example, if the max attachment size is set to 512000
KB, this is what the user sees:

> Sorry, the file you are trying to upload is too big (maximum
size is 512000KB)

This makes the user do math. In almost all file explorers that
a regular user would be familiar width, the file size is shown
in a format based on the maximum increment (e.g. KB, MB, GB).

This commit changes the behaviour to output a humanized file size
instead of the raw KB. For the above example, it would now say:

> Sorry, the file you are trying to upload is too big (maximum
size is 512 MB)

This humanization also handles decimals, e.g. 1536KB = 1.5 MB
This commit is contained in:
Martin Brennan 2021-09-22 07:59:45 +10:00 committed by GitHub
parent 3cda7ec7b9
commit dba6a5eabf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 78 additions and 10 deletions

View File

@ -328,7 +328,11 @@ function displayErrorByResponseStatus(status, body, fileName, siteSettings) {
case 413: case 413:
const type = uploadTypeFromFileName(fileName); const type = uploadTypeFromFileName(fileName);
const max_size_kb = siteSettings[`max_${type}_size_kb`]; const max_size_kb = siteSettings[`max_${type}_size_kb`];
bootbox.alert(I18n.t("post.errors.file_too_large", { max_size_kb })); bootbox.alert(
I18n.t("post.errors.file_too_large_humanized", {
max_size: formatBytes(max_size_kb * 1024),
})
);
return true; return true;
// the error message is provided by the server // the error message is provided by the server
@ -354,3 +358,18 @@ export function bindFileInputChangeListener(element, fileCallbackFn) {
element.addEventListener("change", changeListener); element.addEventListener("change", changeListener);
return changeListener; return changeListener;
} }
export function formatBytes(bytes, decimals) {
if (bytes === 0) {
return "0 Bytes";
}
const kilobyte = 1024,
dm = decimals || 2,
sizes = ["Bytes", "KB", "MB", "GB", "TB", "PB", "EB", "ZB", "YB"],
incr = Math.floor(Math.log(bytes) / Math.log(kilobyte));
return (
parseFloat((bytes / Math.pow(kilobyte, incr)).toFixed(dm)) +
" " +
sizes[incr]
);
}

View File

@ -219,7 +219,7 @@ class UploadsController < ApplicationController
if file_size_too_big?(file_name, file_size) if file_size_too_big?(file_name, file_size)
return render_json_error( return render_json_error(
I18n.t("upload.attachments.too_large", max_size_kb: SiteSetting.max_attachment_size_kb), I18n.t("upload.attachments.too_large_humanized", max_size: ActiveSupport::NumberHelper.number_to_human_size(SiteSetting.max_attachment_size_kb.kilobytes)),
status: 422 status: 422
) )
end end
@ -296,7 +296,7 @@ class UploadsController < ApplicationController
if file_size_too_big?(file_name, file_size) if file_size_too_big?(file_name, file_size)
return render_json_error( return render_json_error(
I18n.t("upload.attachments.too_large", max_size_kb: SiteSetting.max_attachment_size_kb), I18n.t("upload.attachments.too_large_humanized", max_size: ActiveSupport::NumberHelper.number_to_human_size(SiteSetting.max_attachment_size_kb.kilobytes)),
status: 422 status: 422
) )
end end

View File

@ -3052,6 +3052,7 @@ en:
edit: "Sorry, there was an error editing your post. Please try again." edit: "Sorry, there was an error editing your post. Please try again."
upload: "Sorry, there was an error uploading that file. Please try again." upload: "Sorry, there was an error uploading that file. Please try again."
file_too_large: "Sorry, that file is too big (maximum size is %{max_size_kb}kb). Why not upload your large file to a cloud sharing service, then paste the link?" file_too_large: "Sorry, that file is too big (maximum size is %{max_size_kb}kb). Why not upload your large file to a cloud sharing service, then paste the link?"
file_too_large_humanized: "Sorry, that file is too big (maximum size is %{max_size}). Why not upload your large file to a cloud sharing service, then paste the link?"
too_many_uploads: "Sorry, you can only upload one file at a time." too_many_uploads: "Sorry, you can only upload one file at a time."
too_many_dragged_and_dropped_files: too_many_dragged_and_dropped_files:
one: "Sorry, you can only upload %{count} file at a time." one: "Sorry, you can only upload %{count} file at a time."

View File

@ -4039,12 +4039,15 @@ en:
cannot_promote_failure: "The upload cannot be completed, it may have already completed or previously failed." cannot_promote_failure: "The upload cannot be completed, it may have already completed or previously failed."
attachments: attachments:
too_large: "Sorry, the file you are trying to upload is too big (maximum size is %{max_size_kb}KB)." too_large: "Sorry, the file you are trying to upload is too big (maximum size is %{max_size_kb}KB)."
too_large_humanized: "Sorry, the file you are trying to upload is too big (maximum size is %{max_size})."
images: images:
too_large: "Sorry, the image you are trying to upload is too big (maximum size is %{max_size_kb}KB), please resize it and try again." too_large: "Sorry, the image you are trying to upload is too big (maximum size is %{max_size_kb}KB), please resize it and try again."
too_large_humanized: "Sorry, the image you are trying to upload is too big (maximum size is %{max_size}), please resize it and try again."
larger_than_x_megapixels: "Sorry, the image you are trying to upload is too large (maximum dimension is %{max_image_megapixels}-megapixels), please resize it and try again." larger_than_x_megapixels: "Sorry, the image you are trying to upload is too large (maximum dimension is %{max_image_megapixels}-megapixels), please resize it and try again."
size_not_found: "Sorry, but we couldn't determine the size of the image. Maybe your image is corrupted?" size_not_found: "Sorry, but we couldn't determine the size of the image. Maybe your image is corrupted?"
placeholders: placeholders:
too_large: "(image larger than %{max_size_kb}KB)" too_large: "(image larger than %{max_size_kb}KB)"
too_large_humanized: "(image larger than %{max_size})"
avatar: avatar:
missing: "Sorry, we can't find any avatar associated with that email address. Can you try uploading it again?" missing: "Sorry, we can't find any avatar associated with that email address. Can you try uploading it again?"

View File

@ -142,7 +142,15 @@ class CookedPostProcessor
span = create_span_node("url", url) span = create_span_node("url", url)
a.add_child(span) a.add_child(span)
span.add_previous_sibling(create_icon_node("far-image")) span.add_previous_sibling(create_icon_node("far-image"))
span.add_next_sibling(create_span_node("help", I18n.t("upload.placeholders.too_large", max_size_kb: SiteSetting.max_image_size_kb))) span.add_next_sibling(
create_span_node(
"help",
I18n.t(
"upload.placeholders.too_large_humanized",
max_size: ActiveSupport::NumberHelper.number_to_human_size(SiteSetting.max_image_size_kb.kilobytes)
)
)
)
# Only if the image is already linked # Only if the image is already linked
if is_hyperlinked if is_hyperlinked

View File

@ -386,7 +386,9 @@ class UploadCreator
@upload.errors.add(:base, I18n.t("upload.images.larger_than_x_megapixels", max_image_megapixels: SiteSetting.max_image_megapixels)) @upload.errors.add(:base, I18n.t("upload.images.larger_than_x_megapixels", max_image_megapixels: SiteSetting.max_image_megapixels))
true true
elsif max_image_size > 0 && filesize >= max_image_size elsif max_image_size > 0 && filesize >= max_image_size
@upload.errors.add(:base, I18n.t("upload.images.too_large", max_size_kb: SiteSetting.max_image_size_kb)) @upload.errors.add(:base, I18n.t(
"upload.images.too_large_humanized", max_size: ActiveSupport::NumberHelper.number_to_human_size(max_image_size)
))
true true
else else
false false

View File

@ -138,7 +138,10 @@ class UploadValidator < ActiveModel::Validator
max_size_bytes = max_size_kb.kilobytes max_size_bytes = max_size_kb.kilobytes
if upload.filesize > max_size_bytes if upload.filesize > max_size_bytes
message = I18n.t("upload.#{type}s.too_large", max_size_kb: max_size_kb) message = I18n.t(
"upload.#{type}s.too_large_humanized",
max_size: ActiveSupport::NumberHelper.number_to_human_size(max_size_bytes)
)
upload.errors.add(:filesize, message) upload.errors.add(:filesize, message)
end end
end end

View File

@ -1120,6 +1120,7 @@ describe CookedPostProcessor do
end end
it "replaces large image placeholder" do it "replaces large image placeholder" do
SiteSetting.max_image_size_kb = 4096
url = 'https://image.com/my-avatar' url = 'https://image.com/my-avatar'
image_url = 'https://image.com/avatar.png' image_url = 'https://image.com/avatar.png'
@ -1134,6 +1135,7 @@ describe CookedPostProcessor do
cpp.post_process cpp.post_process
expect(cpp.doc.to_s).to match(/<div class="large-image-placeholder">/) expect(cpp.doc.to_s).to match(/<div class="large-image-placeholder">/)
expect(cpp.doc.to_s).to include(I18n.t("upload.placeholders.too_large_humanized", max_size: "4 MB"))
end end
end end

View File

@ -31,6 +31,21 @@ describe UploadValidator do
expect(UploadCreator.new(csv_file, "#{filename}.zip", for_export: true).create_for(user.id)).to be_valid expect(UploadCreator.new(csv_file, "#{filename}.zip", for_export: true).create_for(user.id)).to be_valid
end end
describe "size validation" do
it "does not allow images that are too large" do
SiteSetting.max_image_size_kb = 1536
upload = Fabricate.build(:upload,
user: Fabricate(:admin),
original_filename: "test.png",
filesize: 2097152
)
subject.validate(upload)
expect(upload.errors.full_messages.first).to eq(
"Filesize #{I18n.t("upload.images.too_large_humanized", max_size: "1.5 MB")}"
)
end
end
describe 'when allow_staff_to_upload_any_file_in_pm is true' do describe 'when allow_staff_to_upload_any_file_in_pm is true' do
it 'should allow uploads for pm' do it 'should allow uploads for pm' do
upload = Fabricate.build(:upload, upload = Fabricate.build(:upload,

View File

@ -114,6 +114,21 @@ RSpec.describe UploadCreator do
end end
end end
context "when image is too big" do
let(:filename) { 'logo.png' }
let(:file) { file_from_fixtures(filename) }
it "adds an error to the upload" do
SiteSetting.max_image_size_kb = 1
upload = UploadCreator.new(
file, filename, force_optimize: true
).create_for(Discourse.system_user.id)
expect(upload.errors.full_messages.first).to eq(
"#{I18n.t("upload.images.too_large_humanized", max_size: "1 KB")}"
)
end
end
describe 'pngquant' do describe 'pngquant' do
let(:filename) { "pngquant.png" } let(:filename) { "pngquant.png" }
let(:file) { file_from_fixtures(filename) } let(:file) { file_from_fixtures(filename) }

View File

@ -122,7 +122,7 @@ describe UploadsController do
expect(response.status).to eq(422) expect(response.status).to eq(422)
expect(Jobs::CreateAvatarThumbnails.jobs.size).to eq(0) expect(Jobs::CreateAvatarThumbnails.jobs.size).to eq(0)
errors = response.parsed_body["errors"] errors = response.parsed_body["errors"]
expect(errors.first).to eq(I18n.t("upload.attachments.too_large", max_size_kb: 1)) expect(errors.first).to eq(I18n.t("upload.attachments.too_large_humanized", max_size: "1 KB"))
end end
it 'ensures allow_uploaded_avatars is enabled when uploading an avatar' do it 'ensures allow_uploaded_avatars is enabled when uploading an avatar' do
@ -817,7 +817,7 @@ describe UploadsController do
end end
it "returns 422 when the file is an attachment and it's too big" do it "returns 422 when the file is an attachment and it's too big" do
SiteSetting.max_attachment_size_kb = 1000 SiteSetting.max_attachment_size_kb = 1024
post "/uploads/create-multipart.json", { post "/uploads/create-multipart.json", {
params: { params: {
file_name: "test.zip", file_name: "test.zip",
@ -826,7 +826,7 @@ describe UploadsController do
} }
} }
expect(response.status).to eq(422) expect(response.status).to eq(422)
expect(response.body).to include(I18n.t("upload.attachments.too_large", max_size_kb: SiteSetting.max_attachment_size_kb)) expect(response.body).to include(I18n.t("upload.attachments.too_large_humanized", max_size: "1 MB"))
end end
def stub_create_multipart_request def stub_create_multipart_request

View File

@ -89,7 +89,7 @@ RSpec.describe ExternalUploadManager do
SiteSetting.max_image_size_kb = 1 SiteSetting.max_image_size_kb = 1
upload = subject.promote_to_upload! upload = subject.promote_to_upload!
expect(upload.errors.full_messages).to include( expect(upload.errors.full_messages).to include(
"Filesize " + I18n.t("upload.images.too_large", max_size_kb: SiteSetting.max_image_size_kb) "Filesize " + I18n.t("upload.images.too_large_humanized", max_size: ActiveSupport::NumberHelper.number_to_human_size(SiteSetting.max_image_size_kb.kilobytes))
) )
end end