Merge pull request #1147 from consul/ballot-lines-redone

Ballot lines redux

app/models/budget/ballot.rb

@@ -2,11 +2,14 @@ class Budget
   class Ballot < ActiveRecord::Base
     belongs_to :user
     belongs_to :budget
-    belongs_to :heading
 
     has_many :lines, dependent: :destroy
     has_many :investments, through: :lines
 
+    def add_investment(investment)
+      lines.create!(budget: budget, investment: investment, heading: investment.heading, group_id: investment.heading.group_id)
+    end
+
     def total_amount_spent
       investments.sum(:price).to_i
     end
@@ -18,5 +21,15 @@ class Budget
     def amount_available(heading)
       budget.heading_price(heading) - amount_spent(heading.id)
     end
+
+    def valid_heading?(heading)
+      group = heading.group
+      return false if group.budget_id != budget_id
+
+      line = lines.where(heading_id: group.heading_ids).first
+      return false if line.present? && line.heading_id != heading.id
+
+      true
+    end
   end
 end

app/models/budget/ballot/line.rb

@@ -2,7 +2,25 @@ class Budget
   class Ballot
     class Line < ActiveRecord::Base
       belongs_to :ballot
+      belongs_to :budget
+      belongs_to :group
+      belongs_to :heading
       belongs_to :investment
+
+      validates :ballot_id, :budget_id, :group_id, :heading_id, :investment_id, presence: true
+      validate :insufficient_funds
+      validate :unfeasible
+
+      def insufficient_funds
+        return unless errors.blank?
+        errors.add(:money, "") if ballot.amount_available(heading) < investment.price.to_i
+      end
+
+      def unfeasible
+        return unless errors.blank?
+        errors.add(:unfeasible, "") unless investment.feasible?
+      end
+
     end
   end
 end

app/models/budget/investment.rb

@@ -21,6 +21,7 @@ class Budget
     validates :title, presence: true
     validates :author, presence: true
     validates :description, presence: true
+    validates :heading_id, presence: true
     validates_presence_of :unfeasibility_explanation, if: :unfeasibility_explanation_required?
 
     validates :title, length: { in: 4 .. Budget::Investment.title_max_length }
@@ -48,9 +49,6 @@ class Budget
 
     scope :for_render,             -> { includes(heading: :geozone) }
 
-    scope :with_heading,           -> { where.not(heading_id: nil) }
-    scope :no_heading,             -> { where(heading_id: nil) }
-
     before_save :calculate_confidence_score
     before_validation :set_responsible_name
 
@@ -152,7 +150,7 @@ class Budget
     def reason_for_not_being_ballotable_by(user, ballot)
       return permission_problem(user)    if permission_problem?(user)
       return :no_ballots_allowed         unless budget.balloting?
-      return :different_heading_assigned unless heading_id.blank? || ballot.blank? || heading_id == ballot.heading_id || ballot.heading_id.nil?
+      return :different_heading_assigned unless ballot.valid_heading?(heading)
       return :not_enough_money           if ballot.present? && !enough_money?(ballot)
     end
 

db/migrate/20160610094658_desnormalize_ballot_line.rb

@@ -0,0 +1,7 @@
+class DesnormalizeBallotLine < ActiveRecord::Migration
+  def change
+    add_column :budget_ballot_lines, :budget_id, :integer, index: true
+    add_column :budget_ballot_lines, :group_id, :integer, index: true
+    add_column :budget_ballot_lines, :heading_id, :integer, index: true
+  end
+end

db/migrate/20160614091639_remove_heading_id_from_ballot.rb

@@ -0,0 +1,5 @@
+class RemoveHeadingIdFromBallot < ActiveRecord::Migration
+  def change
+    remove_column :budget_ballots, :heading_id, :integer
+  end
+end

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: 20160609152026) do
+ActiveRecord::Schema.define(version: 20160614091639) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -83,6 +83,9 @@ ActiveRecord::Schema.define(version: 20160609152026) do
     t.integer  "investment_id"
     t.datetime "created_at",    null: false
     t.datetime "updated_at",    null: false
+    t.integer  "budget_id"
+    t.integer  "group_id"
+    t.integer  "heading_id"
   end
 
   add_index "budget_ballot_lines", ["ballot_id"], name: "index_budget_ballot_lines_on_ballot_id", using: :btree
@@ -93,11 +96,8 @@ ActiveRecord::Schema.define(version: 20160609152026) do
     t.integer  "budget_id"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
-    t.integer  "heading_id"
   end
 
-  add_index "budget_ballots", ["heading_id"], name: "index_budget_ballots_on_heading_id", using: :btree
-
   create_table "budget_groups", force: :cascade do |t|
     t.integer "budget_id"
     t.string  "name",      limit: 50

spec/factories.rb

@@ -222,6 +222,7 @@ FactoryGirl.define do
     association :heading, factory: :budget_heading
     association :author, factory: :user
     description          'Spend money on this'
+    price                10
     unfeasibility_explanation ''
     external_url         'http://external_documention.org'
     terms_of_service     '1'
@@ -249,8 +250,11 @@ FactoryGirl.define do
   end
 
   factory :budget_ballot_line, class: 'Budget::Ballot::Line' do
-    association :ballot, factory: :budget_ballot
-    investment { FactoryGirl.build(:budget_investment, :feasible) }
+    budget
+    ballot     { create :budget_ballot, budget: budget }
+    group      { create :budget_group, budget: budget }
+    heading    { create :budget_heading, group: group }
+    investment { create :budget_investment, :feasible, heading: heading }
   end
 
   factory :vote do

spec/models/budget/ballot/line_spec.rb

@@ -0,0 +1,104 @@
+require 'rails_helper'
+
+describe "Budget::Ballot::Line" do
+
+  let(:ballot_line) { build(:budget_ballot_line) }
+
+  describe 'Validations' do
+
+    it "should be valid" do
+      expect(ballot_line).to be_valid
+    end
+
+    it "should be invalid if missing id from ballot|budget|group|heading|investment" do
+      budget = create(:budget)
+      group = create(:budget_group, budget: budget)
+      heading = create(:budget_heading, group: group, price: 10000000)
+      investment = create(:budget_investment, :feasible, price: 5000000, heading: heading)
+      ballot = create(:budget_ballot, budget: budget)
+
+      ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: group, heading: heading, investment: investment)
+      expect(ballot_line).to be_valid
+
+      ballot_line = build(:budget_ballot_line, ballot: nil, budget: budget, group: group, heading: heading, investment: investment)
+      expect(ballot_line).to_not be_valid
+
+      ballot_line = build(:budget_ballot_line, ballot: ballot, budget: nil, group: group, heading: heading, investment: investment)
+      expect(ballot_line).to_not be_valid
+
+      ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: nil, heading: heading, investment: investment)
+      expect(ballot_line).to_not be_valid
+
+      ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: group, heading: nil, investment: investment)
+      expect(ballot_line).to_not be_valid
+
+      ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: group, heading: heading, investment: nil)
+      expect(ballot_line).to_not be_valid
+    end
+
+    describe 'Money' do
+      it "should not be valid if insufficient funds" do
+        budget = create(:budget)
+        group = create(:budget_group, budget: budget)
+        heading = create(:budget_heading, group: group, price: 10000000)
+        investment = create(:budget_investment, :feasible, price: heading.price + 1, heading: heading)
+        ballot = create(:budget_ballot, budget: budget)
+
+        ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: group, heading: heading, investment: investment)
+
+        expect(ballot_line).to_not be_valid
+      end
+
+      it "should be valid if sufficient funds" do
+        budget = create(:budget)
+        group = create(:budget_group, budget: budget)
+        heading = create(:budget_heading, group: group, price: 10000000)
+        investment = create(:budget_investment, :feasible, price: heading.price - 1, heading: heading, )
+        ballot = create(:budget_ballot, budget: budget)
+
+        ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: group, heading: heading, investment: investment)
+
+        expect(ballot_line).to be_valid
+      end
+    end
+
+    describe 'Feasibility' do
+      it "should not be valid if investment is unfeasible" do
+        budget = create(:budget)
+        group = create(:budget_group, budget: budget)
+        heading = create(:budget_heading, group: group, price: 10000000)
+        investment = create(:budget_investment, :feasible, price: 20000, feasibility: "unfeasible")
+        ballot = create(:budget_ballot, budget: budget)
+
+        ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: group, heading: heading, investment: investment)
+
+        expect(ballot_line).to_not be_valid
+      end
+
+      it "should not be valid if investment feasibility is undecided" do
+        budget = create(:budget)
+        group = create(:budget_group, budget: budget)
+        heading = create(:budget_heading, group: group, price: 10000000)
+        investment = create(:budget_investment, price: 20000, feasibility: "undecided")
+        ballot = create(:budget_ballot, budget: budget)
+
+        ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: group, heading: heading, investment: investment)
+
+        expect(ballot_line).to_not be_valid
+      end
+
+      it "should be valid if investment is feasible" do
+        budget = create(:budget)
+        group = create(:budget_group, budget: budget)
+        heading = create(:budget_heading, group: group, price: 10000000)
+        investment = create(:budget_investment, price: 20000, feasibility: "feasible")
+        ballot = create(:budget_ballot, budget: budget)
+
+        ballot_line = build(:budget_ballot_line, ballot: ballot, budget: budget, group: group, heading: heading, investment: investment)
+
+        expect(ballot_line).to be_valid
+      end
+    end
+
+  end
+end

spec/models/budget/ballot_spec.rb

@@ -4,32 +4,38 @@ describe Budget::Ballot do
 
   describe "#amount_spent" do
     it "returns the total amount spent in investments" do
-      inv1 = create(:budget_investment, :feasible, price: 10000)
-      inv2 = create(:budget_investment, :feasible, price: 20000)
+      budget = create(:budget)
+      group1 = create(:budget_group, budget: budget)
+      group2 = create(:budget_group, budget: budget)
+      heading1 = create(:budget_heading, group: group1, price: 100000)
+      heading2 = create(:budget_heading, group: group2, price: 200000)
+      inv1 = create(:budget_investment, :feasible, price: 10000, heading: heading1)
+      inv2 = create(:budget_investment, :feasible, price: 20000, heading: heading2)
 
-      ballot = create(:budget_ballot)
-      ballot.investments << inv1
+      ballot = create(:budget_ballot, budget: budget)
+      ballot.add_investment inv1
 
       expect(ballot.total_amount_spent).to eq 10000
 
-      ballot.investments << inv2
+      ballot.add_investment inv2
 
       expect(ballot.total_amount_spent).to eq 30000
     end
 
     it "returns the amount spent on all investments assigned to a specific heading" do
       heading = create(:budget_heading)
+      budget = heading.group.budget
       inv1 = create(:budget_investment, :feasible, price: 10000, heading: heading)
-      inv2 = create(:budget_investment, :feasible, price: 20000, heading: create(:budget_heading))
+      inv2 = create(:budget_investment, :feasible, price: 20000, heading: create(:budget_heading, group: heading.group))
       inv3 = create(:budget_investment, :feasible, price: 40000, heading: heading)
 
-      ballot = create(:budget_ballot)
-      ballot.investments << inv1
-      ballot.investments << inv2
+      ballot = create(:budget_ballot, budget: budget)
+      ballot.add_investment inv1
+      ballot.add_investment inv2
 
       expect(ballot.amount_spent(heading.id)).to eq 10000
 
-      ballot.investments << inv3
+      ballot.add_investment inv3
 
       expect(ballot.amount_spent(heading.id)).to eq 50000
     end
@@ -39,20 +45,22 @@ describe Budget::Ballot do
     it "returns how much is left after taking some investments" do
       budget = create(:budget)
       group = create(:budget_group, budget: budget)
-      heading = create(:budget_heading, group: group, price: 1000)
-      inv1 = create(:budget_investment, :feasible, price: 100, heading: heading)
-      inv2 = create(:budget_investment, :feasible, price: 200, heading: create(:budget_heading))
-      inv3 = create(:budget_investment, :feasible, price: 400, heading: heading)
+      heading1 = create(:budget_heading, group: group, price: 1000)
+      heading2 = create(:budget_heading, group: group, price: 300)
+      inv1 = create(:budget_investment, :feasible, price: 100, heading: heading1)
+      inv2 = create(:budget_investment, :feasible, price: 200, heading: heading2)
+      inv3 = create(:budget_investment, :feasible, price: 400, heading: heading1)
 
       ballot = create(:budget_ballot, budget: budget)
-      ballot.investments << inv1
-      ballot.investments << inv2
+      ballot.add_investment inv1
+      ballot.add_investment inv2
 
-      expect(ballot.amount_available(heading)).to eq 900
+      expect(ballot.amount_available(heading1)).to eq 900
+      expect(ballot.amount_available(heading2)).to eq 100
 
-      ballot.investments << inv3
+      ballot.add_investment inv3
 
-      expect(ballot.amount_available(heading)).to eq 500
+      expect(ballot.amount_available(heading1)).to eq 500
     end
   end
 

spec/models/budget/investment_spec.rb

@@ -66,7 +66,7 @@ describe Budget::Investment do
   end
 
   describe "by_admin" do
-    it "should return spending investments assigned to specific administrator" do
+    it "should return investments assigned to specific administrator" do
       investment1 = create(:budget_investment, administrator_id: 33)
       create(:budget_investment)
 
@@ -78,7 +78,7 @@ describe Budget::Investment do
   end
 
   describe "by_valuator" do
-    it "should return spending proposals assigned to specific valuator" do
+    it "should return investments assigned to specific valuator" do
       investment1 = create(:budget_investment)
       investment2 = create(:budget_investment)
       investment3 = create(:budget_investment)
@@ -99,7 +99,7 @@ describe Budget::Investment do
 
   describe "scopes" do
     describe "valuation_open" do
-      it "should return all spending proposals with false valuation_finished" do
+      it "should return all investments with false valuation_finished" do
         investment1 = create(:budget_investment, valuation_finished: true)
         investment2 = create(:budget_investment)
 
@@ -111,7 +111,7 @@ describe Budget::Investment do
     end
 
     describe "without_admin" do
-      it "should return all open spending proposals without assigned admin" do
+      it "should return all open investments without assigned admin" do
         investment1 = create(:budget_investment, valuation_finished: true)
         investment2 = create(:budget_investment, administrator: create(:administrator))
         investment3 = create(:budget_investment)
@@ -124,7 +124,7 @@ describe Budget::Investment do
     end
 
     describe "managed" do
-      it "should return all open spending proposals with assigned admin but without assigned valuators" do
+      it "should return all open investments with assigned admin but without assigned valuators" do
         investment1 = create(:budget_investment, administrator: create(:administrator))
         investment2 = create(:budget_investment, administrator: create(:administrator), valuation_finished: true)
         investment3 = create(:budget_investment, administrator: create(:administrator))
@@ -138,7 +138,7 @@ describe Budget::Investment do
     end
 
     describe "valuating" do
-      it "should return all spending proposals with assigned valuator but valuation not finished" do
+      it "should return all investments with assigned valuator but valuation not finished" do
         investment1 = create(:budget_investment)
         investment2 = create(:budget_investment)
         investment3 = create(:budget_investment, valuation_finished: true)
@@ -154,7 +154,7 @@ describe Budget::Investment do
     end
 
     describe "valuation_finished" do
-      it "should return all spending proposals with valuation finished" do
+      it "should return all investments with valuation finished" do
         investment1 = create(:budget_investment)
         investment2 = create(:budget_investment)
         investment3 = create(:budget_investment, valuation_finished: true)
@@ -170,7 +170,7 @@ describe Budget::Investment do
     end
 
     describe "feasible" do
-      it "should return all feasible spending proposals" do
+      it "should return all feasible investments" do
         feasible_investment = create(:budget_investment, :feasible)
         create(:budget_investment)
 
@@ -179,7 +179,7 @@ describe Budget::Investment do
     end
 
     describe "unfeasible" do
-      it "should return all unfeasible spending proposals" do
+      it "should return all unfeasible investments" do
         unfeasible_investment = create(:budget_investment, :unfeasible)
         create(:budget_investment, :feasible)
 
@@ -283,12 +283,12 @@ describe Budget::Investment do
 
   describe "#with_supports" do
     it "should return proposals with supports" do
-      sp1 = create(:budget_investment)
-      sp2 = create(:budget_investment)
-      create(:vote, votable: sp1)
+      inv1 = create(:budget_investment)
+      inv2 = create(:budget_investment)
+      create(:vote, votable: inv1)
 
-      expect(Budget::Investment.with_supports).to include(sp1)
-      expect(Budget::Investment.with_supports).to_not include(sp2)
+      expect(Budget::Investment.with_supports).to include(inv1)
+      expect(Budget::Investment.with_supports).to_not include(inv2)
     end
   end
 
@@ -327,11 +327,9 @@ describe Budget::Investment do
           expect(investment.reason_for_not_being_ballotable_by(user, ballot)).to be_nil
         end
 
-        it "accepts valid district selections" do
+        it "accepts valid selections" do
           budget.phase = "selecting"
           expect(investment.reason_for_not_being_selectable_by(user)).to be_nil
-          ballot.heading_id = heading.id
-          expect(investment.reason_for_not_being_selectable_by(user)).to be_nil
         end
 
         it "rejects users with different headings" do
@@ -340,22 +338,25 @@ describe Budget::Investment do
           california = create(:budget_heading, group: group)
           new_york = create(:budget_heading, group: group)
 
-          sp1 = create(:budget_investment, :feasible, heading: california)
-          sp2 = create(:budget_investment, :feasible, heading: new_york)
-          b = create(:budget_ballot, user: user, heading: california, investments: [sp1])
+          inv1 = create(:budget_investment, :feasible, heading: california)
+          inv2 = create(:budget_investment, :feasible, heading: new_york)
+          b = create(:budget_ballot, user: user, budget: budget)
+          b.add_investment inv1
 
-          expect(sp2.reason_for_not_being_ballotable_by(user, b)).to eq(:different_heading_assigned)
+          expect(inv2.reason_for_not_being_ballotable_by(user, b)).to eq(:different_heading_assigned)
         end
 
         it "rejects proposals with price higher than current available money" do
           budget.phase = "balloting"
           distritos = create(:budget_group, budget: budget)
           carabanchel = create(:budget_heading, group: distritos, price: 35)
-          sp1 = create(:budget_investment, :feasible, heading: carabanchel, price: 30)
-          sp2 = create(:budget_investment, :feasible, heading: carabanchel, price: 10)
-          ballot = create(:budget_ballot, user: user, heading: carabanchel, investments: [sp1])
+          inv1 = create(:budget_investment, :feasible, heading: carabanchel, price: 30)
+          inv2 = create(:budget_investment, :feasible, heading: carabanchel, price: 10)
 
-          expect(sp2.reason_for_not_being_ballotable_by(user, ballot)).to eq(:not_enough_money)
+          ballot = create(:budget_ballot, user: user, budget: budget)
+          ballot.add_investment inv1
+
+          expect(inv2.reason_for_not_being_ballotable_by(user, ballot)).to eq(:not_enough_money)
         end
 
       end