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

has_and_belongs_to_many autosave: true, dont save child records #13923

Closed
maurogeorge opened this issue Feb 2, 2014 · 9 comments
Closed

has_and_belongs_to_many autosave: true, dont save child records #13923

maurogeorge opened this issue Feb 2, 2014 · 9 comments
Assignees
Milestone

Comments

@maurogeorge
Copy link
Contributor

Guys,

I was working on shoulda-matchers to make his works with the rails 4.1.0.beta1. I guess the has_and_belongs_to_many with autosave: true is broken.

I show some code. First the models

class Person < ActiveRecord::Base
  has_and_belongs_to_many :relatives, :autosave => true
end

class Relative < ActiveRecord::Base
end

class PeopleRelative  < ActiveRecord::Base
end

In rails 3.2:

> p = Person.create!
=> #<Person id: 1, name: nil, created_at: "2014-02-02 14:01:07", updated_at: "2014-02-02 14:01:07">
> r = Relative.create!
=> #<Relative id: 1, adopted: nil, created_at: "2014-02-02 14:01:07", updated_at: "2014-02-02 14:01:07">
> p.relatives << r
=> [#<Relative id: 1, adopted: nil, created_at: "2014-02-02 14:01:07", updated_at: "2014-02-02 14:01:07">]
> p.name = 'John Doe'
=> "John Doe"
> r.adopted = false
=> false
> p.save!
=> true
> p.reload
=> #<Person id: 1, name: "John Doe", created_at: "2014-02-02 14:01:07", updated_at: "2014-02-02 14:01:07">
> r.reload
=> #<Relative id: 1, adopted: false, created_at: "2014-02-02 14:01:07", updated_at: "2014-02-02 14:01:07">

Works as expected on rails 3.2, when I save Person, its saves the Relative, the adopted change from nil to false.

In Rails 4.1.0.beta1

> p = Person.create!
=> #<Person id: 1, name: nil, created_at: "2014-02-02 14:02:04", updated_at: "2014-02-02 14:02:04">
> r = Relative.create!
=> #<Relative id: 1, adopted: nil, created_at: "2014-02-02 14:02:04", updated_at: "2014-02-02 14:02:04">
> p.relatives << r
=> [#<Relative id: 1, adopted: nil, created_at: "2014-02-02 14:02:04", updated_at: "2014-02-02 14:02:04">]
> p.name = 'John Doe'
=> "John Doe"
> r.adopted = false
=> false
> p.save!
=> true
> p.reload
=> #<Person id: 1, name: "John Doe", created_at: "2014-02-02 14:02:04", updated_at: "2014-02-02 14:02:04">
> r.reload
=> #<Relative id: 1, adopted: nil, created_at: "2014-02-02 14:02:04", updated_at: "2014-02-02 14:02:04">

On rails 4.1.0.beta1, when I save Person, its dont save the Relative, the adopted keeps nil.

@rafaelfranca
Copy link
Member

@maurogeorge
Copy link
Contributor Author

@rafaelfranca here https://gist.github.com/maurogeorge/8776223. Got the same issue

@senny
Copy link
Member

senny commented Feb 3, 2014

I can confirm this is a regression from 4-0-stable. It's caused by the fact that now habtm associations are implemented in terms of hm:t. The :autosave option is lost in that translation. I'll take a look and write a patch.

@senny senny self-assigned this Feb 3, 2014
@maurogeorge
Copy link
Contributor Author

Great! Thanks guys.

@senny senny closed this as completed in 584a464 Feb 3, 2014
@senny
Copy link
Member

senny commented Feb 3, 2014

The error was in fact the refactoring to use hm:t for habtm associations. Our tests didn't catch the regression because the relevant test-case used an association that was using accepts_nested_attributes_for. That macro enables the :autosave flag and kept working after the refactoring 😓

I added another test-case with a raw association to prevent against further regressions.

@maurogeorge thanks for reporting 💛

@girishso
Copy link
Contributor

girishso commented Feb 3, 2014

@senny you beat me by a couple of hours... this was supposed to be my first rails commit! Although your solution is more elegant than mine! :)

@senny
Copy link
Member

senny commented Feb 3, 2014

@girishso sorry to hear, please respond on tickets you are planning to work on. Also share your investigation results so that the invested time can be used to move the issues forward.

@maurogeorge
Copy link
Contributor Author

@senny thanks a lot 💚

@girishso
Copy link
Contributor

girishso commented Feb 3, 2014

Thank you @senny will keep that in mind!

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