Custom collection method not called if using CanCan's load_resource to load collection

[TylerRick]

It can be surprising when you have a collection method defined but it's not getting called. I wasted an hour or so on this today. Through trial and error, I discovered that when I removed the load_resource callback, collection started getting called again...

As noted on https://github.com/CanCanCommunity/cancancan/wiki/Inherited-Resources:

Warning: when overwriting the collection method in a controller the load part of a load_and_authorize_resource call will not work correctly. See ryanb/cancan#274 for the discussions..

In this case you can override collection like

skip_load_and_authorize_resource :only => :index                                
                                                                                
def collection                                                                  
  @products ||= end_of_association_chain.accessible_by(current_ability).paginate(:page => params[:page], :per_page => 10)
end                                                                             

So there is a workaround. The question is: should it automatically call collection?

ryanb/cancan#274 proposed

load_resource does not seem to call the collection method to load the resource. I think it should.

So that behavior was added ([in MR/commit __]), but later reverted ([in MR/commit __]) since it apparently broke other things.

Overriding load_collection to use collection was my first thought, as well. I wonder what problems that caused. (More research needed.)

So, is there a way we can automatically integrate this so your custom collection method (or end_of_association_chain) gets called if you've defined one?

It seems like it should, to be consistent with how we automatically call the controller's custom resource method if one is defined...

---

One proposal which I really liked was this comment from @aq1018:

override #end_of_association_chain to call #accessible_by at the end. This makes more sense to me because I consider accessible_by apply extra scope for authorization purposes, which should be consider really the 'end' of the association chain, ensuring #find to be scoped correctly.

But @amw makes a good point:

end_of_association_chain is widely used in inherited_resources, not just for collection. You don't want accessible_by called when loading resource instance.

Also block ability rules don't support accessible_by, and your code will fail for abilities that use them. See load_collection?.

Seems like we could handle the latter case (possibly by reusing load_collection?).

---

Some people have commented that if you override collection, then you should take responsibility for everything — both loading collection and authorizing it. This comment, for example:

If you're overriding collection, you're taking responsibility for setting the instance variable, so you don't want to use load_resource in a before filter. I'm not sure that it makes sense for cancan to insert itself in this case. If you're going to bypass load_resource you may as well handle the authorize part in your collection definition.

Another proposal was from @vitaly in this comment:

What about overwriting apply_scopes with cancan so that you can apply accessible_by(current_ability) on the passed scope. then IR's collection will just work including all the overwrites, pagination and all.

That also seems like a pretty reasonable option.

Any thoughts?