[MBL-1194] Implement Validate Checkout #1999

scottkicks · 2024-03-26T01:56:26Z

📲 What

When tapping pledge, we need to run this mutation so the backend can do some extra validations before we ask Stripe to do its thing.

For New Payment Methods

We'll use the payment intent that is passed back to the VM via PaymentSourceSelected
We can then call Stripe's STPAPIClient.shared.retrievePaymentIntent to get the required paymentSourceId (paymentMethodId in Stripe)

For Pre-Existing Payment Methods that were originally created with a setup intent

We'll need to call Create Payment Intent again to pass to this mutation
We'll also need to use the selected existing card's id to filter the user's stored cards and grab the stripeCardId

I opted to make separate calls to this mutation depending on the case since they require enough distinct information to warrant it. There might be an opportunity to remove some duplicate code so if there are any nits please call them out!

The mutation accepts the following params:

the checkoutId (from the [MBL-1273] Implement Create Checkout Mutation #1982 on ConfirmDetailsViewController). One update to this was made. Validate Checkout requires that the checkoutId be decoded to base 64.
the paymentMethodId that we can pull in using, STPAPIClient.shared.retrievePaymentIntent(withClientSecret: clientSecret) for new cards, or the stripeCardId property on existing cards.
and the paymentIntentClientSecret that we create using the backend.

Extras

I also updated PostCampaignCheckoutViewController to use our MessageBannerViewControllerPresenting to show error messages when an API call fails.

It's important to note that in the case of a failure, the expected UX is to navigate the backer back to the ConfirmDetails screen so that we can being the process of creating a checkout Id/validating/etc. before attempting another pledge. This is because once we confirm the payment with Stripe, the backend will need to invalidate that request, essentially processing a refund to the backer and invalidating our previous validations.

🤔 Why

Gotta validate, ya dig?

✅ Acceptance criteria

Tapping Pledge, after adding a card or selecting an existing card, successfully runs the validateCheckout mutation with the correct values.

⏰ TODO

Next step is to confirm the payment intent via Stripe.confirmPayment()

…id' into scott/pcp/validate-checkout-confirm-payment

…ent secret

…/github.com/kickstarter/ios-oss into scott/pcp/validate-checkout-confirm-payment

Library/ViewModels/ConfirmDetailsViewModel.swift

amy-at-kickstarter

Mostly LGTM, but a couple things to address first:

How will we handle saved credit cards (which have no PaymentIntent)?
Why do we need to decode the GraphID of a Checkout? Can this be resolved at the mutation level, instead of doing it in the client?

A few non-blocking nits:

Is the VC the correct place to call Stripe? Should it be in the VM or will that break testability?
Some miscellanea/style comments

Also left a few nits/comments, but those aren't blocking.

Kickstarter-iOS/Features/PledgeView/Controllers/PostCampaignCheckoutViewController.swift

Library/ViewModels/ConfirmDetailsViewModel.swift

Library/ViewModels/PostCampaignCheckoutViewModel.swift

scottkicks · 2024-03-26T14:32:14Z

@amy-at-kickstarter Looking into your other comments.

Why do we need to decode the GraphID of a Checkout? Can this be resolved at the mutation level, instead of doing it in the client?

I spoke with the backend, and they would rather we did this. It sounded like they could (they do in other cases), but they aren't willing to spend the time updating the different pieces involved right now.

* unrelated quick fixes

Library/ViewModels/PostCampaignCheckoutViewModel.swift

scottkicks · 2024-03-26T19:36:33Z

Library/ViewModels/ConfirmDetailsViewModel.swift

@@ -211,7 +211,7 @@ public class ConfirmDetailsViewModel: ConfirmDetailsViewModelType, ConfirmDetail

    /// Hide when there is a reward and shipping is enabled (accounts for digital rewards), and in a pledge context
    self.pledgeSummaryViewHidden = Signal.zip(baseReward, context).map { baseReward, context in
-      (baseReward.isNoReward == false && baseReward.shipping.enabled) && context == .pledge
+      baseReward.isNoReward == false && context == .pledge


unrelated fix for showing the summary view on details

Kickstarter-iOS/Features/ConfirmDetails/Controllers/ConfirmDetailsViewController.swift

amy-at-kickstarter

LGTM with some questions and nits

amy-at-kickstarter · 2024-03-26T20:16:21Z

Kickstarter-iOS/Features/PledgeView/Controllers/PostCampaignCheckoutViewController.swift

+      .observeValues { [weak self] _ in
+        guard let self else { return }
+
+        // TODO: Confirm paymentIntent using Stripe.confirmPayment()


Is this // TODO: correct for the existing card case, too?

Yes we'll have to confirm the payment either way using the intent

amy-at-kickstarter · 2024-03-26T20:17:24Z

Kickstarter-iOS/Features/PledgeView/Controllers/PostCampaignCheckoutViewController.swift

+          return
+        }
+
+        let paymentMethodId = intent.paymentMethodId!


Nit: Could also throw this let into the above guard statement, just in case.

amy-at-kickstarter · 2024-03-26T20:17:32Z

Kickstarter-iOS/Features/PledgeView/Controllers/PostCampaignCheckoutViewController.swift

+      }
+    case let .savedCreditCard(savedCardId):
+      self.viewModel.inputs
+        .creditCardSelected(source: paymentSource, paymentMethodId: savedCardId, isNewPaymentMethod: false)


amy-at-kickstarter · 2024-03-26T20:18:04Z

KsApi/models/graphql/UserCreditCards.swift

@@ -9,6 +9,7 @@ public struct UserCreditCards: Decodable {
    public var id: String
    public var lastFour: String
    public var type: CreditCardType?
+    public var stripeCardId: String?


Ah, yeah, I only added it to the GraphQL object. Good find.

amy-at-kickstarter · 2024-03-26T20:18:22Z

Library/ViewModels/ConfirmDetailsViewModel.swift

@@ -211,7 +211,7 @@ public class ConfirmDetailsViewModel: ConfirmDetailsViewModelType, ConfirmDetail

    /// Hide when there is a reward and shipping is enabled (accounts for digital rewards), and in a pledge context
    self.pledgeSummaryViewHidden = Signal.zip(baseReward, context).map { baseReward, context in
-      (baseReward.isNoReward == false && baseReward.shipping.enabled) && context == .pledge
+      baseReward.isNoReward == false && context == .pledge


amy-at-kickstarter · 2024-03-26T20:18:55Z

Library/ViewModels/ConfirmDetailsViewModel.swift

+        }
+
+        return checkoutId
+      }


Any resolution on why this needs to be decoded from GraphQL? At a minimum, this should really have a //TODO: explaining the hack.

Ah yea i left an unfortunate comment earlier.

its silly that they would give us a string, that we only use to send to one of their mutations, and not format it correctly for us

amy-at-kickstarter · 2024-03-26T20:19:47Z

Library/ViewModels/PostCampaignCheckoutViewModel.swift

+      }
+
+    let storedCardsValues = storedCardsEvent.values()
+      .filter(second >>> isFalse)


What does this do?

Nit: Would prefer this to be a non-Prelude operator for clarity.

amy-at-kickstarter · 2024-03-26T20:21:45Z

Library/ViewModels/PostCampaignCheckoutViewModel.swift

+    let selectedExistingCard = self.creditCardSelectedProperty.signal.skipNil()
+      .filter { $0.isNewPaymentMethod == false }
+
+    let newPaymentIntentEvent = initialData


This only needs to happen when you're not adding a new card via the payment method selector, since in that case, we've already made a PI for you. Should this be attached to a different event than initialData?

amy-at-kickstarter · 2024-03-26T20:23:39Z

Library/ViewModels/PostCampaignCheckoutViewModel.swift

+    let selectedNewCreditCard = self.creditCardSelectedProperty.signal.skipNil()
+      .filter { $0.isNewPaymentMethod }
+
+    // Runs validation for new cards that were created with payment intents.


Loving these comments

scottkicks and others added 13 commits March 25, 2024 12:51

Checkout screen should inherit MessageBannerViewControllerPresenting

ff23611

Add submitButtonTapped VM input

19cae31

Implement creditCardSelected delegate

239fffe

Rename PaymentSourceSelected.paymentSourceId to .savedCreditCard

495c400

Merge branch 'main' into scott/pcp/validate-checkout-confirm-payment

13baaac

Merge branch 'feat/adyer/rename-payment-source-selected-paymentsource…

66cc579

…id' into scott/pcp/validate-checkout-confirm-payment

Get stripeId and pass to view model along side the payment intent cli…

414dda9

…ent secret

Call Validate Checkout on submit

9a39f83

Add message banner delegate

9044f34

Add validateCheckoutSuccess observer

cfb2888

decode checkout id to base64

6ed64f9

Pop view controller after error banner shows

d5c8d62

show an error message if Stripe fails to retrieve the stripeCardId

8f0b39b

scottkicks self-assigned this Mar 26, 2024

scottkicks added the post-campaign-pledging label Mar 26, 2024

scottkicks added 3 commits March 25, 2024 20:57

Merge branch 'main' into scott/pcp/validate-checkout-confirm-payment

247abb1

cleanup

6618b39

Merge branch 'scott/pcp/validate-checkout-confirm-payment' of https:/…

1dc2792

…/github.com/kickstarter/ios-oss into scott/pcp/validate-checkout-confirm-payment

scottkicks commented Mar 26, 2024

View reviewed changes

Library/ViewModels/ConfirmDetailsViewModel.swift Outdated Show resolved Hide resolved

cleanup

7005ac7

scottkicks force-pushed the scott/pcp/validate-checkout-confirm-payment branch from fec43c3 to 7005ac7 Compare March 26, 2024 13:52

scottkicks requested a review from amy-at-kickstarter March 26, 2024 14:00

scottkicks marked this pull request as ready for review March 26, 2024 14:00

amy-at-kickstarter requested changes Mar 26, 2024

View reviewed changes

actually set message banner delegate...derp

b0e813f

scottkicks added 4 commits March 26, 2024 09:34

pr feedback cleanup suggestions

10e6a69

fix tests

e7c50b4

confirm details fixes

430327b

* unrelated quick fixes

Handle pre-existing payment method selected

e5d3ef8

scottkicks commented Mar 26, 2024

View reviewed changes

Library/ViewModels/PostCampaignCheckoutViewModel.swift Outdated Show resolved Hide resolved

scottkicks added 3 commits March 26, 2024 14:17

run validations for new and existing cards separately

9901a62

Merge branch 'main' into scott/pcp/validate-checkout-confirm-payment

703594e

cleanup

4d7210e

scottkicks force-pushed the scott/pcp/validate-checkout-confirm-payment branch from da13cfc to 4d7210e Compare March 26, 2024 19:30

scottkicks commented Mar 26, 2024

View reviewed changes

Kickstarter-iOS/Features/ConfirmDetails/Controllers/ConfirmDetailsViewController.swift Outdated Show resolved Hide resolved

fix tests

b5660ee

scottkicks requested a review from amy-at-kickstarter March 26, 2024 20:03

amy-at-kickstarter approved these changes Mar 26, 2024

View reviewed changes

pr feedback

af0f20a

scottkicks merged commit 140fa8f into main Mar 26, 2024
5 checks passed

scottkicks deleted the scott/pcp/validate-checkout-confirm-payment branch March 26, 2024 20:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MBL-1194] Implement Validate Checkout #1999

[MBL-1194] Implement Validate Checkout #1999

scottkicks commented Mar 26, 2024 •

edited

amy-at-kickstarter left a comment

scottkicks commented Mar 26, 2024 •

edited

scottkicks Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

amy-at-kickstarter left a comment

amy-at-kickstarter Mar 26, 2024

scottkicks Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

scottkicks Mar 26, 2024

scottkicks Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

amy-at-kickstarter Mar 26, 2024

[MBL-1194] Implement Validate Checkout #1999

[MBL-1194] Implement Validate Checkout #1999

Conversation

scottkicks commented Mar 26, 2024 • edited

📲 What

The mutation accepts the following params:

Extras

🤔 Why

✅ Acceptance criteria

⏰ TODO

amy-at-kickstarter left a comment

Choose a reason for hiding this comment

scottkicks commented Mar 26, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amy-at-kickstarter left a 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

Choose a reason for hiding this comment

scottkicks commented Mar 26, 2024 •

edited

scottkicks commented Mar 26, 2024 •

edited