-
Notifications
You must be signed in to change notification settings - Fork 333
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PurchasesOrchestrator
: return early if receipt has no transactions when checking for promo offers
#3123
Conversation
…when checking for promo offers
@@ -426,6 +427,32 @@ class PurchasesOrchestratorTests: StoreKitConfigTestCase { | |||
expect(self.offerings.invokedPostOffer) == false | |||
} | |||
|
|||
func testGetPromotionalOfferFailsWithIneligibleIfReceiptHasNoTransactions() async throws { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's add another explicit test for the opposite case (doesn't fail if there are transactions). It's already indirectly tested, but having a direct one might help ensure that if other tests are refactored in some way, this one would still fail with a clear error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great!!
We should add at least one storekit integration test. @NachoSoto should be able to help guide you through that, we can do it as a follow-up PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is perfect 👍🏻
We already have testGetPromotionalOfferWithNoPurchasesReturnsIneligible
in StoreKitIntegrationTests
.
I would update that to verify that no requests are made:
self.logger.verifyMessageWasNotLogged("API request started")
expect(error).to(matchError(ErrorCode.ineligibleError)) | ||
} | ||
|
||
expect(self.offerings.invokedPostOffer) == false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏🏻
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3123 +/- ##
==========================================
+ Coverage 86.44% 86.62% +0.17%
==========================================
Files 219 219
Lines 15702 15711 +9
==========================================
+ Hits 13573 13609 +36
+ Misses 2129 2102 -27 ☔ View full report in Codecov by Sentry. |
**This is an automatic release.** ### Bugfixes * `DebugViewModel`: fixed runtime crash on iOS < 16 (#3139) via NachoSoto (@NachoSoto) ### Performance Improvements * `PurchasesOrchestrator`: return early if receipt has no transactions when checking for promo offers (#3123) via Mark Villacampa (@MarkVillacampa) * `Purchases`: don't clear intro eligibility / purchased products cache on first launch (#3067) via NachoSoto (@NachoSoto) ### Dependency Updates * `SPM`: update `Package.resolved` (#3130) via NachoSoto (@NachoSoto) ### Other Changes * `ReceiptParser`: fixed SPM build (#3144) via NachoSoto (@NachoSoto) * `carthage_installation_tests`: optimize SPM package loading (#3129) via NachoSoto (@NachoSoto) * `CI`: add workaround for `Carthage` timing out (#3119) via NachoSoto (@NachoSoto) * `Integration Tests`: workaround to not lose debug logs (#3108) via NachoSoto (@NachoSoto)
…when checking for promo offers (#3123) Avoids an extra request to the /offers endpoint in the common case where receipt has no trasactions.
**This is an automatic release.** ### Bugfixes * `DebugViewModel`: fixed runtime crash on iOS < 16 (#3139) via NachoSoto (@NachoSoto) ### Performance Improvements * `PurchasesOrchestrator`: return early if receipt has no transactions when checking for promo offers (#3123) via Mark Villacampa (@MarkVillacampa) * `Purchases`: don't clear intro eligibility / purchased products cache on first launch (#3067) via NachoSoto (@NachoSoto) ### Dependency Updates * `SPM`: update `Package.resolved` (#3130) via NachoSoto (@NachoSoto) ### Other Changes * `ReceiptParser`: fixed SPM build (#3144) via NachoSoto (@NachoSoto) * `carthage_installation_tests`: optimize SPM package loading (#3129) via NachoSoto (@NachoSoto) * `CI`: add workaround for `Carthage` timing out (#3119) via NachoSoto (@NachoSoto) * `Integration Tests`: workaround to not lose debug logs (#3108) via NachoSoto (@NachoSoto)
Avoids an extra request to the /offers endpoint in the common case where receipt has no trasactions.