8000 feat: load remote config and feature flags by ioannisj · Pull Request #283 · PostHog/posthog-ios · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

feat: load remote config and feature flags #283

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Jan 23, 2025
Merged

feat: load remote config and feature flags #283

merged 5 commits into from
Jan 23, 2025

Conversation

ioannisj
Copy link
Contributor

💡 Motivation and Context

Building the foundation for mobile surveys. This is one part of many on the feature branch feat/mobile-surveys.

This adds remote config loading and caching on SDK

  • Added remote config loading and caching on startup
  • Refactored a bit feature flags loading to take into account remote config value hasFeatureFlags. When this value is false, we can skip loading feature flags cause the team doesn't have any flags enabled.
  • On new remote config load, if hasFeatureFlags is true we make sure that flags are reloaded

#skip-changelog

💚 How did you test it?

Modified and added some unit tests

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • I updated the docs if needed.
  • No breaking change or entry added to the changelog.

@ioannisj ioannisj requested a review from marandaneto as a code owner January 21, 2025 08:37
@ioannisj ioannisj requested a review from a team January 21, 2025 08:37
ioannisj
ioannisj commented Jan 21, 2025
View reviewed changes
PostHog/PostHogApi.swift
@@ -217,4 +237,45 @@ class PostHogApi {
}
}.resume()
}

func remoteConfig(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call to new remote config endpoint

ioannisj
ioannisj commented Jan 21, 2025
View reviewed changes
PostHog/PostHogConfig.swift
Comment on lines +64 to +69
#if os(iOS)
/// Enable mobile surveys
/// Experimental support
/// Default: false
@objc public var surveysEnabled: Bool = false
#endif
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No functionality for now, just setting up the public api

ioannisj
ioannisj commented Jan 21, 2025
View reviewed changes
PostHog/PostHogRemoteConfig.swift
// PostHog
//
// Created by Manoel Aranda Neto on 10.10.23.
//

import Foundation

class PostHogFeatureFlags {
class PostHogRemoteConfig {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grouped config and feature flags in a single object here

ioannisj
ioannisj commented Jan 21, 2025
View reviewed changes
PostHog/PostHogRemoteConfig.swift
}

private func preloadFeatureFlags() { 10000
featureFlagsLock.withLock {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about this. I think we may need to move it in config.preloadFeatureFlags check below

ioannisj
ioannisj commented Jan 21, 2025
View reviewed changes
PostHog/PostHogRemoteConfig.swift
let cachedFeatureFlags = featureFlagsLock.withLock { self.featureFlags }
let hasFeatureFlags = remoteConfig["hasFeatureFlags"] as? Bool == true
if hasFeatureFlags, cachedFeatureFlags == nil {
reloadFeatureFlags()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where we reload feature flags in case hasFeatureFlags is true, but flags are missing

marandaneto
marandaneto reviewed Jan 21, 2025
View reviewed changes
PostHog/PostHogApi.swift
return nil
}

let url = baseUrl.appendingPathComponent("/array/\(config.apiKey)/config")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should fix #274 otherwise its one more place that this will fail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, I'll follow up with another PR to address that part

marandaneto
marandaneto reviewed Jan 21, 2025
View reviewed changes
marandaneto
marandaneto reviewed Jan 21, 2025
View reviewed changes
@marandaneto marandaneto mentioned this pull request Jan 22, 2025
Closed
@ioannisj ioannisj merged commit c4a16dc into feat/mobile-surveys Jan 23, 2025
6 checks passed
@ioannisj ioannisj deleted the feat/mobile-surveys-remote-config branch January 23, 2025 08:33
ioannisj added a commit that referenced this pull request Apr 9, 2025
@ioannisj
feat: add support for surveys on iOS (#325)
afd37c6 
* feat: load remote config and feature flags (#283)

* feat: load remote config and feature flags

* fix: cleanup

* fix: redundant assignment

* fix: hasFeatureFlags

* fix: lint

* feat: add survey domain models (#284)

* feat: add survey domain models

* fix: reuse date formatter

* fix: improve test fixtures and model consistency

* Merge branch 'main' into feat/mobile-surveys-merge-main (#313)

* chore: session replay ga (#286)

* fix: xcframework builds (#288)

* fix: xcframework builds

* fix: import access level

* feat: add sample xcframework project

* fix: build examples

* chore: update CHANGELOG

* Update version

* chore: skip creating a visionOS simulator if not needed (#289)

* chore: skip creating a visionOS simulator if not needed

* fix: external sdk example

* Update version

* feat: add survey domain models

* Merge branch 'main' into feat/mobile-surveys-merge-main

# Conflicts:
#	CHANGELOG.md
#	Makefile
#	PostHog.podspec
#	PostHog.xcodeproj/project.pbxproj
#	PostHog/PostHogApi.swift
#	PostHog/PostHogSDK.swift
#	PostHog/PostHogVersion.swift
#	PostHogTests/PostHogFeatureFlagsTest.swift
#	PostHogTests/PostHogSessionManagerTest.swift
#	PostHogTests/TestUtils/MockApplicationLifecyclePublisher.swift
#	PostHogTests/TestUtils/TestPostHog.swift

* fix: lint

* feat: add base survey integration (#314)

* feat: add survey domain models

* Merge branch 'main' into feat/mobile-surveys-merge-main

# Conflicts:
#	CHANGELOG.md
#	Makefile
#	PostHog.podspec
#	PostHog.xcodeproj/project.pbxproj
#	PostHog/PostHogApi.swift
#	PostHog/PostHogSDK.swift
#	PostHog/PostHogVersion.swift
#	PostHogTests/PostHogFeatureFlagsTest.swift
#	PostHogTests/PostHogSessionManagerTest.swift
#	PostHogTests/TestUtils/MockApplicationLifecyclePublisher.swift
#	PostHogTests/TestUtils/TestPostHog.swift

* feat: base surveys integration

* feat: base survey display

* fix: builds

* fix: test

* fix: tests

* fix: remote config tests

* feat: add survey UI and display logic (#320)

* feat: add survey domain models

* Merge branch 'main' into feat/mobile-surveys-merge-main

# Conflicts:
#	CHANGELOG.md
#	Makefile
#	PostHog.podspec
#	PostHog.xcodeproj/project.pbxproj
#	PostHog/PostHogApi.swift
#	PostHog/PostHogSDK.swift
#	PostHog/PostHogVersion.swift
#	PostHogTests/PostHogFeatureFlagsTest.swift
#	PostHogTests/PostHogSessionManagerTest.swift
#	PostHogTests/TestUtils/MockApplicationLifecyclePublisher.swift
#	PostHogTests/TestUtils/TestPostHog.swift

* feat: base surveys integration

* feat: base survey display

* fix: builds

* fix: test

* fix: tests

* fix: remote config tests

* feat: add survey ui components

* feat: survey question type display and next question logic

* feat: send survey seen, sent, dismissed events

* feat: add event-activated surveys

* fix: empty question description

* fix: hit test on iOS 18

* fix: safe area inset

* refactor: move display related to logic to display manager

* fix: handle case displayThankYouMessage is false

* fix: tests

* fix: handle optional questions

* fix: build

* feat: public api

* chore: update CHANGELOG

* feat: iOS15 presentation detents (#324)

* feat: add survey domain models

* Merge branch 'main' into feat/mobile-surveys-merge-main

# Conflicts:
#	CHANGELOG.md
#	Makefile
#	PostHog.podspec
#	PostHog.xcodeproj/project.pbxproj
#	PostHog/PostHogApi.swift
#	PostHog/PostHogSDK.swift
#	PostHog/PostHogVersion.swift
#	PostHogTests/PostHogFeatureFlagsTest.swift
#	PostHogTests/PostHogSessionManagerTest.swift
#	PostHogTests/TestUtils/MockApplicationLifecyclePublisher.swift
#	PostHogTests/TestUtils/TestPostHog.swift

* feat: base surveys integration

* feat: base survey display

* fix: builds

* fix: test

* fix: tests

* fix: remote config tests

* feat: add survey ui components

* feat: survey question type display and next question logic

* feat: send survey seen, sent, dismissed events

* feat: add event-activated surveys

* fix: empty question description

* fix: hit test on iOS 18

* fix: safe area inset

* refactor: move display related to logic to display manager

* fix: handle case displayThankYouMessage is false

* fix: tests

* feat: add backport for iOS 15 presentation detents

* fix: lint

* fix: sheet height

* fix: remove ViewBuilder attribute

* feat: add configuration option for remoteConfig

* chore: prefix survey models

* fix: cache needed integrations in sdk instance

* chore: split survey models to multiple files

* chore: refactor remote config callback
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marandaneto marandaneto marandaneto approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ioannisj @marandaneto
0