-
Notifications
You must be signed in to change notification settings - Fork 24
feat: Add Support For Feature Flag Payloads #53
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
feat: Add Support For Feature Flag Payloads #53
Conversation
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.
I just realised my review from a few months ago is still sitting in 'pending' and you can't actually see it 🤦 - sorry about this!
* @param array $groupProperties | ||
* @return mixed | ||
*/ | ||
public function getFeatureFlagPayload( |
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.
The only thing blocking a merge here is support for local evaluation -> since getFeatureFlag
supports local evaluation, the payload should too.
You can copy the exact code from python: https://github.com/PostHog/posthog-python/blob/master/posthog/client.py#L602 - and compare the get_feature_flag
function to the PHP equivalent - they're exactly the same (with some language specific quirks)
You can also then copy all the tests below this line: https://github.com/PostHog/posthog-python/pull/81/files#diff-0e2fa9949bd8d80eecebfa2ff9b994a1bc528fbb003c7e80d104e6d5b3c5f07fR1379-R1384 to make sure it works fine.
Let me know if any questions! :) If this is too much for your bandwidth, let me know too.
Hey @neilkakkar @elliottlawson, is there anything outstanding blocking this being merged and released? I'm happy to help contribute to this to get it over the line, as it's a feature I'd quite like to be able to use :) Thanks! |
The `flagEndpointResponse` property is used to set a local eval response, not a DECIDE response.
The tests were setting the local eval endpoint response with a decide response. We need to add a new variable to `MockedHttpClient` to handle alternative decide responses.
We don't need this configurable
@elliottlawson thanks for getting this started. @jacretney I've made the necessary changes to merge this as soon as CI approves it. |
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.
Changes look good! I fixed a few minor things.
This PR enables support for retrieving the payloads of feature flags, by leveraging the
.../decide/?v=3
query parameter.The decide version can now be customized by passing a version number in the options parameter when initializing PostHog. This enables the decide call to return the
featureFlagPayloads
object in the payload. The version defaults to2
if no value is specified.Usage:
Tests
Tests are included for the following scenarios