Skip to content
New issue

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

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

Already on GitHub? # to your account

Add support for CastsInboundAttributes #1329

Merged
merged 2 commits into from
Sep 13, 2022

Conversation

sforward
Copy link
Contributor

@sforward sforward commented Mar 8, 2022

Summary

This PR adds support for custom cast classes that implement the CastsInboundAttributes interface ( https://laravel.com/docs/9.x/eloquent-mutators#inbound-casting). Currently the Cast class will incorrectly end up as return value of the attribute.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Misc. change (internal, infrastructure, maintenance, etc.)

Checklist

  • Existing tests have been adapted and/or new tests have been added
  • Add a CHANGELOG.md entry
  • Update the README.md
  • Code style has been fixed via composer fix-style

@sforward sforward force-pushed the fix/inbound-attribute-cast branch from 7d6df5b to 50b3c55 Compare March 8, 2022 08:34
Copy link
Collaborator

@mfn mfn left a comment

Choose a reason for hiding this comment

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

Nice PR, LGMT!

@barryvdh IMHO good to merge!

@mfn
Copy link
Collaborator

mfn commented Sep 12, 2022

Pinging @barryvdh once more 🙏🏼 PR still LGTM; not sure how cleanly it applies currently though.

Maybe @sforward has the time to sync it with master and make sure it's good (changelog especially)

@sforward sforward force-pushed the fix/inbound-attribute-cast branch from cd15a3c to dcc7bb2 Compare September 12, 2022 12:31
@sforward
Copy link
Contributor Author

Sure, I have rebased the code again. I'm willing to help testing PR's if that's the current bottleneck of this repo.

@mfn
Copy link
Collaborator

mfn commented Sep 13, 2022

I'm willing to help testing PR's if that's the current bottleneck of this repo.

I think this is always welcome, I'm definitely not vetting PRs enough whose features I don't know well / not use.

Ultimate it's up to @barryvdh for merging and I try to assist where time permits. Technically I've the ability to merge stuff, but we've a social agreement that he's the decision maker.

For this particular PR, I've no doubts about the technical part and my approval+merge still stands 😄

@barryvdh barryvdh merged commit dda200f into barryvdh:master Sep 13, 2022
ppmathis pushed a commit to ppmathis/laravel-ide-helper that referenced this pull request Nov 12, 2022
* Add support for CastsInboundAttributes

* Update changelog

Co-authored-by: Simon Verwaard <simon@bttr.nl>
@sforward sforward deleted the fix/inbound-attribute-cast branch April 2, 2023 06:08
d3v2a pushed a commit to d3v2a/laravel-ide-helper that referenced this pull request Feb 16, 2024
* Add support for CastsInboundAttributes

* Update changelog

Co-authored-by: Simon Verwaard <simon@bttr.nl>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants