Bulk Update Controller Action and ActiveRecord Transactions

Ruby on Rails
1

Situation: A payment logging system. A user can accept multiple payments (cash or check) which get logged as ‘pending review’. The user then goes to a new page that shows a summary of all the pending payments, they verify all the pending payments and verify that the program’s total matches their total(on a calculator). Then they want a single button to accept all the pending payments.

Initial Implementation: Controller finds the clients pending payments and calls the service object PaymentApplication which provides some other complex logic before updating the payment as ‘applied’.

class PaymentsController < ApplicationController
  def apply_pending
    payments = @client.payments.pending
    return_value = false
    Payment.transaction do
      payments.each do |payment|
        pay_apply = PaymentApplication.new(payment)
        pay_apply.apply
     end
     return_value = true
  end
end

Issue #1: Is this an improper way to use transactions? There’s gotta be a better way to do that, and should I be raising an active record rollback or something instead of that business with the ‘return_value’ variable?

Issue #2: What if another user from the same client adds more pending payments while user #1 is reviewing his payments? The current implementation applies all pending payments. How would you avoid that? Would you just include the payment ids in the view somehow and then pass them back to the controller apply action indicating the payment ids which should be accepted?

2

Transactions or not, your solution to Issue #2 will avoid the race conditions between user #1’s reviews and user #2’s new payments, so yeah - I think it makes total sense to pass the payment ids as an array to the controller. Otherwise, you might have a user accepting a payment they didn’t see in your “review” view.

It looks like your example is missing some code. I assume you’re returning return_value at the bottom of #apply_pending - I agree that feels odd but I don’t know of a better way. Perhaps others can chime in.

3

Thanks for the feedback. You’re right, I did leave off my respond_to block which utilizes the return_value…

respond_to do |format|
  if return_value == true
    format.html { redirect_to payments_path, :notice => "Payments have been applied." }
  else
    format.html { redirect_to review_payments_path, :notice => "Payment application failed." }
  end
end
4

So how would I go about passing an array of ids to a controller?

Here’s my initial thought for the view code…

link_to "Accept Payments", apply_pending_payments_path("payment_ids" => @payments.pluck(:id).to_s)

Which produces a link that looks like this:
http://localhost:3000/payments?payment_ids=[20%2C+13%2C+12%2C+9%2C+8%2C+6%2C+4%2C+2%2C+1%2C+16]

Is that a proper way to handle that? It looks sorta strange.

Or should I be using some sort of hidden form or something?

5

Have you thought about making a form where a pending transaction has a checkbox. Then you can check off which payments they want to confirm and submit that info.

6

Yeah, that might be the way to go… The customer wants to be able to just look at a summary, confirm that the total matches her total and click a single button to apply. She will sometimes accept upwards of a hundred payments at a time and it would be a pain to select all those checkboxes.

Though I guess I could preselect them all, then should could deselect as she sees fit?

7

Like with other applications you can have a button for “select all”, “deselect all” to help.

8

Yeah, but why make it so easy? :smile:

Sounds like the way to go. Thanks man.