Add option for accessible_by querying strategy

[ghiculescu]

Inspired by this comment, let users choose the querying strategy cancan will use when calling accessible_by.

You can choose :subquery (the default since #619), or :left_joins (how it worked before #619).

Most tests run exactly the same on both strategies. Where they differ I have split into two separate test contexts.

I have deliberately not added this to the README, because out of the box I don't think you should change this. I would like to add a mention to https://github.com/CanCanCommunity/cancancan/wiki/Fetching-Records if this is merged (I don't have edit access to that page).

[ghiculescu]

@coorasse let me know your thoughts

[madmax]

@ghiculescu I think it will be god if we can decide by ability when using subquery and when left_joins not only globaly. Some queries will be better with joins some with subquery.

[ghiculescu]

I looked into doing it that way @madmax but it added a fair bit more complexity. I'm keen to start with this, because it allows everyone to match pre-#619 behavior. We can always add more options later.

[coorasse]

I love the effort you put in this PR! There is a lot of good work! I have to agree with @madmax . I strongly believe that this option could/should be also on a "per call" level. Because most of the times you might want to change it only in one specific place. Nevertheless: I am also in favour of a global configuration, so I am keen in merging after a review.

[ghiculescu]

For the record, we'll be using this configuration option globally in our app.

[coorasse]

I really like your work! There's only one question that I have

[coorasse]

I love the effort you put in this PR! There is a lot of good work! I have to agree with @madmax . I strongly believe that this option could/should be also on a "per call" level. Because most of the times you might want to change it only in one specific place. Nevertheless: I am also in favour of a global configuration, so I am keen in merging after a review.

[ghiculescu]

I thought of a way of making this work per-query without cluttering up the cancan API a lot.

CanCan.accessible_by_strategy = :subquery
User.accessible_by(current_ability) # uses :subquery

CanCan.with_accessible_by_strategy(:left_joins) do
  # all CanCan queries generated inside the block use :left_joins
  User.accessible_by(current_ability)
end

User.accessible_by(current_ability) # uses :subquery

Thoughts? Should be relatively easy to implement.

[coorasse]

What about opening a discussion regarding this point? I just enabled the tab in the project. We can start from your proposal @ghiculescu . So we can proceed with this PR and merge it in the meantime and proceed elsewhere to decide how we want to implement the per-query API.

[coorasse]

Maybe an update to the docs will be necessary, but it can be done in a separate PR. Thanks!

[coorasse]

I like this. very descriptive

[ghiculescu]

@coorasse happy to add docs, any suggestions on where? I was thinking https://github.com/CanCanCommunity/cancancan/blob/develop/docs/Defining-Abilities:-Best-Practices.md but I'm not sure any of the current docs feels right.

ps. I'm not sure what https://github.com/CanCanCommunity/cancancan/blob/develop/docs/mvc--deficiencies.md is but feel like it shouldn't be there.

pps. I started a discussion here #668

[coorasse]

Thanks for finding the irrelevant doc page, I removed it. The whole docs would need a bit of re-organisation. I'd definitely need some help there.
Probably here? https://github.com/CanCanCommunity/cancancan/blob/develop/docs/Fetching-Records.md