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

Can no longer validate presence of an association after upgrade to Rails 4.1 #16684

Closed
taryneast opened this issue Aug 25, 2014 · 5 comments
Closed

Comments

@taryneast
Copy link
Contributor

class MyNotification
   belongs_to :my_other_class, :autosave => true
   validates :recipient, presence: true
   delegate :recipient, :recipient=, to: :my_other_class
   before_create :build_my_class_if_missing

   def build_my_class_if_missing
     my_other_class ||= build_my_other_class(message_type: "MyMessageType")
   end
end

describe "creation" do
  subject(:notification) { MyNotification.create!(recipient: user, request: my_request) }

  its(:recipient) { should == user }
  its(:request) { should == the_request }
end

The create! fails with Validation failed: Recipient can't be blank even though I'm clearly passing a recipient.

This worked in Rails 4.0.
I can see nothing in the upgrade notes or the release notes for 4.1 about what changed to make this stop working.

What changed?
Why did it change?
What can I do to make it work again?

@senny
Copy link
Member

senny commented Aug 25, 2014

@taryneast thank you for your report. Can you attach an executable test-case to reproduce the error? You can use this script as a starting point.

@m5rk
Copy link

m5rk commented Aug 26, 2014

We attempted to reproduce this issue for Rails 4.0.3, but this gist fails:

https://gist.github.com/m5rk/415570a52fb03f5ff041

@taryneast
Copy link
Contributor Author

cool, thanks guys, I'll have a go at making a new testable set based on your examples. I've got it running in Rails 4.0 - just need to make sure it breaks (the same way) in Rails 4.1

@taryneast
Copy link
Contributor Author

Ok, here's the gist. It consistently passes on Rails 4.0 and fails on 4.1.
As you can see, a little more complicated than my original example. We're using a concern for a bunch of this stuff... if you directly copy that stuff into the ItemRequestNotification class it all works, but put it back in the included concern and it fails.

https://gist.github.com/taryneast/957d13ec70f65b262e78

@senny
Copy link
Member

senny commented Sep 9, 2014

I pushed a fix (will backport it to 4-1-stable as well). The bug was not related to the validations but to the notification method. With Rails 4.0 it did overwrite the CollectionProxy but with Rails 4.1 it didn't. This was due to a bug in Active Records module inclusion order.

@taryneast thank you for reporting.

senny added a commit that referenced this issue Sep 9, 2014
Closes #16684.

This is achieved by always generating `GeneratedAssociationMethods` when
`ActiveRecord::Base` is subclassed. When some of the included modules
of `ActiveRecord::Base` were reordered this behavior was broken as
`Core#initialize_generated_modules` was no longer called. Meaning that
the module was generated on first access.

Conflicts:
	activerecord/test/cases/attribute_methods/read_test.rb
trungpham pushed a commit to trungpham/rails that referenced this issue Sep 18, 2014
Closes rails#16684.

This is achieved by always generating `GeneratedAssociationMethods` when
`ActiveRecord::Base` is subclassed. When some of the included modules
of `ActiveRecord::Base` were reordered this behavior was broken as
`Core#initialize_generated_modules` was no longer called. Meaning that
the module was generated on first access.
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

4 participants