Pull request #806 breaks ability condition building #859
Comments
|
When you say "cancan fails", do you mean that a test is failing? Are you saying that the issue described in #806 still exists? If the tests are all passing, can you describe the error, or better yet write a failing test? Thanks! |
|
Thanks for looking into this. I mean it errors inside CanCan. With a schema like: User has_many Alerts, Alert belongs_to InstallationSite, InstallationSite belongs_to CityArea and an ability definition such as: can :read, Alert , :installation_site => {
:city_area => {
:country => target.country, :region_name => target.region
}
}I get: If only one condition is specified, say: can :read, Alert , :installation_site => {
:city_area => {
:country => target.country
}
}It works as expected. can :read, InstallationSite,
:city_area => {
:country => target.country, :region => target.region
}it stills errors at the same point. Master was at @ff2b632f1524 when I submited this. I also tried ther recent, 1.6.10 release, the result is the same. With the 1.6.9 release there is no error generated, just the messed SQL string when doing two joins (first case above) that #806 was trying to solve. I am arguing that #806 is to blame because it was the only change since 1.6.9 release that touched active_record_adapter.rb (although I could be wrong, my GitHub skills are not that great :( ) |
|
I've written a failing test which produces the It's just a starting point though. Can you help me refine the test so that it asserts the correct behavior? Thanks. |
|
Your test looks right to me. To play safe, I'd also add one for the single join as well. Something like it "should support more than one nested conditions" do
@ability.can :read, Article, :category => {
:name => 'foo', :visible => true
}
expect { Article.accessible_by(@ability) }.to_not raise_error
end |
|
As for asserting the correct behaviour, I'd just copy-paste the existing deeply-nested test from @yuszuv : it "should return appropriate sql conditions in complex case with nested joins and more than one nested conditions" do
@ability.can :read, Comment, :article => { :category => { :visible => true , :name => 'foo'} }
@ability.model_adapter(Comment, :read).conditions.should == { Category.table_name.to_sym => { :visible => true , :name => 'foo' } }
end
it "should return appropriate sql conditions in complex case with nested joins of different depth and more than one nested conditions" do
@ability.can :read, Comment, :article => { :published => true, :category => { :visible => true, :name => 'foo' } }
@ability.model_adapter(Comment, :read).conditions.should == { Article.table_name.to_sym => { :published => true }, Category.table_name.to_sym => { :visible => true, :name => 'foo' } }
end |
|
I added pull request #871 that fixes the issue and incorporates @jaredbeck's test condition from 45bc237 |
|
Hey, that was a good catch !!! :) I confirm that #871 fixes my problem as reported. Please consider applying. |
|
Just came across this issue too. Would it be possible to have this merged and a new gem version released? |
|
Also ran into this. Pretty painful. |
* ricec/master: Fixes nested ability conditions in issue ryanb#859
|
IMO this defect should be re-opened until that pull request is applied. Are there any contributors able to apply the PR? |
|
I've been running into this issue as well. Could a contributor please apply the PR? |
- The "name" variable in the tableized_conditions method can potentially be over written while looping through conditions causing the next iteration of the loop to fail - Failure will occur on deeply nested conditions or on where clauses with more than one condition specified - See the links belows for details ryanb#889 ryanb#859
|
I think most people consider this particular project dead. Fortunately there is a community led fork here: https://github.com/CanCanCommunity/cancancan It includes the pull request above and many more (including support for strong parameters). I've recently switched to it and it was seamless (which is the aim of the fork). |
I just pulled master to get #806 which hasn't made it into any relase yet, however it appears that CanCan now fails even when doing something like this:
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:
Or is this just me? Rails 3.2.12 + Ruby 1.9.3p327
The text was updated successfully, but these errors were encountered: