This project is archived and is in readonly mode.

#902 ✓duplicate
Jon B.

field_changed? does not always typecast before comparison

Reported by Jon B. | August 25th, 2008 @ 08:38 PM | in 2.x

I've recently been working on a project and came across an issue with the way field_changed? was handling incoming values. Specifically, if assigning a string value of "0" (eg, from an HTTP POST via update_attributes) to an integer column that is nullable, and with a previous value of 0, the field is marked as changed.

=> #<ActiveRecord::ConnectionAdapters::MysqlColumn:0x4edfd10 @sql_type="int(11)"
, @default=nil, @precision=nil, @primary=false, @limit=4, @name="severity", @nul
l=true, @type=:integer, @scale=nil>
>> bug = Bug.find(2)
=> #<Bug id: 2, summary: "Ticket with few changes", status: "Confirmed", severit
y: 0, ticket_type: "Defect", resolution: "Open", description: "With a descriptio
n", steps_to_reproduce: "", additional_information: "", project_id: 1, project_b
ug_id: 2, user_id: 1, owner_id: 2, created_at: "2008-08-04 14:09:56", updated_at
: "2008-08-04 14:11:17", priority: 2, comment: "ffffff">
>> bug.severity = "0"
=> "0"
>> bug.changes
=> {"severity"=>[0, 0]}

After some digging, it seems to be because in this very specific case, the incoming value is not typecast before comparison; it enters the conditional but then does nothing since the new value is not blank.

It seems that a solution is to simply move "value = column.type_cast(value)" outside of the conditional. IE,

          if column.type == :integer && column.null && (old.nil? || old == 0)
            # For nullable integer columns, NULL gets stored in database for blank (i.e. '') values.
            # Hence we don't record it as a change if the value changes from nil to ''.
            # If an old value of 0 is set to '' we want this to get changed to nil as otherwise it'll
            # be typecast back to 0 (''.to_i => 0)
            value = nil if value.blank?
          value = column.type_cast(value)

I haven't done a lot of testing to ensure that this does not create other problems, but in the few tests I ran it seemed to be alright. I've included the change I made in a (hopefully correct) patch file.

Comments and changes to this ticket

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=""></a>

People watching this ticket