Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ActiveRecord::Associations::CollectionProxy shovel method behaves different from rails 6.0.6.1 to 6.1.7.3 #48074

Closed
theobarango opened this issue Apr 26, 2023 · 3 comments

Comments

@theobarango
Copy link

Summary

Unlike in rails 6.0.6.1, ActiveRecord::Associations::CollectionProxy is behaving like a set in rails 6.1.7.3 where it no longer allows concating objects with the same object ID.

Steps to reproduce

class Person < ApplicationRecord
  has_many :friends
end

class Friend < ApplicationRecord
  belongs_to :person
end

p = Person.new # instantiate new Person memory object not persisted to db

p.friends
# => ActiveRecord::Associations::CollectionProxy []

friend = Friend.new # instantiate new Friend memory object not persisted to db

p.friends << friend

puts p.friends.size
# => 1

# add the same friend record again
p.friends << friend

puts p.friends.size
# in rails 6.0.6.1 the result will be 2
# in rails 6.1.7.3 the result is 1

Expected behavior

I expected that after adding the same friend object to the friends collection proxy twice, the size of the collection will increase by 2 as was the case in rails 6.0.6.1, especially as since this change was not documented in the changelog for Rails 6.1 😄

Actual behavior

In rails 6.1.7.3, the size remains at 1. Is ActiveRecord::Associations::CollectionProxy now behaving like a set and enforcing that its elements have unique object IDs?

System configuration

Rails version: 6.1.7.3

Ruby version: 2.7.6

@skipkayhil
Copy link
Member

Hey @theobarango, thanks for opening an issue. Can you explain why you are expecting the previous behavior? (or are you just concerned about the change not being in the changelog?)

Ref #43445

@skipkayhil skipkayhil added the more-information-needed When reporter needs to provide more information label Apr 27, 2023
@theobarango
Copy link
Author

Hi @skipkayhil Hartley, thank you for getting back to me.

Yes my concern is that it was not included in the changelog, causing some of our specs that depended on this behaviour to fail following the upgrade to rails 6.1.7.3.

Looking at the PR I can see the motivation for the behaviour though. FWIW, i'm not asking for the behaviour to be reinstated, just pointing out that it was sort of a 'breaking change' that wasn't documented in the changelog.

@rails-bot rails-bot bot removed the more-information-needed When reporter needs to provide more information label Apr 27, 2023
@skipkayhil
Copy link
Member

I missed it last time I looked but there actually was a CHANGELOG entry for that PR in 6.1.5:

* Fix duplicate active record objects on `inverse_of`.
*Justin Carvalho*

Since the change was actually documented, I'm going to close this as I don't think there's much else to do here.

@skipkayhil skipkayhil closed this as not planned Won't fix, can't repro, duplicate, stale Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants