Skip to content

Fix google play purchases missing purchase date#2703

Merged
tonidero merged 1 commit into
mainfrom
fix/play-purchases-missing-purchase-date
Jun 26, 2023
Merged

Fix google play purchases missing purchase date#2703
tonidero merged 1 commit into
mainfrom
fix/play-purchases-missing-purchase-date

Conversation

@tonidero

Copy link
Copy Markdown
Contributor

Description

Reported in RevenueCat/purchases-flutter#738.

After #2654, we were using the new Google Play identifiers that include the plan id in expiration dates, but we didn't change the system that parses purchase dates. Later on, when we map purchases, we iterate over the expiration dates map, and since that id wasn't found in the purchase dates, it was null, causing the issue in flutter.

This makes the purchase date calculation use the same algorithm that we use for expiration dates to get the new google play product identifiers.

@tonidero tonidero added the pr:fix A bug fix label Jun 26, 2023
@tonidero tonidero requested review from a team and MarkVillacampa June 26, 2023 07:47
@tonidero tonidero merged commit 48127ff into main Jun 26, 2023
@tonidero tonidero deleted the fix/play-purchases-missing-purchase-date branch June 26, 2023 09:17
tonidero pushed a commit that referenced this pull request Jun 26, 2023
**This is an automatic release.**

### Bugfixes
* Fix google play purchases missing purchase date (#2703) via Toni Rico
(@tonidero)
### Other Changes
* `PurchaseTester`: fixed `watchOS` build and ASC deployment (#2701) via
NachoSoto (@NachoSoto)
* Add `Data.sha1` (#2696) via NachoSoto (@NachoSoto)
* Refactor: extract `ErrorResponse` into its own file (#2697) via
NachoSoto (@NachoSoto)
* Add `Sequence<AdditiveArithmetic>.sum()` (#2694) via NachoSoto
(@NachoSoto)
* Refactored `Data.asString` implementation (#2695) via NachoSoto
(@NachoSoto)
* `Diagnostics`: new `FileHandler` for abstracting file operations
(#2673) via NachoSoto (@NachoSoto)

@aboedo aboedo left a comment

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.

Thanks for taking care of this so quickly!


private extension CustomerInfo {

static func productID(productID: String, purchase: CustomerInfoResponse.Subscription) -> String {

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.

I'm a bit late to the game, but I think a more swift-y name for this would be something like:

static func productID(_ productID: String, from purchase: CustomerInfoResponse.Subscription)

and given that this is called from a function called extractPurchaseDates, I'd follow the same scheme here, i.e.:

static func extractProductIDAndBasePlan(from productID: String, purchase: CustomerInfoResponse.Subscription)
  • making clear that it's also doing something with a base plan, since having a function called productID that takes the productID seems confusing unless you look at the implementation
  • using extract for consistency
  • using parameter names to make the name slightly cleaner

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.

Done in #2708

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

Thanks for fixing this!
I agree with @aboedo’s recommendation but you can refactor this in a separate PR, I’m glad you released the fix so quickly.

expect(purchaseDate) == Date(timeIntervalSince1970: 1526797490)
}

func testPurchaseDateForGooglePlayProductIdentifier() throws {

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.

👌🏻

tonidero added a commit that referenced this pull request Jun 26, 2023
…ses (#2708)

### Description
Small rename from PR review in #2703. That was merged quickly to solve
the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants