feat(core): Add ability to cache properties for flags #315
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,6 +74,25 @@ public open class PostHogConfig( | |
| * Defaults to null (evaluate all flags) | ||
| */ | ||
| public var evaluationEnvironments: List<String>? = null, | ||
| /** | ||
| * Automatically set common device and app properties as person properties for feature flag evaluation. | ||
| * | ||
| * When enabled, the SDK will automatically set the following person properties: | ||
|
There was a problem hiding this comment. we should add There was a problem hiding this comment. Added in f917d11 |
||
| * - $app_version: App version from package info | ||
| * - $app_build: App build number from package info | ||
| * - $os_name: Operating system name (Android) | ||
| * - $os_version: Operating system version | ||
|
Comment on lines
+84
to
+85
There was a problem hiding this comment. should we add There was a problem hiding this comment. if we decide to add those and the ones mentioned in the comment above, i'd suggest also adding to the iOS SDK https://github.com/PostHog/posthog-ios/blob/ba5dd65d2341ab8ea1949f072594ef67c490af3a/PostHog/PostHogConfig.swift#L98-L110 There was a problem hiding this comment. This is an option, though it'd also require another change to set There was a problem hiding this comment. I've added these in f917d11. If this is something we want to support, we can keep them in - it does no harm to include them. There was a problem hiding this comment. mm, maybe adding something that isn't supported yet would just add confusion then? There was a problem hiding this comment. Maybe - I suppose I didn't update the JavaDoc for this to include these additional parameters (I'll do that now!), but to your point, it'd be confusing to see it documented but not usable. I'll pull out the non-default properties. There was a problem hiding this comment. Actually I do see these properties being sent in test data so maybe I'm wrong. I'll have to dig a little deeper. There was a problem hiding this comment. In the end I ended up with The others ( |
||
| * - $device_type: Device type (Mobile, Tablet, TV, etc.) | ||
| * - $device_manufacturer: Device manufacturer | ||
| * - $device_model: Device model | ||
| * - $locale: User's current locale | ||
| * | ||
| * This helps ensure feature flags that rely on these properties work correctly | ||
| * without waiting for server-side processing of identify() calls. | ||
| * | ||
| * Default: true | ||
| */ | ||
| public var setDefaultPersonProperties: Boolean = true, | ||
| /** | ||
| * Preload PostHog remote config automatically | ||
| * Defaults to true | ||
|
|
@@ -192,8 +211,20 @@ public open class PostHogConfig( | |
| /** | ||
| * Factory to instantiate a custom [com.posthog.internal.PostHogRemoteConfigInterface] implementation. | ||
| */ | ||
| public val remoteConfigProvider: ( | ||
| PostHogConfig, | ||
| PostHogApi, | ||
| ExecutorService, | ||
| (() -> Map<String, Any>)?, | ||
| ) -> PostHogFeatureFlagsInterface = | ||
| { | ||
| config, | ||
| api, | ||
| executor, | ||
| getDefaultPersonProperties, | ||
| -> | ||
| PostHogRemoteConfig(config, api, executor, getDefaultPersonProperties ?: { emptyMap() }) | ||
| }, | ||
| /** | ||
| * Factory to instantiate a custom queue implementation. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -19,13 +19,13 @@ internal class PostHogFlagsRequest( | |
| this["\$anon_distinct_id"] = anonymousId | ||
| } | ||
| if (groups?.isNotEmpty() == true) { | ||
| this["groups"] = groups | ||
|
There was a problem hiding this comment. we have to fix this on iOS as well https://github.com/PostHog/posthog-ios/blob/ba5dd65d2341ab8ea1949f072594ef67c490af3a/PostHog/PostHogApi.swift#L221C14-L233 |
||
| } | ||
| if (personProperties?.isNotEmpty() == true) { | ||
| this["person_properties"] = personProperties | ||
| } | ||
| if (groupProperties?.isNotEmpty() == true) { | ||
| this["group_properties"] = groupProperties | ||
| } | ||
| if (evaluationEnvironments?.isNotEmpty() == true) { | ||
| this["evaluation_environments"] = evaluationEnvironments | ||
|
|
||
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.
after #312
should this be
Map<String, Any?>instead? (Any?)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 nullable keys are needed for local evaluation. In this case, we're always evaluating remotely so we don't need to handle null values.