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

Add hoc sequences in create_list #1650

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mohammednasser-32
Copy link

Fixes #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
Copy link
Contributor

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 mohammednasser-32 force-pushed the add_hoc_sequences_in_create_list branch from f5fbc18 to 213bd29 Compare May 13, 2024 19:06
@mohammednasser-32
Copy link
Author

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
Copy link
Author

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 😅 )

@mike-burns
Copy link
Contributor

mike-burns commented May 23, 2024 via email

@smaboshe smaboshe self-assigned this May 24, 2024
@smaboshe
Copy link
Contributor

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.

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

Successfully merging this pull request may close these issues.

Ad hoc sequences in create_list
3 participants