From bea8d223cd131454b97cc85146eff0108e6738ce Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 14 Aug 2026 14:39:23 +0200 Subject: [PATCH 1/5] Report the refund calculation flow in POS analytics The server-computed create and the classic create emitted identical events, and the route-availability fallback emitted nothing, so a rollout of woo_pos_server_refunds could not be measured: no adoption figure, no way to compare failure rates between the flows, and no telemetry to attribute a "totals look wrong" report to either. Add a refund_flow property (local / server_computed) to the refund processing events, carry the store's REST error code on failures so deterministic rejections separate from transport errors, and emit a new event when a preview probe finds the route missing, carrying the store's WooCommerce version so a misjudged version gate is visible. The reported flow is decided by the same predicate the submission path branches on, extracted as isServerComputedRefundConfirmed(), so it cannot drift from the path actually taken. Co-Authored-By: Claude Opus 5 (1M context) --- RELEASE-NOTES.txt | 1 + .../details/refund/WooPosRefundPreview.kt | 8 ++ .../refund/WooPosRefundSubmissionProcessor.kt | 1 + .../refund/WooPosRefundSubmissionState.kt | 6 ++ .../details/refund/WooPosRefundViewModel.kt | 48 ++++++++--- .../util/analytics/WooPosAnalyticsEvent.kt | 63 +++++++++++++- .../analytics/WooPosAnalyticsEventConstant.kt | 18 ++++ .../details/refund/WooPosRefundPreviewTest.kt | 41 ++++++++- .../refund/WooPosRefundViewModelTest.kt | 83 ++++++++++++++++++- 9 files changed, 251 insertions(+), 18 deletions(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index 26fc932943b6..bd2a9f80447d 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 - [*] 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 d4a712500c39..ef94bdbe0806 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,11 @@ 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) + // Reported once per store and version: the cache short-circuits the resolver on + // later refunds, so this counts stores that fell back rather than refunds. + 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 d33205a1594e..25e50077355b 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 d840bb2e1606..7315c73e38db 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 @@ -13,10 +13,16 @@ sealed class WooPosRefundSubmissionState { data object ProcessingReaderRefund : WooPosRefundSubmissionState() data object NotifyingStore : WooPosRefundSubmissionState() data object Success : WooPosRefundSubmissionState() + /** + * [apiErrorCode] is the REST error code the store returned, when there was one. It is carried + * for analytics: the message alone is localized to the store and varies by wording, so it + * cannot separate a deterministic server rejection from a transport failure. + */ data class Failure( val message: String, 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 4d712815c6f0..3ae11e840157 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,23 @@ class WooPosRefundViewModel @AssistedInject constructor( ) } + /** + * Whether this submission will go through the server-computed create. Extracted so the flow + * reported in analytics is decided by the same predicate [buildSubmissionRequest] branches on, + * and cannot drift from the path actually taken. + */ + 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 +658,7 @@ class WooPosRefundViewModel @AssistedInject constructor( } is WooPosRefundSubmissionState.Failure -> { - handleRefundSubmissionFailure(submissionState) + handleRefundSubmissionFailure(submissionState, refundFlowFor(request)) } } } @@ -659,7 +679,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 +696,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 7c4a73a2da4a..fbec54e8c340 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,71 @@ 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() { + /** + * [apiErrorCode] is the store's REST error code when it returned one. It separates + * deterministic server rejections (`woocommerce_rest_*`) from transport failures, which the + * message cannot do — it is localized to the store and varies by wording. + */ + 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) } + } + ) + } + } + + /** + * Emitted when a preview probe finds the server-calculated refund route missing and the + * store falls back to local calculation. [wooVersion] tells us whether the version gate is + * behaving or the store is genuinely too old. Fired where the availability cache is marked + * unavailable, so it counts stores rather than refunds. + */ + 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 559e7ac267b2..bd82c5b5e091 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,24 @@ package com.woocommerce.android.ui.woopos.util.analytics object WooPosAnalyticsEventConstant { + /** + * Which side calculated the refund totals. Reported on the refund processing events so success + * and failure rates can be compared between the two flows during the server-refunds rollout. + * Keep the values in step with iOS, which reports the same `refund_flow` property. + */ + 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 de3827f6daaa..8e967e76a4da 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 the fallback is measurable, and the version says whether the gate misjudged the store + 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 0640e9913879..4ad01d4fff58 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 — server refunds are confirmed available, so the computed create is used. + 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 both the start and the outcome are attributable to the server flow. + 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 the deterministic rejection is separable from a transport failure. + 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 { From adac916073f6b60d7bee14d47cd6b76b94c90d72 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 14 Aug 2026 14:40:17 +0200 Subject: [PATCH 2/5] Add the PR link to the release note Co-Authored-By: Claude Opus 5 (1M context) --- RELEASE-NOTES.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RELEASE-NOTES.txt b/RELEASE-NOTES.txt index bd2a9f80447d..7542bedbff9d 100644 --- a/RELEASE-NOTES.txt +++ b/RELEASE-NOTES.txt @@ -4,7 +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 +- [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] From 948aec6651cbc79a1ad149f028f329515c5d3560 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 14 Aug 2026 16:00:47 +0200 Subject: [PATCH 3/5] Fix detekt spacing and drop prose from the new test comments Co-Authored-By: Claude Opus 5 (1M context) --- .../orders/details/refund/WooPosRefundSubmissionState.kt | 1 + .../woopos/orders/details/refund/WooPosRefundPreviewTest.kt | 2 +- .../orders/details/refund/WooPosRefundViewModelTest.kt | 6 +++--- 3 files changed, 5 insertions(+), 4 deletions(-) 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 7315c73e38db..2f73a7a9adce 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 @@ -13,6 +13,7 @@ sealed class WooPosRefundSubmissionState { data object ProcessingReaderRefund : WooPosRefundSubmissionState() data object NotifyingStore : WooPosRefundSubmissionState() data object Success : WooPosRefundSubmissionState() + /** * [apiErrorCode] is the REST error code the store returned, when there was one. It is carried * for analytics: the message alone is localized to the store and varies by wording, so it 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 8e967e76a4da..1633ae1a4e81 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 @@ -120,7 +120,7 @@ class WooPosRefundPreviewTest { // WHEN sut(ORDER_ID, lineItems) - // THEN the fallback is measurable, and the version says whether the gate misjudged the store + // THEN verify(analyticsTracker).track( WooPosAnalyticsEvent.Event.RefundServerFlowUnavailable(wooVersion = MIN_VERSION) ) 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 4ad01d4fff58..fa62dbf39889 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 @@ -2145,7 +2145,7 @@ class WooPosRefundViewModelTest { @Test fun `given server-computed refund succeeds, when API call completes, then events report the server flow`() = runTest { - // GIVEN — server refunds are confirmed available, so the computed create is used. + // GIVEN serverRefundAvailabilityCache.markAvailable(testSite.localId().value, MIN_VERSION) val refundableItems = listOf(testRefundableItem) whenever(ordersDataSource.refreshOrderById(testOrderId)).thenReturn(Result.success(testOrder)) @@ -2166,7 +2166,7 @@ class WooPosRefundViewModelTest { viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) advanceUntilIdle() - // THEN both the start and the outcome are attributable to the server flow. + // THEN verify(analyticsTracker).track( WooPosAnalyticsEvent.Event.RefundProcessingStarted(RefundFlow.SERVER_COMPUTED) ) @@ -2209,7 +2209,7 @@ class WooPosRefundViewModelTest { viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) advanceUntilIdle() - // THEN the deterministic rejection is separable from a transport failure. + // THEN verify(analyticsTracker).track( WooPosAnalyticsEvent.Event.RefundProcessingFailed( refundFlow = RefundFlow.LOCAL, From f6f293df88a12be17f05b236106d7089736c71e0 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 14 Aug 2026 16:32:06 +0200 Subject: [PATCH 4/5] Remove Given/When/Then markers from the new tests Co-Authored-By: Claude Opus 5 (1M context) --- .../woopos/orders/details/refund/WooPosRefundPreviewTest.kt | 6 ------ .../orders/details/refund/WooPosRefundViewModelTest.kt | 5 ----- 2 files changed, 11 deletions(-) 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 1633ae1a4e81..3117c05c795b 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 @@ -113,14 +113,11 @@ class WooPosRefundPreviewTest { @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) ) @@ -128,14 +125,11 @@ class WooPosRefundPreviewTest { @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()) } 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 fa62dbf39889..de03792a847a 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 @@ -2145,7 +2145,6 @@ class WooPosRefundViewModelTest { @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)) @@ -2160,13 +2159,11 @@ class WooPosRefundViewModelTest { 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) ) @@ -2205,11 +2202,9 @@ class WooPosRefundViewModelTest { viewModel.onUIEvent(WooPosRefundUIEvent.RefundFlowOpened) advanceUntilIdle() - // WHEN viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) advanceUntilIdle() - // THEN verify(analyticsTracker).track( WooPosAnalyticsEvent.Event.RefundProcessingFailed( refundFlow = RefundFlow.LOCAL, From 63e2a5eef5d1e88a5f0bb44561e5739ec799f328 Mon Sep 17 00:00:00 2001 From: samiuelson Date: Fri, 14 Aug 2026 16:41:38 +0200 Subject: [PATCH 5/5] Remove comments from the production code, restore them in tests Reverses the previous commit, which stripped the Given/When/Then markers from the new tests. The intent was the opposite: production code carries no comments here, tests keep their structure markers. Co-Authored-By: Claude Opus 5 (1M context) --- .../orders/details/refund/WooPosRefundPreview.kt | 2 -- .../details/refund/WooPosRefundSubmissionState.kt | 6 ------ .../orders/details/refund/WooPosRefundViewModel.kt | 5 ----- .../ui/woopos/util/analytics/WooPosAnalyticsEvent.kt | 11 ----------- .../util/analytics/WooPosAnalyticsEventConstant.kt | 5 ----- .../orders/details/refund/WooPosRefundPreviewTest.kt | 6 ++++++ .../details/refund/WooPosRefundViewModelTest.kt | 5 +++++ 7 files changed, 11 insertions(+), 29 deletions(-) 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 ef94bdbe0806..806da9d12094 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 @@ -48,8 +48,6 @@ 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) - // Reported once per store and version: the cache short-circuits the resolver on - // later refunds, so this counts stores that fell back rather than refunds. analyticsTracker.track( WooPosAnalyticsEvent.Event.RefundServerFlowUnavailable(wooVersion = flow.wooVersion) ) 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 2f73a7a9adce..e9733dd69430 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 @@ -13,12 +13,6 @@ sealed class WooPosRefundSubmissionState { data object ProcessingReaderRefund : WooPosRefundSubmissionState() data object NotifyingStore : WooPosRefundSubmissionState() data object Success : WooPosRefundSubmissionState() - - /** - * [apiErrorCode] is the REST error code the store returned, when there was one. It is carried - * for analytics: the message alone is localized to the store and varies by wording, so it - * cannot separate a deterministic server rejection from a transport failure. - */ data class Failure( val message: String, val retryBackendNotificationOnly: Boolean = false, 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 3ae11e840157..185345e05822 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 @@ -585,11 +585,6 @@ class WooPosRefundViewModel @AssistedInject constructor( ) } - /** - * Whether this submission will go through the server-computed create. Extracted so the flow - * reported in analytics is decided by the same predicate [buildSubmissionRequest] branches on, - * and cannot drift from the path actually taken. - */ private fun isServerComputedRefundConfirmed(): Boolean { val flow = resolveRefundFlow() return flow is WooPosRefundFlow.ServerComputed && 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 fbec54e8c340..ee90553799b4 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 @@ -1057,11 +1057,6 @@ sealed class WooPosAnalyticsEvent : IAnalyticsEvent { } } - /** - * [apiErrorCode] is the store's REST error code when it returned one. It separates - * deterministic server rejections (`woocommerce_rest_*`) from transport failures, which the - * message cannot do — it is localized to the store and varies by wording. - */ data class RefundProcessingFailed( val refundFlow: RefundFlow, val apiErrorCode: String? = null, @@ -1078,12 +1073,6 @@ sealed class WooPosAnalyticsEvent : IAnalyticsEvent { } } - /** - * Emitted when a preview probe finds the server-calculated refund route missing and the - * store falls back to local calculation. [wooVersion] tells us whether the version gate is - * behaving or the store is genuinely too old. Fired where the availability cache is marked - * unavailable, so it counts stores rather than refunds. - */ data class RefundServerFlowUnavailable(val wooVersion: String) : Event() { override val name: String = "refund_server_flow_unavailable" 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 bd82c5b5e091..a9ab3c7e9974 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,11 +1,6 @@ package com.woocommerce.android.ui.woopos.util.analytics object WooPosAnalyticsEventConstant { - /** - * Which side calculated the refund totals. Reported on the refund processing events so success - * and failure rates can be compared between the two flows during the server-refunds rollout. - * Keep the values in step with iOS, which reports the same `refund_flow` property. - */ enum class RefundFlow(val value: String) { LOCAL("local"), SERVER_COMPUTED("server_computed"); 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 3117c05c795b..1633ae1a4e81 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 @@ -113,11 +113,14 @@ class WooPosRefundPreviewTest { @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) ) @@ -125,11 +128,14 @@ class WooPosRefundPreviewTest { @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()) } 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 de03792a847a..fa62dbf39889 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 @@ -2145,6 +2145,7 @@ class WooPosRefundViewModelTest { @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)) @@ -2159,11 +2160,13 @@ class WooPosRefundViewModelTest { 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) ) @@ -2202,9 +2205,11 @@ class WooPosRefundViewModelTest { viewModel.onUIEvent(WooPosRefundUIEvent.RefundFlowOpened) advanceUntilIdle() + // WHEN viewModel.onUIEvent(WooPosRefundUIEvent.OnRefundConfirmed) advanceUntilIdle() + // THEN verify(analyticsTracker).track( WooPosAnalyticsEvent.Event.RefundProcessingFailed( refundFlow = RefundFlow.LOCAL,