Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve link form regeneration #4095

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,7 @@ extension LinkInlineSignupElementSnapshotTests {
configuration: configuration,
showCheckbox: showCheckbox,
accountService: MockAccountService(),
previousCustomerInput: nil,
linkAccount: linkAccount,
country: country
)
Expand Down
2 changes: 1 addition & 1 deletion Stripe/StripeiOSTests/LinkSignupViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import StripeCoreTestUtils
import XCTest

@testable@_spi(STP) import Stripe
@testable@_spi(STP) import StripeCore
@testable@_spi(STP) import StripePayments
@testable@_spi(STP) import StripePaymentSheet
import StripePaymentsTestUtils
Expand Down Expand Up @@ -208,6 +207,7 @@ extension LinkInlineSignupViewModelTests {
configuration: PaymentSheet.Configuration(),
showCheckbox: showCheckbox,
accountService: MockAccountService(shouldFailLookup: shouldFailLookup),
previousCustomerInput: nil,
linkAccount: linkAccount,
country: country
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,7 @@
B615E8752CA4CC38007D684C /* EmbeddedPaymentElementDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = B615E8742CA4CC38007D684C /* EmbeddedPaymentElementDelegate.swift */; };
B61E2C202C5C44FE0045B5CF /* PaymentSheetAnalyticsHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = B61E2C1F2C5C44FE0045B5CF /* PaymentSheetAnalyticsHelper.swift */; };
B626EE932BF2872200B05B05 /* PaymentMethodTypeImageView.swift in Sources */ = {isa = PBXBuildFile; fileRef = B626EE922BF2872200B05B05 /* PaymentMethodTypeImageView.swift */; };
B62934F22CAE0ACC009F7C8F /* CardSectionElementTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B62934F12CAE0ACC009F7C8F /* CardSectionElementTest.swift */; };
B63B2CF12BF8313D003810F3 /* VerticalPaymentMethodListViewControllerTest.swift in Sources */ = {isa = PBXBuildFile; fileRef = B63B2CF02BF8313D003810F3 /* VerticalPaymentMethodListViewControllerTest.swift */; };
B63B2CF32BFBEE7B003810F3 /* VerticalPaymentMethodListViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B63B2CF22BFBEE7B003810F3 /* VerticalPaymentMethodListViewController.swift */; };
B63B2CF52BFBEEAD003810F3 /* PaymentMethodFormViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B63B2CF42BFBEEAD003810F3 /* PaymentMethodFormViewController.swift */; };
Expand Down Expand Up @@ -582,6 +583,7 @@
B61E2C1F2C5C44FE0045B5CF /* PaymentSheetAnalyticsHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentSheetAnalyticsHelper.swift; sourceTree = "<group>"; };
B61FFE76D0960C7F1E34B405 /* PaymentSheetAppearance.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentSheetAppearance.swift; sourceTree = "<group>"; };
B626EE922BF2872200B05B05 /* PaymentMethodTypeImageView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentMethodTypeImageView.swift; sourceTree = "<group>"; };
B62934F12CAE0ACC009F7C8F /* CardSectionElementTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CardSectionElementTest.swift; sourceTree = "<group>"; };
B63B2CF02BF8313D003810F3 /* VerticalPaymentMethodListViewControllerTest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VerticalPaymentMethodListViewControllerTest.swift; sourceTree = "<group>"; };
B63B2CF22BFBEE7B003810F3 /* VerticalPaymentMethodListViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VerticalPaymentMethodListViewController.swift; sourceTree = "<group>"; };
B63B2CF42BFBEEAD003810F3 /* PaymentMethodFormViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = PaymentMethodFormViewController.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1408,6 +1410,7 @@
3F70E5F0FE6218715432D55A /* AddPaymentMethodViewControllerSnapshotTests.swift */,
25AB69CF78A2B13839EB32EC /* AddressViewController */,
BE92D55DA4B4D449734B2917 /* BacsDDMandateViewSnapshotTests.swift */,
B62934F12CAE0ACC009F7C8F /* CardSectionElementTest.swift */,
0C53358C028E528F0FC626A2 /* CustomerSheet */,
989C2E3E03E42DA64A2FAE0D /* CustomerSheetSnapshotTests.swift */,
31699A822BE183D40048677F /* DownloadManagerTest.swift */,
Expand Down Expand Up @@ -1750,6 +1753,7 @@
4A1A0A542B824C830A200BE0 /* StubbedBackend.swift in Sources */,
6151DDC02B14FDCF00ED4F7E /* UpdateCardViewControllerSnapshotTests.swift in Sources */,
34CF08CBC636F596B8BA4C12 /* TextFieldElement+CardTest.swift in Sources */,
B62934F22CAE0ACC009F7C8F /* CardSectionElementTest.swift in Sources */,
52B734BA0B91706F37025523 /* STPAnalyticsClient+PaymentSheetTests.swift in Sources */,
AE8EF3966E7BABDFC3B426ED /* PaymentSheet+DashboardConfirmParamsTest.swift in Sources */,
B65B42972C013DED00EC565D /* PaymentMethodFormViewControllerTest.swift in Sources */,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,11 @@
@_spi(STP) import StripeUICore
import UIKit

final class LinkInlineSignupElement: Element {
// TODO: Refactor this to be a ContainerElement and contain its sub-elements.
final class LinkInlineSignupElement: PaymentMethodElement {
let collectsUserInput: Bool = true

private let signupView: LinkInlineSignupView
let signupView: LinkInlineSignupView

lazy var view: UIView = {

Expand All @@ -39,12 +40,14 @@ final class LinkInlineSignupElement: Element {
configuration: PaymentSheet.Configuration,
linkAccount: PaymentSheetLinkAccount?,
country: String?,
showCheckbox: Bool
showCheckbox: Bool,
previousCustomerInput: IntentConfirmParams?
) {
self.init(viewModel: LinkInlineSignupViewModel(
configuration: configuration,
showCheckbox: showCheckbox,
accountService: LinkAccountService(apiClient: configuration.apiClient),
previousCustomerInput: previousCustomerInput?.linkInlineSignupCustomerInput,
linkAccount: linkAccount,
country: country
))
Expand All @@ -55,6 +58,15 @@ final class LinkInlineSignupElement: Element {
self.signupView.delegate = self
}

func updateParams(params: IntentConfirmParams) -> IntentConfirmParams? {
params.linkInlineSignupCustomerInput = .init(
phoneNumber: signupView.phoneNumberElement.phoneNumber,
name: signupView.nameElement.text,
email: signupView.emailElement.emailAddressString,
checkboxSelected: signupView.checkboxElement.isChecked
)
return params
}
}

extension LinkInlineSignupElement: LinkInlineSignupViewDelegate {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ extension LinkInlineSignupView {
private let appearance: PaymentSheet.Appearance
/// Controls the stroke color of the checkbox
private let borderColor: UIColor
let initialIsSelectedValue: Bool

var view: UIView {
return checkboxButton
Expand Down Expand Up @@ -56,15 +57,16 @@ extension LinkInlineSignupView {

let checkbox = CheckboxButton(text: text, description: description, theme: appearanceCopy.asElementsTheme)
checkbox.addTarget(self, action: #selector(didToggleCheckbox), for: .touchUpInside)
checkbox.isSelected = false
checkbox.isSelected = initialIsSelectedValue

return checkbox
}()

init(merchantName: String, appearance: PaymentSheet.Appearance, borderColor: UIColor) {
init(merchantName: String, appearance: PaymentSheet.Appearance, borderColor: UIColor, isSelected: Bool) {
self.merchantName = merchantName
self.appearance = appearance
self.borderColor = borderColor
self.initialIsSelectedValue = isSelected
}

func setUserInteraction(isUserInteractionEnabled: Bool) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,12 @@ final class LinkInlineSignupView: UIView {
private(set) lazy var checkboxElement = CheckboxElement(
merchantName: viewModel.configuration.merchantDisplayName,
appearance: viewModel.configuration.appearance,
borderColor: borderColor
borderColor: borderColor,
isSelected: viewModel.saveCheckboxChecked
)

private(set) lazy var emailElement: LinkEmailElement = {
let element = LinkEmailElement(defaultValue: viewModel.emailAddress,
let element = LinkEmailElement(defaultValue: viewModel.initialEmail ?? viewModel.emailAddress,
isOptional: viewModel.isEmailOptional,
showLogo: viewModel.mode != .textFieldsOnlyPhoneFirst,
theme: theme)
Expand All @@ -44,7 +45,7 @@ final class LinkInlineSignupView: UIView {
}()

private(set) lazy var nameElement: TextFieldElement = {
let configuration = TextFieldElement.NameConfiguration(type: .full, defaultValue: viewModel.legalName)
let configuration = TextFieldElement.NameConfiguration(type: .full, defaultValue: viewModel.initialName ?? viewModel.legalName)
return TextFieldElement(configuration: configuration, theme: theme)
}()

Expand All @@ -53,15 +54,28 @@ final class LinkInlineSignupView: UIView {
// Otherwise, we'd imply consumer consent when it hasn't occurred.
switch viewModel.mode {
case .checkbox:
let defaultCountryCode = viewModel.initialPhoneNumber?.countryCode ?? viewModel.configuration.defaultBillingDetails.address.country
let defaultPhoneNumber = viewModel.initialPhoneNumber?.number ?? viewModel.configuration.defaultBillingDetails.phone
return PhoneNumberElement(
defaultCountryCode: viewModel.configuration.defaultBillingDetails.address.country,
defaultPhoneNumber: viewModel.configuration.defaultBillingDetails.phone,
defaultCountryCode: defaultCountryCode,
defaultPhoneNumber: defaultPhoneNumber,
theme: theme
)
case .textFieldsOnlyEmailFirst:
return PhoneNumberElement(isOptional: viewModel.isPhoneNumberOptional, theme: theme)
return PhoneNumberElement(
defaultCountryCode: viewModel.initialPhoneNumber?.countryCode,
defaultPhoneNumber: viewModel.initialPhoneNumber?.number,
isOptional: viewModel.isPhoneNumberOptional,
theme: theme
)
case .textFieldsOnlyPhoneFirst:
return PhoneNumberElement(isOptional: viewModel.isPhoneNumberOptional, infoView: LinkMoreInfoView(), theme: theme)
return PhoneNumberElement(
defaultCountryCode: viewModel.initialPhoneNumber?.countryCode,
defaultPhoneNumber: viewModel.initialPhoneNumber?.number,
isOptional: viewModel.isPhoneNumberOptional,
infoView: LinkMoreInfoView(),
theme: theme
)
}
}()

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ class LinkEmailElement: Element {

weak var delegate: ElementDelegate?

private let emailAddressElement: TextFieldElement
let emailAddressElement: TextFieldElement

private let activityIndicator: ActivityIndicator = {
// TODO: Consider adding the activity indicator to TextFieldView
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,13 @@ protocol LinkInlineSignupViewModelDelegate: AnyObject {
func signupViewModelDidUpdate(_ viewModel: LinkInlineSignupViewModel)
}

struct LinkInlineSignupCustomerInput: Equatable {
let phoneNumber: PhoneNumber?
let name: String?
let email: String?
let checkboxSelected: Bool?
}

final class LinkInlineSignupViewModel {
enum Action: Equatable {
case signupAndPay(account: PaymentSheetLinkAccount, phoneNumber: PhoneNumber, legalName: String?)
Expand All @@ -38,8 +45,11 @@ final class LinkInlineSignupViewModel {
let configuration: PaymentSheet.Configuration

let mode: Mode
let initialEmail: String?
let initialPhoneNumber: PhoneNumber?
let initialName: String?

var saveCheckboxChecked: Bool = false {
var saveCheckboxChecked: Bool {
didSet {
if saveCheckboxChecked != oldValue {
notifyUpdate()
Expand Down Expand Up @@ -291,13 +301,18 @@ final class LinkInlineSignupViewModel {
configuration: PaymentSheet.Configuration,
showCheckbox: Bool,
accountService: LinkAccountServiceProtocol,
previousCustomerInput: LinkInlineSignupCustomerInput?,
linkAccount: PaymentSheetLinkAccount? = nil,
country: String? = nil
) {
self.configuration = configuration
self.accountService = accountService
self.linkAccount = linkAccount
self.emailAddress = linkAccount?.email
self.saveCheckboxChecked = previousCustomerInput?.checkboxSelected ?? false
self.initialEmail = previousCustomerInput?.email
self.initialPhoneNumber = previousCustomerInput?.phoneNumber
self.initialName = previousCustomerInput?.name
if let email = self.emailAddress,
!email.isEmpty {
emailWasPrefilled = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ final class IntentConfirmParams {

var financialConnectionsLinkedBank: FinancialConnectionsLinkedBank?
var instantDebitsLinkedBank: InstantDebitsLinkedBank?
/// Hack: Contains the customer input in the link inline signup element (e.g. email, checkbox state) so that it can be preserved across `FlowController.update` etc.
var linkInlineSignupCustomerInput: LinkInlineSignupCustomerInput?

var paymentSheetLabel: String {
if let last4 = (financialConnectionsLinkedBank?.last4 ?? instantDebitsLinkedBank?.last4) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ extension PaymentSheetFormFactory {
configuration: configuration,
linkAccount: linkAccount,
country: countryCode,
showCheckbox: !shouldDisplaySaveCheckbox
showCheckbox: !shouldDisplaySaveCheckbox,
previousCustomerInput: previousCustomerInput
)
elements.append(inlineSignupElement)
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
//
// CardSectionElementTest.swift
// StripePaymentSheetTests
//
// Created by Yuki Tokuhiro on 10/2/24.
//

@testable@_spi(STP) import StripeCore
@testable@_spi(STP) import StripePayments
@testable@_spi(STP) import StripePaymentSheet
@testable@_spi(STP) import StripePaymentsTestUtils
@testable@_spi(STP) import StripePaymentsUI
@testable@_spi(STP) import StripeUICore
import XCTest

@MainActor
final class CardSectionElementTest: XCTestCase {
let window: UIWindow = UIWindow(frame: .init(x: 0, y: 0, width: 428, height: 926))

func testLinkSignupPreservesPreviousCustomerInput() async {
await PaymentSheetLoader.loadMiscellaneousSingletons()
func makeForm(previousCustomerInput: IntentConfirmParams?) -> PaymentMethodElement {
let intent: Intent = ._testPaymentIntent(paymentMethodTypes: [.card])
let formVC = PaymentMethodFormViewController(
type: .stripe(.card),
intent: intent,
elementsSession: ._testValue(paymentMethodTypes: ["card"], isLinkPassthroughModeEnabled: true),
previousCustomerInput: previousCustomerInput,
formCache: .init(),
configuration: .init(),
headerView: nil,
analyticsHelper: ._testValue(),
delegate: self
)

// Add to window to avoid layout errors due to zero size and presentation errors
window.rootViewController = formVC

// Simulate view appearance. This makes SimpleMandateElement mark its mandate as having been displayed.
formVC.viewDidAppear(false)
return formVC.form
}
let form = makeForm(previousCustomerInput: nil)
let linkInlineSignupElement: LinkInlineSignupElement = form.getElement()!
let linkInlineView = linkInlineSignupElement.signupView
XCTAssertNotNil(linkInlineView.checkboxElement) // Checkbox should appear since this is a PI w/o customer
form.getTextFieldElement("Card number")?.setText("4242424242424242")
form.getTextFieldElement("MM / YY").setText("1232")
form.getTextFieldElement("CVC").setText("123")
form.getTextFieldElement("ZIP").setText("65432")

// Simulate selecting checkbox
linkInlineView.checkboxElement.isChecked = true
linkInlineView.checkboxElement.didToggleCheckbox()

// Set the email & phone number
let email = "\(UUID().uuidString)@foo.com"
linkInlineView.emailElement.emailAddressElement.setText(email)
linkInlineView.phoneNumberElement.countryDropdownElement.setRawData("GB")
linkInlineView.phoneNumberElement.textFieldElement.setText("1234567890")
linkInlineView.nameElement.setText("John Doe")

// Generate params from the form
guard let intentConfirmParams = form.updateParams(params: IntentConfirmParams(type: .stripe(.card))) else {
XCTFail("Form failed to create params. Validation state: \(form.validationState) \n Form: \(form)")
return
}

// Re-generate the form and validate that it carries over all previous customer input
let regeneratedForm = makeForm(previousCustomerInput: intentConfirmParams)
let regeneratedLinkInlineSignupElement: LinkInlineSignupElement = regeneratedForm.getElement()!
let regeneratedLinkInlineView = linkInlineSignupElement.signupView
guard let regeneratedIntentConfirmParams = regeneratedForm.updateParams(params: IntentConfirmParams(type: .stripe(.card))) else {
XCTFail("Regenerated form failed to create params. Validation state: \(regeneratedForm.validationState) \n Form: \(regeneratedForm)")
return
}
XCTAssertEqual(regeneratedIntentConfirmParams, intentConfirmParams)
XCTAssertTrue(regeneratedLinkInlineSignupElement.isChecked)
XCTAssertEqual(regeneratedLinkInlineView.emailElement.emailAddressString, email)
XCTAssertEqual(regeneratedLinkInlineView.nameElement.text, "John Doe")
XCTAssertEqual(regeneratedLinkInlineView.phoneNumberElement.phoneNumber, PhoneNumber(number: "1234567890", countryCode: "GB"))
}
}

extension CardSectionElementTest: PaymentMethodFormViewControllerDelegate {
func didUpdate(_ viewController: StripePaymentSheet.PaymentMethodFormViewController) {

}

func updateErrorLabel(for error: (any Error)?) {

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -979,11 +979,15 @@ extension IntentConfirmParams: Equatable {
print("Instant debits linked banks not equal: \(lhs.instantDebitsLinkedBank.debugDescription) vs \(rhs.instantDebitsLinkedBank.debugDescription)")
return false
}
if lhs.linkInlineSignupCustomerInput != rhs.linkInlineSignupCustomerInput {
print("Link inline signup customer input not equal: \(lhs.linkInlineSignupCustomerInput.debugDescription) vs \(rhs.linkInlineSignupCustomerInput.debugDescription)")
return false
}

// Sanity check to make sure when we add new properties, we check them here
let mirror = Mirror(reflecting: lhs)
let propertyCount = mirror.children.count
XCTAssertEqual(propertyCount, 7)
XCTAssertEqual(propertyCount, 8)

return true
}
Expand Down
Loading