Paywalls: Move logic for events file helper to common generic class (4)#1446
Conversation
There was a problem hiding this comment.
Had to make this nullable since for diagnostics we don't parse the file back to DiagnosticEntry objects, as we do for PaywallStoredEvent. So had to add another read method that reads the file as JSONObject.
The alternative was to make diagnostics use kotlin serialization, but as I started on that, I noticed I would have to rewrite a lot of things and didn't want to spend a lot of time there.
There was a problem hiding this comment.
Maybe
| private val stringToEventConverter: ((String) -> T)? = null, | |
| private val eventDeserializer: ((String) -> T)? = null, |
And I would leave a comment for how this behaves if it's null.
There was a problem hiding this comment.
This is the only method remaining here. I want to finish paywall events before dedicating more time to rewriting the whole thing for diagnostics.
e7607fe to
cf6223f
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1446 +/- ##
==========================================
+ Coverage 83.62% 84.03% +0.40%
==========================================
Files 206 206
Lines 6811 6848 +37
Branches 989 993 +4
==========================================
+ Hits 5696 5755 +59
+ Misses 726 706 -20
+ Partials 389 387 -2
☔ View full report in Codecov by Sentry. |
vegaro
left a comment
There was a problem hiding this comment.
I think this makes sense as a first refactor
There was a problem hiding this comment.
This is public?
Also I'd add a docstring
There was a problem hiding this comment.
Maybe
| private val stringToEventConverter: ((String) -> T)? = null, | |
| private val eventDeserializer: ((String) -> T)? = null, |
And I would leave a comment for how this behaves if it's null.
6dbfdc1 to
795838c
Compare
643fa77 to
af4c529
Compare
**This is an automatic release.** ### RevenueCatUI * `Paywalls`: improve error log when images fail to load (#1454) via NachoSoto (@NachoSoto) ### Other Changes * Paywall events: Send paywall data with post receipt requests (#1452) via Toni Rico (@tonidero) * Paywalls: Track paywall events (#1447) via Toni Rico (@tonidero) * Paywall events: Handle errors parsing specific paywall event lines (#1451) via Toni Rico (@tonidero) * Paywalls: Move logic for events file helper to common generic class (4) (#1446) via Toni Rico (@tonidero) * Paywalls: Add paywall events flush logic and tests (3) (#1445) via Toni Rico (@tonidero) * Paywalls: Support sending paywall events to servers (2) (#1442) via Toni Rico (@tonidero) * `CircleCI`: fix `record-revenuecatui-snapshots` (#1455) via NachoSoto (@NachoSoto) * Lower request jitter log level from warning to debug (#1453) via Toni Rico (@tonidero) Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Description
I was working on sharing logic between diagnostics and paywall events, but there were quite a few differences that made this tricky. Diagnostics doesn't use kotlin serialization, it anonymizes the events, and has a bunch of other logic that we need to remove to be able to share most of the code with paywall events.
In this first PR, I abstracted most of the logic of
PaywallEventsFileHelperandDiagnosticsFileHelperto a commonEventsFileHelper.