Make setCallback() add to previous callbacks

[jesse-r-s-hines]

setCallback() replaces previous callbacks making you have to repeat any logic in the "base case' definition callback if you have a callback on one of the groups.

See issue #472

A better solution may be to add a addCallback method instead of modifying setCallback().

[GrahamCampbell]

Thanks for preparing this. This is certainly a breaking change, so can't be merged as is, and it is not obvious how we should implement addCallback along side setCallback in a way that makes sense.

[GrahamCampbell]

Returning false to exit the callback chain is incompatible with the fact that false also determines if a save should go ahead or not. What if someone wants to exit the callback chain but still wants another persit to run?

[jesse-r-s-hines]

If I understand correctly, the current implementation will not persist the model if the callback returns false? So this line should make it so that if any of the callbacks return false, the model won't be persisted. Would you rather that all callbacks are called regardless of whether a prior one "failed" and then return false if any of them failed afterwards to prevent it being persisted?

[GrahamCampbell]

If I understand correctly, the current implementation will not persist the model if the callback returns false?

No, it will not persist the model again. Possibly someone might want the model to be updated in the database again after their callback does something, but does not want the remaining callbacks to be executed.

[jesse-r-s-hines]

Ah, okay.
So I should change it to something like

$this->callback = function ($model, $saved) use ($oldCallback, $callback) {
        $oldResult = $oldCallback($model, $saved);
        $newResult = $callback($model, $saved);
        return $oldResult !== false && $newResult !== false;
};

So that all the callbacks are always called, and returning false in any of them will prevent the second persist.

Then if we move this to addCallback(), then you can still use setCallback() to override all the other callbacks if you don't want them called.

[jesse-r-s-hines]

If we added an addCallback() function it would have to add a callback to the "chain", starting with any made with a prior setCallback() or prior addCallback()s. And calling setCallback() again would overwrite all the callbacks that have been accumulated. That would prevent it from being a breaking change.

[jesse-r-s-hines]

I'm going to close this and submit another pull request that implements addCallback() to maintain backwards compatibility.