-
Notifications
You must be signed in to change notification settings - Fork 24
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
isFeatureEnabled now returns true if multivariant flag #42
Conversation
posthog/src/main/java/com/posthog/internal/PostHogFeatureFlags.kt
Outdated
Show resolved
Hide resolved
Actually, I am a bit confused, the so if the flag is within the response payload, it is enabled, always, then the iOS implementation is correct. Should @neilkakkar mind explaining? |
Decide returns all enabled flags, correct. But just because a flag is enabled doesn't mean that the API will always return true. For example, if you have a filter on Country = X on the flag, the API will return false if you're outside country X. Same for, say, if the rollout % is not 100%. |
Correct, and nice finding! |
@@ -25,7 +25,7 @@ configure<JavaPluginExtension> { | |||
|
|||
buildConfig { | |||
useKotlinOutput() | |||
packageName("com.posthog.internal") |
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.
To avoid pinterest/ktlint#1874
💡 Motivation and Context
By doing #41 I noticed that
isFeatureEnabled
is also wrong.I also noticed that the iOS SDK never returns
false
if a flag has afalse
value.https://github.com/PostHog/posthog-ios/blob/9298825fe26899a58b976d400254e0b050daa578/PostHog/Classes/PHGPostHog.m#L317
Is it a bug on the iOS SDK I believe, right?
💚 How did you test it?
📝 Checklist