Extract post receipt logic to PostReceiptHelper#967
Conversation
There was a problem hiding this comment.
how does the subs attrs manager know which ones we did sync in this post?
I don't see any locking, so in theory you could have added unsynced attrs between calling this method and getting the response back, and we're only sending in the unsynced ones. So could the subs attrs manager be marking new attributes as synced when they were added after line 110 executed but before this completion got called?
There was a problem hiding this comment.
No, note how on line 110 we get the unsyncedSubscriberAttributesByKey and those are the ones we markAsSynced. So if more attributes are added in between, they will be sent on the next opportunity. Lmk if you meant something else!
There was a problem hiding this comment.
ohhh man look at all that code finally leaving Purchases.kt 🙌
eeda66f to
3655550
Compare
There was a problem hiding this comment.
All these tests have been kind of moved to PostReceiptHelperTest. I tried to keep all the logic still tested the same as before, but it was easier to simply rewrite many of the tests.
There was a problem hiding this comment.
All the tests related to syncing attributes on post receipts have also been moved to PostReceiptHelperTest.
There was a problem hiding this comment.
A few tests here are testing logic that now lives within postReceiptHelper, so I removed these tests from here.
There was a problem hiding this comment.
This whole region of tests was removed (the side-by-side view probably won't help much here)
| allPurchases.sortedBy { it.purchaseTime }.let { sortedByTime -> | ||
| sortedByTime.forEach { purchase -> | ||
| subscriberAttributesManager.getUnsyncedSubscriberAttributes(appUserID) { attrsByKey -> | ||
| val receiptInfo = ReceiptInfo(productIDs = purchase.productIds) |
There was a problem hiding this comment.
Note this small difference on how before we were creating a ReceiptInfo with only productIDs here, but now we will be creating one with a presentedOfferingIdentifier and subscriptionOptionId from the StoreTransaction. However, since these transactions come from queryAllPurchases, those 2 fields will be null, so the behavior should remain the same.
|
Sorry for the huge diff :(. Most of it is on the tests since we've moved some logic that was tested in several places before. I've tested the main flows, making sure attributes are synced in all 3 situations (purchases, syncs, restores) and purchases go through... still this was a tricky refactor so lmk if you have any concerns! |
| * This method will post a token and receiptInfo to the backend without consuming any purchases. | ||
| * It will store that the token was sent to the backend so it doesn't send it again. | ||
| */ | ||
| fun postTokenWithoutConsuming( |
| /** | ||
| * This method will post a StoreTransaction and optionally a StoreProduct info to the backend. | ||
| * It will consume the purchase if finishTransactions is true. | ||
| */ |
There was a problem hiding this comment.
should we make clear this one doesn't save the token to the cache?
There was a problem hiding this comment.
It also does. It's part of the consumeAndSave function. Only difference is this method consumes it first
| @After | ||
| fun tearDown() { | ||
| postReceiptError = null | ||
| postReceiptCompletion = null |
There was a problem hiding this comment.
I noticed there was a ton of unused code here after I removed the tests (since they were moved). So took the opportunity to remove it.
**This is an automatic release.** ### New Features * CAT-859 Expose whether or not a SubscriptionOption is Prepaid in the SDK (#1005) via Deema AlShamaa (@dalshamaa) ### Bugfixes * [CF-1324] Fix personalizedPrice defaulting to false (#952) via beylmk (@beylmk) ### Performance Improvements * Store and return ETag last refresh time header (#978) via Toni Rico (@tonidero) ### Dependency Updates * Bump fastlane-plugin-revenuecat_internal from `3b03efa` to `fe45299` (#991) via dependabot[bot] (@dependabot[bot]) * Bump danger from 9.2.0 to 9.3.0 (#981) via dependabot[bot] (@dependabot[bot]) * Bump fastlane-plugin-revenuecat_internal from `8482a43` to `3b03efa` (#974) via dependabot[bot] (@dependabot[bot]) * Bump fastlane from 2.212.1 to 2.212.2 (#973) via dependabot[bot] (@dependabot[bot]) * Bump fastlane-plugin-revenuecat_internal from `9255366` to `8482a43` (#961) via dependabot[bot] (@dependabot[bot]) ### Other Changes * Add proration modes to post to backend (#977) via swehner (@swehner) * Added ENTITLEMENTS_COMPUTED_ON_DEVICE (#939) via Cesar de la Vega (@vegaro) * Fix flaky test in OfflineCustomerInfoCalculatorTest (#997) via Cesar de la Vega (@vegaro) * Fix `OfflineCustomerInfoCalculatorTest` `Unresolved reference: ProducType` (#995) via Cesar de la Vega (@vegaro) * Add support for product_plan_identifier for offline customer info (#959) via Cesar de la Vega (@vegaro) * Add non-subscriptions support to offline customer info (#958) via Cesar de la Vega (@vegaro) * Query only active purchases when generating offline entitlements customer info (#1003) via Toni Rico (@tonidero) * Fix `PurchasesIntegrationTest` building issue (#996 into main) (#998) via Cesar de la Vega (@vegaro) * Fail offline entitlements computation if product entitlement mapping not available (#999) via Toni Rico (@tonidero) * Fix build_magic_weather lane (#993) via Cesar de la Vega (@vegaro) * Add backend integration tests and test product entitlement mapping endpoint (#988) via Toni Rico (@tonidero) * Fix purchases integration tests (#980) via Toni Rico (@tonidero) * Disable offline entitlements if active inapp purchases exist (#983) via Toni Rico (@tonidero) * Clear cached customer info upon entering offline entitlements mode (#989) via Toni Rico (@tonidero) * Update product entitlement mapping request to new format (#976) via Toni Rico (@tonidero) * Support enabling/disabling offline entitlements (#964) via Toni Rico (@tonidero) * Add back integration tests automation (#972) via Toni Rico (@tonidero) * Upgrade to AGP 8.0 (#975) via Toni Rico (@tonidero) * Extract post receipt logic to PostReceiptHelper (#967) via Toni Rico (@tonidero) * Add isServerDown to error callback for postReceipt and getCustomerInfo requests (#963) via Toni Rico (@tonidero) * Add back integration test flavors (#962) via Toni Rico (@tonidero) * Fix storing test results (#966) via Cesar de la Vega (@vegaro) * Extract detekt job from test job (#965) via Cesar de la Vega (@vegaro) [CF-1324]: https://revenuecats.atlassian.net/browse/CF-1324?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: revenuecat-ops <ops@revenuecat.com> Co-authored-by: Toni Rico <antonio.rico.diez@revenuecat.com>
Description
This became a complex refactor, specially on the tests... Basically, we currently have 3 places where we post receipts to the backend: restores, sync purchases and purchases. All this logic lived in
Purchasesand part of it was duplicated. In this refactor, I'm extracting this logic from all 3 places to a newPostReceiptHelperso we can share the common logic between them.The main code itself wasn't too bad to refactor, but the tests were very painful to untangle, since we had them in different places.
The purpose of doing this now is so we can share the logic for offline entitlements in #964 and also some very needed cleanup of
Purchases.TODO