Expose an invert_where method that will invert all scope conditions.#40249
Expose an invert_where method that will invert all scope conditions.#40249rafaelfranca merged 1 commit intorails:masterfrom kddnewton:invert-where
invert_where method that will invert all scope conditions.#40249Conversation
|
I really like this idea. Few notes after initial thinking:
|
|
@simi yeah those all make sense. The inspiration for this was based on Do you mean a non-mutating alternative as in something that returns a new relation? I think I'd rather only expose new API. I'd be fine with it if it were a non-mutating one though that spawned a new relation with the inverted where. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. |
There was a problem hiding this comment.
| self.where_clause = where_clause.invert | |
| self | |
| scope = spawn | |
| scope.where_clause = where_clause.invert | |
| scope |
We don't change relation inline for public API methods in Relation.
There was a problem hiding this comment.
Sure! I'll fix that.
While I'm here, would you prefer invert_where or where.invert? I saw where.missing and where.associated made it in, so maybe that would fit better.
There was a problem hiding this comment.
Since it will not take any argument I'd prefer invert_where
There was a problem hiding this comment.
Updated! I used the existing examples to do an invert_where that does spawn.invert_where! and an invert_where! that modifies in place.
There was a problem hiding this comment.
Since it will not take any argument I'd prefer
invert_where
That would also match rewhere. It's closer to manipulating the where clause itself than doing extra operations or expressions on it, like where.not etc.
|
Can you rebase the PR? It has conflicts. |
```ruby
class User
scope :active, -> { where(accepted: true, locked: false) }
end
User.active
User.active.invert_where
```
|
@rafaelfranca rebased |
|
Wouldn't |
|
This method seem to be an incredibly dangerous footgun. What if the where clause had a current account or user scope on it that the developer wasn't immediately aware of? This would invert that scope and show all records not associated with that user, potentially leaking cross-tenant records. I don't really think this is a wise change. Imagine, somewhere within the request flow, perhaps automatically scoped by Pundit, current_user = User.first
posts = Post.where(author: current_user)And then later on in a controller… published_posts = posts.where(published: true)
draft_posts = published_posts.invert_whereThe I think this could be resolved by explicitly taking in the clauses to invert, rather than inverting everything for the current scope. Regardless, I feel like |
|
In addition to the scope issue Zeke proves above, there is another pretty substantial issue. Reversing An example:
This is a famous issue with the incomparability of NULL values, and it is the first example that came to my mind when I saw this feature in the Rails Weekly mailing list. Of course, this is no different than if one just did |
Came up at work that there was a complicated scope that someone was trying to invert and couldn't think of a clean way without touching internals. Was happy to see there is already an invert method present since it's used for
where.not. This just exposes that method.