Merge pull request #13275 from woocommerce/task/add-partial-success-f…

…or-bulk-update-order

[Bulk Update Orders] Better handling for partial success and other results from updating

RELEASE-NOTES.txt

@@ -8,6 +8,7 @@
 - [*] Puerto Rico is now available in the list of countries that are supported by in-person payments [https://github.com/woocommerce/woocommerce-android/pull/13200]
 - [Internal] [*] XMLRPC is not needed now for Jetpack Connection during an Account Mismatch flow [https://github.com/woocommerce/woocommerce-android/pull/13308]
 - [*] Removed the outdated feedback survey for shipping labels [https://github.com/woocommerce/woocommerce-android/pull/13319]
+- [*] Bulk update order status now shows improved result messages, including for partial success. [https://github.com/woocommerce/woocommerce-android/pull/13275]
 
 21.4
 -----

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/BulkUpdateOrderResult.kt

@@ -0,0 +1,16 @@
+package com.woocommerce.android.ui.orders.list
+
+sealed class BulkUpdateOrderResult {
+    data class PartialSuccess(
+        val successCount: Int,
+        val failureCount: Int
+    ) : BulkUpdateOrderResult()
+
+    data object AllSuccess : BulkUpdateOrderResult()
+
+    data object AllFailed : BulkUpdateOrderResult()
+
+    data object NoOrdersUpdated : BulkUpdateOrderResult()
+
+    data class Error(val exception: Exception) : BulkUpdateOrderResult()
+}

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListRepository.kt

@@ -40,8 +40,6 @@ class OrderListRepository @Inject constructor(
     companion object {
         private const val TAG = "OrderListRepository"
         private const val ORDER_STATUS_TRASH = "trash"
-        private const val BULK_UPDATE_ORDER_STATUS_ALL_FAILED = "Unable to update any orders."
-        private const val BULK_UPDATE_ORDER_NOTHING_UPDATED = "No orders were updated."
         private const val BULK_UPDATE_ORDER_NO_RESPONSE = "No response received."
     }
 
@@ -181,7 +179,7 @@ class OrderListRepository @Inject constructor(
         }
     }
 
-    suspend fun bulkUpdateOrderStatus(orderIds: List<Long>, newStatus: Order.Status): Result<Unit> {
+    suspend fun bulkUpdateOrderStatus(orderIds: List<Long>, newStatus: Order.Status): BulkUpdateOrderResult {
         val result = orderStore.batchUpdateOrdersStatus(
             site = selectedSite.get(),
             orderIds = orderIds,
@@ -190,29 +188,28 @@ class OrderListRepository @Inject constructor(
 
         return if (result.isError) {
             WooLog.e(ORDERS, "Error bulk updating order status: ${result.error.message}")
-            Result.failure(WooException(result.error))
+            BulkUpdateOrderResult.Error(WooException(result.error))
         } else {
             result.model?.let {
                 logBulkOrderUpdateResults(it)
 
-                // We want to return success if at least one order was updated.
-                // However, if:
-                // - there's no updated orders but there are failed orders, return failure
-                // - there's no updated orders and no failed orders, return failure
                 when {
-                    it.failedOrders.isNotEmpty() && it.updatedOrders.isEmpty() ->
-                        Result.failure(Exception(BULK_UPDATE_ORDER_STATUS_ALL_FAILED))
-
-                    it.failedOrders.isEmpty() && it.updatedOrders.isEmpty() ->
-                        Result.failure(Exception(BULK_UPDATE_ORDER_NOTHING_UPDATED))
-
-                    else -> Result.success(Unit)
+                    it.failedOrders.isNotEmpty() && it.updatedOrders.isEmpty() -> BulkUpdateOrderResult.AllFailed
+                    it.failedOrders.isEmpty() && it.updatedOrders.isEmpty() -> BulkUpdateOrderResult.NoOrdersUpdated
+                    it.failedOrders.isNotEmpty() && it.updatedOrders.isNotEmpty() ->
+                        BulkUpdateOrderResult.PartialSuccess(
+                            successCount = it.updatedOrders.size,
+                            failureCount = it.failedOrders.size
+                        )
+
+                    else -> BulkUpdateOrderResult.AllSuccess
                 }
-            } ?: Result.failure(Exception(BULK_UPDATE_ORDER_NO_RESPONSE))
+            } ?: BulkUpdateOrderResult.Error(Exception(BULK_UPDATE_ORDER_NO_RESPONSE))
         }
     }
 
     private fun logBulkOrderUpdateResults(model: WCOrderStore.UpdateOrdersStatusResult) {
+        WooLog.i(ORDERS, "Bulk update order status completed.")
         if (model.updatedOrders.isNotEmpty()) {
             WooLog.i(ORDERS, "Successfully updated ${model.updatedOrders.size} orders")
         }

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListViewModel.kt

@@ -11,6 +11,7 @@ import androidx.lifecycle.LifecycleRegistry
 import androidx.lifecycle.LiveData
 import androidx.lifecycle.MediatorLiveData
 import androidx.lifecycle.MutableLiveData
+import androidx.lifecycle.Observer
 import androidx.lifecycle.SavedStateHandle
 import androidx.lifecycle.asLiveData
 import androidx.lifecycle.viewModelScope
@@ -31,6 +32,7 @@ import com.woocommerce.android.analytics.IsScreenLargerThanCompactValue
 import com.woocommerce.android.analytics.deviceTypeToAnalyticsString
 import com.woocommerce.android.extensions.NotificationReceivedEvent
 import com.woocommerce.android.extensions.WindowSizeClass
+import com.woocommerce.android.extensions.drop
 import com.woocommerce.android.extensions.filter
 import com.woocommerce.android.extensions.filterNotNull
 import com.woocommerce.android.extensions.runWithContext
@@ -87,7 +89,6 @@ import org.wordpress.android.fluxc.store.ListStore.ListErrorType.PARSE_ERROR
 import org.wordpress.android.fluxc.store.ListStore.ListErrorType.TIMEOUT_ERROR
 import org.wordpress.android.fluxc.store.WCOrderStore
 import org.wordpress.android.fluxc.store.WCOrderStore.OnOrderSummariesFetched
-import org.wordpress.android.mediapicker.util.distinct
 import java.util.Locale
 import javax.inject.Inject
 import kotlin.time.Duration.Companion.seconds
@@ -229,8 +230,6 @@ class OrderListViewModel @Inject constructor(
 
     fun isSelecting() = viewState.orderListState == ViewState.OrderListState.Selecting
 
-    private var isQueueingBulkUpdateSuccessMessage = false
-
     init {
         lifecycleRegistry.currentState = Lifecycle.State.CREATED
         lifecycleRegistry.currentState = Lifecycle.State.STARTED
@@ -480,14 +479,6 @@ class OrderListViewModel @Inject constructor(
             _isLoadingMore.value = it
         }
 
-        // Observe status changes in the data
-        pagedListWrapper.data.distinct().observe(this) {
-            if (isQueueingBulkUpdateSuccessMessage) {
-                isQueueingBulkUpdateSuccessMessage = false
-                triggerEvent(Event.ShowSnackbar(R.string.orderlist_bulk_update_status_updated))
-            }
-        }
-
         pagedListWrapper.listError
             .filter { !dismissListErrors }
             .filterNotNull()
@@ -923,6 +914,7 @@ class OrderListViewModel @Inject constructor(
                 viewState = viewState.copy(selectionCount = count)
                 showMaximumBulkSelectionNotice()
             }
+
             count > 0 && !isSelecting() -> enterSelectionMode(count)
             count > 0 -> viewState = viewState.copy(selectionCount = count)
         }
@@ -1002,21 +994,7 @@ class OrderListViewModel @Inject constructor(
                         newStatus = newStatus
                     )
 
-                    if (result.isFailure) {
-                        viewState = viewState.copy(isBulkUpdating = false)
-                        trackBulkOrderUpdateFailure()
-                        triggerEvent(Event.ShowSnackbar(R.string.error_generic))
-                    } else {
-                        isQueueingBulkUpdateSuccessMessage = true
-                        ordersPagedListWrapper?.fetchFirstPage()
-
-                        analyticsTracker.track(
-                            AnalyticsEvent.ORDERS_LIST_BULK_UPDATE_SUCCESS,
-                            mapOf(
-                                AnalyticsTracker.KEY_PROPERTY to AnalyticsTracker.VALUE_STATUS,
-                            )
-                        )
-                    }
+                    handleBulkUpdateResult(result)
                 }
             }
         } else {
@@ -1026,6 +1004,66 @@ class OrderListViewModel @Inject constructor(
         exitSelectionMode()
     }
 
+    private fun handleBulkUpdateResult(result: BulkUpdateOrderResult) {
+        when (result) {
+            is BulkUpdateOrderResult.AllSuccess,
+            is BulkUpdateOrderResult.PartialSuccess -> {
+                // Prepare to show a success message after the list has been updated
+                val observable = ordersPagedListWrapper?.data?.drop(1)
+                observable?.observe(
+                    this,
+                    object : Observer<PagedOrdersList> {
+                        override fun onChanged(value: PagedOrdersList) {
+                            val message = when (result) {
+                                is BulkUpdateOrderResult.AllSuccess -> resourceProvider.getString(
+                                    R.string.orderlist_bulk_update_status_updated
+                                )
+
+                                is BulkUpdateOrderResult.PartialSuccess -> resourceProvider.getString(
+                                    R.string.orderlist_bulk_update_result_partial_success,
+                                    result.successCount,
+                                    result.failureCount
+                                )
+
+                                else -> resourceProvider.getString(R.string.orderlist_bulk_update_status_updated)
+                            }
+                            triggerEvent(OrderListEvent.ShowSnackbarString(message))
+                            observable.removeObserver(this)
+                        }
+                    }
+                )
+
+                ordersPagedListWrapper?.fetchFirstPage()
+                trackBulkOrderUpdateSuccess()
+            }
+
+            is BulkUpdateOrderResult.NoOrdersUpdated,
+            is BulkUpdateOrderResult.AllFailed,
+            is BulkUpdateOrderResult.Error -> {
+                viewState = viewState.copy(isBulkUpdating = false)
+                trackBulkOrderUpdateFailure()
+                val messageRes = when (result) {
+                    is BulkUpdateOrderResult.NoOrdersUpdated ->
+                        R.string.orderlist_bulk_update_result_no_orders_updated
+
+                    is BulkUpdateOrderResult.AllFailed -> R.string.orderlist_bulk_update_result_all_failed
+                    is BulkUpdateOrderResult.Error -> R.string.error_generic
+                    else -> R.string.error_generic
+                }
+                triggerEvent(Event.ShowSnackbar(messageRes))
+            }
+        }
+    }
+
+    private fun trackBulkOrderUpdateSuccess() {
+        analyticsTracker.track(
+            AnalyticsEvent.ORDERS_LIST_BULK_UPDATE_SUCCESS,
+            mapOf(
+                AnalyticsTracker.KEY_PROPERTY to AnalyticsTracker.VALUE_STATUS,
+            )
+        )
+    }
+
     private fun trackBulkOrderUpdateFailure() {
         analyticsTracker.track(
             AnalyticsEvent.ORDERS_LIST_BULK_UPDATE_FAILURE,

WooCommerce/src/main/res/values/strings.xml

@@ -619,6 +619,9 @@
     <string name="orderlist_selection_menu_update_status">Update status</string>
     <string name="orderlist_bulk_update_status_updated">Status updated!</string>
     <string name="orderlist_bulk_update_maximum_reached">Maximum selection count (%d) is reached.</string>
+    <string name="orderlist_bulk_update_result_no_orders_updated">No orders updated. Please try again.</string>
+    <string name="orderlist_bulk_update_result_all_failed">Failed to update orders. Please try again.</string>
+    <string name="orderlist_bulk_update_result_partial_success">%d order(s) updated, and %d order(s) failed to update. Please try again.</string>
 
     <!--
          Simple Payments

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt

@@ -1,6 +1,7 @@
 package com.woocommerce.android.ui.orders
 
 import androidx.lifecycle.MutableLiveData
+import androidx.paging.PagedList
 import com.google.android.material.snackbar.Snackbar
 import com.woocommerce.android.AppPrefsWrapper
 import com.woocommerce.android.FeedbackPrefs
@@ -29,6 +30,7 @@ import com.woocommerce.android.ui.orders.filters.domain.GetSelectedOrderFiltersC
 import com.woocommerce.android.ui.orders.filters.domain.GetWCOrderListDescriptorWithFilters
 import com.woocommerce.android.ui.orders.filters.domain.GetWCOrderListDescriptorWithFiltersAndSearchQuery
 import com.woocommerce.android.ui.orders.filters.domain.ShouldShowCreateTestOrderScreen
+import com.woocommerce.android.ui.orders.list.BulkUpdateOrderResult
 import com.woocommerce.android.ui.orders.list.FetchOrdersRepository
 import com.woocommerce.android.ui.orders.list.ObserveOrdersListLastUpdate
 import com.woocommerce.android.ui.orders.list.OrderListFragmentArgs
@@ -1012,7 +1014,6 @@ class OrderListViewModelTest : BaseUnitTest() {
     fun `when selection count changes to 0, then exit selection mode`() = testBlocking {
         // First enter selection mode
         viewModel.onSelectionChanged(2)
-        assertThat(viewModel.isSelecting()).isTrue()
 
         // Then exit
         viewModel.onSelectionChanged(0)
@@ -1057,6 +1058,8 @@ class OrderListViewModelTest : BaseUnitTest() {
     fun `given offline, when bulk update status requested, then show offline error and exit selection mode`() = testBlocking {
         whenever(networkStatus.isConnected()).thenReturn(false)
 
+        // First enter selection mode
+        viewModel.onSelectionChanged(2)
         viewModel.onBulkOrderStatusChanged(listOf(1L, 2L), Order.Status.Completed)
 
         assertThat(viewModel.event.value).isInstanceOf(Event.ShowSnackbar::class.java)
@@ -1067,7 +1070,11 @@ class OrderListViewModelTest : BaseUnitTest() {
     @Test
     fun `when bulk update fails, then show error message and exit selection`() = testBlocking {
         whenever(networkStatus.isConnected()).thenReturn(true)
-        whenever(orderListRepository.bulkUpdateOrderStatus(any(), any())).thenReturn(Result.failure(Exception()))
+        whenever(orderListRepository.bulkUpdateOrderStatus(any(), any()))
+            .thenReturn(BulkUpdateOrderResult.Error(Exception()))
+
+        // First enter selection mode
+        viewModel.onSelectionChanged(1)
 
         viewModel.onBulkOrderStatusChanged(listOf(1L), Order.Status.Completed)
 
@@ -1077,17 +1084,88 @@ class OrderListViewModelTest : BaseUnitTest() {
     }
 
     @Test
-    fun `when bulk update completes successfully, then exit selection mode`() = testBlocking {
+    fun `when bulk update results in no orders updated, then show message and exit selection mode`() = testBlocking {
+        whenever(networkStatus.isConnected()).thenReturn(true)
+        whenever(orderListRepository.bulkUpdateOrderStatus(any(), any()))
+            .thenReturn(BulkUpdateOrderResult.NoOrdersUpdated)
+
+        viewModel.onSelectionChanged(1)
+
+        viewModel.onBulkOrderStatusChanged(listOf(1L), Order.Status.Completed)
+
+        assertThat(viewModel.event.value).isInstanceOf(Event.ShowSnackbar::class.java)
+        assertThat((viewModel.event.value as Event.ShowSnackbar).message)
+            .isEqualTo(R.string.orderlist_bulk_update_result_no_orders_updated)
+        assertThat(viewModel.isSelecting()).isFalse()
+    }
+
+    @Test
+    fun `when bulk update fails for all orders, then show message and exit selection mode`() = testBlocking {
         whenever(networkStatus.isConnected()).thenReturn(true)
-        whenever(orderListRepository.bulkUpdateOrderStatus(any(), any())).thenReturn(Result.success(Unit))
+        whenever(orderListRepository.bulkUpdateOrderStatus(any(), any()))
+            .thenReturn(BulkUpdateOrderResult.AllFailed)
 
         // First enter selection mode
         viewModel.onSelectionChanged(1)
-        assertThat(viewModel.isSelecting()).isTrue()
 
         viewModel.onBulkOrderStatusChanged(listOf(1L), Order.Status.Completed)
 
+        assertThat(viewModel.event.value).isInstanceOf(Event.ShowSnackbar::class.java)
+        assertThat((viewModel.event.value as Event.ShowSnackbar).message)
+            .isEqualTo(R.string.orderlist_bulk_update_result_all_failed)
+        assertThat(viewModel.isSelecting()).isFalse()
+    }
+
+    @Test
+    fun `when bulk update fully succeeds, then refresh and exit selection mode`() = testBlocking {
+        // Given
+        whenever(networkStatus.isConnected()).thenReturn(true)
+        whenever(orderListRepository.bulkUpdateOrderStatus(any(), any()))
+            .thenReturn(BulkUpdateOrderResult.AllSuccess)
+        val pagedListData = MutableLiveData<PagedList<OrderListItemUIType>>(mock())
+        whenever(pagedListWrapper.data).thenReturn(pagedListData)
+
+        // First load order to initialize orderPagedListWrapper, then enter selection mode
+        viewModel.loadOrders()
+        viewModel.onSelectionChanged(2)
+
+        // When
+        viewModel.onBulkOrderStatusChanged(listOf(1L, 2L), Order.Status.Completed)
+        // Sending a different instance of PagedList to trigger the Snackbar
+        pagedListData.value = mock()
+
+        // Then
+        assertThat(viewModel.isSelecting()).isFalse()
+        val expectedEvent = OrderListEvent.ShowSnackbarString(
+            resourceProvider.getString(R.string.orderlist_bulk_update_status_updated)
+        )
+        assertThat(viewModel.event.value).isEqualTo(expectedEvent)
+
+        // Invoked once during loadOrders() and once during onBulkOrderStatusChanged()
+        verify(viewModel.ordersPagedListWrapper, times(2))?.fetchFirstPage()
+    }
+
+    @Test
+    fun `when bulk update partially succeeds, then refresh and exit selection mode`() = testBlocking {
+        // Given
+        val successCount = 3
+        val failureCount = 2
+        whenever(networkStatus.isConnected()).thenReturn(true)
+        whenever(orderListRepository.bulkUpdateOrderStatus(any(), any()))
+            .thenReturn(BulkUpdateOrderResult.PartialSuccess(successCount, failureCount))
+
+        // First load order to initialize orderPagedListWrapper, then enter selection mode
+        viewModel.loadOrders()
+        viewModel.onSelectionChanged(5)
+
+        // When
+        viewModel.onBulkOrderStatusChanged(listOf(1L, 2L, 3L, 4L, 5L), Order.Status.Completed)
+
+        // Then
         assertThat(viewModel.isSelecting()).isFalse()
+
+        // Invoked once during loadOrders() and once during onBulkOrderStatusChanged()
+        verify(viewModel.ordersPagedListWrapper, times(2))?.fetchFirstPage()
     }
 
     @Test