Where to dispatch jobs/send events about laravel-json-api HOT 6 CLOSED

Hi Josh.

Your totally right, this would be best on the controller rather than a model listener. In the current version you'd need to overload the commit method:
https://github.com/cloudcreativity/laravel-json-api/blob/master/src/Http/Controllers/EloquentController.php#L239

For example:

protected function commit(Model $model)
{
    $isUpdate = $model->exists;
    $result = parent::commit($model):

    if ($result && $isUpdate) {
        dispatch(new SomeEvent($model));
    }

    return $result;
}

That'll work with the current version (0.4.0).

A slightly better solution might be for me to put in protected methods - didUpdate, didCreate for the those two instances and didCommit that gets called for both create/update. What do you think about that as an approach? It would mean instead of the above you could do:

protected function didUpdate(Model $model)
{
    dispatch(new SomeEvent($model));
}

from laravel-json-api.

Yeah, that seems like a good solution to me.

I guess there could also be a beforeUpdate/beforeCreate/beforeCommit too?

Maybe the naming convention could be similar to Eloquent?

protected function updating(Model $model) { // ... }

protected function updated(Model $model) { // ... }

What do you think? I don't mind helping out with a PR for this.

from laravel-json-api.

I like the idea of using the Eloquent names - I'd normally use willUpdate/didUpdate, but in terms of it being more intuitive for people who don't know my quirks the Eloquent names make sense!

I'm happy for you to submit a PR if you have time. You need to submit against the develop branch. The change should be non-breaking so can be included in the 0.4.1 release I'm preparing (you'll see a number of non-breaking changes on the develop branch at the moment).

from laravel-json-api.

Oh, if you are doing a PR you don't need to worry about tests. I haven't got this package unit tested at the moment as it's fully integration tested in two production apps. So I'd check that your PR doesn't break those apps before merging.

(I am planning on adding unit tests in the future but at the moment I've got close to 100% test coverage with the integration tests, so to save development time I'm relying on those.)

from laravel-json-api.

Ok, no problem. I should be able to take a look at it this evening.

from laravel-json-api.

@josh-taylor thanks for your input! This has been included in the v0.4.1 release.

from laravel-json-api.