Where to dispatch jobs/send events

[josh-taylor]

My normal approach for dispatching jobs/sending events would be in the controller action, for example here where we would send an email that a model has been updated:

public function update(Model $model, Request $request) {
    $model->update($request->all());

    dispatch(new SendModelUpdatedEmail($model));

    // ...
}

What would be the recommended approach when extending from the EloquentController to achieve something similar?

My initial thoughts would be to:

1. Override the update() method on the controller
   • Would have to duplicate the code used in the parent class.
2. Using Eloquents updated event
   • For me, this logic shouldn't be handled in the Model.

[lindyhopchris]

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));
}

[josh-taylor]

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.

[lindyhopchris]

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

[lindyhopchris]

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

[josh-taylor]

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

[lindyhopchris]

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