Should Model use Tappable trait by default ?

[bilboque]

I find the interraction between the model and the builder quite error prone when using tap on a model without the Tappable trait.

SomeModel::find($id)
    ->tap(fn ($instance) => doSomething)
    ->update('field', $value);

I would expect by default to be updating the the model instance but instead, in this exemple, I am updating my whole table since we are calling tap through $this->forwardCallTo($this->newQuery(), $method, $parameters); of the __call method of the model.

Also I dont understand why the traits BuildQueries has a custom tap method instead of just using the Tappable trait.

[macropay-solutions]

The issue is that the Builder is returned instead of the model.
forwardDecoratedCallTo can replace forwardCallTo and that would fix it. Can you pls confirm?

[macropay-solutions]

This is the result of developer convenience. Model::where.... instead of Model::query()->where...

[macropay-solutions]

Maybe the fix is in

    public static function __callStatic($method, $parameters)
    {
        return (new static())->$method(...$parameters);
    }

Moving :

        return $this->forwardCallTo($this->newQuery(), $method, $parameters);

from __call to __callStatic

This is related also to
#57262
#49009
#48595

[macropay-solutions]

This could be a possible solution

[marius-ciclistu]

@bilboque The answer is yes. You should add that trait in your model if you want to use tap. You can also use the general helper tap().

[macropay-solutions]

We just made a change for the above because hydrate was called on an instance that does not exist.

And we reverted the changes to __callStatic

    public static function __callStatic($method, $parameters)
    {
        return (new static())->$method(...$parameters);
    }