Skip to content

Paywalls: create new event session when paywall appears#3330

Merged
joshdholtz merged 14 commits into
mainfrom
paywalls-only-track-impression-and-close-once
Oct 24, 2023
Merged

Paywalls: create new event session when paywall appears#3330
joshdholtz merged 14 commits into
mainfrom
paywalls-only-track-impression-and-close-once

Conversation

@joshdholtz

Copy link
Copy Markdown
Member

Motivation

Prevent paywall_impression and paywall_close be recorded multiple times for the same session (which could happen if same PaywallView is displayed multiple times)

Description

  • PaywallView's onAppear will always create/update the session
  • PurchaseHandler
    • trackPaywallImpression() will store the PaywallEvent.Data for cancel and close to use
    • trackPaywallClose() will set eventData to nil so that close can only be called once

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

Looks great! Just a few suggestions.

Comment thread RevenueCatUI/PaywallView.swift Outdated
Comment thread Tests/RevenueCatUITests/Purchasing/PurchaseHandlerTests.swift Outdated
Comment thread Tests/RevenueCatUITests/Purchasing/PurchaseHandlerTests.swift Outdated
}

self.track(.close(data.withCurrentDate()))
self.eventData = nil

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'd add another test to verify that cancelling purchases doesn't reset this (so you can track close after a purchase cancels).

Base automatically changed from paywalls-ios-17-tests to paywalls-purchase-completed-transaction October 24, 2023 00:52
@NachoSoto NachoSoto force-pushed the paywalls-purchase-completed-transaction branch from 9a29ba3 to d7795f3 Compare October 24, 2023 00:53
@NachoSoto NachoSoto changed the title Paywalls: create new even session when paywall appears Paywalls: create new event session when paywall appears Oct 24, 2023
@joshdholtz joshdholtz added the pr:fix A bug fix label Oct 24, 2023
Base automatically changed from paywalls-purchase-completed-transaction to main October 24, 2023 03:43
guard self.session.lastPaywall != newPaywall else { return }

self.session.lastPaywall = newPaywall
self.session.id = .init()

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.

You can remove this now.

NachoSoto and others added 7 commits October 24, 2023 05:02
The current API only provided a `CustomerInfo`.
Note that this does not include `userCanceled`. We can expose separately with another `.onPurchaseCancelled` modifier in the future.
These were failing in some of the earlier Xcode 15 betas, but they all seem to be passing now.
`PresentIfNeededTests` are still not working though. But I just manually verified that these modifiers do work correctly on iOS 17 on an app.
@joshdholtz joshdholtz force-pushed the paywalls-only-track-impression-and-close-once branch from 93e699e to ffbd794 Compare October 24, 2023 10:02
@codecov

codecov Bot commented Oct 24, 2023

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (e778230) 85.95% compared to head (4f6d981) 85.96%.
Report is 6 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3330   +/-   ##
=======================================
  Coverage   85.95%   85.96%           
=======================================
  Files         237      237           
  Lines       17081    17081           
=======================================
+ Hits        14682    14683    +1     
+ Misses       2399     2398    -1     

see 3 files with indirect coverage changes

☔ 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.

Let's just remove the session state property now :)

Comment thread Tests/RevenueCatUITests/Purchasing/PurchaseHandlerTests.swift
@joshdholtz joshdholtz merged commit 32dc5ed into main Oct 24, 2023
@joshdholtz joshdholtz deleted the paywalls-only-track-impression-and-close-once branch October 24, 2023 20:21
This was referenced Oct 25, 2023
NachoSoto added a commit that referenced this pull request Oct 26, 2023
**This is an automatic release.**

### New Features
* `PaywallColor`: change visibility of `Color.init(light:dark:)` to
`private` (#3345) via NachoSoto (@NachoSoto)
### RevenueCatUI
* `Paywalls`: new `.onPurchaseCompleted` overload with
`StoreTransaction` (#3323) via NachoSoto (@NachoSoto)
* `Paywalls`: finished template 5 (#3340) via NachoSoto (@NachoSoto)
* `Paywalls`: new `onDismiss` parameter for `presentPaywallIfNeeded`
(#3342) via NachoSoto (@NachoSoto)
* `Paywalls`: disable shimmering on footer loading view (#3324) via
NachoSoto (@NachoSoto)
### Bugfixes
* `ErrorUtils.purchasesError(withSKError:)`: handle `URLError`s (#3346)
via NachoSoto (@NachoSoto)
### Other Changes
* `Paywalls`: add identifier to events (#3332) via Josh Holtz
(@joshdholtz)
* `Paywalls`: create new event session when paywall appears (#3330) via
Josh Holtz (@joshdholtz)
* `HTTPClient`: verbose logs for request IDs (#3320) via NachoSoto
(@NachoSoto)
* `Paywalls Tester`: fix `macOS` build (#3341) via NachoSoto
(@NachoSoto)
* `ProductFetcherSK1`: enable `TimingUtil` log (#3327) via NachoSoto
(@NachoSoto)
* `Paywall Tester`: fixed paywall presentation (#3339) via NachoSoto
(@NachoSoto)
* `CI`: replace Carthage build jobs with `xcodebuild` (#3338) via
NachoSoto (@NachoSoto)
* `Integration Tests`: use repetition count from test plan (#3329) via
NachoSoto (@NachoSoto)
* `Integration Tests`: new logs for troubleshooting flaky tests (#3328)
via NachoSoto (@NachoSoto)
* `CircleCI`: change iOS 17 job to use M1 Large resource (#3322) via
NachoSoto (@NachoSoto)
* `Paywalls Tester`: fix release build (#3321) via NachoSoto
(@NachoSoto)
* `Paywalls`: enable all iOS 17 tests (#3331) via NachoSoto (@NachoSoto)
* `CI`: added workaround for Snapshots in `Xcode Cloud` (#2857) via
NachoSoto (@NachoSoto)
* `StoreKit 1`: disabled `finishTransactions` log on observer mode
(#3314) via NachoSoto (@NachoSoto)

---------

Co-authored-by: NachoSoto <ignaciosoto90@gmail.com>
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.

2 participants