Delete intrusive temporary object from proxy object

[dgm]

When we create an object in this proxy object, it persists on into the rest of the view, which can cause unintended problems.

I think wrapping this in a begin/ensure means the return value is still whatever the function returned, but we can restore the proxy object back to what is was before.

[dgm]

This still doesn't fix a bigger problem of how to test a proxied object. Take for example, a Person has_many Households, :through => :household_people

p=Person.first h=p.households.build h.people #=> []

In order for the rule to work, that last array needs the person in it!

has_permission_on [:households], :to => [:index, :show] do
  if_attribute :people => {:user => is { user }}
end

How can we get the proxy object to work in reverse?

[urkle]

Another place where this temporary proxy object bites me is with nested controllers. the new_controller_object_for_collection method creates a new dummy object on an association proxy which throws off the amount of items in the list.. e.g.

# inside IssuesController where Issues is nested_in Sprockets
filter_resource_access :nested_in => :sprockets
def index
   @issues = @sprocket.issues
# @issues.last.persisted? == false !!!

end

The workaround I'm using currently is tacking on with_permissions_to so @sprocket.issues.with_permissions_to

[dgm]

As for the reverse direction, I wonder if there is a way for reflections to find the relationship going the other way, and and tie it together. Might be hard if there are multiple ways...

[stffn]

I agree that we need to fix this behavior.

However, we'd need tests for both patches so that I can merge them.

Another point on wrapping the object.new call in begin/rescue: how is the performance impact from this? Isn't rescue rather costly? How often do we expect the permitted_to! to raise?

[dgm]

It's not raise issue, but rather that the object needs to be torn down no matter what return route is taken. If you don't want to use begin/rescue, then you have to rewrite the entire function to not use multiple returns in the middle of the function.

I don't think it is a big performance issue -someone feel free to show me a use case where it is. (and running it 1 million times is not a use case)

[jhawthorn]

How about using options[:object].scoped.new instead, as this will never add the temporary object to the association.

https://github.com/jhawthorn/declarative_authorization/commit/8f34765e04755c89a5a985c2cdc27c15f32eab57

[dgm]

I'm not sure that will work because it needs the relationship to work backwards, and I think it actually queries the database... Been a while since I looked at it.

[jhawthorn]

@dgm as far as I can tell the record returned by .scoped.new behaves the same as .new (other than, of course, not being added to the association).

With .new

>> u = User.first
User Load (0.4ms)  SELECT `users`.* FROM `users` LIMIT 1
=> #<User id: 1,...
>> u.comments.new
=> #<Comment id: nil, user_id: 1, body: nil, created_at: nil, updated_at: nil>
>> _.user
User Load (0.7ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` = 1 LIMIT 1
=> #<User id: 1, ...
>> u.comments
=> [#<Comment id: nil, user_id: 1, body: nil, created_at: nil, updated_at: nil>]

vs .scoped.new

>> u = User.first
User Load (0.4ms)  SELECT `users`.* FROM `users` LIMIT 1
=> #<User id: 1,...
>> u.comments.scoped.new
=> #<Comment id: nil, user_id: 1, body: nil, created_at: nil, updated_at: nil>
>> _.user
User Load (0.7ms)  SELECT `users`.* FROM `users` WHERE `users`.`id` = 1 LIMIT 1
=> #<User id: 1, ...
>> u.comments
=> []

I've used this workaround previously and not yet encountered issues.

[stffn]

Fixed in #152