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

hafizrahman · 2025-01-09T12:26:46Z

Description

The API call for bulk order status update returns values for both successful and failed order updates, which means a call can either be successful, partially successful, or different form of failures.

Previously, the app simply treats any result as success as long as there's at least 1 successful Order update (and informs merchants such). Partial success/failure is logged in application log but not informed to user.

This PR improves the success handling by creating specific types based on the result, each having its own snackbar message:

Type	Message to merchant
PartialSuccess	"X order(s) updated, and Y order(s) failed to update. Please try again."
AllSuccess	"Status updated!"
AllFailed	"Failed to update orders. Please try again"
NoOrdersUpdated	"No orders updated. Please try again."
Error	"An error occured"

Steps to reproduce

I'd recommend using API Faker to test the different cases:

Go to Settings > Developer Options > API Faker
Add new endpoint:
- Type: WordPress REST API
- HTTP Method: POST
- Path: /wc/v3/orders/batch
- Query parameters: %
- Response: Status code 200, then for the Body value use the example values shared below.

TC 1: Partial success:

Copy response example from here and paste to API faker body, save it,
Go to Orders List, long press to select and Order, then tap triple dot and Update Status,
Update to any status (doesn't matter since we're overriding the response with API faker),
Ensure the same message is shown as in the table above.

TC 2: Full success:

Copy response example from here and paste to API faker body, save it,
Go to Orders List, long press to select and Order, then tap triple dot and Update Status,
Update to any status (doesn't matter since we're overriding the response with API faker),
Ensure the same message is shown as in the table above.

TC 3: All failed:

Copy response example from here and paste to API faker body, save it,
Go to Orders List, long press to select and Order, then tap triple dot and Update Status,
Update to any status (doesn't matter since we're overriding the response with API faker),
Ensure the same message is shown as in the table above.

TC 4: No Orders Updated:

Copy response example from here and paste to API faker body, save it,
Go to Orders List, long press to select and Order, then tap triple dot and Update Status,
Update to any status (doesn't matter since we're overriding the response with API faker),
Ensure the same message is shown as in the table above.

TC 5: Error:

Update API Faker to return 404 as status code, and save it.
Go to Orders List, long press to select and Order, then tap triple dot and Update Status,
Update to any status (doesn't matter since we're overriding the response with API faker),
Ensure the same message is shown as in the table above.

Testing information

The tests that have been performed

I have tested all five TCs using API Faker as above.

Images/gif

n/a

I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

Reviewer (or Author, in the case of optional code reviews):

Please make sure these conditions are met before approving the PR, or request changes if the PR needs improvement:

The PR is small and has a clear, single focus, or a valid explanation is provided in the description. If needed, please request to split it into smaller PRs.
Ensure Adequate Unit Test Coverage: The changes are reasonably covered by unit tests or an explanation is provided in the PR description.
Manual Testing: The author listed all the tests they ran, including smoke tests when needed (e.g., for refactorings). The reviewer confirmed that the PR works as expected on big (tablet) and small (phone) in case of UI changes, and no regressions are added.

wpmobilebot · 2025-01-09T12:31:48Z

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce-Wear Android
	Platform	⌚️ Wear OS
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`1ab82d5`
	Direct Download	`woocommerce-wear-prototype-build-pr13275-1ab82d5.apk`

wpmobilebot · 2025-01-09T12:45:11Z

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce Android
	Platform	📱 Mobile
	Flavor	Jalapeno
	Build Type	Debug
	Commit	`1ab82d5`
	Direct Download	`woocommerce-prototype-build-pr13275-1ab82d5.apk`

codecov-commenter · 2025-01-09T12:52:54Z

Codecov Report

Attention: Patch coverage is 68.51852% with 17 lines in your changes missing coverage. Please review.

Project coverage is 40.81%. Comparing base (c2c1e55) to head (1ab82d5).
Report is 32 commits behind head on trunk.

Files with missing lines	Patch %	Lines
...erce/android/ui/orders/list/OrderListRepository.kt	0.00%	11 Missing ⚠️
...merce/android/ui/orders/list/OrderListViewModel.kt	82.85%	3 Missing and 3 partials ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##              trunk   #13275      +/-   ##
============================================
+ Coverage     40.79%   40.81%   +0.01%     
- Complexity     6411     6420       +9     
============================================
  Files          1353     1354       +1     
  Lines         77678    77713      +35     
  Branches      10687    10696       +9     
============================================
+ Hits          31689    31718      +29     
- Misses        43182    43187       +5     
- Partials       2807     2808       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hafizrahman · 2025-01-10T14:05:32Z

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

+    }
+
+    @Test
+    fun `when bulk update fully succeeds, then refresh and exit selection mode`() = testBlocking {


❓ I'd love to be able to check Event and message here also, but the string and event happens separately when observing pagedListWrapper.data here https://github.com/woocommerce/woocommerce-android/pull/13275/files#diff-daf78e7103c7d0e51140c59b8490193f96db648214827d13e0d3f66c42c576d9R485 and I can't figure out exactly how to mock that. Suggestions welcome.

You can simulate this with something like this patch:

Index: WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt (revision 1ab82d50733c4e2425daa593dec1d60a5ef4aa67) +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt (date 1736932809675) @@ -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 @@ -1126,6 +1127,8 @@ 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() @@ -1134,9 +1137,14 @@ // When viewModel.onBulkOrderStatusChanged(listOf(1L, 2L), Order.Status.Completed) + 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()

We here force sending a different instance of PagedList by the second mock() call, which triggers the SnackBar.

hichamboushaba

Sorry for the late review @hafizrahman, this slipped my mind.

Nice work here, it works well, I left some comments and tried to answer your question, but nothing blocker 👏

hichamboushaba · 2025-01-15T09:08:18Z

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

+        when (result) {
+            is BulkUpdateOrderResult.AllSuccess,
+            is BulkUpdateOrderResult.PartialSuccess -> {
+                isQueueingBulkUpdateSuccessMessage = true


np, I understand why you are using an additional class variable for this, but personally I try to avoid them when it's possible, because they make the code more spread across different areas and hard to follow, and also they are not easy to maintain.
I have a suggestion on how to achieve what you want here without using them, you can check it by applying this patch:

Index: WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListViewModel.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListViewModel.kt b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListViewModel.kt --- a/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListViewModel.kt (revision 1ab82d50733c4e2425daa593dec1d60a5ef4aa67) +++ b/WooCommerce/src/main/kotlin/com/woocommerce/android/ui/orders/list/OrderListViewModel.kt (date 1736931926138) @@ -11,6 +11,7 @@ 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.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 @@ -229,9 +231,6 @@ fun isSelecting() = viewState.orderListState == ViewState.OrderListState.Selecting - private var isQueueingBulkUpdateSuccessMessage = false - private var bulkUpdateSuccessMessage = "" - init { lifecycleRegistry.currentState = Lifecycle.State.CREATED lifecycleRegistry.currentState = Lifecycle.State.STARTED @@ -481,14 +480,6 @@ _isLoadingMore.value = it } - // Observe status changes in the data - pagedListWrapper.data.distinct().observe(this) { - if (isQueueingBulkUpdateSuccessMessage) { - isQueueingBulkUpdateSuccessMessage = false - triggerEvent(OrderListEvent.ShowSnackbarString(bulkUpdateSuccessMessage)) - } - } - pagedListWrapper.listError .filter { !dismissListErrors } .filterNotNull() @@ -1017,20 +1008,28 @@ when (result) { is BulkUpdateOrderResult.AllSuccess, is BulkUpdateOrderResult.PartialSuccess -> { - isQueueingBulkUpdateSuccessMessage = true - bulkUpdateSuccessMessage = when (result) { - is BulkUpdateOrderResult.AllSuccess -> resourceProvider.getString( - R.string.orderlist_bulk_update_status_updated - ) + // 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 - ) + 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) - } + else -> resourceProvider.getString(R.string.orderlist_bulk_update_status_updated) + } + triggerEvent(OrderListEvent.ShowSnackbarString(message)) + observable.removeObserver(this) + } + }) + ordersPagedListWrapper?.fetchFirstPage() trackBulkOrderUpdateSuccess() }

Applying this is not required, I just wanted to share it with you to see how we can add a pending job to be executed just once.

hichamboushaba · 2025-01-15T09:24:42Z

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

+    }
+
+    @Test
+    fun `when bulk update fully succeeds, then refresh and exit selection mode`() = testBlocking {


You can simulate this with something like this patch:

Index: WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt --- a/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt (revision 1ab82d50733c4e2425daa593dec1d60a5ef4aa67) +++ b/WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt (date 1736932809675) @@ -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 @@ -1126,6 +1127,8 @@ 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() @@ -1134,9 +1137,14 @@ // When viewModel.onBulkOrderStatusChanged(listOf(1L, 2L), Order.Status.Completed) + 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()

We here force sending a different instance of PagedList by the second mock() call, which triggers the SnackBar.

hichamboushaba · 2025-01-15T09:26:13Z

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

+
+        // First enter selection mode
+        viewModel.onSelectionChanged(1)
+        assertThat(viewModel.isSelecting()).isTrue()


np, just a remark about these assertions you have here as part of the Given part, I don't think they are needed because you already have a test case specific for the selection mode, and generally we assert as part of the Then section, so they feel out of place.

hafizrahman added 3 commits January 9, 2025 18:17

Refactor result from bulk update to handled various cases.

1e6eb5c

Update unit tests

f01116e

Add full success and partial success unit tests.

f30b618

hafizrahman added this to the 21.4 milestone Jan 9, 2025

hafizrahman modified the milestones: 21.4, 21.5 Jan 10, 2025

hafizrahman added type: enhancement A request for an enhancement. feature: order list Related to the order list. labels Jan 10, 2025

hafizrahman added 2 commits January 10, 2025 13:52

Update logging for better clarity and separation with subsequent update.

01286fd

Reword message to not wrap number inside parenthesis.

1ab82d5

hafizrahman commented Jan 10, 2025

View reviewed changes

hafizrahman marked this pull request as ready for review January 10, 2025 14:05

hafizrahman requested a review from hichamboushaba January 10, 2025 14:05

hichamboushaba self-assigned this Jan 10, 2025

hichamboushaba approved these changes Jan 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

hafizrahman commented Jan 9, 2025 •

edited

Loading

wpmobilebot commented Jan 9, 2025 •

edited

Loading

wpmobilebot commented Jan 9, 2025 •

edited

Loading

codecov-commenter commented Jan 9, 2025 •

edited

Loading

hafizrahman Jan 10, 2025

hichamboushaba Jan 15, 2025

hichamboushaba left a comment

hichamboushaba Jan 15, 2025

hichamboushaba Jan 15, 2025

hichamboushaba Jan 15, 2025

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

Are you sure you want to change the base?

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

Conversation

hafizrahman commented Jan 9, 2025 • edited Loading

Description

Steps to reproduce

TC 1: Partial success:

TC 2: Full success:

TC 3: All failed:

TC 4: No Orders Updated:

TC 5: Error:

Testing information

The tests that have been performed

Images/gif

Reviewer (or Author, in the case of optional code reviews):

wpmobilebot commented Jan 9, 2025 • edited Loading

wpmobilebot commented Jan 9, 2025 • edited Loading

codecov-commenter commented Jan 9, 2025 • edited Loading

Codecov Report

hafizrahman Jan 10, 2025

Choose a reason for hiding this comment

hichamboushaba Jan 15, 2025

Choose a reason for hiding this comment

hichamboushaba left a comment

Choose a reason for hiding this comment

hichamboushaba Jan 15, 2025

Choose a reason for hiding this comment

hichamboushaba Jan 15, 2025

Choose a reason for hiding this comment

hichamboushaba Jan 15, 2025

Choose a reason for hiding this comment

hafizrahman commented Jan 9, 2025 •

edited

Loading

wpmobilebot commented Jan 9, 2025 •

edited

Loading

wpmobilebot commented Jan 9, 2025 •

edited

Loading

codecov-commenter commented Jan 9, 2025 •

edited

Loading