-
Notifications
You must be signed in to change notification settings - Fork 4
fix: corrected transactionReason when a previously cancelled subscription was purchased again #21
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -333,6 +333,10 @@ private extension StoreKitTypesBridge { | |
| let string: String | ||
| let uppercased: String | ||
| } | ||
|
|
||
| struct JSONTransactionReason: Codable { | ||
| let transactionReason: String | ||
| } | ||
|
|
||
| static func transactionReasonDetails(from transaction: StoreKit.Transaction) -> TransactionReason { | ||
| if let revocation = transaction.revocationReason { | ||
|
|
@@ -358,9 +362,8 @@ private extension StoreKitTypesBridge { | |
| } | ||
|
|
||
| // Try to infer renewal for iOS <17 | ||
| if transaction.productType == .autoRenewable, | ||
| let expirationDate = transaction.expirationDate, | ||
| expirationDate > transaction.purchaseDate { | ||
| if let decodedReason = try? JSONDecoder().decode(JSONTransactionReason.self, from: transaction.jsonRepresentation), | ||
| decodedReason.transactionReason == "RENEWAL" { | ||
|
Comment on lines
+365
to
+366
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The current JSON decoding approach is good, but there's a more reliable way to detect renewals using // More reliable - compares originalID with current transaction ID
if transaction.productType == .autoRenewable,
transaction.originalID != transaction.id {
return TransactionReason(lowercased: "renewal", string: "renewal", uppercased: "RENEWAL")
}Why this is better:
Evidence from issue hyochan/react-native-iap#3054:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey, I’ve tested your implementation, but the case where a previously cancelled subscription is purchased again is still marked as a I believe Here is the part of purchase object that was tested.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right! Thanks for testing with actual data. When a user re-purchases a cancelled subscription:
Apple's
Your implementation is correct. My suggestion was flawed - thanks for catching this! 🙏 |
||
| return TransactionReason(lowercased: "renewal", string: "renewal", uppercased: "RENEWAL") | ||
| } | ||
|
|
||
|
|
||
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.
🛠️ Refactor suggestion | 🟠 Major
Rename to align with coding guidelines.
According to the coding guidelines, acronyms in Swift should use Pascal case (first letter caps, rest lowercase) when at the beginning or middle of an identifier.
JSONTransactionReasonshould beJsonTransactionReason.As per coding guidelines.
Apply this diff to fix the naming:
Also update the usage at line 365:
📝 Committable suggestion
🤖 Prompt for AI Agents