Merge pull request #4061 from consul/ballot_race_condition

Fix race condition with ballot lines

Gemfile

@@ -68,6 +68,7 @@ end
 group :development, :test do
   gem "bullet", "~> 5.9.0"
   gem "byebug", "~> 11.1.1"
+  gem "database_cleaner", "~> 1.7.0"
   gem "factory_bot_rails", "~> 4.8.2"
   gem "faker", "~> 1.8.7"
   gem "i18n-tasks", "~> 0.9.29"
@@ -94,7 +95,6 @@ group :development do
   gem "capistrano-rails", "~> 1.4.0", require: false
   gem "capistrano3-delayed-job", "~> 1.7.3"
   gem "capistrano3-puma", "~> 4.0.0"
-  gem "database_cleaner", "~> 1.7.0"
   gem "erb_lint", require: false
   gem "github_changelog_generator", "~> 1.15.0"
   gem "mdl", "~> 0.5.0", require: false

app/models/budget/ballot/line.rb

@@ -19,6 +19,7 @@ class Budget
       after_save :store_user_heading
 
       def check_sufficient_funds
+        ballot.lock!
         errors.add(:money, "insufficient funds") if ballot.amount_available(investment.heading) < investment.price.to_i
       end
 

spec/models/budget/ballot/line_spec.rb

@@ -26,6 +26,18 @@ describe Budget::Ballot::Line do
         investment.update!(price: heading.price - 1)
         expect(ballot_line).to be_valid
       end
+
+      it "validates sufficient funds when creating lines at the same time", :race_condition do
+        investment.update!(price: heading.price)
+        other_investment = create(:budget_investment, :selected, price: heading.price, heading: heading)
+        other_line = build(:budget_ballot_line, ballot: ballot, investment: other_investment)
+
+        [ballot_line, other_line].map do |line|
+          Thread.new { line.save }
+        end.each(&:join)
+
+        expect(Budget::Ballot::Line.count).to be 1
+      end
     end
 
     describe "Selectibility" do
@@ -49,7 +61,7 @@ describe Budget::Ballot::Line do
 
       create(:budget_ballot_line, ballot: ballot, investment: investment)
 
-      expect(user.balloted_heading_id).to eq(investment.heading.id)
+      expect(user.reload.balloted_heading_id).to eq(investment.heading.id)
     end
   end
 

spec/models/budget/stats_spec.rb

@@ -10,11 +10,13 @@ describe Budget::Stats do
     let!(:author_and_voter) { create(:user, :hidden, votables: [investment]) }
     let!(:voter) { create(:user, votables: [investment]) }
     let!(:voter_and_balloter) { create(:user, votables: [investment], ballot_lines: [investment]) }
-    let!(:balloter) { create(:user, :hidden, ballot_lines: [investment]) }
+    let!(:balloter) { create(:user, ballot_lines: [investment]) }
     let!(:poll_balloter) { create(:user, :level_two) }
     let!(:non_participant) { create(:user, :level_two) }
 
     before do
+      balloter.hide
+
       create(:budget_investment, :selected, budget: budget, author: author_and_voter)
 
       create(:poll_voter, :from_booth, user: poll_balloter, budget: budget)

spec/spec_helper.rb

@@ -30,6 +30,15 @@ RSpec.configure do |config|
     Setting["feature.user.skip_verification"] = nil
   end
 
+  config.around(:each, :race_condition) do |example|
+    self.use_transactional_tests = false
+    example.run
+    self.use_transactional_tests = true
+
+    DatabaseCleaner.clean_with(:truncation)
+    Rails.application.load_seed
+  end
+
   config.before(:each, type: :system) do
     Capybara::Webmock.start
   end