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

Duplicate insert on autosave for specific HABTM-has_one combo #24032

Closed
nbekirov opened this issue Mar 3, 2016 · 8 comments
Closed

Duplicate insert on autosave for specific HABTM-has_one combo #24032

nbekirov opened this issue Mar 3, 2016 · 8 comments

Comments

@nbekirov
Copy link

nbekirov commented Mar 3, 2016

Steps to reproduce

class Organisation < ActiveRecord::Base
  has_one :user
end

class User < ActiveRecord::Base
  has_and_belongs_to_many :roles
  belongs_to :organisation
end

class Role < ActiveRecord::Base
  has_and_belongs_to_many :users
end

user = User.new
user.organisation = Organisation.new
user.roles << Role.new
user.save!

Here is my Active Record Executable Test Case

This issue is related to Rolify: Duplicate Roles #228

Some observations:

  • If I change the order of has_and_belongs_to_many and belongs_to in User there is no issue
  • If I add autosave: false to has_and_belongs_to_many in User there is no issue

Here is an issue I've found that seems related but I don't think is quite the same: has_and_belongs_to_many autosaves duplicate records

Expected behavior

One User with one Organisation and one Role saved

INSERT INTO "organisations" DEFAULT VALUES
INSERT INTO "users" ("organisation_id") VALUES (?)  [["organisation_id", 1]]
INSERT INTO "roles" DEFAULT VALUES
INSERT INTO "roles_users" ("user_id", "role_id") VALUES (?, ?)  [["user_id", 1], ["role_id", 1]]

Actual behavior

One User with one Organisation and two Role-s saved

INSERT INTO "organisations" DEFAULT VALUES
INSERT INTO "users" ("organisation_id") VALUES (?)  [["organisation_id", 1]]
INSERT INTO "roles" DEFAULT VALUES
INSERT INTO "roles_users" ("user_id", "role_id") VALUES (?, ?)  [["user_id", 1], ["role_id", 1]]
INSERT INTO "roles_users" ("user_id", "role_id") VALUES (?, ?)  [["user_id", 1], ["role_id", 1]]

System configuration

Rails version:
N/A

Ruby version:

$ ruby -v
ruby 2.3.0p0 (2015-12-25 revision 53290) [x86_64-linux-gnu]
@meinac
Copy link
Contributor

meinac commented Mar 5, 2016

Also If you change has_one :user to has_many :users or ?has_one :users? it works as expected, interesting 😕

@betesh
Copy link
Contributor

betesh commented Mar 27, 2017

Introduced in c7adc61

@rafaelfranca rafaelfranca added this to the 5.1.0 milestone Mar 27, 2017
@meinac
Copy link
Contributor

meinac commented Mar 28, 2017

@rafaelfranca I see that this has been added into 5.1 milestone, do you want to go with the existing pr or do you think something different.

@rafaelfranca
Copy link
Member

Oh, I thought it was caused by a commit only present in 5.1, but seems this bug is also present in 4.2. I'm removing the milestone and I'll leave the PR review with @sgrif

@rafaelfranca rafaelfranca removed this from the 5.1.0 milestone Mar 28, 2017
@betesh
Copy link
Contributor

betesh commented Mar 28, 2017

There's definitely a regression from 5.0.2 to 5.1.0.beta1. Is @nbekirov's test case is failing on 4.2? If so, I'll try to put together another test case that demonstrates the issue.

Basically, what I'm seeing is that when upgrading from 5.0.2 to 5.1.0.beta1, I get ActiveRecord::StatementInvalid from a has_and_belongs_to_many with a unique index on the join table. autosave: false fixes it, as does moving the has_and_belongs_to_many to after all the belongs_to's.

@rafaelfranca
Copy link
Member

@betesh
Copy link
Contributor

betesh commented Mar 28, 2017

Here's the regression, and this is what I was referring to in my comment above about it being introduced in c7adc61:

begin
  require 'bundler/inline'
rescue LoadError => e
  $stderr.puts 'Bundler version 1.10 or later is required. Please update your Bundler'
  raise e
end

gemfile(true) do
  source 'https://rubygems.org'
  # Activate the gem you are reporting the issue against.
  # gem 'activerecord', '5.1.0.beta1' # FAILS with this uncommented
  gem 'activerecord', '5.0.2' # PASSES with this uncommented
  gem 'sqlite3'
end

require 'active_record'
require 'minitest/autorun'
require 'logger'

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(adapter: 'sqlite3', database: ':memory:')
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  create_table :organisations, force: true do |t|
  end

  create_table :users, force: true do |t|
    t.references :organisation, index: true
  end

  create_table :roles, force: true do |t|
  end

  create_join_table :roles, :users, force: true do |t|
    t.index [:user_id, :role_id], unique: true
  end
end

class Organisation < ActiveRecord::Base
  has_one :user
end

class User < ActiveRecord::Base
  has_and_belongs_to_many :roles
  belongs_to :organisation
  accepts_nested_attributes_for :organisation
end

class Role < ActiveRecord::Base
  has_and_belongs_to_many :users
end

class BugTest < ActiveSupport::TestCase
  def test_association
    assert_nothing_raised do
      User.create! organisation: Organisation.new, roles: [Role.new]
    end
  end
end

I do remember reading a bug report about using accepts_nested_attributes for a has_one relationship, but I don't remember the details and could not find it when I looked around just now.

@rafaelfranca rafaelfranca added this to the 5.1.0 milestone Mar 28, 2017
betesh added a commit to betesh/rails that referenced this issue Mar 29, 2017
@betesh
Copy link
Contributor

betesh commented Mar 29, 2017

PR #28596 is ready

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants