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 enum casts arrayable behaviour #40885

Merged
merged 3 commits into from
Feb 12, 2022
Merged

Conversation

diegotibi
Copy link
Contributor

This PR fix a wrong behavoiur when an attribute is casted to an enum implementig Arrayable in comparision to the behavoiur when the same thing is done on a collection object.

Clarifying example:

Let's take this enum:

enum Status: string implements Arrayable
{
    case pending = 'pending';
    case done = 'done';
    
    /**
     * @inheritDoc
     */
    public function toArray()
    {
        return [
            'name' => $this->name,
            'value' => $this->value,
            'description' => "description of {$this->name}",
        ];
    }
}

Behaviour in a collection object:

collect(Status::pending)->toArray();
/**
[
     "name" => "pending",
     "value" => "pending",
     "description" => "description of pending",
]
*/

Behaviour in a casted model attribute:

class MyModel extends Model
{
    protected $casts = [
        'status' => Status::class
    ];
}


MyModel::first()->toArray();
/**
[
    'id' => 1,
    'status' => 'pending'
]
*/

Expected behaviour:

MyModel::first()->toArray();
/**
[
    'id' => 1,
    'status' => [
         "name" => "pending",
         "value" => "pending",
         "description" => "description of pending",
   ]
]
*/

Related issues: #40580 and #40693

@diegotibi diegotibi changed the title [9.x] Fix enum casts toArray behaviour [9.x] Fix enum casts arrayable behaviour Feb 9, 2022
@driesvints
Copy link
Member

I definitely wouldn't expect enums to be converted to their array version. Their value should be used. The current behavior is expected imo.

@X-Coder264
Copy link
Contributor

Without this change there's no point in an enum implementing Arrayable as it does nothing for enums currently - which is inconsistent with how all other Arrayable implementations are treated by Eloquent.

On the other hand I don't see much of an use case in transforming a scalar value into an array.

I guess there's no harm in letting enums behave like all other Arrayable implementations as I don't see any other use case as to why users would implement Arrayable on their enums.

@driesvints
Copy link
Member

@X-Coder264 hmm I guess you're right about that.

@GrahamCampbell
Copy link
Member

If this is considered a bug fix, this should probably land on Laravel 8, not 9.

@diegotibi
Copy link
Contributor Author

@GrahamCampbell yes, you're probably right, this PR can ported to 8.x branch with no work at all. However enums are a php 8.1 feature so it's probably more usefull on 9.x branch

@GrahamCampbell
Copy link
Member

PHP 8.1 can also be used with Laravel 8, so that's not relevant, actually.

@taylorotwell taylorotwell merged commit fef0260 into laravel:9.x Feb 12, 2022
@diegotibi
Copy link
Contributor Author

@taylorotwell Should I make a PR on 8.x branch too, like @GrahamCampbell suggested?

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.

5 participants