From 073cf748182df8128e1d992283570a8fab01cf59 Mon Sep 17 00:00:00 2001 From: rgarcia Date: Thu, 22 Mar 2018 23:08:41 +0100 Subject: [PATCH] Fix edge case The user was able to vote as many investments as wanted in the first heading voted. However in the second heading voted, only one investment could be voted This was due to the previous implementation, where you could only vote in one heading. Note the `first` call in method `heading_voted_by_user?(user)` This commits simplifies the logic and allows voting for any investment in any heading that the user has previously voted in --- app/models/budget/investment.rb | 11 +++------ spec/models/budget/investment_spec.rb | 33 +++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 8 deletions(-) diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 6e4a9f477..53d27a741 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -231,7 +231,7 @@ class Budget end def valid_heading?(user) - voted_in?([heading.id], user) || + voted_in?(heading, user) || can_vote_in_another_heading?(user) end @@ -243,13 +243,8 @@ class Budget user.votes.for_budget_investments(budget.investments.where(group: group)).votables.map(&:heading_id).uniq end - def voted_in?(heading_ids, user) - heading_ids.include? heading_voted_by_user?(user) - end - - def heading_voted_by_user?(user) - user.votes.for_budget_investments(budget.investments.where(group: group)) - .votables.map(&:heading_id).first + def voted_in?(heading, user) + headings_voted_by_user(user).include?(heading.id) end def ballotable_by?(user) diff --git a/spec/models/budget/investment_spec.rb b/spec/models/budget/investment_spec.rb index d277647de..cdee81087 100644 --- a/spec/models/budget/investment_spec.rb +++ b/spec/models/budget/investment_spec.rb @@ -603,6 +603,22 @@ describe Budget::Investment do expect(salamanca_investment.valid_heading?(user)).to eq(true) end + it "accepts votes in any heading previously voted in" do + group.update(max_votable_headings: 2) + + carabanchel = create(:budget_heading, group: group) + salamanca = create(:budget_heading, group: group) + + carabanchel_investment = create(:budget_investment, heading: carabanchel) + salamanca_investment = create(:budget_investment, heading: salamanca) + + create(:vote, votable: carabanchel_investment, voter: user) + create(:vote, votable: salamanca_investment, voter: user) + + expect(carabanchel_investment.valid_heading?(user)).to eq(true) + expect(salamanca_investment.valid_heading?(user)).to eq(true) + end + it "allows votes in a group with a single heading" do all_city_investment = create(:budget_investment, heading: heading) expect(all_city_investment.valid_heading?(user)).to eq(true) @@ -702,6 +718,23 @@ describe Budget::Investment do end end + describe "#voted_in?" do + + let(:user) { create(:user) } + let(:investment) { create(:budget_investment) } + + it "returns true if the user has voted in this heading" do + create(:vote, votable: investment, voter: user) + + expect(investment.voted_in?(investment.heading, user)).to eq(true) + end + + it "returns false if the user has not voted in this heading" do + expect(investment.voted_in?(investment.heading, user)).to eq(false) + end + + end + describe "Order" do describe "#sort_by_confidence_score" do