PayPal Web Demo App Refactor #223

jaxdesmarais · 2023-11-21T16:46:37Z

Reason for changes

There was a lot of logic that's currently duplicated across views or using the same type. This PR overall reduces the number of states used in the PayPal Demo app. It also includes refactors that can be used or duplicated across other demo apps. As necessary we can extract out portions of this code to be reused as we refactor the other portions of the demo app.

Summary of changes

Create CurrentState enum - this should be able to be shared in other demo features
Rename PayPalWebView to PayPalWebDemoView to reduce confusion with using the term "web view" - this is the entry point to the feature
Rename PayPalTransactionView to PayPalWebButtonsView - this view contains the PayPal buttons so the name did not accurately represent the contents of the view
Rename CreateOrderPayPalWebView to PayPalWebCreateOrderView to match other file naming conventions
Removed unneeded PayPalWebOrderCompletionView and PayPalWebState
Singular PayPalWebResultView can now be used vs the previous PayPalWebApprovalResultView, OrderCreatePayPalWebResultView, PayPalWebOrderCompletionResultView
Created new PayPalWebStatusView that contains logic for what to display based on the status vs having this in multiple views
Created new PayPalWebTransactionView which contains the view for authorizing/capturing an order (replaces PayPalWebOrderActionButton)
PayPalWebViewModel changes
- Moved logic into the ViewModel directly where views didn't need to control logic for certain pieces of code - dumb views down where possible
- Update logic as needed to update state as expected
- Cleanup method signatures where possible

Future Considerations / Improvements

For the most part aside from updating labels this PR keeps the UI 1:1 with what exists today - we should consider if we want to improve the UI further in a future update or if this is our desired flow
For the PayPal Web flow do we need to display 3 buttons with 3 different funding sources or are we okay just demo-ing 1 button (on BT we have all 3 so may make sense to leave as is)
There are likely pieces of code that can be extracted out and shared among other features such as buttons - most of them add the same attributes so we can consider this in a future refactor
Once all features are complete we should clean up the file structure of each feature and ensure the demo app is well organized and easy to navigate
There are some classes that can be removed entirely like CustomButton and CustomTestField these are UIKit Views and no longer used at all in the Demo app - there are likely other things we can also consider cleaning up

Next Steps

Pick the next view to tackle!

Checklist

~~[ ] Added a changelog entry~~

Authors

@jaxdesmarais

… Web

…replace result view with status view so they do not dissapear on change for transaction view

jaxdesmarais · 2023-11-21T16:49:42Z

Demo/Demo/Models/Order.swift

@@ -2,8 +2,8 @@ struct Order: Codable, Equatable {

    let id: String
    let status: String
-    var paymentSource: PaymentSource?
-
+    let paymentSource: PaymentSource?


This isn't related to this PR but at one point I was poking around at this file and noticed we had this one as a variable - should be a constant to match all other structs in this file

jaxdesmarais · 2023-11-21T16:50:21Z

Demo/Demo/SwiftUIComponents/CurrentState.swift

@@ -0,0 +1,10 @@
+import Foundation
+
+enum CurrentState: Equatable {


Ideally we can use this across the other demo features as well vs the individual states per feature that exist today

Demo/Demo/SwiftUIComponents/FeatureSelectionView.swift

jaxdesmarais · 2023-11-21T16:53:08Z

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebButtonsView.swift

@@ -1,24 +1,23 @@
 import SwiftUI
 import PaymentButtons

-struct PayPalTransactionView: View {
+struct PayPalWebButtonsView: View {


Renamed from "TransactionView" to "ButtonView" since this contains all of the payment buttons and does not do anything with transactions

jaxdesmarais · 2023-11-21T16:54:16Z

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebCreateOrderView.swift

@@ -1,13 +1,11 @@
 import SwiftUI

-struct CreateOrderPayPalWebView: View {
+struct PayPalWebCreateOrderView: View {


We may be able to reuse some sort of create order view across all of the features - left separate for now but could be something to consider as we continue this refactor

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebResultView.swift

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebStatusView.swift

jaxdesmarais · 2023-11-21T16:59:24Z

Demo/Demo/ViewModels/PayPalWebViewModel.swift

-
-        let amountRequest = Amount(currencyCode: "USD", value: amount)
+    func createOrder() async throws {
+        let amountRequest = Amount(currencyCode: "USD", value: "10.00")


We were hardcoding this amount in a view so just hardcoding it here instead - if we add an amount text box in the future we can update this method signature to take in an amount

jaxdesmarais · 2023-11-21T17:00:36Z

Demo/Demo/ViewModels/PayPalWebViewModel.swift

-    func paypalWebCheckoutSuccessResult(checkoutResult: PayPalWebState.CheckoutResult) {
-        DispatchQueue.main.async {
-            self.state.checkoutResultResponse = .loaded(checkoutResult)
-        }
-    }
-
-    func paypalWebCheckoutFailureResult(checkoutError: CorePayments.CoreSDKError) {
-        DispatchQueue.main.async {
-            self.state.checkoutResultResponse = .error(message: checkoutError.localizedDescription)
-        }
-    }


These methods are only doing 1 thing so moved this logic into the delegates directly vs calling a method

jaxdesmarais · 2023-11-21T17:06:02Z

Sources/PayPalWebPayments/PayPalWebCheckoutFundingSource.swift

@@ -6,6 +6,7 @@ public enum PayPalWebCheckoutFundingSource: String {
    /// Eligible costumers receive a revolving line of credit that they can use to pay over time.
    case paypalCredit = "credit"

+    // NEXT_MAJOR_VERSION: rename to `payLater`


Noticed we have the casing weird here so added this next major version note

jaxdesmarais · 2023-11-21T17:24:10Z

Demo/Demo/Networking/DemoMerchantAPI.swift

@@ -53,7 +53,21 @@ final class DemoMerchantAPI {
            throw error
        }
    }
-
+
+    func completeOrder(intent: Intent, orderID: String) async throws -> Order {


When we refactor card we should use this singular method - the only difference between authorize and capture is the intent so we don't need 2 separate methods for that

sshropshire

🌊

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebStatusView.swift

scannillo · 2023-11-30T21:38:28Z

Demo/Demo.xcodeproj/project.pbxproj

@@ -33,12 +31,9 @@
 		3BA56FF82A9FDB5A0081D14F /* CardPaymentOrderCompletionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3BA56FF72A9FDB5A0081D14F /* CardPaymentOrderCompletionView.swift */; };
 		3BA56FFA2A9FE4180081D14F /* CardOrderCompletionResultView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3BA56FF92A9FE4180081D14F /* CardOrderCompletionResultView.swift */; };
 		3BA56FFC2A9FEFE90081D14F /* PayPalWebViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3BA56FFB2A9FEFE90081D14F /* PayPalWebViewModel.swift */; };


Could be for another PR, but could we rename PayPalViewModel.swift --> PayPalNativeViewModel.swift to follow the same convention as the Web one? Or is there a plan to further consolidate the 2?

Yeah, I was leaving that for a follow up PR when we do those files as a whole. I think they will need to be separate but agree that one should be renamed to PayPalNative.

Right now we also have a separate directory for the Views vs the ViewModels. Do we like that pattern or should we move each features ViewModel into it's top level directory? It could be nice for merchants to just look in 1 place for everything related to a feature vs traversing multiple directories.

I like that idea - breaking it up by feature instead. Things are a little hard to find ATM

Agreed! Moved: 5a95765

scannillo · 2023-11-30T21:49:21Z

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebResultView.swift

+    var body: some View {
+        switch payPalWebViewModel.state {
+        case .idle, .loading:
+            EmptyView()


Would it be better to show some text like "Loading ..." onscreen vs empty?

So we do add a loading indicator to the button when the state is loading (example here). But I don't think we want the entire view to show the loading indicator which is why we set the View to EmptyView here. Once the state is updated from loading we display the entire result view.

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebStatusView.swift

Demo/Demo/ViewModels/PayPalWebViewModel.swift

scannillo · 2023-11-30T21:52:30Z

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebStatusView.swift

+                    LeadingText("Intent", weight: .bold)
+                    LeadingText("\(payPalViewModel.intent)")
+                    LeadingText("Order ID", weight: .bold)
+                    LeadingText("\(order.id)")


These 4 lines should apply the same to each case, right? Maybe we can pull the intent & orderID part out of the switch statement

They don't apply to all of them, each view has the following:

Order Started: order created header with order ID and status

Order Approved: order approved header with intent, order ID, and payer ID

Order Completed: order intent header with order ID, status, and optional email, customer ID, vault ID

I posed the question here of do we need different data for different states. We could consider displaying the same data for all views and adding additional data in later states.

I am not totally sure why we need to display the status for created and the intent + payer ID for approved. Do we find this data useful? If not we can still update the headers but display a more consistent set of data. What do you think?

I agree, the results should correlate with the steps that are performed.

Is there a reason we only need status for order started and completed? Same with intent and payer ID only for approved?

Imo I don't think we need status displayed at all. Instead we could have: order ID, and intent for all views. Then we could append payer ID with approved and append email, customer ID and vault ID with completed. What do you think?

Actually I take that back - I think we do want different views here otherwise we end up updating all views based on the latest order data. For now I think it makes sense to have some data tied to status to avoid updating the earlier views on other screens.

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebStatusView.swift

…pdate logic into methods

# Conflicts: # Demo/Demo/Models/Order.swift # Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebCreateOrderView.swift # Demo/Demo/SwiftUIComponents/PayPalWebViews/PayPalWebOrderCompletionResultView.swift # Demo/Demo/ViewModels/PayPalWebViewModel.swift

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebButtonsView.swift

Demo/Demo/SwiftUIComponents/PayPalWeb/PayPalWebTransactionView.swift

… status

Demo/Demo/SwiftUIComponents/PayPalWebPayments/PayPalWebTransactionView.swift

Demo/Demo/SwiftUIComponents/PayPalWebPayments/PayPalWebPaymentsView.swift

KunJeongPark · 2023-12-05T11:59:27Z

Adding the TransactionOrder in viewModel on auth/capture order works as well to distinguish the last step.
Great job renaming the files, and consolidating the views!

jaxdesmarais added 15 commits November 17, 2023 11:14

consolidate order states in PayPal Web views

b34ed3f

add generic SuccessView

32c4b9b

WIP - shared result view

e327686

WIP - start consolidating result views

9d956dd

WIP - continue refactoring state for PayPal Web

bc4a9e3

WIP - continued paypal web demo app refactor

d685080

WIP - start cleaning up file names; method signatures; etc for PayPal…

8aed081

… Web

cleanup how data is passed

4474449

add new state and general cleanup

72aaa7a

cleanup funding source name, remove redundant types

5a02090

cleanup views and remove order completion view

9e8019b

rename PayPalWebCompleteTransactionView to PayPalWebTransactionView; …

d96b216

…replace result view with status view so they do not dissapear on change for transaction view

update funding source

994a3ac

extract intent from view model directly

3dd128e

minor cleanup

c01d5c5

jaxdesmarais requested a review from a team as a code owner November 21, 2023 16:46

sort paypal web files by name

664f56c