-
Notifications
You must be signed in to change notification settings - Fork 65
fix: feature flags for person and groups and reload flags #389
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -983,10 +983,28 @@ let maxRetryDelay = 30.0 | |
| /// // Feature flags will now use only server-side properties | ||
| /// let flagValue = PostHogSDK.shared.isFeatureEnabled("feature") | ||
| /// ``` | ||
| /// | ||
| /// - Note: This method does not automatically reload feature flags. Call `reloadFeatureFlags()` | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. people might be calling
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah the only problem I see is that the reloadFeatureFlags callback will never be called. Maybe not for this PR but we need a way to save the callback if there's another reload request in-flight and call it once the original request finishes
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not sure i follow, which callback?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is similar to call
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if thats the case, they could call the method with
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
not sure i understand, mind clarifying?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I mean is maybe we can give people a way to do: Instead of them calling
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i dont think thats a good API
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i will merge to unblock the fix
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeap agreed. I think a future improvement is saving the callbacks from reloadFeatureFlags calls in memory and calling them whenever flags reload (for whatever reason) so that we make sure that the logic that people attach there always gets executed |
||
| /// after resetting if you want to immediately refresh flags with the cleared properties. | ||
| @objc public func resetPersonPropertiesForFlags() { | ||
| resetPersonPropertiesForFlags(reloadFeatureFlags: true) | ||
| } | ||
|
|
||
| /// Resets all person properties that were set for feature flag evaluation. | ||
| /// | ||
| /// After calling this method, feature flag evaluation will only use server-side person properties | ||
| /// and will not include any locally overridden properties. | ||
| /// | ||
| /// ## Example Usage | ||
| /// ```swift | ||
| /// // Clear all locally set person properties for flags | ||
| /// PostHogSDK.shared.resetPersonPropertiesForFlags(reloadFeatureFlags: true) | ||
| /// | ||
| /// // Feature flags will now use only server-side properties | ||
| /// let flagValue = PostHogSDK.shared.isFeatureEnabled("feature") | ||
| /// ``` | ||
| /// | ||
| /// - Parameters: | ||
| /// - reloadFeatureFlags: Whether to automatically reload feature flags after resetting properties | ||
| @objc(resetPersonPropertiesForFlagsWithReloadFeatureFlags:) | ||
| public func resetPersonPropertiesForFlags(reloadFeatureFlags: Bool = true) { | ||
| if !isEnabled() { | ||
| return | ||
| } | ||
|
|
@@ -996,6 +1014,10 @@ let maxRetryDelay = 30.0 | |
| } | ||
|
|
||
| remoteConfig?.resetPersonPropertiesForFlags() | ||
|
|
||
| if reloadFeatureFlags { | ||
| remoteConfig?.reloadFeatureFlags() | ||
| } | ||
| } | ||
|
|
||
| /// Sets properties for a specific group type to include when evaluating feature flags. | ||
|
|
@@ -1042,7 +1064,7 @@ let maxRetryDelay = 30.0 | |
| /// - properties: Dictionary of properties to set for this group type | ||
| /// - reloadFeatureFlags: Whether to automatically reload feature flags after setting properties | ||
| @objc(setGroupPropertiesForFlags:properties:reloadFeatureFlags:) | ||
| public func setGroupPropertiesForFlags(_ groupType: String, properties: [String: Any], reloadFeatureFlags: Bool) { | ||
| public func setGroupPropertiesForFlags(_ groupType: String, properties: [String: Any], reloadFeatureFlags: Bool = true) { | ||
| if !isEnabled() { | ||
| return | ||
| } | ||
|
|
@@ -1067,11 +1089,20 @@ let maxRetryDelay = 30.0 | |
| /// // Clear all group properties | ||
| /// PostHogSDK.shared.resetGroupPropertiesForFlags() | ||
| /// ``` | ||
| /// | ||
| /// - Note: This method does not automatically reload feature flags. Call `reloadFeatureFlags()` | ||
| /// after resetting if you want to immediately refresh flags with the cleared properties. | ||
| @objc public func resetGroupPropertiesForFlags() { | ||
| resetGroupPropertiesForFlags(groupType: nil) | ||
| internalResetGroupPropertiesForFlags(groupType: nil, reloadFeatureFlags: true) | ||
| } | ||
|
|
||
| /// Clears all group properties for feature flag evaluation. | ||
| /// | ||
| /// ## Example Usage | ||
| /// ```swift | ||
| /// // Clear all group properties | ||
| /// PostHogSDK.shared.resetGroupPropertiesForFlags(reloadFeatureFlags: true) | ||
| /// ``` | ||
| @objc(resetGroupPropertiesForFlagsWithReloadFeatureFlags:) | ||
| public func resetGroupPropertiesForFlags(reloadFeatureFlags: Bool = true) { | ||
| internalResetGroupPropertiesForFlags(groupType: nil, reloadFeatureFlags: reloadFeatureFlags) | ||
| } | ||
|
|
||
| /// Clears group properties for feature flag evaluation for a specific group type. | ||
|
|
@@ -1083,15 +1114,29 @@ let maxRetryDelay = 30.0 | |
| /// ``` | ||
| /// | ||
| /// - Parameter groupType: The group type to clear properties for | ||
| /// - Note: This method does not automatically reload feature flags. Call `reloadFeatureFlags()` | ||
| /// after resetting if you want to immediately refresh flags with the cleared properties. | ||
| @objc(resetGroupPropertiesForFlagsWithGroupType:) | ||
| public func resetGroupPropertiesForFlags(_ groupType: String) { | ||
| resetGroupPropertiesForFlags(groupType: groupType) | ||
| internalResetGroupPropertiesForFlags(groupType: groupType, reloadFeatureFlags: true) | ||
| } | ||
|
|
||
| /// Clears group properties for feature flag evaluation for a specific group type. | ||
| /// | ||
| /// ## Example Usage | ||
| /// ```swift | ||
| /// // Clear properties for specific group type | ||
| /// PostHogSDK.shared.resetGroupPropertiesForFlags("organization", reloadFeatureFlags: true) | ||
| /// ``` | ||
| /// | ||
| /// - Parameters: | ||
| /// - groupType: The group type to clear properties for | ||
| /// - reloadFeatureFlags: Whether to automatically reload feature flags after setting properties | ||
| @objc(resetGroupPropertiesForFlagsWithGroupType:reloadFeatureFlags:) | ||
| public func resetGroupPropertiesForFlags(_ groupType: String, reloadFeatureFlags: Bool = true) { | ||
| internalResetGroupPropertiesForFlags(groupType: groupType, reloadFeatureFlags: reloadFeatureFlags) | ||
| } | ||
|
|
||
| /// Internal implementation for resetting group properties. | ||
| private func resetGroupPropertiesForFlags(groupType: String?) { | ||
| private func internalResetGroupPropertiesForFlags(groupType: String?, reloadFeatureFlags: Bool) { | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. to avoid name clash with the new method |
||
| if !isEnabled() { | ||
| return | ||
| } | ||
|
|
@@ -1101,6 +1146,10 @@ let maxRetryDelay = 30.0 | |
| } | ||
|
|
||
| remoteConfig?.resetGroupPropertiesForFlags(groupType) | ||
|
|
||
| if reloadFeatureFlags { | ||
| remoteConfig?.reloadFeatureFlags() | ||
| } | ||
| } | ||
|
|
||
| @objc public func reloadFeatureFlags() { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.