Lighthouse has a new layout. Prefer the old one? Return to the old layout, and switch back any time from the link at the top of each page.

This project is archived and is in readonly mode.

[Patch] Rails3 - accepts_nested_attributes_for reject_if => {blank id} raises ActiveRecord::RecordNotFound when id is an empty string

#5638

This happens if there is a select list to choose an existing 'task' id, with :include_blank => true, and the user submits the form with the blank task selected.

class Project < ActiveRecord::Base
  has_many :tasks
  accepts_nested_attributes_for :tasks, :reject_if => proc { |attributes| attributes[:id].blank? } 
end

class Task < ActiveRecord::Base
  belongs_to :project
end

p = Project.new(:name => "project", :tasks_attributes => {"0" => {"name" => "new name", "id" => ""}})
p.save

=> ActiveRecord::RecordNotFound (Couldn't find Account with ID= [WHERE (`accounts`.`deleted_at` IS NULL)]):

We traced the problem to activerecord/lib/active_record/nested_attributes.rb in the assign_nested_attributes_for_one_to_one_association method, at line 326:

elsif attributes['id']

should be

elsif not attributes['id'].blank?

(Note: assign_nested_attributes_for_collection_association uses attributes['id'].blank? already at line 395)

This is on Rails 3.0.0

Patch is attached (including test), or on my rails fork: http://github.com/ndbroadbent/rails/commit/f13a5674885f3d3388b5bc03...

Reported by Nathan B · September 16th, 2010 @ 05:25 AM

State: committed
Milestone: 3.0.2
Assigned to: Santiago Pastorino Santiago Pastorino
Importance: Low

Activity

  1. Nathan B
  2. Neeraj Singh
    Neeraj Singh
    • Importance changed from to Low

    Not sure how much of overlap is there between this ticket and #5579 . I will look into that later.

    September 16th, 2010 @ 01:14 PM

  3. Ryan Bigg
    Ryan Bigg
    • Tag cleared.

    Automatic cleanup of spam.

    October 14th, 2010 @ 04:51 AM

  4. José Valim
    José Valim
    • Milestone cleared.
    • Assigned user set to José Valim

    October 15th, 2010 @ 11:01 PM

  5. Jeremy Kemper
  6. Ryan Bigg
    Ryan Bigg
    • Tag cleared.

    Automatic cleanup of spam.

    October 16th, 2010 @ 02:22 AM

  7. Ryan Bigg
    Ryan Bigg

    Automatic cleanup of spam.

    October 19th, 2010 @ 08:35 AM

  8. Jeff Kreeftmeijer
  9. Neeraj Singh
    Neeraj Singh
    • State changed from new to open
    • Tag set to patched

    +1

    Reviewing this patch as per request from Santiago. Tested the patch and it looks good.

    November 11th, 2010 @ 04:04 PM

  10. José Valim
    José Valim
    • Assigned user changed from José Valim to Santiago Pastorino

    Yup, it looks good. I want just to note that in Rails Code Guidelines is stated that we don't use "not". So this would probably be better written as !a["id"].blank? or a["id"].present?

    November 11th, 2010 @ 04:11 PM

  11. Repository
    Repository
    • State changed from open to committed

    (from [894fd288596b005148c3882e6431f6453d198101]) Fixed bug in active_record/nested_attributes where an empty string id caused an ActiveRecord::RecordNotFound error. Found by [Ben Tillman]

    [#5638 state:committed]

    Signed-off-by: Santiago Pastorino santiago@wyeworks.com
    https://github.com/rails/rails/commit/894fd288596b005148c3882e6431f...

    November 11th, 2010 @ 05:27 PM

  12. Repository
    Repository

    (from [d54ab371df9eb34ce26386a6656144e81eadc4b3]) Fixed bug in active_record/nested_attributes where an empty string id caused an ActiveRecord::RecordNotFound error. Found by [Ben Tillman]

    [#5638 state:committed]

    Signed-off-by: Santiago Pastorino santiago@wyeworks.com
    https://github.com/rails/rails/commit/d54ab371df9eb34ce26386a665614...

    November 11th, 2010 @ 05:32 PM