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

[4.x] Bug: Values class not forwarding calls to proxied collection methods #9412

Conversation

caseydwyer
Copy link
Contributor

This PR makes a few changes to how the magic __call method works, within Fields\Values.php. Currently, it performs a single check and throws an exception if this condition is not met:

$this->getProxiedInstance()->has($method)

We get the collection back as the proxied instance, but the has method checks for the existence of a particular key in the data—not an available method on the class itself.

With this PR, we're adding in an initial check to see if the passed $method exists on the Collection, via the getProxiedInstance(). If it does exist, call it with the additional $args and return the value. Otherwise, continue to the same query builder check we had originally. If neither of these apply, throw an exception w/ a slightly more descriptive message.

Also added to the ValuesTest, with an example that illustrates our particular use-case for some custom front-end output.


Disclaimer: the way this method was defined lead me to believe it's missing the functionality outlined above, but by all means, check my work/logic here, and don't hesitate to trash this PR if it's not inline with the intended use of that class or method.

if available, call and return the Collection's method w/ $args
if applicable, return builder
this is just to keep the method "in-line" with toArray above—both of which, I believe, are to satisfy contracts (ie, Arrayable / JsonSerializable).
@jasonvarga
Copy link
Member

jasonvarga commented Feb 26, 2024

Sorry for the delay. Can you please explain the actual issue that this PR is fixing? How exactly were you using this?

Closing until you have a chance to respond. Thanks!

@jasonvarga jasonvarga closed this Feb 28, 2024
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