Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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,
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -552,13 +561,7 @@ class WooPosRefundViewModel @AssistedInject constructor(
contentState: WooPosRefundState.Content,
selectedItems: List<WooPosRefundableItem>,
): 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,
Expand All @@ -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,
Expand Down Expand Up @@ -638,7 +658,7 @@ class WooPosRefundViewModel @AssistedInject constructor(
}

is WooPosRefundSubmissionState.Failure -> {
handleRefundSubmissionFailure(submissionState)
handleRefundSubmissionFailure(submissionState, refundFlowFor(request))
}
}
}
Expand All @@ -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 ->
Expand All @@ -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,
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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");
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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(
Expand Down Expand Up @@ -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<WooPosAnalyticsEvent.Event.RefundServerFlowUnavailable>())
}

@Test
fun `given non-404 error, when invoked, then returns Error`() = runTest {
// GIVEN
Expand Down Expand Up @@ -201,6 +239,7 @@ class WooPosRefundPreviewTest {
selectedSiteB,
availabilityCache,
resolveRefundFlowFor(selectedSiteB),
analyticsTracker,
)

// WHEN siteB requests a preview
Expand Down
Loading
Loading