Revert changes in saving Eloquent hook

@Sinnbeck this is just a way to explain how i want it to be ... there is a someCondition() up there also

@ghiath.dev do you have xdebug so you can see where the request goes after you throw?

Can you try using a custom exception.

@Sinnbeck Same results...

This is test code i use

try {
		$this->expectException(CustomException::class);
		$model->change();
} catch (\Exception $exception) {
// assertions
		throw $exception;
}

@ghiath.dev So this is in test? Does it make a difference if you run it in the browser?

@Sinnbeck Nope same results in the browser... Values are changed

@ghiath.dev Can you show the full controller method so we can replicate it?

@Sinnbeck It is a unit test for a function in the model itself which makes some updates using the ->update method

@ghiath.dev I dont think ->update() calls ->save() on the model, if its a query. Only if its on a single instance.

So could you show the code?

@Sinnbeck

The saving / saved events will dispatch when a model is created or updated - even if the model's attributes have not been changed.

https://laravel.com/docs/9.x/eloquent#events

in the model i am just updating the model with the following code

$this->update([
 'attributes' => 'another',
]);

@ghiath.dev I just tested

Added this to the User model

   protected static function booted()
   {
       static::saving(function ($user) {
           throw new \Exception();
       });
   }

This does not trigger an exception

Usere::where('id', 1)->update(['password' => 'foo']);

This does (and does not persist anything to the database)

$user = User::first();
$user->update([
        'password' => 'fooasd',
    ]);

Tested in laravel 8, but I can test it in 9 in case it was changed.

@ghiath.dev Yeah calling ->update() directly on the model should work. It does for me. I just tested in a closure route.

@Sinnbeck That's an interesting point there!

I just tested it again with catching the exception

Route::get('test', function () {
    $user = \App\User::first();
    try {
        $user->update([
            'domain' => 'fooasd',
        ]);
    }
    catch (\Exception $e) {
        return 'foooo';
    }
    return view('welcome');
});

And it still works as excepted. Unless you can give me a way to replicate the error, I don't think I can help much. Does it work for you if you test with the code I just posted?

@Sinnbeck Event is not firing when the updating from a query and that very interesting point.

I have tested on the model and it is working now

@ghiath.dev exelent :)

Also there is a need to mention that model keeps the changes on its attributes even the exception is thrown.

Since i am catching the error and continue on the same model it shows that attributes are changed, I had to refresh the model inside the saving hook or resetting it like

$model->setRawAttributes($model->getRawOriginal(), true);

I think it is better cause the refresh method makes a not necessary query to the database

Would you agree?

@ghiath.dev I don't think a single request to get the data reset is bad, but if your solution works, then that is most likely fine.

If the problem is solved, please mark a best answer to set the thread as solved

@ghiath.dev The fact that this won’t work with a query is mentioned in the docs:

When issuing a mass update via Eloquent, the saving, saved, updating, and updated model events will not be fired for the updated models. This is because the models are never actually retrieved when issuing a mass update.

@tykus As i mentioned before throwing the exception here still commit the changes

@ghiath.dev is someCondition true; is the Exception actually thrown?

Anyway, to prevent saving, you can return false in the Closure.

@tykus Yes i can catch it somewhere else... thats why returning false is not a solution; It wont tell there is a logic error there

@ghiath.dev what happens when you catch the Exception? If the Exception was thrown, I would expect that the saving event would be stopped

@tykus Nope.

The saving event stops i think but it doesn't stop the full process; I mean saving the new values

@ghiath.dev so the model event must return exactly false to stop the save

        if ($this->fireModelEvent('saving') === false) {
            return false;
        }

But I don't understand why the operation would continue past a thrown exception.

@tykus As it is mentioned in the Laravel docs the event itself returns a void; I don't think returning a false stops the process https://laravel.com/docs/9.x/eloquent#observers

@ghiath.dev if it explicitly returns false, it does stop the save process, yes. The === in the code I shared above means void will not stop the save process. Check out the HasEvents trait

@tykus It leaves the code without any exceptions thrown and that's not what i want to.

I use this model function in a controller that catch the exception and return the message for the user.

@ghiath.dev in that case, I think the Model Event is not the correct place to throw the exception

@tykus Where else it could be? and why it is not the correct place?