Add Completed result to LinkActivityResult #9855

toluo-stripe · 2025-01-04T02:52:18Z

Summary

Current result set is [completed(paymentMethod), canceled, failed].
New result set is [completed, paymentMethodObtained(paymentMethod), canceled, failed].

This accounts for Link handling the payment confirmation. In this scenario, we don't need to send a payment method as part of the result.

Motivation

JIRA

Testing

Added tests
Modified tests
Manually verified

github-actions · 2025-01-04T02:58:14Z

Diffuse output:

OLD: identity-example-release-base.apk (signature: V1, V2)
NEW: identity-example-release-pr.apk (signature: V1, V2)

          │          compressed          │         uncompressed         
          ├───────────┬───────────┬──────┼───────────┬───────────┬──────
 APK      │ old       │ new       │ diff │ old       │ new       │ diff 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
      dex │     2 MiB │     2 MiB │  0 B │   4.1 MiB │   4.1 MiB │  0 B 
     arsc │     1 MiB │     1 MiB │  0 B │     1 MiB │     1 MiB │  0 B 
 manifest │   2.3 KiB │   2.3 KiB │  0 B │     8 KiB │     8 KiB │  0 B 
      res │ 301.8 KiB │ 301.8 KiB │  0 B │ 455.5 KiB │ 455.5 KiB │  0 B 
   native │   6.2 MiB │   6.2 MiB │  0 B │  15.8 MiB │  15.8 MiB │  0 B 
    asset │   7.2 KiB │   7.2 KiB │  0 B │   6.9 KiB │   6.9 KiB │  0 B 
    other │  90.2 KiB │  90.2 KiB │ -8 B │ 170.3 KiB │ 170.3 KiB │  0 B 
──────────┼───────────┼───────────┼──────┼───────────┼───────────┼──────
    total │   9.6 MiB │   9.6 MiB │ -8 B │  21.5 MiB │  21.5 MiB │  0 B 

 DEX     │ old   │ new   │ diff      
─────────┼───────┼───────┼───────────
   files │     1 │     1 │ 0         
 strings │ 19971 │ 19971 │ 0 (+0 -0) 
   types │  6191 │  6191 │ 0 (+0 -0) 
 classes │  4982 │  4982 │ 0 (+0 -0) 
 methods │ 29771 │ 29771 │ 0 (+0 -0) 
  fields │ 17541 │ 17541 │ 0 (+0 -0) 

 ARSC    │ old  │ new  │ diff 
─────────┼──────┼──────┼──────
 configs │  164 │  164 │  0   
 entries │ 3622 │ 3622 │  0

APK

   compressed    │   uncompressed   │                                           
──────────┬──────┼───────────┬──────┤                                           
 size     │ diff │ size      │ diff │ path                                      
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
 28.4 KiB │ -6 B │  62.9 KiB │  0 B │ ∆ META-INF/CERT.SF                        
 25.3 KiB │ -4 B │  62.8 KiB │  0 B │ ∆ META-INF/MANIFEST.MF                    
    272 B │ +2 B │     120 B │  0 B │ ∆ META-INF/version-control-info.textproto 
──────────┼──────┼───────────┼──────┼───────────────────────────────────────────
   54 KiB │ -8 B │ 125.8 KiB │  0 B │ (total)

amk-stripe · 2025-01-06T17:46:57Z

paymentsheet/src/main/java/com/stripe/android/link/analytics/DefaultLinkAnalyticsHelper.kt

+            is LinkActivityResult.Failed -> {
+                linkEventsReporter.onPopupError(linkActivityResult.error)
+            }
+            LinkActivityResult.Completed -> Unit


why does completed not also get a onPopupSuccess event?

amk-stripe · 2025-01-06T17:49:04Z

paymentsheet/src/test/java/com/stripe/android/link/analytics/DefaultLinkAnalyticsHelperTest.kt

do we need a test for when LinkActivityResult is the new Completed type?

amk-stripe · 2025-01-06T17:51:34Z

paymentsheet/src/main/java/com/stripe/android/link/LinkActivityResult.kt

@@ -12,10 +12,16 @@ import org.json.JSONObject

 internal sealed class LinkActivityResult : Parcelable {
    /**
-     * Indicates that the flow was completed successfully.
+     * Indicates that the flow was completed successfully


Suggested change

* Indicates that the flow was completed successfully

* Indicates that the flow was completed successfully.

amk-stripe · 2025-01-06T17:52:12Z

paymentsheet/src/main/java/com/stripe/android/link/LinkActivityResult.kt

+    data object Completed : LinkActivityResult()
+
+    /**
+     * Indicates that the user selected a payment method. This payment method should be used for confirmation.


Suggested change

* Indicates that the user selected a payment method. This payment method should be used for confirmation.

* Indicates that the user selected a payment method. This payment method has not yet been confirmed.

amk-stripe · 2025-01-06T18:00:41Z

paymentsheet/src/test/java/com/stripe/android/link/LinkPaymentLauncherTest.kt

Could you look into a few things to clean up these tests:

Are all of the mocks here necessary? Can we swap any for fakes?

Is it possible to share more setup between tests? It looks like the set up to many of these tests is the exact same. It'd be nice to factor that out to make it easier to update these tests in the future and to make them more readable

amk-stripe · 2025-01-06T18:02:14Z

paymentsheet/src/test/java/com/stripe/android/link/LinkPaymentLauncherTest.kt

+private open class LauncherLinkAnalyticsHelper : FakeLinkAnalyticsHelper() {
+    override fun onLinkLaunched() = Unit
+}
+
+private class TrackingLinkAnalyticsHelper : LauncherLinkAnalyticsHelper() {
+    var lastResult: LinkActivityResult? = null
+    override fun onLinkResult(linkResult: LinkActivityResult) {
+        lastResult = linkResult
+    }
+}


Wdyt of just making FakeLinkAnalyticsHelper functional so that you don't have to create these custom overrides? By "functional" I just mean take whatever implementation you needed in these tests and add that implementation to the fake directly. It should simplify this code and also make it easier to test with that fake going forward

Also to add, it would be nice if TrackingLinkAnalyticsHelper worked similar to RecordingLinkStore.test so that we can ensure that all the interactions with LauncherLinkAnalyticsHelper are consumed and we have no unexpected calls.

samer-stripe · 2025-01-06T17:35:18Z

.../main/java/com/stripe/android/paymentelement/confirmation/link/LinkConfirmationDefinition.kt

+                    intent = confirmationParameters.intent,
+                    deferredIntentConfirmationType = deferredIntentConfirmationType
+                )
+            }


Happy to see that this was easy to do with the changes we made to the ConfirmationDefinition design!

samer-stripe · 2025-01-06T18:06:43Z

paymentsheet/src/test/java/com/stripe/android/link/LinkPaymentLauncherTest.kt

+private open class LauncherLinkAnalyticsHelper : FakeLinkAnalyticsHelper() {
+    override fun onLinkLaunched() = Unit
+}
+
+private class TrackingLinkAnalyticsHelper : LauncherLinkAnalyticsHelper() {
+    var lastResult: LinkActivityResult? = null
+    override fun onLinkResult(linkResult: LinkActivityResult) {
+        lastResult = linkResult
+    }
+}


Also to add, it would be nice if TrackingLinkAnalyticsHelper worked similar to RecordingLinkStore.test so that we can ensure that all the interactions with LauncherLinkAnalyticsHelper are consumed and we have no unexpected calls.

Add Completed result to LinkActivityResult

3a02439

toluo-stripe added 3 commits January 3, 2025 23:00

Update unit tests

8f6a807

Fix lint

4228026

Update paymentsheet.api

a6ff2f9

toluo-stripe marked this pull request as ready for review January 6, 2025 17:12

toluo-stripe requested review from a team as code owners January 6, 2025 17:12

toluo-stripe requested review from samer-stripe and amk-stripe January 6, 2025 17:12

amk-stripe reviewed Jan 6, 2025

View reviewed changes

samer-stripe reviewed Jan 6, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Completed result to LinkActivityResult #9855

Add Completed result to LinkActivityResult #9855

toluo-stripe commented Jan 4, 2025 •

edited

Loading

github-actions bot commented Jan 4, 2025 •

edited

Loading

amk-stripe Jan 6, 2025

amk-stripe Jan 6, 2025

amk-stripe Jan 6, 2025

amk-stripe Jan 6, 2025

amk-stripe Jan 6, 2025

amk-stripe Jan 6, 2025

samer-stripe Jan 6, 2025

samer-stripe Jan 6, 2025

samer-stripe Jan 6, 2025

	* Indicates that the flow was completed successfully
	* Indicates that the flow was completed successfully.

	* Indicates that the user selected a payment method. This payment method should be used for confirmation.
	* Indicates that the user selected a payment method. This payment method has not yet been confirmed.

Add Completed result to LinkActivityResult #9855

Are you sure you want to change the base?

Add Completed result to LinkActivityResult #9855

Conversation

toluo-stripe commented Jan 4, 2025 • edited Loading

Summary

Motivation

Testing

github-actions bot commented Jan 4, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toluo-stripe commented Jan 4, 2025 •

edited

Loading

github-actions bot commented Jan 4, 2025 •

edited

Loading