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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate validation messages for image dimensions and size #9165

Merged
merged 5 commits into from
Apr 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,8 @@ module Decidim::Admin
before do
expect(form).to receive(:invalid?).and_return(false)
expect(organization).to receive(:valid?).at_least(:once).and_return(false)
organization.errors.add(:official_img_header, "Image too big")
organization.errors.add(:official_img_footer, "Image too big")
organization.errors.add(:official_img_header, "File resolution is too large")
organization.errors.add(:official_img_footer, "File resolution is too large")
end

it "broadcasts invalid" do
Expand Down
10 changes: 5 additions & 5 deletions decidim-assemblies/spec/commands/create_assembly_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -87,8 +87,8 @@ module Decidim::Assemblies
persisted?: false,
valid?: false,
errors: {
hero_image: "Image too big",
banner_image: "Image too big"
hero_image: "File resolution is too large",
banner_image: "File resolution is too large"
}
).as_null_object
end
Expand All @@ -103,8 +103,8 @@ module Decidim::Assemblies
end

it "adds errors to the form" do
expect(errors).to receive(:add).with(:hero_image, "Image too big")
expect(errors).to receive(:add).with(:banner_image, "Image too big")
expect(errors).to receive(:add).with(:hero_image, "File resolution is too large")
expect(errors).to receive(:add).with(:banner_image, "File resolution is too large")
subject.call
end
end
Expand Down Expand Up @@ -137,7 +137,7 @@ module Decidim::Assemblies

it "broadcasts invalid" do
expect { subject.call }.to broadcast(:invalid)
expect(form.errors.messages[:hero_image]).to contain_exactly("The image is too big")
expect(form.errors.messages[:hero_image]).to contain_exactly("File resolution is too large")
end
end

Expand Down
6 changes: 3 additions & 3 deletions decidim-assemblies/spec/commands/update_assembly_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -120,16 +120,16 @@ module Decidim::Assemblies

it "broadcasts invalid" do
expect { command.call }.to broadcast(:invalid)
expect(form.errors.messages[:hero_image]).to contain_exactly("The image is too big")
expect(form.errors.messages[:hero_image]).to contain_exactly("File resolution is too large")
end
end

describe "when the assembly is not valid" do
before do
expect(form).to receive(:invalid?).and_return(false)
expect(my_assembly).to receive(:valid?).at_least(:once).and_return(false)
my_assembly.errors.add(:hero_image, "Image too big")
my_assembly.errors.add(:banner_image, "Image too big")
my_assembly.errors.add(:hero_image, "File resolution is too large")
my_assembly.errors.add(:banner_image, "File resolution is too large")
end

it "broadcasts invalid" do
Expand Down
8 changes: 4 additions & 4 deletions decidim-conferences/spec/commands/create_conference_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ module Decidim::Conferences
persisted?: false,
valid?: false,
errors: {
hero_image: "Image too big",
banner_image: "Image too big"
hero_image: "File resolution is too large",
banner_image: "File resolution is too large"
}
).as_null_object
end
Expand All @@ -96,8 +96,8 @@ module Decidim::Conferences
end

it "adds errors to the form" do
expect(errors).to receive(:add).with(:hero_image, "Image too big")
expect(errors).to receive(:add).with(:banner_image, "Image too big")
expect(errors).to receive(:add).with(:hero_image, "File resolution is too large")
expect(errors).to receive(:add).with(:banner_image, "File resolution is too large")
subject.call
end
end
Expand Down
4 changes: 2 additions & 2 deletions decidim-conferences/spec/commands/update_conference_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,8 @@ module Decidim::Conferences
before do
expect(form).to receive(:invalid?).and_return(false)
expect(my_conference).to receive(:valid?).at_least(:once).and_return(false)
my_conference.errors.add(:hero_image, "Image too big")
my_conference.errors.add(:banner_image, "Image too big")
my_conference.errors.add(:hero_image, "File resolution is too large")
my_conference.errors.add(:banner_image, "File resolution is too large")
end

it "broadcasts invalid" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ module Admin
persisted?: false,
valid?: false,
errors: {
banner_image: "Image too big"
banner_image: "File resolution is too large"
}
).as_null_object
end
Expand All @@ -63,7 +63,7 @@ module Admin
end

it "adds errors to the form" do
expect(errors).to receive(:add).with(:banner_image, "Image too big")
expect(errors).to receive(:add).with(:banner_image, "File resolution is too large")
subject.call
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,8 @@ module Admin
persisted?: false,
valid?: false,
errors: {
banner_image: "Image too big",
hero_image: "Image too big"
banner_image: "File resolution is too large",
hero_image: "File resolution is too large"
}
).as_null_object
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ module Admin
before do
expect(form).to receive(:invalid?).and_return(false)
expect(consultation).to receive(:valid?).at_least(:once).and_return(false)
consultation.errors.add(:banner_image, "Image too big")
consultation.errors.add(:banner_image, "File resolution is too large")
end

it "broadcasts invalid" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,8 @@ module Admin
before do
expect(form).to receive(:invalid?).and_return(false)
expect(question).to receive(:valid?).at_least(:once).and_return(false)
question.errors.add(:banner_image, "Image too big")
question.errors.add(:hero_image, "Image too big")
question.errors.add(:banner_image, "File resolution is too large")
question.errors.add(:hero_image, "File resolution is too large")
end

it "broadcasts invalid" do
Expand Down
2 changes: 1 addition & 1 deletion decidim-core/app/cells/decidim/upload_modal/modal.erb
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
locales: {
error: t("decidim.forms.upload.labels.error"),
title_required: t("decidim.forms.upload.labels.title_required"),
file_too_big: t("decidim.forms.upload.labels.file_too_big", megabytes: (max_file_size_mb)),
file_size_too_large: t("decidim.forms.upload.labels.file_size_too_large", megabytes: (max_file_size_mb)),
remove: t("decidim.forms.upload.labels.remove"),
title: t("decidim.forms.upload.labels.title"),
uploaded: t("decidim.forms.upload.labels.uploaded"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export class Uploader {
this.fileTooBig = false;
if (modal.options.maxFileSize && options.file.size > modal.options.maxFileSize) {
this.fileTooBig = true;
this.showError([modal.locales.file_too_big]);
this.showError([modal.locales.file_size_too_large]);
} else {
this.upload = new DirectUpload(options.file, options.url, this);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ def validate_dimensions
return unless image?(self)

manipulate! do |image|
raise CarrierWave::IntegrityError, I18n.t("carrierwave.errors.image_too_big") if image.dimensions.any? { |dimension| dimension > max_image_height_or_width }
raise CarrierWave::IntegrityError, I18n.t("carrierwave.errors.file_resolution_too_large") if image.dimensions.any? { |dimension| dimension > max_image_height_or_width }

image
end
Expand Down
4 changes: 2 additions & 2 deletions decidim-core/app/uploaders/decidim/cw/image_uploader.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ def extension_allowlist
# See https://hackerone.com/reports/390
def validate_dimensions
manipulate! do |image|
validation_error!(I18n.t("carrierwave.errors.image_too_big")) if image.dimensions.any? { |dimension| dimension > max_image_height_or_width }
validation_error!(I18n.t("carrierwave.errors.file_resolution_too_large")) if image.dimensions.any? { |dimension| dimension > max_image_height_or_width }
image
end
end

def validate_size
manipulate! do |image|
validation_error!(I18n.t("carrierwave.errors.image_too_big")) if image.size > maximum_upload_size
validation_error!(I18n.t("carrierwave.errors.file_size_too_large")) if image.size > maximum_upload_size
image
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def validate_image_size(record, attribute, file, uploader)
return unless uploader.validable_dimensions
return if (image = extract_image(file)).blank?

record.errors.add attribute, I18n.t("carrierwave.errors.image_too_big") if image.dimensions.any? { |dimension| dimension > uploader.max_image_height_or_width }
record.errors.add attribute, I18n.t("carrierwave.errors.file_resolution_too_large") if image.dimensions.any? { |dimension| dimension > uploader.max_image_height_or_width }
end

def extract_image(file)
Expand Down
6 changes: 4 additions & 2 deletions decidim-core/config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,8 @@ en:
'true': 'Yes'
carrierwave:
errors:
image_too_big: The image is too big
file_resolution_too_large: File resolution is too large
file_size_too_large: File size is too large
not_inside_organization: The file is not attached to any organization
date:
formats:
Expand Down Expand Up @@ -771,6 +772,7 @@ en:
message_2: The service crops the image.
file_validation:
allowed_file_extensions: 'Allowed file extensions: %{extensions}'
max_file_dimension: 'Maximum file dimensions: %{resolution} pixels'
max_file_size: 'Maximum file size: %{megabytes}MB'
upload:
labels:
Expand All @@ -780,7 +782,7 @@ en:
cancel: Cancel
edit_image: Edit image
error: Error!
file_too_big: 'File is too big! Maximun file size: %{megabytes}MB'
file_size_too_large: 'File size is too large! Maximun file size: %{megabytes}MB'
remove: Remove
replace: Replace
save: Save
Expand Down
14 changes: 14 additions & 0 deletions decidim-core/lib/decidim/file_validator_humanizer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ def messages
)
end

if (file_dimensions = max_file_dimensions)
messages << I18n.t(
"max_file_dimension",
resolution: file_dimensions,
scope: "decidim.forms.file_validation"
)
end

if (extensions = extension_allowlist)
messages << I18n.t(
"allowed_file_extensions",
Expand Down Expand Up @@ -80,6 +88,12 @@ def max_file_size
end
# rubocop: enable Metrics/CyclomaticComplexity

def max_file_dimensions
return unless uploader.respond_to?(:max_image_height_or_width)

"#{uploader.max_image_height_or_width}x#{uploader.max_image_height_or_width}"
end

def extension_allowlist
return unless uploader.respond_to?(:extension_allowlist, true)

Expand Down
2 changes: 1 addition & 1 deletion decidim-core/spec/system/account_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@
input_element = find("input[type='file']", visible: :all)
input_element.attach_file(Decidim::Dev.asset("5000x5000.png"))

expect(page).to have_content("The image is too big", count: 1)
expect(page).to have_content("File resolution is too large", count: 1)
expect(page).to have_css(".upload-errors .form-error", count: 1)
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ module Admin
before do
expect(form).to receive(:invalid?).and_return(false)
expect(voting).to receive(:valid?).at_least(:once).and_return(false)
voting.errors.add(:banner_image, "Image too big")
voting.errors.add(:banner_image, "File resolution is too large")
end

it "broadcasts invalid" do
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ module Decidim::ParticipatoryProcesses
persisted?: false,
valid?: false,
errors: {
hero_image: "Image too big",
banner_image: "Image too big"
hero_image: "File resolution is too large",
banner_image: "File resolution is too large"
}
).as_null_object
end
Expand All @@ -85,8 +85,8 @@ module Decidim::ParticipatoryProcesses
end

it "adds errors to the form" do
expect(errors).to receive(:add).with(:hero_image, "Image too big")
expect(errors).to receive(:add).with(:banner_image, "Image too big")
expect(errors).to receive(:add).with(:hero_image, "File resolution is too large")
expect(errors).to receive(:add).with(:banner_image, "File resolution is too large")
subject.call
end
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,8 @@ module Decidim::ParticipatoryProcesses
before do
expect(form).to receive(:invalid?).and_return(false)
expect(my_process).to receive(:valid?).at_least(:once).and_return(false)
my_process.errors.add(:hero_image, "Image too big")
my_process.errors.add(:banner_image, "Image too big")
my_process.errors.add(:hero_image, "File resolution is too large")
my_process.errors.add(:banner_image, "File resolution is too large")
end

it "broadcasts invalid" do
Expand Down