From a28967817e955d15b419692e65b7cdf01104af45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 12 Mar 2025 17:13:41 +0100 Subject: [PATCH] Split some system tests checking the database As mentioned in commits like a586ba806, a7664ad81, 006128da5, b41fbfa52 and c480cdd91, accessing the database after starting the browser with the `visit` method sometimes results in database corruption and failing tests on our CI due to the process running the test accessing the database after the process running the browser has started. IMHO this is also a bad practice for system tests, since these tests should be checking what users experience. In these cases, however, I haven't been able to test the user experience. For example, it looks like failed census calls for unregistered users aren't displayed anywhere and can only be accessed by manually checking the database. Similarly, there's no interface showing that all the options from a poll have been deleted (which makes sense, since we only display options in the context of their poll) or a place showing the responsible name for a proposal. So we're splitting the tests in two, with the controller test running the database checks. --- .../admin_notifications_controller_spec.rb | 16 ++++++++++++ .../admin/poll/polls_controller_spec.rb | 13 ++++++++++ .../officing/residence_controller_spec.rb | 22 ++++++++++++++++ .../officing/voters_controller_spec.rb | 1 + spec/controllers/proposals_controller_spec.rb | 25 +++++++++++++++++++ spec/system/admin/admin_notifications_spec.rb | 5 +--- spec/system/admin/poll/polls_spec.rb | 7 ++---- spec/system/officing/residence_spec.rb | 8 ------ spec/system/officing/voters_spec.rb | 2 -- spec/system/proposals_spec.rb | 5 ++-- 10 files changed, 83 insertions(+), 21 deletions(-) create mode 100644 spec/controllers/admin/admin_notifications_controller_spec.rb create mode 100644 spec/controllers/officing/residence_controller_spec.rb diff --git a/spec/controllers/admin/admin_notifications_controller_spec.rb b/spec/controllers/admin/admin_notifications_controller_spec.rb new file mode 100644 index 000000000..37582cd5a --- /dev/null +++ b/spec/controllers/admin/admin_notifications_controller_spec.rb @@ -0,0 +1,16 @@ +require "rails_helper" + +describe Admin::AdminNotificationsController, :admin do + describe "POST deliver" do + it "sends notifications to every recipient" do + 2.times { create(:user) } + notification = create(:admin_notification, segment_recipient: :all_users) + + post :deliver, params: { id: notification } + + User.find_each do |user| + expect(user.notifications.count).to eq 1 + end + end + end +end diff --git a/spec/controllers/admin/poll/polls_controller_spec.rb b/spec/controllers/admin/poll/polls_controller_spec.rb index f20ba75c8..c2535a908 100644 --- a/spec/controllers/admin/poll/polls_controller_spec.rb +++ b/spec/controllers/admin/poll/polls_controller_spec.rb @@ -8,4 +8,17 @@ describe Admin::Poll::PollsController, :admin do expect { get :index }.to raise_exception(FeatureFlags::FeatureDisabled) end end + + describe "DELETE destroy" do + it "deletes every question and every option from that poll" do + poll = create(:poll, name: "Do you support CONSUL?") + create(:poll_question, :yes_no, poll: poll) + + delete :destroy, params: { id: poll } + + expect(Poll.count).to eq 0 + expect(Poll::Question.count).to eq 0 + expect(Poll::Question::Option.count).to eq 0 + end + end end diff --git a/spec/controllers/officing/residence_controller_spec.rb b/spec/controllers/officing/residence_controller_spec.rb new file mode 100644 index 000000000..337cb46ad --- /dev/null +++ b/spec/controllers/officing/residence_controller_spec.rb @@ -0,0 +1,22 @@ +require "rails_helper" + +describe Officing::ResidenceController do + describe "POST create" do + it "creates a failed census called when the census data is invalid" do + officer = create(:poll_officer) + create(:poll_officer_assignment, officer: officer) + + sign_in(officer.user) + + expect do + post :create, params: { + residence: { document_type: "1", document_number: "23456789A", year_of_birth: "1980" } + } + end.to change { FailedCensusCall.count }.by(1) + + failed_census_call = FailedCensusCall.last + expect(failed_census_call.poll_officer).to eq officer + expect(officer.failed_census_calls).to eq [failed_census_call] + end + end +end diff --git a/spec/controllers/officing/voters_controller_spec.rb b/spec/controllers/officing/voters_controller_spec.rb index 33906db84..146e230c7 100644 --- a/spec/controllers/officing/voters_controller_spec.rb +++ b/spec/controllers/officing/voters_controller_spec.rb @@ -20,6 +20,7 @@ describe Officing::VotersController do end.each(&:join) expect(Poll::Voter.count).to eq 1 + expect(Poll::Voter.last.officer_id).to eq(officer.id) end end end diff --git a/spec/controllers/proposals_controller_spec.rb b/spec/controllers/proposals_controller_spec.rb index 1771de8ce..206b19dc4 100644 --- a/spec/controllers/proposals_controller_spec.rb +++ b/spec/controllers/proposals_controller_spec.rb @@ -31,4 +31,29 @@ describe ProposalsController do expect(other_proposal.reload.map_location).not_to be nil end end + + describe "POST create" do + before { InvisibleCaptcha.timestamp_enabled = false } + after { InvisibleCaptcha.timestamp_enabled = true } + + it "assigns the responsible name to the proposal" do + sign_in(create(:user, document_number: "13572468A")) + + post :create, params: { + proposal: { + translations_attributes: { + "0" => { + locale: "en", + title: "I'm responsible", + summary: "I have a document number", + description: "But you won't see my document number" + } + }, + terms_of_service: "1" + } + } + + expect(Proposal.last.responsible_name).to eq "13572468A" + end + end end diff --git a/spec/system/admin/admin_notifications_spec.rb b/spec/system/admin/admin_notifications_spec.rb index 0f15ce649..7bf6f5c68 100644 --- a/spec/system/admin/admin_notifications_spec.rb +++ b/spec/system/admin/admin_notifications_spec.rb @@ -193,10 +193,7 @@ describe "Admin Notifications", :admin do accept_confirm { click_button "Send notification" } expect(page).to have_content "Notification sent successfully" - - User.find_each do |user| - expect(user.notifications.count).to eq(1) - end + expect(page).to have_content "3 users got notified" end scenario "A sent Admin notification can not be sent" do diff --git a/spec/system/admin/poll/polls_spec.rb b/spec/system/admin/poll/polls_spec.rb index 7a79342c2..5559679f8 100644 --- a/spec/system/admin/poll/polls_spec.rb +++ b/spec/system/admin/poll/polls_spec.rb @@ -129,11 +129,8 @@ describe "Admin polls", :admin do accept_confirm { click_button "Delete" } end - expect(page).to have_content("Poll deleted successfully") - expect(page).not_to have_content("Do you support CONSUL?") - - expect(Poll::Question.count).to eq(0) - expect(Poll::Question::Option.count).to eq(0) + expect(page).to have_content "Poll deleted successfully" + expect(page).not_to have_content "Do you support CONSUL?" end scenario "Can destroy polls with options including videos" do diff --git a/spec/system/officing/residence_spec.rb b/spec/system/officing/residence_spec.rb index f1ee43f8c..5fe96ddca 100644 --- a/spec/system/officing/residence_spec.rb +++ b/spec/system/officing/residence_spec.rb @@ -53,7 +53,6 @@ describe "Residence", :with_frozen_time do end scenario "Error on Census (document number)" do - initial_failed_census_calls_count = officer.failed_census_calls_count within("#side_menu") do click_link "Validate document" end @@ -65,13 +64,6 @@ describe "Residence", :with_frozen_time do click_button "Validate document" expect(page).to have_content "The Census was unable to verify this document" - - officer.reload - fcc = FailedCensusCall.last - expect(fcc).to be - expect(fcc.poll_officer).to eq(officer) - expect(officer.failed_census_calls.last).to eq(fcc) - expect(officer.failed_census_calls_count).to eq(initial_failed_census_calls_count + 1) end scenario "Error on Census (year of birth)" do diff --git a/spec/system/officing/voters_spec.rb b/spec/system/officing/voters_spec.rb index 84d0844d1..23c780a6f 100644 --- a/spec/system/officing/voters_spec.rb +++ b/spec/system/officing/voters_spec.rb @@ -30,8 +30,6 @@ describe "Voters" do page.evaluate_script("window.location.reload()") expect(page).to have_content "Has already participated in this poll" expect(page).not_to have_button "Confirm vote" - - expect(Poll::Voter.last.officer_id).to eq(officer.id) end scenario "Cannot vote" do diff --git a/spec/system/proposals_spec.rb b/spec/system/proposals_spec.rb index fa5cd290d..dc9912e05 100644 --- a/spec/system/proposals_spec.rb +++ b/spec/system/proposals_spec.rb @@ -435,7 +435,7 @@ describe "Proposals" do expect(page).to have_field "Full name of the person submitting the proposal", with: "Isabel Garcia" end - scenario "Responsible name field is not shown for verified users" do + scenario "Responsible name field is not shown anywhere" do author = create(:user, :level_two) login_as(author) @@ -453,7 +453,8 @@ describe "Proposals" do click_link "No, I want to publish the proposal" click_link "Not now, go to my proposal" - expect(Proposal.last.responsible_name).to eq(author.document_number) + expect(page).to have_css "h1", exact_text: "Help refugees" + expect(page).not_to have_content author.document_number end scenario "Errors on create" do