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

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

Open
fsateler opened this issue Apr 30, 2019 · 6 comments

Comments

@fsateler
Copy link

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

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 #687, for example) so maybe we can improve the documentation around this.

@fsateler
Copy link
Author

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

composerinteralia commented Apr 30, 2019

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:

def association(runner)
runner.run(:null)
end

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

fsateler commented May 2, 2019

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

fsateler commented May 2, 2019

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants