From b05ea2964cd575b174656a9cebc6c8aaa8a2bc10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Jan 2019 20:23:46 +0100 Subject: [PATCH 1/6] Simplify poll voter factory associations --- spec/factories/polls.rb | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/spec/factories/polls.rb b/spec/factories/polls.rb index 26c047017..e8139e25c 100644 --- a/spec/factories/polls.rb +++ b/spec/factories/polls.rb @@ -91,12 +91,11 @@ FactoryBot.define do origin "web" trait :from_booth do - association :booth_assignment, factory: :poll_booth_assignment origin "booth" - before :create do |voter| - voter.officer_assignment = create(:poll_officer_assignment, - officer: voter.officer, - booth_assignment: voter.booth_assignment) + association :booth_assignment, factory: :poll_booth_assignment + + officer_assignment do + association :poll_officer_assignment, booth_assignment: booth_assignment, officer: officer end end From 4830b563eae0137b35297d6e2b76e6bf3c861f75 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Jan 2019 20:41:14 +0100 Subject: [PATCH 2/6] Create only one poll for a poll voter The factories were creating strange database relations: * The voter belonged to a poll, to a booth and to an officer * The booth belonged to a different poll * The officer belonged to a different booth The code uses an unusual syntax because the following code: association :booth_assignment, factory: :poll_booth_assignment, poll: poll Would generate the following error: ActiveRecord::AssociationTypeMismatch: Poll(#46976420451940) expected, got FactoryBot::Declaration::Implicit --- spec/factories/polls.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/spec/factories/polls.rb b/spec/factories/polls.rb index e8139e25c..51da0246b 100644 --- a/spec/factories/polls.rb +++ b/spec/factories/polls.rb @@ -92,7 +92,10 @@ FactoryBot.define do trait :from_booth do origin "booth" - association :booth_assignment, factory: :poll_booth_assignment + + booth_assignment do + association :poll_booth_assignment, poll: poll + end officer_assignment do association :poll_officer_assignment, booth_assignment: booth_assignment, officer: officer From 4a3115607a87f4265dc7caad7b93424ef9eed0c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 11 Jan 2019 20:52:39 +0100 Subject: [PATCH 3/6] Simplify poll stats test Adding the option to assign a poll to a poll recount factory meant we didn't need to create so much data. Also note we're removing the `create(:poll_voter, origin: "booth")` code, since it isn't used in the stats calculations. --- spec/factories/polls.rb | 17 ++++++++++++++++- spec/models/poll/stats_spec.rb | 17 ++++++++--------- 2 files changed, 24 insertions(+), 10 deletions(-) diff --git a/spec/factories/polls.rb b/spec/factories/polls.rb index 51da0246b..dfe2a7ed3 100644 --- a/spec/factories/polls.rb +++ b/spec/factories/polls.rb @@ -88,7 +88,12 @@ FactoryBot.define do poll association :user, :level_two association :officer, factory: :poll_officer - origin "web" + from_web + + trait :from_web do + origin "web" + token SecureRandom.hex(32) + end trait :from_booth do origin "booth" @@ -129,6 +134,16 @@ FactoryBot.define do factory :poll_recount, class: "Poll::Recount" do association :author, factory: :user origin "web" + + trait :from_booth do + origin "booth" + + transient { poll nil } + + booth_assignment do + association :poll_booth_assignment, poll: poll + end + end end factory :officing_residence, class: "Officing::Residence" do diff --git a/spec/models/poll/stats_spec.rb b/spec/models/poll/stats_spec.rb index 78af334ca..b2130a370 100644 --- a/spec/models/poll/stats_spec.rb +++ b/spec/models/poll/stats_spec.rb @@ -2,16 +2,15 @@ require "rails_helper" describe Poll::Stats do - describe "Calculate stats" do - it "Generate the correct stats" do + describe "#generate" do + it "generates the correct stats" do poll = create(:poll) - booth = create(:poll_booth) - booth_assignment = create(:poll_booth_assignment, poll: poll, booth: booth) - create(:poll_voter, poll: poll, origin: "web") - 3.times {create(:poll_voter, poll: poll, origin: "booth")} - create(:poll_voter, poll: poll) - create(:poll_recount, origin: "booth", white_amount: 1, null_amount: 0, total_amount: 2, booth_assignment_id: booth_assignment.id) - stats = described_class.new(poll).generate + 2.times { create(:poll_voter, :from_web, poll: poll) } + 3.times { create(:poll_voter, :from_booth, poll: poll) } + create(:poll_recount, :from_booth, poll: poll, + white_amount: 1, null_amount: 0, total_amount: 2) + + stats = Poll::Stats.new(poll).generate expect(stats[:total_participants]).to eq(5) expect(stats[:total_participants_web]).to eq(2) From 8118926ba77a6e8cfe86a2ed38e58e3f945a76e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 15 Mar 2019 20:31:09 +0100 Subject: [PATCH 4/6] Simplify tests creating poll voters --- spec/features/admin/poll/polls_spec.rb | 5 +---- spec/models/poll/poll_spec.rb | 4 ++-- spec/models/poll/voter_spec.rb | 14 +++++++------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/spec/features/admin/poll/polls_spec.rb b/spec/features/admin/poll/polls_spec.rb index 7b758a053..cfc1da4bc 100644 --- a/spec/features/admin/poll/polls_spec.rb +++ b/spec/features/admin/poll/polls_spec.rb @@ -318,10 +318,7 @@ feature "Admin polls" do unvoted_poll = create(:poll) voted_poll = create(:poll) - booth_assignment = create(:poll_booth_assignment, poll: voted_poll) - create(:poll_voter, :from_booth, :valid_document, - booth_assignment: booth_assignment, - poll: voted_poll) + create(:poll_voter, :from_booth, :valid_document, poll: voted_poll) visit admin_poll_results_path(unvoted_poll) diff --git a/spec/models/poll/poll_spec.rb b/spec/models/poll/poll_spec.rb index 4fea6431c..ced234dd7 100644 --- a/spec/models/poll/poll_spec.rb +++ b/spec/models/poll/poll_spec.rb @@ -235,7 +235,7 @@ describe Poll do user = create(:user, :level_two) poll = create(:poll) - create(:poll_voter, poll: poll, user: user, origin: "booth") + create(:poll_voter, :from_booth, poll: poll, user: user) expect(poll.voted_in_booth?(user)).to be end @@ -251,7 +251,7 @@ describe Poll do user = create(:user, :level_two) poll = create(:poll) - create(:poll_voter, poll: poll, user: user, origin: "web") + create(:poll_voter, :from_web, poll: poll, user: user) expect(poll.voted_in_booth?(user)).not_to be end diff --git a/spec/models/poll/voter_spec.rb b/spec/models/poll/voter_spec.rb index 466cbc891..85c72aa7d 100644 --- a/spec/models/poll/voter_spec.rb +++ b/spec/models/poll/voter_spec.rb @@ -76,7 +76,7 @@ describe Poll::Voter do it "is not valid if the user has voted via web" do answer = create(:poll_answer) - answer.record_voter_participation("token") + create(:poll_voter, :from_web, user: answer.author, poll: answer.poll) voter = build(:poll_voter, poll: answer.question.poll, user: answer.author) expect(voter).not_to be_valid @@ -113,9 +113,9 @@ describe Poll::Voter do describe "#web" do it "returns voters with a web origin" do - voter1 = create(:poll_voter, origin: "web") - voter2 = create(:poll_voter, origin: "web") - voter3 = create(:poll_voter, origin: "booth") + voter1 = create(:poll_voter, :from_web) + voter2 = create(:poll_voter, :from_web) + voter3 = create(:poll_voter, :from_booth) web_voters = described_class.web @@ -128,9 +128,9 @@ describe Poll::Voter do describe "#booth" do it "returns voters with a booth origin" do - voter1 = create(:poll_voter, origin: "booth") - voter2 = create(:poll_voter, origin: "booth") - voter3 = create(:poll_voter, origin: "web") + voter1 = create(:poll_voter, :from_booth) + voter2 = create(:poll_voter, :from_booth) + voter3 = create(:poll_voter, :from_web) booth_voters = described_class.booth From d69adbccb70ee6996b1d248d79f8003fb1773bb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 15 Mar 2019 20:41:26 +0100 Subject: [PATCH 5/6] Avoid creating extra poll records in tests The line: create(:poll_voter, booth_assignment: booth_assignment_final_recounted) Creates a new poll for the poll voter. Not only it wastes time by creating new database records, but it doesn't make sense to have a poll voter for a poll which isn't the same as its booth assignment's poll. --- spec/features/admin/poll/polls_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/features/admin/poll/polls_spec.rb b/spec/features/admin/poll/polls_spec.rb index cfc1da4bc..a6c24446e 100644 --- a/spec/features/admin/poll/polls_spec.rb +++ b/spec/features/admin/poll/polls_spec.rb @@ -222,7 +222,9 @@ feature "Admin polls" do total_amount: 21) end - 2.times { create(:poll_voter, booth_assignment: booth_assignment_final_recounted) } + 2.times do + create(:poll_voter, poll: poll, booth_assignment: booth_assignment_final_recounted) + end create(:poll_recount, booth_assignment: booth_assignment_final_recounted, From b3a8924fe05fb446b06045b353aa45618b9d63af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 15 Mar 2019 20:46:14 +0100 Subject: [PATCH 6/6] Don't create an officer for poll voter factories For web poll voters, it isn't necessary in order to make the record valid, and it adds an extra record to the database for each poll voter created. --- spec/factories/polls.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/spec/factories/polls.rb b/spec/factories/polls.rb index dfe2a7ed3..cf62728d2 100644 --- a/spec/factories/polls.rb +++ b/spec/factories/polls.rb @@ -87,7 +87,6 @@ FactoryBot.define do factory :poll_voter, class: "Poll::Voter" do poll association :user, :level_two - association :officer, factory: :poll_officer from_web trait :from_web do @@ -103,7 +102,9 @@ FactoryBot.define do end officer_assignment do - association :poll_officer_assignment, booth_assignment: booth_assignment, officer: officer + association :poll_officer_assignment, + booth_assignment: booth_assignment, + officer: officer || association(:poll_officer) end end