Skip to content

Mutator can not handle array #2635

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

Closed
wants to merge 7 commits into from
Closed

Mutator can not handle array #2635

wants to merge 7 commits into from

Conversation

hans-thomas
Copy link
Contributor

Hi,
As mentioned in #2634, creating a mutator for the array type attribute will throw an error while setting the value to the attribute. This error will be thrown because of the invalid value prepared for the update action. So I override normalizeCastClassResponse method from Illuminate\Database\Eloquent\Concerns\HasAttributes trait and force it to add the key name all the time.

So if we have something like this on our model class

protected function skills(): Attribute
{
    return Attribute::make(
        get: fn ($value) => $value,
        set: fn ($values) => array_unique($values)
    );
}

we do not get an error anymore.

@hans-thomas hans-thomas marked this pull request as ready for review October 7, 2023 13:14
@GromNaN GromNaN self-requested a review October 7, 2023 18:48
@codecov-commenter

This comment was marked as resolved.

@hans-thomas hans-thomas requested a review from GromNaN October 9, 2023 17:04
@hans-thomas
Copy link
Contributor Author

@GromNaN I think this would be fix this issue. Now it's supporting mutators that return [ key => value ] to change the key name and also mutators that return an array of values. I write tests for both to prevent any breaking changes in the future.

@hans-thomas
Copy link
Contributor Author

So if the field's key can be an integer, this can't fix the problem and I think there is no way to solve this. Thank you for your time❤️

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.

3 participants