Fix: Include product ID in Stripe checkout swipe-dismiss abandon#453
Closed
Fix: Include product ID in Stripe checkout swipe-dismiss abandon#453
Conversation
When a user swipes to dismiss the Stripe checkout sheet before the JS stripe_checkout_abandon message fires, the fallback abandon path now uses the product ID from stripeCheckoutStart instead of an empty string. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…productId in mock Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tStart stripe_checkout_start fires too late (from within the Stripe checkout page), so the product ID was always nil on swipe-dismiss. Now the product ID is passed via the open_url message with browserType payment_sheet, which arrives before the checkout sheet opens. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goes with this PR: https://github.com/superwall/paywall-next/pull/2833/changes
Summary
stripe_checkout_abandonmessage firing), the fallback abandon path usedStoreProduct.blank()with no product identifier, resulting in an emptyabandoned_product_id.handleStripeCheckoutStart(productId:)toPaywallMessageHandlerDelegateto forward the product ID fromstripeCheckoutStart.lastStripeCheckoutProductIdonPaywallViewControllerand uses it in the fallback abandon:StoreProduct.blank(productIdentifier:).Test plan
stripe_checkout_abandonpath still works as before.lastStripeCheckoutProductIdis reset when opening a new checkout and after handling dismiss.CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes.swiftlintin the main directory and fixed any issues.🤖 Generated with Claude Code
Greptile Summary
This PR fixes a bug where swiping to dismiss the Stripe checkout sheet produced a transaction abandon event with an empty
abandoned_product_id. It does so by threading theproductIdentifierfrom theopen_url(payment_sheet) JS message all the way throughPaywallMessage,PaywallMessageHandler, andPaywallMessageHandlerDelegateintoPaywallViewController, where it is stored inlastStripeCheckoutProductIdand used in the swipe-dismiss fallback abandon path.Key changes:
PaywallMessage.openPaymentSheetgains an optionalproductId: String?associated value, decoded withtry?for backward compatibility.PaywallMessageHandlerDelegate.openPaymentSheet(_:productId:)signature updated and propagated throughPaywallMessageHandler.PaywallViewControllerstoreslastStripeCheckoutProductIdwhen opening checkout, captures it as a local constant before theDispatchWorkItem, and resets it tonilinonDismiss— correctly avoiding any race with the reset at end of the closure.openPaymentSheetProductIdfor test assertions.One area worth revisiting:
lastStripeCheckoutProductIdis sourced exclusively from the optionalproductIdentifierfield in theopen_urlmessage. The PR description originally intended to also hook into thestripeCheckoutStartevent (which carries a non-optionalproductId), providing a reliable second chance to populate the field when older backends don't includeproductIdentifierin theopen_urlpayload. If that source is dropped, a warning log when falling back to""would improve debuggability.Confidence Score: 4/5
productIdentifieris absent from theopen_urlpayload the behaviour is identical to before. The only concern is that thestripeCheckoutStartevent's non-optionalproductId(planned in the PR description as an additional source) was not wired up, leaving a silent empty-string fallback if the new backend field is absent. This is a minor robustness gap, not a correctness bug, so a score of 4 is appropriate.onDismissblock inPaywallViewController.swiftaround line 1163 is worth a final read to confirm the empty-string fallback behaviour is intentional.Important Files Changed
lastStripeCheckoutProductId: String?to store the product ID fromopenPaymentSheet, captures it as a local constant before theonDismissreset, and uses it in the swipe-dismiss abandon event. Logic is sound and backward-compatible; capturesabandonProductIdas a localletbefore the workItem, avoiding any race with the nil-reset at the end of the closure.productId: String?to theopenPaymentSheetenum case and decodes it withtry?from theopen_urlmessage payload whenbrowserType == "payment_sheet". The decode is safely optional, preserving backward compatibility with backends that don't yet send the field.productIdthrough the privateopenPaymentSheethelper and thePaywallMessageHandlerDelegateprotocol. Purely mechanical pass-through with no logic changes; no issues found.openPaymentSheet(_:productId:)signature and captures the passed-inproductIdfor test assertions. No issues found.Sequence Diagram
sequenceDiagram participant JS as Paywall JS participant MH as PaywallMessageHandler participant PVC as PaywallViewController participant CVC as CheckoutWebViewController JS->>MH: open_url (browserType=payment_sheet, productIdentifier?) MH->>PVC: openPaymentSheet(url, productId?) PVC->>PVC: lastStripeCheckoutProductId = productId PVC->>CVC: present(checkoutVC) alt Stripe JS fires checkout start JS->>MH: stripe_checkout_start (productId) MH->>MH: trackStripeCheckoutEvent(.start) note over MH,PVC: lastStripeCheckoutProductId NOT updated here end alt User swipe-dismisses (no JS abandon message) CVC-->>PVC: onDismiss callback PVC->>PVC: abandonProductId = lastStripeCheckoutProductId ?? "" PVC->>PVC: track Transaction(.abandon(StoreProduct.blank(productIdentifier: abandonProductId))) PVC->>PVC: lastStripeCheckoutProductId = nil else JS fires stripe_checkout_abandon JS->>MH: stripe_checkout_abandon (productId) MH->>PVC: handleStripeCheckoutAbandon(checkoutContextId, productId) PVC->>PVC: didReceiveStripeCheckoutAbandonMessage = true CVC-->>PVC: onDismiss callback PVC->>PVC: skip abandon tracking (flag already set) PVC->>PVC: lastStripeCheckoutProductId = nil endComments Outside Diff (2)
Sources/SuperwallKit/Paywall/View Controller/PaywallViewController.swift, line 1163-1168 ([link](https://github.com/superwall/superwall-ios/blob/f75af6a148e3f156512f38f1828f7cae2fe8ec46/Sources/SuperwallKit/Paywall/View Controller/PaywallViewController.swift#L1163-L1168))lastStripeCheckoutProductIdalreadynilwhen work item fireslastStripeCheckoutProductIdis set tonilon line 1186 immediately after the work item is scheduled with a 1-second delay. The work item capturesselfweakly and readsself.lastStripeCheckoutProductIdat execution time — which is always 1 second later — so it will always readniland fall back to"", completely defeating the purpose of this fix.The product ID must be captured by value in the closure, not by reference through
self:Alternatively, capture
lastStripeCheckoutProductIdas a localletconstant before creating theDispatchWorkItem.Tests/SuperwallKitTests/Paywall/View Controller/Web View/Message Handling/PaywallMessageHandlerDelegateMock.swift, line 39-112 ([link](https://github.com/superwall/superwall-ios/blob/f75af6a148e3f156512f38f1828f7cae2fe8ec46/Tests/SuperwallKitTests/Paywall/View Controller/Web View/Message Handling/PaywallMessageHandlerDelegateMock.swift#L39-L112))Mock missing new protocol method — will not compile
PaywallMessageHandlerDelegateMockconforms toPaywallMessageHandlerDelegatebut does not implement the newly requiredhandleStripeCheckoutStart(productId:)method. Swift will refuse to compile the test target until this stub is added.Sources/SuperwallKit/Paywall/View Controller/PaywallViewController.swift, line 1163 ([link](https://github.com/superwall/superwall-ios/blob/8405571e0060d3507e9be03b4693e77ac676b68f/Sources/SuperwallKit/Paywall/View Controller/PaywallViewController.swift#L1163))Potential ordering issue:
lastStripeCheckoutProductIdreset before being usedopenPaymentSheetresetslastStripeCheckoutProductId = nilat its start (line 1139). IfstripeCheckoutStartfires from JS beforeopenPaymentSheet(which is plausible — the JS layer may signal checkout start before requesting the native sheet to open), the product ID stored byhandleStripeCheckoutStartwill be overwritten bynilwhenopenPaymentSheetruns. TheonDismissclosure would then fall back to"".If the JS ordering is
stripeCheckoutStart→openPaymentSheet, the fix would not work as intended. It would be worth verifying (or documenting) the guaranteed message ordering, or alternatively, only resettinglastStripeCheckoutProductIdinsideopenPaymentSheetwhen the new product ID is not already set for the current checkout session.Tests/SuperwallKitTests/Paywall/View Controller/Web View/Message Handling/PaywallMessageHandlerDelegateMock.swift, line 95 ([link](https://github.com/superwall/superwall-ios/blob/8405571e0060d3507e9be03b4693e77ac676b68f/Tests/SuperwallKitTests/Paywall/View Controller/Web View/Message Handling/PaywallMessageHandlerDelegateMock.swift#L95))Mock doesn't capture
productIdfor test assertionsThe new mock method discards the
productIdargument, which prevents unit tests from asserting that the correct product ID was forwarded fromstripeCheckoutStart. Adding a stored property (similar to howstripeCheckoutSubmitis tracked as a tuple) would allow tests to verify the fix end-to-end.Sources/SuperwallKit/Paywall/View Controller/PaywallViewController.swift, line 1163 ([link](https://github.com/superwall/superwall-ios/blob/b64a33b770c138dd6b6986cd9f868f6b00f47ea4/Sources/SuperwallKit/Paywall/View Controller/PaywallViewController.swift#L1163))Silent empty-string fallback when product ID is missing
lastStripeCheckoutProductId ?? ""falls back to an empty string when theopen_urlmessage didn't carry aproductIdentifierfield (e.g., an older backend). This silently produces the same brokenabandoned_product_id: ""behaviour that the PR is meant to fix, with no indication anything went wrong.Consider either logging a warning when the fallback is triggered, or — since
stripeCheckoutStartalready contains a non-optionalproductIdand fires before the user can dismiss — also updatinglastStripeCheckoutProductIdfrom that message as an additional safety net (the PR description originally planned this viahandleStripeCheckoutStart(productId:)):This would ensure the product ID is stored even if
productIdentifierwas absent from theopen_urlpayload.Prompt To Fix All With AI
Last reviewed commit: b64a33b