Reference id mixup fix (#147)

* Correct distinction between reference_id and account_reference_id * Spotless apply * Make function changes be backwards compatible * Update CHANGELOG * Fix UTs
cashapp · Jul 19, 2024 · 67845e0 · 67845e0
1 parent f08c551
commit 67845e0
Show file tree

Hide file tree

Showing 11 changed files with 84 additions and 40 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,4 +1,7 @@
-# 2.5.0 - UPCOMING
+# 2.5.0
+
+ - Fix correct usage of `account_reference_id` in `OnFileAction`
+ - Add `reference_id` as a parameter of creating a customer request. This is to bring the Android SDK to parity with iOS and Web.
 
 # 2.4.0
 

diff --git a/core/src/main/java/app/cash/paykit/core/CashAppPay.kt b/core/src/main/java/app/cash/paykit/core/CashAppPay.kt
@@ -45,9 +45,10 @@ interface CashAppPay {
    * @param paymentAction A wrapper class that contains all of the necessary ingredients for building a customer requests.
    *                      Look at [PayKitPaymentAction] for more details.
    * @param redirectUri The URI for Cash App to redirect back to your app. If you do not set this, back navigation from CashApp might not work as intended.
+   * @param referenceId Optional identifier generated by you for this request, typically used to associate the resource with a record in an external system.
    */
   @WorkerThread
-  fun createCustomerRequest(paymentAction: CashAppPayPaymentAction, redirectUri: String?)
+  fun createCustomerRequest(paymentAction: CashAppPayPaymentAction, redirectUri: String?, referenceId: String? = null)
 
   /**
    * Create customer request given list of [CashAppPayPaymentAction].
@@ -57,9 +58,11 @@ interface CashAppPay {
    * @param paymentActions A wrapper class that contains all of the necessary ingredients for building one or more customer requests.
    *                      Look at [PayKitPaymentAction] for more details.
    * @param redirectUri The URI for Cash App to redirect back to your app. If you do not set this, back navigation from CashApp might not work as intended.
+   * @param referenceId Optional identifier generated by you for this request, typically used to associate the resource with a record in an external system.
+   *
    */
   @WorkerThread
-  fun createCustomerRequest(paymentActions: List<CashAppPayPaymentAction>, redirectUri: String?)
+  fun createCustomerRequest(paymentActions: List<CashAppPayPaymentAction>, redirectUri: String?, referenceId: String? = null)
 
   /**
    * Update an existing customer request given its [requestId] and the updated definitions contained within [CashAppPayPaymentAction].
@@ -69,11 +72,13 @@ interface CashAppPay {
    * @param requestId ID of the request we intent do update.
    * @param paymentAction A wrapper class that contains all of the necessary ingredients for updating a customer request for a given [requestId].
    *                      Look at [CashAppPayPaymentAction] for more details.
+   * @param referenceId Optional identifier generated by you for this request, typically used to associate the resource with a record in an external system.
    */
   @WorkerThread
   fun updateCustomerRequest(
     requestId: String,
     paymentAction: CashAppPayPaymentAction,
+    referenceId: String? = null,
   )
 
   /**
@@ -84,11 +89,13 @@ interface CashAppPay {
    * @param requestId ID of the request we intent do update.
    * @param paymentActions A wrapper class that contains all of the necessary ingredients for updating one more more customer requests that share the same [requestId].
    *                      Look at [CashAppPayPaymentAction] for more details.
+   * @param referenceId Optional identifier generated by you for this request, typically used to associate the resource with a record in an external system.
    */
   @WorkerThread
   fun updateCustomerRequest(
     requestId: String,
     paymentActions: List<CashAppPayPaymentAction>,
+    referenceId: String? = null,
   )
 
   /**

diff --git a/core/src/main/java/app/cash/paykit/core/NetworkManager.kt b/core/src/main/java/app/cash/paykit/core/NetworkManager.kt
@@ -28,12 +28,14 @@ internal interface NetworkManager {
     clientId: String,
     paymentActions: List<CashAppPayPaymentAction>,
     redirectUri: String?,
+    referenceId: String?,
   ): NetworkResult<CustomerTopLevelResponse>
 
   @Throws(IOException::class)
   fun updateCustomerRequest(
     clientId: String,
     requestId: String,
+    referenceId: String?,
     paymentActions: List<CashAppPayPaymentAction>,
   ): NetworkResult<CustomerTopLevelResponse>
 

diff --git a/core/src/main/java/app/cash/paykit/core/analytics/PayKitAnalyticsEventDispatcherImpl.kt b/core/src/main/java/app/cash/paykit/core/analytics/PayKitAnalyticsEventDispatcherImpl.kt
@@ -47,7 +47,6 @@ import app.cash.paykit.core.models.request.CustomerRequestDataFactory.CHANNEL_IN
 import app.cash.paykit.core.models.response.CustomerResponseData
 import app.cash.paykit.core.models.response.Grant
 import app.cash.paykit.core.models.sdk.CashAppPayPaymentAction
-import app.cash.paykit.core.models.sdk.CashAppPayPaymentAction.OnFileAction
 import app.cash.paykit.core.network.MoshiProvider
 import app.cash.paykit.core.utils.Clock
 import app.cash.paykit.core.utils.ClockRealImpl
@@ -218,14 +217,6 @@ internal class PayKitAnalyticsEventDispatcherImpl(
     val moshiAdapter: JsonAdapter<List<Action>> = moshi.adapter()
     val apiActionsAsJson: String = moshiAdapter.toJson(apiActions)
 
-    // Inner payload of the ES2 event.
-    var possibleReferenceId: String? = null
-    for (paymentAction in paymentKitActions) {
-      if (paymentAction is OnFileAction) {
-        possibleReferenceId = paymentAction.accountReferenceId
-      }
-    }
-
     return AnalyticsCustomerRequestPayload(
       sdkVersion,
       userAgent,
@@ -235,7 +226,6 @@ internal class PayKitAnalyticsEventDispatcherImpl(
       createActions = apiActionsAsJson,
       createChannel = CHANNEL_IN_APP,
       createRedirectUrl = redirectUri?.let { PiiString(redirectUri) },
-      createReferenceId = possibleReferenceId?.let { PiiString(possibleReferenceId) },
       environment = sdkEnvironment,
     )
   }
@@ -285,7 +275,6 @@ internal class PayKitAnalyticsEventDispatcherImpl(
       customerId = customerResponseData?.customerProfile?.id,
       customerCashTag = customerResponseData?.customerProfile?.cashTag,
       requestId = customerResponseData?.id,
-      referenceId = customerResponseData?.referenceId,
       environment = sdkEnvironment,
     )
   }

diff --git a/core/src/main/java/app/cash/paykit/core/impl/CashAppPayImpl.kt b/core/src/main/java/app/cash/paykit/core/impl/CashAppPayImpl.kt
@@ -120,8 +120,12 @@ internal class CashAppPayImpl(
     analyticsEventDispatcher.sdkInitialized()
   }
 
-  override fun createCustomerRequest(paymentAction: CashAppPayPaymentAction, redirectUri: String?) {
-    createCustomerRequest(listOf(paymentAction), redirectUri)
+  override fun createCustomerRequest(
+    paymentAction: CashAppPayPaymentAction,
+    redirectUri: String?,
+    referenceId: String?,
+  ) {
+    createCustomerRequest(listOf(paymentAction), redirectUri, referenceId)
   }
 
   /**
@@ -132,7 +136,11 @@ internal class CashAppPayImpl(
    *                      Look at [PayKitPaymentAction] for more details.
    */
   @WorkerThread
-  override fun createCustomerRequest(paymentActions: List<CashAppPayPaymentAction>, redirectUri: String?) {
+  override fun createCustomerRequest(
+    paymentActions: List<CashAppPayPaymentAction>,
+    redirectUri: String?,
+    referenceId: String?,
+  ) {
     enforceRegisteredStateUpdatesListener()
 
     // Validate [paymentActions] is not empty.
@@ -145,7 +153,12 @@ internal class CashAppPayImpl(
     currentState = CreatingCustomerRequest
 
     // Network call.
-    val networkResult = networkManager.createCustomerRequest(clientId, paymentActions, redirectUri)
+    val networkResult = networkManager.createCustomerRequest(
+      clientId = clientId,
+      paymentActions = paymentActions,
+      redirectUri = redirectUri,
+      referenceId = referenceId,
+    )
     when (networkResult) {
       is Failure -> {
         currentState = CashAppPayExceptionState(networkResult.exception)
@@ -159,8 +172,12 @@ internal class CashAppPayImpl(
     }
   }
 
-  override fun updateCustomerRequest(requestId: String, paymentAction: CashAppPayPaymentAction) {
-    updateCustomerRequest(requestId, listOf(paymentAction))
+  override fun updateCustomerRequest(
+    requestId: String,
+    paymentAction: CashAppPayPaymentAction,
+    referenceId: String?,
+  ) {
+    updateCustomerRequest(requestId, listOf(paymentAction), referenceId)
   }
 
   /**
@@ -175,6 +192,7 @@ internal class CashAppPayImpl(
   override fun updateCustomerRequest(
     requestId: String,
     paymentActions: List<CashAppPayPaymentAction>,
+    referenceId: String?,
   ) {
     enforceRegisteredStateUpdatesListener()
 
@@ -188,7 +206,12 @@ internal class CashAppPayImpl(
     currentState = UpdatingCustomerRequest
 
     // Network request.
-    val networkResult = networkManager.updateCustomerRequest(clientId, requestId, paymentActions)
+    val networkResult = networkManager.updateCustomerRequest(
+      clientId = clientId,
+      requestId = requestId,
+      referenceId = referenceId,
+      paymentActions = paymentActions,
+    )
     when (networkResult) {
       is Failure -> {
         currentState = CashAppPayExceptionState(networkResult.exception)

diff --git a/core/src/main/java/app/cash/paykit/core/impl/NetworkManagerImpl.kt b/core/src/main/java/app/cash/paykit/core/impl/NetworkManagerImpl.kt
@@ -81,8 +81,14 @@ internal class NetworkManagerImpl(
     clientId: String,
     paymentActions: List<CashAppPayPaymentAction>,
     redirectUri: String?,
+    referenceId: String?,
   ): NetworkResult<CustomerTopLevelResponse> {
-    val customerRequestData = CustomerRequestDataFactory.build(clientId, redirectUri, paymentActions)
+    val customerRequestData = CustomerRequestDataFactory.build(
+      clientId = clientId,
+      redirectUri = redirectUri,
+      referenceId = referenceId,
+      paymentActions = paymentActions,
+    )
     val createCustomerRequest = CreateCustomerRequest(
       idempotencyKey = UUID.randomUUID().toString(),
       customerRequestData = customerRequestData,
@@ -103,16 +109,27 @@ internal class NetworkManagerImpl(
   override fun updateCustomerRequest(
     clientId: String,
     requestId: String,
+    referenceId: String?,
     paymentActions: List<CashAppPayPaymentAction>,
   ): NetworkResult<CustomerTopLevelResponse> {
     val customerRequestData =
-      CustomerRequestDataFactory.build(clientId, null, paymentActions, isRequestUpdate = true)
+      CustomerRequestDataFactory.build(
+        clientId = clientId,
+        redirectUri = null,
+        referenceId = referenceId,
+        paymentActions = paymentActions,
+        isRequestUpdate = true,
+      )
     val createCustomerRequest = CreateCustomerRequest(
       customerRequestData = customerRequestData,
     )
 
     // Record analytics.
-    analyticsEventDispatcher?.updatedCustomerRequest(requestId, paymentActions, customerRequestData.actions)
+    analyticsEventDispatcher?.updatedCustomerRequest(
+      requestId = requestId,
+      paymentKitActions = paymentActions,
+      apiActions = customerRequestData.actions,
+    )
 
     return executeNetworkRequest(
       PATCH,

diff --git a/core/src/main/java/app/cash/paykit/core/models/common/Action.kt b/core/src/main/java/app/cash/paykit/core/models/common/Action.kt
@@ -15,6 +15,7 @@
  */
 package app.cash.paykit.core.models.common
 
+import app.cash.paykit.core.models.pii.PiiString
 import com.squareup.moshi.Json
 import com.squareup.moshi.JsonClass
 
@@ -28,4 +29,6 @@ data class Action(
   val scopeId: String,
   @Json(name = "type")
   val type: String,
+  @Json(name = "account_reference_id")
+  val accountReferenceId: PiiString? = null,
 )
diff --git a/core/src/main/java/app/cash/paykit/core/models/request/CustomerRequestDataFactory.kt b/core/src/main/java/app/cash/paykit/core/models/request/CustomerRequestDataFactory.kt
@@ -33,20 +33,15 @@ object CustomerRequestDataFactory {
   fun build(
     clientId: String,
     redirectUri: String?,
+    referenceId: String?,
     paymentActions: List<CashAppPayPaymentAction>,
     isRequestUpdate: Boolean = false,
   ): CustomerRequestData {
     val actions = ArrayList<Action>(paymentActions.size)
-    var possibleReferenceId: String? = null
 
     for (paymentAction in paymentActions) {
       when (paymentAction) {
-        is OnFileAction -> {
-          actions.add(buildFromOnFileAction(clientId, paymentAction))
-          if (paymentAction.accountReferenceId != null) {
-            possibleReferenceId = paymentAction.accountReferenceId
-          }
-        }
+        is OnFileAction -> actions.add(buildFromOnFileAction(clientId, paymentAction))
         is OneTimeAction -> actions.add(buildFromOneTimeAction(clientId, paymentAction))
       }
     }
@@ -56,14 +51,14 @@ object CustomerRequestDataFactory {
         actions = actions,
         channel = null,
         redirectUri = null,
-        referenceId = possibleReferenceId?.let { PiiString(it) },
+        referenceId = referenceId?.let { PiiString(it) },
       )
     } else {
       CustomerRequestData(
         actions = actions,
         channel = CHANNEL_IN_APP,
         redirectUri = redirectUri?.let { PiiString(it) },
-        referenceId = possibleReferenceId?.let { PiiString(it) },
+        referenceId = referenceId?.let { PiiString(it) },
       )
     }
   }
@@ -78,6 +73,7 @@ object CustomerRequestDataFactory {
     return Action(
       scopeId = scopeIdOrClientId,
       type = PAYMENT_TYPE_ON_FILE,
+      accountReferenceId = onFileAction.accountReferenceId?.let { PiiString(it) },
     )
   }
 

diff --git a/core/src/main/java/app/cash/paykit/core/models/sdk/CashAppPayPaymentAction.kt b/core/src/main/java/app/cash/paykit/core/models/sdk/CashAppPayPaymentAction.kt
@@ -20,7 +20,7 @@ import app.cash.paykit.core.CashAppPay
 /**
  * This class holds the information necessary for [CashAppPay.createCustomerRequest] to be executed.
  */
-sealed class CashAppPayPaymentAction(scopeId: String?) {
+sealed class CashAppPayPaymentAction(open val scopeId: String?, open val referenceId: String?) {
 
   /**
    * Describes an intent for a client to charge a customer a given amount.
@@ -38,8 +38,9 @@ sealed class CashAppPayPaymentAction(scopeId: String?) {
   data class OneTimeAction(
     val currency: CashAppPayCurrency?,
     val amount: Int?,
-    val scopeId: String? = null,
-  ) : CashAppPayPaymentAction(scopeId)
+    override val scopeId: String? = null,
+    override val referenceId: String? = null,
+  ) : CashAppPayPaymentAction(scopeId, referenceId)
 
   /**
    * Describes an intent for a client to store a customer's account, allowing a client to create payments
@@ -49,8 +50,9 @@ sealed class CashAppPayPaymentAction(scopeId: String?) {
    * @param accountReferenceId Identifier of the account or customer associated to the on file action.
    */
   data class OnFileAction(
-    val scopeId: String? = null,
+    override val scopeId: String? = null,
     val accountReferenceId: String? = null,
+    override val referenceId: String? = null,
   ) :
-    CashAppPayPaymentAction(scopeId)
+    CashAppPayPaymentAction(scopeId, referenceId)
 }
diff --git a/core/src/test/java/app/cash/paykit/core/CashAppPayExceptionsTests.kt b/core/src/test/java/app/cash/paykit/core/CashAppPayExceptionsTests.kt
@@ -56,7 +56,7 @@ class CashAppPayExceptionsTests {
     val listener = mockk<CashAppPayListener>(relaxed = true)
     payKit.registerForStateUpdates(listener)
 
-    every { networkManager.createCustomerRequest(any(), any(), any()) } returns NetworkResult.failure(
+    every { networkManager.createCustomerRequest(any(), any(), any(), any()) } returns NetworkResult.failure(
       Exception("bad"),
     )
     payKit.createCustomerRequest(FakeData.oneTimePayment, FakeData.REDIRECT_URI)

diff --git a/core/src/test/java/app/cash/paykit/core/CashAppPayStateTests.kt b/core/src/test/java/app/cash/paykit/core/CashAppPayStateTests.kt
@@ -76,7 +76,7 @@ class CashAppPayStateTests {
     val listener = mockk<CashAppPayListener>(relaxed = true)
     payKit.registerForStateUpdates(listener)
 
-    every { networkManager.createCustomerRequest(any(), any(), any()) } returns NetworkResult.failure(
+    every { networkManager.createCustomerRequest(any(), any(), any(), any()) } returns NetworkResult.failure(
       Exception("bad"),
     )
 
@@ -95,6 +95,7 @@ class CashAppPayStateTests {
         any(),
         any(),
         any(),
+        any(),
       )
     } returns NetworkResult.failure(
       Exception("bad"),
@@ -180,6 +181,7 @@ class CashAppPayStateTests {
         any(),
         any(),
         any(),
+        any(),
       )
     } returns customerTopLevelResponse