From 1c85c8af7950775bcb757848fa2f797906ff6a77 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 2 Oct 2017 13:08:55 +0200 Subject: [PATCH 1/9] Sort Shift recount and scrutiny dates --- app/helpers/shifts_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/shifts_helper.rb b/app/helpers/shifts_helper.rb index af291779a..dc9f97a91 100644 --- a/app/helpers/shifts_helper.rb +++ b/app/helpers/shifts_helper.rb @@ -5,7 +5,7 @@ module ShiftsHelper end def shift_recount_scrutiny_dates(polls) - date_options(polls.map(&:ends_at).map(&:to_date).inject([]) { |total, date| total << (date..date + 1.week).to_a }.flatten.uniq) + date_options(polls.map(&:ends_at).map(&:to_date).sort.inject([]) { |total, date| total << (date..date + 1.week).to_a }.flatten.uniq) end def date_options(dates) From 35f6a5bb65817d1d99d14ee7d66dd8a2a0817766 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Mon, 2 Oct 2017 13:45:19 +0200 Subject: [PATCH 2/9] Add task to unique triple index on Shift for booth, officer and task --- app/models/poll/shift.rb | 2 +- db/migrate/20171002103314_add_poll_shift_task_index.rb | 7 +++++++ db/schema.rb | 5 +++-- 3 files changed, 11 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20171002103314_add_poll_shift_task_index.rb diff --git a/app/models/poll/shift.rb b/app/models/poll/shift.rb index d64bce9ac..cc5f7045e 100644 --- a/app/models/poll/shift.rb +++ b/app/models/poll/shift.rb @@ -6,8 +6,8 @@ class Poll validates :booth_id, presence: true validates :officer_id, presence: true validates :date, presence: true - validates :date, uniqueness: { scope: [:officer_id, :booth_id] } validates :task, presence: true + validates :date, uniqueness: { scope: [:officer_id, :booth_id, :task] } enum task: { vote_collection: 0, recount_scrutiny: 1 } diff --git a/db/migrate/20171002103314_add_poll_shift_task_index.rb b/db/migrate/20171002103314_add_poll_shift_task_index.rb new file mode 100644 index 000000000..d15275556 --- /dev/null +++ b/db/migrate/20171002103314_add_poll_shift_task_index.rb @@ -0,0 +1,7 @@ +class AddPollShiftTaskIndex < ActiveRecord::Migration + def change + remove_index "poll_shifts", name: "index_poll_shifts_on_booth_id_and_officer_id" + add_index :poll_shifts, :task + add_index :poll_shifts, [:booth_id, :officer_id, :task], unique: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 500f03540..f6ff7447d 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170927110953) do +ActiveRecord::Schema.define(version: 20171002103314) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -701,9 +701,10 @@ ActiveRecord::Schema.define(version: 20170927110953) do t.integer "task", default: 0, null: false end - add_index "poll_shifts", ["booth_id", "officer_id"], name: "index_poll_shifts_on_booth_id_and_officer_id", using: :btree + add_index "poll_shifts", ["booth_id", "officer_id", "task"], name: "index_poll_shifts_on_booth_id_and_officer_id_and_task", unique: true, using: :btree add_index "poll_shifts", ["booth_id"], name: "index_poll_shifts_on_booth_id", using: :btree add_index "poll_shifts", ["officer_id"], name: "index_poll_shifts_on_officer_id", using: :btree + add_index "poll_shifts", ["task"], name: "index_poll_shifts_on_task", using: :btree create_table "poll_total_results", force: :cascade do |t| t.integer "author_id" From 4e85e136d1da2095646a903b8d30ea7ac8f7d9b1 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 3 Oct 2017 12:02:05 +0200 Subject: [PATCH 3/9] Remove OfficerAssignment composed index --- app/models/poll/officer_assignment.rb | 2 +- ...20171003095936_remove_officer_assigment_composed_index.rb | 5 +++++ db/schema.rb | 3 +-- 3 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20171003095936_remove_officer_assigment_composed_index.rb diff --git a/app/models/poll/officer_assignment.rb b/app/models/poll/officer_assignment.rb index 9fcf02890..417e6a150 100644 --- a/app/models/poll/officer_assignment.rb +++ b/app/models/poll/officer_assignment.rb @@ -8,7 +8,7 @@ class Poll validates :officer_id, presence: true validates :booth_assignment_id, presence: true - validates :date, presence: true, uniqueness: { scope: [:officer_id, :booth_assignment_id] } + validates :date, presence: true delegate :poll_id, :booth_id, to: :booth_assignment diff --git a/db/migrate/20171003095936_remove_officer_assigment_composed_index.rb b/db/migrate/20171003095936_remove_officer_assigment_composed_index.rb new file mode 100644 index 000000000..874672f84 --- /dev/null +++ b/db/migrate/20171003095936_remove_officer_assigment_composed_index.rb @@ -0,0 +1,5 @@ +class RemoveOfficerAssigmentComposedIndex < ActiveRecord::Migration + def change + remove_index "poll_officer_assignments", name: "index_poll_officer_assignments_on_officer_id_and_date" + end +end diff --git a/db/schema.rb b/db/schema.rb index 5c5b99d11..e75c2aac3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171002191347) do +ActiveRecord::Schema.define(version: 20171003095936) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -639,7 +639,6 @@ ActiveRecord::Schema.define(version: 20171002191347) do end add_index "poll_officer_assignments", ["booth_assignment_id"], name: "index_poll_officer_assignments_on_booth_assignment_id", using: :btree - add_index "poll_officer_assignments", ["officer_id", "date"], name: "index_poll_officer_assignments_on_officer_id_and_date", using: :btree add_index "poll_officer_assignments", ["officer_id"], name: "index_poll_officer_assignments_on_officer_id", using: :btree create_table "poll_officers", force: :cascade do |t| From 34fbf1472c0d0f39d794c56e3570d158b29381ff Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 3 Oct 2017 12:26:04 +0200 Subject: [PATCH 4/9] Join in a single validation both Shift date validates statements --- app/models/poll/shift.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/poll/shift.rb b/app/models/poll/shift.rb index cc5f7045e..7fc5e09d1 100644 --- a/app/models/poll/shift.rb +++ b/app/models/poll/shift.rb @@ -5,9 +5,8 @@ class Poll validates :booth_id, presence: true validates :officer_id, presence: true - validates :date, presence: true + validates :date, presence: true, uniqueness: { scope: [:officer_id, :booth_id, :task] } validates :task, presence: true - validates :date, uniqueness: { scope: [:officer_id, :booth_id, :task] } enum task: { vote_collection: 0, recount_scrutiny: 1 } From 332b9be94a3e1562779ea0d539fa1ba401858ccd Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 3 Oct 2017 12:26:43 +0200 Subject: [PATCH 5/9] Reorder before/after create methods --- app/models/poll/shift.rb | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/app/models/poll/shift.rb b/app/models/poll/shift.rb index 7fc5e09d1..2870709d7 100644 --- a/app/models/poll/shift.rb +++ b/app/models/poll/shift.rb @@ -13,19 +13,21 @@ class Poll before_create :persist_data after_create :create_officer_assignments - def create_officer_assignments - booth.booth_assignments.each do |booth_assignment| - attrs = { officer_id: officer_id, - date: date, - booth_assignment_id: booth_assignment.id } - Poll::OfficerAssignment.create!(attrs) - end - end - def persist_data self.officer_name = officer.name self.officer_email = officer.email end + def create_officer_assignments + booth.booth_assignments.each do |booth_assignment| + attrs = { + officer_id: officer_id, + date: date, + booth_assignment_id: booth_assignment.id, + final: recount_scrutiny? + } + Poll::OfficerAssignment.create!(attrs) + end + end end end From fc5ff61365c0ae78d98c5e2dd6a612a1fbea032a Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 3 Oct 2017 12:30:42 +0200 Subject: [PATCH 6/9] Destroy all related OfficerAssignments when destroying a Shift --- app/models/poll/shift.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/models/poll/shift.rb b/app/models/poll/shift.rb index 2870709d7..4edeb26ef 100644 --- a/app/models/poll/shift.rb +++ b/app/models/poll/shift.rb @@ -12,6 +12,7 @@ class Poll before_create :persist_data after_create :create_officer_assignments + before_destroy :destroy_officer_assignments def persist_data self.officer_name = officer.name @@ -29,5 +30,12 @@ class Poll Poll::OfficerAssignment.create!(attrs) end end + + def destroy_officer_assignments + Poll::OfficerAssignment.where(booth_assignment: booth.booth_assignments, + officer: officer, + date: date, + final: recount_scrutiny?).destroy_all + end end end From c624129175cb8e2b9d441d568e682db76663b38e Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 3 Oct 2017 12:57:40 +0200 Subject: [PATCH 7/9] Check Shift creates final/not-final OfficerAssigments correctly --- spec/models/poll/shift_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/models/poll/shift_spec.rb b/spec/models/poll/shift_spec.rb index b7918b95c..86755a310 100644 --- a/spec/models/poll/shift_spec.rb +++ b/spec/models/poll/shift_spec.rb @@ -50,10 +50,32 @@ describe :shift do expect(oa1.officer).to eq(officer) expect(oa1.date).to eq(Date.current) expect(oa1.booth_assignment).to eq(booth_assignment1) + expect(oa1.final).to be_falsey expect(oa2.officer).to eq(officer) expect(oa2.date).to eq(Date.current) expect(oa2.booth_assignment).to eq(booth_assignment2) + expect(oa2.final).to be_falsey + end + + it "should create final officer_assignments" do + poll = create(:poll) + booth = create(:poll_booth) + officer = create(:poll_officer) + + booth_assignment = create(:poll_booth_assignment, poll: poll, booth: booth) + + shift = create(:poll_shift, booth: booth, officer: officer, date: Date.current, task: :recount_scrutiny) + + officer_assignments = Poll::OfficerAssignment.all + expect(officer_assignments.count).to eq(1) + + officer_assignment = officer_assignments.first + + expect(officer_assignment.officer).to eq(officer) + expect(officer_assignment.date).to eq(Date.current) + expect(officer_assignment.booth_assignment).to eq(booth_assignment) + expect(officer_assignment.final).to be_truthy end end From 690ebfba01c3e10cb45c8ec695f8392090c8f7a7 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 3 Oct 2017 13:24:07 +0200 Subject: [PATCH 8/9] Check Shift destroy also destroy only associated OfficerAssigments --- spec/models/poll/shift_spec.rb | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/spec/models/poll/shift_spec.rb b/spec/models/poll/shift_spec.rb index 86755a310..b62f220da 100644 --- a/spec/models/poll/shift_spec.rb +++ b/spec/models/poll/shift_spec.rb @@ -28,7 +28,7 @@ describe :shift do describe "officer_assignments" do - it "should create corresponding officer_assignments" do + it "should create and destroy corresponding officer_assignments" do poll1 = create(:poll) poll2 = create(:poll) poll3 = create(:poll) @@ -39,11 +39,9 @@ describe :shift do booth_assignment1 = create(:poll_booth_assignment, poll: poll1, booth: booth) booth_assignment2 = create(:poll_booth_assignment, poll: poll2, booth: booth) - shift = create(:poll_shift, booth: booth, officer: officer, date: Date.current) + expect { create(:poll_shift, booth: booth, officer: officer, date: Date.current) }.to change {Poll::OfficerAssignment.all.count}.by(2) officer_assignments = Poll::OfficerAssignment.all - expect(officer_assignments.count).to eq(2) - oa1 = officer_assignments.first oa2 = officer_assignments.second @@ -56,6 +54,10 @@ describe :shift do expect(oa2.date).to eq(Date.current) expect(oa2.booth_assignment).to eq(booth_assignment2) expect(oa2.final).to be_falsey + + create(:poll_officer_assignment, officer: officer, booth_assignment: booth_assignment1, date: Date.tomorrow) + + expect { Poll::Shift.last.destroy }.to change {Poll::OfficerAssignment.all.count}.by(-2) end it "should create final officer_assignments" do From 0384baf26aac314fac2afa9e8aa18ef24dc1cd4a Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 3 Oct 2017 13:29:26 +0200 Subject: [PATCH 9/9] Unify both Shift task type creation on a single scenario to check double task on same day --- spec/features/admin/poll/shifts_spec.rb | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/spec/features/admin/poll/shifts_spec.rb b/spec/features/admin/poll/shifts_spec.rb index 46c737c64..2e957ef11 100644 --- a/spec/features/admin/poll/shifts_spec.rb +++ b/spec/features/admin/poll/shifts_spec.rb @@ -30,12 +30,12 @@ feature 'Admin shifts' do expect(page).to have_content officer.name end - scenario "Create Vote Collection Shift", :js do + scenario "Create Vote Collection Shift and Recount & Scrutiny Shift on same date", :js do poll = create(:poll) - vote_collection_dates = (poll.starts_at.to_date..poll.ends_at.to_date).to_a.map { |date| I18n.l(date, format: :long) } - booth = create(:poll_booth) officer = create(:poll_officer) + vote_collection_dates = (poll.starts_at.to_date..poll.ends_at.to_date).to_a.map { |date| I18n.l(date, format: :long) } + recount_scrutiny_dates = (poll.ends_at.to_date..poll.ends_at.to_date + 1.week).to_a.map { |date| I18n.l(date, format: :long) } visit admin_booths_path @@ -60,14 +60,6 @@ feature 'Admin shifts' do expect(page).to have_content("Collect Votes") expect(page).to have_content(officer.name) end - end - - scenario "Create Recount & Scrutiny Shift", :js do - poll = create(:poll) - recount_scrutiny_dates = (poll.ends_at.to_date..poll.ends_at.to_date + 1.week).to_a.map { |date| I18n.l(date, format: :long) } - - booth = create(:poll_booth) - officer = create(:poll_officer) visit admin_booths_path @@ -89,7 +81,7 @@ feature 'Admin shifts' do expect(page).to have_content "Shift added" within("#shifts") do - expect(page).to have_css(".shift", count: 1) + expect(page).to have_css(".shift", count: 2) expect(page).to have_content(I18n.l(poll.ends_at.to_date + 4.days, format: :long)) expect(page).to have_content("Recount & Scrutiny") expect(page).to have_content(officer.name)