Rails Ruby on Rails

Comments and changes to this ticket

• 

  Santiago Pastorino August 20th, 2010 @ 12:24 AM

  • Importance changed from “” to “Low”

  Javier can you do a test case following this http://rails.lighthouseapp.com/projects/8994/sending-patches Thanks very much.

• You flagged this item as spam.
  

  Javier Fernandez-Ivern August 20th, 2010 @ 12:56 AM

  • Tag changed from “mark_for_destruction validates_uniqueness_of fields_for update_attributes” to “fields_for, mark_for_destruction, patch, update_attributes, validates_uniqueness_of”

  Done. Sorry, I thought that process was only for people sending solutions not just tests.

  • test_issue_5417.diff 1.9 KB

• You flagged this item as spam.
  

  Javier Fernandez-Ivern August 20th, 2010 @ 12:56 AM

  • Tag changed from “fields_for, mark_for_destruction, patch, update_attributes, validates_uniqueness_of” to “fields_for, mark_for_destruction, patch, test, update_attributes, validates_uniqueness_of”
• 

  Santiago Pastorino August 20th, 2010 @ 05:50 AM

  Javier, it's ok a test case help others to make a patch, so when someone post the solution your test and the solution are applied, giving some credit to you too ;).
  Now ... do you want to try a patch? ;).
  Thanks.

• You flagged this item as spam.
  

  Javier Fernandez-Ivern August 20th, 2010 @ 06:07 AM

  Yes, but would you mind discussing the problem a little bit first? I no longer like my test case, because there's no guarantee that the record that is marked_for_deletion is actually going to be saved. So in the general case of the target record being marked for deletion, I'm not sure this is a change that needs to be made.

  The case that's more clear-cut is when an association is being saved and it contains items that are marked for deletion and items that conflict with them in uniqueness. In this case, we know both changes are going to happen (barring a transaction blowup or another validation failure). I'd therefore focus on addressing this problem at the association save level, by ordering the child saves so that elements to be destroyed go first.

  Does this seem like the right approach? If so I'll rewrite the test case and take a shot at a patch.

• You flagged this item as spam.
  

  Javier Fernandez-Ivern August 20th, 2010 @ 03:02 PM

  Well, I'm looking into resolving this, it's somewhat tricky (to me, at least) since I need to make it work when validating the association, not just when saving. Else it'd be pretty strange.

  Until then, I'm attaching a better test:

    def test_should_skip_uniqueness_validation_if_existing_record_is_marked_for_destruction
      roger = UniqueParrot.create(:name => "Roger")
      wilco = UniqueParrot.create(:name => "Wilco")
      
      @pirate.parrots << roger << wilco
      assert @pirate.valid?
      assert @pirate.save
      
      wilco.name = roger.name
      assert !@pirate.valid?
      assert !@pirate.save
      
      roger.mark_for_destruction
      assert @pirate.valid?
      assert @pirate.save
    end
  

  • better_test_5417.diff 2 KB
• 

  Jeff Kreeftmeijer November 8th, 2010 @ 08:28 AM

  • Tag cleared.

  Automatic cleanup of spam.

• 

  Santiago Pastorino February 9th, 2011 @ 12:31 AM

  • State changed from “new” to “open”

  This issue has been automatically marked as stale because it has not been commented on for at least three months.

  The resources of the Rails core team are limited, and so we are asking for your help. If you can still reproduce this error on the 3-0-stable branch or on master, please reply with all of the information you have about it and add "[state:open]" to your comment. This will reopen the ticket for review. Likewise, if you feel that this is a very important feature for Rails to include, please reply with your explanation so we can consider it.

  Thank you for all your contributions, and we hope you will understand this step to focus our efforts where they are most helpful.

• 

  Santiago Pastorino February 9th, 2011 @ 12:31 AM

  • State changed from “open” to “stale”

• You flagged this item as spam.
  

  2kan February 9th, 2011 @ 03:00 AM

  • State changed from “stale” to “open”

  [state:open]

  The problem with this ticket and Javier Fernandez-Ivern's test is not in mark_for_destruction itself. In edge validations do not run for associations which are marked for destruction but the problem in the whole illustrated situation:

  He marks for deletion the roger object. So validation for it doesn't run. But then it validates the wilco object which is not marked for destruction and has the same name with the rojer object which is marked for destruction but still exists in database, so when we try to validate for wilco for uniqueness it fails.