5
\$\begingroup\$

I am new to Rails and have a system that needs to process transactions.

A user can enter a transaction to which one or more users are tied. These users owe some amount of money to the person making the transaction. For example, Bill might buy lunch for 4 friends and the bill is $125. They decide to split the bill 5 ways, so each owes $25. Bill would enter a total of $125 and enter each friend (including himself) as owing $25 to the transaction.

I have code in my controller and in my model to accomplish this goal, but I don't really know if I am using transactions and locking correctly. Also, is this the intended way to have this information in the controller?

I am using a transaction since all of these actions must occur together or fail (atomicity) and I need locking in case multiple users try to submit at the same time (isolation).

Maybe I should let the db on the backend handle locking? Does it do that already - say, MySQL?

trans_controller.rb

class TransController < ApplicationController
    # POST trans/
    def create
        @title = "Create Transaction"
        trans_successful = false

        # Add the transaction from the client
        @tran = Tran.new(params[:tran])

        # Update the current user
        @tran.submitting_user_id = current_user.id

        # Update the data to the database
        # This call commits the transaction and transaction users 
        # It also calls a method to update the balances of each user since that isn't
        # part of the regular commit (why isn't it?)
        begin 
            @tran.transaction do
                @tran.save! 
                @tran.update_user_balances
                trans_successful = true
            end 
        rescue

        end

        # Save the transaction
        if trans_successful
            flash[:success] = 'Transaction was successfully created.'
            redirect_to trans_path
        else
            flash.now[:error] = @tran.errors.full_messages.to_sentence          
            render 'new'
        end
    end

tran.rb

class Tran < ActiveRecord::Base
    has_many :transaction_users, :dependent => :destroy, :class_name => 'TransactionUser'
    belongs_to :submitting_user, :class_name => 'User'
    belongs_to :buying_user, :class_name => 'User'

    accepts_nested_attributes_for :transaction_users, :allow_destroy => true

    validates :description, :presence => true,
                            :length => {:maximum => 100 }
    #validates :total,      :presence => true
    validates_numericality_of :total, :greater_than => 0

    validates :submitting_user_id,      :presence => true               
    validates :buying_user_id,          :presence => true   

    #validates_associated :transaction_users

    validate :has_transaction_users?
    validate :has_correct_transaction_user_sum?
    validate :has_no_repeat_users?

    def update_user_balances
        # Update the buying user in the transaction
        self.buying_user.lock!
        # Update the user's total, since they were in the transction
        self.buying_user.update_attribute :current_balance, self.buying_user.current_balance - self.total
        # Add an offsetting transaction_user for this record
        buying_tran_user = TransactionUser.create!(:amount => -1 * self.total, :user_id => self.buying_user_id, :tran => self)
        #if buying_tran_user.valid?
        #   raise "Error"
        #end

        # Loop through each transaction user and update their balances.  Make sure to lock each record before doing the update.
        self.transaction_users.each do |tu|
            tu.user.lock!
            tu.user.update_attribute :current_balance, tu.user.current_balance + tu.amount
        end
    end

    def has_transaction_users?
        errors.add :base, "A transcation must have transaction users." if self.transaction_users.blank?
    end

    def has_correct_transaction_user_sum?
        sum_of_items = 0;

        self.transaction_users.inspect
        self.transaction_users.each do |key|
            sum_of_items += key.amount if !key.amount.nil?
        end

        if sum_of_items != self.total
            errors.add :base, "The transcation items do not sum to the total of the transaction." 
        end 
    end

    def has_no_repeat_users?
        user_array = []
        self.transaction_users.each do |key|
            if(user_array.include? key.user.email) 
                errors.add :base, "The participant #{key.user.full_name} has been listed more than once."
            end

            user_array << key.user.email
        end
    end 
end
\$\endgroup\$
3
\$\begingroup\$

I think it's a bad practice to use transactions in Controllers. You should add a method on the Tran model instead (for example, save_and_update_user_balances):

def create
    @title = "Create Transaction"

    # Add the transaction from the client
    @tran = Tran.new(params[:tran])

    # Update the current user
    @tran.submitting_user_id = current_user.id

    # Update the data to the database
    # This call commits the transaction and transaction users 
    # It also calls a method to update the balances of each user since that isn't
    # part of the regular commit (why isn't it?)
    if(@tran.save_and_update_user_balances)
      redirect_to trans_path, success: 'Transaction was successfully created.'
    rescue
      flash.now[:error] = @tran.errors.full_messages.to_sentence          
      render 'new'
    end
end

In tran.rb:

def save_and_update_user_balances
  transaction do
    begin
      save! 
      update_user_balances!
      true
    rescue
      false
    end
  end
end

In update_user_balances! use update_attributes! and force to fail (and rollback) if an error happens.

Also, since update_user_balances! is called inside a transaction, you don't need to create another transaction, or lock. This is because of the way ActiveRecord works:

Though the transaction class method is called on some Active Record class, the objects within the transaction block need not all be instances of that class. This is because transactions are per-database connection, not per-model.

So the method should be:

def update_user_balances!
    self.buying_user.update_attributes! current_balance: self.buying_user.current_balance - self.total

    # Add an offsetting transaction_user for this record
    # Note: You could improve this by using a relationship to create it, like this?
    # transaction_users.create!(amount: -1 * self.total, user_id: buying_user_id)
    TransactionUser.create!(:amount => -1 * self.total, :user_id => self.buying_user_id, :tran => self)

    # Loop through each transaction user and update their balances.
    transaction_users.each do |tu|
        tu.user.update_attributes! current_balance: tu.user.current_balance + tu.amount
    end
end

For further reference: Ruby on Rails documentation on Transactions.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.