Created event fired by Eloqunt save() method working incorrectly

I am trying to do some updates after table record is created. To do that I have added event mapping in model like below

protected $dispatchesEvents = [
    'created' => TicketCreated::class,
];

I've also added correct mapping in EventServiceProvider class

protected $listen = [
    TicketCreated::class => [
        AddStatus::class
    ],
];

Then my listener tries to update record with additional data, like:

public function handle(TicketCreated $event): void
{
    $ticket = $event->ticket;
	$ticket->status = 1;
	$ticket->save();																
}

By doing that I am receiving db exception:

Cannot update identity column 'id'

In my opionion this is due to below (pls see my comments there):

 // Save the model to the database.
public function save(array $options = [])
{
    ...
    if ($this->exists) {
       ...
    }
    else {

        // !!! performInsert() function fire 'created' event at the very end. !!!
        $saved = $this->performInsert($query);
       	// !!! This will add missing connection name to the model object !!!
        if (! $this->getConnectionName() &&
            $connection = $query->getConnection()) {
            $this->setConnection($connection->getName());
        }
    }

    if ($saved) {
        // !!! finishSave() function will add original attribute containing original data, obviously the same as original array in this example !!!
        $this->finishSave($options);
    }

    return $saved;
}

Not sure if intended, but when creating new record using Eloquent: Illuminate\Database\Eloquent\Model save() function fires created event in performInsert() function before setting connection and executing finishSave() function.

As a result model received by 'created' event has:

'connection' attribute empty what results in some issues when using multiple dbs
'original' array empty what results in issue with updating this model in event/listener class as then save() method tries to save also id which is primary key here.

To handle those issue I need to refresh model received by event.

Example of dd() of model received by event:

{
  #connection: null
  #table: "ticket"
  #primaryKey: "id"
  #keyType: "int"
  +incrementing: true
  #with: []
  #withCount: []
  +preventsLazyLoading: false
  #perPage: 15
  +exists: true
  +wasRecentlyCreated: true
  #escapeWhenCastingToString: false
  #attributes: array:17 [▶]
  #original: []
  #changes: []
  #casts: []
...
}

Example of dd() of model dumped right after save():

{
  #connection: "tmts"
  #table: "ticket"
  #primaryKey: "id"
  #keyType: "int"
  +incrementing: true
  #with: []
  #withCount: []
  +preventsLazyLoading: false
  #perPage: 15
  +exists: true
  +wasRecentlyCreated: true
  #escapeWhenCastingToString: false
  #attributes: array:17 [▶]
  #original: array:17 [▶]
  #changes: []
  #casts: []
...
}

You can see differences here and what I am refering to. If event would receive object with all data like the last one then everything will work as expected and save() method would not try to save primary key id here.

I've make it working by using refresh() on model in my event but feel like it is a kind of bug/overlooking in the eloquent class itself.

Any ideas how to make it better?

LaryAI

2 years ago

Level 58

It seems like you've encountered a situation where the Eloquent model's created event is being fired before some of the model's internal state, such as the connection and original attributes, is fully set up. This can indeed cause issues when you try to perform further operations on the model within the event listener.

Using refresh() in your event listener is a valid workaround, as it will rehydrate the model with fresh data from the database, including the original attribute and the connection name. However, I understand your concern that this might not be the most elegant solution.

Here's an alternative approach that avoids calling refresh():

Instead of directly modifying and saving the model in the event listener, you could dispatch a job that will handle the update. This job will be processed asynchronously (if you're using a queue) and by the time it runs, the model's state should be fully set up.

Here's an example of how you could implement this:

Create a new job:

php artisan make:job UpdateTicketStatus

In the generated job class, add a constructor to accept the ticket and the status, and a handle method to perform the update:

use App\Models\Ticket;

class UpdateTicketStatus implements ShouldQueue
{
    protected $ticket;
    protected $status;

    public function __construct(Ticket $ticket, $status)
    {
        $this->ticket = $ticket;
        $this->status = $status;
    }

    public function handle()
    {
        $this->ticket->status = $this->status;
        $this->ticket->save();
    }
}

Dispatch the job from your event listener:

public function handle(TicketCreated $event): void
{
    UpdateTicketStatus::dispatch($event->ticket, 1);
}

By dispatching a job, you're deferring the update operation until after the current request cycle, which should ensure that the model's internal state is fully set up. This approach also has the benefit of keeping your event listener clean and focused on its primary responsibility, which is to react to the event and delegate work, rather than doing the work itself.

If you're not using queues and want the job to be processed immediately but still after the current request cycle, you can use the dispatchSync method instead:

UpdateTicketStatus::dispatchSync($event->ticket, 1);

This will execute the job synchronously without pushing it to a queue.

Remember that if you're using the ShouldQueue interface and you haven't set up a queue system, you'll need to configure one or remove the interface to run the job synchronously.

This approach should help you avoid the immediate issue with the created event and provide a more robust solution for handling post-creation logic.

Please or to participate in this conversation.