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.

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

[MBL-1194] Implement Validate Checkout #1999

Merged
merged 27 commits into from
Mar 26, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
ff23611
Checkout screen should inherit MessageBannerViewControllerPresenting
scottkicks Mar 25, 2024
19cae31
Add submitButtonTapped VM input
scottkicks Mar 25, 2024
239fffe
Implement creditCardSelected delegate
scottkicks Mar 25, 2024
495c400
Rename PaymentSourceSelected.paymentSourceId to .savedCreditCard
amy-at-kickstarter Mar 25, 2024
13baaac
Merge branch 'main' into scott/pcp/validate-checkout-confirm-payment
scottkicks Mar 25, 2024
66cc579
Merge branch 'feat/adyer/rename-payment-source-selected-paymentsource…
scottkicks Mar 25, 2024
414dda9
Get stripeId and pass to view model along side the payment intent cli…
scottkicks Mar 25, 2024
9a39f83
Call Validate Checkout on submit
scottkicks Mar 25, 2024
9044f34
Add message banner delegate
scottkicks Mar 25, 2024
cfb2888
Add validateCheckoutSuccess observer
scottkicks Mar 25, 2024
6ed64f9
decode checkout id to base64
scottkicks Mar 26, 2024
d5c8d62
Pop view controller after error banner shows
scottkicks Mar 26, 2024
8f0b39b
show an error message if Stripe fails to retrieve the stripeCardId
scottkicks Mar 26, 2024
247abb1
Merge branch 'main' into scott/pcp/validate-checkout-confirm-payment
scottkicks Mar 26, 2024
6618b39
cleanup
scottkicks Mar 26, 2024
1dc2792
Merge branch 'scott/pcp/validate-checkout-confirm-payment' of https:/…
scottkicks Mar 26, 2024
7005ac7
cleanup
scottkicks Mar 26, 2024
b0e813f
actually set message banner delegate...derp
scottkicks Mar 26, 2024
10e6a69
pr feedback cleanup suggestions
scottkicks Mar 26, 2024
e7c50b4
fix tests
scottkicks Mar 26, 2024
430327b
confirm details fixes
scottkicks Mar 26, 2024
e5d3ef8
Handle pre-existing payment method selected
scottkicks Mar 26, 2024
9901a62
run validations for new and existing cards separately
scottkicks Mar 26, 2024
703594e
Merge branch 'main' into scott/pcp/validate-checkout-confirm-payment
scottkicks Mar 26, 2024
4d7210e
cleanup
scottkicks Mar 26, 2024
b5660ee
fix tests
scottkicks Mar 26, 2024
af0f20a
pr feedback
scottkicks Mar 26, 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
scottkicks marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
Expand Up @@ -135,11 +135,8 @@ final class ConfirmDetailsViewController: UIViewController, MessageBannerViewCon
// MARK: - Configuration

private func configureChildViewControllers() {
_ = (self.rootScrollView, self.view)
|> ksr_addSubviewToParent()

_ = (self.continueCTAView, self.view)
|> ksr_addSubviewToParent()
self.view.addSubview(self.rootScrollView)
self.view.addSubview(self.continueCTAView)

_ = (self.rootStackView, self.rootScrollView)
|> ksr_addSubviewToParent()
Expand All @@ -152,27 +149,22 @@ final class ConfirmDetailsViewController: UIViewController, MessageBannerViewCon
self.pledgeRewardsSummaryViewController
]

let arrangedSubviews = [
self.rootInsetStackView
]
self.rootStackView.addArrangedSubview(self.rootInsetStackView)

let arrangedInsetSubviews = [
[self.titleLabel],
self.inputsSectionViews,
self.pledgeAmountSummarySectionViews,
[self.pledgeRewardsSummaryViewController.view]
self.pledgeAmountSummarySectionViews
]
.flatMap { $0 }
.compact()

arrangedSubviews.forEach { view in
self.rootStackView.addArrangedSubview(view)
}

arrangedInsetSubviews.forEach { view in
self.rootInsetStackView.addArrangedSubview(view)
}

self.rootStackView.addArrangedSubview(self.pledgeRewardsSummaryViewController.view)

childViewControllers.forEach { viewController in
self.addChild(viewController)
viewController.didMove(toParent: self)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,17 @@ private enum PostCampaignCheckoutLayout {
}
}

final class PostCampaignCheckoutViewController: UIViewController {
final class PostCampaignCheckoutViewController: UIViewController, MessageBannerViewControllerPresenting {
// MARK: - Properties

internal var messageBannerViewController: MessageBannerViewController?

private lazy var titleLabel = UILabel(frame: .zero)

private lazy var paymentMethodsViewController = {
PledgePaymentMethodsViewController.instantiate()
// TODO: Add self as delegate and add support for delegate methods.
|> \.messageDisplayingDelegate .~ self
|> \.delegate .~ self
}()

private lazy var pledgeCTAContainerView: PledgeViewCTAContainerView = {
Expand Down Expand Up @@ -72,6 +75,9 @@ final class PostCampaignCheckoutViewController: UIViewController {

self.title = Strings.Back_this_project()

self.messageBannerViewController = self.configureMessageBannerViewController(on: self)
self.messageBannerViewController?.delegate = self

self.configureChildViewControllers()
self.setupConstraints()

Expand Down Expand Up @@ -199,6 +205,28 @@ final class PostCampaignCheckoutViewController: UIViewController {
self.presentHelpWebViewController(with: helpType, presentationStyle: .formSheet)
}

self.viewModel.outputs.validateCheckoutSuccess
.observeForControllerAction()
.observeValues { [weak self] _ in
guard let self else { return }

// TODO: Confirm paymentIntent using Stripe.confirmPayment()
}

self.viewModel.outputs.validateCheckoutExistingCardSuccess
.observeForControllerAction()
.observeValues { [weak self] _ in
guard let self else { return }

// TODO: Confirm paymentIntent using Stripe.confirmPayment()
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

}

self.viewModel.outputs.showErrorBannerWithMessage
.observeForControllerAction()
.observeValues { [weak self] errorMessage in
self?.messageBannerViewController?.showBanner(with: .error, message: errorMessage)
}

self.sessionStartedObserver = NotificationCenter.default
.addObserver(forName: .ksr_sessionStarted, object: nil, queue: nil) { [weak self] _ in
self?.viewModel.inputs.userSessionStarted()
Expand Down Expand Up @@ -253,11 +281,70 @@ extension PostCampaignCheckoutViewController: PledgeViewCTAContainerViewDelegate

func submitButtonTapped() {
self.paymentMethodsViewController.cancelModalPresentation(true)
// TODO: Respond to button tap
self.viewModel.inputs.submitButtonTapped()
}

func termsOfUseTapped(with helpType: HelpType) {
self.paymentMethodsViewController.cancelModalPresentation(true)
self.viewModel.inputs.termsOfUseTapped(with: helpType)
}
}

// MARK: - PledgePaymentMethodsViewControllerDelegate

extension PostCampaignCheckoutViewController: PledgePaymentMethodsViewControllerDelegate {
func pledgePaymentMethodsViewController(
_: PledgePaymentMethodsViewController,
didSelectCreditCard paymentSource: PaymentSourceSelected
) {
switch paymentSource {
case let .paymentIntentClientSecret(clientSecret):
return STPAPIClient.shared.retrievePaymentIntent(withClientSecret: clientSecret) { intent, _ in
scottkicks marked this conversation as resolved.
Show resolved Hide resolved
guard let intent = intent else {
self.messageBannerViewController?
.showBanner(with: .error, message: Strings.Something_went_wrong_please_try_again())
return
}

let paymentMethodId = intent.paymentMethodId!
Copy link
Contributor

Choose a reason for hiding this comment

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

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


self.viewModel.inputs
.creditCardSelected(
source: paymentSource,
paymentMethodId: paymentMethodId,
isNewPaymentMethod: true
)
}
case let .savedCreditCard(savedCardId):
self.viewModel.inputs
.creditCardSelected(source: paymentSource, paymentMethodId: savedCardId, isNewPaymentMethod: false)
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

default:
break
scottkicks marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

// MARK: - PledgeViewControllerMessageDisplaying

extension PostCampaignCheckoutViewController: PledgeViewControllerMessageDisplaying {
func pledgeViewController(_: UIViewController, didErrorWith message: String) {
self.messageBannerViewController?.showBanner(with: .error, message: message)
}

func pledgeViewController(_: UIViewController, didSucceedWith message: String) {
self.messageBannerViewController?.showBanner(with: .success, message: message)
}
}

// MARK: - MessageBannerViewControllerDelegate

extension PostCampaignCheckoutViewController: MessageBannerViewControllerDelegate {
func messageBannerViewDidHide(type: MessageBannerType) {
switch type {
case .error:
self.navigationController?.popViewController(animated: true)
default:
break
}
}
}
1 change: 1 addition & 0 deletions KsApi/models/graphql/UserCreditCards.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ public struct UserCreditCards: Decodable {
public var id: String
public var lastFour: String
public var type: CreditCardType?
public var stripeCardId: String?
Copy link
Contributor

Choose a reason for hiding this comment

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

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


public var formattedExpirationDate: String {
return String(self.expirationDate.dropLast(3))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ extension UserCreditCards {
expirationDate: node.expirationDate,
id: node.id,
lastFour: node.lastFour,
type: CreditCardType(rawValue: node.type.rawValue)
type: CreditCardType(rawValue: node.type.rawValue),
stripeCardId: node.stripeCardId
)
}

Expand Down
15 changes: 12 additions & 3 deletions Library/ViewModels/ConfirmDetailsViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor Author

Choose a reason for hiding this comment

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

unrelated fix for showing the summary view on details

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

}

let allRewardsTotal = Signal.combineLatest(
Expand Down Expand Up @@ -353,7 +353,16 @@ public class ConfirmDetailsViewModel: ConfirmDetailsViewModelType, ConfirmDetail
}

let checkoutValues = createCheckoutEvents.values()
.map { $0.checkout.id }
.map { values in
var checkoutId: String?

if let decoded = decodeBase64(values.checkout.id), let range = decoded.range(of: "Checkout-") {
let id = decoded[range.upperBound...]
checkoutId = String(id)
}

return checkoutId
}
scottkicks marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yea i left an unfortunate comment earlier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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


self.createCheckoutSuccess = checkoutValues.withLatestFrom(
Signal.combineLatest(
Expand Down Expand Up @@ -385,7 +394,7 @@ public class ConfirmDetailsViewModel: ConfirmDetailsViewModelType, ConfirmDetail
shipping: shipping,
refTag: initialData.refTag,
context: initialData.context,
checkoutId: checkoutValue
checkoutId: checkoutValue ?? ""
)
}

Expand Down
4 changes: 2 additions & 2 deletions Library/ViewModels/ConfirmDetailsViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -810,7 +810,7 @@ final class ConfirmDetailsViewModelTests: TestCase {
}

func testContinueButton_CallsCreateBackingMutation_Success() {
let expectedId = "id"
let expectedId = "Checkout-12345"
let createCheckout = CreateCheckoutEnvelope.Checkout(id: expectedId, paymentUrl: "paymentUrl")

let mockService = MockService(
Expand Down Expand Up @@ -875,7 +875,7 @@ final class ConfirmDetailsViewModelTests: TestCase {
shipping: expectedShipping,
refTag: nil,
context: .pledge,
checkoutId: expectedId
checkoutId: "12345"
)
self.createCheckoutSuccess.assertValue(expectedValue)
}
Expand Down