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

fix Enums not being cast when calling Model::toArray() #2522

Merged
merged 1 commit into from Mar 14, 2023

Conversation

jeromegamez
Copy link
Contributor

@jeromegamez jeromegamez commented Mar 14, 2023

Hey there 👋,

while testing the migration of a project to Laravel 10, calling toArray() on a model caused a warning when attributes are defined to be cast as Enums.

Attempt to read property "my_property" on string in vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php on line 1181

(and it didn't work). After diving a bit into the code, I noticed that the previously created $castValue was not used when converting the Enum to a string in the addCastAttributesToArray() method.

I didn't know where to put the Enum for the test, so I just added it to the models directory although it felt wrong 😅.

The changed order in the use statements is due to StyleCI complaining.

Let me know if you'd like me to change the Enum location or anything else! 🙏🏻

:octocat:

@jeromegamez
Copy link
Contributor Author

Alright, I think StyleCI and PHP CS Fixer are in conflict about whether the use statements should be ordered alphanumerically or separately 😅.

If you'd like, I could update and apply the PHP CS Fixer config to be in sync with StyleCI in an additional commit or in a new PR.

@codecov-commenter
Copy link

codecov-commenter commented Mar 14, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (7669dc2) 89.91% compared to head (037a258) 89.91%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #2522   +/-   ##
=========================================
  Coverage     89.91%   89.91%           
  Complexity      712      712           
=========================================
  Files            32       32           
  Lines          1726     1726           
=========================================
  Hits           1552     1552           
  Misses          174      174           
Impacted Files Coverage Δ
src/Eloquent/Model.php 91.66% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@alcaeus alcaeus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change looks good, thanks for the fix and test 🎉

As for StyleCI vs. PHP CS Fixer, I'd let @divine make a decision on that.

@divine divine changed the title Fix Enums not being cast when calling Model::toArray() fix Enums not being cast when calling Model::toArray() Mar 14, 2023
@divine
Copy link
Contributor

divine commented Mar 14, 2023

Hello,

We can’t disable styleci and the preferred is always php-cs-fixer.

Thanks!

@divine divine merged commit 23c396c into mongodb:master Mar 14, 2023
9 checks passed
@jeromegamez jeromegamez deleted the fix-enum-casts branch March 20, 2023 08:30
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.

None yet

4 participants