Association issue

[cj]

Hi,

So we have a model structure like https://gist.github.com/cj/701807d253fa288d665d

When we do:

@signing = Signing.restrict!(current_user).find params[:signing_id]
@notes = @signing.notes.joins(
  :creator
).includes(
  :creator
)

We are getting this error: Association named 'role' was not found on SigningNote; perhaps you misspelled it?

Any ideas on what could be the issue? We really want to use your gem! :)

[inossidabile]

Interesting. I managed to reproduce it. Trying to get what's going on. A quick tip for you meanwhile, don't do .joins(:field).includes(:field) – there's .eager_load(:field) in ActiveRecord.

[inossidabile]

It's a bug of ActiveRecord: https://github.com/rails/rails/blob/master/activerecord/lib/active_record/relation/merger.rb#L50-L52. It sums arrays of eager_loading and includes instead of nesting it like it does with where and join. I'm not sure how to arrange it right now – whether I should make a workaround inside of Protector or produce a pull request. Probably both doh.

What you can do right now: remove default_scope from the User model. It's the only issue at the moment. Regarding your second question: it's nil and it's correct – Protector is a white-list security. It means you can not read any field of your model unless can :view is called. You snippet tries to read creator_id field of the protected SigningNote model. And it can not. Add can :view or can :view, %w(creator_id) to your protect block and it will work fine.

[inossidabile]

Here's how to reproduce the bug without Protector: SigningNote.eager_load(:creator).merge(User.all)

[inossidabile]

Additionaly you can remove .joins and leave only .includes. It will solve the issue you are trying to solve. One more additional SQL primary-key indexed query won't make a big change.

[inossidabile]

Okay, so here's what I tell you:

1. ActiveRecord (any version) does not respect default_scope when you eager_load association but it treats it well when you preload association (which is SO WEIRD).
2. ActiveRecord 4 breaks all method consistency. It acts differently for model class and relation.

I've implemented workaround for the second issue and fixed Protector to follow the default behaviour (dropping default_scope). Your code should work now.

But! I STRONGLY advice you to:

1. Stop using .joins(...).includes(...). And even better eager_load(...) alias. It gonna bite you. If this is optimization, it not gonna work. Two queries here are never slower than one. But you make it REALLY harder for the AR to map data. Just to make sure you understand it right: eager_load is a rescue tool. Use it when you have to, at the same time, join association and filter by it. In any other case – DO NOT use eager_load unless you are really sure in that.
2. Stop using default_scope. It has already bitten you and I bet you didn't notice. Since AR drops default_scope for eager_loading – you never actually know what exactly is executing among your query. It has ton of another downsides like unpredictable order (that you also do already in your code as well).

Both of this tools are sugar for complex cases. Using them together is a hell. Do not do that. However with the latest commit Protector has nothing to do with this advices. I think it should work no matter what even if you decide to not follow my suggestions ;).

Try using master branch from github. If it works for you I will release a minor tick.

[cj]

@inossidabile Thank you for the quick response and the detailed help. That latest commit does indeed fix the issue. I think I'll go with your advice though and remove the default_scope :)

Thank you again!