cancan sucks

[Hernan Astudillo]

if you set a rule for an object with conditions for the parent, say:

can :update, Child, :parent => {:user_id => current_user.id}

adds a Join, which goes through the cancan_bridge up to AS beginning_of_chain
making @record readonly  (record is originated at find_if_allowed at update.rb), and by that, disabling for update.

I was trying to find how to get rid of this. Rails people say that you should avoid :joins and use :includes instead, or force :readonly => false. But to force :readonly => false it will require to change that in the global finder.rb which doesn't look like a good idea. I think. what do you say?

cheers,
nano

[Nick Roosevelt]

If you use a col name instead of an association, there will be no join, so depending on your model, perhaps it is easy to just put the condition on parent_id, or but perhaps you dont' have enough info in the model's table itself to do the condition, in which case at least one more query or a join may be necessary.  By the way, joins are not evil!

--
You received this message because you are subscribed to the Google Groups "ActiveScaffold : Ruby on Rails plugin" group.
To post to this group, send email to actives...@googlegroups.com.
To unsubscribe from this group, send email to activescaffol...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/activescaffold?hl=en.

[Hernan Astudillo]

That works only for conditions checking the id:

:user_id => current_user.id

but doesn't work for any deeper than that

:parent => {:user_id => current_user.id}

As the docs says, accessing parent in ability ( https://github.com/ryanb/cancan/wiki/Nested-Resources )

can :manage, Task, :project => { :user_id => user.id }

Which is fine for filtering lists and :show in AS. But as for :update, or :delete, you're screwed since the code is all full of join everywhere:

https://github.com/ryanb/cancan/blob/master/lib/cancan/model_adapters/active_record_adapter.rb

join join join... switching for :includes looks like a looong way.

They could've done using :include instead of :join, since :include doesn't make :readonly. as explained here: http://stackoverflow.com/questions/639171/what-is-causing-this-activerecordreadonlyrecord-error

So IMHO:
The problem doesn't affect all ruby. Just AS. Ways to fix it are:
1) AS :edit and :destroy shouldn't go through beginning_of_chain, or at least, use it just to check authorization and then fetch the clean record directly with no joins and complex sql, which may affect record's associations handling.
2) AS :edit and :destroy could use find_if_allowed(id).readonly(false) (if it works, not tested, RoR3-arel says it works).
3) as you can see in https://github.com/ryanb/cancan/blob/master/lib/cancan/model_additions.rb , accessible_by is made for narrowing list of records. Suitable for :index.

joins aren't evil. In fact, they're more efficient than :includes which implies left outer join. But they're not suitable for writing records.

cheers,

[Sergio Cambra]

I think joins didn't set readonly in previous versions, but now AS calls readonly(false) in nested when joins is used. It should fix this issue