Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[9.x] Fix HasAttributes::mutateAttributeForArray when accessing non-cached attribute #42130

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

aijorgenson
Copy link
Contributor

@aijorgenson aijorgenson commented Apr 25, 2022

Currently there is no check for hasAttributeGetMutator in the HasAttributes::mutateAttributeForArray method.

This causes serializing a model to an array to fail when an attribute is specified in the appends array, as it falls back trying to retrieve the value via mutateAttribute. mutateAttribute utilizes the previous standard getFirstNameAttribute vs the new firstName(): Attribute approach referred to in
https://laravel.com/docs/9.x/eloquent-mutators.

@taylorotwell
Copy link
Member

Please provide a bullet point list of how to recreate the error in a new Laravel application. Thanks.

@aijorgenson
Copy link
Contributor Author

aijorgenson commented Apr 26, 2022

@taylorotwell

Given the following test:

class SomeModel extends \Illuminate\Database\Eloquent\Model
{
    public $appends = ['firstName'];

    public function firstName(): \Illuminate\Database\Eloquent\Casts\Attribute
    {
        return \Illuminate\Database\Eloquent\Casts\Attribute::make(
            get: fn() => 'foo'
        );
    }
}

class HasAttributesTest extends TestCase
{
    public function test_it_doesnt_explode_when_serializing_attributed_model_to_array()
    {
        $model = new SomeModel;
        $this->assertEquals([
            'firstName' => 'foo'
        ], $model->toArray());
    }
}

test_it_doesnt_explode_when_serializing_attributed_model_to_array will fail with the following exception:

BadMethodCallException : Call to undefined method Tests\Unit\framework\SomeModel::getFirstNameAttribute()
 /app/vendor/laravel/framework/src/Illuminate/Support/Traits/ForwardsCalls.php:71
 /app/vendor/laravel/framework/src/Illuminate/Support/Traits/ForwardsCalls.php:36
 /app/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php:2133
 /app/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:621
 /app/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:671
 /app/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php:202
 /app/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php:1487
 /app/tests/Unit/framework/HasAttributesTest.php:24

with the changes in this PR, it passes.

spinning up a new Laravel example app real quick to confirm in a fresh environment.

@aijorgenson
Copy link
Contributor Author

aijorgenson commented Apr 26, 2022

Example in a brand new example-app ( https://laravel.com/docs/9.x#getting-started-on-linux ) running laravel/framework v9.9.0

Added the following to app/Models/User.php

    protected $appends = ['firstName'];

    public function firstName(): \Illuminate\Database\Eloquent\Casts\Attribute
    {
        return \Illuminate\Database\Eloquent\Casts\Attribute::make(
            get: fn() => 'foo'
        );
    }

tinker console:

   >>> $u = new App\Models\User;
=> App\Models\User {#3504
    +firstName: "foo",
  }
>>> $u->toArray()
BadMethodCallException with message 'Call to undefined method App\Models\User::getFirstNameAttribute()'

tinker console after fix is applied:

>>> $u = new App\Models\User;
=> App\Models\User {#3504
     +firstName: "foo",
   }
>>> $u->toArray()
=> [
     "firstName" => "foo",
   ]

@taylorotwell taylorotwell merged commit 40dc9b9 into laravel:9.x Apr 26, 2022
@taylorotwell
Copy link
Member

Can you PR a test for this?

@aijorgenson
Copy link
Contributor Author

aijorgenson commented Apr 26, 2022

Of course! I didn't see any existing tests for the trait so I wasn't sure where to add it.

Looking closer I see attribute tests in framework/tests/Database/DatabaseEloquentModelAttributeCastingTest.php, I'll add it there

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants