Open mohammednasser-32 opened 5 months ago
Probably the easiest thing to do is to add a test to spec/acceptance/callbacks_spec.rb
. You'll want to make sure that before(:build)
is called before anything is built.
Spitballing here, something like:
class TitleSetter
def self.title=(new_title)
@@title = new_title
end
def self.title
@@title
end
end
define_model("Article", title: :string)
FactoryBot.define do
factory :article_with_before_callbacks, class: :article do
before(:build) { TitleSetter.title = "title from before build" }
after(:build) { TitleSetter.title = "title from after build" }
title { TitleSetter.title }
end
end
it "runs the before callback" do
article = FactoryBot.build(:article_with_before_callbacks)
expect(article.title).to eq("title from before build")
end
@mike-burns Thank you for your suggestion :pray:
but won't title from after build
in the after(:build)
callback always overwrite the title from before build
? why is this scenario expected to pass?
also would something like this do it?
define_model("User", full_name: :string)
FactoryBot.define do
factory :user_with_before_callbacks, class: :user do
before(:build) { user.full_name = "first }
after(:build) { user.full_name = "#{user.full_name} last" }
end
end
it "runs the before callback" do
article = FactoryBot.build(:user_with_before_callbacks)
expect(user.full_name).to eq("first last")
end
We need to make sure before(:build)
is called before any building happens.
My theory on why my suggestion works is this timeline:
FactoryBot.build(:article)
before(:build) { TitleSetter.title = "title from before build" }
TitleSetter.title = "title from before build"
build
happensresource = Article.new
resource.title = (-> { TitleSetter.title }).call
TitleSetter.title
# => "title from before build"
resource
after(:build) { TitleSetter.title = "title from after build" }
TitleSetter.title = "title from after build"
resource.title
has not changed since step (6), because we already called #call
.(For what it's worth, I'm not sure your current code calls the callback before the object is built.)
@mike-burns you are right the approach was not correct and it was actually called after the build, thanks for raising the hand! I changed the approach here, now it is being called before the object is built and your test passes.
Hey @mike-burns , sorry to ping you again 😅 I am just wondering if the new approach is okay or do I need to investigate other options? Thank you
Oh hey, this approach is good, thanks for taking it on.
I need to find time to do a final review and merge it. I can do small tweaks from here and give you credit.
Thank you, much appreciated 🙏
Hey @sarahraqueld , sorry for pinging..I am just keen to have some contributions in the gem (if possible) 😅 so I would appreciate it if someone can take a look on this PR and tell me if there is any change I need to do. (I also had another PR here 🙈) Thank you so much.
Hey @sarahraqueld , sorry for pinging..I am just keen to have some contributions in the gem (if possible) 😅 so I would appreciate it if someone can take a look on this PR and tell me if there is any change I need to do. (I also had another PR here 🙈) Thank you so much.
Hi @mohammednasser-32, me and @smaboshe are trying our best to deal with all the issues and PRs we inherited when we became maintainers. We thank you for your patience, and will review this today.
@sarahraqueld @smaboshe Thank you so much, much appreciated and sorry for the noise 🙏
@mohammednasser-32 We see that the CI seems to be stuck. Can you rebase your branch and push to see if that triggers the tests to run correctly this time?
@sarahraqueld Done
Fixes https://github.com/thoughtbot/factory_bot/issues/1633
add support for
before(:build)
callback as requested in the issueTested the changes by adding the local gem to a project
I am honestly not sure how to add unit tests for the change, any help would be appreciated