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

State on Card component not applied when apps comes back to foreground #1400

Open
ronaldvanduren opened this issue Nov 24, 2023 · 10 comments
Open
Labels
Confirmed bug Indicates that issue has been confirmed to be a bug by an Adyen developer

Comments

@ronaldvanduren
Copy link

ronaldvanduren commented Nov 24, 2023

Describe the bug
In our company we just started to integrate the Adyen components. With the Card component I notice that the state is not applied whenever the compose screen recomposes. This happens when the user fills in the credit card details, a payment request is made, the user is redirected to a webview for the 3ds challenge, the payment is cancelled and the deeplink returns the user to our app. At that point the fields of the card component are empty again, however, the state that is returned from the card component still have the old state. We use version 4.13.3, I tried 5.0.1 as well but it has the same issue.

To Reproduce
Steps to reproduce the behavior:

  1. Make a payment request with test card data for a 3DS challenge.
  2. Handle the RedirectAction so that the 3DS webview opens.
  3. Cancel the payment.
  4. See that the card component is empty, however, listening to the state returns a CardComponentState with the state before we left he app.

Expected behavior
When the user returns to the app after the 3DS challenge and a recomposition happens (due to other changes in the UI), I expect that the CardView's old state is applied or that the state is invalided, so that cardComponent.observe(lifecycleOwner) returns an invalidated state.

Screenshots
incorrect-behaviour

Smartphone (please complete the following information):

  • Device: Pixel 6
  • OS: Android 14

Additional context
We use Compose to build our UI.

The Composable used:

@Composable
internal fun CreditCardContent(
    cardComponent: CardComponent,
    onCreditCardInputChanged: (CardComponentState) -> Unit = { }
) {
    Column(
        Modifier.height(PAYMENT_CONTENT_HEIGHT.dp)
    ) {
        
        val lifecycleOwner = LocalLifecycleOwner.current

        DisposableEffect(cardComponent) {

            cardComponent.observe(lifecycleOwner) { paymentComponentState ->
                onCreditCardInputChanged.invoke(paymentComponentState)
            }

            onDispose {
                cardComponent.removeObservers(lifecycleOwner)
            }
        }

        AndroidView(factory = { context ->
            CardView(context).apply {
                attach(cardComponent, lifecycleOwner)
            }
        }, modifier = Modifier.fillMaxWidth()
        )
    }
@jreij jreij added the Bug report Indicates that issue has been marked as a possible bug label Nov 29, 2023
@araratthehero
Copy link
Contributor

araratthehero commented Dec 12, 2023

Hi @ronaldvanduren, thank you for reporting this bug to us.

We just released 5.1.0, which includes some changes on how we draw views, which also fixes this issue. By design, V4 is not compatible with Compose.

Additionally when using a v5.x.x version, you can use the card component to handle redirect and 3DS2 actions (cardComponent.handleAction(...)), instead of doing them manually.

Based on the flow you use, you can find the card component integration example for advanced flow and the sessions flow.

Could you please try the new 5.1.0 version and let us know if you still experience issues?

@ronaldvanduren
Copy link
Author

ronaldvanduren commented Dec 13, 2023

Hi, I checked today the latest version and the problem still persists. I use the CardComponent.PROVIDER.get without specifying a key so I get the same one back on recomposition. This works, because when onStateChanged is called with the ComponentCallback I provide, I see that the previous state is returned. But the values of this state are not applied to the input fields. We rely on this state to enable the payment button, but it's a bit strange now that it's enabled but the fields are empty. I provided a GIF with the flow to make it a bit clearer (you can see that the previous state is partially applied as the VISA card is still detected). If you need more info, let me know 😄.

wrong-state

@araratthehero
Copy link
Contributor

Hi @ronaldvanduren, thanks for trying with the latest version of our SDK.

We verified again by using the information you provided, but unfortunately we are not able to reproduce the issue.

To assist you further, we would need some additional information.
Could you please send us the following:

  • Extensive logs (You can enable extensive logs by calling com.adyen.checkout.core.AdyenLogger.setLogLevel(Log.VERBOSE) in your application class and filter your logs by CO.)
  • More information on how you handle navigation in your app.
  • More information on how you handle redirect (going back from the action screen). Also what happens to your Activity when redirecting back.
  • More information on how you set up compose and handle recomposition.
  • More code samples or a test project, which we could use to reproduce the issue.
  • Any other information you think might be relevant to the ones I asked above.

@ronaldvanduren
Copy link
Author

Hi @araratthehero, sorry for the late reply, we had a Christmas recess.

I will work on this again and come back to you with the information you ask for. In the meantime, would it possible to add a feature to invalidate the component state?

@araratthehero
Copy link
Contributor

Hi @ronaldvanduren, thank you for keeping us updated.

We are currently working on improving components, including handling of component states. We appreciate your feedback and will certainly take it into consideration.

It would still be helpful if you could share information that would help us understand the issue better, and if possible, find a solution for you.

@ronaldvanduren
Copy link
Author

Hi @araratthehero, great to see a team that is dedicated to improve the SDK 😄, I have seen the opposite too many times unfortunately.

I found the issue, it's related to handeling a deeplink with the NavController combined with Compose. Whenever a deeplink is received by the app, and the app isn't closed, it's handled by onNewIntent. Here we called the NavController to handle the intent.

    override fun onNewIntent(intent: Intent) {
        super.onNewIntent(intent)
        if (isAdyenRedirectUri(intent.data ?: Uri.EMPTY)) {
                      navController.handleDeepLink(intent)
       }
}

This would trigger our Composable to be called because it's registered in our NavController like this:

composable(
"/our-screen-with-adyen-component/",

deepLinks = listOf(navDeepLink {
    uriPattern =
        "our-scheme://our-host?some-params"
})

This triggers a full screen recomposition. Normally this shouldn't be an issue, the ViewModel holds the state and this state is applied again. However, the internal state of the AdyenComponent is not correctly applied in this scenario. I checked the source code of the Adyen Card Component and the input fields are only being read from but never set. I am not sure if this is the intented behaviour, but to work together with the current behaviour of the AdyenComponent I made a small change in our handeling of the deeplink.

In onNewIntent I check if the current destination of our screen that should handle the deeplink is the correct screen. If this is the case, a separate component (lets call it FooComponent) will handle the intent and not the NavController. If this is not the case, the NavController will handle this. FooComponent is injected both in the Activity (so it can be used in onNewIntent) and the ViewModel of the Composable that should handle the deeplink and the ViewModel will listen to changes from FooComponent.

In code it will look something like this

    //In the Activity
    override fun onNewIntent(intent: Intent) {
        super.onNewIntent(intent)
        if (isAdyenRedirectUri(intent.data ?: Uri.EMPTY)) {
            if(navController.currentDestination?.route?.contains("our-screen-with-adyen-component") == true) {
                fooComponent.handleDeeplink(intent)
            } else {
                navController.handleDeepLink(intent)
            }
        }
    }


  //In the ViewModel
   init {
        launch {
            fooComponent.deeplinkData().collect { deeplinkData ->
                if (deeplinkData.orderId == orderId) {
                    onAdyenDataReceived(AdyenDeeplinkData(deeplinkData.payinId, deeplinkData.redirectResult))
                }
            }
        }
    }

So whenever the screen is already opened, we update the view simply by listening to FooComponent. If the screen wasn't opened (for example, the user closed the app in the background), we let the NavController handle it because the full screen recomposition should happen anyway.

We are still in the process of testing edge cases with this approach, but so far it looks solid.

@araratthehero
Copy link
Contributor

Hi @ronaldvanduren, thank you for describing your approach so thoroughly.

Our component acts as a ViewModel and depending on which lifecycle it is tied to, it can keep or not keep its state.

I am curious to know where you create the CardComponent. Could you share a code sample with me?

If you create the CardComponent in your activity, then the state will not be cleared until activity gets destroyed. But if you create the CardComponent in a composable, then every time you let NavController handle the deeplink, it should clear the view state and recreate the component.
We have extension functions to do that. You can find the module here and an implementation example here (under Create the Component and attach it to a view).

But if you want to keep the state of the view, then indeed your approach is the way to do it, by making sure to not recompose the screen, when handling the deeplink. And that is what we are working on to fix.

@ronaldvanduren
Copy link
Author

Hi @araratthehero, I have a mixed approach at the moment of what you describe. At the moment the CardComponent is created inside the composable but it's referencing the ComponentActivity. This is done by using a custom CompositionLocalProvider. In code it looks like this (note that it's a WIP):

//Compose utils
val LocalComponentActivity = compositionLocalOf<ComponentActivity> {
    error("ComponentActivity scope not found")
}

//Activity
    override fun onCreate(savedInstanceState: Bundle?) {
  ......
     SomeTheme {
                    CompositionLocalProvider(
                        LocalKoinScopeProvider provides KoinScopeProvider(scope),
                        LocalComponentActivity provides this as ComponentActivity
                    ) {
                        Surface {
                            SomeNavGraph(this, someArgs, navController = navController)
                        }
                    }
                }
  ......
}

//The composable with the Adyen component
@Composable
internal fun CreditCardContent(
    creditCardConfiguration: CreditCardConfiguration,
    onCreditCardInputChanged: (CardComponentState) -> Unit = {}
) {
    Column(
        Modifier.height(PAYMENT_CONTENT_HEIGHT.dp)
    ) {

        val cardConfiguration = remember {
            CardConfiguration.Builder(
                creditCardConfiguration.locale,
                Environment.TEST,
                BuildConfig.ADYEN_SDK_KEY
            ).setHolderNameRequired(true)
                .setShowStorePaymentField(false)
                .setSubmitButtonVisible(false)
                .build()
        }

        val cardComponent = CardComponent.PROVIDER.get(
            LocalComponentActivity.current,
            paymentMethod = creditCardConfiguration.creditCardPaymentMethod,
            configuration = cardConfiguration,
            callback = object : ComponentCallback<CardComponentState> {
                override fun onAdditionalDetails(actionComponentData: ActionComponentData) {
                    TODO("Not yet implemented")
                }

                override fun onError(componentError: ComponentError) {
                    TODO("Not yet implemented")
                }

                override fun onSubmit(state: CardComponentState) {
                    TODO("Not yet implemented")
                }

                override fun onStateChanged(state: CardComponentState) {
                    onCreditCardInputChanged(state)
                }
            }
        )

        Timber.d("cardComponent: $cardComponent")

        AdyenComponent(
            component = cardComponent,
            modifier = Modifier.fillMaxWidth()
        )
    }
}

After a recomposition, the same instance of the CardComponent is returned. Even when using the NavController to handle the deeplink.

@araratthehero
Copy link
Contributor

Hi @ronaldvanduren, that is true. If you create CardComponent by passing an activity as a lifecycle owner (the way you did above), the component will be tied to the lifecycle of the activity, thus the state will only be cleared when activity gets destroyed.

But if you use a different get() function for CardComponent.PROVIDER where you do not pass an activity as the lifecycle owner, we will tie the component to the LocalLifecycleOwner.current, which in your case should be a NavBackStackEntry whose lifecycle should be valid for the lifetime of this destination on the back stack, e.g.

val cardComponent = CardComponent.PROVIDER.get(
       checkoutSession = checkoutSession,
       paymentMethod = paymentMethod,
       configuration = configuration,
       componentCallback = callback,
       // This key is required to ensure a new Component gets created for each different screen or payment session.
       // Generate a new value for this key every time you need to reset the Component.
       key = "YOUR_UNIQUE_KEY_FOR_THIS_COMPONENT",
   )

I got this code from here. You can make use of those functions by implementing the following module:

implementation "com.adyen.checkout:components-compose:YOUR_VERSION

In this case there should be no need to use CompositionLocalProvider or a reference to the ComponentActivity.

Could you try this approach and let me know if it worked for you?

@ronaldvanduren
Copy link
Author

Hi @araratthehero, I tested your suggested approach and that works as well, great! 🙌

@jreij jreij added Confirmed bug Indicates that issue has been confirmed to be a bug by an Adyen developer and removed Bug report Indicates that issue has been marked as a possible bug labels Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Confirmed bug Indicates that issue has been confirmed to be a bug by an Adyen developer
Projects
None yet
Development

No branches or pull requests

3 participants