Avoid stubbing id for records without primary key

Fixes #1305

Before Rails 6 when the build_stubbed strategy assigned an id on a
record without a primary key column, the `id=` method would no-op, and
the record would end up looking like a new_record (i.e. `new_record?`
would return true because the id was nil).

This problem surfaced because of a [change in Rails 6][rails], which
caused `id=` to raise a potentially confusing
`ActiveModel::MissingAttributeError: can't write unknown attribute ''`
for records without a primary key column.

Since build_stubbed stubbed was calling `id=` for all instances,
regardless of whether they had primary keys, it was raising the above
error.

To avoid this error, we check whether the instance has a primary_key
defined before setting the id.

We also changed `persisted?` and `new_record?` to be less dependent on
the id. That way those methods will work as expected for records without
primary keys.

[rails]: b6828fc915

Co-authored-by: Jesse Bailey <jbailey117@gmail.com>
This commit is contained in:
Daniel Colson 2019-07-12 13:32:50 -04:00
parent 87d1b42ff5
commit 5a3247dd8c
3 changed files with 58 additions and 6 deletions

View File

@ -44,15 +44,17 @@ module FactoryBot
end
def stub_database_interaction_on_result(result_instance)
result_instance.id ||= next_id
if has_settable_id?(result_instance)
result_instance.id ||= next_id
end
result_instance.instance_eval do
def persisted?
!new_record?
true
end
def new_record?
id.nil?
false
end
def destroyed?
@ -68,6 +70,11 @@ module FactoryBot
end
end
def has_settable_id?(result_instance)
!result_instance.class.respond_to?(:primary_key) ||
result_instance.class.primary_key
end
def clear_changes_information(result_instance)
if result_instance.respond_to?(:clear_changes_information)
result_instance.clear_changes_information

View File

@ -58,3 +58,44 @@ describe "a stubbed instance overriding strategy" do
expect(subject.user).not_to be_new_record
end
end
describe "a stubbed instance with no primary key" do
it "builds a stubbed instance" do
using_model_without_pk do
FactoryBot.define do
factory :model_without_pk
end
model = FactoryBot.build_stubbed(:model_without_pk)
expect(model).to be_truthy
end
end
it "behaves like a persisted record" do
using_model_without_pk do
FactoryBot.define do
factory :model_without_pk
end
model = FactoryBot.build_stubbed(:model_without_pk)
expect(model).to be_persisted
expect(model).not_to be_new_record
end
end
def using_model_without_pk
define_class("ModelWithoutPk", ActiveRecord::Base)
connection = ActiveRecord::Base.connection
begin
clear_generated_table("model_without_pks")
connection.create_table("model_without_pks", id: false) do |t|
t.column :updated_at, :datetime
end
yield
ensure
clear_generated_table("model_without_pks")
end
end
end

View File

@ -33,13 +33,17 @@ module DefineConstantMacros
def clear_generated_tables
created_tables.each do |table_name|
ActiveRecord::Base.
connection.
execute("DROP TABLE IF EXISTS #{table_name}")
clear_generated_table(table_name)
end
created_tables.clear
end
def clear_generated_table(table_name)
ActiveRecord::Base.
connection.
execute("DROP TABLE IF EXISTS #{table_name}")
end
private
def created_tables