Skip to content
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: load remote config and feature flags #283

Merged
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
@@ -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

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

// 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

}

private func preloadFeatureFlags() {
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

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

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

@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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants