Adyen / adyen-ios

Adyen iOS Drop-in and Components
https://docs.adyen.com/checkout/ios
MIT License
150 stars 120 forks source link

[BUG] allowsSkippingPaymentList has no effect on redirect payment methods #737

Closed dragomir-ivanov-212 closed 2 years ago

dragomir-ivanov-212 commented 2 years ago

Describe the bug allowsSkippingPaymentList has no effect on redirect payment methods and the payments list is always shown

To Reproduce Initialize a DropIn component with DropInComponent.Configuration.allowsSkippingPaymentList = true and a single redirect payment method (regular, not stored). For example (but not limited to) GiroPay, iDeal, Dotpay.

Expected behaviour The payment list step should be skipped, and the respective BrowserComponent should be presented as root instead. All delegate methods should work as expected?

Screenshots ex1 ex2

Smartphone (please complete the following information):

Additional context Example payment methods used in DropInComponent.init: "PaymentMethods(paid: [], regular: [Adyen.RedirectPaymentMethod(type: \"giropay\", name: \"GiroPay\")], stored: [])"

erenbesel commented 2 years ago

Hi @dragomir-ivanov-212,

This is the expected behavior of the allowsSkippingPaymentList flag. Sorry if the description of the flag is not very clear. The purpose of if was to only skip a step for methods that require user input inside the DropIn.

This does not apply to PayPal, WeChat Pay or payment methods that require a redirect. Payment methods that directly call /payments also don't fall into this group.

However, with iDeal, skipping should work, as it does not go into redirect flow directly and opens the list of banks inside the DropIn (requires user input).

Hope this helps and let me know if you have questions!

dragomir-ivanov-212 commented 2 years ago

Thank you for the concise response! I was fairly sure this was intended, looking at the code. Still unfortunate.

I'm using AdyenActionComponent instead of DropInComponent when the payment method is a single RedirectPaymentMethod, implementing ActionComponentDelegate and PresentationDelegate alongside DropInComponentDelegate.

I had to make the abovementioned RedirectPaymentMethod public (from internal), so I could do:

switch paymentMethod {
        case is RedirectPaymentMethod:
            // allowsSkippingPaymentList does not work for Redirect methods; Use a generic AdyenActionComponent instead.
            actionComponent = AdyenActionComponent(...)
            actionComponent?.delegate = self
            actionComponent?.presentationDelegate = self
            // InstantPaymentDetails(type: paymentMethod.type)
            // PaymentComponentData(paymentMethodDetails: paymentMethodDetails, amount: nil, order: nil)
            // submit PaymentComponentData as DropInComponent.didSubmit would
            // actionComponent.handle(redirectAction)
            // present component.viewController from func present(component: PresentableComponent)
        default:
             dropInComponent = DropInComponent(...)
             dropInComponent?.delegate = self
             // present dropInComponent.viewController
}

I'm sharing this piece of (pseudo)code in the hope that someone else finds it useful in a similar situation.

I'm using cocoapods-patch for this little change, so thankfully, no forking is required. I'd appreciate it if this change could be made official so not even that is required (provided you don't have strong objections against it, of course).

descorp commented 2 years ago

Hey @dragomir-ivanov-212

There is an easy way to do it:

internal func presentPayPal() {
    guard let paymentMethod = paymentMethods?.regular.first(where: { $0.type == "paypal" }) else { return }
    let instantPaymentComponent = InstantPaymentComponent(paymentMethod: paymentMethod,
                                                          paymentData: nil,
                                                          apiContext: apiContext)
    instantPaymentComponent.delegate = self
    instantPaymentComponent.payment = payment
    currentComponent = instantPaymentComponent

    instantPaymentComponent.initiatePayment()
}

In general we are treating all payment methods that are not 'PresentableComponent' as redirect (see example).

dragomir-ivanov-212 commented 2 years ago

Thanks, @descorp, for the alternative. One of my early implementations actually involved InstantPaymentComponent. I might have overengineered it. I don't particularly like making string comparisons, especially for 3rd-party frameworks. If only there were an easy (and public) way of figuring out what component should handle what payment method.

descorp commented 2 years ago

@dragomir-ivanov-212

I would say that those names are fixed and rarely change, however you have a good point. I have created a separate ticket as a note to get back to this improvement discussion after winter holidays 😁

For now you can rely on PaymentComponentBuilder for reference PaymentMethod->Component and use ComponentManager as implementation example.