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.

Sign up for GitHub

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

Jump to bottom

MBL-1123: Refactor PaymentSourceSelected to be an enum #1990

Merged
merged 2 commits into from
Mar 21, 2024
+71 −130
Merged

MBL-1123: Refactor PaymentSourceSelected to be an enum #1990

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e8058be
MBL-1123: Refactor PaymentSourceSelected to be an enum, instead of a …
amy-at-kickstarter Mar 20, 2024
5bb1591
Tweak per review #1990
amy-at-kickstarter Mar 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions Kickstarter.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1525,6 +1525,7 @@
E17611E22B73D9A400DF2F50 /* Data+PKCE.swift in Sources */ = {isa = PBXBuildFile; fileRef = E17611E12B73D9A400DF2F50 /* Data+PKCE.swift */; };
E17611E42B751E8100DF2F50 /* Paginator.swift in Sources */ = {isa = PBXBuildFile; fileRef = E17611E32B751E8100DF2F50 /* Paginator.swift */; };
E17611E62B75242A00DF2F50 /* PaginatorTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E17611E52B75242A00DF2F50 /* PaginatorTests.swift */; };
E1801FAA2BAB6D0900EBB533 /* PaymentSourceSelected.swift in Sources */ = {isa = PBXBuildFile; fileRef = E1801FA82BAB6CD400EBB533 /* PaymentSourceSelected.swift */; };
E182E5BA2B8CDFDE0008DD69 /* AppEnvironmentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A7ED1F121E830FDC00BFFA01 /* AppEnvironmentTests.swift */; };
E182E5BC2B8D36FE0008DD69 /* AppEnvironmentTests+OAuthInKeychain.swift in Sources */ = {isa = PBXBuildFile; fileRef = E182E5BB2B8D36FE0008DD69 /* AppEnvironmentTests+OAuthInKeychain.swift */; };
E1A1491E2ACDD76800F49709 /* FetchBackerProjectsQuery.graphql in Resources */ = {isa = PBXBuildFile; fileRef = E1A1491D2ACDD76700F49709 /* FetchBackerProjectsQuery.graphql */; };
Expand Down Expand Up @@ -3148,6 +3149,7 @@
E17611E12B73D9A400DF2F50 /* Data+PKCE.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Data+PKCE.swift"; sourceTree = "<group>"; };
E17611E32B751E8100DF2F50 /* Paginator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Paginator.swift; sourceTree = "<group>"; };
E17611E52B75242A00DF2F50 /* PaginatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaginatorTests.swift; sourceTree = "<group>"; };
E1801FA82BAB6CD400EBB533 /* PaymentSourceSelected.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PaymentSourceSelected.swift; sourceTree = "<group>"; };
E182E5BB2B8D36FE0008DD69 /* AppEnvironmentTests+OAuthInKeychain.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "AppEnvironmentTests+OAuthInKeychain.swift"; sourceTree = "<group>"; };
E1889D8D2B6065D6004FBE21 /* CombineTestObserverTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CombineTestObserverTests.swift; sourceTree = "<group>"; };
E1A1491D2ACDD76700F49709 /* FetchBackerProjectsQuery.graphql */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text; path = FetchBackerProjectsQuery.graphql; sourceTree = "<group>"; };
Expand Down Expand Up @@ -6529,6 +6531,7 @@
D710ADFA243FB94A00DC7199 /* PledgeViewCTAContainerViewModel.swift */,
D710ADFC2441172100DC7199 /* PledgeViewCTAContainerViewModelTests.swift */,
37DEC1E62257C9F30051EF9B /* PledgeViewModel.swift */,
E1801FA82BAB6CD400EBB533 /* PaymentSourceSelected.swift */,
37DEC21F2257CA0A0051EF9B /* PledgeViewModelTests.swift */,
395A3BC82BA8D43F0091A379 /* PostCampaignCheckoutViewModel.swift */,
6044532D2BA0905600B8F485 /* PostCampaignPledgeRewardsSummaryViewModel.swift */,
Expand Down Expand Up @@ -7698,6 +7701,7 @@
D7ADDFE722E0DAFA00157D83 /* RewardCellProjectBackingStateType.swift in Sources */,
D79D61151FD0979D000F958B /* String+Base64.swift in Sources */,
7727494723FC66EA0065E9F2 /* CategorySelectionViewModel.swift in Sources */,
E1801FAA2BAB6D0900EBB533 /* PaymentSourceSelected.swift in Sources */,
77EFBAE32268D48A00DA5C3C /* RewardsCollectionViewModel.swift in Sources */,
3705CF0F222EE7670025D37E /* EnvironmentVariables.swift in Sources */,
A73379601D0EDFEE00C91445 /* UIViewController-Preparation.swift in Sources */,
Expand Down
23 changes: 23 additions & 0 deletions Library/ViewModels/PaymentSourceSelected.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import Foundation
public enum PaymentSourceSelected: Equatable {
case paymentSourceId(String)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Suggestions for better names welcome - as far as I know, this is an ID that comes from GraphQL and represents a particular payment methods stored in our own DB.

case setupIntentClientSecret(String)

public var paymentSourceId: String? {
switch self {
case let .paymentSourceId(value):
return value
default:
return nil
}
}

public var setupIntentClientSecret: String? {
switch self {
case let .setupIntentClientSecret(value):
return value
default:
return nil
}
}
}
10 changes: 2 additions & 8 deletions Library/ViewModels/PledgePaymentMethodsViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,15 +259,9 @@ public final class PledgePaymentMethodsViewModel: PledgePaymentMethodsViewModelT

switch (selectedPaymentMethodCardId, selectedPaymentSheetPaymentMethodCardId) {
case let (.none, .some(selectedPaymentSheetPaymentMethodCardId)):
return PaymentSourceSelected(
paymentSourceId: selectedPaymentSheetPaymentMethodCardId,
isSetupIntentClientSecret: true
)
return PaymentSourceSelected.setupIntentClientSecret(selectedPaymentSheetPaymentMethodCardId)
case let (.some(selectedPaymentMethodCardId), .none):
return PaymentSourceSelected(
paymentSourceId: selectedPaymentMethodCardId,
isSetupIntentClientSecret: false
)
return PaymentSourceSelected.paymentSourceId(selectedPaymentMethodCardId)
Comment on lines -263 to +264
Copy link
Contributor

Choose a reason for hiding this comment

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

so much clearer!

default:
return nil
}
Expand Down
19 changes: 7 additions & 12 deletions Library/ViewModels/PledgePaymentMethodsViewModelTests.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -740,7 +740,7 @@ final class PledgePaymentMethodsViewModelTests: TestCase {
self.scheduler.run()

self.notifyDelegateCreditCardSelected.assertValues(
[PaymentSourceSelected(paymentSourceId: UserCreditCards.amex.id, isSetupIntentClientSecret: false)],
[PaymentSourceSelected.paymentSourceId(UserCreditCards.amex.id)],
"First card selected by default"
)

Expand All @@ -752,11 +752,8 @@ final class PledgePaymentMethodsViewModelTests: TestCase {
self.vm.inputs.didSelectRowAtIndexPath(discoverIndexPath)

self.notifyDelegateCreditCardSelected.assertValues([
PaymentSourceSelected(paymentSourceId: UserCreditCards.amex.id, isSetupIntentClientSecret: false),
PaymentSourceSelected(
paymentSourceId: UserCreditCards.discover.id,
isSetupIntentClientSecret: false
)
PaymentSourceSelected.paymentSourceId(UserCreditCards.amex.id),
PaymentSourceSelected.paymentSourceId(UserCreditCards.discover.id)
])
}
}
Expand All @@ -774,7 +771,7 @@ final class PledgePaymentMethodsViewModelTests: TestCase {
self.scheduler.run()

self.notifyDelegateCreditCardSelected.assertValues(
[PaymentSourceSelected(paymentSourceId: UserCreditCards.visa.id, isSetupIntentClientSecret: false)],
[PaymentSourceSelected.paymentSourceId(UserCreditCards.visa.id)],
"First card selected by default"
)

Expand All @@ -793,11 +790,9 @@ final class PledgePaymentMethodsViewModelTests: TestCase {
)

self.notifyDelegateCreditCardSelected.assertValues([
PaymentSourceSelected(paymentSourceId: UserCreditCards.visa.id, isSetupIntentClientSecret: false),
PaymentSourceSelected(
paymentSourceId: "seti_1LVlHO4VvJ2PtfhK43R6p7FI_secret_MEDiGbxfYVnHGsQy8v8TbZJTQhlNKLZ",
isSetupIntentClientSecret: true
)
PaymentSourceSelected.paymentSourceId(UserCreditCards.visa.id),
PaymentSourceSelected
.setupIntentClientSecret("seti_1LVlHO4VvJ2PtfhK43R6p7FI_secret_MEDiGbxfYVnHGsQy8v8TbZJTQhlNKLZ")
])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a good example of how these are different now.

}
}
Expand Down
59 changes: 18 additions & 41 deletions Library/ViewModels/PledgeViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ import PassKit
import Prelude
import ReactiveSwift

public struct PaymentSourceSelected: Equatable {
let paymentSourceId: String
let isSetupIntentClientSecret: Bool
}

public typealias StripeConfigurationData = (merchantIdentifier: String, publishableKey: String)
public typealias CreateBackingData = (
project: Project,
Expand Down Expand Up @@ -394,14 +389,8 @@ public class PledgeViewModel: PledgeViewModelType, PledgeViewModelInputs, Pledge
)
}

/// The `selectedPaymentSourceId` will do as it did before taking the payment source id (A) or the setup intent client secret (B), one or the other for comparison against the existing backing payment source id. It does not care which of two payment sources the id refers to.
let selectedPaymentSourceId = Signal.merge(
initialData.mapConst(nil),
self.creditCardSelectedSignal.map { $0.paymentSourceId }.wrapInOptional()
)

/// The `selectedPaymentSourceIdOrSetupIntentClientSecret` will take the payment source id (A) or the setup intent client secret (B) and map only to `createBackingData` or `updateBackingData`
let selectedPaymentSourceIdOrSetupIntentClientSecret = Signal.merge(
/// The `selectedPaymentSource` will take the payment source id (A) or the setup intent client secret (B) and map only to `createBackingData` or `updateBackingData`
let selectedPaymentSource = Signal.merge(
initialData.mapConst(nil),
self.creditCardSelectedSignal.wrapInOptional()
)
Expand Down Expand Up @@ -556,7 +545,7 @@ public class PledgeViewModel: PledgeViewModelType, PledgeViewModelInputs, Pledge
pledgeTotal,
selectedQuantities,
selectedShippingRule,
selectedPaymentSourceIdOrSetupIntentClientSecret,
selectedPaymentSource,
applePayParamsData,
refTag
)
Expand All @@ -566,19 +555,13 @@ public class PledgeViewModel: PledgeViewModelType, PledgeViewModelInputs, Pledge
pledgeTotal,
selectedQuantities,
selectedShippingRule,
selectedPaymentSourceIdOrSetupIntentClientSecret,
selectedPaymentSource,
applePayParams,
refTag
) -> CreateBackingData in
var paymentSourceId: String?
var setupIntentClientSecret: String?

if let isSetupIntentClientSecretAvailable = selectedPaymentSourceIdOrSetupIntentClientSecret {
paymentSourceId = isSetupIntentClientSecretAvailable
.isSetupIntentClientSecret ? nil : isSetupIntentClientSecretAvailable.paymentSourceId
setupIntentClientSecret = isSetupIntentClientSecretAvailable
.isSetupIntentClientSecret ? isSetupIntentClientSecretAvailable.paymentSourceId : nil
}

var paymentSourceId = selectedPaymentSource?.paymentSourceId
var setupIntentClientSecret = selectedPaymentSource?.setupIntentClientSecret

return (
project: project,
Expand Down Expand Up @@ -639,7 +622,7 @@ public class PledgeViewModel: PledgeViewModelType, PledgeViewModelInputs, Pledge
pledgeTotal,
selectedQuantities,
selectedShippingRule,
selectedPaymentSourceIdOrSetupIntentClientSecret,
selectedPaymentSource,
applePayParamsData
)
.map { (
Expand All @@ -648,18 +631,11 @@ public class PledgeViewModel: PledgeViewModelType, PledgeViewModelInputs, Pledge
pledgeTotal,
selectedQuantities,
selectedShippingRule,
selectedPaymentSourceIdOrSetupIntentClientSecret,
selectedPaymentSource,
applePayParams
) -> UpdateBackingData in
var paymentSourceId: String?
var setupIntentClientSecret: String?

if let isSetupIntentClientSecretAvailable = selectedPaymentSourceIdOrSetupIntentClientSecret {
paymentSourceId = isSetupIntentClientSecretAvailable
.isSetupIntentClientSecret ? nil : isSetupIntentClientSecretAvailable.paymentSourceId
setupIntentClientSecret = isSetupIntentClientSecretAvailable
.isSetupIntentClientSecret ? isSetupIntentClientSecretAvailable.paymentSourceId : nil
}
var paymentSourceId = selectedPaymentSource?.paymentSourceId
var setupIntentClientSecret = selectedPaymentSource?.setupIntentClientSecret

return (
backing: backing,
Expand Down Expand Up @@ -774,7 +750,7 @@ public class PledgeViewModel: PledgeViewModelType, PledgeViewModelInputs, Pledge
Signal.combineLatest(
project,
baseReward,
self.creditCardSelectedSignal.map { $0.paymentSourceId },
self.creditCardSelectedSignal,
context
)
.map(paymentMethodValid)
Expand Down Expand Up @@ -956,11 +932,12 @@ public class PledgeViewModel: PledgeViewModelType, PledgeViewModelInputs, Pledge
let willRetryPaymentMethod = Signal.combineLatest(
context,
project,
selectedPaymentSourceId
selectedPaymentSource
)
.map { context, project, selectedPaymentSourceId -> Bool in
.map { context, project, selectedPaymentSource -> Bool in

context == .fixPaymentMethod
&& project.personalization.backing?.paymentSource?.id == selectedPaymentSourceId
&& project.personalization.backing?.paymentSource?.id == selectedPaymentSource?.paymentSourceId
}
.skipRepeats()

Expand Down Expand Up @@ -1335,7 +1312,7 @@ private func shippingRuleValid(
private func paymentMethodValid(
project: Project,
reward: Reward,
paymentSourceId: String,
paymentSource: PaymentSourceSelected,
context: PledgeViewContext
) -> Bool {
guard
Expand All @@ -1348,7 +1325,7 @@ private func paymentMethodValid(

if project.personalization.backing?.status == .errored {
return true
} else if backedPaymentSourceId != paymentSourceId {
} else if backedPaymentSourceId != paymentSource.paymentSourceId {
return true
}

Expand Down