Skip to content

Commit

Permalink
Fix embedded update preserving invalid paymentoption (#4189)
Browse files Browse the repository at this point in the history
## Summary
Previously, embedded would always preserve the selected payment option
across `update`, even if it's no longer valid. This PR fixes that.

Also contains some minor unrelated tweaks.

## Testing
See test

## Changelog
Not user facing
  • Loading branch information
yuki-stripe authored Oct 26, 2024
1 parent 5275d8d commit 62085fb
Show file tree
Hide file tree
Showing 9 changed files with 70 additions and 16 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@
{
"parallelizable" : true,
"skippedTests" : [
"EmbeddedUITests",
"EmbeddedUITests\/testUpdate()",
"PaymentSheetBillingCollectionLPMUITests",
"PaymentSheetBillingCollectionUICardTests",
"PaymentSheetBillingCollectionUITestCase",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
"skippedTests" : [
"CustomerSheetSnapshotTests",
"CustomerSheetUITest",
"EmbeddedUITests",
"EmbeddedUITests\/testUpdate()",
"LinkPaymentControllerUITest",
"PaymentSheetBillingCollectionBankTests",
"PaymentSheetBillingCollectionLPMUITests",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
"skippedTests" : [
"CustomerSheetSnapshotTests",
"CustomerSheetUITest",
"EmbeddedUITests",
"EmbeddedUITests\/testUpdate()",
"LinkPaymentControllerUITest",
"PaymentSheetBillingCollectionBankTests",
"PaymentSheetCVCRecollectionUITests",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@
B641A4192C2BA25D00AE654A /* PaymentSheetVerticalUITest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B641A4182C2BA25D00AE654A /* PaymentSheetVerticalUITest.swift */; };
B6CA975C2C486DE700DAE441 /* PaymentSheetLPMUITest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6CA975B2C486DE700DAE441 /* PaymentSheetLPMUITest.swift */; };
B6D6AAA666859847BB59749C /* PaymentSheet+AddressTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A61C5739DBE6405A4D9FD5F2 /* PaymentSheet+AddressTests.swift */; };
B6DA0FEE2CC97F4900BF41B7 /* EmbeddedUITest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B6DA0FED2CC97F3D00BF41B7 /* EmbeddedUITest.swift */; };
C39CA2EBE08EE768559A8FD1 /* StripeCore.framework in Embed Frameworks */ = {isa = PBXBuildFile; fileRef = 1FD1F5193E4A361EA9E8FED3 /* StripeCore.framework */; settings = {ATTRIBUTES = (CodeSignOnCopy, RemoveHeadersOnCopy, ); }; };
CC9090D573972E8B14D204CB /* CustomerSheetTestPlaygroundSettings.swift in Sources */ = {isa = PBXBuildFile; fileRef = F2F10B63B9ECBC62AFCF8B32 /* CustomerSheetTestPlaygroundSettings.swift */; };
D235E82A7173E8051BC5A261 /* ExampleCustomCheckoutViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = ECF244A593BD92A973B1C369 /* ExampleCustomCheckoutViewController.swift */; };
Expand Down Expand Up @@ -249,6 +250,7 @@
B641A4182C2BA25D00AE654A /* PaymentSheetVerticalUITest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentSheetVerticalUITest.swift; sourceTree = "<group>"; };
B69C155A2B9FDCBD009CE667 /* PaymentSheet Example.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; name = "PaymentSheet Example.entitlements"; path = "PaymentSheet Example/PaymentSheet Example.entitlements"; sourceTree = "<group>"; };
B6CA975B2C486DE700DAE441 /* PaymentSheetLPMUITest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentSheetLPMUITest.swift; sourceTree = "<group>"; };
B6DA0FED2CC97F3D00BF41B7 /* EmbeddedUITest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EmbeddedUITest.swift; sourceTree = "<group>"; };
B7AFD32B5EAD3BEEEC3D4260 /* ExampleSwiftUIPaymentSheet.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ExampleSwiftUIPaymentSheet.swift; sourceTree = "<group>"; };
B7C76D3D1BF7666D1963DD0B /* bg-BG */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = "bg-BG"; path = "bg-BG.lproj/LaunchScreen.strings"; sourceTree = "<group>"; };
B96951DD278BA611BBCBB10A /* PaymentSheet Example-Shard1.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "PaymentSheet Example-Shard1.xctestplan"; sourceTree = "<group>"; };
Expand Down Expand Up @@ -422,6 +424,7 @@
5CA299EAF5914484195167EB /* PaymentSheetUITest.swift */,
B6CA975B2C486DE700DAE441 /* PaymentSheetLPMUITest.swift */,
B641A4182C2BA25D00AE654A /* PaymentSheetVerticalUITest.swift */,
B6DA0FED2CC97F3D00BF41B7 /* EmbeddedUITest.swift */,
36BB679CF53EEF943F0BAAC9 /* XCUITest+Utilities.swift */,
6A5CCDA32C0E6F5D003306A4 /* LinkPaymentControllerUITest.swift */,
);
Expand Down Expand Up @@ -657,6 +660,7 @@
isa = PBXSourcesBuildPhase;
buildActionMask = 2147483647;
files = (
B6DA0FEE2CC97F4900BF41B7 /* EmbeddedUITest.swift in Sources */,
5441EB34BBE32DD98B7A6883 /* CustomerSheetTestPlaygroundSettings.swift in Sources */,
E66F4E053FC6DD05EA245CDF /* CustomerSheetUITest.swift in Sources */,
B6CA975C2C486DE700DAE441 /* PaymentSheetLPMUITest.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ private class EmbeddedPaymentOptionView: UIView {
label.font = .preferredFont(forTextStyle: .subheadline)
label.numberOfLines = 1
label.translatesAutoresizingMaskIntoConstraints = false
label.accessibilityIdentifier = "Payment method"
return label
}()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -731,7 +731,6 @@ extension PlaygroundController {
case .embedded:
guard !shouldUpdateEmbeddedInsteadOfRecreating else {
// Update embedded rather than re-creating it
self.embeddedPlaygroundViewController?.isLoading = false
self.updateEmbedded()
self.currentlyRenderedSettings = self.settings
return
Expand Down Expand Up @@ -979,6 +978,7 @@ extension PlaygroundController {
break
case .failed(let error):
// Display error to user in an alert, let them retry
self.embeddedPlaygroundViewController?.isLoading = false
let alert = UIAlertController(title: "Error", message: error.localizedDescription, preferredStyle: .alert)
alert.addAction(.init(title: "Retry", style: .default, handler: { _ in
self.updateEmbedded()
Expand All @@ -987,6 +987,7 @@ extension PlaygroundController {
embeddedPlaygroundViewController.present(alert, animated: true)
case .succeeded:
self.isLoading = false
self.embeddedPlaygroundViewController?.isLoading = false
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
//
// EmbeddedUITest.swift
// PaymentSheet Example
//
// Created by Yuki Tokuhiro on 10/23/24.
//

import XCTest

class EmbeddedUITests: PaymentSheetUITestCase {
func testUpdate() {
var settings = PaymentSheetTestPlaygroundSettings.defaultValues()
settings.customerMode = .new
settings.mode = .payment
settings.integrationType = .deferred_csc
settings.uiStyle = .embedded
loadPlayground(app, settings)
app.buttons["Present embedded payment element"].waitForExistenceAndTap()
// TODO: Test card form (see PaymentSheetVerticalUITests testUpdate)

// Selecting Alipay w/ deferred PaymentIntent...
app.buttons["Alipay"].waitForExistenceAndTap()
XCTAssertEqual(app.staticTexts["Payment method"].label, "Alipay")
// ...and *updating* to a SetupIntent...
app.buttons.matching(identifier: "Setup").element(boundBy: 1).tap()
// ...(wait for it to finish updating)...
_ = app.buttons["Reload"].waitForExistence(timeout: 10)
// ...should cause Alipay to no longer be the selected payment method, since it is not valid for setup.
XCTAssertFalse(app.staticTexts["Payment method"].exists)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ public final class EmbeddedPaymentElement {
) async -> UpdateResult {
embeddedPaymentMethodsView.isUserInteractionEnabled = false
// Cancel the old task and let it finish so that merchants receive update results in order
currentUpdateTask?.cancel()
_ = await currentUpdateTask?.value
latestUpdateTask?.cancel()
_ = await latestUpdateTask?.value
// Start the new update task
let currentUpdateTask = Task { @MainActor [weak self, configuration, analyticsHelper] in
// ⚠️ Don't modify `self` until the end to avoid being canceled halfway through and leaving self in a partially updated state.
Expand Down Expand Up @@ -158,7 +158,7 @@ public final class EmbeddedPaymentElement {
}
return .succeeded
}
self.currentUpdateTask = currentUpdateTask
self.latestUpdateTask = currentUpdateTask
let updateResult = await currentUpdateTask.value
embeddedPaymentMethodsView.isUserInteractionEnabled = true
return updateResult
Expand All @@ -170,8 +170,8 @@ public final class EmbeddedPaymentElement {
/// - Note: This method requires that the last call to `update` succeeded. If the last `update` call failed, this call will fail. If this method is called while a call to `update` is in progress, it waits until the `update` call completes.
public func confirm() async -> EmbeddedPaymentElementResult {
// Wait for the last update to finish and fail if didn't succeed. A failure means the view is out of sync with the intent and could e.g. not be showing a required mandate.
if let currentUpdateTask {
switch await currentUpdateTask.value {
if let latestUpdateTask {
switch await latestUpdateTask.value {
case .succeeded:
// The view is in sync with the intent. Continue on with confirm!
break
Expand All @@ -194,7 +194,7 @@ public final class EmbeddedPaymentElement {
internal private(set) var containerView: EmbeddedPaymentElementContainerView
internal private(set) var embeddedPaymentMethodsView: EmbeddedPaymentMethodsView
internal private(set) var loadResult: PaymentSheetLoader.LoadResult
internal private(set) var currentUpdateTask: Task<UpdateResult, Never>?
internal private(set) var latestUpdateTask: Task<UpdateResult, Never>?
private let analyticsHelper: PaymentSheetAnalyticsHelper
internal var _paymentOption: PaymentOption? {
// TODO: Handle forms. See `PaymentSheetVerticalViewController.selectedPaymentOption`.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ class EmbeddedPaymentMethodsView: UIView {
delegate: EmbeddedPaymentMethodsViewDelegate? = nil
) {
self.appearance = appearance
self.selection = initialSelection
self.mandateProvider = mandateProvider
self.shouldShowMandate = shouldShowMandate
self.delegate = delegate
Expand Down Expand Up @@ -90,21 +89,30 @@ class EmbeddedPaymentMethodsView: UIView {

if initialSelection == selection {
savedPaymentMethodButton.isSelected = true
self.selection = initialSelection
}

stackView.addArrangedSubview(savedPaymentMethodButton)
}

// Add card before Apple Pay and Link if present and before any other LPMs
if paymentMethodTypes.contains(.stripe(.card)) {
stackView.addArrangedSubview(RowButton.makeForPaymentMethodType(paymentMethodType: .stripe(.card),
savedPaymentMethodType: savedPaymentMethod?.type,
appearance: rowButtonAppearance,
shouldAnimateOnPress: true,
isEmbedded: true,
didTap: { [weak self] rowButton in
self?.didTap(selectedRowButton: rowButton, selection: .new(paymentMethodType: .stripe(.card)))
}))
let selection: Selection = .new(paymentMethodType: .stripe(.card))
let cardRowButton = RowButton.makeForPaymentMethodType(
paymentMethodType: .stripe(.card),
savedPaymentMethodType: savedPaymentMethod?.type,
appearance: rowButtonAppearance,
shouldAnimateOnPress: true,
isEmbedded: true,
didTap: { [weak self] rowButton in
self?.didTap(selectedRowButton: rowButton, selection: selection)
}
)
if initialSelection == selection {
cardRowButton.isSelected = true
self.selection = initialSelection
}
stackView.addArrangedSubview(cardRowButton)
}

if shouldShowApplePay {
Expand All @@ -117,6 +125,7 @@ class EmbeddedPaymentMethodsView: UIView {

if initialSelection == selection {
applePayRowButton.isSelected = true
self.selection = initialSelection
}

stackView.addArrangedSubview(applePayRowButton)
Expand All @@ -130,6 +139,7 @@ class EmbeddedPaymentMethodsView: UIView {

if initialSelection == selection {
linkRowButton.isSelected = true
self.selection = initialSelection
}

stackView.addArrangedSubview(linkRowButton)
Expand All @@ -151,6 +161,7 @@ class EmbeddedPaymentMethodsView: UIView {
)
if initialSelection == selection {
rowButton.isSelected = true
self.selection = initialSelection
}
stackView.addArrangedSubview(rowButton)
}
Expand Down

0 comments on commit 62085fb

Please sign in to comment.