Skip to content
Merged
Show file tree
Hide file tree
Changes from 8 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 @@ -9,6 +9,7 @@
- [*] POS: Extra step into the barcode scanner setup flow to explain how to setup software keyboard [https://github.com/woocommerce/woocommerce-android/pull/14395]
- [Internal] Introduced Material 3 based theme and migrated some components to Material 3 [https://github.com/woocommerce/woocommerce-android/pull/14401]
- [Internal] Added logging to POS tab visibility and eligibility checks [https://github.com/woocommerce/woocommerce-android/pull/14426]
- [*] Shipping labels: Improved the error message for empty shipping rates list [https://github.com/woocommerce/woocommerce-android/pull/14433]

22.9.1
-----
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import com.woocommerce.android.ui.orders.wooshippinglabels.purchased.ObserveShip
import com.woocommerce.android.ui.orders.wooshippinglabels.purchased.printing.FetchShippingLabelFile
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.datasource.WooShippingRateModel
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.domain.GetShippingRates
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.domain.NoAvailableRatesException
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.ui.CarrierUI
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.ui.ShippingRateOption
import com.woocommerce.android.ui.orders.wooshippinglabels.rates.ui.ShippingRateUI
Expand Down Expand Up @@ -636,8 +637,14 @@ class WooShippingLabelCreationViewModel @Inject constructor(
trackShippingRatesLoading(isSuccess = true)
},
onFailure = { exception ->
updateState(ShippingRatesState.Error)
trackShippingRatesLoading(isSuccess = false, error = exception.message)
val errorMessage = if (exception is NoAvailableRatesException) {
exception.messageResId
} else {
trackShippingRatesLoading(isSuccess = false, error = exception.message)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trackShippingRatesLoading() was being triggered when the endpoint returned empty rates, but an empty rates case is not actually an error. So, I removed the tracking for it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure about this. I agree empty rates is not an error but we might want to track it anyway. We could add something like:

            analyticsTracker.track(AnalyticsEvent.WCS_RATE_SELECTION_STEP, mapOf(KEY_STATE to "loading_success_empty_rates"))

Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to include @itsmeichigo in the discussion. Do you track empty rates on iOS?

Copy link

@itsmeichigo itsmeichigo Aug 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On iOS we display error when the shipping rates loading returns an empty list. We track the error as loading failed as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JorgeMucientes, I reverted the change I introduced, so empty rates will continue to be tracked: 21d31bd.
I didn’t introduce a new state as you suggested, since iOS also doesn’t have that state. The empty rates case will now be tracked with the message “No available rates”.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good, thanks for double checking with the iOS implementation 👍🏼

R.string.woo_shipping_labels_package_creation_shipping_rates_loading_error
}

updateState(ShippingRatesState.Error(errorMessage))
}
)
selectedRatesFlow.value = selectedRatesFlow.value.toMutableList().apply { set(index, null) }
Expand Down Expand Up @@ -1228,7 +1235,7 @@ class WooShippingLabelCreationViewModel @Inject constructor(
val missingDescription: Int
) : ShippingRatesState()

data object Error : ShippingRatesState()
data class Error(@StringRes val message: Int) : ShippingRatesState()

data class Loading(
val selectedRatesSortOrder: ShippingSortOption
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import androidx.compose.foundation.background
import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.material.MaterialTheme
import androidx.compose.material.Text
Expand All @@ -30,7 +31,8 @@ fun ErrorMessageWithButton(
Column(
modifier = modifier
.fillMaxSize()
.background(MaterialTheme.colors.surface),
.background(MaterialTheme.colors.surface)
.padding(16.dp),
horizontalAlignment = Alignment.CenterHorizontally,
verticalArrangement = Arrangement.spacedBy(24.dp, Alignment.CenterVertically)
) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
package com.woocommerce.android.ui.orders.wooshippinglabels.rates.domain

import androidx.annotation.StringRes
import com.woocommerce.android.R
import com.woocommerce.android.model.Address
import com.woocommerce.android.ui.orders.shippinglabels.creation.ShippingLabelHazmatCategory
import com.woocommerce.android.ui.orders.wooshippinglabels.customs.CustomsData
Expand Down Expand Up @@ -36,7 +38,13 @@ class GetShippingRates @Inject constructor(
).fold(
onSuccess = { ratesResponse ->
if (ratesResponse.isEmpty()) {
Result.failure(Exception("no_rates_available"))
val message = if (hazmatSelection == null) {
R.string.woo_shipping_labels_package_creation_shipping_rates_empty
} else {
R.string.woo_shipping_labels_package_creation_shipping_rates_empty_with_hazmat
}

Result.failure(NoAvailableRatesException(message))
} else {
try {
val mappedRates = shippingMapper(ratesResponse, currencyCode)
Expand All @@ -51,3 +59,5 @@ class GetShippingRates @Inject constructor(
}
)
}

class NoAvailableRatesException(@StringRes val messageResId: Int) : Exception()
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import androidx.compose.foundation.layout.sizeIn
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.unit.dp
import com.woocommerce.android.R
import com.woocommerce.android.ui.orders.wooshippinglabels.WooShippingLabelCreationViewModel
import com.woocommerce.android.ui.orders.wooshippinglabels.packages.components.ErrorMessageWithButton

Expand All @@ -32,8 +31,8 @@ internal fun ShippingRatesSection(
)
}

WooShippingLabelCreationViewModel.ShippingRatesState.Error -> ErrorMessageWithButton(
message = R.string.woo_shipping_labels_package_creation_shipping_rates_loading_error,
is WooShippingLabelCreationViewModel.ShippingRatesState.Error -> ErrorMessageWithButton(
message = shippingRatesState.message,
modifier = Modifier.sizeIn(minHeight = 300.dp),
onRetryClick = { onRefreshShippingRates() }
)
Expand Down
2 changes: 2 additions & 0 deletions WooCommerce/src/main/res/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4601,6 +4601,8 @@
<string name="woo_shipping_labels_package_creation_carrier_loading_error">We are unable to load carrier packages.</string>
<string name="woo_shipping_labels_package_creation_saved_loading_error">We are unable to load saved packages.</string>
<string name="woo_shipping_labels_package_creation_shipping_rates_loading_error">We are unable to load shipping rates.</string>
<string name="woo_shipping_labels_package_creation_shipping_rates_empty">We couldn\'t find a shipping service for the combination of the selected package and the total shipment weight. Please adjust your input and try again.</string>
<string name="woo_shipping_labels_package_creation_shipping_rates_empty_with_hazmat">We couldn\'t find a shipping service for the combination of the selected HAZMAT category, the selected package and the total shipment weight. Please adjust your input and try again.</string>
<string name="woo_shipping_labels_package_creation_starred">Star this item</string>
<string name="woo_shipping_labels_package_creation_unstarred">Unstar this item</string>
<string name="woo_shipping_labels_shipping_rates_missing_destination">Add a destination address to get shipping rates</string>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -130,5 +130,6 @@ class GetShippingRatesTest : BaseUnitTest() {
)

assertTrue(result.isFailure)
assertTrue(result.exceptionOrNull() is NoAvailableRatesException)
}
}