Add Authorize/Capture for PayPalWebPayments in Demo App #216

KunJeongPark · 2023-11-07T19:28:47Z

Summary of changes

Create authorize/capture on order in demo app after successful checkout

PayPalWebAuth.mp4

Checklist

~~- [ ] Added a changelog entry~~

Authors

@KunJeongPark

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalOrderActionButton.swift

sshropshire

🌊

Demo/Demo/Models/Order.swift

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalTransactionButtonsView.swift

Demo/Demo/Models/Order.swift

scannillo · 2023-11-13T16:31:56Z

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalWebOrderCompletionResultView.swift

@@ -0,0 +1,53 @@
+import SwiftUI
+
+struct PayPalWebOrderCompletionResultView: View {


The names of these classes confuse me a bit. What is the difference b/w PayPalWebOrderCompletionResultView & PayPalWebOrderCompletionView?

I have this view (PayPalOrderCompletionView) where I carry over order approval results to second screen to display authorize/capture results (PayPalWebOrderCompletionResultView)

If the order completion has already taken place maybe PayPalWebResultView is more appropriate?

There are several results in these flows. For PayPalWeb, I have order creation result, approval result and the order completion result (result of authorize/capture). So I am distinguishing them with the names.

scannillo · 2023-11-13T16:38:45Z

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalWebOrderCompletionView.swift

@@ -0,0 +1,38 @@
+import SwiftUI
+
+struct PayPalWebOrderCompletionView: View {


This class looks pretty much the exact same as CardPaymentOrderCompletionView.swift. The views should be re-usable across payment method/ view model types.

This way if we want to update this view, we do it in 1 place versus multiple.

See comment below.

scannillo · 2023-11-13T16:47:40Z

Demo/Demo/ViewModels/PayPalWebViewModel.swift

+    func captureOrder(orderID: String, selectedMerchantIntegration: MerchantIntegration) async throws {
+        do {
+            DispatchQueue.main.async {
+                self.state.capturedOrderResponse = .loading
+            }
+            let order = try await DemoMerchantAPI.sharedService.captureOrder(
+                orderID: orderID,
+                selectedMerchantIntegration: selectedMerchantIntegration
+            )
+            DispatchQueue.main.async {
+                self.state.capturedOrderResponse = .loaded(order)
+            }
+        } catch {
+            DispatchQueue.main.async {
+                self.state.capturedOrderResponse = .error(message: error.localizedDescription)
+            }
+            print("Error capturing order: \(error.localizedDescription)")
+        }
+    }
+
+    func authorizeOrder(orderID: String, selectedMerchantIntegration: MerchantIntegration) async throws {
+        do {
+            DispatchQueue.main.async {
+                self.state.authorizedOrderResponse = .loading
+            }
+            let order = try await DemoMerchantAPI.sharedService.authorizeOrder(
+                orderID: orderID,
+                selectedMerchantIntegration: selectedMerchantIntegration
+            )
+            DispatchQueue.main.async {
+                self.state.authorizedOrderResponse = .loaded(order)
+            }
+        } catch {
+            DispatchQueue.main.async {
+                self.state.authorizedOrderResponse = .error(message: error.localizedDescription)
+            }
+            print("Error authorizing order: \(error.localizedDescription)")
+        }
+    }


Same here, this looks like a copy-paste from CardPaymentViewModel (see snippet). That duplication is usually a good sign that we should refactor.

Happy to talk through some refactor options. We could go the route of a parent class or a protocol to represent a generic PaymentMethodViewModel that has a set of default functionality that is re-usable across all payment methods (ex: auth, capture).

That is a great suggestion. I will start with PaymentMethodViewModel protocol and incorporate it into common views.

@scannillo , I looked into making a PaymentMethodViewModel protocol and extension to provide common default authorizeOrder and captureOrder function implementations and I ran into issue of having to change stored properties(states) in the conforming types. Protocol extensions, as I understand, cannot mutate stored properties of conforming types. For instance:

self.state.authorizedOrderResponse = .loaded(order)

I could provide in PaymentMethodViewModel protocol delegate methods like didAuthorizeOrder and didCaptureOrder but it would reduce duplication only slightly and introduce complexity.
I am looking into protocol with associatedType but it doesn't look pretty imo.

I ran into similar issue with factoring out views that looks the same - state in each ViewModel is of different type. I would need to use associated types and also run into issue of observing different properties of states of different types for each viewModels.

I think benefits of reducing a bit of code duplication doesn't justify the complexity introduced.

It seems like there could be a few options in this StackOverflow that may be worth exploring: https://stackoverflow.com/questions/38885622/swift-protocol-extensions-property-default-values. Not sure if Sammy had a different approach in mind as well!

I will continue working w PaymentMethodViewModel protocol. I guess all both flows have views for order creation result, order approval result and order capture/authorize result.

I created another ticket for refactoring common views and functionalities in the viewModels. https://paypal.atlassian.net/browse/DTPPCPSDK-1437

PayPal.xcodeproj/project.pbxproj

jaxdesmarais · 2023-11-15T15:58:42Z

Demo/Demo/SwiftUIComponents/CardPaymentViews/OrderCreateCardResultView.swift

@@ -1,6 +1,6 @@
 import SwiftUI

-struct OrderCreateCardPaymentResultView: View {
+struct OrderCreateCardResultView: View {


In the refactor ticket can we please include updating some of these names? This name doesn't seem to match up with what the code in this file is doing and it's not clear why it's being changed since there are no other code changes in this file. I don't think the original name was super clear but it's especially confusing we made this change with no other changes here.

Basically we have result views for each Orders API calls to check if they are working as expected.
so there are OrderCreateCardResultView, OrderCreatePayPalWebResultView, etc.
And there are also container views since I didn't want them to be all on one scrollable page.

It sounds like this will be a great thing to refactor or can be shared in some way across payment methods since most of the logic is the same. I think that will help with naming as then we can have a singular OrderResultView.

That is what I want to investigate in the refactor ticket. How to organize the different states to minimize views where it makes sense. Most of the result views have different content. There are two that are identical, order create result views and the "action buttons" for authorize/capture. To me, it's easier to follow if there are separate views for each API call result so we can find them more easily but I will explore different options.

That's why I wanted to create a different PR for the refactor as it will be quite a bit of code change in the viewModels and some backtracking and modifications based on feedback. I wanted to be able to track these changes.

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalOrderActionButton.swift

jaxdesmarais · 2023-11-15T16:07:58Z

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalTransactionsView.swift

@@ -1,7 +1,7 @@
 import SwiftUI
 import PaymentButtons

-struct PayPalTransactionButtonsView: View {
+struct PayPalTransactionsView: View {


Suggested change

struct PayPalTransactionsView: View {

struct PayPalTransactionView: View {

There are three options of transactions on that view PayPal, PayPal Credit, Pay Later. That's why I put Transactions. What do you think?

I think the singular is fine even though it's used in multiple places there will ever only be 1 transaction

jaxdesmarais

Thanks for addressing all of my questions/suggestions!

Demo/Demo/ViewModels/PayPalWebViewModel.swift

jaxdesmarais · 2023-11-16T14:34:58Z

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalOrderActionButton.swift

+                try await paypalWebViewModel.completeOrder(
+                    with: intent,
+                    orderID: orderID,
+                    selectedMerchantIntegration: selectedMerchantIntegration
+                )


KunJeongPark · 2023-11-16T15:44:56Z

Demo/Demo/ViewModels/PayPalWebViewModel.swift

add authorize/capture for PayPalWebPayments

691dc9b

KunJeongPark requested a review from a team as a code owner November 7, 2023 19:28

KunJeongPark changed the title ~~Add Authorize/apture for PayPalWebPayments~~ Add Authorize/Capture for PayPalWebPayments in Demo App Nov 7, 2023

sshropshire reviewed Nov 7, 2023

View reviewed changes

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalOrderActionButton.swift Outdated Show resolved Hide resolved

sshropshire approved these changes Nov 7, 2023

View reviewed changes

jaxdesmarais reviewed Nov 7, 2023

View reviewed changes

Demo/Demo/Models/Order.swift Outdated Show resolved Hide resolved

Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalTransactionButtonsView.swift Outdated Show resolved Hide resolved

Demo/Demo/Models/Order.swift Show resolved Hide resolved

KunJeongPark added 3 commits November 7, 2023 14:11

add missing pbxproj changes, remove name header

263b3d7

spacing PayPalTransactionButtonView

293097b

display sandbox email in demo app

e8d1cf5

scannillo reviewed Nov 13, 2023

View reviewed changes

PayPal.xcodeproj/project.pbxproj Outdated Show resolved Hide resolved

KunJeongPark added 2 commits November 13, 2023 14:32

Revert project.pbxproj to match main

80ac172

Change view names for consistency between PayPalWeb and Card

1bdfd45

jaxdesmarais reviewed Nov 15, 2023

View reviewed changes

Jax PR feedback

ac238c5

jaxdesmarais approved these changes Nov 16, 2023

View reviewed changes

Update Demo/Demo/ViewModels/PayPalWebViewModel.swift

9fff03a

KunJeongPark commented Nov 16, 2023

View reviewed changes

Demo/Demo/ViewModels/PayPalWebViewModel.swift

Copy link

Collaborator Author

KunJeongPark Nov 16, 2023 •

edited

Loading

Choose a reason for hiding this comment

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

Thank you!

KunJeongPark merged commit dbc73a7 into main Nov 16, 2023
4 checks passed

KunJeongPark deleted the paypalAuthorizeCapture branch November 16, 2023 15:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Authorize/Capture for PayPalWebPayments in Demo App #216

Add Authorize/Capture for PayPalWebPayments in Demo App #216

KunJeongPark commented Nov 7, 2023 •

edited

Loading

sshropshire left a comment

scannillo Nov 13, 2023

KunJeongPark Nov 13, 2023 •

edited

Loading

jaxdesmarais Nov 14, 2023

KunJeongPark Nov 14, 2023

scannillo Nov 13, 2023

KunJeongPark Nov 14, 2023

scannillo Nov 13, 2023

KunJeongPark Nov 13, 2023 •

edited

Loading

KunJeongPark Nov 14, 2023 •

edited

Loading

jaxdesmarais Nov 14, 2023

KunJeongPark Nov 14, 2023 •

edited

Loading

KunJeongPark Nov 14, 2023

jaxdesmarais Nov 15, 2023

KunJeongPark Nov 15, 2023

jaxdesmarais Nov 15, 2023

KunJeongPark Nov 15, 2023 •

edited

Loading

jaxdesmarais Nov 15, 2023

KunJeongPark Nov 15, 2023

jaxdesmarais Nov 15, 2023

jaxdesmarais left a comment

jaxdesmarais Nov 16, 2023

KunJeongPark Nov 16, 2023 •

edited

Loading

		@@ -0,0 +1,53 @@
		import SwiftUI

		struct PayPalWebOrderCompletionResultView: View {

		@@ -0,0 +1,38 @@
		import SwiftUI

		struct PayPalWebOrderCompletionView: View {

	struct PayPalTransactionsView: View {
	struct PayPalTransactionView: View {

Add Authorize/Capture for PayPalWebPayments in Demo App #216

Add Authorize/Capture for PayPalWebPayments in Demo App #216

Conversation

KunJeongPark commented Nov 7, 2023 • edited Loading

Summary of changes

Checklist

Authors

sshropshire left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KunJeongPark Nov 13, 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

KunJeongPark Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

KunJeongPark Nov 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KunJeongPark Nov 14, 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

KunJeongPark Nov 15, 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

jaxdesmarais left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KunJeongPark Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

KunJeongPark commented Nov 7, 2023 •

edited

Loading

KunJeongPark Nov 13, 2023 •

edited

Loading

KunJeongPark Nov 13, 2023 •

edited

Loading

KunJeongPark Nov 14, 2023 •

edited

Loading

KunJeongPark Nov 14, 2023 •

edited

Loading

KunJeongPark Nov 15, 2023 •

edited

Loading

KunJeongPark Nov 16, 2023 •

edited

Loading