Add hoc sequences in create_list

[mohammednasser-32]

Fixes https://github.com/thoughtbot/factory_bot/issues/1647

Allow creation of sequence attributes in create_list

This PR is more of a proof of concept, if I get the green light I can add documentation and more tests if needed.

It is working and all tests pass, however I am not confident about the code structure..in this approach FactoryBot.build_sequence is just a syntactical sugar for a Proc, however it will work fine if we pass a Proc too. Is it better to have a dedicated class for this instead?

[mike-burns]

That was fast!

Is it better to have a dedicated class for this instead?

There's a backward compatibility issue to figure out. Imagine this:

class UserWithCallback
  def initialize(name, dob, on_save)
    @name = name
    @dob = dob
    @on_save = on_save
  end

  def save
    user = User.create!(name: @name, dob: @dob)
    @on_save.call(user)
  end
end

FactoryBot.define do
  factory :user_with_callback do
    name { "Zero Cool" }
    dob { "1988-08-10" }
    on_save do
      proc { |user| Logger.info("created user #{user}") }
    end
  end
end

This is, currently, a valid factory_bot definition(*) for a valid Ruby class. Good code? Probably not. Possible? For sure.

So we need to make sure this still works even if we add new functionality.

For that reason, I think we might need a new class.

(* I haven't tried it.)

[mohammednasser-32]

Yes right didn't consider that, thanks for raising it! here is another approach where I created FactoryBot::AttributeSequence class which encapsulates the proc so we can differentiate between a normal proc and an attribute sequence one

[mohammednasser-32]

Hello @mike-burns! I can see you are no longer maintaining this repository, Can you please tell me who should I follow up regarding this (and the other PR) (I am really sorry for being obtrusive, I am just keen to contribute here :sweat_smile: )

[mike-burns]

You were quick to notice!

We're still collecting maintainers, but these people will be in charge: https://github.com/thoughtbot/factory_bot/pull/1651/files

May 23, 2024 08:18:44 Mohammed Nasser @.***>:

Hello @mike-burns[https://github.com/mike-burns]! I can see you are no longer maintaining this repository, Can you please tell me who should I follow up regarding this (and the other PR[https://github.com/thoughtbot/factory_bot/pull/1639]) (I am really sorry for being obtrusive, I am just keen to contribute here 😅 )

— Reply to this email directly, view it on GitHub[https://github.com/thoughtbot/factory_bot/pull/1650#issuecomment-2127404601], or unsubscribe[https://github.com/notifications/unsubscribe-auth/AAABDRVLAZWMLS5EUSFS72LZDYCFFAVCNFSM6AAAAABHS4SI6CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCMRXGQYDINRQGE]. You are receiving this because you were mentioned. [Tracking image][https://github.com/notifications/beacon/AAABDRTT6EKSRSGVAD6S3ETZDYCFFA5CNFSM6AAAAABHS4SI6CWGG33NNVSW45C7OR4XAZNMJFZXG5LFINXW23LFNZ2KUY3PNVWWK3TUL5UWJTT6ZWPDS.gif]

[smaboshe]

Hello @mike-burns! I can see you are no longer maintaining this repository, Can you please tell me who should I follow up regarding this (and the other PR) (I am really sorry for being obtrusive, I am just keen to contribute here 😅 )

We're steadily getting up to speed 😅. Thanks in advance for your patience, @mohammednasser-32.

[smaboshe]

What do you think about adding an additional test for the default behaviour. Something like:

context "with an unspecified sequential attribute" do 
    subject { FactoryBot.create_list(:post, 20, title: FactoryBot.build_sequence ) }

    it "creates sequential attribute values" do
      subject.each_with_index do |record, i|
        expect(record.title).to eq ????
      end
    end
  end

Credit: @gkosmo

Open to a discussion about what the default behaviour should ideally be 🙂.

[mohammednasser-32]

Hey @smaboshe , thanks for the review :pray: yeah I missed this case, I would say we have one of 3 options

• raise an error
• return the index
• return the default value They all seem valid so I will apply whatever you believe fits the best :smile: