From fb7c9f7305281bdf2b10309a6ebdc39b42e5f624 Mon Sep 17 00:00:00 2001 From: Ngan Pham Date: Mon, 3 Aug 2026 19:39:55 -0700 Subject: [PATCH] Require an id when exposing a record, not persistence The guard added alongside class/id capture used `persisted?`, which rejects an unsaved instance built solely to carry the id of a real row: expose(address: Addresses::Db::Address.new(id: company_address.id)) Only the class and the id are ever stored, so that is a legitimate way to expose a row under a different model class than the one that created it -- and it worked before the guard existed. A zenpayroll fixture relies on it. Check for a nil id instead. That still catches the case the guard was for, an unsaved record silently resolving to nil in tests, and it no longer rejects a reference that points at a real row. Records looked up through `Repository` use `unscoped`, so an id is the only thing a lookup needs. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PQjkiuuGX2t3TSZKpzqpPv --- docs/reference.md | 12 +++++++++--- lib/fixture_kit/definition.rb | 4 ++-- spec/unit/definition_spec.rb | 21 ++++++++++++--------- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/docs/reference.md b/docs/reference.md index d9eb4fd..fc075a3 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -80,9 +80,15 @@ end - Duplicate exposed names raise `FixtureKit::DuplicateNameError`. - Records are captured as class/id pairs at the moment `expose` is called, not at the end of the definition. The record objects themselves are not retained. -- Exposing a record that is not persisted raises - `FixtureKit::UnpersistedRecordError`. This applies to records inside an - exposed collection as well, and to records that have been destroyed. +- Exposing a record with no id raises `FixtureKit::UnpersistedRecordError`. + This applies to records inside an exposed collection as well. +- Only the class and the id are stored, so an unsaved instance carrying the id + of a real row is a valid way to expose that row under a different model class + than the one that created it: + + ```ruby + expose(address: Addresses::Db::Address.new(id: company_address.id)) + ``` Because capture happens when `expose` is called, expose a record only once it has been saved: diff --git a/lib/fixture_kit/definition.rb b/lib/fixture_kit/definition.rb index e646551..793a74a 100644 --- a/lib/fixture_kit/definition.rb +++ b/lib/fixture_kit/definition.rb @@ -40,9 +40,9 @@ def serialize(name, record) end def reference(name, record) - unless record.persisted? + if record.id.nil? raise FixtureKit::UnpersistedRecordError, - "cannot expose #{name.inspect}: the #{record.class} is not persisted. " \ + "cannot expose #{name.inspect}: the #{record.class} has no id. " \ "Exposed records are captured as class/id pairs when `expose` is called, " \ "so save the record before exposing it." end diff --git a/spec/unit/definition_spec.rb b/spec/unit/definition_spec.rb index 58534d0..3ae2cb8 100644 --- a/spec/unit/definition_spec.rb +++ b/spec/unit/definition_spec.rb @@ -101,17 +101,17 @@ expect(definition.exposed).to eq({ sedan: { Car => sedan.id } }) end - it "raises when the record is not persisted" do + it "raises when the record has no id" do unsaved = User.new(name: "Alice", email: "alice-unsaved@example.com") definition = described_class.new { expose(alice: unsaved) } expect { definition.evaluate(Object.new) }.to raise_error( FixtureKit::UnpersistedRecordError, - /cannot expose :alice: the User is not persisted/ + /cannot expose :alice: the User has no id/ ) end - it "raises when a record inside a collection is not persisted" do + it "raises when a record inside a collection has no id" do saved = User.create!(name: "Alice", email: "alice-mixed@example.com") unsaved = User.new(name: "Bob", email: "bob-mixed@example.com") definition = described_class.new { expose(users: [saved, unsaved]) } @@ -122,13 +122,16 @@ ) end - it "raises when the record has been destroyed" do - destroyed = User.create!(name: "Alice", email: "alice-destroyed@example.com") - destroyed.destroy! - definition = described_class.new { expose(alice: destroyed) } + # Only the class and the id are stored, so an unsaved instance carrying the + # id of a real row is a valid way to expose that row under a different + # model class than the one that created it. + it "accepts an unsaved instance that carries the id of a real row" do + alice = User.create!(name: "Alice", email: "alice-reference@example.com") + definition = described_class.new { expose(alice: User.new(id: alice.id)) } - expect { definition.evaluate(Object.new) } - .to raise_error(FixtureKit::UnpersistedRecordError) + definition.evaluate(Object.new) + + expect(definition.exposed).to eq({ alice: { User => alice.id } }) end it "allows an empty collection" do