Support using a nil relation as a condition #653
Conversation
…elation returns an empty array. The use case is I want to only allow deleting a record if it has no associated records for a given association.
In ActiveRecord you can do this with syntax like this:
> Article.joins(:comments).where(comments: { id: nil })
This generates the SQL:
> SELECT "articles".* FROM "articles" INNER JOIN "comments" ON "comments"."article_id" = "articles"."id" WHERE "comments"."id" IS NULL
Which returns an empty array.
This PR makes cancan support the same style of query. So you can do:
> @ability.can :destroy, Article, comments: { id: nil }
This will allow you to `:destroy` an `Article` that has no comments, but not an article that has comments.
| expect(Article.accessible_by(@ability)).to eq([]) | ||
|
|
||
| if CanCan::ModelAdapters::ActiveRecordAdapter.version_greater_or_equal?('5.0.0') | ||
| expect(@ability.model_adapter(Article, :read)).to generate_sql(%( |
There was a problem hiding this comment.
Haha, good question. I tried to avoid adding too much logic specific to this scenario, and mostly rely on how the Rails querying interface already works. In this case, we are generating a query that can never be true, but Rails hasn't necessarily understood that or optimised it out.
There was a problem hiding this comment.
I suppose I could try and refactor it to return Article.none in this case, but it feels like it could add a fair bit more maintenance load.
| a2 = Article.create! | ||
| a2.comments = [Comment.create!] | ||
|
|
||
| @ability.can :read, Article, comments: { id: nil } |
There was a problem hiding this comment.
would can :read, Article, comments: nil work as well? I'd expect so from 3.2.0.
And what about can :read, Article, comments: []? Maybe I'd expect this to work as well as can :read, Article, comments: { id: [] }
There was a problem hiding this comment.
Not currently. The cancan conditions need to match conditions you'd pass into AR#where. Both of your examples wouldn't work as regular queries.
(byebug) Article.left_outer_joins(:comments).where(comments: nil)
#<ActiveRecord::Relation []>
(byebug) Article.left_outer_joins(:comments).where(comments: nil).to_sql
"SELECT \"articles\".* FROM \"articles\" LEFT OUTER JOIN \"comments\" ON \"comments\".\"article_id\" = \"articles\".\"id\" WHERE \"articles\".\"id\" IS NULL" # note how it looks for article.ids IS NULL, not comments.id(byebug) Article.left_outer_joins(:comments).where(comments: [])
#<ActiveRecord::Relation []>
(byebug) Article.left_outer_joins(:comments).where(comments: []).to_sql
"SELECT \"articles\".* FROM \"articles\" LEFT OUTER JOIN \"comments\" ON \"comments\".\"article_id\" = \"articles\".\"id\" WHERE 1=0"Whereas with my example:
(byebug) Article.left_outer_joins(:comments).where(comments: { id: nil })
#<ActiveRecord::Relation [#<Article id: 1, name: nil, created_at: "2020-12-14 00:50:15", updated_at: "2020-12-14 00:50:15", published: nil, secret: nil, priority: nil, category_id: nil, project_id: nil, user_id: nil>]>
(byebug) Article.left_outer_joins(:comments).where(comments: { id: nil }).to_sql
"SELECT \"articles\".* FROM \"articles\" LEFT OUTER JOIN \"comments\" ON \"comments\".\"article_id\" = \"articles\".\"id\" WHERE \"comments\".\"id\" IS NULL"There was a problem hiding this comment.
I'm open to trying to support more use cases, but I think it'd add a bit more complexity as we'd need to translate specific values (nils, [], etc) into the corresponding AR query statements.
|
@coorasse anything more you need from me to get this merged? |
|
I believe this is ready to be merged. Could you squash and rebase, please? |
|
I'm just gonna run our CI suite against this now that a few changes upstream have been merged in. |
|
Sure! Just let me know, but I believe is good to go 👍 |
coorasse
added
next minor release
waiting for feedback
and removed
waiting for feedback
labels
|
@coorasse green build. Go for it. |
I want to define an ability check that only applies if a
has_manyrelation returns an empty array. The use case is I want to only allow deleting a record if it has no associated records for a given association.In ActiveRecord you can do this with syntax like this:
This generates the SQL:
Which returns an empty array.
This PR makes cancan support the same style of query. So you can do:
This will allow you to
:destroyanArticlethat has no comments, but not an article that has comments.