diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 26fc932943b..7542bedbff9 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -4,6 +4,7 @@ 25.5 ----- +- [Internal] Woo POS refunds: refund events now report whether totals were calculated locally or by the server, the API error code on failures, and when a store falls back for lack of the server route [https://github.com/woocommerce/woocommerce-android/pull/16419] - [*] The Pay In Person toggle on the Payments screen no longer looks turned off when its status could not be loaded - [Internal] Woo POS: every POS analytics event now carries device_type (phone/tablet) and entry_point, so POS usage can be split by form factor and attributed to where POS was opened from [https://github.com/woocommerce/woocommerce-android/pull/16414] - [*] Woo POS: Remote Tap to Pay failures now explain what went wrong - phone not eligible for Tap to Pay, NFC turned off, or a payment service error - instead of a generic message or raw error text [https://github.com/woocommerce/woocommerce-android/pull/16384] diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundPreview.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundPreview.kt index d4a712500c3..806da9d1209 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundPreview.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundPreview.kt @@ -1,6 +1,8 @@ package com.woocommerce.android.ui.woopos.orders.details.refund import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent +import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker import com.woocommerce.android.util.WooLog import org.wordpress.android.fluxc.model.refunds.RefundPreviewLineItem import org.wordpress.android.fluxc.model.refunds.WCRefundPreview @@ -21,6 +23,7 @@ class WooPosRefundPreview @Inject constructor( private val selectedSite: SelectedSite, private val availabilityCache: WooPosServerRefundAvailabilityCache, private val resolveRefundFlow: WooPosResolveRefundFlow, + private val analyticsTracker: WooPosAnalyticsTracker, ) { suspend operator fun invoke( orderId: Long, @@ -45,6 +48,9 @@ class WooPosRefundPreview @Inject constructor( if (response.error.type == WooErrorType.API_NOT_FOUND) { WooLog.i(WooLog.T.POS, "WooPosRefund: preview route not available; falling back to local") availabilityCache.markUnavailable(localSiteId, flow.wooVersion) + analyticsTracker.track( + WooPosAnalyticsEvent.Event.RefundServerFlowUnavailable(wooVersion = flow.wooVersion) + ) Result.FallbackToLocal } else { WooLog.e( diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundSubmissionProcessor.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundSubmissionProcessor.kt index d33205a1594..25e50077355 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundSubmissionProcessor.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundSubmissionProcessor.kt @@ -299,6 +299,7 @@ class WooPosRefundSubmissionProcessor @Inject constructor( WooPosRefundSubmissionState.Failure( message = result.error.message ?: resourceProvider.getString(R.string.error_generic), retryBackendNotificationOnly = retryBackendNotificationOnly, + apiErrorCode = error.apiErrorCode, ) ) } else { diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundSubmissionState.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundSubmissionState.kt index d840bb2e160..e9733dd6943 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundSubmissionState.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundSubmissionState.kt @@ -18,5 +18,6 @@ sealed class WooPosRefundSubmissionState { val retryBackendNotificationOnly: Boolean = false, val retryCardRefund: Boolean = false, val canRetry: Boolean = !retryBackendNotificationOnly, + val apiErrorCode: String? = null, ) : WooPosRefundSubmissionState() } diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundViewModel.kt index 4d712815c6f..185345e0582 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundViewModel.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundViewModel.kt @@ -12,6 +12,7 @@ import com.woocommerce.android.ui.woopos.common.data.WooPosRetrieveOrderRefunds import com.woocommerce.android.ui.woopos.orders.WooPosGetPaymentMethod import com.woocommerce.android.ui.woopos.orders.WooPosOrdersDataSource import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent +import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.RefundFlow import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker import com.woocommerce.android.util.CurrencyFormatter import com.woocommerce.android.util.PriceUtils @@ -508,7 +509,15 @@ class WooPosRefundViewModel @AssistedInject constructor( contentStateBeforeRefund = contentState _state.value = contentState.copy(step = WooPosRefundState.Content.RefundStep.Processing) - analyticsTracker.track(WooPosAnalyticsEvent.Event.RefundProcessingStarted) + analyticsTracker.track( + WooPosAnalyticsEvent.Event.RefundProcessingStarted( + refundFlow = if (isServerComputedRefundConfirmed()) { + RefundFlow.SERVER_COMPUTED + } else { + RefundFlow.LOCAL + } + ) + ) val order = currentOrder ?: run { WooLog.e( @@ -552,13 +561,7 @@ class WooPosRefundViewModel @AssistedInject constructor( contentState: WooPosRefundState.Content, selectedItems: List, ): WooPosRefundSubmissionRequest? { - val flow = resolveRefundFlow() - val serverRefundsConfirmedAvailable = flow is WooPosRefundFlow.ServerComputed && - serverRefundAvailabilityCache.isAvailable( - localSiteId = selectedSite.get().localId().value, - wooVersion = flow.wooVersion, - ) == true - if (serverRefundsConfirmedAvailable) { + if (isServerComputedRefundConfirmed()) { return WooPosRefundSubmissionRequest( order = order, refundAmount = contentState.total, @@ -582,6 +585,18 @@ class WooPosRefundViewModel @AssistedInject constructor( ) } + private fun isServerComputedRefundConfirmed(): Boolean { + val flow = resolveRefundFlow() + return flow is WooPosRefundFlow.ServerComputed && + serverRefundAvailabilityCache.isAvailable( + localSiteId = selectedSite.get().localId().value, + wooVersion = flow.wooVersion, + ) == true + } + + private fun refundFlowFor(request: WooPosRefundSubmissionRequest): RefundFlow = + if (request.serverLineItems != null) RefundFlow.SERVER_COMPUTED else RefundFlow.LOCAL + private fun submitRefund( contentState: WooPosRefundState.Content, request: WooPosRefundSubmissionRequest, @@ -638,7 +653,7 @@ class WooPosRefundViewModel @AssistedInject constructor( } is WooPosRefundSubmissionState.Failure -> { - handleRefundSubmissionFailure(submissionState) + handleRefundSubmissionFailure(submissionState, refundFlowFor(request)) } } } @@ -659,7 +674,7 @@ class WooPosRefundViewModel @AssistedInject constructor( contentState: WooPosRefundState.Content, request: WooPosRefundSubmissionRequest, ) { - analyticsTracker.track(WooPosAnalyticsEvent.Event.RefundProcessingSuccess) + analyticsTracker.track(WooPosAnalyticsEvent.Event.RefundProcessingSuccess(refundFlowFor(request))) val receiptSentMessage = request.order.billingAddress.email .takeIf { it.isNotBlank() } ?.let { email -> @@ -676,8 +691,14 @@ class WooPosRefundViewModel @AssistedInject constructor( private suspend fun handleRefundSubmissionFailure( submissionState: WooPosRefundSubmissionState.Failure, + refundFlow: RefundFlow, ) { - analyticsTracker.track(WooPosAnalyticsEvent.Event.RefundProcessingFailed) + analyticsTracker.track( + WooPosAnalyticsEvent.Event.RefundProcessingFailed( + refundFlow = refundFlow, + apiErrorCode = submissionState.apiErrorCode, + ) + ) _state.value = WooPosRefundState.Error( message = submissionState.message, errorType = WooPosRefundState.Error.ErrorType.Processing, diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt index 7c4a73a2da4..ee90553799b 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEvent.kt @@ -1,5 +1,6 @@ package com.woocommerce.android.ui.woopos.util.analytics +import com.woocommerce.android.analytics.AnalyticsTracker import com.woocommerce.android.analytics.IAnalyticsEvent import com.woocommerce.android.ui.woopos.home.cart.WooPosCartItemViewState import com.woocommerce.android.ui.woopos.home.items.WooPosItemsViewModel @@ -12,6 +13,7 @@ import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventCons import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.ItemsListProductType import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.ItemsListSource import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.ItemsListSourceType +import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.RefundFlow import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.SyncErrorType import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.SyncSkipReason import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.SyncType @@ -1027,16 +1029,60 @@ sealed class WooPosAnalyticsEvent : IAnalyticsEvent { } } - data object RefundProcessingStarted : Event() { + data class RefundProcessingStarted( + val refundFlow: RefundFlow + ) : Event() { override val name: String = "refund_processing_started" + + init { + addProperties( + mapOf( + RefundFlow.REFUND_FLOW to refundFlow.value + ) + ) + } } - data object RefundProcessingSuccess : Event() { + data class RefundProcessingSuccess( + val refundFlow: RefundFlow + ) : Event() { override val name: String = "refund_processing_success" + + init { + addProperties( + mapOf( + RefundFlow.REFUND_FLOW to refundFlow.value + ) + ) + } } - data object RefundProcessingFailed : Event() { + data class RefundProcessingFailed( + val refundFlow: RefundFlow, + val apiErrorCode: String? = null, + ) : Event() { override val name: String = "refund_processing_failed" + + init { + addProperties( + buildMap { + put(RefundFlow.REFUND_FLOW, refundFlow.value) + apiErrorCode?.let { put(AnalyticsTracker.KEY_API_ERROR_CODE, it) } + } + ) + } + } + + data class RefundServerFlowUnavailable(val wooVersion: String) : Event() { + override val name: String = "refund_server_flow_unavailable" + + init { + addProperties( + mapOf( + "woocommerce_version" to wooVersion + ) + ) + } } data class RefundFlowAborted(val refundStep: String) : Event() { diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEventConstant.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEventConstant.kt index 559e7ac267b..a9ab3c7e997 100644 --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEventConstant.kt +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/util/analytics/WooPosAnalyticsEventConstant.kt @@ -1,6 +1,19 @@ package com.woocommerce.android.ui.woopos.util.analytics object WooPosAnalyticsEventConstant { + enum class RefundFlow(val value: String) { + LOCAL("local"), + SERVER_COMPUTED("server_computed"); + + override fun toString(): String { + return value + } + + companion object { + const val REFUND_FLOW = "refund_flow" + } + } + enum class DeviceType(val value: String) { PHONE("phone"), TABLET("tablet"); diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundPreviewTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundPreviewTest.kt index de3827f6daa..1633ae1a4e8 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundPreviewTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundPreviewTest.kt @@ -1,6 +1,8 @@ package com.woocommerce.android.ui.woopos.orders.details.refund import com.woocommerce.android.tools.SelectedSite +import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent +import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker import com.woocommerce.android.util.FeatureFlag import com.woocommerce.android.util.FeatureFlagRepository import com.woocommerce.android.util.GetWooCorePluginCachedVersion @@ -41,6 +43,8 @@ class WooPosRefundPreviewTest { on { isEnabled(FeatureFlag.WOO_POS_SERVER_REFUNDS) } doReturn true } + private val analyticsTracker: WooPosAnalyticsTracker = mock() + private val site = SiteModel().apply { id = LOCAL_SITE_ID siteId = SITE_ID @@ -49,7 +53,13 @@ class WooPosRefundPreviewTest { private val sut by lazy { whenever(selectedSite.get()).thenReturn(site) - WooPosRefundPreview(refundStore, selectedSite, availabilityCache, resolveRefundFlowFor(selectedSite)) + WooPosRefundPreview( + refundStore, + selectedSite, + availabilityCache, + resolveRefundFlowFor(selectedSite), + analyticsTracker, + ) } private fun resolveRefundFlowFor(selectedSite: SelectedSite) = WooPosResolveRefundFlow( @@ -101,6 +111,34 @@ class WooPosRefundPreviewTest { assertThat(availabilityCache.isAvailable(LOCAL_SITE_ID, MIN_VERSION)).isFalse() } + @Test + fun `given preview returns 404, when invoked, then tracks the fallback with the store woo version`() = runTest { + // GIVEN + whenever(refundStore.previewRefund(eq(site), eq(ORDER_ID), eq(lineItems))) + .thenReturn(WooResult(WooError(WooErrorType.API_NOT_FOUND, GenericErrorType.NOT_FOUND))) + + // WHEN + sut(ORDER_ID, lineItems) + + // THEN + verify(analyticsTracker).track( + WooPosAnalyticsEvent.Event.RefundServerFlowUnavailable(wooVersion = MIN_VERSION) + ) + } + + @Test + fun `given preview succeeds, when invoked, then does not track a fallback`() = runTest { + // GIVEN + whenever(refundStore.previewRefund(eq(site), eq(ORDER_ID), eq(lineItems))) + .thenReturn(WooResult(preview())) + + // WHEN + sut(ORDER_ID, lineItems) + + // THEN + verify(analyticsTracker, never()).track(any()) + } + @Test fun `given non-404 error, when invoked, then returns Error`() = runTest { // GIVEN @@ -201,6 +239,7 @@ class WooPosRefundPreviewTest { selectedSiteB, availabilityCache, resolveRefundFlowFor(selectedSiteB), + analyticsTracker, ) // WHEN siteB requests a preview diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundViewModelTest.kt index 0640e991387..fa62dbf3988 100644 --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundViewModelTest.kt +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/orders/details/refund/WooPosRefundViewModelTest.kt @@ -14,6 +14,7 @@ import com.woocommerce.android.ui.woopos.orders.WooPosGetPaymentMethod import com.woocommerce.android.ui.woopos.orders.WooPosOrdersDataSource import com.woocommerce.android.ui.woopos.util.WooPosCoroutineTestRule import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent +import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEventConstant.RefundFlow import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker import com.woocommerce.android.util.CurrencyFormatter import com.woocommerce.android.util.FeatureFlag @@ -2075,7 +2076,7 @@ class WooPosRefundViewModelTest { viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) advanceUntilIdle() - verify(analyticsTracker).track(WooPosAnalyticsEvent.Event.RefundProcessingStarted) + verify(analyticsTracker).track(WooPosAnalyticsEvent.Event.RefundProcessingStarted(RefundFlow.LOCAL)) } @Test @@ -2102,7 +2103,7 @@ class WooPosRefundViewModelTest { viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) advanceUntilIdle() - verify(analyticsTracker).track(WooPosAnalyticsEvent.Event.RefundProcessingSuccess) + verify(analyticsTracker).track(WooPosAnalyticsEvent.Event.RefundProcessingSuccess(RefundFlow.LOCAL)) } @Test @@ -2136,11 +2137,87 @@ class WooPosRefundViewModelTest { viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) advanceUntilIdle() - verify(analyticsTracker).track(WooPosAnalyticsEvent.Event.RefundProcessingFailed) + verify(analyticsTracker).track(WooPosAnalyticsEvent.Event.RefundProcessingFailed(RefundFlow.LOCAL)) val errorState = viewModel.state.value as WooPosRefundState.Error assertThat(errorState.canRetry).isTrue() } + @Test + fun `given server-computed refund succeeds, when API call completes, then events report the server flow`() = + runTest { + // GIVEN + serverRefundAvailabilityCache.markAvailable(testSite.localId().value, MIN_VERSION) + val refundableItems = listOf(testRefundableItem) + whenever(ordersDataSource.refreshOrderById(testOrderId)).thenReturn(Result.success(testOrder)) + whenever(retrieveOrderRefunds.invoke(eq(testOrder), any())).thenReturn(Result.success(emptyList())) + whenever(getRefundableItems.invoke(any(), any())).thenReturn(refundableItems) + whenever(refundPreview.invoke(any(), any())).thenReturn( + WooPosRefundPreview.Result.ServerCalculated( + refundPreview(subtotal = "20.00", tax = "2.00", total = "22.00", maxRefundable = "22.00") + ) + ) + viewModel = createViewModel() + viewModel.onUIEvent(WooPosRefundUIEvent.RefundFlowOpened) + advanceUntilIdle() + + // WHEN + viewModel.onUIEvent(WooPosRefundUIEvent.ContinueToReviewClicked) + advanceUntilIdle() + viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) + advanceUntilIdle() + + // THEN + verify(analyticsTracker).track( + WooPosAnalyticsEvent.Event.RefundProcessingStarted(RefundFlow.SERVER_COMPUTED) + ) + verify(analyticsTracker).track( + WooPosAnalyticsEvent.Event.RefundProcessingSuccess(RefundFlow.SERVER_COMPUTED) + ) + } + + @Test + fun `given refund rejected with an API error code, when it fails, then the code is reported`() = + runTest { + val refundableItems = listOf(testRefundableItem) + val groupedItems = listOf( + RefundRequestItem( + itemId = 1L, + quantity = 1, + refundTotal = BigDecimal("20.00"), + refundTax = emptyList() + ) + ) + + whenever(ordersDataSource.refreshOrderById(testOrderId)).thenReturn(Result.success(testOrder)) + whenever(retrieveOrderRefunds.invoke(eq(testOrder), any())).thenReturn(Result.success(emptyList())) + whenever(getRefundableItems.invoke(any(), any())).thenReturn(refundableItems) + whenever(groupRefundItems.invoke(eq(refundableItems), eq(testOrder), any())).thenReturn(groupedItems) + whenever(refundSubmissionProcessor.submit(any())).thenReturn( + flowOf( + WooPosRefundSubmissionState.Failure( + message = "Refund failed", + apiErrorCode = "woocommerce_rest_refund_exceeds_remaining", + ) + ) + ) + + viewModel = createViewModel() + viewModel.onUIEvent(WooPosRefundUIEvent.RefundFlowOpened) + advanceUntilIdle() + + // WHEN + viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) + advanceUntilIdle() + + // THEN + verify(analyticsTracker).track( + WooPosAnalyticsEvent.Event.RefundProcessingFailed( + refundFlow = RefundFlow.LOCAL, + apiErrorCode = "woocommerce_rest_refund_exceeds_remaining", + ) + ) + } + @Test fun `given backend notification fails after terminal refund succeeds, when API call completes, then error is not retryable`() = runTest {