batch_action methods param should be an ActiveRecord::Relation object, not an array of ActiveRecord::Base objects

[benjamin-thomas]

In order to be consistent with the filter API, more flexible, and allow more performance, I think this should be considered.

Right now the following code generates 30 queries (for 30 ids selected)

def batch_action_publish(array)
  array.each do |article|
    article.update(publish: true)
  end
end

Whereas this one would generate just one

def batch_action_publish(relation)
  relation.update_all(publish: true)
end

What do you think? With a relation object, one could still loop over the collection, if that's required.

[anderscarling]

👍

[jensljungblad]

First problem is we run find on an array of ids, which returns an array and not a relation. Mainly because we wanted it to raise an error if the id didn't exist. We could probably replace that with a where instead.

Second problem is this part:

if authorization_enabled?
  records = records.select { |r| policy(r).send("batch_action_#{params[:batch_action]}?") }
end

Basically we run each record through the authorization policy, if authorization is enabled.
The code above filters out those records you are not permitted to batch action. We could replace this with a new query. So we'd have an additional query prior to the batch action executing, but a lot less for the actual batch action.

[benjamin-thomas]

Got you. Haven't had a change to play with the policy object yet, but I may have a suggestion.

If the policy object looked like this:

class ArticlePolicy < Godmin::Authorization::Policy
  def batch_action_destroy?
    Article.where(user_id: user.id)
  end
end

Then you could do something along those lines:

if authorization_enabled?
  policy(relation).send("batch_action_#{params[:batch_action]}?")
end

Would that be granular enough?