ruby / gem_rbs_collection

A collection of RBS for gems.
MIT License
246 stars 101 forks source link

activerecord: CollectionProxy#build can take Hash-like object #598

Closed tk0miya closed 1 week ago

tk0miya commented 2 weeks ago

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

github-actions[bot] commented 2 weeks ago

@tk0miya Thanks for your contribution!

Please follow the instructions below for each change. See also: https://github.com/ruby/gem_rbs_collection/blob/main/docs/CONTRIBUTING.md

Available commands

You can use the following commands by commenting on this PR.


activerecord

You changed RBS files for an existing gem. You need to get approval from the reviewers of this gem.

@hibariya, @ksss, @Little-Rubyist, please review this pull request. If this change is acceptable, please make a review comment including APPROVE from here. Screen Shot 2024-03-19 at 14 13 36

After that, the PR author or the reviewers can merge this PR. Just comment /merge to merge this PR.

ksss commented 2 weeks ago
$ 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
github-actions[bot] commented 1 week ago

Thanks for your review, @ksss!

@tk0miya, @ksss This PR is ready to be merged. Just comment /merge to merge this PR.

tk0miya commented 1 week ago

/merge