Use method overloading to improve WhereChain definition #2070
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I tested this against our core monolith and it worked fine. |
egiurleo
approved these changes
Nov 8, 2024
`ActiveRecord::QueryMethods#where` returns a `WhereChain` instance only when it is called with no arguments. When called with arguments, it returns the same `ActiveRecord::Relation` instance. Previously, we didn't have proper overloading support in RBI files, so the best thing we could do was act like `WhereChain` was a subclass of the related relation class. This was not ideal because it would allow users to call methods on `WhereChain` that were not actually defined on it. For example, one could do `User.all.where(name: "John").not` which is not valid. Now that we have support for overloading, we can define `where` to return a `WhereChain` instance only when called with no arguments, and in all other cases, we can say that it returns the same relation instance.
egiurleo
force-pushed
the
uk-improve-where-chain
branch
from
702133f to
519d65a
Compare
|
Force pushed to get CI fixes :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
ActiveRecord::QueryMethods#wherereturns aActiveRecord::QueryMethods::WhereChaininstance only when it is called with no arguments. When called with arguments, it returns the sameActiveRecord::Relationinstance.Previously, we didn't have proper overloading support in RBI files, so the best thing we could do was act like
ActiveRecord::QueryMethods::WhereChainwas a subclass of the related relation class. This was not ideal because it would allow users to call methods onWhereChainthat were not actually defined on it. For example, one could doUser.all.where(name: "John").notwhich is not valid.Implementation
Now that we have support for overloading, we can define
whereto return aWhereChaininstance only when called with no arguments, and in all other cases, we can say that it returns the same relation instance.Tests
Updated existing tests.