Improve update balance

app/models/account.rb

@@ -13,12 +13,12 @@ class Account < ActiveRecord::Base
 
   before_create :add_organization
 
+  # Denormalizes the account's balance summing the amount in all its movements.
+  # It will flag the account if its balance falls out of the min or max limits.
   def update_balance
     new_balance = movements.sum(:amount)
     self.balance = new_balance
-    if balance_changed?
-      self.flagged = !allowed?(balance)
-    end
+    self.flagged = !within_allowed_limits? if balance_changed?
     save
   end
 
@@ -51,8 +51,34 @@ def add_organization
 
   private
 
-  def allowed?(new_balance)
-    new_balance < (max_allowed_balance || Float::INFINITY) &&
-      new_balance > (min_allowed_balance || -Float::INFINITY)
+  # Checks whether the balance falls within the max and min allowed balance,
+  # none of these included
+  #
+  # @return [Boolean]
+  def within_allowed_limits?
+    less_than_upper_limit? && greater_than_lower_limit?
+  end
+
+  def less_than_upper_limit?
+    balance < max_allowed_balance
+  end
+
+  def greater_than_lower_limit?
+    balance > min_allowed_balance
+  end
+
+  # Returns the maximum allowed balance, falling back to infinite it not set
+  #
+  # @return [Integer | Float]
+  def max_allowed_balance
+    super || Float::INFINITY
+  end
+
+  # Returns the minimum allowed balance, falling back to minus infinite it not
+  # set
+  #
+  # @return [Integer | Float]
+  def min_allowed_balance
+    super || -Float::INFINITY
   end
 end

spec/models/account_spec.rb

@@ -39,4 +39,68 @@
     expect(organization.account.organization).to eq organization
   end
 
+  describe '#update_balance' do
+    let(:account) { member.account }
+
+    context 'when the balance did not change since last balance update' do
+      before do
+        account.movements << Movement.new(amount: 5)
+        account.save
+      end
+
+      it 'updates the account balance' do
+        # a callback in Movement already called #update_balance after creating
+        # the movement above
+        account.update_balance
+        expect(account.balance).to eq(5)
+      end
+
+      it 'does not flag the account' do
+        # a callback in Movement already called #update_balance after creating
+        # the movement above
+        account.update_balance
+        expect(account.flagged).to be_falsy
+      end
+    end
+
+    context 'when the balance changed since last balance update' do
+      context 'and the new balance falls within the limits allowed' do
+        before do
+          account.max_allowed_balance = 100
+          account.min_allowed_balance = 0
+
+          account.movements << Movement.new(amount: 5)
+          account.save
+        end
+
+        it 'updates the account balance' do
+          # a callback in Movement calls #update_balance after creating the
+          # movement above
+          expect(account.balance).to eq(5)
+        end
+
+        it 'does not flag the account' do
+          # a callback in Movement calls #update_balance after creating the
+          # movement above
+          expect(account.flagged).to be_falsy
+        end
+      end
+
+      context 'and the new balance does not fall within the limits allowed' do
+        before do
+          account.max_allowed_balance = 0
+          account.min_allowed_balance = 0
+
+          account.movements << Movement.new(amount: 5)
+          account.save
+        end
+
+        it 'does not flag the account' do
+          # a callback in Movement calls #update_balance after creating the
+          # movement above
+          expect(account.flagged).to be_truthy
+        end
+      end
+    end
+  end
 end