feat: add Purchasely SDK mode setting (full/paywallObserver) with restart#1
feat: add Purchasely SDK mode setting (full/paywallObserver) with restart#1kherembourg wants to merge 2 commits intomainfrom
Conversation
Summary of ChangesHello @kherembourg, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant enhancement to the Shaker application by enabling users to control the Purchasely SDK's running mode directly from the settings. This cross-platform feature ensures that the chosen mode, either 'Paywall Observer' or 'Full', is not only persisted across app sessions but also correctly applied during SDK initialization. The implementation includes a robust mechanism to handle SDK restarts upon mode changes, guiding users through the necessary app relaunch for a seamless experience. This change greatly improves the flexibility and testability of the Purchasely integration. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
The pull request introduces a new feature allowing users to toggle the Purchasely SDK running mode between 'Paywall Observer' and 'Full' via the app's settings. This change is implemented across both Android and iOS platforms, ensuring persistence of the selected mode and requiring an SDK restart with user notification upon mode change. The README.md and manual-test-matrix.md files have been updated to reflect these changes and provide necessary documentation. The implementation looks solid, with good separation of concerns for the SDK mode logic and UI components.
| | Recipe detail paywall | `fetchPresentation` + `display` | `fetchPresentation` + `display` | `recipe_detail` | | ||
| | Favorites paywall | `fetchPresentation` + `display` | `fetchPresentation` + `display` | `favorites` | | ||
| | Filters paywall | `fetchPresentation` + `display` | `fetchPresentation` + `display` | `filters` | |
There was a problem hiding this comment.
The README.md update correctly reflects the change from presentationView and presentationController to fetchPresentation + display for paywall flows. This is a good clarification for anyone setting up the project.
| - You can override it with local config files: | ||
| - Android: `android/local.properties` (`purchasely.apiKey=...`) | ||
| - iOS: `ios/Config.xcconfig` (`PURCHASELY_API_KEY = ...`) | ||
| - Paywall flows require `fetchPresentation + display()`. Do not use convenience APIs like `presentationView` or `presentationController`. |
There was a problem hiding this comment.
This new constraint is important for developers integrating the SDK. It clearly states the recommended approach for paywall flows, avoiding potential misuse of convenience APIs.
| "PURCHASELY_API_KEY", | ||
| "\"6cda6b92-d63c-4444-bd55-5a164c989bd4\"" | ||
| //"\"${localProperties.getProperty("purchasely.apiKey", "")}\"" | ||
| "\"${localProperties.getProperty("purchasely.apiKey", "6cda6b92-d63c-4444-bd55-5a164c989bd4")}\"" |
There was a problem hiding this comment.
The change to use localProperties.getProperty with a default value is a good improvement. It allows for easier local configuration while providing a fallback API key for quick startup, as mentioned in the README.md.
"${localProperties.getProperty("purchasely.apiKey", "6cda6b92-d63c-4444-bd55-5a164c989bd4")}"| import io.purchasely.ext.LogLevel | ||
| import io.purchasely.ext.PLYEvent | ||
| import io.purchasely.ext.PLYPresentationAction | ||
| import io.purchasely.ext.PLYRunningMode |
There was a problem hiding this comment.
The import for PLYRunningMode is no longer needed in ShakerApp.kt as PurchaselySdkMode now encapsulates this. Removing unused imports helps keep the code clean.
| .start { isConfigured, error -> | ||
| if (isConfigured) { | ||
| Log.d(TAG, "[Shaker] Purchasely SDK configured successfully") | ||
| Log.d(TAG, "[Shaker] Purchasely SDK configured successfully (mode: ${selectedMode.label})") |
There was a problem hiding this comment.
Updating the log message to include the selected SDK mode provides more context during debugging and startup, which is very helpful.
| Log.d(TAG, "[Shaker] Purchasely SDK configured successfully (mode: ${selectedMode.label})") | |
| Log.d(TAG, "[Shaker] Purchasely SDK configured successfully (mode: ${selectedMode.label})") |
| .alert("SDK Restart Required", isPresented: .init( | ||
| get: { viewModel.sdkModeRestartMessage != nil }, | ||
| set: { if !$0 { viewModel.clearSdkModeRestartMessage() } } | ||
| )) { | ||
| Button("OK") { viewModel.clearSdkModeRestartMessage() } | ||
| } message: { | ||
| Text(viewModel.sdkModeRestartMessage ?? "") | ||
| } |
There was a problem hiding this comment.
The alert for SDK restart in iOS is well-implemented, providing clear communication to the user about the required action after changing the SDK mode.
| .alert("SDK Restart Required", isPresented: .init( | |
| get: { viewModel.sdkModeRestartMessage != nil }, | |
| set: { if !$0 { viewModel.clearSdkModeRestartMessage() } } | |
| )) { | |
| Button("OK") { viewModel.clearSdkModeRestartMessage() } | |
| } message: { | |
| Text(viewModel.sdkModeRestartMessage ?? "") | |
| } | |
| .alert("SDK Restart Required", isPresented: .init( | |
| get: { viewModel.sdkModeRestartMessage != nil }, | |
| set: { if !$0 { viewModel.clearSdkModeRestartMessage() } } | |
| )) { | |
| Button("OK") { viewModel.clearSdkModeRestartMessage() } | |
| } message: { | |
| Text(viewModel.sdkModeRestartMessage ?? "") | |
| } |
| @Published var sdkMode: PurchaselySDKMode | ||
| @Published var sdkModeRestartMessage: String? |
There was a problem hiding this comment.
Adding sdkMode and sdkModeRestartMessage as @Published properties ensures that the UI can react to changes in these values, which is essential for the new SDK mode setting feature.
| @Published var sdkMode: PurchaselySDKMode | |
| @Published var sdkModeRestartMessage: String? | |
| @Published var sdkMode: PurchaselySDKMode | |
| @Published var sdkModeRestartMessage: String? |
| sdkMode = PurchaselySDKMode.current() | ||
| sdkModeRestartMessage = nil |
There was a problem hiding this comment.
Initializing sdkMode from PurchaselySDKMode.current() and setting sdkModeRestartMessage to nil on initialization ensures the correct state is loaded when the view model is created.
| sdkMode = PurchaselySDKMode.current() | |
| sdkModeRestartMessage = nil | |
| sdkMode = PurchaselySDKMode.current() | |
| sdkModeRestartMessage = nil |
| func setSdkMode(_ mode: PurchaselySDKMode) { | ||
| guard sdkMode != mode else { return } | ||
|
|
||
| sdkMode = mode | ||
| mode.persist() | ||
| NotificationCenter.default.post(name: .purchaselySdkModeDidChange, object: nil) | ||
| sdkModeRestartMessage = | ||
| "Purchasely SDK switched to \(mode.title). Please kill and relaunch the app." | ||
| print("[Shaker] SDK mode updated to \(mode.rawValue)") |
There was a problem hiding this comment.
The setSdkMode function correctly updates the sdkMode, persists it, posts a notification for the AppViewModel to handle the SDK restart, and sets the restart message for the UI. This is a well-encapsulated piece of logic.
| func setSdkMode(_ mode: PurchaselySDKMode) { | |
| guard sdkMode != mode else { return } | |
| sdkMode = mode | |
| mode.persist() | |
| NotificationCenter.default.post(name: .purchaselySdkModeDidChange, object: nil) | |
| sdkModeRestartMessage = | |
| "Purchasely SDK switched to \(mode.title). Please kill and relaunch the app." | |
| print("[Shaker] SDK mode updated to \(mode.rawValue)") | |
| func setSdkMode(_ mode: PurchaselySDKMode) { | |
| guard sdkMode != mode else { return } | |
| sdkMode = mode | |
| mode.persist() | |
| NotificationCenter.default.post(name: .purchaselySdkModeDidChange, object: nil) | |
| sdkModeRestartMessage = | |
| "Purchasely SDK switched to \(mode.title). Please kill and relaunch the app." | |
| print("[Shaker] SDK mode updated to \(mode.rawValue)") | |
| } |
| func clearSdkModeRestartMessage() { | ||
| sdkModeRestartMessage = nil | ||
| } |
There was a problem hiding this comment.
The clearSdkModeRestartMessage function is necessary to dismiss the alert after the user acknowledges it, providing a clean UI state.
| func clearSdkModeRestartMessage() { | |
| sdkModeRestartMessage = nil | |
| } | |
| func clearSdkModeRestartMessage() { | |
| sdkModeRestartMessage = nil | |
| } |
1 participant
Summary
Paywall Observer/Full)paywallObserverKey files
android/app/src/main/java/com/purchasely/shaker/data/PurchaselySdkMode.ktandroid/app/src/main/java/com/purchasely/shaker/ShakerApp.ktandroid/app/src/main/java/com/purchasely/shaker/ui/screen/settings/SettingsViewModel.ktandroid/app/src/main/java/com/purchasely/shaker/ui/screen/settings/SettingsScreen.ktios/Shaker/AppViewModel.swiftios/Shaker/Screens/Settings/SettingsViewModel.swiftios/Shaker/Screens/Settings/SettingsScreen.swiftdocs/solutions/integration-issues/sdk-running-mode-settings-toggle.mdValidation
./gradlew :app:assembleDebug(success)xcodebuild build -workspace Shaker.xcworkspace -scheme Shaker CODE_SIGNING_ALLOWED=NO(success)