This project is archived and is in readonly mode.

#191 ✓resolved
Tim Harper

Belongs to polymorphic association assignment with new records doesn't update foreign id or type

Reported by Tim Harper | May 14th, 2008 @ 07:24 AM

Patch on github:

http://github.com/timcharper/rai...

Fixes the following scenarios:

  • I have validates_inclusion_of on the type field for a polymorphic

belongs_to association. I assign a new record to the model's polymorphic

relationship of the proper type. validation fails because the type field

has not been updated.

  • I replace the value for a polymorphic association to a new record of

another class. The type field still says its the previous class, and the

id field points to the previous record as well.

-----

[7:22pm] technoweenie: timcharper: it probably shouldnt set the record.id if record is new
[7:22pm] technoweenie: i'm not sure what record.id will be, nil I guess?  maybe it doesnt matter
[7:22pm] timcharper: yeah - that's what I'm thinking
[7:22pm] timcharper: it should at least clear it
[7:22pm] technoweenie: i guess in rare cases someone might pre-set the id  and would want it set
[7:22pm] timcharper: i was debating between that or a ternary
[7:23pm] technoweenie: record.new_record? ? nil : record.id
[7:23pm] timcharper: yeah
[7:23pm] timcharper: but was thinking the case would be in the minority that they set the id before saving the record
[7:23pm] timcharper: and if so, they probably did so intentionally, and would likely want the foreign key to receive that value as well
[7:24pm] technoweenie: yup
[7:24pm] technoweenie: as long as it doesnt blow up for the rest of us
[7:24pm] technoweenie: blow up, or set bad values
[7:25pm] timcharper: can you think of how it would ?  I can't think of anything
[7:25pm] technoweenie: nope
[7:25pm] technoweenie: i guess if you're a moron and try to set it wrong, you deserve what you get
[7:26pm] timcharper: yeah, that sounds about right
[7:26pm] timcharper: submitting the ticket
[7:26pm] technoweenie: and if its committed, and there is some wacky case where it blows up, it just means we were missing a regression test anyway

Comments and changes to this ticket

  • Rick

    Rick May 14th, 2008 @ 02:34 AM

    • Milestone set to 2.1.1
    • State changed from “new” to “open”
    • Assigned user set to “Rick”
  • Rick

    Rick May 20th, 2008 @ 06:39 PM

    • Milestone cleared.
    • State changed from “open” to “new”
    • Title changed from “Belongs to polymorphic association assignment with new_records doesn't update ” to “Belongs to polymorphic association assignment with new records doesn't update foreign id or type”
  • Rick

    Rick May 20th, 2008 @ 06:40 PM

    • State changed from “new” to “open”
  • Tim Harper

    Tim Harper May 30th, 2008 @ 01:06 AM

    Is this patch not going to make it into 2.1 ?

  • Repository

    Repository May 31st, 2008 @ 09:40 PM

    (from [0580b31b36c0f7dd1a0f8bdd1b1806e3bd65b22d]) belongs_to polymorphic association assignments update the foreign_id and foreign_type fields regardless of whether the record being assigned is new or not.

    fixes the following scenarios:

    • I have validates_inclusion_of on the type field for a polymorphic belongs_to association. I assign a new record to the model's polymorphic relationship of the proper type. validation fails because the type field has not been updated.
    • I replace the value for a ppolymorphic association to a new record of another class. The type field still says its the previous class, and the id field points to the previous record as well.

    [#191 state:closed]

    http://github.com/rails/rails/co...

  • Rick

    Rick May 31st, 2008 @ 11:03 PM

    • State changed from “open” to “resolved”

    Uh weird, why wasn't this closed

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile »

<h2 style="font-size: 14px">Tickets have moved to Github</h2>

The new ticket tracker is available at <a href="https://github.com/rails/rails/issues">https://github.com/rails/rails/issues</a>

People watching this ticket

Pages