Vault Without Purchase PayPal #225

KunJeongPark · 2023-11-30T15:09:19Z

Summary of changes

Change PaymentSource in demo app to allow SetUpToken creation for both card and PayPal
Implement parent class VaultViewModel inherited by CardVaultViewModel and PayPalVaultViewModel
to reuse common properties and views.
Rename CardVaultState to VaultState to define all common properties, move payment specific properties as
published properties in their respective view models
Implement getting url from SetUpToken request
Create UI for PayPal Vault without Purchase
Implement vault(url:) function to launch ASWebSession
Parse results from ASWebSession
Define PayPalVaultDelegate protocol, methods. Add PayPalVaultDelegate protocol conformance to PayPalVaultViewModel and implement methods in PayPalVaultViewModel
Update states for UI with vault method.
Handle success, failure results from vault(url:) in SDK through PayPalWebCheckoutClient's vault delegate functions
Merchant server side PaymentToken call and Demo app output
Unit tests for vault(url:) function

PayPalVaultNPVid.mp4

Checklist

Added a changelog entry

Authors

@KunJeongPark

KunJeongPark · 2023-12-07T23:55:21Z

Demo/Demo/ViewModels/VaultState.swift

@@ -12,6 +12,7 @@ struct CardVaultState: Equatable {
    var setupToken: SetUpTokenResponse?
    var updateSetupToken: UpdateSetupTokenResult?
    var paymentToken: PaymentTokenResponse?
+    var paypalVaultToken: String?



I am wondering if I should add cancel state in LoadingState.
I do provide delegate function for cancel so merchant can handle it the way they want
but I didn't include it as a separate state in the demo app because it didn't make sense for me to
display anything for canceling.

I think it depends on the Merchant's needs. For now, we can leave it as is; It LGTM. In fact, if you'd like, we can even consider the approach used in the func cardDidCancel(_ cardClient: CardPayments.CardClient) within the CardPaymentViewModel, where we use the .idle state.

KunJeongPark · 2023-12-08T00:00:36Z

Demo/Demo/ViewModels/VaultState.swift

@@ -21,6 +22,14 @@ struct CardVaultState: Equatable {
        }
    }

+    var paypalVaultTokenResponse: LoadingState<String> = .idle {
+        didSet {


I think I prefer the separate variables of loading states for each API response.
This way, I can have a result view to toggle errorView and successView.
I prefer separated result views here instead of toggling generic one with different options from the
container view, especially with just a few different result views here that are shared across different payment methods.

KunJeongPark · 2023-12-08T07:54:27Z

I've been thinking about the approach to minimize the number of views in the demo app as suggested in previous PR's such as suggestion to use general view that toggles content for each API result and approach that I explored above, which was to use parent viewModel between two payment methods for vaulting without payment so that SetupTokenView and PaymentTokenView and their result views can be reused across different payment methods when applicable.

I considered other payment methods being added as stand alone vault features in this approach to reducing the number of views by having a parent VaultViewModel class with shared state and payment specific properties as published properties in their respective viewModels. In JS, so far we have PayPal and Card vaulting and in Braintree, we have PayPal, Card and Venmo vaulting if I remember correctly.

Assuming that purpose of our demo app is for internal testing, demonstration of features and giving example usage of SDK to merchants, I think simplicity might be the best approach.

We can adapt as we add more payment methods to vault. As far as I know, the flow for vault without pay is the same: create setupToken, do approval in SDK with payment method and merchants use this temporary setupToken to create paymentToken.

Co-authored-by: Ricardo Herrera <<[email protected]>

KunJeongPark · 2023-12-10T23:07:01Z

Demo/Demo/ViewModels/PayPalVaultViewModel.swift

+class PayPalVaultViewModel: VaultViewModel, PayPalVaultDelegate {
+
+    @Published var paypalVaultToken: PayPalVaultResult?
+


I considered having same VaultResult for card and PayPal vaulting but UpdateSetupToken endpoint
for card vaulting returns a lot more values than url string from PayPal approval web session, so I opted to keep them separate in case there is request in future to return more values to merchant.

If only minimum values that I display here are needed, they can be consolidated into a single VaultResult and VaultView and included in the VaultState.

From my perspective, communicating the viewModel with the view without using the State structure somewhat breaks the paradigm. For now, I think it's better to move PayPalVaultViewModel.paypalVaultTokenResponse and CardVaultViewModel.updateSetupTokenResponse back to VaultState, as it was in previous commits. Maybe someone else on the team could provide some input. Considering that the purpose of the Demo app is all about internal testing, showcase functionalities, and providing merchants with examples of SDK usage

Yes, I thought I might get comment that it's confusing to be able to access updateSetupToken from PayPalVaultViewModel but I prefer them all in vaultState as well.

richherrera · 2023-12-11T20:21:06Z

Demo/Demo/SwiftUIComponents/VaultViews/SetupTokenResultView.swift

@@ -28,6 +28,10 @@ struct SetupTokenResultView: View {
            LeadingText("\(setupTokenResponse.customer?.id ?? "")")
            LeadingText("Status", weight: .bold)
            LeadingText("\(setupTokenResponse.status)")
+            if let url = setupTokenResponse.paypalURL {
+                LeadingText("PayPalURL", weight: .bold)
+                LeadingText("\(setupTokenResponse.paypalURL ?? "No url")")


Suggested change

LeadingText("\(setupTokenResponse.paypalURL ?? "No url")")

LeadingText(url)

Thank you for catching that.

richherrera · 2023-12-12T00:54:02Z

Demo/Demo/ViewModels/VaultState.swift


-struct CardVaultState: Equatable {
+struct VaultState: Equatable {

    struct UpdateSetupTokenResult: Decodable, Equatable {


This structure is duplicated on CardVaultViewModel. I see that we don't have any rules for handling nested structures in the States structures. For now, I think we should just define which one to work with

that was a mistake leaving it there. Good catch.

richherrera · 2023-12-12T01:13:14Z

Demo/Demo/SwiftUIComponents/CardVaultViews/CardVaultView.swift

                    )
-                    SetupTokenResultView(cardVaultViewModel: cardVaultViewModel)
+                    SetupTokenResultView(vaultViewModel: cardVaultViewModel)


nitpick: How about we just use viewModel instead of specifying which viewModel it is (cardVaultViewModel or vaulViewModel)? It's most convenient for a view to have only one ViewModel, so it's not really necessary to specify the type of ViewModel in the parameter

Suggested change

SetupTokenResultView(vaultViewModel: cardVaultViewModel)

SetupTokenResultView(viewModel: cardVaultViewModel)

that's true, we are only dealing with vaultViewModels here, so it's redundant.

UnitTests/PayPalWebPaymentsTests/PayPalWebCheckoutClient_Tests.swift

richherrera · 2023-12-12T01:22:01Z

Demo/Demo/ViewModels/PayPalVaultViewModel.swift

+
+class PayPalVaultViewModel: VaultViewModel, PayPalVaultDelegate {
+
+    @Published var paypalVaultToken: PayPalVaultResult?


nitpick: I think renaming it to paypalVaultResult would be the most convenient option. This name was from before the refactor. What do you think?

Suggested change

@Published var paypalVaultToken: PayPalVaultResult?

@Published var paypalVaultResult: PayPalVaultResult?

I remember why I used Token as the name: to be consistent with updateSetupToken of UpdateSetupTokenResult type for card vaulting. In the same vault state, especially, I wanted the to have same naming convention

richherrera · 2023-12-12T03:31:43Z

Demo/Demo/ViewModels/VaultState.swift

@@ -12,6 +12,7 @@ struct CardVaultState: Equatable {
    var setupToken: SetUpTokenResponse?
    var updateSetupToken: UpdateSetupTokenResult?
    var paymentToken: PaymentTokenResponse?
+    var paypalVaultToken: String?



I think it depends on the Merchant's needs. For now, we can leave it as is; It LGTM. In fact, if you'd like, we can even consider the approach used in the func cardDidCancel(_ cardClient: CardPayments.CardClient) within the CardPaymentViewModel, where we use the .idle state.

richherrera · 2023-12-12T03:52:26Z

Demo/Demo/ViewModels/PayPalVaultViewModel.swift

+class PayPalVaultViewModel: VaultViewModel, PayPalVaultDelegate {
+
+    @Published var paypalVaultToken: PayPalVaultResult?
+


From my perspective, communicating the viewModel with the view without using the State structure somewhat breaks the paradigm. For now, I think it's better to move PayPalVaultViewModel.paypalVaultTokenResponse and CardVaultViewModel.updateSetupTokenResponse back to VaultState, as it was in previous commits. Maybe someone else on the team could provide some input. Considering that the purpose of the Demo app is all about internal testing, showcase functionalities, and providing merchants with examples of SDK usage

scannillo · 2023-12-12T15:44:50Z

Demo/Demo/Models/PaymentTokenResponse.swift

@@ -14,7 +14,8 @@ struct Customer: Codable, Equatable {

 struct PaymentSource: Decodable, Equatable {

-    let card: BasicCard
+    var card: BasicCard?
+    var paypal: BasicPayPal?
 }

 struct BasicCard: Decodable, Equatable {


What does BasicCard and BasicPayPal mean?

meaning they contain very minimal results like last four digits and email.

2cents: On Android we went with CardPaymentSource and PayPalPaymentSource. JSON encoding of this request is weird on Android too.

scannillo · 2023-12-12T15:48:29Z

Demo/Demo/Models/SetupTokenRequest.swift

+        var paymentSourceDict: [String: Any] = [:]
+
+        switch paymentSource {
+        case .card:
+            paymentSourceDict["card"] = [:]
+        case .paypal(let usageType):
+            paymentSourceDict["paypal"] = [
+                "usage_type": usageType,
+                "experience_context": [
+                    "return_url": "sdk.ios.paypal://vault/success",
+                    "cancel_url": "sdk.ios.paypal://vault/cancel"
+                ]
            ]
+        }
+
+        let requestBody: [String: Any] = [
+            "customer": ["id": customerID],
+            "payment_source": paymentSourceDict
        ]

        return try? JSONSerialization.data(withJSONObject: requestBody)


Instead of using a raw dict + JSONSerizliation, throughout the rest of the demo app we conform structs to Encodable & then use JSONEncoder

We should keep things consistent

Hey Sammy, I did this because I didn't know a way of using struct to make blank payment JSON object
{
"payment_source":
"card:" {}
}

I know when we get the update setup token endpoint to work with custom url scheme, we will use experience context field but right now, it errors out with that field set with our custom url scheme so I am sending an empty card in the card vault request.

I remember looking at this a while back and not finding a quick answer on Encodable support for empty objects. Maybe we can use an empty struct to represent [:] like here?

I've never seen this. Thanks Steven. I will check it out.

We found EmptyParams()

sshropshire · 2023-12-12T20:08:37Z

Demo/Demo/SwiftUIComponents/PayPalVaultViews/PayPalVaultResultView.swift

+
+struct PayPalVaultResultView: View {
+
+    @ObservedObject var paypalVaultViewModel: PayPalVaultViewModel


Suggested change

@ObservedObject var paypalVaultViewModel: PayPalVaultViewModel

@ObservedObject var viewModel: PayPalVaultViewModel

🧶 : Similar to Rich's callout, since the var is strongly typed here we can omit type from the name.

Forgot to fix this one on last PR feedback fix commit

sshropshire · 2023-12-12T20:33:20Z

Demo/Demo/Models/SetupTokenResponse.swift

+    var paypalURL: String? {
+        if let link = links.first(where: { $0.rel == "approve" }) {
+            let url = link.href
+            return url
+        }
+        return nil
+    }


Suggested change

var paypalURL: String? {

if let link = links.first(where: { $0.rel == "approve" }) {

let url = link.href

return url

}

return nil

}

var paypalURL: String? {

return links.first { $0.rel == "approve" }?.href

}

🧶 : ^ would be technically equivalent.

😅

Suggested change

var paypalURL: String? {

if let link = links.first(where: { $0.rel == "approve" }) {

let url = link.href

return url

}

return nil

}

var paypalURL: String? {

links.first { $0.rel == "approve" }?.href

}

Oh nice haha. 🙌

Co-authored-by: Ricardo Herrera <[email protected]>

KunJeongPark added 8 commits November 29, 2023 22:00

setupTokenPaymentSource

8d1cc4e

setup token output, UI for PP vault

6b76523

vault(url:) method, custom url scheme

a103662

merge conflict with files moved, deleted

31125dd

comma omission typo in demo pbxproj file

3cc3d1d

Display parsed vault token

21cd8dc

PaymentToken endpoint and output

0b9f8fe

add MockPayPalVaultDelegate and unit tests

690f8b3

KunJeongPark commented Dec 7, 2023

View reviewed changes

KunJeongPark marked this pull request as ready for review December 7, 2023 23:59

KunJeongPark requested a review from a team as a code owner December 7, 2023 23:59

KunJeongPark commented Dec 8, 2023

View reviewed changes

add comments on PayPalVaultDelegate

4d2831b

KunJeongPark added 5 commits December 8, 2023 11:34

PayPalVaultResult to include approvalSessionID

34fb320

Co-authored-by: Ricardo Herrera <<[email protected]>

merge conflict resolution

12f58ed

move payment specific properties out of parent viewModel

9fb823f

make PayPalResultView look consisitent with CardVaultResultView

8e1e84c

Create new VaultViews folder, move files

4c35a89

KunJeongPark commented Dec 10, 2023

View reviewed changes

richherrera reviewed Dec 12, 2023

View reviewed changes

scannillo reviewed Dec 12, 2023

View reviewed changes

Rich PR feedback

9363cc4

sshropshire reviewed Dec 12, 2023

View reviewed changes

sshropshire approved these changes Dec 12, 2023

View reviewed changes

sshropshire reviewed Dec 12, 2023

View reviewed changes

KunJeongPark and others added 2 commits December 12, 2023 15:29

Encodable for setupToken payment source, pr feedback

d8f8f66

Co-authored-by: Ricardo Herrera <[email protected]>

vault function comments and changelog

578854d

Co-authored-by: Ricardo Herrera <[email protected]>

richherrera approved these changes Dec 14, 2023

View reviewed changes

KunJeongPark merged commit 52f2106 into main Dec 18, 2023
4 checks passed

KunJeongPark deleted the vaultWithoutPurchasePayPal branch December 18, 2023 14:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Vault Without Purchase PayPal #225

Vault Without Purchase PayPal #225

KunJeongPark commented Nov 30, 2023 •

edited

Loading

KunJeongPark Dec 7, 2023

richherrera Dec 12, 2023

KunJeongPark Dec 8, 2023

KunJeongPark commented Dec 8, 2023 •

edited

Loading

KunJeongPark Dec 10, 2023 •

edited

Loading

richherrera Dec 12, 2023

KunJeongPark Dec 12, 2023 •

edited

Loading

richherrera Dec 11, 2023

KunJeongPark Dec 12, 2023 •

edited

Loading

richherrera Dec 12, 2023

KunJeongPark Dec 12, 2023

richherrera Dec 12, 2023

KunJeongPark Dec 12, 2023

richherrera Dec 12, 2023

KunJeongPark Dec 12, 2023

richherrera Dec 12, 2023

richherrera Dec 12, 2023

scannillo Dec 12, 2023

KunJeongPark Dec 12, 2023

sshropshire Dec 12, 2023

scannillo Dec 12, 2023

KunJeongPark Dec 12, 2023

KunJeongPark Dec 12, 2023 •

edited

Loading

sshropshire Dec 12, 2023

KunJeongPark Dec 12, 2023

KunJeongPark Dec 12, 2023

sshropshire Dec 12, 2023

KunJeongPark Dec 12, 2023

sshropshire Dec 12, 2023

richherrera Dec 12, 2023

sshropshire Dec 12, 2023

		class PayPalVaultViewModel: VaultViewModel, PayPalVaultDelegate {

		@Published var paypalVaultToken: PayPalVaultResult?

	LeadingText("\(setupTokenResponse.paypalURL ?? "No url")")
	LeadingText(url)

	SetupTokenResultView(vaultViewModel: cardVaultViewModel)
	SetupTokenResultView(viewModel: cardVaultViewModel)

	@Published var paypalVaultToken: PayPalVaultResult?
	@Published var paypalVaultResult: PayPalVaultResult?


		struct PayPalVaultResultView: View {

		@ObservedObject var paypalVaultViewModel: PayPalVaultViewModel

	@ObservedObject var paypalVaultViewModel: PayPalVaultViewModel
	@ObservedObject var viewModel: PayPalVaultViewModel

Vault Without Purchase PayPal #225

Vault Without Purchase PayPal #225

Conversation

KunJeongPark commented Nov 30, 2023 • edited Loading

Summary of changes

Checklist

Authors

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KunJeongPark commented Dec 8, 2023 • edited Loading

KunJeongPark Dec 10, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KunJeongPark Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KunJeongPark Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KunJeongPark Dec 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KunJeongPark commented Nov 30, 2023 •

edited

Loading

KunJeongPark commented Dec 8, 2023 •

edited

Loading

KunJeongPark Dec 10, 2023 •

edited

Loading

KunJeongPark Dec 12, 2023 •

edited

Loading

KunJeongPark Dec 12, 2023 •

edited

Loading

KunJeongPark Dec 12, 2023 •

edited

Loading