Fix race condition with ballot lines

With two concurrent requests, it's possible to create two ballot lines
when only one of them should be created.

The reason is the code validating the line is not thread safe:

```
if ballot.amount_available(investment.heading) < investment.price.to_i
  errors.add(:money, "insufficient funds")
end
```

If the second request executes this code after the first request has
executed it but before the first request has saved the record to the
database, both records will pass this validation and both will be saved
to the database.

So we need to introduce a lock. Now when the second request tries to
lock the ballot, it finds it's already locked by the first request, and
will wait for the transaction of the first request to finish before
checking whether there are sufficient funds.

Note we need to disable transactions during the test; otherwise the
second thread will wait for the first one to finish.

Also note that we need to update a couple of tests because records are
reloaded when they're locked.

In one case, reloading the ballot causes `ballot.user` to be `nil`,
since the user is hidden. So we hide the user after creating all its
associated records (which is the scenario that would take place in real
life).

In the other case, reloading the ballot causes `ballot.user` to reload
as well. So we need to reload the user object used in the test too so it
gets the updates done on `ballot.user`.

I haven't been able to reproduce this behavior in a system test. The
following test works with Rails 5.0, but it stopped working when we
moved to system tests in commit 9427f014. After that commit, for reasons
I haven't been able to debug (reintroducing truncation with
DatabaseClaner didn't seem to affect this test, and neither did
increasing the number of threads in Puma), the two AJAX requests
executed here are no longer simultaneous; the second request waits for
the first one to finish.

scenario "Race conditions with simultaneous requests", :js do
  allow_any_instance_of(Budget::Ballot::Line).to receive(:check_sufficient_funds) do |object|
    allow(object).to receive(:check_sufficient_funds).and_call_original
    object.check_sufficient_funds
    sleep 0.3
  end

  ["First", "Second"].each do |title|
    create(:budget_investment, :selected,
      heading: california,
      price:   california.price,
      title:   title
    )
  end

  login_as(user)
  visit budget_investments_path(budget, heading_id: california.id)

  within(".budget-investment", text: "First") { click_link "Vote" }
  within(".budget-investment", text: "Second") { click_link "Vote" }

  expect(page).to have_link "Remove vote"
  expect(Budget::Ballot::Line.count).to eq 1
end

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