-
Notifications
You must be signed in to change notification settings - Fork 10
Add onboarding analytics support #391
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 |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import com.superwall.sdk.config.models.SurveyOption | |
| import com.superwall.sdk.models.customer.CustomerInfo | ||
| import com.superwall.sdk.models.triggers.TriggerResult | ||
| import com.superwall.sdk.paywall.presentation.PaywallInfo | ||
| import com.superwall.sdk.paywall.view.webview.messaging.PageViewData | ||
| import com.superwall.sdk.paywall.presentation.internal.PaywallPresentationRequestStatus | ||
| import com.superwall.sdk.paywall.presentation.internal.PaywallPresentationRequestStatusReason | ||
| import com.superwall.sdk.paywall.view.webview.WebviewError | ||
|
|
@@ -138,6 +139,15 @@ sealed class SuperwallEvent { | |
| get() = "paywall_open" | ||
| } | ||
|
|
||
| // / When a page view occurs in a multi-page paywall. | ||
| data class PaywallPageView( | ||
| val paywallInfo: PaywallInfo, | ||
| val data: PageViewData, | ||
| ) : SuperwallEvent() { | ||
| override val rawName: String | ||
| get() = SuperwallEvents.PaywallPageView.rawName | ||
| } | ||
|
Comment on lines
+143
to
+149
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.
Consider moving Prompt To Fix With AIThis is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/analytics/superwall/SuperwallEvent.kt
Line: 143-149
Comment:
**`PageViewData` is a public API type from an internal package**
`SuperwallEvent.PaywallPageView` is part of the SDK's public surface, but its `data` parameter is typed as `PageViewData` from `com.superwall.sdk.paywall.view.webview.messaging` — an implementation-detail package. SDK consumers using this event via `SuperwallDelegate` will need to import from the webview messaging internals.
Consider moving `PageViewData` to a more appropriate location (e.g. alongside `PaywallInfo` in `paywall.presentation`) or to a dedicated model package so the public API doesn't expose an internal layer.
How can I resolve this? If you propose a fix, please make it concise. |
||
|
|
||
| // / When a paywall is closed. | ||
| data class PaywallClose( | ||
| val paywallInfo: PaywallInfo, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| package com.superwall.sdk.paywall.view.webview.messaging | ||
|
|
||
| data class PageViewData( | ||
| val pageNodeId: String, | ||
| val flowPosition: Int, | ||
| val pageName: String, | ||
| val navigationNodeId: String, | ||
| val previousPageNodeId: String?, | ||
| val previousFlowPosition: Int?, | ||
| val navigationType: String, | ||
| val timeOnPreviousPageMs: Int?, | ||
| ) |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -11,6 +11,7 @@ import kotlinx.serialization.json.Json | |||||
| import kotlinx.serialization.json.JsonObject | ||||||
| import kotlinx.serialization.json.booleanOrNull | ||||||
| import kotlinx.serialization.json.contentOrNull | ||||||
| import kotlinx.serialization.json.int | ||||||
| import kotlinx.serialization.json.intOrNull | ||||||
| import kotlinx.serialization.json.jsonArray | ||||||
| import kotlinx.serialization.json.jsonObject | ||||||
|
|
@@ -129,6 +130,10 @@ sealed class PaywallMessage { | |||||
| val variables: Map<String, Any>?, | ||||||
| ) : PaywallMessage() | ||||||
|
|
||||||
| data class PageView( | ||||||
| val data: PageViewData, | ||||||
| ) : PaywallMessage() | ||||||
|
|
||||||
| data class HapticFeedback( | ||||||
| val hapticType: HapticType, | ||||||
| ) : PaywallMessage() { | ||||||
|
|
@@ -256,6 +261,20 @@ private fun parsePaywallMessage(json: JsonObject): PaywallMessage { | |||||
| ) | ||||||
| } | ||||||
|
|
||||||
| "page_view" -> | ||||||
| PaywallMessage.PageView( | ||||||
| PageViewData( | ||||||
| pageNodeId = json["page_node_id"]!!.jsonPrimitive.content, | ||||||
| flowPosition = json["flow_position"]!!.jsonPrimitive.int, | ||||||
| pageName = json["page_name"]!!.jsonPrimitive.content, | ||||||
| navigationNodeId = json["navigation_node_id"]!!.jsonPrimitive.content, | ||||||
| previousPageNodeId = json["previous_page_node_id"]?.jsonPrimitive?.contentOrNull, | ||||||
| previousFlowPosition = json["previous_flow_position"]?.jsonPrimitive?.intOrNull, | ||||||
| navigationType = json["type"]!!.jsonPrimitive.content, | ||||||
|
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.
Every other field in If the upstream JS contract really sends
Suggested change
Prompt To Fix With AIThis is a comment left during a code review.
Path: superwall/src/main/java/com/superwall/sdk/paywall/view/webview/messaging/PaywallMessage.kt
Line: 273
Comment:
**JSON key `"type"` is inconsistent with field name `navigationType`**
Every other field in `PageViewData` maps directly from a snake_case JSON key that matches its Kotlin name (e.g. `navigation_node_id` → `navigationNodeId`), but `navigationType` maps from the ambiguous generic key `"type"`. This deviates from the established convention and is easy to misread.
If the upstream JS contract really sends `"type"`, consider documenting this explicitly, or (if the contract can be changed) align it to `"navigation_type"`.
```suggestion
navigationType = json["navigation_type"]!!.jsonPrimitive.content,
```
How can I resolve this? If you propose a fix, please make it concise. |
||||||
| timeOnPreviousPageMs = json["time_on_previous_page_ms"]?.jsonPrimitive?.intOrNull, | ||||||
| ), | ||||||
| ) | ||||||
|
|
||||||
| "haptic_feedback" -> { | ||||||
| val style = | ||||||
| json["haptic_type"]?.jsonPrimitive?.contentOrNull?.let { | ||||||
|
|
||||||
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.
PageViewDatais inserted between twoanalytics.superwallimports, splitting what should be a contiguous alphabetical group. This will likely failktlint. The same issue occurs inSuperwallEvent.kt(lines 8-9), wherepaywall.view.webview.messaging.PageViewDataprecedespaywall.presentation.internal.*imports out of alphabetical order.In
SuperwallEvent.kt, move thePageViewDataimport after thePaywallInfoimport and beforePaywallPresentationRequestStatusto maintain alphabetical ordering within thepaywall.*group.Prompt To Fix With AI