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.

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

#1735

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.

Reported by Michael Lovitt · January 12th, 2009 @ 05:35 AM

State: committed
Milestone: 2.x
Assigned to: nobody
Importance: none

Activity

  1. Pablo Mercado
    Pablo Mercado

    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).

    January 15th, 2009 @ 05:33 PM

  2. José Valim
    José Valim

    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.

    January 15th, 2009 @ 09:03 PM

  3. Michael Koziarski
    Michael Koziarski

    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.

    January 15th, 2009 @ 09:09 PM

  4. Repository
    Repository
    • 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 after_save callback should not be called if a before_* callback was cancelled state:committed] http://github.com/rails/rails/co...

    January 15th, 2009 @ 09:12 PM

  5. Mathijs Kwik
    Mathijs Kwik

    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'

    January 18th, 2009 @ 10:12 AM