[Android] Rename CheckoutEventProcessor → CheckoutListener, drop stale callbacks#132
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
89c10c1 to
d4da055
Compare
d27977c to
7dceb15
Compare
8b0659f to
8afa420
Compare
7dceb15 to
86ecd8b
Compare
8afa420 to
14a4aac
Compare
14a4aac to
57bb1fe
Compare
86ecd8b to
9aa6c02
Compare
57bb1fe to
ffb59f3
Compare
9aa6c02 to
b4e5670
Compare
| private fun syntheticCompleteMessage(orderId: String?): String { | ||
| val orderJson = orderId?.let { ""","order":{"id":"$it"}""" } ?: "" | ||
| return """ | ||
| |{"jsonrpc":"2.0","method":"${EmbeddedCheckoutProtocol.METHOD_COMPLETE}", | ||
| |"params":{"id":"${orderId ?: ""}","currency":"","line_items":[],"links":[], | ||
| |"status":"completed","totals":[], | ||
| |"ucp":{"version":"${CheckoutProtocol.specVersion}","status":"success"}$orderJson}} | ||
| """.trimMargin().replace("\n", "") |
There was a problem hiding this comment.
Fallback completion never reaches CheckoutProtocol.complete. FallbackWebView.kt synthesizes ec.complete with checkout fields directly under params, but CheckoutProtocol.kt only decodes params.checkout.
There was a problem hiding this comment.
Good catch. Moved the construction into CheckoutProtocol.encodeRecoveryCompleteNotification(orderId:) so it nests under params.checkout. Added a round-trip test so a future hand-rolled payload can't regress this — handler now fires in the recovery confirmation path.
There was a problem hiding this comment.
Update: pulled this back out — Kieran flagged in his Swift review that we shouldn't be adding new recovery code in this PR. Synthesis call and the protocol helper are both gone now. Full recovery removal is the next PR.
|
React Native Android still references the removed API. CustomCheckoutEventProcessor.java still extends |
|
|
||
| > [!Note] | ||
| > The `DefaultCheckoutEventProcessor` provides default implementations for current and future callback functions (such as `onLinkClicked()`), which can be overridden by clients wanting to change default behavior. | ||
| > The `DefaultCheckoutListener` provides default implementations for the OS-level prompt callbacks, which can be overridden by clients wanting to change default behavior. Checkout completion and external link clicks are delivered through the Embedded Checkout Protocol client instead — register a handler on `CheckoutProtocol.complete` and implement `CheckoutCommunicationClient.openExternalUrl(...)`, then pass the client as the 4th argument of `ShopifyCheckoutKit.present(...)`. |
There was a problem hiding this comment.
The README tells consumers to implement CheckoutCommunicationClient.openExternalUrl(...) at README.md, but CheckoutCommunicationClient.kt only exposes process(message), and ec.window.open_request is handled internally at EmbeddedCheckoutProtocol.kt.
There was a problem hiding this comment.
Right, that was wrong — interface only exposes process(message). Dropped the line and clarified ec.window.open_request is kit-internal. Only CheckoutProtocol.complete registration stays on the consumer.
josemiguel-alvarez
left a comment
There was a problem hiding this comment.
There are some references to the removed code that should be updated.
| } | ||
|
|
||
| private fun getOrderIdFromQueryString(uri: Uri): String? = uri.getQueryParameter("order_id") | ||
| private fun syntheticCompleteMessage(orderId: String?): String { |
There was a problem hiding this comment.
Similar question from swift side too, I was thinking these are handled by the protocol stuff @kieran-osgood-shopify setup so hopefully dont need to be manually constructed, but let me know if complete has something unique about it
There was a problem hiding this comment.
Yeah, you're right. Pushed it down — Android has CheckoutProtocol.encodeRecoveryCompleteNotification(orderId:), Swift has the same in CheckoutProtocol+Recovery.swift. Fallback paths just call into it, no hand-rolled JSON on either side. Nothing unique about complete, the manual one was just buggy.
There was a problem hiding this comment.
Reverted — see Jose's thread above. Helper and synthesis call are both out now, full recovery removal lands next.
b4e5670 to
b578111
Compare
114f122 to
bcf828a
Compare
| */ | ||
| public abstract class DefaultCheckoutEventProcessor : CheckoutEventProcessor { | ||
| public abstract class DefaultCheckoutListener @JvmOverloads constructor( | ||
| @Suppress("UnusedPrivateProperty") private val context: Context, |
There was a problem hiding this comment.
maybe I'm mixing things up, but I think we said we could just remove these params rather than suppress?
There was a problem hiding this comment.
Yep, dropped both. Context and LogWrapper are gone from the constructor — subclasses pass through if they need them. apiDump regenerated to match.
| log.d(LOG_TAG, "Finished page has confirmationUrl. Emitting minimal checkout completed event.") | ||
| getEventProcessor().onCheckoutViewComplete( | ||
| emptyCompletedEvent(id = getOrderIdFromQueryString(uri)) | ||
| log.d(LOG_TAG, "Confirmation page reached in fallback; synthesizing ec.complete with partial data.") |
There was a problem hiding this comment.
Are we supporting recovery in V1?
There was a problem hiding this comment.
Good question — Kieran flagged the same thing on the Swift PR. We're not shipping recovery in v1. For this PR I pulled the new synthesis additions back out so we're not growing the surface we're about to delete. Full recovery removal lands as the next PR on the stack.
b93b925 to
ba15a01
Compare
b578111 to
dfa64f0
Compare
|
Tracked for the RN follow-up PR — needs the rename plus the zero-arg |
ba15a01 to
3d6f9c6
Compare
dfa64f0 to
4037fe6
Compare
| public final fun onGeolocationPermissionsShowPrompt (Lkotlin/jvm/functions/Function2;)V | ||
| public final fun onPermissionRequest (Lkotlin/jvm/functions/Function1;)V | ||
| public final fun onShowFileChooser (Lkotlin/jvm/functions/Function3;)V | ||
| ======= | ||
| public abstract interface class com/shopify/checkoutkit/CheckoutListener { | ||
| public abstract fun onCheckoutCanceled ()V | ||
| public abstract fun onCheckoutFailed (Lcom/shopify/checkoutkit/CheckoutException;)V | ||
| public abstract fun onGeolocationPermissionsHidePrompt ()V | ||
| public abstract fun onGeolocationPermissionsShowPrompt (Ljava/lang/String;Landroid/webkit/GeolocationPermissions$Callback;)V | ||
| public abstract fun onPermissionRequest (Landroid/webkit/PermissionRequest;)V | ||
| public abstract fun onShowFileChooser (Landroid/webkit/WebView;Landroid/webkit/ValueCallback;Landroid/webkit/WebChromeClient$FileChooserParams;)Z | ||
| >>>>>>> ba15a01 ([Android] Rename CheckoutEventProcessor → CheckoutListener, drop stale callbacks) |
There was a problem hiding this comment.
[Re: lines +749 to +766]
There are still conflict markers here.
See this comment inline on Graphite.
There was a problem hiding this comment.
yes, pushed too soon, fixing them
4037fe6 to
623b169
Compare
3d6f9c6 to
035d5a7
Compare
josemiguel-alvarez
left a comment
There was a problem hiding this comment.
React Native Android still references the removed Android API. CustomCheckoutEventProcessor.java still extends DefaultCheckoutEventProcessor, and it still overrides onCheckoutCompleted.
josemiguel-alvarez
left a comment
There was a problem hiding this comment.
Just saw that there's a follow-up for the React Native stale code.
There are some conflicts markers that should be removed. Everything else looks good to me!
035d5a7 to
1951caf
Compare
1951caf to
c24c019
Compare
623b169 to
70531ba
Compare
) ### What changes are you making? react native android CI was failing after #132 merged because it didn't run CI for react-native This PR renames the android portion to match the new naming of the event processor <img width="847" height="556" alt="image" src="https://github.com/user-attachments/assets/b3366989-2b9d-4bf6-a67d-496fea4561a0" /> ### How to test <!-- Please outline the steps to test your changes --> --- ### Before you merge > [!IMPORTANT] > > - [ ] I've added tests to support my implementation > - [ ] I have read and agree with the [Contribution Guidelines](./CONTRIBUTING.md) > - [ ] I have read and agree with the [Code of Conduct](./CODE_OF_CONDUCT.md) > - [ ] I've updated the relevant platform README (`platforms/swift/README.md` and/or `platforms/android/README.md`) --- <details> <summary>Releasing a new Swift version?</summary> - [ ] I have bumped the version in `ShopifyCheckoutKit.podspec` - [ ] I have bumped the version in `platforms/swift/Sources/ShopifyCheckoutKit/ShopifyCheckoutKit.swift` - [ ] I have updated `platforms/swift/CHANGELOG.md` - [ ] I have updated the SwiftPM/CocoaPods version snippets in `platforms/swift/README.md` (major version only) </details> <details> <summary>Releasing a new Android version?</summary> - [ ] I have bumped the `versionName` in `platforms/android/lib/build.gradle` - [ ] I have updated `platforms/android/CHANGELOG.md` - [ ] I have updated the Gradle/Maven version snippets in `platforms/android/README.md` </details> > [!TIP] > See the [Contributing documentation](./CONTRIBUTING.md) for the full release process per platform.

What changes are you making?
Renames
CheckoutEventProcessor→CheckoutListeneron Android (withDefaultCheckoutEventProcessor→DefaultCheckoutListener) and drops two methods now owned by the protocol layer:onCheckoutCompleted→ useCheckoutProtocol.completeonCheckoutLinkClicked→ useCheckoutCommunicationClient.openExternalUrl(...)Removes the legacy code paths that fed those callbacks: the
COMPLETEDcase inCheckoutBridgeand the legacy intent-launching defaults onDefaultCheckoutEventProcessor.CheckoutWebView'sshouldOverrideUrlLoadingoverride stays — it's still the only path that catchesmailto:/tel:clicks, which checkout-web deliberately doesn't route throughwindow.open(seecheckoutProtocolLinkInterceptor.ts:18).One behavior tweak to keep parity with the previous Android implementation: the
ec.completehandler now invalidates the preload cache (was done by the oldonCheckoutViewCompletepath).Internal field/class names follow the rename for consistency (
CheckoutWebViewEventProcessor→CheckoutWebViewListener,eventProcessor→listenerthroughout). README andlib/api/lib.apiare updated.Note: The React Native Android wrapper extends
DefaultCheckoutEventProcessorand overridesonCheckoutCompleted— it will need a follow-up PR to rename the extends clause and route completion throughCheckoutProtocol.complete.How to test
./gradlew :lib:test :lib:apiCheck :lib:detekt :lib:lintReleaseCheckoutProtocol.completehandler inCartViewModelfires (clears cart and pops back to the product screen)Before you merge
Important