activerecord: CollectionProxy#build can take Hash-like object #598
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
|
@tk0miya Thanks for your contribution! Please follow the instructions below for each change. Available commandsYou can use the following commands by commenting on this PR.
|
tk0miya
force-pushed
the
activerecord_support_parameters
branch
from
9915d72
to
d0100ff
Compare
tk0miya
commented
Jun 11, 2024
| def build: (?::Hash[untyped, untyped] attributes) -> untyped | ... | ||
| def create: (?::Hash[untyped, untyped] attributes) -> untyped | ... | ||
| def create!: (?::Hash[untyped, untyped] attributes) -> untyped | ... | ||
| # Collection proxies in Active Record are middlemen between an |
There was a problem hiding this comment.
At first, I moved this definition from activerecord-generated.rbs to here, and merge the above definition into it.
c68dd60
| # | ||
| # person.pets.size # => 5 # size of the collection | ||
| # person.pets.count # => 0 # count from database | ||
| def build: (?_EachPair attributes) ?{ () -> untyped } -> untyped |
There was a problem hiding this comment.
Second, I modified the definition of #build, #create and #create! to take _EachPair.
d0100ff
$ bundle exec rbs --repo gems -r activemodel diff --detail --format diff --type-name 'ActiveRecord::Associations::CollectionProxy' --before gems/activerecord/6.0 --after 598
- [::ActiveRecord::Associations::CollectionProxy public] def build: (?::Hash[untyped, untyped] attributes) -> untyped | (?::Hash[untyped, untyped] attributes) { () -> untyped } -> untyped
+ [::ActiveRecord::Associations::CollectionProxy public] def build: (?::ActiveRecord::Associations::CollectionProxy::_EachPair attributes) ?{ () -> untyped } -> untyped
- [::ActiveRecord::Associations::CollectionProxy public] def create: (?::Hash[untyped, untyped] attributes) -> untyped | (?::Hash[untyped, untyped] attributes) { () -> untyped } -> untyped
+ [::ActiveRecord::Associations::CollectionProxy public] def create: (?::ActiveRecord::Associations::CollectionProxy::_EachPair attributes) ?{ () -> untyped } -> untyped
- [::ActiveRecord::Associations::CollectionProxy public] def create!: (?::Hash[untyped, untyped] attributes) -> untyped | (?::Hash[untyped, untyped] attributes) { () -> untyped } -> untyped
+ [::ActiveRecord::Associations::CollectionProxy public] def create!: (?::ActiveRecord::Associations::CollectionProxy::_EachPair attributes) ?{ () -> untyped } -> untyped |
ksss
reviewed
Jun 12, 2024
| # instantiation of the actual post records. | ||
| class CollectionProxy < Relation | ||
| interface _EachPair | ||
| def each_pair: () { ([untyped, untyped]) -> untyped } -> self |
There was a problem hiding this comment.
Is #empty? not necessary?
And #each ?
There was a problem hiding this comment.
I'm not sure what is the best way for such an interface. But no reason to ignore them. Okay, I'll add them soon.
There was a problem hiding this comment.
Done. I merged #603 into this PR to support ActionController::Parameters.
tk0miya
force-pushed
the
activerecord_support_parameters
branch
from
be96959
to
4253a73
Compare
tk0miya
commented
Jun 13, 2024
The definitions for `ActiveRecord::Associations::CollectionProxy` are placed both `activerecord.rbs` and `activerecord-generated.rbs`. Before change them, this merges them into the single definition.
The current definition of `#build`, `#craete` and `#create!` methods of `ActiveRecord::Associations::CollectionProxy#build` take a Hash object. But they can also take Hash-like object. Usually, Rails apps pass an instance of `ActionController::Parameters`. The technique is well known as "Mass Assignment". Internally, they expects the object should have `#each_pair` method. Therefore, this changes the signature of these methods to take a Hash-like object instead of Hash. refs: https://github.com/rails/rails/blob/v7.0.0/activemodel/lib/active_model/attribute_assignment.rb#L29
tk0miya
force-pushed
the
activerecord_support_parameters
branch
from
4253a73
to
b0992ee
Compare
|
/merge |
rhiroe
added a commit
to rhiroe/gem_rbs_collection
that referenced
this pull request
Jul 29, 2024
- Symlink of _test/metadata.yaml in 6.0 to 6.1 - Annotated `User#articles` definition using `@dynamic` and removed _test/steep_expectations.yml These files are added in ruby#598
github-actions bot
pushed a commit
that referenced
this pull request
Aug 2, 2024
- Symlink of _test/metadata.yaml in 6.0 to 6.1 - Annotated `User#articles` definition using `@dynamic` and removed _test/steep_expectations.yml These files are added in #598
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.
The current definition of
#build,#craeteand#create!methods ofActiveRecord::Associations::CollectionProxy#buildtake a Hash object.But they can also take Hash-like object. Usually, Rails apps pass an
instance of
ActionController::Parameters. The technique is well knownas "Mass Assignment".
Internally, they expects the object should have
#each_pairmethod.Therefore, this changes the signature of these methods to take a
Hash-like object instead of Hash.
refs: https://github.com/rails/rails/blob/v7.0.0/activemodel/lib/active_model/attribute_assignment.rb#L29