Skip to content

Paywalls: Store paywall events on disk and API (1)#1436

Merged
tonidero merged 2 commits into
mainfrom
toniricodiez/pwl-321-paywall-events
Nov 6, 2023
Merged

Paywalls: Store paywall events on disk and API (1)#1436
tonidero merged 2 commits into
mainfrom
toniricodiez/pwl-321-paywall-events

Conversation

@tonidero

@tonidero tonidero commented Nov 6, 2023

Copy link
Copy Markdown
Contributor

Description

This adds support for storing paywall events into disk. In followup PRs, we will send these events to our servers for analytics and use these events to indicate whether a purchase came from our paywalls.

  • Send events to server
  • Use events to detect whether a purchase came from paywall.

* Used by `RevenueCatUI` to keep track of [PaywallEvent]s.
*/
@ExperimentalPreviewRevenueCatPurchasesAPI
@JvmSynthetic

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Made this synthetic since this is really only meant for our use in RevenueCatUI which uses Kotlin.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TIL but makes sense.

val dispatcher = Dispatcher(createDefaultExecutor(), runningIntegrationTests)
val backendDispatcher = Dispatcher(service ?: createDefaultExecutor(), runningIntegrationTests)
val diagnosticsDispatcher = Dispatcher(createDiagnosticsExecutor(), runningIntegrationTests)
val eventsDispatcher = Dispatcher(createEventsExecutor(), runningIntegrationTests)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I decided to share the dispatcher between diagnostics and paywall events, so I renamed a few things.

identityManager: IdentityManager,
eventsDispatcher: Dispatcher,
): PaywallEventsManager? {
return if (isAndroidNOrNewer()) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This shouldn't be a problem since revenuecatui already is N+. But since the main SDK isn't and we're using streams, I had to add this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment mentioning that?

/**
* Types of paywall events. Meant for RevenueCatUI use.
*/
@ExperimentalPreviewRevenueCatPurchasesAPI

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I added experimental annotations on these classes for now. I think we can remove them once paywalls is completely final.

data class PaywallEvent(
val creationData: CreationData,
val data: Data,
val type: PaywallEventType,

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I thought about using a sealed class to mimick what we do in iOS... But that was slightly trickier to serialize using kotlin serializers, and I didn't think it was worth the hassle, specially for now where all events have the same data. If we have events with different data in the future, we can revisit this.

}

@Synchronized
fun readFile(streamBlock: ((Stream<PaywallStoredEvent>) -> Unit)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This is currently unused, but will be used in a followup PR. Partially copied from DiagnosticsFileHelper.

@tonidero tonidero changed the title Paywalls: Store paywall events on disk and API Paywalls: Store paywall events on disk and API (1) Nov 6, 2023
@tonidero tonidero marked this pull request as ready for review November 6, 2023 15:09
@tonidero tonidero requested a review from a team November 6, 2023 15:09
@codecov

codecov Bot commented Nov 6, 2023

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 69.86301% with 22 lines in your changes missing coverage. Please review.

Project coverage is 83.99%. Comparing base (0f688aa) to head (61d58a8).
Report is 318 commits behind head on main.

Files with missing lines Patch % Lines
...venuecat/purchases/paywalls/events/PaywallEvent.kt 72.72% 3 Missing and 3 partials ⚠️
...otlin/com/revenuecat/purchases/PurchasesFactory.kt 61.53% 4 Missing and 1 partial ⚠️
...rchases/paywalls/events/PaywallEventsFileHelper.kt 50.00% 5 Missing ⚠️
.../com/revenuecat/purchases/PurchasesOrchestrator.kt 50.00% 2 Missing and 1 partial ⚠️
.../purchases/paywalls/events/PaywallEventsManager.kt 92.30% 1 Missing ⚠️
...at/purchases/paywalls/events/PaywallStoredEvent.kt 66.66% 0 Missing and 1 partial ⚠️
...ecat/purchases/utils/serializers/DateSerializer.kt 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1436      +/-   ##
==========================================
- Coverage   84.13%   83.99%   -0.14%     
==========================================
  Files         197      203       +6     
  Lines        6649     6717      +68     
  Branches      965      975      +10     
==========================================
+ Hits         5594     5642      +48     
- Misses        684      698      +14     
- Partials      371      377       +6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@NachoSoto NachoSoto left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Awesome!

* Used by `RevenueCatUI` to keep track of [PaywallEvent]s.
*/
@ExperimentalPreviewRevenueCatPurchasesAPI
@JvmSynthetic

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

TIL but makes sense.

identityManager: IdentityManager,
eventsDispatcher: Dispatcher,
): PaywallEventsManager? {
return if (isAndroidNOrNewer()) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe add a comment mentioning that?

@Test
fun `can encode paywall event correctly`() {
val eventString = PaywallEventsManager.json.encodeToString(event)
assertThat(eventString).isEqualTo(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I imagine you compared these with iOS 👍🏻

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

These are different than in iOS, but it's the way they are stored in disk, so it shouldn't matter if they are different I think. In iOS, the encoding is pretty different, but all the data is there.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh yeah of course never mind

@tonidero tonidero enabled auto-merge (squash) November 6, 2023 16:46
@tonidero tonidero merged commit 4d5c5bf into main Nov 6, 2023
@tonidero tonidero deleted the toniricodiez/pwl-321-paywall-events branch November 6, 2023 17:02
NachoSoto added a commit that referenced this pull request Nov 8, 2023
**This is an automatic release.**

### RevenueCatUI
* `PaywallActivityLauncher`: new constructor for a generic
`ActivityResultCaller` (#1441) via NachoSoto (@NachoSoto)
* Improve fullscreen templates in landscape orientation (#1435) via Toni
Rico (@tonidero)
* `Paywalls`: improve Japanese localization (#1439) via NachoSoto
(@NachoSoto)
### Other Changes
* Remove side effect from setting purchasesUpdatedListener (#1443) via
Cesar de la Vega (@vegaro)
* Paywalls: Store paywall events on disk and API (1) (#1436) via Toni
Rico (@tonidero)

---------

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
tonidero added a commit that referenced this pull request Nov 10, 2023
### Description
This follows up on #1436. 

Here we are adding the code to send paywall events to our servers. This
is not connected with the work done in #1436 yet. That will be done in
followup PRs.
- [ ] Actually read events from disk and use this to send them to
servers.
- [ ] Use paywall events to detect whether a purchase came from a
paywall.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants