Skip to content
This repository was archived by the owner on Oct 16, 2025. It is now read-only.

fix: corrected transactionReason when a previously cancelled subscription was purchased again#21

Merged
hyochan merged 1 commit intohyodotdev:mainfrom
nahlebn1k:main
Oct 11, 2025
Merged

fix: corrected transactionReason when a previously cancelled subscription was purchased again#21
hyochan merged 1 commit intohyodotdev:mainfrom
nahlebn1k:main

Conversation

@nahlebn1k
Copy link
Contributor

@nahlebn1k nahlebn1k commented Oct 10, 2025

Hello, I discovered that transactionReason was incorrectly set to RENEW instead of PURCHASE, for iOS purchases that had been previously cancelled and then purchased again in the purchaseUpdatedListener callback.

This fix restores the previous logic for determining transactionReason, consistent with the behavior in react-native-iap versions 12.x.x and 13.x.x and real App Store data encoded in jsonRepresentation.

hyochan/react-native-iap#3056

Summary by CodeRabbit

  • Bug Fixes
    • Improved accuracy in detecting subscription renewals, ensuring renewal events are correctly identified.
    • Reduces instances where subscriptions might appear incorrectly as new purchases or non-renewals.
    • Leads to more reliable subscription status, history, and related notifications for users.

@coderabbitai
Copy link

coderabbitai bot commented Oct 10, 2025

Walkthrough

Introduces an internal JSONTransactionReason struct to decode transaction.jsonRepresentation and identify renewal transactions by checking for "RENEWAL". If decoded as renewal, returns a renewal TransactionReason; otherwise, falls back to the previous logic.

Changes

Cohort / File(s) Summary
StoreKit types bridging
Sources/Helpers/StoreKitTypesBridge.swift
Added internal JSONTransactionReason for decoding transaction.jsonRepresentation. Replaced prior renewal detection heuristic with JSON-based check for transactionReason == "RENEWAL", falling back to existing flow if decoding fails or reason is not renewal.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App
  participant StoreKitBridge as StoreKitTypesBridge
  participant Transaction as StoreKit.Transaction

  App->>StoreKitBridge: handle(transaction)
  StoreKitBridge->>Transaction: get jsonRepresentation
  Note over StoreKitBridge: Decode JSONTransactionReason
  alt Decoding succeeds and reason == "RENEWAL"
    StoreKitBridge-->>App: return TransactionReason.renewal
  else Fallback
    Note over StoreKitBridge: Use previous detection logic
    StoreKitBridge-->>App: return prior-derived TransactionReason
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I sniffed the JSON, twitchy and keen,
Found “RENEWAL” hidden between—
If true, I thump: “It’s back again!”
If not, I hop the older lane.
Ears up, I bridge the bytes with cheer—
A tidy trail, no bugs to fear. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed The title clearly and concisely summarizes the main change by indicating that the transactionReason logic is corrected for subscriptions that were previously cancelled and then repurchased, which matches the PR description and objectives.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
Sources/Helpers/StoreKitTypesBridge.swift (1)

365-368: Logic looks correct for the fix.

The new approach of decoding transaction.jsonRepresentation to check for "RENEWAL" properly addresses the issue described in the PR where previously cancelled subscriptions were incorrectly marked as renewals. The graceful fallback using try? and the default to "purchase" align with the expected behavior.

However, the comment on line 364 mentions "Try to infer renewal for iOS <17" but doesn't explain what the preferred approach would be for iOS 17+. Consider clarifying whether there's a direct API available in iOS 17+ that should be used instead, or if this approach is still the recommended solution across all versions.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 391205a and 6c18cec.

📒 Files selected for processing (1)
  • Sources/Helpers/StoreKitTypesBridge.swift (2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
Sources/**/*.swift

📄 CodeRabbit inference engine (CLAUDE.md)

Sources/**/*.swift: iOS-specific functions MUST have IOS suffix
Android-specific functions MUST have Android suffix
Cross-platform functions must have NO platform suffix
Acronyms in Swift should be ALL CAPS only when used as a suffix; otherwise use Pascal case (first letter caps, rest lowercase)
Specific casing: iOS -> Ios in beginning/middle, IOS as suffix
Specific casing: IAP -> Iap in beginning/middle, IAP as suffix
Specific casing: API -> Api in beginning/middle, API as suffix
Specific casing: URL -> Url in beginning/middle, URL as suffix
OpenIapError static code constants use PascalCase names; raw string values remain E_ codes; avoid introducing new E_-prefixed identifiers in Swift

Files:

  • Sources/Helpers/StoreKitTypesBridge.swift
Sources/Helpers/**/*.swift

📄 CodeRabbit inference engine (CLAUDE.md)

Helpers should use descriptive names ending with purpose (e.g., Manager, Cache, Status)

Files:

  • Sources/Helpers/StoreKitTypesBridge.swift
Sources/Helpers/**/*Store*.swift

📄 CodeRabbit inference engine (CLAUDE.md)

Avoid using 'Store' in names for caching/helper classes; use Cache or Manager instead

Files:

  • Sources/Helpers/StoreKitTypesBridge.swift

Comment on lines +337 to +339
struct JSONTransactionReason: Codable {
let transactionReason: String
}
Copy link

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. JSONTransactionReason should be JsonTransactionReason.

As per coding guidelines.

Apply this diff to fix the naming:

-    struct JSONTransactionReason: Codable {
+    struct JsonTransactionReason: Codable {
         let transactionReason: String
     }

Also update the usage at line 365:

-        if let decodedReason = try? JSONDecoder().decode(JSONTransactionReason.self, from: transaction.jsonRepresentation),
+        if let decodedReason = try? JSONDecoder().decode(JsonTransactionReason.self, from: transaction.jsonRepresentation),
             decodedReason.transactionReason == "RENEWAL" {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
struct JSONTransactionReason: Codable {
let transactionReason: String
}
struct JsonTransactionReason: Codable {
let transactionReason: String
}
Suggested change
struct JSONTransactionReason: Codable {
let transactionReason: String
}
if let decodedReason = try? JSONDecoder().decode(JsonTransactionReason.self, from: transaction.jsonRepresentation),
decodedReason.transactionReason == "RENEWAL" {
🤖 Prompt for AI Agents
In Sources/Helpers/StoreKitTypesBridge.swift around lines 337–339, rename the
struct JSONTransactionReason to JsonTransactionReason to follow Pascal-case for
acronyms; update its declaration and all references (including the usage at line
365) to the new name so the type matches everywhere.

Comment on lines +365 to +366
if let decodedReason = try? JSONDecoder().decode(JSONTransactionReason.self, from: transaction.jsonRepresentation),
decodedReason.transactionReason == "RENEWAL" {
Copy link
Member

@hyochan hyochan Oct 11, 2025

Choose a reason for hiding this comment

The 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 originalID:

// 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:

  • No JSON parsing overhead
  • More reliable: originalID != id definitively means renewal
  • First purchase: originalID == id
  • Any renewal: originalID != id (points to original purchase)

Evidence from issue hyochan/react-native-iap#3054:
Both monthly renewal and yearly upgrade showed originalTransactionIdentifierIOS: "2000001030743975" (different from their own transaction IDs), proving they are renewals/upgrades.

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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 renewal, and its originalId remains the originalId of the first-ever purchase.

I believe jsonRepresentation is more reliable here, as it reflects the actual state of transactionReason.

Here is the part of purchase object that was tested.

countryCodeIOS: "UKR"
environmentIOS: "Sandbox"
expirationDateIOS: 1760169564000
id: "2000001031849543"
isAutoRenewing: true
isUpgradedIOS: falseofferIOS: null
originalTransactionDateIOS: 1747052007000
originalTransactionIdentifierIOS: "2000000917067139"
ownershipTypeIOS: "purchased"
platform: "ios"
productId: "uapro_month"
purchaseState: "purchased"
reasonIOS: "renewal"
reasonStringRepresentationIOS: "renewal"
transactionDate: 1760169384000
transactionReasonIOS: "RENEWAL"

Copy link
Member

Choose a reason for hiding this comment

The 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:

  • The originalID still points to the very first purchase
  • So originalID != id would wrongly mark it as "RENEWAL" instead of "PURCHASE"

Apple's transactionReason in JSON correctly distinguishes:

  • Re-purchase after cancel = "PURCHASE"
  • Actual renewal = "RENEWAL"

Your implementation is correct. My suggestion was flawed - thanks for catching this! 🙏

Copy link
Member

@hyochan hyochan left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution and please see the code review 🙏

@hyochan hyochan added the 🛠 bugfix All kinds of bug fixes label Oct 11, 2025
@nahlebn1k nahlebn1k requested a review from hyochan October 11, 2025 08:09
@hyochan hyochan merged commit 2081fdd into hyodotdev:main Oct 11, 2025
2 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

🛠 bugfix All kinds of bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants