Rails Ruby on Rails → 3.x

Comments and changes to this ticket

• You flagged this item as spam.
  

  Svelix July 12th, 2010 @ 12:47 PM

  Confirmed.

• 

  Neeraj Singh July 12th, 2010 @ 01:32 PM

  • Milestone set to “3.x”
  • State changed from “new” to “open”
  • Importance changed from “” to “Low”

  @Jacob thanks for the patch and the test. It is a good patch however I would recommend following changes

  • as per your patch if a new record is destroyed then destroyed flag would be set to true. That should not happen. Please add a test so that Person.new.destroy.destroyed? returns false

  • change

    assert_equal true, p1.destroyed?
    

    to
    

    assert p1.destroyed?
    

    . I know you just followed what was there a line above. If you want you can fix that line too.

• You flagged this item as spam.
  

  jlewalle (at gmail) July 12th, 2010 @ 08:00 PM

  No problem... about the destroy of a new record, that doesn't appear to be the behavior of the non-locking destroy:

  def destroy
    if persisted?
      self.class.unscoped.where(self.class.arel_table[self.class.primary_key].eq(id)).delete_all
    end
  
    @destroyed = true
    freeze
  end
  

  Just wanna make sure I understand things.

  I'll change the asserts.. thanks for the feedback!

• 

  José Valim July 12th, 2010 @ 09:00 PM

  • Assigned user set to “José Valim”

  Hey guys,

  Actually, if destroy is called on a record which was not persisted? (a new_record?), it should raise an error. I believe Neeraj is already working on a patch for it. :)

• 

  Neeraj Singh July 12th, 2010 @ 09:04 PM

  I will create a new ticket and will attach patch after this one is committed.

• You flagged this item as spam.
  

  jlewalle (at gmail) July 12th, 2010 @ 09:09 PM

  Awesome, let me know if you need anything else from me. Here's the patch with the cleaner assert's.

  • optimistic-locking-destroyed-flag-2.patch 1.1 KB
• 

  José Valim July 13th, 2010 @ 07:34 AM

  Jacob, your patch does not apply here. Git complains the format is invalid. Could you please follow the conventions here?

  http://rails.lighthouseapp.com/projects/8994/sending-patches

  Thanks!

• You flagged this item as spam.
  

  jlewalle (at gmail) July 13th, 2010 @ 07:46 AM

  Ah ok... I verified this one works. Thanks for your patience. I've also got a github fork here:

  http://github.com/jlewallen/rails

  • optimistic-locking-destroyed-flag-3.patch 1.3 KB
• 

  Neeraj Singh July 14th, 2010 @ 03:23 AM

  I am still not able to apply the *-flag3.patch.

  However this commit http://github.com/jlewallen/rails/commit/c7d2a50309b4a74ffc3564c4b5... can be pulled .

• You flagged this item as spam.
  

  jlewalle (at gmail) July 14th, 2010 @ 03:28 AM

  Ah... well.. still weird the patch wasn't working. I'm able to apply with:

  git am < optimistic-locking-destroyed-flag-3.patch

  What's the error you see?

• 

  Neeraj Singh July 14th, 2010 @ 03:31 AM

  Applying: Set destroyed=true in opt locking's destroy
  error: activerecord/lib/active_record/locking/optimistic.rb: does not match index
  error: activerecord/test/cases/locking_test.rb: does not match index
  Patch failed at 0001 Set destroyed=true in opt locking's destroy
  When you have resolved this problem run "git am --resolved".
  If you would prefer to skip this patch, instead run "git am --skip".
  To restore the original branch and stop patching run "git am --abort".
  

• You flagged this item as spam.
  

  jlewalle (at gmail) July 14th, 2010 @ 03:40 AM

  Can you give me the sha1 of the branch you're applying to?

• 

  Repository July 14th, 2010 @ 07:13 AM

  • State changed from “open” to “resolved”

  (from [8298bef72eac980e5a5abab12a9539c0ae1b8ceb]) Set destroyed=true in opt locking's destroy [#5058 state:resolved]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/8298bef72eac980e5a5abab12a9539...

• 

  Repository July 14th, 2010 @ 07:14 AM

  (from [d10ecfefb8bd4461127f552f14970ad6b3df507f]) Set destroyed=true in opt locking's destroy [#5058 state:resolved]

  Signed-off-by: José Valim jose.valim@gmail.com
  http://github.com/rails/rails/commit/d10ecfefb8bd4461127f552f14970a...

• 

  Ryan Bigg November 8th, 2010 @ 01:47 AM

  • Tag cleared.

  Automatic cleanup of spam.