-
-
Notifications
You must be signed in to change notification settings - Fork 439
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 rehydration of subscription args with differing internal representations #1415
base: master
Are you sure you want to change the base?
Conversation
@stayallive opened the PR to see the results of the test run. |
That's a tricky one. We currently store the internal value of the I think we should try and get a hold of the raw args/variables that are passed during the initial subscription request and serialize exactly those. That will ensure the rehydrated requests function exactly the same. |
Yeah was thinking the same, will brew a bit on this, thanks for the pointers 👍 |
# Conflicts: # src/Subscriptions/Subscriber.php
That prettify docs shouldn't have ran here and or had any effect... that will mess up some PR's 😉 |
@stayallive a tricky part is that the raw request may contain literal argument values as well as variables. We don't ever see an intermediary representation where those are merged, but not cast to their internal values (e.g. Enum classes). I dug in to |
# Conflicts: # src/Subscriptions/SubscriptionBroadcaster.php # src/Subscriptions/SubscriptionResolverProvider.php # tests/Integration/Subscriptions/SubscriptionTest.php
public function parseValue($value) | ||
{ | ||
if ($value instanceof $this->enumClass) { | ||
return $value; | ||
} | ||
|
||
return parent::parseValue($value); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @stayallive, I think this might fix the issue at least for enums from bensampo/laravel-enum
. I don't know what we can do about the enums modified by @enum
tnough.
Changes
Breaking changes