From 413d0ed9beffe7ff8ce62c37edd9e1224e18c8d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 31 Oct 2025 15:31:20 +0100 Subject: [PATCH] Return the persisted line in add_investment This method was returning a boolean value and caused a `Naming/PredicateMethod` when upgrading rubocop. So, instead, we're returning the created line when it was successfully created, and `nil` when it wasn't. Having said that, I'm not sure why we added the `.persisted?` back in commit 3eb22ab7b since as far as I can tell we don't use the return value for anything. The test added in commit da43e9e2e for this change passes if we simply return `lines.create(investment: investment)`. For now I'm leaving the `persisted?` check just in case, but removing it might be fine. --- app/models/budget/ballot.rb | 4 +++- spec/models/poll/ballot_spec.rb | 20 ++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/app/models/budget/ballot.rb b/app/models/budget/ballot.rb index f93838dac..22ba6b64b 100644 --- a/app/models/budget/ballot.rb +++ b/app/models/budget/ballot.rb @@ -10,7 +10,9 @@ class Budget has_many :headings, -> { distinct }, through: :groups def add_investment(investment) - lines.create(investment: investment).persisted? + line = lines.create(investment: investment) + + line if line.persisted? end def total_amount_spent diff --git a/spec/models/poll/ballot_spec.rb b/spec/models/poll/ballot_spec.rb index 981b7ba20..8c6741f9a 100644 --- a/spec/models/poll/ballot_spec.rb +++ b/spec/models/poll/ballot_spec.rb @@ -53,43 +53,43 @@ describe Poll::Ballot do describe "Money" do it "is not valid if insufficient funds" do investment.update!(price: heading.price + 1) - expect(poll_ballot.add_investment(investment.id)).to be false + expect(poll_ballot.add_investment(investment.id)).to be nil end it "is valid if sufficient funds" do investment.update!(price: heading.price - 1) - expect(poll_ballot.add_investment(investment.id)).to be true + expect(poll_ballot.add_investment(investment.id)).to be_truthy end end describe "Heading" do it "is not valid if investment heading is not valid" do - expect(poll_ballot.add_investment(investment.id)).to be true + expect(poll_ballot.add_investment(investment.id)).to be_truthy other_heading = create(:budget_heading, group: group, price: 10000000) other_investment = create(:budget_investment, :selected, price: 1000000, heading: other_heading) - expect(poll_ballot.add_investment(other_investment.id)).to be false + expect(poll_ballot.add_investment(other_investment.id)).to be nil end it "is valid if investment heading is valid" do - expect(poll_ballot.add_investment(investment.id)).to be true + expect(poll_ballot.add_investment(investment.id)).to be_truthy other_investment = create(:budget_investment, :selected, price: 1000000, heading: heading) - expect(poll_ballot.add_investment(other_investment.id)).to be true + expect(poll_ballot.add_investment(other_investment.id)).to be_truthy end end describe "Selectibility" do it "is not valid if investment is unselected" do investment.update!(selected: false) - expect(poll_ballot.add_investment(investment.id)).to be false + expect(poll_ballot.add_investment(investment.id)).to be nil end it "is valid if investment is selected" do investment.update!(selected: true, price: 20000) - expect(poll_ballot.add_investment(investment.id)).to be true + expect(poll_ballot.add_investment(investment.id)).to be_truthy end end @@ -101,7 +101,7 @@ describe Poll::Ballot do end it "is valid if investment belongs to the poll's budget" do - expect(poll_ballot.add_investment(investment.id)).to be true + expect(poll_ballot.add_investment(investment.id)).to be_truthy end end @@ -112,7 +112,7 @@ describe Poll::Ballot do end it "is valid if does not already exist" do - expect(poll_ballot.add_investment(investment.id)).to be true + expect(poll_ballot.add_investment(investment.id)).to be_truthy end end end