attributes_for(:factory, some_association: foo) returns nil for :some_association

[fsateler]

I have searched the issue list for something similar, but I have not found the exact same issue.

The problem is that, given these two models:

factory :employee do
  # various attributes
end

factory :job do
  association :employee, strategy: :build
end

attributes_for ignores any explicit employee assignments:

attributes_for(:job, employee: some_other_employee)[:employee] # this is nil!

build and create do not suffer from this problem. Removing the association annotation from the second factory solves the problem, but then I can't have a default association for create.

The following spec fails on current master:

  it "allows specifying association values" do
    sub = attributes_for(:post, user: User.new)
    expect(sub[:user]).not_to eq nil
  end

Note that the following does work:

  it "allows overriding association keys" do
    sub = attributes_for(:post, user_id: 5)
    expect(sub[:user_id]).to eq 5
  end

[composerinteralia]

That is the expected behavior for attributes_for. It does not include attributes for associations. There has been some confusion about this in the past (see https://github.com/thoughtbot/factory_bot/issues/687, for example) so maybe we can improve the documentation around this.

[fsateler]

Hi @composerinteralia , thanks for your quick response.

I think this issue is different from #687 , #450, #408, #359 or other related issues. Those issues are about attributes_for not returning the attributes for the associations that would be built during build or create. I agree they should not be returned there.

The problem here is that attributes_for is explicitly ignoring parameters I'm passing it:

# Assume foo and bar are *not* attributes of Job
attributes_for(:job, foo: :bar, bar: :foobar) # returns { foo: :bar, bar: :foobar }
attributes_for(:job, foo: :bar, bar: :foobar, employee: some_employee) # returns { foo: :bar, bar: :foobar }

In other words, I don't want attributes_for to build attributes for all associations. I want it to stop discarding a parameter I'm passing it.

[composerinteralia]

It discards it because it is an association. The fact that the value is passed in as an override doesn't change how attributes_for behaves; it always uses the null strategy for associations: https://github.com/thoughtbot/factory_bot/blob/99ac02400fd56bd1872fc3ed84f81ea5e8f27737/lib/factory_bot/strategy/attributes_for.rb#L4-L6

Could you share your use case? Perhaps we can find a workaround. We can also discuss the current behavior more, but changing it would be non-trivial and I would want to make sure there was a common use case before exploring that.

[fsateler]

My use case is that I have some methods that create the models, and I'd like to test those:

# in Job
def self.create_with_metadata params, metadata
  transaction do
    job = create!(params)
    do_stuff_with_metadata(metadata)
  end
end

# in the controller
def create
  @employee = Employee.find(params[:employee_id])
  job = Job.create_with_metadata(job_params.merge(employee: employee), metadata)
  redirect_to job_path(job)
rescue ActiveRecord::RecordInvalid => e
  @job = e.record
  render 'edit'
end

Now, I'd like to test this create_with_metadata method. My current workaround is relatively simple, but a bit annoying:

test "it should work with end_date" do
  employee = build(:employee)
  job = Job.create_with_metadata(attributes_for(:job, end_date: Date.today).merge(employee: employee))
  assert_equal Date.today, job.end_date
end

The annoying part is that, as a user, it is not obvious that by declaring an association you render the attribute unsettable in the attributes_for call. So every time I bump into this I waste a few minutes wondering why attributes_for(:job, employee: employee) doesn't work as I intended.

[fsateler]

We can also discuss the current behavior more, but changing it would be non-trivial

I can understand that, and maybe it is not worth fixing. However, I think the current behavior is not intuitive. Perhaps attributes_for should simply refuse (ie, raise an error) attributes it will clear later.

[composerinteralia]

Yeah, I see your point. Thanks for that example.