load resource and collections

[dgm]

I was surprised to see a test explicitly stating that index and collections do not load resources. :)

I little prodding and I think it would be possible to add

resource_base.accessible_by(@controller.current_ability)

to controller_resource.rb, plus a few related methods... and have it load the collection automatically. Note this is closer to rails 3 lazy loading though, it's a scope, not an array, so you can chain on more scope if needed.

[ryanb]

To clarify, you would like the index action to use accessible_by behind the scenes in a before filter similar to how the singular record is loaded in the other actions? So let's say we have a ProductsController. When doing load_resource in CanCan the before filter would do this for the index action.

@products = Product.accessible_by(current_ability)

Then I guess if one wants to add further restrictions it can be done through this scope.

def index
  @products = @products.where(:available => true)
end

Since this is just a scope, there's almost no performance cost. So if one decides to set @products to something else or not use it, no harm done.

I like this idea, but it would only work with Active Record or whatever responds to accessible_by. I'm opening this issue for discussion, what does everyone think?

[ryanb]

Also, this will not work if abilities are defined with blocks. Between that and it being Active Record only, I'm leaning toward keeping it something one manually adds to the index action.

[dgm]

When blocks are used, I thought load_and_authorize_resource was out of the question anyway. Why is accessible_by a problem? It's defined by cancan... beside, we could just check to see if it responds to it, and skip the creation as we already are doing... I'll fork and send a rough draft up...

[dgm]

I sent a pull request from http://github.com/dgm/cancan/tree/collection_resources

[ryanb]

Thanks for the pull request, I'll check it out. Blocks are useable with load_and_authorize_resource, it's just accessible_by which does not work with it.

[dgm]

oh, because it loads first, and then authorizes. I guess that's a major difference between this and declarative_authorization.

With a single resource, I guess that works fine... with any sort of collection, it would be nice to have the named scopes to help load it automatically. We could still jsut add a test to see if it's possible and bail when it is not possible.

[ryanb]

True, we could ask the current ability if a block is used and if so simply not use accessible_by. I'm not sure if the @products instance variable should be set to anything in the cases it can't be used. I'd rather not have it sometimes silently set it and other times not.

[dgm]

when it can't be used due to a block, set errors_on for the object so the template will render an error for the developer?

[ryanb]

errors_on? Can you give an example?

[dgm]

errors for? the standard rails error reporting .... Or we could insert a flash message... It shouldn't be too hard to alert the developer...

[ryanb]

The standard errors on a model are for working with a single model, not a collection like this. Also flash is for alerting the user, not necessarily the developer. But either way, if we present an error, how would the developer get rid of the error?

Setting @products to nil if there's no way to use accessible_by seems like the best solution to me, but I don't like the inconsistency of it sometimes being set and other times not.

[dgm]

IMHO, just skip it (don't set it to nil, just don't set it) as it already does now, when it's not possible. It should be obvious to the developer (and documented in cancan of course) that some obscure authorizations such as blocks cannot be scoped. In that case, you have to fall back to doing it manually, as you already do now.

I like having collections loading as much as possible for me, as it's more DRY, which is the whole point of load_resource anyway!

If you really don't want it falling under load_resource and load_and_authorize_resource how about a subtle addition: load_and_authorize_resources with an 's' which assumes the singular resource plus the collections when available...

[ryanb]

After thinking about it I'm warming up to the idea and will work on adding it to 1.4. My biggest concern is the inconsistency of sometimes setting the instance variable which requires one to modify the controller when switching between a conditions hash and block, but this already is a problem for accessible_by so it's nothing new.

[dgm]

exactly my thought... and the developer is going to hit a pretty quick warning about it when the index page returns a nil error. Just document it well too, cause it took me a long time to figure out that it didn't do it automatically. ;)

[ryanb]

load the collection instance variable on index action - closed by 9d91545