Open mohammednasser-32 opened 1 month ago
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.)
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
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: )
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]
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.
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 aProc
, however it will work fine if we pass aProc
too. Is it better to have a dedicated class for this instead?