rom-factory icon indicating copy to clipboard operation
rom-factory copied to clipboard

overriding factory attributes is causing related field callbacks to fail

Open lowang opened this issue 3 years ago • 5 comments

Describe the bug

Overriding attributes is causing factory field resolver to receive nil instead of overwritten value.

To Reproduce

create file app_spec.rb with:

require 'rom'
require 'rom-factory'

ROM_CONTAINER = ROM.container(:sql, 'sqlite::memory') do |conf|
  conf.default.create_table(:users) do
    primary_key :id
    column :name, String, null: false
    column :email, String, null: false
  end

  class Users < ROM::Relation[:sql]
    schema(infer: true)
  end

  conf.register_relation(Users)
end

Factory = ROM::Factory.configure do |config|
  config.rom = ROM_CONTAINER
end

Factory.define(:user) do |f|
  f.name 'John'
  f.email { |name| name.downcase + '@example.com' }
end

RSpec.describe 'app' do
  let(:data) do
    { name: 'Alice' }
  end

  let(:user) { Factory[:user, data] }

  specify do
    pp user
  end
end

now rspec app_spec.rb is producing following error:

 NoMethodError:
       undefined method `downcase' for nil:NilClass
     # ./app_spec.rb:24:in `block (2 levels) in <top (required)>'
     # ./app_spec.rb:32:in `block (2 levels) in <top (required)>'
     # ./app_spec.rb:35:in `block (2 levels) in <top (required)>'

Expected behavior

I was expecting email callback to receive Alice value.

My environment

  • Affects my production application: YES/NO
  • Ruby version: 2.6.5
  • OS: 10.15.5

lowang avatar Dec 28 '20 11:12 lowang

Thanks for reporting this, clearly a bug. I think we should re-think this API in general. Yielding attribute values via block args can be problematic when you have attributes that match ruby keywords.

solnic avatar Dec 30 '20 06:12 solnic

I still think having a convention for reserved words like do |then| -> do |then_| would be good enough

flash-gordon avatar Dec 30 '20 08:12 flash-gordon

@flash-gordon yeah...seems like a pragmatic choice. I wonder though....maybe we could provide a struct with values that are lazy, this way you could still rely on more than one value but access it through one object.

solnic avatar Dec 30 '20 09:12 solnic

@solnic yeah I think it'd be possible too

flash-gordon avatar Dec 30 '20 11:12 flash-gordon

This makes ROM factory somewhat unusable

yld avatar Apr 29 '24 08:04 yld