Skip to content

[WOOMOB-302][Mobile Payments] Update Stripe's SDK to 4.3.1 #13970

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

Open
wants to merge 21 commits into
base: trunk
Choose a base branch
from

Conversation

toupper
Copy link
Contributor

@toupper toupper commented Apr 24, 2025

Closes: WOOMOB-302

Description

With this PR we update the Stripe SDK to 4.3.1, handling the breaking changes it brings. The main changes we had to adapt are:

  • Renaming: LocalMobile and COTS have been renamed to Tap to Pay. I also followed this pattern in our code to align with the new Stripe nomenclature and make it clearer.
  • TerminalException.TerminalErrorCode is moved to a standalone enum TerminalErrorCode.
  • They consolidated the reader disconnect callbacks for all reader types by removing TerminalListener::onUnexpectedReaderDisconnect. We implement onDisconnect on MobileReaderListener, TapToPayReaderListener listeners to be informed of their corresponding reader disconnects.
  • The new TapToPayConnectionConfiguration now requires a listener, so I implemented it (TapToPayReaderListenerImpl). As we didn't have any related logic before, it only logs events.
  • ReaderListener is renamed to MobileReaderListener
  • Terminal::processPayment returns now a cancelable, so we handle it.

For an exhaustive list check the migration guide and changelog

Steps to reproduce

Try both IPP and TTP flows

Important: TTP with a card can be tested only on the real device in the release builds!

Testing information

See above.

The tests that have been performed

See above.

Images/gif

N/A

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

  • The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
  • Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
  • Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

@toupper toupper marked this pull request as draft April 24, 2025 15:19
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Apr 24, 2025

Project manifest changes for WooCommerce

The following changes in the WooCommerce's merged AndroidManifest.xml file were detected (build variant: vanillaRelease):

--- ./build/reports/diff_manifest/WooCommerce/vanillaRelease/base_manifest.txt	2025-05-01 15:18:05.437596849 +0000
+++ ./build/reports/diff_manifest/WooCommerce/vanillaRelease/head_manifest.txt	2025-05-01 15:18:35.207865615 +0000
@@ -348,9 +348,14 @@
         </service>
 
         <activity
+            android:name="l7ae02aa90bfcbef3f181d5f7.l515457f9dd483c15b2b92380"
+            android:configChanges="orientation|keyboardHidden|screenSize"
+            android:process=":stripetaptopay"
+            android:theme="@style/NoDisplayCutout" />
+        <activity
             android:name="com.stripe.cots.activity.ContactlessPaymentActivity"
             android:exported="false"
-            android:process=":stripelocalmobile"
+            android:process=":stripetaptopay"
             android:theme="@style/Theme.CotsApp" >
             <intent-filter>
                 <action android:name="android.nfc.action.NDEF_DISCOVERED" />
@@ -361,18 +366,23 @@
         <activity
             android:name="com.stripe.cots.simulator.SimulatedContactlessPaymentActivity"
             android:exported="false"
+            android:process=":stripetaptopay"
             android:theme="@style/Theme.CotsApp" />
-        <activity
-            android:name="com.whitecryption.securepin.TuiActivity"
-            android:configChanges="orientation|keyboardHidden|screenSize"
-            android:process=":stripelocalmobile"
-            android:screenOrientation="portrait"
-            android:theme="@style/NoDisplayCutout" />
 
         <service
             android:name="com.stripe.cots.aidlservice.CotsService"
             android:exported="false"
-            android:process=":stripelocalmobile" />
+            android:process=":stripetaptopay" />
+        <service
+            android:name="com.stripe.cots.aidlservice.SimulatedCotsService"
+            android:exported="false"
+            android:process=":stripetaptopay" />
+
+        <provider
+            android:name="com.stripe.cots.content.CotsContentProvider"
+            android:authorities="com.woocommerce.android.stripetaptopay"
+            android:exported="false"
+            android:process=":stripetaptopay" />
 
         <activity
             android:name="zendesk.support.request.RequestActivity"
@@ -579,22 +589,6 @@
         </provider>
 
         <activity
-            android:name="com.stripe.stripeterminal.internal.common.usb.UsbEventReceiverActivity"
-            android:excludeFromRecents="true"
-            android:exported="false"
-            android:label="@string/app_name"
-            android:noHistory="true"
-            android:taskAffinity="com.stripe.stripeterminal.taskAffinityUsbEventReceiver"
-            android:theme="@style/Theme.Transparent" >
-            <intent-filter>
-                <action android:name="android.hardware.usb.action.USB_DEVICE_ATTACHED" />
-            </intent-filter>
-
-            <meta-data
-                android:name="android.hardware.usb.action.USB_DEVICE_ATTACHED"
-                android:resource="@xml/usb_device_filter" />
-        </activity>
-        <activity
             android:name="androidx.compose.ui.tooling.PreviewActivity"
             android:exported="true" />
 
@@ -714,6 +708,23 @@
             android:exported="false"
             android:initOrder="200" />
 
+        <activity
+            android:name="com.stripe.stripeterminal.internal.common.usb.UsbEventReceiverActivity"
+            android:excludeFromRecents="true"
+            android:exported="false"
+            android:label="@string/app_name"
+            android:noHistory="true"
+            android:taskAffinity="com.stripe.stripeterminal.taskAffinityUsbEventReceiver"
+            android:theme="@style/Theme.Transparent" >
+            <intent-filter>
+                <action android:name="android.hardware.usb.action.USB_DEVICE_ATTACHED" />
+            </intent-filter>
+
+            <meta-data
+                android:name="android.hardware.usb.action.USB_DEVICE_ATTACHED"
+                android:resource="@xml/usb_device_filter" />
+        </activity>
+
         <service
             android:name="androidx.room.MultiInstanceInvalidationService"
             android:directBootAware="true"
@@ -745,14 +756,6 @@
             android:authorities="com.woocommerce.android.com.squareup.picasso"
             android:exported="false" />
 
-        <service
-            android:name="com.google.android.datatransport.runtime.backends.TransportBackendDiscovery"
-            android:exported="false" >
-            <meta-data
-                android:name="backend:com.google.android.datatransport.cct.CctBackendFactory"
-                android:value="cct" />
-        </service>
-
         <receiver
             android:name="androidx.profileinstaller.ProfileInstallReceiver"
             android:directBootAware="false"
@@ -774,6 +777,13 @@
         </receiver>
 
         <service
+            android:name="com.google.android.datatransport.runtime.backends.TransportBackendDiscovery"
+            android:exported="false" >
+            <meta-data
+                android:name="backend:com.google.android.datatransport.cct.CctBackendFactory"
+                android:value="cct" />
+        </service>
+        <service
             android:name="com.google.android.datatransport.runtime.scheduling.jobscheduling.JobInfoSchedulerService"
             android:exported="false"
             android:permission="android.permission.BIND_JOB_SERVICE" >

Go to https://buildkite.com/automattic/woocommerce-android/builds/28869/canvas?sid=01968c69-3236-4e7b-bab7-f41dac40c3a5, click on the Artifacts tab and audit the files.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Apr 24, 2025

Project dependencies changes

list
+ New Dependencies
com.stripe:stripeterminal-taptopay:4.3.1

- Removed Dependencies
com.google.android.gms:play-services-safetynet:18.0.1
com.jaredrummler:android-device-names:1.1.9
com.neovisionaries:nv-i18n:1.29
com.stripe:stripeterminal-internal-models:3.7.1
com.stripe:stripeterminal-localmobile:3.7.1

! Upgraded Dependencies
androidx.annotation:annotation:1.9.1, (changed from 1.8.1)
androidx.annotation:annotation-jvm:1.9.1, (changed from 1.8.1)
com.squareup.wire:wire-moshi-adapter:4.9.11, (changed from 4.9.9)
com.squareup.wire:wire-runtime:4.9.11, (changed from 4.9.9)
com.squareup.wire:wire-runtime-jvm:4.9.11, (changed from 4.9.9)
com.stripe:stripeterminal-core:4.3.1, (changed from 3.7.1)
com.stripe:stripeterminal-external:4.3.1, (changed from 3.7.1)
com.stripe:stripeterminal-internal-common:4.3.1, (changed from 3.7.1)
io.reactivex.rxjava3:rxjava:3.1.9, (changed from 3.1.8)
org.jetbrains.kotlin:kotlin-reflect:1.9.25, (changed from 1.9.24)
org.jetbrains.kotlinx:kotlinx-datetime:0.6.1, (changed from 0.6.0)
org.jetbrains.kotlinx:kotlinx-datetime-jvm:0.6.1, (changed from 0.6.0)
org.jetbrains.kotlinx:kotlinx-serialization-bom:1.6.3, (changed from 1.5.1)
org.jetbrains.kotlinx:kotlinx-serialization-core:1.6.3, (changed from 1.5.1)
org.jetbrains.kotlinx:kotlinx-serialization-core-jvm:1.6.3, (changed from 1.5.1)
org.jetbrains.kotlinx:kotlinx-serialization-json:1.6.3, (changed from 1.5.1)
org.jetbrains.kotlinx:kotlinx-serialization-json-jvm:1.6.3, (changed from 1.5.1)

⚠️ Project dependencies tree is too large. View it in Buildkite artifacts

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Apr 24, 2025

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App Name WooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit454764a
Direct Downloadwoocommerce-wear-prototype-build-pr13970-454764a.apk

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Apr 24, 2025

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App Name WooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit454764a
Direct Downloadwoocommerce-prototype-build-pr13970-454764a.apk

@toupper toupper added this to the 22.3 milestone Apr 25, 2025
@toupper toupper marked this pull request as ready for review April 25, 2025 14:48
@toupper toupper added feature: mobile payments Related to mobile payments / card present payments / Woo Payments. type: task An internally driven task. labels Apr 25, 2025
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 45.83333% with 26 lines in your changes missing coverage. Please review.

Project coverage is 38.31%. Comparing base (508c9c6) to head (5f04514).

Files with missing lines Patch % Lines
.../internal/connection/TapToPayReaderListenerImpl.kt 0.00% 10 Missing ⚠️
...ardreader/internal/connection/ConnectionManager.kt 44.44% 4 Missing and 1 partial ⚠️
.../internal/payments/actions/ProcessPaymentAction.kt 57.14% 2 Missing and 1 partial ⚠️
...rce/android/cardreader/CardReaderManagerFactory.kt 0.00% 2 Missing ⚠️
...internal/connection/BluetoothReaderListenerImpl.kt 0.00% 2 Missing ⚠️
...id/cardreader/internal/wrappers/TerminalWrapper.kt 0.00% 2 Missing ⚠️
...manuals/CardReaderManualsSupportedReadersMapper.kt 0.00% 0 Missing and 1 partial ⚠️
...reader/internal/connection/TerminalListenerImpl.kt 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##              trunk   #13970      +/-   ##
============================================
- Coverage     38.32%   38.31%   -0.01%     
+ Complexity     9474     9473       -1     
============================================
  Files          2113     2114       +1     
  Lines        116202   116223      +21     
  Branches      14869    14871       +2     
============================================
  Hits          44529    44529              
- Misses        67613    67632      +19     
- Partials       4060     4062       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samiuelson samiuelson self-assigned this Apr 28, 2025
Copy link
Contributor

@samiuelson samiuelson left a comment

Choose a reason for hiding this comment

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

Thanks @toupper for tackling this task!

I tested the PR on physical devices, running jalapenoRelease build variant. My observations:

Tap To Pay works fine

Unexpected behavior in card reader flow in store management. In case reader is disconnected, when collecting payment with card reader, during reader connection flow the "We couldn't connect your reader" error dialog is displayed for a few seconds. If I let it wait, the reader connected successfully and payment was collected.

  • logs:
2025-04-28 14:12:44.868  4018-4953  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onConnectionStatusChange: DISCOVERING
2025-04-28 14:12:45.451  4018-4953  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onConnectionStatusChange: NOT_CONNECTED
2025-04-28 14:12:45.452  4018-4018  WooCommerce-CARD_READER com.woocommerce.android.prealpha     E  Connecting to reader failed.
2025-04-28 14:12:45.452  4018-4953  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onConnectionStatusChange: CONNECTING
2025-04-28 14:12:49.157  4018-4953  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onConnectionStatusChange: CONNECTED
2025-04-28 14:12:49.158  4018-4953  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onPaymentStatusChange: READY
2025-04-28 14:12:49.165  4018-4018  WooCommerce-CARD_READER com.woocommerce.android.prealpha     E  Connecting to reader succeeded.
2025-04-28 14:12:49.307  4018-4953  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onBatteryLevelUpdate: batteryStatus: NOMINAL, batteryLevel: 0.99, isCharging: true
2025-04-28 14:12:50.374  4018-4953  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: Creating payment intent succeeded
2025-04-28 14:12:50.389  4018-4953  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onPaymentStatusChange: WAITING_FOR_INPUT
2025-04-28 14:12:50.412  4018-5136  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onRequestReaderInput: Swipe / Insert / Tap
2025-04-28 14:12:50.793  4018-5136  WooCommerce-CARD_READER com.woocommerce.android.prealpha     D  CardReader: onRequestReaderInput: Swipe / Insert / Tap

☝️ Notice the onConnectionStatusChange: NOT_CONNECTED state is emmited for some reason. I'm not sure why 🤔

  • screen recording of the payment flow from this PR:
Screen_recording_20250428_133347.mp4

I verified that this is not happening on trunk:

Screen_recording_20250428_140640.mp4

Could you check if you experience the same on your devices, @malinajirka, @kidinov and @toupper?

@samiuelson samiuelson removed their assignment Apr 28, 2025
@malinajirka
Copy link
Contributor

I checked on my phone and I can confirm the behavior - might be a bug in the SDK, so I'd definitely start by checking their change and back logs.

@@ -104,6 +105,10 @@ internal class BluetoothReaderListenerImpl(
_displayMessagesEvents.value = BluetoothCardReaderMessages.CardReaderInputMessage(options.toString())
}

override fun onDisconnect(reason: DisconnectReason) {
logWrapper.d(LOG_TAG, "onDisconnect")
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe we shouldn't just log it. We may need to handle it properly and share it with the user

import com.woocommerce.android.cardreader.LogWrapper
import com.woocommerce.android.cardreader.internal.LOG_TAG

class TapToPayReaderListenerImpl(
Copy link
Contributor

@kidinov kidinov Apr 29, 2025

Choose a reason for hiding this comment

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

I don't think we can just log status changes. Probably we need to propagate this to the user and offer a correct explanation with further steps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kidinov Thanks for the comment! Before, we didn't even have this listener, so we didn't propagate the events to the user. As this PR issue is just to update the SDK without modifying behavior, what do you think if we leave it as it is on this PR and create an issue enhancement for propagating these events to the users and letting them to handle them?

Copy link
Contributor

Choose a reason for hiding this comment

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

@toupper 👋

For Tap to Pay readers, the TapToPayConnectionConfiguration now takes in an TapToPayReaderListener parameter, replacing ReaderReconnectionListener.

Oh, I see. We didn't have ReaderReconnectionListener either, as automatic reconnection was already added later!

As this PR issue is just to update the SDK without modifying behavior, what do you think if we leave it as it is on this PR and create an issue enhancement for propagating these events to the users and letting them to handle them?

Yep, updating to a major version often brings changes in behavior, beyond just the API modifications, indicating a need for adaptation on our side too. However, I’m uncertain in this case since it's unclear how the app will behave during the reconnection process and the duration it may require. Previously, we would display an error message when it was "not connected" and that was the end of it. Now, it will attempt to reconnect, but we will ignore the process and not present any information to the user.

It feels like if we don't want to actually handle the reconnection, we may need to disable it to keep the same behavior like now, and have a separate issue to handle this as you proposed

We recommend displaying notifications in your app to inform the users about the reader status throughout the reconnection process. To handle reader reconnection methods, the ReaderReconnectionListener has been inherited by the respective ReaderListeners. Use MobileReaderListener for mobile readers, and TapToPayReaderListener for Tap to Pay readers to handle reconnection events.

If you implemented your own reader reconnection logic and want to maintain this behavior, you can turn off auto reconnection by setting autoReconnectOnUnexpectedDisconnect to false.

@kidinov
Copy link
Contributor

kidinov commented Apr 29, 2025

@toupper Thank you for taking this on!

I can confirm what @samiuelson noticed as well.

Good news that the bugs we had with 3.10.2 with TTP on my Galaxy Ultra 24 is fixed here. Having said that we may want to setup our brand colors for TTP as it's been done here

Reading the migration guide, we may need to address, if not yet:

We added a new enum value, DISCOVERING, to ConnectionStatus to represent when reader discovery is running. Make sure your integration can handle this new state and provide relevant information to your customers.

We recommend displaying notifications in your app to inform the users about the reader status throughout the reconnection process. To handle reader reconnection methods, the ReaderReconnectionListener has been inherited by the respective ReaderListeners. Use MobileReaderListener for mobile readers, and TapToPayReaderListener for Tap to Pay readers to handle reconnection events.

For Tap to Pay on Android, Terminal::collectPaymentMethod and Terminal::collectSetupIntentPaymentMethod now time out after 60 seconds for Tap to Pay on Android transactions. A TerminalException is raised with the error code TerminalErrorCode.CARD_READ_TIMED_OUT.

For Tap to Pay on Android, when PIN collection is requested for a payment, a TerminalException is raised with the error code FEATURE_NOT_ENABLED_ON_ACCOUNT instead of DECLINED_BY_STRIPE_API with an ONLINE_OR_OFFLINE_PIN_REQUIRED ApiError.

The last one I don't remember what we do when PIN is required 🤔

@malinajirka malinajirka removed their request for review April 30, 2025 09:11
@dangermattic
Copy link
Collaborator

1 Warning
⚠️ This PR is assigned to the milestone 22.3. This milestone is due in less than 2 days.
Please make sure to get it merged by then or assign it to a milestone with a later deadline.

Generated by 🚫 Danger

@toupper
Copy link
Contributor Author

toupper commented May 1, 2025

Thanks for your test @samiuelson!

❓ Unexpected behavior in card reader flow in store management. In case reader is disconnected, when collecting payment with card reader, during reader connection flow the "We couldn't connect your reader" error dialog is displayed for a few seconds. If I let it wait, the reader connected successfully and payment was collected.

I fixed that on this commit fd7e278 As advised in their doc, we have to handle now unexpected disconnects on BluetoothReaderListenerImpl, and not in TerminalListenerImpl as I was doing before. Could you check that? With this new change, we still handle disconnects (e.g. disconnecting Bluetooth) as usual:

Screen_Recording_20250501_115307_Woo.Pre-Alpha.mp4

@toupper
Copy link
Contributor Author

toupper commented May 1, 2025

Thanks for your comment @kidinov! Let’s go one by one:

🔹 Brand colors for TTP
As I'd like to keep this PR focused on updating the SDK and maintaining existing functionality, what do you think about creating a separate issue for this? That way we can scope it properly and track it as an enhancement. I've created the issue WOOMOB-421 PR #13997

🔹 New enum value: DISCOVERING
We already handle this in TerminalListenerImpl via logging, so the app won't crash. That said, we can consider displaying it to users in a future task if we think it adds value.

🔹 Reader reconnection / notification recommendations
Same approach — we’ll stick to the current behavior in this PR. If we decide to improve the reconnection UX, we can tackle that in a future PR or issue.

🔹 Timeout after 60s on collectPaymentMethod
We handle exceptions (including timeouts) in CollectPaymentAction. For now, we’re just logging, but we could consider surfacing this to users in the future.

🔹 Change from DECLINED_BY_STRIPE_API to FEATURE_NOT_ENABLED_ON_ACCOUNT
We’re already handling this generically via logging, so the change in error code doesn’t impact us functionally.

@toupper toupper requested review from samiuelson and kidinov May 1, 2025 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature: mobile payments Related to mobile payments / card present payments / Woo Payments. type: task An internally driven task.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants