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

fix: filter old subscription transactions during upgrade#22

Merged
hyochan merged 2 commits intomainfrom
fix/filter-old-subscription-on-upgrade
Oct 11, 2025
Merged

fix: filter old subscription transactions during upgrade#22
hyochan merged 2 commits intomainfrom
fix/filter-old-subscription-on-upgrade

Conversation

@hyochan
Copy link
Member

@hyochan hyochan commented Oct 11, 2025

Problem

Fixes hyochan/react-native-iap#3054

When upgrading from a monthly subscription to a yearly subscription (within the same subscription group), onPurchaseSuccess was emitting old purchase objects first, and the correct upgraded transaction arrived only after 3-4 minutes in iOS Sandbox.

Root Cause

Analysis of actual purchase data from the issue reporter revealed that isUpgraded is not reliably set to true during subscription upgrades in Sandbox. Both the old and new transactions show isUpgradedIOS: false with reasonIOS: "renewal".

This happens because StoreKit treats subscription upgrades that occur at renewal boundaries as "renewal" events, not "upgrade" events.

Example data from issue reporter:

Monthly (arrives first):

{
  "transactionDate": 1760118720000,
  "isUpgradedIOS": false,  // ❌ Not set despite being superseded
  "reasonIOS": "renewal"
}

Yearly (arrives 180 seconds later):

{
  "transactionDate": 1760118900000,
  "isUpgradedIOS": false,
  "reasonIOS": "renewal"
}

Solution

Instead of relying on the unreliable isUpgraded flag, we now track the latest transaction date per subscription group:

// For subscriptions, skip if we've already seen a newer transaction in the same group
// This handles subscription upgrades where isUpgraded is not reliably set
if await self.state.shouldProcessSubscriptionTransaction(transaction) == false {
    OpenIapLog.debug("⏭️ Skipping older subscription transaction: \(transactionId) (superseded by newer transaction in same group)")
    continue
}

How It Works

  1. Track the latest purchaseDate for each subscriptionGroupID
  2. When a transaction arrives, check if a newer transaction in the same group has already been processed
  3. Skip the older transaction if a newer one exists
  4. This works regardless of the isUpgraded flag value

Changes

IapState.swift:

  • Added latestTransactionDateByGroup dictionary to track latest transaction per subscription group
  • Added shouldProcessSubscriptionTransaction() to determine if a transaction should be processed

OpenIapModule.swift:

  • Removed unreliable isUpgraded check
  • Added subscription group-based filtering
  • Enhanced debug logging to show transaction details

Testing

  • ✅ All unit tests passing (10 tests)
  • ✅ Swift build successful
  • ✅ Maintains existing functionality (subscription renewals, duplicate prevention, refund filtering)

Behavior After Fix

Subscription Upgrade (Monthly → Yearly):

  • ❌ Before: Monthly emitted first, Yearly after 3-4 min delay
  • ✅ After: Only the newest transaction (Yearly) is emitted; Monthly is skipped as superseded

Subscription Renewal:

  • ✅ Works as before (newer renewal transaction replaces older)

Refunded Purchase:

  • ✅ Still filtered out (revocationDate != nil)

Non-subscription IAP:

  • ✅ No change (group tracking only applies to subscriptions)

Summary by CodeRabbit

  • New Features
    • Per-subscription-group tracking to ensure only the most recent transaction in a group is processed.
  • Bug Fixes
    • Skip revoked transactions to avoid invalid purchase events.
    • Prevent duplicate or outdated subscription purchase updates by filtering superseded transactions.
  • Refactor
    • Streamlined transaction processing and enhanced debug logging when purchase updates are emitted.

@coderabbitai
Copy link

coderabbitai bot commented Oct 11, 2025

Walkthrough

Adds per-subscription-group latest transaction tracking and a gating function to skip superseded subscription transactions. Updates OpenIapModule to log transactions, skip revoked transactions, consult the per-group gate, preserve processed checks, store pending transactions, and log emitted purchase updates. (≤50 words)

Changes

Cohort / File(s) Summary
Subscription transaction gating state
Sources/Helpers/IapState.swift
Adds latestTransactionByGroup: [String: (date: Date, id: UInt64)], clears it on reset, and implements shouldProcessSubscriptionTransaction(_:) to skip transactions older or not newer (by date then id) than the recorded latest for the subscription group.
Transaction processing flow and logging
Sources/OpenIapModule.swift
Logs full transaction details, skips transactions with revocationDate, consults shouldProcessSubscriptionTransaction to drop superseded group transactions, retains already-processed checks, marks/stores pending transactions, emits purchase updates and logs those emissions.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant StoreKit
  participant OpenIapModule
  participant IapState

  User->>StoreKit: purchase / upgrade
  StoreKit-->>OpenIapModule: transaction update
  OpenIapModule->>OpenIapModule: log transaction details
  alt revoked (revocationDate set)
    OpenIapModule-->>StoreKit: skip (revoked)
  else not revoked
    alt subscription transaction (has subscriptionGroupID)
      OpenIapModule->>IapState: shouldProcessSubscriptionTransaction(tx)
      IapState-->>OpenIapModule: true / false
      alt false (superseded)
        OpenIapModule-->>StoreKit: skip (superseded by newer in group)
      else true
        OpenIapModule->>OpenIapModule: check already-processed
        alt already processed
          OpenIapModule-->>StoreKit: skip (already processed)
        else mark/store pending and emit
          OpenIapModule-->>User: emitPurchaseUpdate (logged)
        end
      end
    else non-subscription
      OpenIapModule->>OpenIapModule: check already-processed → mark/store → emit
      OpenIapModule-->>User: emitPurchaseUpdate (logged)
    end
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

🛠 bugfix

Poem

A hop, a skip, a tidy trace,
New subs take the freshest place.
Dates and ids we keep in mind,
Old ghosts skipped, the flow’s aligned.
Rabbit logs a happy cheer—no stale buys here! 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title clearly summarizes the primary change to filter out old subscription transactions during upgrade, aligning directly with the implemented subscription-group-based gating logic and the core fix described in the PR. It is concise, specific, and accurately reflects the main alteration.
Linked Issues Check ✅ Passed The pull request implements subscription-group tracking and a gating function that skips superseded transactions, ensuring only the newest transaction is processed, thereby preventing re-emission of old purchases and relying no longer on the unreliable isUpgraded flag. This directly fulfills the objectives of issue #3054 by emitting only the upgraded transaction promptly, preserving correct behavior for renewals and non-upgrade flows, and preventing old transactions from reappearing.
Out of Scope Changes Check ✅ Passed All modifications are focused on removing the unreliable upgrade flag, introducing subscription-group-based filtering, and enhancing related logging; no unrelated or out-of-scope functionality was altered.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch fix/filter-old-subscription-on-upgrade

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2750e9c and 7f271a8.

📒 Files selected for processing (2)
  • ISSUE_ANALYSIS.md (1 hunks)
  • Sources/Helpers/IapState.swift (3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/IapState.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/IapState.swift
🪛 markdownlint-cli2 (0.18.1)
ISSUE_ANALYSIS.md

12-12: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


23-23: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)


54-54: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

🔇 Additional comments (4)
Sources/Helpers/IapState.swift (3)

11-12: LGTM! Clear property declaration with explanatory comment.

The dictionary structure tracking both date and transaction ID per subscription group is well-designed for filtering superseded transactions and handling tie-breaking scenarios.


24-24: LGTM! Correctly clears the new tracking state on reset.

The reset method now properly clears latestTransactionByGroup along with other state, maintaining consistency.


40-65: LGTM! Tie-breaker logic correctly addresses the past review concern.

The implementation now properly handles identical timestamps by using transaction ID as a secondary sort key:

  • Skips if transactionDate < latest.date (older transaction)
  • Skips if transactionDate == latest.date && transactionId <= latest.id (duplicate or older ID)
  • Processes and updates if the transaction is newer by date or ID

This correctly addresses the past review comment about handling identical purchaseDate values.

Minor note: The logic assumes transaction IDs are monotonically increasing within a subscription group, which is a reasonable assumption for StoreKit but could be documented in the function comment for clarity.

ISSUE_ANALYSIS.md (1)

1-95: Excellent documentation explaining the root cause and solution.

The analysis clearly articulates:

  • The unexpected behavior where isUpgradedIOS is false for both transactions
  • Why the date-based tracking approach works regardless of the isUpgraded flag
  • The step-by-step flow with concrete examples

This will be valuable for future maintainers understanding why this approach was chosen.

Minor markdown formatting issue: The static analysis tool flagged that fenced code blocks should be surrounded by blank lines (lines 12, 23, 54). While not critical, adding blank lines around these code blocks would improve readability and comply with common markdown best practices.

Example fix for lines 11-13:

 
 **Monthly (arrives first):**
+
 ```json

If desired, you can apply similar fixes at lines 23 and 54.


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

📜 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 2750e9c.

📒 Files selected for processing (2)
  • Sources/Helpers/IapState.swift (3 hunks)
  • Sources/OpenIapModule.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/IapState.swift
  • Sources/OpenIapModule.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/IapState.swift
Sources/{OpenIapProtocol.swift,OpenIapModule.swift}

📄 CodeRabbit inference engine (CLAUDE.md)

Sources/{OpenIapProtocol.swift,OpenIapModule.swift}: Public API names MUST match openiap.dev and React Native OpenIAP (Apple module)
Use standard Apple module API names exactly: initConnection(), endConnection(), fetchProducts(), getAvailablePurchases(), requestPurchase(), finishTransaction()

Files:

  • Sources/OpenIapModule.swift
🧠 Learnings (1)
📚 Learning: 2025-10-09T19:13:15.972Z
Learnt from: CR
PR: hyodotdev/openiap-apple#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-09T19:13:15.972Z
Learning: Applies to Sources/{OpenIapProtocol.swift,OpenIapModule.swift} : Use standard Apple module API names exactly: initConnection(), endConnection(), fetchProducts(), getAvailablePurchases(), requestPurchase(), finishTransaction()

Applied to files:

  • Sources/OpenIapModule.swift
🧬 Code graph analysis (2)
Sources/Helpers/IapState.swift (2)
Sources/OpenIapStore.swift (1)
  • shouldReplaceSubscription (442-465)
Sources/Helpers/StoreKitTypesBridge.swift (1)
  • subscriptionAutoRenewState (147-168)
Sources/OpenIapModule.swift (1)
Sources/Helpers/IapState.swift (1)
  • shouldProcessSubscriptionTransaction (40-58)
🔇 Additional comments (5)
Sources/OpenIapModule.swift (3)

841-855: LGTM! Transaction logging and revocation check are well-implemented.

The detailed logging provides excellent visibility into transaction processing, and the explicit revocation check correctly skips invalid transactions before any further processing. The placement of the revocation check before the subscription filtering ensures revoked transactions don't affect the latest-transaction tracking.


916-916: Good observability improvement.

The debug log provides helpful information about purchase update emissions, making it easier to diagnose listener issues.


857-862: IapState actor isolation confirmed: IapState is declared as an actor and all state interactions are awaited, guaranteeing serialized processing and proper skipping of older subscription transactions.

Sources/Helpers/IapState.swift (2)

11-12: LGTM! Per-group tracking added correctly.

The latestTransactionDateByGroup dictionary effectively tracks the newest transaction per subscription group, enabling the filtering of superseded transactions during upgrades.


24-24: Correct reset behavior.

Clearing latestTransactionDateByGroup on reset ensures that the tracking state is properly reinitialized when the connection is reset.

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.

The tie-breaking logic concern has been addressed in commit 7f271a8. Resolving this thread.

@hyochan hyochan merged commit 48365cc into main Oct 11, 2025
2 checks passed
@hyochan hyochan deleted the fix/filter-old-subscription-on-upgrade branch October 11, 2025 03:03
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Issue: onPurchaseSuccess returns previous purchase objects and delays correct one during plan upgrade (iOS)

1 participant