Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

added support for nested join conditions #806

Merged
merged 1 commit into from Feb 22, 2013

Conversation

Projects
None yet
8 participants
Contributor

yuszuv commented Jan 20, 2013

There's a bug/feature incompleteness in the active_record_adapter, if you specify conditions via nested joins. E.g. if there's a comment model, that belongs_to a article, that belongs_to a category, you might to specify

can  :read, Comment, :article => { :category => { :condition => true } }

At the moment the neccessary joins are extracted correctly by CanCan::ModelAdapters::ActiveRecordAdapter#joins, i.e. { :article => :category }. But the conditions hash for the where clause is still nested, i.e. it becomes { :articles => { :categories => { :condition => true } } }. My patch does correct this to { :categories => { :condition => true } }.

chadcf commented Jan 23, 2013

Just to add this seems to be a recent issue with rails. My app did this and has been working fine for over a year with rails 3.2.02. I recently applied the 3.2.11 update and my cancan nested joins broke.

antage commented Jan 26, 2013

I use this fork with my application (rails 3.2.11).
Following ability works fine.

can :read, TeaserStat, teaser: { campaign: { user_id: user.id } }

TeaserStat.accessibly_by(Ability.new(User.first)).to_sql generates:

SELECT "teaser_stats".* FROM "teaser_stats" INNER JOIN "teasers" ON "teasers"."id" = "teaser_stats"."teaser_id" INNER JOIN "campaigns" ON "campaigns"."id" = "teasers"."campaign_id" WHERE "campaigns"."user_id" = 1

Current version of cancan generates something strange:

SELECT "teaser_stats".* FROM "teaser_stats" INNER JOIN "teasers" ON "teasers"."id" = "teaser_stats"."teaser_id" INNER JOIN "campaigns" ON "campaigns"."id" = "teasers"."campaign_id" WHERE "teasers"."campaigns" = '---
:user_id: 1
'

@runemadsen runemadsen referenced this pull request Feb 8, 2013

Closed

CanCan status? #819

natebird commented Feb 8, 2013

Just as an FYI - the Travis CI build passed. Not sure why GitHub isn't updated yet. It's been 19 days…

tonywok commented Feb 11, 2013

Rails 3.2.6 introduced this problem. I suspect a lot of people are going to be affected w/ the push to upgrade to Rails 3.2.11.

This pull request seems to fix my issue and there's some tests to back it up. I'd love to see this merged in. 👍

Thanks for this, I'm currently using the fork.

chadcf commented Feb 11, 2013

Yep, I've been running this in production for several weeks now, no problems.

ryanb added a commit that referenced this pull request Feb 22, 2013

Merge pull request #806 from yuszuv/support_for_nested_joins
added support for nested join conditions

@ryanb ryanb merged commit cbd352c into ryanb:master Feb 22, 2013

1 check was pending

default The Travis build is in progress
Details
Owner

ryanb commented Feb 22, 2013

Thanks for the contribution! Didn't realize Rails 3.2.6 introduced this problem.

I just pulled master to get this changeset, however, it appears that is fails when doing something like this:

can  :read, Comment, :article => { :condition => true , :condition2 => true }

ie, specifiing 2 conditions, even for a single join. The above works correctly for the released 1.6.9 version. If I remove the condition2, it works. Same goes for a 2-level join, like the one in the original issue description:

can  :read, Comment, :article => { :category => { :condition => true, :condition2 => true } }

I've written a failing test that I hope demonstrates @koukou73gr 's issue. @yuszuv, can you please take a look and tell us if we're on the right track?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment