Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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 [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]
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,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(
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 @@ -18,5 +18,6 @@ sealed class WooPosRefundSubmissionState {
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,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,
Expand Down Expand Up @@ -638,7 +653,7 @@ class WooPosRefundViewModel @AssistedInject constructor(
}

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