Skip to content

Refactor syncing pending transactions logic out of Purchases#1058

Merged
tonidero merged 5 commits into
mainfrom
toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-2
Jun 20, 2023
Merged

Refactor syncing pending transactions logic out of Purchases#1058
tonidero merged 5 commits into
mainfrom
toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-2

Conversation

@tonidero

@tonidero tonidero commented Jun 8, 2023

Copy link
Copy Markdown
Contributor

Description

SDK-3155

This is another refactor in the process of posting unsynced purchases before doing customer info requests, to make sure the information received from the backend is up to date.

In this PR, we are extracting the logic that syncs purchases on app foreground out of Purchases and added a callback to that process. The idea is to reuse the same logic when trying to fetch customer info first, and if that returns an empty customer info (that is, no unsynced purchases), then we actually fetch customer info from the backend.

@tonidero tonidero left a comment

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.

The diff actually increases a lot, but most of it are tests... As I refactor this, I see some opportunities to test more things and it ends up growing...

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 has been moved to PostTransactionWithProductDetailsHelper

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 has been moved to SyncPendingTransactionsHelper

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.

The callback didn't exist before. Right now it's unused, but my plan is to use it once we want to sync purchases before fetching customer info.

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.

Note how I'm calling the error callback if any of the requests in the sync fails.

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.

Almost to 3k lines 😓. Hopefully we can get this thinner with these refactors.

@codecov

codecov Bot commented Jun 8, 2023

Copy link
Copy Markdown

Codecov Report

Merging #1058 (d3cdbcc) into main (8ae7c65) will increase coverage by 0.08%.
The diff coverage is 89.15%.

❗ Current head d3cdbcc differs from pull request most recent head b5cd813. Consider uploading reports for the commit b5cd813 to get more accurate results

@@            Coverage Diff             @@
##             main    #1058      +/-   ##
==========================================
+ Coverage   85.76%   85.84%   +0.08%     
==========================================
  Files         176      179       +3     
  Lines        6265     6336      +71     
  Branches      864      876      +12     
==========================================
+ Hits         5373     5439      +66     
+ Misses        557      556       -1     
- Partials      335      341       +6     
Impacted Files Coverage Δ
...om/revenuecat/purchases/strings/PurchaseStrings.kt 0.00% <ø> (ø)
...rchases/PostTransactionWithProductDetailsHelper.kt 82.85% <82.85%> (ø)
...enuecat/purchases/PostPendingTransactionsHelper.kt 86.20% <86.20%> (ø)
...revenuecat/purchases/CustomerInfoUpdateReceiver.kt 87.87% <87.87%> (ø)
...lin/com/revenuecat/purchases/CustomerInfoHelper.kt 89.61% <100.00%> (-1.39%) ⬇️
...tlin/com/revenuecat/purchases/PostReceiptHelper.kt 94.11% <100.00%> (-0.07%) ⬇️
.../main/kotlin/com/revenuecat/purchases/Purchases.kt 81.29% <100.00%> (+0.22%) ⬆️
...otlin/com/revenuecat/purchases/PurchasesFactory.kt 86.45% <100.00%> (+2.00%) ⬆️

... and 1 file with indirect coverage changes

@tonidero tonidero marked this pull request as ready for review June 8, 2023 14:52
@tonidero tonidero requested a review from a team June 8, 2023 14:52
@tonidero tonidero force-pushed the toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-1 branch from 2a4f9b1 to a3172f6 Compare June 13, 2023 09:46
@tonidero tonidero force-pushed the toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-2 branch from 89fd8ba to f46db41 Compare June 13, 2023 09:47

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.

Sync might be confusingly mistaken with sync purchases. Maybe PostPendingTransactionsHelper would be better?

@tonidero tonidero force-pushed the toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-1 branch from 53de5f0 to 7a6f2a5 Compare June 19, 2023 08:22
@tonidero tonidero force-pushed the toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-2 branch from f46db41 to d3cdbcc Compare June 19, 2023 08:30

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This test name should specify that autosync is false. So something like "If autosync is disabled, and sync is called, success callback with null value is called`

Base automatically changed from toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-1 to main June 20, 2023 08:18
@tonidero tonidero force-pushed the toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-2 branch from d3cdbcc to 4a44b45 Compare June 20, 2023 08:20
@tonidero tonidero enabled auto-merge (squash) June 20, 2023 08:25
@tonidero tonidero merged commit 6edb303 into main Jun 20, 2023
@tonidero tonidero deleted the toniricodiez/sdk-3155-android-if-sdk-detects-unsynced-purchases-when-calling-2 branch June 20, 2023 08:32
tonidero added a commit that referenced this pull request Jun 20, 2023
### Description
Followup to #1052 and #1058 

This changes the behavior of fetching customer info. Now, when fetching
customer info, we will first try to sync any pending purchases if any.
If there are pending purchases, we will return the customer info from
the last request sent. If there aren't, we will then hit the current
customer info endpoint to get the latest customer info.
tonidero pushed a commit that referenced this pull request Jun 23, 2023
**This is an automatic release.**

### Bugfixes
* Default customer info schema version to latest known by SDK (#1080)
via Toni Rico (@tonidero)
* Handle other diagnostics-related exceptions (#1076) via Toni Rico
(@tonidero)
* Return error in queryPurchases if error connecting to billing client
(#1072) via Toni Rico (@tonidero)
### Other Changes
* Fix offline entitlements integration tests (#1085) via Toni Rico
(@tonidero)
* Add defaultsRelease variant tests run configuration (#1074) via Toni
Rico (@tonidero)
* Compose sample app: move to gradle catalog (#1081) via Toni Rico
(@tonidero)
* Compose sample app: automate builds (#1082) via Toni Rico (@tonidero)
* Compose sample app (#1056) via Toni Rico (@tonidero)
* Migrate to Gradle version catalog (#1059) via Cesar de la Vega
(@vegaro)
* Trusted entitlements: Add logs with verification mode (#1067) via Toni
Rico (@tonidero)
* Sync pending purchases before getting customer info (#1073) via Toni
Rico (@tonidero)
* Refactor syncing pending transactions logic out of `Purchases` (#1058)
via Toni Rico (@tonidero)
* Refactor CustomerInfo listener and cache logic into
CustomerInfoUpdater (#1052) via Toni Rico (@tonidero)
* Trusted entitlements: Add integration tests (#1071) via Toni Rico
(@tonidero)
* Trusted entitlements: Add internal mechanism to force signing errors
for tests (#1070) via Toni Rico (@tonidero)

Co-authored-by: revenuecat-ops <ops@revenuecat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants