This project is archived and is in readonly mode.

#1735 ✓committed
Michael Lovitt

after_save callback should not be called if a before_* callback was cancelled

Reported by Michael Lovitt | January 12th, 2009 @ 05:35 AM | in 2.x

Two problems:

  • When creating an AR object, if a before_create callback returns false, any after_save callbacks will still be invoked.
  • When updating an AR object, if a before_update callback returns false, any after_save callbacks will still be invoked.

This behavior is incorrect. According to the Rails docs, "if a before_* callback returns false, all the later callbacks and the associated action are canceled."

I've attached a patch with tests and a fix.

Comments and changes to this ticket

  • Pablo Mercado

    Pablo Mercado January 15th, 2009 @ 05:33 PM

    I have observed the reported behaviour. The scenario I observed is this: A before_create callback 'cancels' the save; the object is not saved. the after_save callback is still called.

    The documentation suggests that the after_save callback would not be called in such a scenario.

    I have reviewed the patch; the patch is straight-forward and effective. I also reviewed the test cases; I am glad to see that, with the application of the patch, the cancelling of callback chains is actually tested now (I could not find a test cases for this scenario prior to applying the patch).

  • José Valim

    José Valim January 15th, 2009 @ 09:03 PM

    I don't know if this is correct or not.

    I think it would break a lot of applications, so if this patch is accepted, at least a deprecation warning should be set.

  • Michael Koziarski

    Michael Koziarski January 15th, 2009 @ 09:09 PM

    This change is consistent with the documentation so I'm going to apply it.

    However I think we'll just put it into 2.3, not a 2.2 point release. This should limit the risk of surprise breakage.

  • Repository

    Repository January 15th, 2009 @ 09:12 PM

    • State changed from “new” to “committed”

    (from [7a0e7c7270548138a333bc39aab5aec80580174b]) Fixed broken after_save callback; was being called when before_create was canceled or before_update was canceled

    Signed-off-by: Michael Koziarski michael@koziarski.com [#1735 state:committed] http://github.com/rails/rails/co...

  • Mathijs Kwik

    Mathijs Kwik January 18th, 2009 @ 10:12 AM

    The description of the bug in the changelog is wrong. Look at the code before the patch, it has: "return false if callback(:before_save) == false" in it. So the case mentioned in the changelog (after save still executing if before_save fails) is wrong.

    The problem this patch is trying to fix, is that after_save is being called if a before_create or before_update has cancelled the saving. Not a before_save. That one worked already. This is also the reason why the other before/after callbacks didn't need 'fixing'

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>

Attachments

Referenced by

Pages