Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Mass assignment warning when building associated model #481

Closed
qoobaa opened this Issue · 9 comments

4 participants

@qoobaa

After Jose's patches everything seems to work fine, except of this little warning:

>> Goal.first.invitations.build(nil)
WARNING: Can't mass-assign protected attributes: goal_id
=> #<Goal::Invitation id: nil, goal_id: 1>
@jonleighton jonleighton was assigned
@pixeltrix pixeltrix reopened this
@pixeltrix
Owner

Jon, this also applies to has_one associations:

class Account < ActiveRecord::Base
  belongs_to :company
end

class Company < ActiveRecord::Base
  has_one :account
end

>> Company.new.build_account
WARNING: Can't mass-assign protected attributes: company_id
=> #<Account id: nil, company_id: nil>
@pixeltrix
Owner

Looks like the set_owner_attributes method ensures that the keys are set anyway but you do get the warning.

@pixeltrix
Owner

Looked at doing the same in SingularAssociation but come up against another bug. The AssociationScope for belongs_to includes :id, which is what you need for a find but when you're replacing the record via model.create_assoc_name the old :id gets included in scope_for_create resulting in an error when AR tries to create a record with the same primary key. Previously the default attribute protection for :id would prevent any errors but switching to :without_protection => true means that it gets assigned from the scope. The easiest fix for this is to remove the primary key field from the scope_for_create hash.

@jonleighton
Collaborator

@pixeltrix: Good catch. I am happy to have a look at this when I get time (hopefully in the next few days), but if you come up with a fix before me then that's cool too :)

@jonleighton jonleighton closed this issue from a commit
@jonleighton jonleighton Don't use mass-assignment protection when setting foreign keys or ass…
…ociation conditions on singular associations. Fixes #481 (again).
6e466f1
@sikachu sikachu referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@vatrai vatrai referenced this issue from a commit
Commit has since been removed from the repository and is no longer available.
@pixeltrix
Owner

I had pretty much the same fix ready to go but one thing I was undecided on was whether the removing of the primary key should be done in scope_for_create - is there a situation where it's ever right for it to be there?

@jonleighton
Collaborator

scope_for_create is made in part from the where_values_hash, which comes from the conditions on the scope. So we'd need to remove the primary key from the actual scope, which would be bad, because then we wouldn't get the right record :)

@jonleighton
Collaborator

I don't think we should be changing anything inside Relation itself. There could be a situation where somebody would want the primary key in the scope_for_create. So it's a case of ignoring it within the associations code, and I think the current place is the right place to do that.

@MMore

Now after release of rails 3.2, an exception can be thrown after mass assignment violation.
So this becomes an issue again.

UPDATE: Found solving issue: #4051

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.