From fb98cb61ac270ff8eab0409afc314159f7f8e039 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 15 Nov 2024 13:03:30 +0100 Subject: [PATCH 01/14] Extract budgets from shared nested imageable specs to system specs Removed `imageable_path_arguments`, `has_many_images`, and `management` parameters because they are not used by budgets. Hardcoded `path`, `fill_resource_method_name`, `submit_button`, and `imageable_success_notice`parameters for budgets. These remain fixed for now until dynamic values are required in the next commits. --- spec/support/common_actions/images.rb | 21 --- spec/system/admin/budgets_spec.rb | 8 -- spec/system/nested_imageable_spec.rb | 183 ++++++++++++++++++++++++++ 3 files changed, 183 insertions(+), 29 deletions(-) create mode 100644 spec/system/nested_imageable_spec.rb diff --git a/spec/support/common_actions/images.rb b/spec/support/common_actions/images.rb index 33a3d5aa3..20682d95a 100644 --- a/spec/support/common_actions/images.rb +++ b/spec/support/common_actions/images.rb @@ -1,13 +1,4 @@ module Images - def imageable_redirected_to_resource_show_or_navigate_to(imageable) - case imageable.class.to_s - when "Budget" - visit edit_admin_budget_path(Budget.last) - when "Proposal" - click_link "Not now, go to my proposal" rescue Capybara::ElementNotFound - end - end - def imageable_attach_new_file(path, success = true) click_link "Add image" within "#nested-image" do @@ -38,16 +29,4 @@ module Images fill_in_ckeditor "Description", with: "Budget investment description" check :budget_investment_terms_of_service end - - def expect_image_has_title(title) - image = find(".image-fields") - - within image do - expect(find("input[name$='[title]']").value).to eq title - end - end - - def show_caption_for?(imageable_factory_name) - imageable_factory_name != "budget" - end end diff --git a/spec/system/admin/budgets_spec.rb b/spec/system/admin/budgets_spec.rb index 82d017138..e150448d7 100644 --- a/spec/system/admin/budgets_spec.rb +++ b/spec/system/admin/budgets_spec.rb @@ -1,14 +1,6 @@ require "rails_helper" describe "Admin budgets", :admin do - it_behaves_like "nested imageable", - "budget", - "new_admin_budgets_wizard_budget_path", - {}, - "imageable_fill_new_valid_budget", - "Continue to groups", - "New participatory budget created successfully!" - context "Load" do before { create(:budget, slug: "budget_slug") } diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb new file mode 100644 index 000000000..bb2f2ac0a --- /dev/null +++ b/spec/system/nested_imageable_spec.rb @@ -0,0 +1,183 @@ +require "rails_helper" + +describe "Nested imageable" do + factories = [ + :budget + ] + + let(:factory) { factories.sample } + let!(:imageable) { create(factory) } + let!(:user) { create(:administrator).user } + let(:path) { new_admin_budgets_wizard_budget_path } + let(:submit_button_text) { "Continue to groups" } + let(:notice_text) { "New participatory budget created successfully!" } + + before do + login_as(user) + visit path + end + + scenario "Should show new image link when imageable has not an associated image defined" do + expect(page).to have_css "#new_image_link" + end + + scenario "Should hide new image link after adding one image" do + click_link "Add image" + + expect(page).not_to have_css "#new_image_link" + end + + scenario "Should update image file name after choosing any file" do + click_link "Add image" + attach_file "Choose image", file_fixture("clippy.jpg") + + expect(page).to have_css ".file-name", text: "clippy.jpg" + end + + scenario "Should update image file title after choosing a file when no title is defined" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + + expect_image_has_title("clippy.jpg") + end + + scenario "Should not update image file title after choosing a file when a title is already defined" do + click_link "Add image" + input_title = find(".image-fields input[name$='[title]']") + fill_in input_title[:id], with: "Title" + attach_file "Choose image", file_fixture("clippy.jpg") + + expect(find("##{factory}_image_attributes_title").value).to eq "Title" + end + + scenario "Should update loading bar style after valid file upload" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + + expect(page).to have_css ".loading-bar.complete" + end + + scenario "Should update loading bar style after invalid file upload" do + imageable_attach_new_file(file_fixture("logo_header.png"), false) + + expect(page).to have_css ".loading-bar.errors" + end + + scenario "Should update image cached_attachment field after valid file upload" do + click_link "Add image" + + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty + + attach_file "Choose image", file_fixture("clippy.jpg") + + expect(page).to have_css(".loading-bar.complete") + expect(cached_attachment_field.value).not_to be_empty + end + + scenario "Should not update image cached_attachment field after invalid file upload" do + imageable_attach_new_file(file_fixture("logo_header.png"), false) + + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + + expect(cached_attachment_field.value).to be_empty + end + + scenario "Should show image errors after invalid form submit" do + click_link "Add image" + click_button submit_button_text + + within "#nested-image .image-fields" do + expect(page).to have_content("can't be blank", count: 2) + end + end + + scenario "Render image preview after sending the form with validation errors" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + within_fieldset("Descriptive image") { fill_in "Title", with: "" } + + click_button submit_button_text + + expect(page).to have_content "can't be blank" + expect(page).to have_css "img[src$='clippy.jpg']" + end + + scenario "Should remove image after valid file upload and click on remove button" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + + within "#nested-image .image-fields" do + click_link "Remove image" + end + + expect(page).not_to have_css "#nested-image .image-fields" + end + + scenario "Should show successful notice when resource filled correctly without any nested images" do + imageable_fill_new_valid_budget + + click_button submit_button_text + + expect(page).to have_content notice_text + end + + scenario "Should show successful notice when resource filled correctly and after valid file uploads" do + imageable_fill_new_valid_budget + + imageable_attach_new_file(file_fixture("clippy.jpg")) + + expect(page).to have_css ".loading-bar.complete" + + click_button submit_button_text + + expect(page).to have_content notice_text + end + + scenario "Should show new image after successful creation with one uploaded file" do + imageable_fill_new_valid_budget + + imageable_attach_new_file(file_fixture("clippy.jpg")) + + expect(page).to have_css ".loading-bar.complete" + + click_button submit_button_text + + expect(page).to have_content notice_text + + redirected_to_resource_show_or_navigate_to(imageable) + + expect(page).to have_css "figure img" + expect(page).to have_css "figure figcaption" if show_caption_for?(factory) + end + + scenario "Different URLs for different images" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + + original_src = find(:fieldset, "Descriptive image").find("img")[:src] + + click_link "Remove image" + imageable_attach_new_file(file_fixture("custom_map.jpg")) + + updated_src = find(:fieldset, "Descriptive image").find("img")[:src] + + expect(updated_src).not_to eq original_src + end + + def show_caption_for?(factory) + factory != :budget + end + + def expect_image_has_title(title) + image = find(".image-fields") + + within image do + expect(find("input[name$='[title]']").value).to eq title + end + end + + def redirected_to_resource_show_or_navigate_to(imageable) + case imageable.class.to_s + when "Budget" + visit edit_admin_budget_path(Budget.last) + when "Proposal" + click_link "Not now, go to my proposal" rescue Capybara::ElementNotFound + end + end +end From add50d68f69b2bf1d66469c0b0bdb95a19c1c983 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 15 Nov 2024 14:01:12 +0100 Subject: [PATCH 02/14] Extract proposal new from shared nested imageable specs to system specs Make `path`, `fill_resource_method_name`, `submit_button`, and `imageable_success_notice` dynamic based on the factory. Also adjusted the user. The proposals no longer require the user to be an administrator but do require them to be a level 2 user. Note that we are adding the Style/CaseLikeIf rubocop rule. --- .rubocop.yml | 3 ++ spec/support/common_actions/images.rb | 10 ------ spec/system/nested_imageable_spec.rb | 50 ++++++++++++++++++++++----- spec/system/proposals_spec.rb | 8 ----- 4 files changed, 45 insertions(+), 26 deletions(-) diff --git a/.rubocop.yml b/.rubocop.yml index bd287aa83..0a5d2c226 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -694,6 +694,9 @@ Style/ArrayIntersect: Style/BlockDelimiters: Enabled: true +Style/CaseLikeIf: + Enabled: true + Style/ClassCheck: Enabled: true diff --git a/spec/support/common_actions/images.rb b/spec/support/common_actions/images.rb index 20682d95a..a9ea0321a 100644 --- a/spec/support/common_actions/images.rb +++ b/spec/support/common_actions/images.rb @@ -14,16 +14,6 @@ module Images end end - def imageable_fill_new_valid_proposal - fill_in_new_proposal_title with: "Proposal title" - fill_in "Proposal summary", with: "Proposal summary" - check :proposal_terms_of_service - end - - def imageable_fill_new_valid_budget - fill_in "Name", with: "Budget name" - end - def imageable_fill_new_valid_budget_investment fill_in_new_investment_title with: "Budget investment title" fill_in_ckeditor "Description", with: "Budget investment description" diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index bb2f2ac0a..d80419e69 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -2,17 +2,34 @@ require "rails_helper" describe "Nested imageable" do factories = [ - :budget + :budget, + :proposal ] let(:factory) { factories.sample } let!(:imageable) { create(factory) } - let!(:user) { create(:administrator).user } - let(:path) { new_admin_budgets_wizard_budget_path } - let(:submit_button_text) { "Continue to groups" } - let(:notice_text) { "New participatory budget created successfully!" } + let!(:user) { create(:user, :level_two) } + let(:path) do + case factory + when :budget then new_admin_budgets_wizard_budget_path + when :proposal then new_proposal_path + end + end + let(:submit_button_text) do + case factory + when :budget then "Continue to groups" + when :proposal then "Create proposal" + end + end + let(:notice_text) do + case factory + when :budget then "New participatory budget created successfully!" + when :proposal then "Proposal created successfully" + end + end before do + create(:administrator, user: user) if factory == :budget login_as(user) visit path end @@ -111,7 +128,7 @@ describe "Nested imageable" do end scenario "Should show successful notice when resource filled correctly without any nested images" do - imageable_fill_new_valid_budget + fill_in_required_fields click_button submit_button_text @@ -119,7 +136,7 @@ describe "Nested imageable" do end scenario "Should show successful notice when resource filled correctly and after valid file uploads" do - imageable_fill_new_valid_budget + fill_in_required_fields imageable_attach_new_file(file_fixture("clippy.jpg")) @@ -131,7 +148,7 @@ describe "Nested imageable" do end scenario "Should show new image after successful creation with one uploaded file" do - imageable_fill_new_valid_budget + fill_in_required_fields imageable_attach_new_file(file_fixture("clippy.jpg")) @@ -180,4 +197,21 @@ describe "Nested imageable" do click_link "Not now, go to my proposal" rescue Capybara::ElementNotFound end end + + def fill_in_required_fields + case factory + when :budget then fill_budget + when :proposal then fill_proposal + end + end + + def fill_proposal + fill_in_new_proposal_title with: "Proposal title" + fill_in "Proposal summary", with: "Proposal summary" + check :proposal_terms_of_service + end + + def fill_budget + fill_in "Name", with: "Budget name" + end end diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index 4ea70abe0..0f0a01813 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -1291,14 +1291,6 @@ describe "Proposals" do it_behaves_like "imageable", "proposal", "proposal_path", { id: "id" } - it_behaves_like "nested imageable", - "proposal", - "new_proposal_path", - {}, - "imageable_fill_new_valid_proposal", - "Create proposal", - "Proposal created successfully" - it_behaves_like "nested imageable", "proposal", "edit_proposal_path", From 2212a2a2f4b3da5d8291fbf007adf373c3df2d32 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 15 Nov 2024 14:52:04 +0100 Subject: [PATCH 03/14] Extract budget investment from shared nested imageable specs to system specs --- spec/support/common_actions/images.rb | 6 ------ spec/system/budgets/investments_spec.rb | 8 -------- spec/system/nested_imageable_spec.rb | 11 +++++++++++ 3 files changed, 11 insertions(+), 14 deletions(-) diff --git a/spec/support/common_actions/images.rb b/spec/support/common_actions/images.rb index a9ea0321a..df59a48cb 100644 --- a/spec/support/common_actions/images.rb +++ b/spec/support/common_actions/images.rb @@ -13,10 +13,4 @@ module Images end end end - - def imageable_fill_new_valid_budget_investment - fill_in_new_investment_title with: "Budget investment title" - fill_in_ckeditor "Description", with: "Budget investment description" - check :budget_investment_terms_of_service - end end diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index f9a08fdda..87eb79e50 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -1118,14 +1118,6 @@ describe "Budget Investments" do "budget_investment_path", { budget_id: "budget_id", id: "id" } - it_behaves_like "nested imageable", - "budget_investment", - "new_budget_investment_path", - { budget_id: "budget_id" }, - "imageable_fill_new_valid_budget_investment", - "Create Investment", - "Budget Investment created successfully." - it_behaves_like "documentable", "budget_investment", "budget_investment_path", diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index d80419e69..1d2f7057b 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -3,6 +3,7 @@ require "rails_helper" describe "Nested imageable" do factories = [ :budget, + :budget_investment, :proposal ] @@ -12,18 +13,21 @@ describe "Nested imageable" do let(:path) do case factory when :budget then new_admin_budgets_wizard_budget_path + when :budget_investment then new_budget_investment_path(budget_id: imageable.budget_id) when :proposal then new_proposal_path end end let(:submit_button_text) do case factory when :budget then "Continue to groups" + when :budget_investment then "Create Investment" when :proposal then "Create proposal" end end let(:notice_text) do case factory when :budget then "New participatory budget created successfully!" + when :budget_investment then "Budget Investment created successfully." when :proposal then "Proposal created successfully" end end @@ -201,6 +205,7 @@ describe "Nested imageable" do def fill_in_required_fields case factory when :budget then fill_budget + when :budget_investment then fill_budget_investment when :proposal then fill_proposal end end @@ -214,4 +219,10 @@ describe "Nested imageable" do def fill_budget fill_in "Name", with: "Budget name" end + + def fill_budget_investment + fill_in_new_investment_title with: "Budget investment title" + fill_in_ckeditor "Description", with: "Budget investment description" + check :budget_investment_terms_of_service + end end From 1cf85560dd33c796fafefaa89660f4ee42df801b Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 15 Nov 2024 15:28:34 +0100 Subject: [PATCH 04/14] Extract poll question option images from shared nested imageable specs to system specs This is the only it_behaves_like "nested imageable" call where the has_many_images parameter is set to true. Previously, the shared example skipped or altered expectations based on this parameter. Now, this behavior is moved to the factory level (:future_poll_question_option). Since this is an administrative section, a related administrator is created for the user. --- .../questions/options/images/images_spec.rb | 9 ------- spec/system/nested_imageable_spec.rb | 24 ++++++++++++++----- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/spec/system/admin/poll/questions/options/images/images_spec.rb b/spec/system/admin/poll/questions/options/images/images_spec.rb index 53558072e..ae4035f6f 100644 --- a/spec/system/admin/poll/questions/options/images/images_spec.rb +++ b/spec/system/admin/poll/questions/options/images/images_spec.rb @@ -4,15 +4,6 @@ describe "Images", :admin do let(:future_poll) { create(:poll, :future) } let(:current_poll) { create(:poll) } - it_behaves_like "nested imageable", - "future_poll_question_option", - "new_admin_option_image_path", - { option_id: "id" }, - nil, - "Save image", - "Image uploaded successfully", - true - context "Index" do scenario "Option with no images" do option = create(:poll_question_option) diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 1d2f7057b..91d188f98 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -4,6 +4,7 @@ describe "Nested imageable" do factories = [ :budget, :budget_investment, + :future_poll_question_option, :proposal ] @@ -14,6 +15,7 @@ describe "Nested imageable" do case factory when :budget then new_admin_budgets_wizard_budget_path when :budget_investment then new_budget_investment_path(budget_id: imageable.budget_id) + when :future_poll_question_option then new_admin_option_image_path(option_id: imageable.id) when :proposal then new_proposal_path end end @@ -21,6 +23,7 @@ describe "Nested imageable" do case factory when :budget then "Continue to groups" when :budget_investment then "Create Investment" + when :future_poll_question_option then "Save image" when :proposal then "Create proposal" end end @@ -28,12 +31,13 @@ describe "Nested imageable" do case factory when :budget then "New participatory budget created successfully!" when :budget_investment then "Budget Investment created successfully." + when :future_poll_question_option then "Image uploaded successfully" when :proposal then "Proposal created successfully" end end before do - create(:administrator, user: user) if factory == :budget + create(:administrator, user: user) if [:budget, :future_poll_question_option].include?(factory) login_as(user) visit path end @@ -67,7 +71,11 @@ describe "Nested imageable" do fill_in input_title[:id], with: "Title" attach_file "Choose image", file_fixture("clippy.jpg") - expect(find("##{factory}_image_attributes_title").value).to eq "Title" + if factory == :future_poll_question_option + expect(find("input[id$='_title']").value).to eq "Title" + else + expect(find("##{factory}_image_attributes_title").value).to eq "Title" + end end scenario "Should update loading bar style after valid file upload" do @@ -131,12 +139,16 @@ describe "Nested imageable" do expect(page).not_to have_css "#nested-image .image-fields" end - scenario "Should show successful notice when resource filled correctly without any nested images" do - fill_in_required_fields + context "Budgets, investments and proposals" do + let(:factory) { (factories - [:future_poll_question_option]).sample } - click_button submit_button_text + scenario "Should show successful notice when resource filled correctly without any nested images" do + fill_in_required_fields - expect(page).to have_content notice_text + click_button submit_button_text + + expect(page).to have_content notice_text + end end scenario "Should show successful notice when resource filled correctly and after valid file uploads" do From 9d7fa9d0f88337f88e9141f9159fd9a856d5465c Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 20 Nov 2024 11:06:28 +0100 Subject: [PATCH 05/14] Unify notice responders for budget investments create action --- .../management/budgets/investments_controller.rb | 4 ++-- spec/system/budgets/investments_spec.rb | 4 ++-- spec/system/emails_spec.rb | 2 +- spec/system/management/budget_investments_spec.rb | 6 +++--- spec/system/tags/budget_investments_spec.rb | 10 +++++----- 5 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/controllers/management/budgets/investments_controller.rb b/app/controllers/management/budgets/investments_controller.rb index c03ced8bb..5d1b32577 100644 --- a/app/controllers/management/budgets/investments_controller.rb +++ b/app/controllers/management/budgets/investments_controller.rb @@ -26,8 +26,8 @@ class Management::Budgets::InvestmentsController < Management::BaseController @investment.heading = @budget.headings.first if @budget.single_heading? if @investment.save - notice = t("flash.actions.create.notice", resource_name: Budget::Investment.model_name.human, count: 1) - redirect_to management_budget_investment_path(@budget, @investment), notice: notice + redirect_to management_budget_investment_path(@budget, @investment), + notice: t("flash.actions.create.budget_investment") else render :new end diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 87eb79e50..b18ff6e81 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -610,7 +610,7 @@ describe "Budget Investments" do click_button "Create Investment" - expect(page).to have_content "Investment created successfully" + expect(page).to have_content "Budget Investment created successfully" expect(page).to have_content "Build a skyscraper" expect(page).to have_content "I want to live in a high tower over the clouds" expect(page).to have_content "City center" @@ -678,7 +678,7 @@ describe "Budget Investments" do click_button "Create Investment" - expect(page).to have_content "Investment created successfully" + expect(page).to have_content "Budget Investment created successfully" expect(page).to have_content "Build a skyscraper" expect(page).to have_content "I want to live in a high tower over the clouds" expect(page).to have_content "City center" diff --git a/spec/system/emails_spec.rb b/spec/system/emails_spec.rb index 0612d6087..f97b521b9 100644 --- a/spec/system/emails_spec.rb +++ b/spec/system/emails_spec.rb @@ -358,7 +358,7 @@ describe "Emails" do check "budget_investment_terms_of_service" click_button "Create Investment" - expect(page).to have_content "Investment created successfully" + expect(page).to have_content "Budget Investment created successfully" email = open_last_email diff --git a/spec/system/management/budget_investments_spec.rb b/spec/system/management/budget_investments_spec.rb index 7fcbe2763..cba7b53f5 100644 --- a/spec/system/management/budget_investments_spec.rb +++ b/spec/system/management/budget_investments_spec.rb @@ -14,7 +14,7 @@ describe "Budget Investments" do { budget_id: "budget_id" }, "documentable_fill_new_valid_budget_investment", "Create Investment", - "Investment created successfully.", + "Budget Investment created successfully.", management: true it_behaves_like "nested imageable", @@ -23,7 +23,7 @@ describe "Budget Investments" do { budget_id: "budget_id" }, "imageable_fill_new_valid_budget_investment", "Create Investment", - "Investment created successfully.", + "Budget Investment created successfully.", management: true it_behaves_like "mappable", @@ -73,7 +73,7 @@ describe "Budget Investments" do click_button "Create Investment" - expect(page).to have_content "Investment created successfully." + expect(page).to have_content "Budget Investment created successfully." expect(page).to have_content "Health" expect(page).to have_content "Build a park in my neighborhood" diff --git a/spec/system/tags/budget_investments_spec.rb b/spec/system/tags/budget_investments_spec.rb index f032c6041..8f49284dd 100644 --- a/spec/system/tags/budget_investments_spec.rb +++ b/spec/system/tags/budget_investments_spec.rb @@ -76,7 +76,7 @@ describe "Tags" do click_button "Create Investment" - expect(page).to have_content "Investment created successfully." + expect(page).to have_content "Budget Investment created successfully." expect(page).to have_content tag_economia.name expect(page).to have_content tag_medio_ambiente.name end @@ -93,7 +93,7 @@ describe "Tags" do find(".js-add-tag-link", text: tag_economia.name).click click_button "Create Investment" - expect(page).to have_content "Investment created successfully." + expect(page).to have_content "Budget Investment created successfully." expect(page).to have_content "Build a skyscraper" within ".tags" do @@ -117,7 +117,7 @@ describe "Tags" do find(".js-add-tag-link", text: "Education").click click_button "Create Investment" - expect(page).to have_content "Investment created successfully." + expect(page).to have_content "Budget Investment created successfully." expect(page).to have_content "Build a skyscraper" within ".tags" do @@ -141,7 +141,7 @@ describe "Tags" do find(".js-add-tag-link", text: "Education").click click_button "Create Investment" - expect(page).to have_content "Investment created successfully." + expect(page).to have_content "Budget Investment created successfully." expect(page).to have_content "Build a skyscraper" within ".tags" do @@ -181,7 +181,7 @@ describe "Tags" do click_button "Create Investment" - expect(page).to have_content "Investment created successfully." + expect(page).to have_content "Budget Investment created successfully." expect(page).to have_content "user_id1" expect(page).to have_content "a3" expect(page).to have_content "scriptalert('hey');script" From cdfaec5217a6a590b9840c60bcf8aff88ad3b1b8 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 20 Nov 2024 11:08:46 +0100 Subject: [PATCH 06/14] Extract management budget investment from shared nested imageable specs to system specs --- .../management/budget_investments_spec.rb | 9 --------- spec/system/nested_imageable_spec.rb | 18 +++++++++++++++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/spec/system/management/budget_investments_spec.rb b/spec/system/management/budget_investments_spec.rb index cba7b53f5..5bcf5d256 100644 --- a/spec/system/management/budget_investments_spec.rb +++ b/spec/system/management/budget_investments_spec.rb @@ -17,15 +17,6 @@ describe "Budget Investments" do "Budget Investment created successfully.", management: true - it_behaves_like "nested imageable", - "budget_investment", - "new_management_budget_investment_path", - { budget_id: "budget_id" }, - "imageable_fill_new_valid_budget_investment", - "Create Investment", - "Budget Investment created successfully.", - management: true - it_behaves_like "mappable", "budget_investment", "investment", diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 91d188f98..dbcdbc4da 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -14,7 +14,11 @@ describe "Nested imageable" do let(:path) do case factory when :budget then new_admin_budgets_wizard_budget_path - when :budget_investment then new_budget_investment_path(budget_id: imageable.budget_id) + when :budget_investment + [ + new_budget_investment_path(budget_id: imageable.budget_id), + new_management_budget_investment_path(budget_id: imageable.budget_id) + ].sample when :future_poll_question_option then new_admin_option_image_path(option_id: imageable.id) when :proposal then new_proposal_path end @@ -37,8 +41,8 @@ describe "Nested imageable" do end before do - create(:administrator, user: user) if [:budget, :future_poll_question_option].include?(factory) - login_as(user) + create(:administrator, user: user) if admin_section? || management_section? + do_login_for(user, management: management_section?) visit path end @@ -237,4 +241,12 @@ describe "Nested imageable" do fill_in_ckeditor "Description", with: "Budget investment description" check :budget_investment_terms_of_service end + + def admin_section? + path.starts_with?("/admin/") + end + + def management_section? + path.starts_with?("/management/") + end end From 3877479b6991f6fbbc95ff14d8f4763cd8d5f34f Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 18 Nov 2024 12:02:19 +0100 Subject: [PATCH 07/14] Extract proposal edit from shared nested imageable specs to system specs --- spec/shared/system/nested_imageable.rb | 246 ------------------ spec/system/nested_imageable_spec.rb | 330 ++++++++++++++----------- spec/system/proposals_spec.rb | 8 - 3 files changed, 190 insertions(+), 394 deletions(-) delete mode 100644 spec/shared/system/nested_imageable.rb diff --git a/spec/shared/system/nested_imageable.rb b/spec/shared/system/nested_imageable.rb deleted file mode 100644 index 6121f0a0f..000000000 --- a/spec/shared/system/nested_imageable.rb +++ /dev/null @@ -1,246 +0,0 @@ -shared_examples "nested imageable" do |imageable_factory_name, path, imageable_path_arguments, - fill_resource_method_name, submit_button, imageable_success_notice, - has_many_images = false, management: false| - let!(:user) { create(:user, :level_two) } - let!(:arguments) { {} } - let!(:imageable) { create(imageable_factory_name) } - let(:management) { management } - - before do - create(:administrator, user: user) - - imageable_path_arguments&.each do |argument_name, path_to_value| - arguments.merge!("#{argument_name}": imageable.send(path_to_value)) - end - - imageable.update(author: user) if imageable.respond_to?(:author) - end - - describe "at #{path}" do - scenario "Should show new image link when imageable has not an associated image defined" do - do_login_for user, management: management - visit send(path, arguments) - - expect(page).to have_css "#new_image_link" - end - - scenario "Should hide new image link after adding one image" do - do_login_for user, management: management - visit send(path, arguments) - - click_link "Add image" - - expect(page).not_to have_css "#new_image_link" - end - - scenario "Should update image file name after choosing any file" do - do_login_for user, management: management - visit send(path, arguments) - - click_link "Add image" - attach_file "Choose image", file_fixture("clippy.jpg") - - expect(page).to have_css ".file-name", text: "clippy.jpg" - end - - scenario "Should update image file title after choosing a file when no title is defined" do - do_login_for user, management: management - visit send(path, arguments) - - imageable_attach_new_file(file_fixture("clippy.jpg")) - - expect_image_has_title("clippy.jpg") - end - - scenario "Should not update image file title after choosing a file when a title is already defined" do - do_login_for user, management: management - visit send(path, arguments) - - click_link "Add image" - input_title = find(".image-fields input[name$='[title]']") - fill_in input_title[:id], with: "Title" - attach_file "Choose image", file_fixture("clippy.jpg") - - if has_many_images - expect(find("input[id$='_title']").value).to eq "Title" - else - expect(find("##{imageable_factory_name}_image_attributes_title").value).to eq "Title" - end - end - - scenario "Should update loading bar style after valid file upload" do - do_login_for user, management: management - visit send(path, arguments) - - imageable_attach_new_file(file_fixture("clippy.jpg")) - - expect(page).to have_css ".loading-bar.complete" - end - - scenario "Should update loading bar style after invalid file upload" do - do_login_for user, management: management - visit send(path, arguments) - - imageable_attach_new_file(file_fixture("logo_header.png"), false) - - expect(page).to have_css ".loading-bar.errors" - end - - scenario "Should update image cached_attachment field after valid file upload" do - do_login_for user, management: management - visit send(path, arguments) - - click_link "Add image" - - cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) - expect(cached_attachment_field.value).to be_empty - - attach_file "Choose image", file_fixture("clippy.jpg") - - expect(page).to have_css(".loading-bar.complete") - expect(cached_attachment_field.value).not_to be_empty - end - - scenario "Should not update image cached_attachment field after invalid file upload" do - do_login_for user, management: management - visit send(path, arguments) - - imageable_attach_new_file(file_fixture("logo_header.png"), false) - - cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) - - expect(cached_attachment_field.value).to be_empty - end - - scenario "Should show image errors after invalid form submit" do - do_login_for user, management: management - visit send(path, arguments) - - click_link "Add image" - click_button submit_button - - within "#nested-image .image-fields" do - expect(page).to have_content("can't be blank", count: 2) - end - end - - scenario "Render image preview after sending the form with validation errors", - unless: imageable_factory_name == "poll_question_option" do - do_login_for user, management: management - visit send(path, arguments) - - imageable_attach_new_file(file_fixture("clippy.jpg")) - within_fieldset("Descriptive image") { fill_in "Title", with: "" } - click_button submit_button - - expect(page).to have_content "can't be blank" - expect(page).to have_css "img[src$='clippy.jpg']" - end - - scenario "Should remove image after valid file upload and click on remove button" do - do_login_for user, management: management - visit send(path, arguments) - - imageable_attach_new_file(file_fixture("clippy.jpg")) - - within "#nested-image .image-fields" do - click_link "Remove image" - end - - expect(page).not_to have_css "#nested-image .image-fields" - end - - scenario "Should show successful notice when resource filled correctly without any nested images", - unless: has_many_images do - do_login_for user, management: management - visit send(path, arguments) - - send(fill_resource_method_name) if fill_resource_method_name - click_button submit_button - expect(page).to have_content imageable_success_notice - end - - scenario "Should show successful notice when resource filled correctly and after valid file uploads" do - do_login_for user, management: management - visit send(path, arguments) - send(fill_resource_method_name) if fill_resource_method_name - - imageable_attach_new_file(file_fixture("clippy.jpg")) - - expect(page).to have_css ".loading-bar.complete" - - click_button submit_button - - expect(page).to have_content imageable_success_notice - end - - scenario "Should show new image after successful creation with one uploaded file" do - do_login_for user, management: management - visit send(path, arguments) - send(fill_resource_method_name) if fill_resource_method_name - - imageable_attach_new_file(file_fixture("clippy.jpg")) - - expect(page).to have_css ".loading-bar.complete" - - click_button submit_button - - expect(page).to have_content imageable_success_notice - - imageable_redirected_to_resource_show_or_navigate_to(imageable) - - expect(page).to have_css "figure img" - expect(page).to have_css "figure figcaption" if show_caption_for?(imageable_factory_name) - end - - scenario "Different URLs for different images" do - do_login_for user, management: management - visit send(path, arguments) - - imageable_attach_new_file(file_fixture("clippy.jpg")) - - original_src = find(:fieldset, "Descriptive image").find("img")[:src] - - click_link "Remove image" - imageable_attach_new_file(file_fixture("custom_map.jpg")) - - updated_src = find(:fieldset, "Descriptive image").find("img")[:src] - - expect(updated_src).not_to eq original_src - end - - if path.include? "edit" - scenario "show persisted image" do - create(:image, imageable: imageable) - do_login_for user, management: management - - visit send(path, arguments) - - expect(page).to have_css ".image-fields", count: 1 - expect(page).not_to have_css "a#new_image_link" - end - - scenario "remove nested field after removing the image" do - create(:image, imageable: imageable) - do_login_for user, management: management - - visit send(path, arguments) - click_link "Remove image" - - expect(page).not_to have_css ".image-fields" - expect(page).to have_link id: "new_image_link" - end - - scenario "don't duplicate fields after removing and adding an image" do - create(:image, imageable: imageable) - do_login_for user, management: management - - visit send(path, arguments) - click_link "Remove image" - click_link "Add image" - - expect(page).to have_css ".image-fields", count: 1, visible: :all - end - end - end -end diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index dbcdbc4da..3b6585cbe 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -20,7 +20,7 @@ describe "Nested imageable" do new_management_budget_investment_path(budget_id: imageable.budget_id) ].sample when :future_poll_question_option then new_admin_option_image_path(option_id: imageable.id) - when :proposal then new_proposal_path + when :proposal then [new_proposal_path, edit_proposal_path(imageable)].sample end end let(:submit_button_text) do @@ -28,7 +28,12 @@ describe "Nested imageable" do when :budget then "Continue to groups" when :budget_investment then "Create Investment" when :future_poll_question_option then "Save image" - when :proposal then "Create proposal" + when :proposal + if edit_path? + "Save changes" + else + "Create proposal" + end end end let(:notice_text) do @@ -36,165 +41,204 @@ describe "Nested imageable" do when :budget then "New participatory budget created successfully!" when :budget_investment then "Budget Investment created successfully." when :future_poll_question_option then "Image uploaded successfully" - when :proposal then "Proposal created successfully" + when :proposal + if edit_path? + "Proposal updated successfully" + else + "Proposal created successfully" + end end end - before do - create(:administrator, user: user) if admin_section? || management_section? - do_login_for(user, management: management_section?) - visit path - end - - scenario "Should show new image link when imageable has not an associated image defined" do - expect(page).to have_css "#new_image_link" - end - - scenario "Should hide new image link after adding one image" do - click_link "Add image" - - expect(page).not_to have_css "#new_image_link" - end - - scenario "Should update image file name after choosing any file" do - click_link "Add image" - attach_file "Choose image", file_fixture("clippy.jpg") - - expect(page).to have_css ".file-name", text: "clippy.jpg" - end - - scenario "Should update image file title after choosing a file when no title is defined" do - imageable_attach_new_file(file_fixture("clippy.jpg")) - - expect_image_has_title("clippy.jpg") - end - - scenario "Should not update image file title after choosing a file when a title is already defined" do - click_link "Add image" - input_title = find(".image-fields input[name$='[title]']") - fill_in input_title[:id], with: "Title" - attach_file "Choose image", file_fixture("clippy.jpg") - - if factory == :future_poll_question_option - expect(find("input[id$='_title']").value).to eq "Title" - else - expect(find("##{factory}_image_attributes_title").value).to eq "Title" - end - end - - scenario "Should update loading bar style after valid file upload" do - imageable_attach_new_file(file_fixture("clippy.jpg")) - - expect(page).to have_css ".loading-bar.complete" - end - - scenario "Should update loading bar style after invalid file upload" do - imageable_attach_new_file(file_fixture("logo_header.png"), false) - - expect(page).to have_css ".loading-bar.errors" - end - - scenario "Should update image cached_attachment field after valid file upload" do - click_link "Add image" - - cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) - expect(cached_attachment_field.value).to be_empty - - attach_file "Choose image", file_fixture("clippy.jpg") - - expect(page).to have_css(".loading-bar.complete") - expect(cached_attachment_field.value).not_to be_empty - end - - scenario "Should not update image cached_attachment field after invalid file upload" do - imageable_attach_new_file(file_fixture("logo_header.png"), false) - - cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) - - expect(cached_attachment_field.value).to be_empty - end - - scenario "Should show image errors after invalid form submit" do - click_link "Add image" - click_button submit_button_text - - within "#nested-image .image-fields" do - expect(page).to have_content("can't be blank", count: 2) - end - end - - scenario "Render image preview after sending the form with validation errors" do - imageable_attach_new_file(file_fixture("clippy.jpg")) - within_fieldset("Descriptive image") { fill_in "Title", with: "" } - - click_button submit_button_text - - expect(page).to have_content "can't be blank" - expect(page).to have_css "img[src$='clippy.jpg']" - end - - scenario "Should remove image after valid file upload and click on remove button" do - imageable_attach_new_file(file_fixture("clippy.jpg")) - - within "#nested-image .image-fields" do - click_link "Remove image" + context "New and edit path" do + before do + create(:administrator, user: user) if admin_section? || management_section? + imageable.update!(author: user) if edit_path? + do_login_for(user, management: management_section?) + visit path end - expect(page).not_to have_css "#nested-image .image-fields" - end + scenario "Should show new image link when imageable has not an associated image defined" do + expect(page).to have_css "#new_image_link" + end - context "Budgets, investments and proposals" do - let(:factory) { (factories - [:future_poll_question_option]).sample } + scenario "Should hide new image link after adding one image" do + click_link "Add image" - scenario "Should show successful notice when resource filled correctly without any nested images" do + expect(page).not_to have_css "#new_image_link" + end + + scenario "Should update image file name after choosing any file" do + click_link "Add image" + attach_file "Choose image", file_fixture("clippy.jpg") + + expect(page).to have_css ".file-name", text: "clippy.jpg" + end + + scenario "Should update image file title after choosing a file when no title is defined" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + + expect_image_has_title("clippy.jpg") + end + + scenario "Should not update image file title after choosing a file when a title is already defined" do + click_link "Add image" + input_title = find(".image-fields input[name$='[title]']") + fill_in input_title[:id], with: "Title" + attach_file "Choose image", file_fixture("clippy.jpg") + + if factory == :future_poll_question_option + expect(find("input[id$='_title']").value).to eq "Title" + else + expect(find("##{factory}_image_attributes_title").value).to eq "Title" + end + end + + scenario "Should update loading bar style after valid file upload" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + + expect(page).to have_css ".loading-bar.complete" + end + + scenario "Should update loading bar style after invalid file upload" do + imageable_attach_new_file(file_fixture("logo_header.png"), false) + + expect(page).to have_css ".loading-bar.errors" + end + + scenario "Should update image cached_attachment field after valid file upload" do + click_link "Add image" + + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty + + attach_file "Choose image", file_fixture("clippy.jpg") + + expect(page).to have_css(".loading-bar.complete") + expect(cached_attachment_field.value).not_to be_empty + end + + scenario "Should not update image cached_attachment field after invalid file upload" do + imageable_attach_new_file(file_fixture("logo_header.png"), false) + + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + + expect(cached_attachment_field.value).to be_empty + end + + scenario "Should show image errors after invalid form submit" do + click_link "Add image" + click_button submit_button_text + + within "#nested-image .image-fields" do + expect(page).to have_content("can't be blank", count: 2) + end + end + + scenario "Render image preview after sending the form with validation errors" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + within_fieldset("Descriptive image") { fill_in "Title", with: "" } + + click_button submit_button_text + + expect(page).to have_content "can't be blank" + expect(page).to have_css "img[src$='clippy.jpg']" + end + + scenario "Should remove image after valid file upload and click on remove button" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + + within "#nested-image .image-fields" do + click_link "Remove image" + end + + expect(page).not_to have_css "#nested-image .image-fields" + end + + context "Budgets, investments and proposals" do + let(:factory) { (factories - [:future_poll_question_option]).sample } + + scenario "Should show successful notice when resource filled correctly without any nested images" do + fill_in_required_fields + + click_button submit_button_text + + expect(page).to have_content notice_text + end + end + + scenario "Should show successful notice when resource filled correctly and after valid file uploads" do fill_in_required_fields + imageable_attach_new_file(file_fixture("clippy.jpg")) + + expect(page).to have_css ".loading-bar.complete" + click_button submit_button_text expect(page).to have_content notice_text end + + scenario "Should show new image after successful creation with one uploaded file" do + fill_in_required_fields + + imageable_attach_new_file(file_fixture("clippy.jpg")) + + expect(page).to have_css ".loading-bar.complete" + + click_button submit_button_text + + expect(page).to have_content notice_text + + redirected_to_resource_show_or_navigate_to(imageable) + + expect(page).to have_css "figure img" + expect(page).to have_css "figure figcaption" if show_caption_for?(factory) + end + + scenario "Different URLs for different images" do + imageable_attach_new_file(file_fixture("clippy.jpg")) + + original_src = find(:fieldset, "Descriptive image").find("img")[:src] + + click_link "Remove image" + imageable_attach_new_file(file_fixture("custom_map.jpg")) + + updated_src = find(:fieldset, "Descriptive image").find("img")[:src] + + expect(updated_src).not_to eq original_src + end end - scenario "Should show successful notice when resource filled correctly and after valid file uploads" do - fill_in_required_fields + context "Only for edit path" do + let(:factory) { :proposal } + let(:path) { edit_proposal_path(imageable) } - imageable_attach_new_file(file_fixture("clippy.jpg")) + before do + create(:image, imageable: imageable) + imageable.update!(author: user) + login_as(user) + visit path + end - expect(page).to have_css ".loading-bar.complete" + scenario "show persisted image" do + expect(page).to have_css ".image-fields", count: 1 + expect(page).not_to have_css "a#new_image_link" + end - click_button submit_button_text + scenario "remove nested field after removing the image" do + click_link "Remove image" - expect(page).to have_content notice_text - end + expect(page).not_to have_css ".image-fields" + expect(page).to have_link id: "new_image_link" + end - scenario "Should show new image after successful creation with one uploaded file" do - fill_in_required_fields + scenario "don't duplicate fields after removing and adding an image" do + click_link "Remove image" + click_link "Add image" - imageable_attach_new_file(file_fixture("clippy.jpg")) - - expect(page).to have_css ".loading-bar.complete" - - click_button submit_button_text - - expect(page).to have_content notice_text - - redirected_to_resource_show_or_navigate_to(imageable) - - expect(page).to have_css "figure img" - expect(page).to have_css "figure figcaption" if show_caption_for?(factory) - end - - scenario "Different URLs for different images" do - imageable_attach_new_file(file_fixture("clippy.jpg")) - - original_src = find(:fieldset, "Descriptive image").find("img")[:src] - - click_link "Remove image" - imageable_attach_new_file(file_fixture("custom_map.jpg")) - - updated_src = find(:fieldset, "Descriptive image").find("img")[:src] - - expect(updated_src).not_to eq original_src + expect(page).to have_css ".image-fields", count: 1, visible: :all + end end def show_caption_for?(factory) @@ -219,6 +263,8 @@ describe "Nested imageable" do end def fill_in_required_fields + return if edit_path? + case factory when :budget then fill_budget when :budget_investment then fill_budget_investment @@ -249,4 +295,8 @@ describe "Nested imageable" do def management_section? path.starts_with?("/management/") end + + def edit_path? + path.ends_with?("/edit") + end end diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index 0f0a01813..840255e4b 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -1291,14 +1291,6 @@ describe "Proposals" do it_behaves_like "imageable", "proposal", "proposal_path", { id: "id" } - it_behaves_like "nested imageable", - "proposal", - "edit_proposal_path", - { id: "id" }, - nil, - "Save changes", - "Proposal updated successfully" - it_behaves_like "documentable", "proposal", "proposal_path", { id: "id" } it_behaves_like "nested documentable", From 71e758f71c6b6a383562a60c690129540b739af6 Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 18 Nov 2024 13:13:31 +0100 Subject: [PATCH 08/14] Remove redundant ".loading-bar.complete" In the imageable_attach_new_file method used in these tests the: > expect(page).to have_css ".loading-bar.complete" is already being checked, so there is no need to verify it twice. --- spec/system/nested_imageable_spec.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 3b6585cbe..849f82e7b 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -94,12 +94,6 @@ describe "Nested imageable" do end end - scenario "Should update loading bar style after valid file upload" do - imageable_attach_new_file(file_fixture("clippy.jpg")) - - expect(page).to have_css ".loading-bar.complete" - end - scenario "Should update loading bar style after invalid file upload" do imageable_attach_new_file(file_fixture("logo_header.png"), false) @@ -172,8 +166,6 @@ describe "Nested imageable" do imageable_attach_new_file(file_fixture("clippy.jpg")) - expect(page).to have_css ".loading-bar.complete" - click_button submit_button_text expect(page).to have_content notice_text @@ -184,8 +176,6 @@ describe "Nested imageable" do imageable_attach_new_file(file_fixture("clippy.jpg")) - expect(page).to have_css ".loading-bar.complete" - click_button submit_button_text expect(page).to have_content notice_text From e45018612231696f5547bd1d630ab9642d7f8964 Mon Sep 17 00:00:00 2001 From: taitus Date: Tue, 19 Nov 2024 14:39:36 +0100 Subject: [PATCH 09/14] Regroup tests for link visibility and upload images --- spec/system/nested_imageable_spec.rb | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 849f82e7b..2e00d0d41 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -58,26 +58,13 @@ describe "Nested imageable" do visit path end - scenario "Should show new image link when imageable has not an associated image defined" do + scenario "Handles image link visibility, upload, and file updates correctly" do expect(page).to have_css "#new_image_link" - end - scenario "Should hide new image link after adding one image" do - click_link "Add image" - - expect(page).not_to have_css "#new_image_link" - end - - scenario "Should update image file name after choosing any file" do - click_link "Add image" - attach_file "Choose image", file_fixture("clippy.jpg") - - expect(page).to have_css ".file-name", text: "clippy.jpg" - end - - scenario "Should update image file title after choosing a file when no title is defined" do imageable_attach_new_file(file_fixture("clippy.jpg")) + expect(page).not_to have_css "#new_image_link" + expect(page).to have_css ".file-name", text: "clippy.jpg" expect_image_has_title("clippy.jpg") end From c80641e5f268a33642e25e594d9344ddc68a7b44 Mon Sep 17 00:00:00 2001 From: taitus Date: Tue, 26 Nov 2024 16:55:25 +0100 Subject: [PATCH 10/14] Regroup tests for edit path --- spec/system/nested_imageable_spec.rb | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 2e00d0d41..2675088cc 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -188,30 +188,20 @@ describe "Nested imageable" do end context "Only for edit path" do - let(:factory) { :proposal } - let(:path) { edit_proposal_path(imageable) } + let(:proposal) { create(:proposal, :with_image, author: user) } - before do - create(:image, imageable: imageable) - imageable.update!(author: user) + scenario "handles image fields correctly" do login_as(user) - visit path - end + visit edit_proposal_path(proposal) - scenario "show persisted image" do expect(page).to have_css ".image-fields", count: 1 expect(page).not_to have_css "a#new_image_link" - end - scenario "remove nested field after removing the image" do click_link "Remove image" expect(page).not_to have_css ".image-fields" expect(page).to have_link id: "new_image_link" - end - scenario "don't duplicate fields after removing and adding an image" do - click_link "Remove image" click_link "Add image" expect(page).to have_css ".image-fields", count: 1, visible: :all From 84bec1241f3ea0e74640b5a80b9aad8c08730466 Mon Sep 17 00:00:00 2001 From: taitus Date: Tue, 19 Nov 2024 15:15:57 +0100 Subject: [PATCH 11/14] Remove redundant "loading-bar.errors" In the imageable_attach_new_file method used in this tests the: > expect(page).to have_css ".loading-bar.errors" is already being checked. Therefore, to leave only the line: > imageable_attach_new_file(file_fixture("logo_header.png"), false) in the test, since there is another test that verifies it, I think we can remove the test altogether. --- spec/system/nested_imageable_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 2675088cc..22a027fa5 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -81,12 +81,6 @@ describe "Nested imageable" do end end - scenario "Should update loading bar style after invalid file upload" do - imageable_attach_new_file(file_fixture("logo_header.png"), false) - - expect(page).to have_css ".loading-bar.errors" - end - scenario "Should update image cached_attachment field after valid file upload" do click_link "Add image" From 08c4ecbed2f9aab418c597376c5449d643de0db0 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 21 Nov 2024 10:42:56 +0100 Subject: [PATCH 12/14] Remove unnecessary method --- spec/system/nested_imageable_spec.rb | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 22a027fa5..900b7aeca 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -65,7 +65,9 @@ describe "Nested imageable" do expect(page).not_to have_css "#new_image_link" expect(page).to have_css ".file-name", text: "clippy.jpg" - expect_image_has_title("clippy.jpg") + within ".image-fields" do + expect(find("input[name$='[title]']").value).to eq "clippy.jpg" + end end scenario "Should not update image file title after choosing a file when a title is already defined" do @@ -206,14 +208,6 @@ describe "Nested imageable" do factory != :budget end - def expect_image_has_title(title) - image = find(".image-fields") - - within image do - expect(find("input[name$='[title]']").value).to eq title - end - end - def redirected_to_resource_show_or_navigate_to(imageable) case imageable.class.to_s when "Budget" From 3fd2a1f4988c69e3e2f89930dfa6027ef78dd1a7 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 21 Nov 2024 15:06:27 +0100 Subject: [PATCH 13/14] Remove redirected_to_resource_show_or_navigate_to method This method was removed as its logic was redundant or unnecessary: - For "Proposal" new: After creating a proposal, we are redirected to the "created" page, where the text "Not now, go to my proposal" is not present, leading to a constant `rescue Capybara::ElementNotFound`. Instead, the "created" page shows a preview of how the proposal will look when published and a link saying "No, I want to publish the proposal". Since the click's purpose was to navigate to the proposal's "show" page, and this can already be verified on the "created" page, no additional handling is needed for this case. - For "Proposal" edit: After updating the proposal, we are directly redirected to the proposal's "show" page, so no click_link logic is necessary here either. - For "Budget": The redirection is now handled directly with: `visit edit_admin_budget_path(imageable) if factory == :budget`. --- spec/system/nested_imageable_spec.rb | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 900b7aeca..202f630cd 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -163,7 +163,7 @@ describe "Nested imageable" do expect(page).to have_content notice_text - redirected_to_resource_show_or_navigate_to(imageable) + visit edit_admin_budget_path(imageable) if factory == :budget expect(page).to have_css "figure img" expect(page).to have_css "figure figcaption" if show_caption_for?(factory) @@ -208,15 +208,6 @@ describe "Nested imageable" do factory != :budget end - def redirected_to_resource_show_or_navigate_to(imageable) - case imageable.class.to_s - when "Budget" - visit edit_admin_budget_path(Budget.last) - when "Proposal" - click_link "Not now, go to my proposal" rescue Capybara::ElementNotFound - end - end - def fill_in_required_fields return if edit_path? From fdee1591855bb42a367a672d26c558752ef19194 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 21 Nov 2024 15:18:57 +0100 Subject: [PATCH 14/14] Remove show_caption_for? method The `show_caption_for?` method was used to determine whether to check for the presence of a `figcaption` element, but its only purpose was to skip the check when the factory was `:budget. The reason we skip the `figcaption` check for `:budget` is that it is the only case where the test is verifying the form's edit page, where displaying a `figcaption` does not make sense. --- spec/system/nested_imageable_spec.rb | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 202f630cd..26038a0d0 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -163,10 +163,12 @@ describe "Nested imageable" do expect(page).to have_content notice_text - visit edit_admin_budget_path(imageable) if factory == :budget - + if factory == :budget + click_link "Go back to edit budget" + else + expect(page).to have_css "figure figcaption" + end expect(page).to have_css "figure img" - expect(page).to have_css "figure figcaption" if show_caption_for?(factory) end scenario "Different URLs for different images" do @@ -204,10 +206,6 @@ describe "Nested imageable" do end end - def show_caption_for?(factory) - factory != :budget - end - def fill_in_required_fields return if edit_path?