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

Conversation

lahdeero
Copy link
Contributor

@lahdeero lahdeero commented Apr 13, 2022

馃帺 What? Why?

Currently image uploads doesn't give any guidance about accepted file resolution, here we add guidance. Also error message when trying to upload image with too big resolution is currently ambiguous: "File too big".

馃搶 Related Issues

Testing

Try to upload images (e.g. identity document, new proposal, hero image)

馃摲 Screenshots

image

鈾ワ笍 Thank you!

@lahdeero lahdeero changed the title Seperate messages for file dimensions and size Seperate validation messages for image dimensions and size Apr 13, 2022
@ahukkanen ahukkanen changed the title Seperate validation messages for image dimensions and size Separate validation messages for image dimensions and size Apr 14, 2022
@andreslucena andreslucena added module: core type: fix PRs that implement a fix for a bug labels Apr 19, 2022
@andreslucena
Copy link
Member

@lahdeero thanks for the PR! A related GitHub tip: if you add the words "Fixes #XXX" or "Closes #XXX" on your PR description or any comment, GH automatically relates this PR with the issue and when it's merged the issue is closed

(I've already done it myself, also removing the checklist from the PR that I think that we should remove as it doesn't bring much value but that's another issue)

@andreslucena
Copy link
Member

Tried it locally and it works as expected 馃憤馃徑

Just a couple small improvements that I think would make it more clear.

Also, I think it'd be nice to also change it in the specs to communicate better to other developers when testing:

~/W/d/decidim (fix/image_validation_messages) [1]> grep -rin "too big" decidim*/spec
decidim-admin/spec/commands/decidim/admin/update_organization_appearance_spec.rb:54:          organization.errors.add(:official_img_header, "Image too big")
decidim-admin/spec/commands/decidim/admin/update_organization_appearance_spec.rb:55:          organization.errors.add(:official_img_footer, "Image too big")
decidim-assemblies/spec/commands/create_assembly_spec.rb:90:            hero_image: "Image too big",
decidim-assemblies/spec/commands/create_assembly_spec.rb:91:            banner_image: "Image too big"
decidim-assemblies/spec/commands/create_assembly_spec.rb:106:        expect(errors).to receive(:add).with(:hero_image, "Image too big")
decidim-assemblies/spec/commands/create_assembly_spec.rb:107:        expect(errors).to receive(:add).with(:banner_image, "Image too big")
decidim-assemblies/spec/commands/update_assembly_spec.rb:131:          my_assembly.errors.add(:hero_image, "Image too big")
decidim-assemblies/spec/commands/update_assembly_spec.rb:132:          my_assembly.errors.add(:banner_image, "Image too big")
decidim-assemblies/spec/forms/assembly_form_spec.rb:169:        context "when attachment (hero_image or banner_image) is too big" do
decidim-conferences/spec/commands/update_conference_spec.rb:109:          my_conference.errors.add(:hero_image, "Image too big")
decidim-conferences/spec/commands/update_conference_spec.rb:110:          my_conference.errors.add(:banner_image, "Image too big")
decidim-conferences/spec/commands/create_conference_spec.rb:83:            hero_image: "Image too big",
decidim-conferences/spec/commands/create_conference_spec.rb:84:            banner_image: "Image too big"
decidim-conferences/spec/commands/create_conference_spec.rb:99:        expect(errors).to receive(:add).with(:hero_image, "Image too big")
decidim-conferences/spec/commands/create_conference_spec.rb:100:        expect(errors).to receive(:add).with(:banner_image, "Image too big")
decidim-conferences/spec/forms/conference_form_spec.rb:90:        context "when attachment (hero_image or banner_image) is too big" do
decidim-consultations/spec/commands/decidim/consultations/admin/create_consultation_spec.rb:52:                banner_image: "Image too big"
decidim-consultations/spec/commands/decidim/consultations/admin/create_consultation_spec.rb:66:            expect(errors).to receive(:add).with(:banner_image, "Image too big")
decidim-consultations/spec/commands/decidim/consultations/admin/update_question_spec.rb:71:            question.errors.add(:banner_image, "Image too big")
decidim-consultations/spec/commands/decidim/consultations/admin/update_question_spec.rb:72:            question.errors.add(:hero_image, "Image too big")
decidim-consultations/spec/commands/decidim/consultations/admin/create_question_spec.rb:59:                banner_image: "Image too big",
decidim-consultations/spec/commands/decidim/consultations/admin/create_question_spec.rb:60:                hero_image: "Image too big"
decidim-consultations/spec/commands/decidim/consultations/admin/update_consultation_spec.rb:67:            consultation.errors.add(:banner_image, "Image too big")
decidim-consultations/spec/forms/decidim/consultations/admin/consultation_form_spec.rb:64:        context "when banner_image is too big" do
decidim-consultations/spec/forms/decidim/consultations/admin/question_form_spec.rb:118:          context "and it is too big" do
decidim-consultations/spec/forms/decidim/consultations/admin/question_form_spec.rb:137:          context "and it is too big" do
decidim-core/spec/models/decidim/editor_image_spec.rb:15:    context "when the file is too big" do
decidim-core/spec/models/decidim/attachment_spec.rb:14:      context "when the file is too big" do
decidim-core/spec/models/decidim/user_spec.rb:152:      context "when the file is too big" do
decidim-core/spec/models/decidim/user_group_spec.rb:67:      context "when the file is too big" do
decidim-core/spec/commands/decidim/update_account_spec.rb:133:      describe "when the avatar dimensions are too big" do
decidim-core/spec/commands/decidim/update_account_spec.rb:134:        let(:message) { "Avatar is too big." }
decidim-core/spec/system/account_spec.rb:45:      it "shows error when image is too big" do
decidim-elections/spec/commands/decidim/votings/admin/update_voting_spec.rb:66:            voting.errors.add(:banner_image, "Image too big")
decidim-elections/spec/forms/decidim/votings/admin/voting_form_spec.rb:120:  context "when banner_image is too big" do
decidim-participatory_processes/spec/commands/create_participatory_process_spec.rb:73:            hero_image: "Image too big",
decidim-participatory_processes/spec/commands/create_participatory_process_spec.rb:74:            banner_image: "Image too big"
decidim-participatory_processes/spec/commands/create_participatory_process_spec.rb:88:        expect(errors).to receive(:add).with(:hero_image, "Image too big")
decidim-participatory_processes/spec/commands/create_participatory_process_spec.rb:89:        expect(errors).to receive(:add).with(:banner_image, "Image too big")
decidim-participatory_processes/spec/commands/update_participatory_process_spec.rb:82:          my_process.errors.add(:hero_image, "Image too big")
decidim-participatory_processes/spec/commands/update_participatory_process_spec.rb:83:          my_process.errors.add(:banner_image, "Image too big")
decidim-participatory_processes/spec/forms/participatory_process_form_spec.rb:74:        context "when banner_image is too big" do
decidim-participatory_processes/spec/forms/participatory_process_group_form_spec.rb:64:        context "when hero_image is too big" do

What do you think @lahdeero?

@lahdeero
Copy link
Contributor Author

if you add the words "Fixes #XXX" or "Closes #XXX" on your PR description or any comment, GH automatically relates this PR with the issue and when it's merged the issue is closed

Every day I learn something new! I committed also other suggestions you made @andreslucena.

Copy link
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the PR @lahdeero 馃憦馃徑 馃憦馃徑

@ahukkanen ahukkanen merged commit c7304d9 into decidim:develop Apr 29, 2022
@ahukkanen ahukkanen deleted the fix/image_validation_messages branch April 29, 2022 13:47
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: core type: fix PRs that implement a fix for a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"File too big" validation is given when image has correct file size
4 participants