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 dependent: :destroy requires id on the associated object, raises NoMethodError: undefined method `to_sym' for nil:NilClass #31022

Closed
spencerroan opened this issue Oct 31, 2017 · 5 comments

Comments

@spencerroan
Copy link

Steps to reproduce

# frozen_string_literal: true

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"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  gem "rails", github: "rails/rails"
  gem "arel", github: "rails/arel"
  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(:doctors) {}

  create_table :appointments, id: false do |t|
    t.integer :doctor_id, null: false
  end
  create_table :tools, id: false do |t|
    t.integer :doctor_id, null: false
  end
end

class Doctor < ActiveRecord::Base
  has_many :appointments, dependent: :destroy
  has_many :tools, dependent: :delete_all
end

class Appointment < ActiveRecord::Base
end

class Tool < ActiveRecord::Base
end

class BugTest < Minitest::Test
  # dependent: destroy requires the association to have :id
  def test_destroy
    d = Doctor.create
    d.appointments << Appointment.new()
    assert d.destroy
  end

  # workaround 1: use delete_all and skip yer callbacks
  def test_delete_all
    d = Doctor.create
    d.tools << Tool.new()
    assert d.destroy
  end
  # workaround 2: give the association an id.
end

Expected behavior

The resource should be destroyed without raising an error.

Actual behavior

NoMethodError: undefined method `to_sym' for nil:NilClass

System configuration

Rails version:
ruby 2.4.2p198 (2017-09-14 revision 59899) [x86_64-linux]
Ruby version:
Rails 5.0.6

@sergiivorobei
Copy link

sergiivorobei commented Nov 3, 2017

Seems this is a duplicate of #25437 issue.
Please view the comments.

Briefly: when you use dependent: :destroy each dependent model is going to be instantiated, and rails can't operate on them if they do not have the primary key, that is why it fails. In case of dependent: :delete_all rails are not going to initialize any dependent models so everything works fine.
It is expected behavior for associations. You should either add primary key (id or another field) or use dependent: :delete_all.

@al2o3cr
Copy link
Contributor

al2o3cr commented Nov 3, 2017

+1 to @kennyx46 - dependent: :destroy is going to try to remove records by loading them and then calling destroy on each one.

That process - like most ActiveRecord processes - expects models to have a primary key (either id or something else, set by doing self.primary_key = ... in the model). I suspect you'd get the exact same to_sym error with code like this:

  tool = Tool.create
  tool.destroy

@kamipo
Copy link
Member

kamipo commented Nov 4, 2017

It is expected behavior. See #25347 (comment)

@kamipo kamipo closed this as completed Nov 4, 2017
@spencerroan
Copy link
Author

Thank you for the update.

@bonafernando
Copy link

I had the same issue. Used to have a table like this:

CREATE TABLE user_country (
  id_user INTEGER(12) NOT NULL,
  id_country VARCHAR(15) NOT NULL,
  PRIMARY KEY (id_user,id_country),
  FOREIGN KEY (id_user) REFERENCES user(id),
  FOREIGN KEY (id_country) REFERENCES country(id)
);

With it, Active Record was always sending me warnings:

WARNING: Active Record does not support composite primary key.

And fixed creating a primary key for it:

CREATE TABLE user_country (
  id INTEGER(12) NOT NULL AUTO_INCREMENT,
  id_user INTEGER(12) NOT NULL,
  id_country VARCHAR(15) NOT NULL,
  PRIMARY KEY (id),
  FOREIGN KEY (id_user) REFERENCES user(id),
  FOREIGN KEY (id_country) REFERENCES country(id)
);

Not my favorite solution, but fixed...

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

No branches or pull requests

5 participants