From 4d8a337e8a53059caaf64c225e276ae9ab732bc1 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 14 May 2025 11:13:04 +0200 Subject: [PATCH 01/25] Extract dashboard_action from shared nested documentable spec to system specs Removed 'documentable_path_arguments' and 'management' parameters because they are not used by dashboard_action. Also moved and renamed the 'documentable_fill_new_valid_dashboard_action' method from the common actions helper to this file, since it is now only used here. Hardcoded 'path', 'submit_button_text', and 'notice_text' for dashboard_action. These remain fixed for now until dynamic values are required in future commits. --- spec/shared/system/nested_documentable.rb | 5 +- spec/support/common_actions/documents.rb | 5 - spec/system/admin/dashboard/actions_spec.rb | 9 - spec/system/nested_documentable_spec.rb | 213 ++++++++++++++++++++ 4 files changed, 215 insertions(+), 17 deletions(-) create mode 100644 spec/system/nested_documentable_spec.rb diff --git a/spec/shared/system/nested_documentable.rb b/spec/shared/system/nested_documentable.rb index c45e6c053..651e32d29 100644 --- a/spec/shared/system/nested_documentable.rb +++ b/spec/shared/system/nested_documentable.rb @@ -201,8 +201,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na expect(page).to have_content documentable_success_notice end - scenario "Should show new document after successful creation with one uploaded file", - unless: documentable_factory_name == "dashboard_action" do + scenario "Should show new document after successful creation with one uploaded file" do do_login_for user_to_login, management: management visit send(path, arguments) send(fill_resource_method_name) if fill_resource_method_name @@ -223,7 +222,7 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na end scenario "Should show resource with new document after successful creation with - maximum allowed uploaded files", unless: documentable_factory_name == "dashboard_action" do + maximum allowed uploaded files" do do_login_for user_to_login, management: management visit send(path, arguments) diff --git a/spec/support/common_actions/documents.rb b/spec/support/common_actions/documents.rb index b0936646d..8f3d77141 100644 --- a/spec/support/common_actions/documents.rb +++ b/spec/support/common_actions/documents.rb @@ -35,11 +35,6 @@ module Documents check :proposal_terms_of_service end - def documentable_fill_new_valid_dashboard_action - fill_in :dashboard_action_title, with: "Dashboard title" - fill_in_ckeditor "Description", with: "Dashboard description" - end - def documentable_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/admin/dashboard/actions_spec.rb b/spec/system/admin/dashboard/actions_spec.rb index 302f2edc4..e0909cdda 100644 --- a/spec/system/admin/dashboard/actions_spec.rb +++ b/spec/system/admin/dashboard/actions_spec.rb @@ -1,15 +1,6 @@ require "rails_helper" describe "Admin dashboard actions", :admin do - it_behaves_like "nested documentable", - "administrator", - "dashboard_action", - "new_admin_dashboard_action_path", - {}, - "documentable_fill_new_valid_dashboard_action", - "Save", - "Action created successfully" - context "when visiting index" do context "and no actions defined" do before do diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb new file mode 100644 index 000000000..aa8749932 --- /dev/null +++ b/spec/system/nested_documentable_spec.rb @@ -0,0 +1,213 @@ +require "rails_helper" + +describe "Nested documentable" do + factories = [ + :dashboard_action + ] + + let(:factory) { factories.sample } + let!(:documentable) { create(factory) } + let!(:user) { create(:administrator).user } + let(:path) { new_admin_dashboard_action_path } + let(:submit_button_text) { "Save" } + let(:notice_text) { "Action created successfully" } + + context "New path" do + describe "When allow attached documents setting is enabled" do + scenario "Should show new document link when max documents allowed limit is not reached" do + login_as user + visit path + + expect(page).to have_link "Add new document" + end + + scenario "Should not show new document link when + documentable max documents allowed limit is reached" do + login_as user + visit path + + documentable.class.max_documents_allowed.times.each do + click_link "Add new document" + end + + expect(page).not_to have_css "#new_document_link" + end + + scenario "Should not show max documents warning when no documents added" do + login_as user + visit path + + expect(page).not_to have_css ".max-documents-notice" + end + + scenario "Should show max documents warning when max documents allowed limit is reached" do + login_as user + visit path + + documentable.class.max_documents_allowed.times.each do + documentable_attach_new_file(file_fixture("empty.pdf")) + end + + expect(page).to have_css ".max-documents-notice" + expect(page).to have_content "Remove document" + end + + scenario "Should hide max documents warning after any document removal" do + login_as user + visit path + + documentable.class.max_documents_allowed.times.each do + click_link "Add new document" + end + + all("a", text: "Cancel").last.click + + expect(page).not_to have_css ".max-documents-notice" + end + + scenario "Should update nested document file name after choosing a file" do + login_as user + visit path + + click_link "Add new document" + within "#nested-documents" do + attach_file "Choose document", file_fixture("empty.pdf") + + expect(page).to have_css ".loading-bar.complete" + end + + expect(page).to have_css ".file-name", text: "empty.pdf" + end + + scenario "Should update nested document file title with + file name after choosing a file when no title defined" do + login_as user + visit path + + documentable_attach_new_file(file_fixture("empty.pdf")) + + expect_document_has_title(0, "empty.pdf") + end + + scenario "Should not update nested document file title with + file name after choosing a file when title already defined" do + login_as user + visit path + + click_link "Add new document" + within "#nested-documents" do + input = find("input[name$='[title]']") + fill_in input[:id], with: "My Title" + attach_file "Choose document", file_fixture("empty.pdf") + + expect(page).to have_css ".loading-bar.complete" + end + + expect_document_has_title(0, "My Title") + end + + scenario "Should update loading bar style after valid file upload" do + login_as user + visit path + + documentable_attach_new_file(file_fixture("empty.pdf")) + + expect(page).to have_css ".loading-bar.complete" + end + + scenario "Should update loading bar style after invalid file upload" do + login_as user + visit path + + documentable_attach_new_file(file_fixture("logo_header.gif"), false) + + expect(page).to have_css ".loading-bar.errors" + end + + scenario "Should update document cached_attachment field after valid file upload" do + login_as user + visit path + + click_link "Add new document" + + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty + + attach_file "Choose document", file_fixture("empty.pdf") + + expect(page).to have_css(".loading-bar.complete") + expect(cached_attachment_field.value).not_to be_empty + end + + scenario "Should not update document cached_attachment field after invalid file upload" do + login_as user + visit path + + documentable_attach_new_file(file_fixture("logo_header.gif"), false) + + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty + end + + scenario "Should show document errors after documentable submit with empty document fields" do + login_as user + visit path + + click_link "Add new document" + click_button submit_button_text + + within "#nested-documents .document-fields" do + expect(page).to have_content("can't be blank", count: 2) + end + end + + scenario "Should delete document after valid file upload and click on remove button" do + login_as user + visit path + + documentable_attach_new_file(file_fixture("empty.pdf")) + click_link "Remove document" + + expect(page).not_to have_css("#nested-documents .document-fields") + end + + scenario "Should show successful notice when resource filled correctly without any nested documents" do + login_as user + visit path + + fill_dashboard_action + 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 + login_as user + visit path + + fill_dashboard_action + + documentable_attach_new_file(file_fixture("empty.pdf")) + click_button submit_button_text + + expect(page).to have_content notice_text + end + end + + describe "When allow attached documents setting is disabled" do + before { Setting["feature.allow_attached_documents"] = false } + + scenario "Add new document button should not be available" do + login_as user + visit path + + expect(page).not_to have_content("Add new document") + end + end + end + + def fill_dashboard_action + fill_in :dashboard_action_title, with: "Dashboard title" + fill_in_ckeditor "Description", with: "Dashboard description" + end +end From b7adf760f6e580ea7fd782caee4782cfabb4a041 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 14 May 2025 15:22:41 +0200 Subject: [PATCH 02/25] Extract budget_investment from shared nested documentable spec to system specs Make 'path', 'submit_button_text' and 'notice_text' dynamic based on the factory. Also adjusted the user. Budget investments require a level 2 user but do not need to be an administrator. Copied and renamed the 'documentable_fill_new_valid_budget_investment' method from common actions, and introduced a 'fill_in_required_fields' method to manage multiple factories. Added the two tests that were conditionally skipped in the shared example using 'unless: documentable_factory_name == "dashboard_action"', but omitted the call to 'documentable_redirected_to_resource_show_or_navigate_to', since it only applies to proposals. Note that when we create the documentable seems do not need use the user as author. --- spec/system/budgets/investments_spec.rb | 9 --- spec/system/nested_documentable_spec.rb | 90 +++++++++++++++++++++++-- 2 files changed, 84 insertions(+), 15 deletions(-) diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 8cd5eaf96..642ef7826 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -1115,15 +1115,6 @@ describe "Budget Investments" do "budget_investment_path", { budget_id: "budget_id", id: "id" } - it_behaves_like "nested documentable", - "user", - "budget_investment", - "new_budget_investment_path", - { budget_id: "budget_id" }, - "documentable_fill_new_valid_budget_investment", - "Create Investment", - "Budget Investment created successfully." - it_behaves_like "mappable", "budget_investment", "investment", diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index aa8749932..eb4c488e9 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -2,18 +2,38 @@ require "rails_helper" describe "Nested documentable" do factories = [ + :budget_investment, :dashboard_action ] let(:factory) { factories.sample } let!(:documentable) { create(factory) } - let!(:user) { create(:administrator).user } - let(:path) { new_admin_dashboard_action_path } - let(:submit_button_text) { "Save" } - let(:notice_text) { "Action created successfully" } + let!(:user) { create(:user, :level_two) } + let(:path) do + case factory + when :budget_investment then new_budget_investment_path(budget_id: documentable.budget_id) + when :dashboard_action then new_admin_dashboard_action_path + end + end + let(:submit_button_text) do + case factory + when :budget_investment then "Create Investment" + when :dashboard_action then "Save" + end + end + let(:notice_text) do + case factory + when :budget_investment then "Budget Investment created successfully." + when :dashboard_action then "Action created successfully" + end + end context "New path" do describe "When allow attached documents setting is enabled" do + before do + create(:administrator, user: user) if admin_section? + end + scenario "Should show new document link when max documents allowed limit is not reached" do login_as user visit path @@ -175,7 +195,7 @@ describe "Nested documentable" do login_as user visit path - fill_dashboard_action + fill_in_required_fields click_button submit_button_text expect(page).to have_content notice_text @@ -185,13 +205,54 @@ describe "Nested documentable" do login_as user visit path - fill_dashboard_action + fill_in_required_fields documentable_attach_new_file(file_fixture("empty.pdf")) click_button submit_button_text expect(page).to have_content notice_text end + + context "Budget investments" do + let(:factory) { (factories - [:dashboard_action]).sample } + + scenario "Should show new document after successful creation with one uploaded file" do + login_as user + visit path + + fill_in_required_fields + + documentable_attach_new_file(file_fixture("empty.pdf")) + click_button submit_button_text + + expect(page).to have_content notice_text + expect(page).to have_content "Documents" + expect(page).to have_content "empty.pdf" + + # Review + # Doble check why the file is stored with a name different to empty.pdf + expect(page).to have_link href: /.pdf\Z/ + end + + scenario "Should show resource with new document after successful creation with + maximum allowed uploaded files" do + login_as user + visit path + + fill_in_required_fields + + %w[clippy empty logo].take(documentable.class.max_documents_allowed).each do |filename| + documentable_attach_new_file(file_fixture("#{filename}.pdf")) + end + + expect(page).not_to have_link "Add new document" + + click_button submit_button_text + + expect(page).to have_content notice_text + expect(page).to have_content "Documents (#{documentable.class.max_documents_allowed})" + end + end end describe "When allow attached documents setting is disabled" do @@ -206,8 +267,25 @@ describe "Nested documentable" do end end + def fill_in_required_fields + case factory + when :budget_investment then fill_budget_investment + when :dashboard_action then fill_dashboard_action + end + end + def fill_dashboard_action fill_in :dashboard_action_title, with: "Dashboard title" fill_in_ckeditor "Description", with: "Dashboard description" 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 + + def admin_section? + path.starts_with?("/admin/") + end end From 12c1d770610bf6471a16aeaa492188f2ee3e9329 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 14 May 2025 16:16:05 +0200 Subject: [PATCH 03/25] Extract management_budget_investment from shared nested documentable spec to system specs Replaced 'login_as' with 'do_login_for' using 'management: management_section?' to handle login requirements correctly for each context. Also removed the now-unused 'documentable_fill_new_valid_budget_investment' helper from common actions. Note that it does not seem necessary to create an administrator with the user, as was done in the original shared example. Also, as in the previous commit, it appears that we do not need to set the user as the author when creating the documentable. While reviewing this, we also noticed that the create(:administrator, user: user) call was unnecessarily included in the nested_imageable system spec in commit cdfaec5217 when the path is a management section. So we use this commit to remove the unnecessary condition. --- spec/support/common_actions/documents.rb | 6 --- .../management/budget_investments_spec.rb | 10 ---- spec/system/nested_documentable_spec.rb | 48 +++++++++++-------- spec/system/nested_imageable_spec.rb | 2 +- 4 files changed, 29 insertions(+), 37 deletions(-) diff --git a/spec/support/common_actions/documents.rb b/spec/support/common_actions/documents.rb index 8f3d77141..4ff31a7ed 100644 --- a/spec/support/common_actions/documents.rb +++ b/spec/support/common_actions/documents.rb @@ -34,10 +34,4 @@ module Documents fill_in "Proposal summary", with: "Proposal summary" check :proposal_terms_of_service end - - def documentable_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/management/budget_investments_spec.rb b/spec/system/management/budget_investments_spec.rb index 5bcf5d256..0e2f76675 100644 --- a/spec/system/management/budget_investments_spec.rb +++ b/spec/system/management/budget_investments_spec.rb @@ -7,16 +7,6 @@ describe "Budget Investments" do let(:heading) { create(:budget_heading, group: group, name: "Health") } let(:user) { create(:user, :level_two) } - it_behaves_like "nested documentable", - "user", - "budget_investment", - "new_management_budget_investment_path", - { budget_id: "budget_id" }, - "documentable_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_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index eb4c488e9..4c1810cef 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -11,7 +11,11 @@ describe "Nested documentable" do let!(:user) { create(:user, :level_two) } let(:path) do case factory - when :budget_investment then new_budget_investment_path(budget_id: documentable.budget_id) + when :budget_investment + [ + new_budget_investment_path(budget_id: documentable.budget_id), + new_management_budget_investment_path(budget_id: documentable.budget_id) + ].sample when :dashboard_action then new_admin_dashboard_action_path end end @@ -35,7 +39,7 @@ describe "Nested documentable" do end scenario "Should show new document link when max documents allowed limit is not reached" do - login_as user + do_login_for(user, management: management_section?) visit path expect(page).to have_link "Add new document" @@ -43,7 +47,7 @@ describe "Nested documentable" do scenario "Should not show new document link when documentable max documents allowed limit is reached" do - login_as user + do_login_for(user, management: management_section?) visit path documentable.class.max_documents_allowed.times.each do @@ -54,14 +58,14 @@ describe "Nested documentable" do end scenario "Should not show max documents warning when no documents added" do - login_as user + do_login_for(user, management: management_section?) visit path expect(page).not_to have_css ".max-documents-notice" end scenario "Should show max documents warning when max documents allowed limit is reached" do - login_as user + do_login_for(user, management: management_section?) visit path documentable.class.max_documents_allowed.times.each do @@ -73,7 +77,7 @@ describe "Nested documentable" do end scenario "Should hide max documents warning after any document removal" do - login_as user + do_login_for(user, management: management_section?) visit path documentable.class.max_documents_allowed.times.each do @@ -86,7 +90,7 @@ describe "Nested documentable" do end scenario "Should update nested document file name after choosing a file" do - login_as user + do_login_for(user, management: management_section?) visit path click_link "Add new document" @@ -101,7 +105,7 @@ describe "Nested documentable" do scenario "Should update nested document file title with file name after choosing a file when no title defined" do - login_as user + do_login_for(user, management: management_section?) visit path documentable_attach_new_file(file_fixture("empty.pdf")) @@ -111,7 +115,7 @@ describe "Nested documentable" do scenario "Should not update nested document file title with file name after choosing a file when title already defined" do - login_as user + do_login_for(user, management: management_section?) visit path click_link "Add new document" @@ -127,7 +131,7 @@ describe "Nested documentable" do end scenario "Should update loading bar style after valid file upload" do - login_as user + do_login_for(user, management: management_section?) visit path documentable_attach_new_file(file_fixture("empty.pdf")) @@ -136,7 +140,7 @@ describe "Nested documentable" do end scenario "Should update loading bar style after invalid file upload" do - login_as user + do_login_for(user, management: management_section?) visit path documentable_attach_new_file(file_fixture("logo_header.gif"), false) @@ -145,7 +149,7 @@ describe "Nested documentable" do end scenario "Should update document cached_attachment field after valid file upload" do - login_as user + do_login_for(user, management: management_section?) visit path click_link "Add new document" @@ -160,7 +164,7 @@ describe "Nested documentable" do end scenario "Should not update document cached_attachment field after invalid file upload" do - login_as user + do_login_for(user, management: management_section?) visit path documentable_attach_new_file(file_fixture("logo_header.gif"), false) @@ -170,7 +174,7 @@ describe "Nested documentable" do end scenario "Should show document errors after documentable submit with empty document fields" do - login_as user + do_login_for(user, management: management_section?) visit path click_link "Add new document" @@ -182,7 +186,7 @@ describe "Nested documentable" do end scenario "Should delete document after valid file upload and click on remove button" do - login_as user + do_login_for(user, management: management_section?) visit path documentable_attach_new_file(file_fixture("empty.pdf")) @@ -192,7 +196,7 @@ describe "Nested documentable" do end scenario "Should show successful notice when resource filled correctly without any nested documents" do - login_as user + do_login_for(user, management: management_section?) visit path fill_in_required_fields @@ -202,7 +206,7 @@ describe "Nested documentable" do end scenario "Should show successful notice when resource filled correctly and after valid file uploads" do - login_as user + do_login_for(user, management: management_section?) visit path fill_in_required_fields @@ -217,7 +221,7 @@ describe "Nested documentable" do let(:factory) { (factories - [:dashboard_action]).sample } scenario "Should show new document after successful creation with one uploaded file" do - login_as user + do_login_for(user, management: management_section?) visit path fill_in_required_fields @@ -236,7 +240,7 @@ describe "Nested documentable" do scenario "Should show resource with new document after successful creation with maximum allowed uploaded files" do - login_as user + do_login_for(user, management: management_section?) visit path fill_in_required_fields @@ -259,7 +263,7 @@ describe "Nested documentable" do before { Setting["feature.allow_attached_documents"] = false } scenario "Add new document button should not be available" do - login_as user + do_login_for(user, management: management_section?) visit path expect(page).not_to have_content("Add new document") @@ -288,4 +292,8 @@ describe "Nested documentable" do def admin_section? path.starts_with?("/admin/") end + + def management_section? + path.starts_with?("/management/") + end end diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 26038a0d0..3cf048407 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -52,7 +52,7 @@ describe "Nested imageable" do context "New and edit path" do before do - create(:administrator, user: user) if admin_section? || management_section? + create(:administrator, user: user) if admin_section? imageable.update!(author: user) if edit_path? do_login_for(user, management: management_section?) visit path From 979171ec45545c20acdcad9832738f5ade74f0b4 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 14 May 2025 17:43:51 +0200 Subject: [PATCH 04/25] Extract proposals from shared nested documentable to system specs Removed the now-unused 'documentable_fill_new_valid_proposal' method from common actions. Note that it does not seem necessary to create an administrator with the user, as was done in the original shared example. Also, as in the previous commit, it appears that we do not need to set the user as the author when creating the documentable. Also removed the documentable_redirected_to_resource_show_or_navigate_to method, which was only used for the :proposal factory but was not necessary. - In the "Proposal new" case (this commit), after submitting the form we are redirected to the "created" page, where the link "Not now, go to my proposal" does not appear. This caused the method to always raise a Capybara::ElementNotFound and return nil. Instead, this "created" page already displays a preview of the proposal and a link to publish it. Since we can verify that the proposal was created successfully here, no redirection or click is needed. - In the "Proposal edit" case (next commit), the user is redirected directly to the proposal's "show" page after update, so again, the method is unnecessary and has been removed. --- spec/shared/system/nested_documentable.rb | 6 ------ spec/support/common_actions/documents.rb | 13 ------------- spec/system/nested_documentable_spec.rb | 15 +++++++++++++-- spec/system/proposals_spec.rb | 9 --------- 4 files changed, 13 insertions(+), 30 deletions(-) diff --git a/spec/shared/system/nested_documentable.rb b/spec/shared/system/nested_documentable.rb index 651e32d29..43b064eaa 100644 --- a/spec/shared/system/nested_documentable.rb +++ b/spec/shared/system/nested_documentable.rb @@ -210,9 +210,6 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na click_button submit_button expect(page).to have_content documentable_success_notice - - documentable_redirected_to_resource_show_or_navigate_to - expect(page).to have_content "Documents" expect(page).to have_content "empty.pdf" @@ -235,9 +232,6 @@ shared_examples "nested documentable" do |login_as_name, documentable_factory_na click_button submit_button expect(page).to have_content documentable_success_notice - - documentable_redirected_to_resource_show_or_navigate_to - expect(page).to have_content "Documents (#{documentable.class.max_documents_allowed})" end diff --git a/spec/support/common_actions/documents.rb b/spec/support/common_actions/documents.rb index 4ff31a7ed..f1dc94d56 100644 --- a/spec/support/common_actions/documents.rb +++ b/spec/support/common_actions/documents.rb @@ -1,11 +1,4 @@ module Documents - def documentable_redirected_to_resource_show_or_navigate_to - find("a", text: "Not now, go to my proposal") - click_link "Not now, go to my proposal" - rescue - nil - end - def documentable_attach_new_file(path, success = true) click_link "Add new document" @@ -28,10 +21,4 @@ module Documents expect(find("input[name$='[title]']").value).to eq title end end - - def documentable_fill_new_valid_proposal - fill_in_new_proposal_title with: "Proposal title #{rand(9999)}" - fill_in "Proposal summary", with: "Proposal summary" - check :proposal_terms_of_service - end end diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 4c1810cef..501d3826e 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -3,7 +3,8 @@ require "rails_helper" describe "Nested documentable" do factories = [ :budget_investment, - :dashboard_action + :dashboard_action, + :proposal ] let(:factory) { factories.sample } @@ -17,18 +18,21 @@ describe "Nested documentable" do new_management_budget_investment_path(budget_id: documentable.budget_id) ].sample when :dashboard_action then new_admin_dashboard_action_path + when :proposal then new_proposal_path end end let(:submit_button_text) do case factory when :budget_investment then "Create Investment" when :dashboard_action then "Save" + when :proposal then "Create proposal" end end let(:notice_text) do case factory when :budget_investment then "Budget Investment created successfully." when :dashboard_action then "Action created successfully" + when :proposal then "Proposal created successfully" end end @@ -217,7 +221,7 @@ describe "Nested documentable" do expect(page).to have_content notice_text end - context "Budget investments" do + context "Budget investments and proposals" do let(:factory) { (factories - [:dashboard_action]).sample } scenario "Should show new document after successful creation with one uploaded file" do @@ -275,6 +279,7 @@ describe "Nested documentable" do case factory when :budget_investment then fill_budget_investment when :dashboard_action then fill_dashboard_action + when :proposal then fill_proposal end end @@ -289,6 +294,12 @@ describe "Nested documentable" do check :budget_investment_terms_of_service end + def fill_proposal + fill_in_new_proposal_title with: "Proposal title #{rand(9999)}" + fill_in "Proposal summary", with: "Proposal summary" + check :proposal_terms_of_service + end + def admin_section? path.starts_with?("/admin/") end diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index dc9912e05..1df049276 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -1297,15 +1297,6 @@ describe "Proposals" do it_behaves_like "documentable", "proposal", "proposal_path", { id: "id" } - it_behaves_like "nested documentable", - "user", - "proposal", - "new_proposal_path", - {}, - "documentable_fill_new_valid_proposal", - "Create proposal", - "Proposal created successfully" - it_behaves_like "nested documentable", "user", "proposal", From e7c5b03d8c35a275758fae535120fcc61bb8ab60 Mon Sep 17 00:00:00 2001 From: taitus Date: Wed, 14 May 2025 18:22:51 +0200 Subject: [PATCH 05/25] Extract edit proposals from shared nested documentable to system specs We can remove shared nested documentable file because has not references. --- spec/shared/system/nested_documentable.rb | 311 ---------------------- spec/support/common_actions/documents.rb | 8 - spec/system/nested_documentable_spec.rb | 99 ++++++- spec/system/proposals_spec.rb | 9 - 4 files changed, 95 insertions(+), 332 deletions(-) delete mode 100644 spec/shared/system/nested_documentable.rb diff --git a/spec/shared/system/nested_documentable.rb b/spec/shared/system/nested_documentable.rb deleted file mode 100644 index 43b064eaa..000000000 --- a/spec/shared/system/nested_documentable.rb +++ /dev/null @@ -1,311 +0,0 @@ -shared_examples "nested documentable" do |login_as_name, documentable_factory_name, path, - documentable_path_arguments, fill_resource_method_name, - submit_button, documentable_success_notice, management: false| - let!(:administrator) { create(:user) } - let!(:user) { create(:user, :level_two) } - let!(:arguments) { {} } - if documentable_factory_name == "dashboard_action" - let!(:documentable) { create(documentable_factory_name) } - else - let!(:documentable) { create(documentable_factory_name, author: user) } - end - let!(:user_to_login) { send(login_as_name) } - let(:management) { management } - - before do - create(:administrator, user: administrator) - - documentable_path_arguments&.each do |argument_name, path_to_value| - arguments.merge!("#{argument_name}": documentable.send(path_to_value)) - end - end - - describe "at #{path}" do - scenario "Should show new document link when max documents allowed limit is not reached" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - expect(page).to have_link id: "new_document_link" - end - - scenario "Should not show new document link when - documentable max documents allowed limit is reached" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - documentable.class.max_documents_allowed.times.each do - click_link "Add new document" - end - - expect(page).not_to have_css "#new_document_link" - end - - scenario "Should not show max documents warning when no documents added" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - expect(page).not_to have_css ".max-documents-notice" - end - - scenario "Should show max documents warning when max documents allowed limit is reached" do - do_login_for user_to_login, management: management - visit send(path, arguments) - documentable.class.max_documents_allowed.times.each do - documentable_attach_new_file(file_fixture("empty.pdf")) - end - - expect(page).to have_css ".max-documents-notice" - expect(page).to have_content "Remove document" - end - - scenario "Should hide max documents warning after any document removal" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - documentable.class.max_documents_allowed.times.each do - click_link "Add new document" - end - - all("a", text: "Cancel").last.click - - expect(page).not_to have_css ".max-documents-notice" - end - - scenario "Should update nested document file name after choosing a file" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - click_link "Add new document" - within "#nested-documents" do - attach_file "Choose document", file_fixture("empty.pdf") - - expect(page).to have_css ".loading-bar.complete" - end - - expect(page).to have_css ".file-name", text: "empty.pdf" - end - - scenario "Should update nested document file title with - file name after choosing a file when no title defined" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - documentable_attach_new_file(file_fixture("empty.pdf")) - - expect_document_has_title(0, "empty.pdf") - end - - scenario "Should not update nested document file title with - file name after choosing a file when title already defined" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - click_link "Add new document" - within "#nested-documents" do - input = find("input[name$='[title]']") - fill_in input[:id], with: "My Title" - attach_file "Choose document", file_fixture("empty.pdf") - - expect(page).to have_css ".loading-bar.complete" - end - - expect_document_has_title(0, "My Title") - end - - scenario "Should update loading bar style after valid file upload" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - documentable_attach_new_file(file_fixture("empty.pdf")) - - expect(page).to have_css ".loading-bar.complete" - end - - scenario "Should update loading bar style after invalid file upload" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - documentable_attach_new_file(file_fixture("logo_header.gif"), false) - - expect(page).to have_css ".loading-bar.errors" - end - - scenario "Should update document cached_attachment field after valid file upload" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - click_link "Add new document" - - cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) - expect(cached_attachment_field.value).to be_empty - - attach_file "Choose document", file_fixture("empty.pdf") - - expect(page).to have_css(".loading-bar.complete") - expect(cached_attachment_field.value).not_to be_empty - end - - scenario "Should not update document cached_attachment field after invalid file upload" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - documentable_attach_new_file(file_fixture("logo_header.gif"), false) - - cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) - expect(cached_attachment_field.value).to be_empty - end - - scenario "Should show document errors after documentable submit with - empty document fields" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - click_link "Add new document" - click_button submit_button - - within "#nested-documents .document-fields" do - expect(page).to have_content("can't be blank", count: 2) - end - end - - scenario "Should delete document after valid file upload and click on remove button" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - documentable_attach_new_file(file_fixture("empty.pdf")) - click_link "Remove document" - - expect(page).not_to have_css("#nested-documents .document-fields") - end - - scenario "Should show successful notice when - resource filled correctly without any nested documents" do - do_login_for user_to_login, 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 documentable_success_notice - end - - scenario "Should show successful notice when - resource filled correctly and after valid file uploads" do - do_login_for user_to_login, management: management - visit send(path, arguments) - send(fill_resource_method_name) if fill_resource_method_name - - documentable_attach_new_file(file_fixture("empty.pdf")) - click_button submit_button - - expect(page).to have_content documentable_success_notice - end - - scenario "Should show new document after successful creation with one uploaded file" do - do_login_for user_to_login, management: management - visit send(path, arguments) - send(fill_resource_method_name) if fill_resource_method_name - - documentable_attach_new_file(file_fixture("empty.pdf")) - click_button submit_button - - expect(page).to have_content documentable_success_notice - expect(page).to have_content "Documents" - expect(page).to have_content "empty.pdf" - - # Review - # Doble check why the file is stored with a name different to empty.pdf - expect(page).to have_link href: /.pdf\Z/ - end - - scenario "Should show resource with new document after successful creation with - maximum allowed uploaded files" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - send(fill_resource_method_name) if fill_resource_method_name - - %w[clippy empty logo].take(documentable.class.max_documents_allowed).each do |filename| - documentable_attach_new_file(file_fixture("#{filename}.pdf")) - end - - click_button submit_button - - expect(page).to have_content documentable_success_notice - expect(page).to have_content "Documents (#{documentable.class.max_documents_allowed})" - end - - if path.include? "edit" - scenario "Should show persisted documents and remove nested_field" do - create(:document, documentable: documentable) - do_login_for user_to_login, management: management - visit send(path, arguments) - - expect(page).to have_css ".document-fields", count: 1 - end - - scenario "Should not show add document button when - documentable has reached maximum of documents allowed" do - create_list(:document, documentable.class.max_documents_allowed, documentable: documentable) - do_login_for user_to_login, management: management - visit send(path, arguments) - - expect(page).not_to have_css "#new_document_link" - end - - scenario "Should show add document button after destroy one document" do - create_list(:document, documentable.class.max_documents_allowed, documentable: documentable) - do_login_for user_to_login, management: management - visit send(path, arguments) - last_document = all("#nested-documents .document-fields").last - within last_document do - click_link "Remove document" - end - - expect(page).to have_link id: "new_document_link" - end - - scenario "Should remove nested field after remove document" do - create(:document, documentable: documentable) - do_login_for user_to_login, management: management - visit send(path, arguments) - click_link "Remove document" - - expect(page).not_to have_css ".document-fields" - end - - scenario "Same attachment URL after editing the title" do - do_login_for user_to_login, management: management - - visit send(path, arguments) - documentable_attach_new_file(file_fixture("empty.pdf")) - within_fieldset("Documents") { fill_in "Title", with: "Original" } - click_button submit_button - - expect(page).to have_content documentable_success_notice - - original_url = find_link(text: "Original")[:href] - - visit send(path, arguments) - within_fieldset("Documents") { fill_in "Title", with: "Updated" } - click_button submit_button - - expect(page).to have_content documentable_success_notice - expect(find_link(text: "Updated")[:href]).to eq original_url - end - end - - describe "When allow attached documents setting is disabled" do - before do - Setting["feature.allow_attached_documents"] = false - end - - scenario "Add new document button should not be available" do - do_login_for user_to_login, management: management - visit send(path, arguments) - - expect(page).not_to have_content("Add new document") - end - end - end -end diff --git a/spec/support/common_actions/documents.rb b/spec/support/common_actions/documents.rb index f1dc94d56..e26cf5756 100644 --- a/spec/support/common_actions/documents.rb +++ b/spec/support/common_actions/documents.rb @@ -13,12 +13,4 @@ module Documents end end end - - def expect_document_has_title(index, title) - document = all(".document-fields")[index] - - within document do - expect(find("input[name$='[title]']").value).to eq title - end - end end diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 501d3826e..19642ac15 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -18,28 +18,43 @@ describe "Nested documentable" do new_management_budget_investment_path(budget_id: documentable.budget_id) ].sample when :dashboard_action then new_admin_dashboard_action_path - when :proposal then new_proposal_path + when :proposal + [ + new_proposal_path, + edit_proposal_path(id: documentable.id) + ].sample end end let(:submit_button_text) do case factory when :budget_investment then "Create Investment" when :dashboard_action then "Save" - when :proposal then "Create proposal" + when :proposal + if edit_path? + "Save changes" + else + "Create proposal" + end end end let(:notice_text) do case factory when :budget_investment then "Budget Investment created successfully." when :dashboard_action then "Action created successfully" - when :proposal then "Proposal created successfully" + when :proposal + if edit_path? + "Proposal updated successfully" + else + "Proposal created successfully" + end end end - context "New path" do + context "New and edit path" do describe "When allow attached documents setting is enabled" do before do create(:administrator, user: user) if admin_section? + documentable.update!(author: user) if edit_path? end scenario "Should show new document link when max documents allowed limit is not reached" do @@ -263,6 +278,68 @@ describe "Nested documentable" do end end + describe "Only for edit path" do + let!(:proposal) { create(:proposal, author: user) } + + scenario "Should show persisted documents and remove nested_field" do + create(:document, documentable: proposal) + login_as user + visit edit_proposal_path(proposal) + + expect(page).to have_css ".document-fields", count: 1 + end + + scenario "Should not show add document button when + documentable has reached maximum of documents allowed" do + create_list(:document, proposal.class.max_documents_allowed, documentable: proposal) + login_as user + visit edit_proposal_path(proposal) + + expect(page).not_to have_css "#new_document_link" + end + + scenario "Should show add document button after destroy one document" do + create_list(:document, proposal.class.max_documents_allowed, documentable: proposal) + login_as user + visit edit_proposal_path(proposal) + last_document = all("#nested-documents .document-fields").last + within last_document do + click_link "Remove document" + end + + expect(page).to have_link "Add new document" + end + + scenario "Should remove nested field after remove document" do + create(:document, documentable: proposal) + login_as user + visit edit_proposal_path(proposal) + click_link "Remove document" + + expect(page).not_to have_css ".document-fields" + end + + scenario "Same attachment URL after editing the title" do + login_as user + visit edit_proposal_path(proposal) + + documentable_attach_new_file(file_fixture("empty.pdf")) + within_fieldset("Documents") { fill_in "Title", with: "Original" } + click_button "Save changes" + + expect(page).to have_content "Proposal updated successfully" + + original_url = find_link(text: "Original")[:href] + + visit edit_proposal_path(proposal) + within_fieldset("Documents") { fill_in "Title", with: "Updated" } + click_button "Save changes" + + expect(page).to have_content "Proposal updated successfully" + expect(find_link(text: "Updated")[:href]).to eq original_url + end + end + describe "When allow attached documents setting is disabled" do before { Setting["feature.allow_attached_documents"] = false } @@ -276,6 +353,8 @@ describe "Nested documentable" do end def fill_in_required_fields + return if edit_path? + case factory when :budget_investment then fill_budget_investment when :dashboard_action then fill_dashboard_action @@ -307,4 +386,16 @@ describe "Nested documentable" do def management_section? path.starts_with?("/management/") end + + def edit_path? + path.ends_with?("/edit") + end + + def expect_document_has_title(index, title) + document = all(".document-fields")[index] + + within document do + expect(find("input[name$='[title]']").value).to eq title + end + end end diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index 1df049276..4e80dbbcc 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -1297,15 +1297,6 @@ describe "Proposals" do it_behaves_like "documentable", "proposal", "proposal_path", { id: "id" } - it_behaves_like "nested documentable", - "user", - "proposal", - "edit_proposal_path", - { id: "id" }, - nil, - "Save changes", - "Proposal updated successfully" - it_behaves_like "mappable", "proposal", "proposal", From 2cc23309b8dd6f4cdafc5597e7cb26adea88ffb7 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 15 May 2025 15:40:12 +0200 Subject: [PATCH 06/25] Unify attachables methods --- spec/support/common_actions.rb | 3 +-- spec/support/common_actions/attachables.rb | 31 ++++++++++++++++++++++ spec/support/common_actions/documents.rb | 16 ----------- spec/support/common_actions/images.rb | 16 ----------- spec/system/nested_documentable_spec.rb | 4 +-- spec/system/nested_imageable_spec.rb | 2 +- 6 files changed, 35 insertions(+), 37 deletions(-) create mode 100644 spec/support/common_actions/attachables.rb delete mode 100644 spec/support/common_actions/documents.rb delete mode 100644 spec/support/common_actions/images.rb diff --git a/spec/support/common_actions.rb b/spec/support/common_actions.rb index b12fe944c..07586dd5e 100644 --- a/spec/support/common_actions.rb +++ b/spec/support/common_actions.rb @@ -2,14 +2,13 @@ Dir["./spec/support/common_actions/*.rb"].each { |f| require f } Dir["./spec/support/common_actions/custom/*.rb"].each { |f| require f } module CommonActions + include Attachables include Budgets include Comments include Cookies include Debates - include Documents include Emails include GraphQLAPI - include Images include Maps include Notifications include Polls diff --git a/spec/support/common_actions/attachables.rb b/spec/support/common_actions/attachables.rb new file mode 100644 index 000000000..c6df4f705 --- /dev/null +++ b/spec/support/common_actions/attachables.rb @@ -0,0 +1,31 @@ +module Attachables + def imageable_attach_new_file(path, success: true) + click_link "Add image" + within "#nested-image" do + image = find(".image-fields") + attach_file "Choose image", path + within image do + if success + expect(page).to have_css(".loading-bar.complete") + else + expect(page).to have_css(".loading-bar.errors") + end + end + end + end + + def documentable_attach_new_file(path, success: true) + click_link "Add new document" + + document = all(".document-fields").last + attach_file "Choose document", path + + within document do + if success + expect(page).to have_css ".loading-bar.complete" + else + expect(page).to have_css ".loading-bar.errors" + end + end + end +end diff --git a/spec/support/common_actions/documents.rb b/spec/support/common_actions/documents.rb deleted file mode 100644 index e26cf5756..000000000 --- a/spec/support/common_actions/documents.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Documents - def documentable_attach_new_file(path, success = true) - click_link "Add new document" - - document = all(".document-fields").last - attach_file "Choose document", path - - within document do - if success - expect(page).to have_css ".loading-bar.complete" - else - expect(page).to have_css ".loading-bar.errors" - end - end - end -end diff --git a/spec/support/common_actions/images.rb b/spec/support/common_actions/images.rb deleted file mode 100644 index df59a48cb..000000000 --- a/spec/support/common_actions/images.rb +++ /dev/null @@ -1,16 +0,0 @@ -module Images - def imageable_attach_new_file(path, success = true) - click_link "Add image" - within "#nested-image" do - image = find(".image-fields") - attach_file "Choose image", path - within image do - if success - expect(page).to have_css(".loading-bar.complete") - else - expect(page).to have_css(".loading-bar.errors") - end - end - end - end -end diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 19642ac15..283e2fc5c 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -162,7 +162,7 @@ describe "Nested documentable" do do_login_for(user, management: management_section?) visit path - documentable_attach_new_file(file_fixture("logo_header.gif"), false) + documentable_attach_new_file(file_fixture("logo_header.gif"), success: false) expect(page).to have_css ".loading-bar.errors" end @@ -186,7 +186,7 @@ describe "Nested documentable" do do_login_for(user, management: management_section?) visit path - documentable_attach_new_file(file_fixture("logo_header.gif"), false) + documentable_attach_new_file(file_fixture("logo_header.gif"), success: false) cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) expect(cached_attachment_field.value).to be_empty diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 3cf048407..072ba42d8 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -96,7 +96,7 @@ describe "Nested imageable" do end scenario "Should not update image cached_attachment field after invalid file upload" do - imageable_attach_new_file(file_fixture("logo_header.png"), false) + imageable_attach_new_file(file_fixture("logo_header.png"), success: false) cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) From addda0c77319f9285e65ac2434cafe6c38eff099 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 15 May 2025 17:16:47 +0200 Subject: [PATCH 07/25] Unify duplicated methods in common_actions folder Replaced the user with a level_two one in the "Default whole city" test to bypass the full name requirement, which only applies to unverified users. --- spec/shared/system/mappable.rb | 12 +-- spec/support/common_actions.rb | 24 ++++-- spec/support/common_actions/attachables.rb | 23 ++++++ spec/support/common_actions/maps.rb | 13 --- spec/system/nested_documentable_spec.rb | 93 +++++++--------------- spec/system/nested_imageable_spec.rb | 54 ++----------- spec/system/proposals_spec.rb | 2 +- 7 files changed, 83 insertions(+), 138 deletions(-) diff --git a/spec/shared/system/mappable.rb b/spec/shared/system/mappable.rb index 680c15ea7..7669fdb08 100644 --- a/spec/shared/system/mappable.rb +++ b/spec/shared/system/mappable.rb @@ -21,7 +21,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management visit send(mappable_new_path, arguments) - send("fill_in_#{mappable_factory_name}_form") + send("fill_in_#{mappable_factory_name}") within ".map-location" do expect(page).not_to have_css(".map-icon") @@ -32,7 +32,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management visit send(mappable_new_path, arguments) - send("fill_in_#{mappable_factory_name}_form") + send("fill_in_#{mappable_factory_name}") find("#new_map_location").click within ".map-location" do @@ -44,7 +44,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management visit send(mappable_new_path, arguments) - send("fill_in_#{mappable_factory_name}_form") + send("fill_in_#{mappable_factory_name}") find("#new_map_location").click send("submit_#{mappable_factory_name}_form") @@ -57,7 +57,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management visit send(mappable_new_path, arguments) - send("fill_in_#{mappable_factory_name}_form") + send("fill_in_#{mappable_factory_name}") expect(page).to have_css ".map-location" send("submit_#{mappable_factory_name}_form") @@ -69,7 +69,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management visit send(mappable_new_path, arguments) - send("fill_in_#{mappable_factory_name}_form") + send("fill_in_#{mappable_factory_name}") expect(page).not_to have_css ".map-location" send("submit_#{mappable_factory_name}_form") @@ -171,7 +171,7 @@ shared_examples "mappable" do |mappable_factory_name, mappable_association_name, do_login_for user, management: management visit send(mappable_new_path, arguments) - send("fill_in_#{mappable_factory_name}_form") + send("fill_in_#{mappable_factory_name}") send("submit_#{mappable_factory_name}_form") expect(page).not_to have_content "Map location can't be blank" diff --git a/spec/support/common_actions.rb b/spec/support/common_actions.rb index 07586dd5e..d1411f2bf 100644 --- a/spec/support/common_actions.rb +++ b/spec/support/common_actions.rb @@ -42,12 +42,24 @@ module CommonActions end def fill_in_proposal - fill_in_new_proposal_title with: "Help refugees" - fill_in "Proposal summary", with: "In summary, what we want is..." - fill_in_ckeditor "Proposal text", with: "This is very important because..." - fill_in "External video URL", with: "https://www.youtube.com/watch?v=yPQfcG-eimk" - fill_in "Full name of the person submitting the proposal", with: "Isabel Garcia" - check "I agree to the Privacy Policy and the Terms and conditions of use" + fill_in_new_proposal_title with: "Proposal title" + fill_in "Proposal summary", with: "Proposal summary" + check :proposal_terms_of_service + end + + def fill_in_budget + fill_in "Name", with: "Budget name" + end + + def fill_in_dashboard_action + fill_in :dashboard_action_title, with: "Dashboard title" + fill_in_ckeditor "Description", with: "Dashboard description" + end + + def fill_in_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 def fill_in_new_proposal_title(with:) diff --git a/spec/support/common_actions/attachables.rb b/spec/support/common_actions/attachables.rb index c6df4f705..75e9e1ffa 100644 --- a/spec/support/common_actions/attachables.rb +++ b/spec/support/common_actions/attachables.rb @@ -28,4 +28,27 @@ module Attachables end end end + + def admin_section?(path) + path.starts_with?("/admin/") + end + + def management_section?(path) + path.starts_with?("/management/") + end + + def edit_path?(path) + path.ends_with?("/edit") + end + + def fill_in_required_fields(factory, path) + return if edit_path?(path) + + case factory + when :budget then fill_in_budget + when :budget_investment then fill_in_budget_investment + when :dashboard_action then fill_in_dashboard_action + when :proposal then fill_in_proposal + end + end end diff --git a/spec/support/common_actions/maps.rb b/spec/support/common_actions/maps.rb index bde0f93c6..15aa6ff6e 100644 --- a/spec/support/common_actions/maps.rb +++ b/spec/support/common_actions/maps.rb @@ -1,11 +1,5 @@ module Maps - def fill_in_proposal_form - fill_in_new_proposal_title with: "Help refugees" - fill_in "Proposal summary", with: "In summary, what we want is..." - end - def submit_proposal_form - check :proposal_terms_of_service click_button "Create proposal" expect(page).to have_content "Proposal created successfully." @@ -16,14 +10,7 @@ module Maps end end - def fill_in_budget_investment_form - fill_in_new_investment_title with: "Budget investment title" - fill_in_ckeditor "Description", with: "Budget investment description" - check :budget_investment_terms_of_service - end - def submit_budget_investment_form - check :budget_investment_terms_of_service click_button "Create Investment" expect(page).to have_content "Budget Investment created successfully" end diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 283e2fc5c..d464da133 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -30,7 +30,7 @@ describe "Nested documentable" do when :budget_investment then "Create Investment" when :dashboard_action then "Save" when :proposal - if edit_path? + if edit_path?(path) "Save changes" else "Create proposal" @@ -42,7 +42,7 @@ describe "Nested documentable" do when :budget_investment then "Budget Investment created successfully." when :dashboard_action then "Action created successfully" when :proposal - if edit_path? + if edit_path?(path) "Proposal updated successfully" else "Proposal created successfully" @@ -53,12 +53,12 @@ describe "Nested documentable" do context "New and edit path" do describe "When allow attached documents setting is enabled" do before do - create(:administrator, user: user) if admin_section? - documentable.update!(author: user) if edit_path? + create(:administrator, user: user) if admin_section?(path) + documentable.update!(author: user) if edit_path?(path) end scenario "Should show new document link when max documents allowed limit is not reached" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path expect(page).to have_link "Add new document" @@ -66,7 +66,7 @@ describe "Nested documentable" do scenario "Should not show new document link when documentable max documents allowed limit is reached" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path documentable.class.max_documents_allowed.times.each do @@ -77,14 +77,14 @@ describe "Nested documentable" do end scenario "Should not show max documents warning when no documents added" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path expect(page).not_to have_css ".max-documents-notice" end scenario "Should show max documents warning when max documents allowed limit is reached" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path documentable.class.max_documents_allowed.times.each do @@ -96,7 +96,7 @@ describe "Nested documentable" do end scenario "Should hide max documents warning after any document removal" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path documentable.class.max_documents_allowed.times.each do @@ -109,7 +109,7 @@ describe "Nested documentable" do end scenario "Should update nested document file name after choosing a file" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path click_link "Add new document" @@ -124,7 +124,7 @@ describe "Nested documentable" do scenario "Should update nested document file title with file name after choosing a file when no title defined" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path documentable_attach_new_file(file_fixture("empty.pdf")) @@ -134,7 +134,7 @@ describe "Nested documentable" do scenario "Should not update nested document file title with file name after choosing a file when title already defined" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path click_link "Add new document" @@ -150,7 +150,7 @@ describe "Nested documentable" do end scenario "Should update loading bar style after valid file upload" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path documentable_attach_new_file(file_fixture("empty.pdf")) @@ -159,7 +159,7 @@ describe "Nested documentable" do end scenario "Should update loading bar style after invalid file upload" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path documentable_attach_new_file(file_fixture("logo_header.gif"), success: false) @@ -168,7 +168,7 @@ describe "Nested documentable" do end scenario "Should update document cached_attachment field after valid file upload" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path click_link "Add new document" @@ -183,7 +183,7 @@ describe "Nested documentable" do end scenario "Should not update document cached_attachment field after invalid file upload" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path documentable_attach_new_file(file_fixture("logo_header.gif"), success: false) @@ -193,7 +193,7 @@ describe "Nested documentable" do end scenario "Should show document errors after documentable submit with empty document fields" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path click_link "Add new document" @@ -205,7 +205,7 @@ describe "Nested documentable" do end scenario "Should delete document after valid file upload and click on remove button" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path documentable_attach_new_file(file_fixture("empty.pdf")) @@ -215,20 +215,20 @@ describe "Nested documentable" do end scenario "Should show successful notice when resource filled correctly without any nested documents" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path - fill_in_required_fields + fill_in_required_fields(factory, path) 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 - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path - fill_in_required_fields + fill_in_required_fields(factory, path) documentable_attach_new_file(file_fixture("empty.pdf")) click_button submit_button_text @@ -240,10 +240,10 @@ describe "Nested documentable" do let(:factory) { (factories - [:dashboard_action]).sample } scenario "Should show new document after successful creation with one uploaded file" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path - fill_in_required_fields + fill_in_required_fields(factory, path) documentable_attach_new_file(file_fixture("empty.pdf")) click_button submit_button_text @@ -259,10 +259,10 @@ describe "Nested documentable" do scenario "Should show resource with new document after successful creation with maximum allowed uploaded files" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path - fill_in_required_fields + fill_in_required_fields(factory, path) %w[clippy empty logo].take(documentable.class.max_documents_allowed).each do |filename| documentable_attach_new_file(file_fixture("#{filename}.pdf")) @@ -344,7 +344,7 @@ describe "Nested documentable" do before { Setting["feature.allow_attached_documents"] = false } scenario "Add new document button should not be available" do - do_login_for(user, management: management_section?) + do_login_for(user, management: management_section?(path)) visit path expect(page).not_to have_content("Add new document") @@ -352,45 +352,6 @@ describe "Nested documentable" do end end - def fill_in_required_fields - return if edit_path? - - case factory - when :budget_investment then fill_budget_investment - when :dashboard_action then fill_dashboard_action - when :proposal then fill_proposal - end - end - - def fill_dashboard_action - fill_in :dashboard_action_title, with: "Dashboard title" - fill_in_ckeditor "Description", with: "Dashboard description" - 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 - - def fill_proposal - fill_in_new_proposal_title with: "Proposal title #{rand(9999)}" - fill_in "Proposal summary", with: "Proposal summary" - check :proposal_terms_of_service - end - - def admin_section? - path.starts_with?("/admin/") - end - - def management_section? - path.starts_with?("/management/") - end - - def edit_path? - path.ends_with?("/edit") - end - def expect_document_has_title(index, title) document = all(".document-fields")[index] diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 072ba42d8..0986ff182 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -29,7 +29,7 @@ describe "Nested imageable" do when :budget_investment then "Create Investment" when :future_poll_question_option then "Save image" when :proposal - if edit_path? + if edit_path?(path) "Save changes" else "Create proposal" @@ -42,7 +42,7 @@ describe "Nested imageable" do when :budget_investment then "Budget Investment created successfully." when :future_poll_question_option then "Image uploaded successfully" when :proposal - if edit_path? + if edit_path?(path) "Proposal updated successfully" else "Proposal created successfully" @@ -52,9 +52,9 @@ describe "Nested imageable" do context "New and edit path" do before do - create(:administrator, user: user) if admin_section? - imageable.update!(author: user) if edit_path? - do_login_for(user, management: management_section?) + create(:administrator, user: user) if admin_section?(path) + imageable.update!(author: user) if edit_path?(path) + do_login_for(user, management: management_section?(path)) visit path end @@ -136,7 +136,7 @@ describe "Nested imageable" 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 + fill_in_required_fields(factory, path) click_button submit_button_text @@ -145,7 +145,7 @@ describe "Nested imageable" do end scenario "Should show successful notice when resource filled correctly and after valid file uploads" do - fill_in_required_fields + fill_in_required_fields(factory, path) imageable_attach_new_file(file_fixture("clippy.jpg")) @@ -155,7 +155,7 @@ describe "Nested imageable" do end scenario "Should show new image after successful creation with one uploaded file" do - fill_in_required_fields + fill_in_required_fields(factory, path) imageable_attach_new_file(file_fixture("clippy.jpg")) @@ -205,42 +205,4 @@ describe "Nested imageable" do expect(page).to have_css ".image-fields", count: 1, visible: :all end end - - def fill_in_required_fields - return if edit_path? - - case factory - when :budget then fill_budget - when :budget_investment then fill_budget_investment - 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 - - 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 - - def admin_section? - path.starts_with?("/admin/") - end - - 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 4e80dbbcc..bcb50ab13 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -559,7 +559,7 @@ describe "Proposals" do scenario "Default whole city" do create(:geozone) - author = create(:user) + author = create(:user, :level_two) login_as(author) visit new_proposal_path From 82dac1225c7ae2107f941c968d9d78fa4bd92365 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 10:32:21 +0200 Subject: [PATCH 08/25] Simplify tests by setting "max documents allowed" to 1 Note that this commit also applies a similar change to the imageable_attach_new_file method by removing an unnecessary variable. In order to testing with more than 1 max documents allowed we keep one test with this value. --- spec/support/common_actions/attachables.rb | 7 ++---- spec/system/nested_documentable_spec.rb | 26 ++++++++++++---------- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/spec/support/common_actions/attachables.rb b/spec/support/common_actions/attachables.rb index 75e9e1ffa..b0429c9fc 100644 --- a/spec/support/common_actions/attachables.rb +++ b/spec/support/common_actions/attachables.rb @@ -2,9 +2,8 @@ module Attachables def imageable_attach_new_file(path, success: true) click_link "Add image" within "#nested-image" do - image = find(".image-fields") attach_file "Choose image", path - within image do + within ".image-fields" do if success expect(page).to have_css(".loading-bar.complete") else @@ -16,11 +15,9 @@ module Attachables def documentable_attach_new_file(path, success: true) click_link "Add new document" - - document = all(".document-fields").last attach_file "Choose document", path - within document do + within ".document-fields" do if success expect(page).to have_css ".loading-bar.complete" else diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index d464da133..3e2b5bf1c 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -53,6 +53,7 @@ describe "Nested documentable" do context "New and edit path" do describe "When allow attached documents setting is enabled" do before do + Setting["uploads.documents.max_amount"] = 1 create(:administrator, user: user) if admin_section?(path) documentable.update!(author: user) if edit_path?(path) end @@ -69,9 +70,7 @@ describe "Nested documentable" do do_login_for(user, management: management_section?(path)) visit path - documentable.class.max_documents_allowed.times.each do - click_link "Add new document" - end + click_link "Add new document" expect(page).not_to have_css "#new_document_link" end @@ -87,9 +86,7 @@ describe "Nested documentable" do do_login_for(user, management: management_section?(path)) visit path - documentable.class.max_documents_allowed.times.each do - documentable_attach_new_file(file_fixture("empty.pdf")) - end + documentable_attach_new_file(file_fixture("empty.pdf")) expect(page).to have_css ".max-documents-notice" expect(page).to have_content "Remove document" @@ -99,9 +96,7 @@ describe "Nested documentable" do do_login_for(user, management: management_section?(path)) visit path - documentable.class.max_documents_allowed.times.each do - click_link "Add new document" - end + click_link "Add new document" all("a", text: "Cancel").last.click @@ -259,13 +254,20 @@ describe "Nested documentable" do scenario "Should show resource with new document after successful creation with maximum allowed uploaded files" do + Setting["uploads.documents.max_amount"] = 3 do_login_for(user, management: management_section?(path)) visit path fill_in_required_fields(factory, path) - %w[clippy empty logo].take(documentable.class.max_documents_allowed).each do |filename| - documentable_attach_new_file(file_fixture("#{filename}.pdf")) + %w[clippy empty logo].each do |filename| + click_link "Add new document" + + attach_file "Choose document", file_fixture("#{filename}.pdf") + + within all(".document-fields").last do + expect(page).to have_css ".loading-bar.complete" + end end expect(page).not_to have_link "Add new document" @@ -273,7 +275,7 @@ describe "Nested documentable" do click_button submit_button_text expect(page).to have_content notice_text - expect(page).to have_content "Documents (#{documentable.class.max_documents_allowed})" + expect(page).to have_content "Documents (3)" end end end From a4e7b702273eaa3d0e6387b365247547224051dc Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 10:33:52 +0200 Subject: [PATCH 09/25] Unify tests related with shows or hides new document link --- spec/system/nested_documentable_spec.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 3e2b5bf1c..0d7cbe6b0 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -58,21 +58,15 @@ describe "Nested documentable" do documentable.update!(author: user) if edit_path?(path) end - scenario "Should show new document link when max documents allowed limit is not reached" do + scenario "Shows or hides new document link depending on max documents limit" do do_login_for(user, management: management_section?(path)) visit path expect(page).to have_link "Add new document" - end - - scenario "Should not show new document link when - documentable max documents allowed limit is reached" do - do_login_for(user, management: management_section?(path)) - visit path click_link "Add new document" - expect(page).not_to have_css "#new_document_link" + expect(page).not_to have_link "Add new document" end scenario "Should not show max documents warning when no documents added" do From 64c1c59a7aba7c1bc0986e754898d34409272325 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 10:36:12 +0200 Subject: [PATCH 10/25] Unify tests related with shows or hides max documents warning --- spec/system/nested_documentable_spec.rb | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 0d7cbe6b0..11cf80428 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -69,30 +69,18 @@ describe "Nested documentable" do expect(page).not_to have_link "Add new document" end - scenario "Should not show max documents warning when no documents added" do + scenario "Shows or hides max documents warning depending on max documents limit" do do_login_for(user, management: management_section?(path)) visit path expect(page).not_to have_css ".max-documents-notice" - end - - scenario "Should show max documents warning when max documents allowed limit is reached" do - do_login_for(user, management: management_section?(path)) - visit path + expect(page).not_to have_content "Remove document" documentable_attach_new_file(file_fixture("empty.pdf")) expect(page).to have_css ".max-documents-notice" - expect(page).to have_content "Remove document" - end - scenario "Should hide max documents warning after any document removal" do - do_login_for(user, management: management_section?(path)) - visit path - - click_link "Add new document" - - all("a", text: "Cancel").last.click + click_link "Remove document" expect(page).not_to have_css ".max-documents-notice" end From 82296a33e2d0af7884d261c6b4fb3b04fa19e427 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 11:16:02 +0200 Subject: [PATCH 11/25] Use documentable_attach_new_file method where possible Use this commit to unify two similar specs. --- spec/support/common_actions/attachables.rb | 15 ++++++++------- spec/system/nested_documentable_spec.rb | 18 ++---------------- 2 files changed, 10 insertions(+), 23 deletions(-) diff --git a/spec/support/common_actions/attachables.rb b/spec/support/common_actions/attachables.rb index b0429c9fc..d283ebe0c 100644 --- a/spec/support/common_actions/attachables.rb +++ b/spec/support/common_actions/attachables.rb @@ -15,13 +15,14 @@ module Attachables def documentable_attach_new_file(path, success: true) click_link "Add new document" - attach_file "Choose document", path - - within ".document-fields" do - if success - expect(page).to have_css ".loading-bar.complete" - else - expect(page).to have_css ".loading-bar.errors" + within "#nested-documents" do + attach_file "Choose document", path + within ".document-fields" do + if success + expect(page).to have_css ".loading-bar.complete" + else + expect(page).to have_css ".loading-bar.errors" + end end end end diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 11cf80428..272ae5b18 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -85,27 +85,13 @@ describe "Nested documentable" do expect(page).not_to have_css ".max-documents-notice" end - scenario "Should update nested document file name after choosing a file" do - do_login_for(user, management: management_section?(path)) - visit path - - click_link "Add new document" - within "#nested-documents" do - attach_file "Choose document", file_fixture("empty.pdf") - - expect(page).to have_css ".loading-bar.complete" - end - - expect(page).to have_css ".file-name", text: "empty.pdf" - end - - scenario "Should update nested document file title with - file name after choosing a file when no title defined" do + scenario "Should update file name and title after choosing a file with no title defined" do do_login_for(user, management: management_section?(path)) visit path documentable_attach_new_file(file_fixture("empty.pdf")) + expect(page).to have_css ".file-name", text: "empty.pdf" expect_document_has_title(0, "empty.pdf") end From d7581955f82c766f2e7af4ebdd65a6c2dbfb1a27 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 11:56:01 +0200 Subject: [PATCH 12/25] Remove duplicated code in imageable and documentable attach file methods --- spec/support/common_actions/attachables.rb | 25 +++++++++------------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/spec/support/common_actions/attachables.rb b/spec/support/common_actions/attachables.rb index d283ebe0c..69f963b1c 100644 --- a/spec/support/common_actions/attachables.rb +++ b/spec/support/common_actions/attachables.rb @@ -1,23 +1,18 @@ module Attachables def imageable_attach_new_file(path, success: true) - click_link "Add image" - within "#nested-image" do - attach_file "Choose image", path - within ".image-fields" do - if success - expect(page).to have_css(".loading-bar.complete") - else - expect(page).to have_css(".loading-bar.errors") - end - end - end + attach_new_file("Add image", "nested-image", "image", "Choose image", path, success) end def documentable_attach_new_file(path, success: true) - click_link "Add new document" - within "#nested-documents" do - attach_file "Choose document", path - within ".document-fields" do + attach_new_file("Add new document", "nested-documents", "document", "Choose document", path, success) + end + + def attach_new_file(link_text, wrapper_id, field_class, input_label, path, success) + click_link link_text + + within "##{wrapper_id}" do + attach_file input_label, path + within ".#{field_class}-fields" do if success expect(page).to have_css ".loading-bar.complete" else From e6089c2b6341339f0056c6ae7f91097f184f004c Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 11:34:17 +0200 Subject: [PATCH 13/25] Remove expect_document_has_title method I think tests are now clearer by using have_field. Note that I have remove the within ".document-fields" because there is only one document. --- spec/system/nested_documentable_spec.rb | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 272ae5b18..1db647358 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -92,7 +92,7 @@ describe "Nested documentable" do documentable_attach_new_file(file_fixture("empty.pdf")) expect(page).to have_css ".file-name", text: "empty.pdf" - expect_document_has_title(0, "empty.pdf") + expect(page).to have_field("Title", with: "empty.pdf") end scenario "Should not update nested document file title with @@ -109,7 +109,7 @@ describe "Nested documentable" do expect(page).to have_css ".loading-bar.complete" end - expect_document_has_title(0, "My Title") + expect(page).to have_field("Title", with: "My Title") end scenario "Should update loading bar style after valid file upload" do @@ -321,12 +321,4 @@ describe "Nested documentable" do end end end - - def expect_document_has_title(index, title) - document = all(".document-fields")[index] - - within document do - expect(find("input[name$='[title]']").value).to eq title - end - end end From 88ba548343c07e1e0c6502d46f5d31c8a001c4da Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 12:34:01 +0200 Subject: [PATCH 14/25] Unify fill_in field in one line --- spec/system/nested_documentable_spec.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 1db647358..975ce3d61 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -102,8 +102,7 @@ describe "Nested documentable" do click_link "Add new document" within "#nested-documents" do - input = find("input[name$='[title]']") - fill_in input[:id], with: "My Title" + fill_in "Title", with: "My Title" attach_file "Choose document", file_fixture("empty.pdf") expect(page).to have_css ".loading-bar.complete" From afdf65dec9d49004571d00a9a8baa3ec0e6550f8 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 13:02:10 +0200 Subject: [PATCH 15/25] Remove unnecessary tests and expectations related with loading-bar These expectations are already covered by attach_new_file, so they are no longer needed: > expect(page).to have_css ".loading-bar.complete" We can remove the tests: > "Should update loading bar style after invalid file upload" because the expectation: > expect(page).to have_css ".loading-bar.errors" It is already tested in "Should not update document cached_attachment field after invalid file upload" --- spec/system/nested_documentable_spec.rb | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 975ce3d61..7d236042d 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -111,24 +111,6 @@ describe "Nested documentable" do expect(page).to have_field("Title", with: "My Title") end - scenario "Should update loading bar style after valid file upload" do - do_login_for(user, management: management_section?(path)) - visit path - - documentable_attach_new_file(file_fixture("empty.pdf")) - - 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_section?(path)) - visit path - - documentable_attach_new_file(file_fixture("logo_header.gif"), success: false) - - expect(page).to have_css ".loading-bar.errors" - end - scenario "Should update document cached_attachment field after valid file upload" do do_login_for(user, management: management_section?(path)) visit path From b7025a8135c4b8469f6ba3edc2ac2327dce8a8e4 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 13:07:53 +0200 Subject: [PATCH 16/25] Remove "outdated" comment Since commit 23682fadd8791, we have had the comment: > # Review > # Doble check why the file is stored with a name different to empty.pdf This might be outdated. --- spec/system/nested_documentable_spec.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 7d236042d..772e92dd7 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -194,10 +194,7 @@ describe "Nested documentable" do expect(page).to have_content notice_text expect(page).to have_content "Documents" - expect(page).to have_content "empty.pdf" - - # Review - # Doble check why the file is stored with a name different to empty.pdf + expect(page).to have_link text: "empty.pdf" expect(page).to have_link href: /.pdf\Z/ end From 28e9832c40c8623b2a78beae07f58e58b5915476 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 14:43:11 +0200 Subject: [PATCH 17/25] Unify tests in order to remove duplicated code Note that we are removing a specific "context" that can be replaced with an "if" in the previous spec. --- spec/system/nested_documentable_spec.rb | 62 +++++++------------------ 1 file changed, 18 insertions(+), 44 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 772e92dd7..56185c3d4 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -169,59 +169,33 @@ describe "Nested documentable" do end scenario "Should show successful notice when resource filled correctly and after valid file uploads" do + Setting["uploads.documents.max_amount"] = 3 do_login_for(user, management: management_section?(path)) visit path fill_in_required_fields(factory, path) - documentable_attach_new_file(file_fixture("empty.pdf")) + %w[clippy empty logo].each do |filename| + click_link "Add new document" + + attach_file "Choose document", file_fixture("#{filename}.pdf") + + within all(".document-fields").last do + expect(page).to have_css ".loading-bar.complete" + end + end + + expect(page).not_to have_link "Add new document" + click_button submit_button_text expect(page).to have_content notice_text - end - - context "Budget investments and proposals" do - let(:factory) { (factories - [:dashboard_action]).sample } - - scenario "Should show new document after successful creation with one uploaded file" do - do_login_for(user, management: management_section?(path)) - visit path - - fill_in_required_fields(factory, path) - - documentable_attach_new_file(file_fixture("empty.pdf")) - click_button submit_button_text - - expect(page).to have_content notice_text - expect(page).to have_content "Documents" - expect(page).to have_link text: "empty.pdf" - expect(page).to have_link href: /.pdf\Z/ - end - - scenario "Should show resource with new document after successful creation with - maximum allowed uploaded files" do - Setting["uploads.documents.max_amount"] = 3 - do_login_for(user, management: management_section?(path)) - visit path - - fill_in_required_fields(factory, path) - - %w[clippy empty logo].each do |filename| - click_link "Add new document" - - attach_file "Choose document", file_fixture("#{filename}.pdf") - - within all(".document-fields").last do - expect(page).to have_css ".loading-bar.complete" - end - end - - expect(page).not_to have_link "Add new document" - - click_button submit_button_text - - expect(page).to have_content notice_text + if factory != :dashboard_action expect(page).to have_content "Documents (3)" + expect(page).to have_link href: /.pdf\Z/, count: 3 + expect(page).to have_link text: "clippy.pdf" + expect(page).to have_link text: "empty.pdf" + expect(page).to have_link text: "logo.pdf" end end end From 59c61d02cc77c265758a30c0d43c6b8d36d6a2f1 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 14:52:22 +0200 Subject: [PATCH 18/25] Remove redundant tests from "Only for edit path" describe These tests already tested in previous tests. --- spec/system/nested_documentable_spec.rb | 42 +++++++++++-------------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 56185c3d4..afdc602dc 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -64,9 +64,13 @@ describe "Nested documentable" do expect(page).to have_link "Add new document" - click_link "Add new document" + documentable_attach_new_file(file_fixture("empty.pdf")) expect(page).not_to have_link "Add new document" + + click_link "Remove document" + + expect(page).to have_link "Add new document" end scenario "Shows or hides max documents warning depending on max documents limit" do @@ -152,7 +156,16 @@ describe "Nested documentable" do do_login_for(user, management: management_section?(path)) visit path - documentable_attach_new_file(file_fixture("empty.pdf")) + click_link "Add new document" + + expect(page).not_to have_link "Add new document" + + within "#nested-documents" do + attach_file "Choose document", file_fixture("empty.pdf") + within ".document-fields" do + expect(page).to have_css ".loading-bar.complete" + end + end click_link "Remove document" expect(page).not_to have_css("#nested-documents .document-fields") @@ -204,40 +217,23 @@ describe "Nested documentable" do let!(:proposal) { create(:proposal, author: user) } scenario "Should show persisted documents and remove nested_field" do + Setting["uploads.documents.max_amount"] = 1 create(:document, documentable: proposal) login_as user visit edit_proposal_path(proposal) + expect(page).not_to have_link "Add new document" expect(page).to have_css ".document-fields", count: 1 end - scenario "Should not show add document button when - documentable has reached maximum of documents allowed" do - create_list(:document, proposal.class.max_documents_allowed, documentable: proposal) - login_as user - visit edit_proposal_path(proposal) - - expect(page).not_to have_css "#new_document_link" - end - - scenario "Should show add document button after destroy one document" do - create_list(:document, proposal.class.max_documents_allowed, documentable: proposal) - login_as user - visit edit_proposal_path(proposal) - last_document = all("#nested-documents .document-fields").last - within last_document do - click_link "Remove document" - end - - expect(page).to have_link "Add new document" - end - scenario "Should remove nested field after remove document" do + Setting["uploads.documents.max_amount"] = 1 create(:document, documentable: proposal) login_as user visit edit_proposal_path(proposal) click_link "Remove document" + expect(page).to have_link "Add new document" expect(page).not_to have_css ".document-fields" end From 2420f277040c8864b3e87c88d29794385447a37c Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 9 Jun 2025 15:17:46 +0200 Subject: [PATCH 19/25] Unify tests when removing documents --- spec/system/nested_documentable_spec.rb | 38 +++++++++++-------------- 1 file changed, 16 insertions(+), 22 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index afdc602dc..4851ae7c0 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -99,20 +99,33 @@ describe "Nested documentable" do expect(page).to have_field("Title", with: "empty.pdf") end - scenario "Should not update nested document file title with - file name after choosing a file when title already defined" do + scenario "Should not change existing titles except when removing the document" do do_login_for(user, management: management_section?(path)) visit path click_link "Add new document" + + expect(page).not_to have_link "Add new document" + within "#nested-documents" do fill_in "Title", with: "My Title" attach_file "Choose document", file_fixture("empty.pdf") expect(page).to have_css ".loading-bar.complete" + expect(page).to have_field "Title", with: "My Title" + + click_link "Remove document" + + expect(page).not_to have_css ".document-fields" + expect(page).not_to have_field "Title" end - expect(page).to have_field("Title", with: "My Title") + click_link "Add new document" + + within "#nested-documents" do + expect(page).to have_field "Title", with: "" + expect(page).not_to have_field "Title", with: "My Title" + end end scenario "Should update document cached_attachment field after valid file upload" do @@ -152,25 +165,6 @@ describe "Nested documentable" do end end - scenario "Should delete document after valid file upload and click on remove button" do - do_login_for(user, management: management_section?(path)) - visit path - - click_link "Add new document" - - expect(page).not_to have_link "Add new document" - - within "#nested-documents" do - attach_file "Choose document", file_fixture("empty.pdf") - within ".document-fields" do - expect(page).to have_css ".loading-bar.complete" - end - end - click_link "Remove document" - - expect(page).not_to have_css("#nested-documents .document-fields") - end - scenario "Should show successful notice when resource filled correctly without any nested documents" do do_login_for(user, management: management_section?(path)) visit path From 9e805d95de460fd7f15e905fca5e0f12be08ef96 Mon Sep 17 00:00:00 2001 From: taitus Date: Fri, 16 May 2025 15:17:14 +0200 Subject: [PATCH 20/25] Unify tests from "Only for edit path" describe in order to reduce duplicated code --- spec/system/nested_documentable_spec.rb | 6 ------ 1 file changed, 6 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 4851ae7c0..f21b28440 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -218,13 +218,7 @@ describe "Nested documentable" do expect(page).not_to have_link "Add new document" expect(page).to have_css ".document-fields", count: 1 - end - scenario "Should remove nested field after remove document" do - Setting["uploads.documents.max_amount"] = 1 - create(:document, documentable: proposal) - login_as user - visit edit_proposal_path(proposal) click_link "Remove document" expect(page).to have_link "Add new document" From 11f09c281cb552687fcee2bc14815e74a17f53ed Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 19 May 2025 09:55:05 +0200 Subject: [PATCH 21/25] Move test for check metadata to nested documentable system --- spec/system/documents_spec.rb | 28 ------------------------- spec/system/nested_documentable_spec.rb | 24 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 28 deletions(-) delete mode 100644 spec/system/documents_spec.rb diff --git a/spec/system/documents_spec.rb b/spec/system/documents_spec.rb deleted file mode 100644 index b1abac580..000000000 --- a/spec/system/documents_spec.rb +++ /dev/null @@ -1,28 +0,0 @@ -require "rails_helper" - -describe "Documents" do - describe "Metadata" do - scenario "download document without metadata" do - login_as(create(:user)) - visit new_proposal_path - - fill_in_new_proposal_title with: "debate" - fill_in "Proposal summary", with: "In summary, what we want is..." - fill_in "Full name of the person submitting the proposal", with: "Isabel Garcia" - documentable_attach_new_file(file_fixture("logo_with_metadata.pdf")) - check "I agree to the Privacy Policy and the Terms and conditions of use" - click_button "Create proposal" - - expect(page).to have_content "Proposal created successfully" - - io = URI.parse(find_link(text: "PDF")[:href]).open - reader = PDF::Reader.new(io) - - expect(reader.info[:Keywords]).not_to eq "Test Metadata" - expect(reader.info[:Author]).not_to eq "Test Developer" - expect(reader.info[:Title]).not_to eq "logo_with_metadata.pdf" - expect(reader.info[:Producer]).not_to eq "Test Producer" - expect(reader.info).to eq({}) - end - end -end diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index f21b28440..193ad26c6 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -205,6 +205,30 @@ describe "Nested documentable" do expect(page).to have_link text: "logo.pdf" end end + + describe "Metadata" do + let(:factory) { (factories - [:dashboard_action]).sample } + + scenario "download document without metadata" do + do_login_for(user, management: management_section?(path)) + visit path + + fill_in_required_fields(factory, path) + documentable_attach_new_file(file_fixture("logo_with_metadata.pdf")) + click_button submit_button_text + + expect(page).to have_content notice_text + + io = URI.parse(find_link(text: "PDF")[:href]).open + reader = PDF::Reader.new(io) + + expect(reader.info[:Keywords]).not_to eq "Test Metadata" + expect(reader.info[:Author]).not_to eq "Test Developer" + expect(reader.info[:Title]).not_to eq "logo_with_metadata.pdf" + expect(reader.info[:Producer]).not_to eq "Test Producer" + expect(reader.info).to eq({}) + end + end end describe "Only for edit path" do From 7a317ef9c12c89c095c93d1a705e35f0615fcaf9 Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 19 May 2025 10:38:48 +0200 Subject: [PATCH 22/25] Unify and move documentable shared specs to nested documentable file --- spec/shared/system/documentable.rb | 33 ------------------------- spec/system/nested_documentable_spec.rb | 16 ++++++++---- 2 files changed, 11 insertions(+), 38 deletions(-) diff --git a/spec/shared/system/documentable.rb b/spec/shared/system/documentable.rb index 43277e9e6..fd69a383b 100644 --- a/spec/shared/system/documentable.rb +++ b/spec/shared/system/documentable.rb @@ -11,16 +11,6 @@ shared_examples "documentable" do |documentable_factory_name, documentable_path, end context "Show documents" do - scenario "Download action should be availabe to anyone and open in the same tab" do - visit send(documentable_path, arguments) - - within "#documents" do - expect(page).to have_link text: document.title - expect(page).to have_css "a[rel=nofollow]", text: document.title - expect(page).not_to have_css "a[target=_blank]" - end - end - describe "Destroy action" do scenario "Should not be able when no user logged in" do visit send(documentable_path, arguments) @@ -49,29 +39,6 @@ shared_examples "documentable" do |documentable_factory_name, documentable_path, end end - describe "When allow attached documents setting is enabled" do - before do - Setting["feature.allow_attached_documents"] = true - end - - scenario "Documents list should be available" do - login_as(user) - visit send(documentable_path, arguments) - - expect(page).to have_css("#documents") - expect(page).to have_content("Documents (1)") - end - - scenario "Documents list increase documents number" do - create(:document, documentable: documentable, user: documentable.author) - login_as(user) - visit send(documentable_path, arguments) - - expect(page).to have_css("#documents") - expect(page).to have_content("Documents (2)") - end - end - describe "When allow attached documents setting is disabled" do before do Setting["feature.allow_attached_documents"] = false diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 193ad26c6..7936a0f40 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -198,11 +198,17 @@ describe "Nested documentable" do expect(page).to have_content notice_text if factory != :dashboard_action - expect(page).to have_content "Documents (3)" - expect(page).to have_link href: /.pdf\Z/, count: 3 - expect(page).to have_link text: "clippy.pdf" - expect(page).to have_link text: "empty.pdf" - expect(page).to have_link text: "logo.pdf" + within "#documents" do + expect(page).to have_content "Documents (3)" + expect(page).to have_link href: /.pdf\Z/, count: 3 + expect(page).to have_link text: "empty.pdf" + expect(page).to have_css "a[rel=nofollow]" + expect(page).to have_link text: "clippy.pdf" + expect(page).to have_css "a[rel=nofollow]" + expect(page).to have_link text: "logo.pdf" + expect(page).to have_css "a[rel=nofollow]" + expect(page).not_to have_css "a[target=_blank]" + end end end From 472e2441036b59c17dec5799a57a3405f9667163 Mon Sep 17 00:00:00 2001 From: taitus Date: Mon, 19 May 2025 11:21:10 +0200 Subject: [PATCH 23/25] Move tests related with attached documents from documentable shared specs to nested documentable file Note that we moved some system tests to component tests, since they don't involve user interaction and can be fully covered at the component level. --- .../documents/document_component_spec.rb | 37 ++++++++++ spec/shared/system/documentable.rb | 74 ------------------- spec/system/budgets/investments_spec.rb | 5 -- spec/system/nested_documentable_spec.rb | 31 ++++++++ spec/system/proposals_spec.rb | 2 - 5 files changed, 68 insertions(+), 81 deletions(-) create mode 100644 spec/components/documents/document_component_spec.rb delete mode 100644 spec/shared/system/documentable.rb diff --git a/spec/components/documents/document_component_spec.rb b/spec/components/documents/document_component_spec.rb new file mode 100644 index 000000000..eff800cba --- /dev/null +++ b/spec/components/documents/document_component_spec.rb @@ -0,0 +1,37 @@ +require "rails_helper" + +describe Documents::DocumentComponent do + let(:user) { create(:user) } + let(:proposal) { create(:proposal, author: user) } + let(:document) { create(:document, documentable: proposal) } + let(:component) { Documents::DocumentComponent.new(document, show_destroy_link: true) } + + describe "Delete document button" do + it "is not shown when no user is logged in" do + render_inline component + + expect(page).not_to have_button "Delete document" + end + + it "is shown when the author is logged in" do + sign_in(user) + render_inline component + + expect(page).to have_button "Delete document" + end + + it "is not shown when an administrator that isn't the author is logged in", :admin do + render_inline component + + expect(page).not_to have_button "Delete document" + end + + it "is not shown when a user that isn't the author is logged in" do + login_as(create(:user)) + + render_inline component + + expect(page).not_to have_button "Delete document" + end + end +end diff --git a/spec/shared/system/documentable.rb b/spec/shared/system/documentable.rb deleted file mode 100644 index fd69a383b..000000000 --- a/spec/shared/system/documentable.rb +++ /dev/null @@ -1,74 +0,0 @@ -shared_examples "documentable" do |documentable_factory_name, documentable_path, documentable_path_arguments| - let(:user) { create(:user) } - let(:arguments) { {} } - let(:documentable) { create(documentable_factory_name, author: user) } - let!(:document) { create(:document, documentable: documentable, user: documentable.author) } - - before do - documentable_path_arguments.each do |argument_name, path_to_value| - arguments.merge!("#{argument_name}": documentable.send(path_to_value)) - end - end - - context "Show documents" do - describe "Destroy action" do - scenario "Should not be able when no user logged in" do - visit send(documentable_path, arguments) - - expect(page).not_to have_button "Delete document" - end - - scenario "Should be able when documentable author is logged in" do - login_as documentable.author - visit send(documentable_path, arguments) - - expect(page).to have_button "Delete document" - end - - scenario "Administrators cannot destroy documentables they have not authored", :admin do - visit send(documentable_path, arguments) - - expect(page).not_to have_button "Delete document" - end - - scenario "Users cannot destroy documentables they have not authored" do - login_as(create(:user)) - visit send(documentable_path, arguments) - - expect(page).not_to have_button "Delete document" - end - end - - describe "When allow attached documents setting is disabled" do - before do - Setting["feature.allow_attached_documents"] = false - end - - scenario "Documents list should not be available" do - login_as(create(:user)) - visit send(documentable_path, arguments) - - expect(page).not_to have_css("#documents") - end - end - end - - context "Destroy" do - scenario "Should show success notice after successful document upload" do - login_as documentable.author - - visit send(documentable_path, arguments) - - within "#document_#{document.id}" do - accept_confirm { click_button "Delete document" } - end - - expect(page).to have_content "Document was deleted successfully." - expect(page).not_to have_content "Documents (0)" - - within "##{ActionView::RecordIdentifier.dom_id(documentable)}" do - expect(page).to have_css "h1", text: documentable.title - end - end - end -end diff --git a/spec/system/budgets/investments_spec.rb b/spec/system/budgets/investments_spec.rb index 642ef7826..e4fd1c897 100644 --- a/spec/system/budgets/investments_spec.rb +++ b/spec/system/budgets/investments_spec.rb @@ -1110,11 +1110,6 @@ describe "Budget Investments" do "budget_investment_path", { budget_id: "budget_id", id: "id" } - it_behaves_like "documentable", - "budget_investment", - "budget_investment_path", - { budget_id: "budget_id", id: "id" } - it_behaves_like "mappable", "budget_investment", "investment", diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 7936a0f40..bf6e29d09 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -287,4 +287,35 @@ describe "Nested documentable" do end end end + + context "Show path" do + let(:factory) { (factories - [:dashboard_action]).sample } + let(:path) { polymorphic_path(documentable) } + + scenario "Documents list should not be available when allow attached documents setting is disabled" do + Setting["feature.allow_attached_documents"] = false + create(:document, documentable: documentable) + visit path + + expect(page).not_to have_css("#documents") + end + + context "Destroy" do + scenario "Should show success notice after successful document upload" do + create(:document, documentable: documentable) + documentable.update!(author: user) + login_as(user) + visit path + + accept_confirm { click_button "Delete document" } + + expect(page).to have_content "Document was deleted successfully." + expect(page).not_to have_content "Documents (0)" + + within "##{ActionView::RecordIdentifier.dom_id(documentable)}" do + expect(page).to have_css "h1", text: documentable.title + end + end + end + end end diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index bcb50ab13..f00a37b40 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -1295,8 +1295,6 @@ describe "Proposals" do it_behaves_like "imageable", "proposal", "proposal_path", { id: "id" } - it_behaves_like "documentable", "proposal", "proposal_path", { id: "id" } - it_behaves_like "mappable", "proposal", "proposal", From 44cfb9bcc25a988ce63fd9dcb0666c7231dcff36 Mon Sep 17 00:00:00 2001 From: taitus Date: Tue, 20 May 2025 16:20:26 +0200 Subject: [PATCH 24/25] Unify lets from documentable and imageable to attachables methods --- spec/support/common_actions/attachables.rb | 44 ++++++++++++++++++++++ spec/system/nested_documentable_spec.rb | 42 ++------------------- spec/system/nested_imageable_spec.rb | 41 ++------------------ 3 files changed, 50 insertions(+), 77 deletions(-) diff --git a/spec/support/common_actions/attachables.rb b/spec/support/common_actions/attachables.rb index 69f963b1c..505f73f37 100644 --- a/spec/support/common_actions/attachables.rb +++ b/spec/support/common_actions/attachables.rb @@ -44,4 +44,48 @@ module Attachables when :proposal then fill_in_proposal end end + + def attachable_path_for(factory, attachable) + case factory + when :budget then new_admin_budgets_wizard_budget_path + when :budget_investment + [ + new_budget_investment_path(budget_id: attachable.budget_id), + new_management_budget_investment_path(budget_id: attachable.budget_id) + ].sample + when :dashboard_action then new_admin_dashboard_action_path + when :future_poll_question_option then new_admin_option_image_path(option_id: attachable.id) + when :proposal then [new_proposal_path, edit_proposal_path(attachable)].sample + end + end + + def submit_button_text_for(factory, path) + case factory + when :budget then "Continue to groups" + when :budget_investment then "Create Investment" + when :dashboard_action then "Save" + when :future_poll_question_option then "Save image" + when :proposal + if edit_path?(path) + "Save changes" + else + "Create proposal" + end + end + end + + def notice_text_for(factory, path) + case factory + when :budget then "New participatory budget created successfully!" + when :budget_investment then "Budget Investment created successfully." + when :dashboard_action then "Action created successfully" + when :future_poll_question_option then "Image uploaded successfully" + when :proposal + if edit_path?(path) + "Proposal updated successfully" + else + "Proposal created successfully" + end + end + end end diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index bf6e29d09..6f5885065 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -10,45 +10,9 @@ describe "Nested documentable" do let(:factory) { factories.sample } let!(:documentable) { create(factory) } let!(:user) { create(:user, :level_two) } - let(:path) do - case factory - when :budget_investment - [ - new_budget_investment_path(budget_id: documentable.budget_id), - new_management_budget_investment_path(budget_id: documentable.budget_id) - ].sample - when :dashboard_action then new_admin_dashboard_action_path - when :proposal - [ - new_proposal_path, - edit_proposal_path(id: documentable.id) - ].sample - end - end - let(:submit_button_text) do - case factory - when :budget_investment then "Create Investment" - when :dashboard_action then "Save" - when :proposal - if edit_path?(path) - "Save changes" - else - "Create proposal" - end - end - end - let(:notice_text) do - case factory - when :budget_investment then "Budget Investment created successfully." - when :dashboard_action then "Action created successfully" - when :proposal - if edit_path?(path) - "Proposal updated successfully" - else - "Proposal created successfully" - end - end - end + let(:path) { attachable_path_for(factory, documentable) } + let(:submit_button_text) { submit_button_text_for(factory, path) } + let(:notice_text) { notice_text_for(factory, path) } context "New and edit path" do describe "When allow attached documents setting is enabled" do diff --git a/spec/system/nested_imageable_spec.rb b/spec/system/nested_imageable_spec.rb index 0986ff182..936432999 100644 --- a/spec/system/nested_imageable_spec.rb +++ b/spec/system/nested_imageable_spec.rb @@ -11,44 +11,9 @@ describe "Nested imageable" do let(:factory) { factories.sample } let!(:imageable) { create(factory) } let!(:user) { create(:user, :level_two) } - let(:path) do - case factory - when :budget then new_admin_budgets_wizard_budget_path - 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, edit_proposal_path(imageable)].sample - end - end - let(:submit_button_text) 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 - if edit_path?(path) - "Save changes" - else - "Create proposal" - end - 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 :future_poll_question_option then "Image uploaded successfully" - when :proposal - if edit_path?(path) - "Proposal updated successfully" - else - "Proposal created successfully" - end - end - end + let(:path) { attachable_path_for(factory, imageable) } + let(:submit_button_text) { submit_button_text_for(factory, path) } + let(:notice_text) { notice_text_for(factory, path) } context "New and edit path" do before do From 49facdca7d48104788aa473c459934b718c2e677 Mon Sep 17 00:00:00 2001 From: taitus Date: Thu, 5 Jun 2025 16:07:54 +0200 Subject: [PATCH 25/25] Refactor nested document tests to simplify contexts and remove duplication --- spec/system/nested_documentable_spec.rb | 356 ++++++++++++------------ 1 file changed, 177 insertions(+), 179 deletions(-) diff --git a/spec/system/nested_documentable_spec.rb b/spec/system/nested_documentable_spec.rb index 6f5885065..13eacb4a1 100644 --- a/spec/system/nested_documentable_spec.rb +++ b/spec/system/nested_documentable_spec.rb @@ -15,228 +15,187 @@ describe "Nested documentable" do let(:notice_text) { notice_text_for(factory, path) } context "New and edit path" do - describe "When allow attached documents setting is enabled" do - before do - Setting["uploads.documents.max_amount"] = 1 - create(:administrator, user: user) if admin_section?(path) - documentable.update!(author: user) if edit_path?(path) - end + before do + Setting["uploads.documents.max_amount"] = 1 + create(:administrator, user: user) if admin_section?(path) + documentable.update!(author: user) if edit_path?(path) + end - scenario "Shows or hides new document link depending on max documents limit" do - do_login_for(user, management: management_section?(path)) - visit path + scenario "Shows or hides new document link depending on max documents limit" do + do_login_for(user, management: management_section?(path)) + visit path - expect(page).to have_link "Add new document" + expect(page).to have_link "Add new document" - documentable_attach_new_file(file_fixture("empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) - expect(page).not_to have_link "Add new document" + expect(page).not_to have_link "Add new document" - click_link "Remove document" + click_link "Remove document" - expect(page).to have_link "Add new document" - end + expect(page).to have_link "Add new document" + end - scenario "Shows or hides max documents warning depending on max documents limit" do - do_login_for(user, management: management_section?(path)) - visit path + scenario "Shows or hides max documents warning depending on max documents limit" do + do_login_for(user, management: management_section?(path)) + visit path - expect(page).not_to have_css ".max-documents-notice" - expect(page).not_to have_content "Remove document" + expect(page).not_to have_css ".max-documents-notice" + expect(page).not_to have_content "Remove document" - documentable_attach_new_file(file_fixture("empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) - expect(page).to have_css ".max-documents-notice" + expect(page).to have_css ".max-documents-notice" - click_link "Remove document" + click_link "Remove document" - expect(page).not_to have_css ".max-documents-notice" - end + expect(page).not_to have_css ".max-documents-notice" + end - scenario "Should update file name and title after choosing a file with no title defined" do - do_login_for(user, management: management_section?(path)) - visit path + scenario "Should update file name and title after choosing a file with no title defined" do + do_login_for(user, management: management_section?(path)) + visit path - documentable_attach_new_file(file_fixture("empty.pdf")) + documentable_attach_new_file(file_fixture("empty.pdf")) - expect(page).to have_css ".file-name", text: "empty.pdf" - expect(page).to have_field("Title", with: "empty.pdf") - end + expect(page).to have_css ".file-name", text: "empty.pdf" + expect(page).to have_field("Title", with: "empty.pdf") + end - scenario "Should not change existing titles except when removing the document" do - do_login_for(user, management: management_section?(path)) - visit path + scenario "Should not change existing titles except when removing the document" do + do_login_for(user, management: management_section?(path)) + visit path - click_link "Add new document" + click_link "Add new document" - expect(page).not_to have_link "Add new document" - - within "#nested-documents" do - fill_in "Title", with: "My Title" - attach_file "Choose document", file_fixture("empty.pdf") - - expect(page).to have_css ".loading-bar.complete" - expect(page).to have_field "Title", with: "My Title" - - click_link "Remove document" - - expect(page).not_to have_css ".document-fields" - expect(page).not_to have_field "Title" - end - - click_link "Add new document" - - within "#nested-documents" do - expect(page).to have_field "Title", with: "" - expect(page).not_to have_field "Title", with: "My Title" - end - end - - scenario "Should update document cached_attachment field after valid file upload" do - do_login_for(user, management: management_section?(path)) - visit path - - click_link "Add new document" - - cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) - expect(cached_attachment_field.value).to be_empty + expect(page).not_to have_link "Add new document" + within "#nested-documents" do + fill_in "Title", with: "My Title" attach_file "Choose document", file_fixture("empty.pdf") - expect(page).to have_css(".loading-bar.complete") - expect(cached_attachment_field.value).not_to be_empty + expect(page).to have_css ".loading-bar.complete" + expect(page).to have_field "Title", with: "My Title" + + click_link "Remove document" + + expect(page).not_to have_css ".document-fields" + expect(page).not_to have_field "Title" end - scenario "Should not update document cached_attachment field after invalid file upload" do - do_login_for(user, management: management_section?(path)) - visit path + click_link "Add new document" - documentable_attach_new_file(file_fixture("logo_header.gif"), success: false) - - cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) - expect(cached_attachment_field.value).to be_empty + within "#nested-documents" do + expect(page).to have_field "Title", with: "" + expect(page).not_to have_field "Title", with: "My Title" end + end - scenario "Should show document errors after documentable submit with empty document fields" do - do_login_for(user, management: management_section?(path)) - visit path + scenario "Should update document cached_attachment field after valid file upload" do + do_login_for(user, management: management_section?(path)) + visit path + click_link "Add new document" + + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty + + attach_file "Choose document", file_fixture("empty.pdf") + + expect(page).to have_css(".loading-bar.complete") + expect(cached_attachment_field.value).not_to be_empty + end + + scenario "Should not update document cached_attachment field after invalid file upload" do + do_login_for(user, management: management_section?(path)) + visit path + + documentable_attach_new_file(file_fixture("logo_header.gif"), success: false) + + cached_attachment_field = find("input[name$='[cached_attachment]']", visible: :hidden) + expect(cached_attachment_field.value).to be_empty + end + + scenario "Should show document errors after documentable submit with empty document fields" do + do_login_for(user, management: management_section?(path)) + visit path + + click_link "Add new document" + click_button submit_button_text + + within "#nested-documents .document-fields" do + expect(page).to have_content("can't be blank", count: 2) + end + end + + scenario "Should show successful notice when resource filled correctly without any nested documents" do + do_login_for(user, management: management_section?(path)) + visit path + + fill_in_required_fields(factory, path) + 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 + Setting["uploads.documents.max_amount"] = 3 + do_login_for(user, management: management_section?(path)) + visit path + + fill_in_required_fields(factory, path) + + %w[clippy empty logo].each do |filename| click_link "Add new document" - click_button submit_button_text - within "#nested-documents .document-fields" do - expect(page).to have_content("can't be blank", count: 2) + attach_file "Choose document", file_fixture("#{filename}.pdf") + + within all(".document-fields").last do + expect(page).to have_css ".loading-bar.complete" end end - scenario "Should show successful notice when resource filled correctly without any nested documents" do - do_login_for(user, management: management_section?(path)) - visit path + expect(page).not_to have_link "Add new document" - fill_in_required_fields(factory, path) - click_button submit_button_text + 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 - Setting["uploads.documents.max_amount"] = 3 - do_login_for(user, management: management_section?(path)) - visit path - - fill_in_required_fields(factory, path) - - %w[clippy empty logo].each do |filename| - click_link "Add new document" - - attach_file "Choose document", file_fixture("#{filename}.pdf") - - within all(".document-fields").last do - expect(page).to have_css ".loading-bar.complete" - end - end - - expect(page).not_to have_link "Add new document" - - click_button submit_button_text - - expect(page).to have_content notice_text - if factory != :dashboard_action - within "#documents" do - expect(page).to have_content "Documents (3)" - expect(page).to have_link href: /.pdf\Z/, count: 3 - expect(page).to have_link text: "empty.pdf" - expect(page).to have_css "a[rel=nofollow]" - expect(page).to have_link text: "clippy.pdf" - expect(page).to have_css "a[rel=nofollow]" - expect(page).to have_link text: "logo.pdf" - expect(page).to have_css "a[rel=nofollow]" - expect(page).not_to have_css "a[target=_blank]" - end - end - end - - describe "Metadata" do - let(:factory) { (factories - [:dashboard_action]).sample } - - scenario "download document without metadata" do - do_login_for(user, management: management_section?(path)) - visit path - - fill_in_required_fields(factory, path) - documentable_attach_new_file(file_fixture("logo_with_metadata.pdf")) - click_button submit_button_text - - expect(page).to have_content notice_text - - io = URI.parse(find_link(text: "PDF")[:href]).open - reader = PDF::Reader.new(io) - - expect(reader.info[:Keywords]).not_to eq "Test Metadata" - expect(reader.info[:Author]).not_to eq "Test Developer" - expect(reader.info[:Title]).not_to eq "logo_with_metadata.pdf" - expect(reader.info[:Producer]).not_to eq "Test Producer" - expect(reader.info).to eq({}) + expect(page).to have_content notice_text + if factory != :dashboard_action + within "#documents" do + expect(page).to have_content "Documents (3)" + expect(page).to have_link href: /.pdf\Z/, count: 3 + expect(page).to have_link text: "empty.pdf" + expect(page).to have_css "a[rel=nofollow]" + expect(page).to have_link text: "clippy.pdf" + expect(page).to have_css "a[rel=nofollow]" + expect(page).to have_link text: "logo.pdf" + expect(page).to have_css "a[rel=nofollow]" + expect(page).not_to have_css "a[target=_blank]" end end end - describe "Only for edit path" do - let!(:proposal) { create(:proposal, author: user) } + describe "Metadata" do + let(:factory) { (factories - [:dashboard_action]).sample } - scenario "Should show persisted documents and remove nested_field" do - Setting["uploads.documents.max_amount"] = 1 - create(:document, documentable: proposal) - login_as user - visit edit_proposal_path(proposal) + scenario "download document without metadata" do + do_login_for(user, management: management_section?(path)) + visit path - expect(page).not_to have_link "Add new document" - expect(page).to have_css ".document-fields", count: 1 + fill_in_required_fields(factory, path) + documentable_attach_new_file(file_fixture("logo_with_metadata.pdf")) + click_button submit_button_text - click_link "Remove document" + expect(page).to have_content notice_text - expect(page).to have_link "Add new document" - expect(page).not_to have_css ".document-fields" - end + io = URI.parse(find_link(text: "PDF")[:href]).open + reader = PDF::Reader.new(io) - scenario "Same attachment URL after editing the title" do - login_as user - visit edit_proposal_path(proposal) - - documentable_attach_new_file(file_fixture("empty.pdf")) - within_fieldset("Documents") { fill_in "Title", with: "Original" } - click_button "Save changes" - - expect(page).to have_content "Proposal updated successfully" - - original_url = find_link(text: "Original")[:href] - - visit edit_proposal_path(proposal) - within_fieldset("Documents") { fill_in "Title", with: "Updated" } - click_button "Save changes" - - expect(page).to have_content "Proposal updated successfully" - expect(find_link(text: "Updated")[:href]).to eq original_url + expect(reader.info[:Keywords]).not_to eq "Test Metadata" + expect(reader.info[:Author]).not_to eq "Test Developer" + expect(reader.info[:Title]).not_to eq "logo_with_metadata.pdf" + expect(reader.info[:Producer]).not_to eq "Test Producer" + expect(reader.info).to eq({}) end end @@ -252,6 +211,45 @@ describe "Nested documentable" do end end + describe "Only for edit path" do + let!(:proposal) { create(:proposal, author: user) } + + scenario "Should show persisted documents and remove nested_field" do + Setting["uploads.documents.max_amount"] = 1 + create(:document, documentable: proposal) + login_as user + visit edit_proposal_path(proposal) + + expect(page).not_to have_link "Add new document" + expect(page).to have_css ".document-fields", count: 1 + + click_link "Remove document" + + expect(page).to have_link "Add new document" + expect(page).not_to have_css ".document-fields" + end + + scenario "Same attachment URL after editing the title" do + login_as user + visit edit_proposal_path(proposal) + + documentable_attach_new_file(file_fixture("empty.pdf")) + within_fieldset("Documents") { fill_in "Title", with: "Original" } + click_button "Save changes" + + expect(page).to have_content "Proposal updated successfully" + + original_url = find_link(text: "Original")[:href] + + visit edit_proposal_path(proposal) + within_fieldset("Documents") { fill_in "Title", with: "Updated" } + click_button "Save changes" + + expect(page).to have_content "Proposal updated successfully" + expect(find_link(text: "Updated")[:href]).to eq original_url + end + end + context "Show path" do let(:factory) { (factories - [:dashboard_action]).sample } let(:path) { polymorphic_path(documentable) }