[Bulk Update Orders] Limit selection to maximum of 100 Orders at once #13219
Conversation
hafizrahman
changed the base branch from
trunk
to
task/13140-connect-bulk-update-func
| var selectMultiple = true | ||
| var currentSelectionCount = 0 |
There was a problem hiding this comment.
This is needed because the predicate does not have access to the tracker itself, thus can't get the latest selection count unless it is passed to it.
📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
|
|
📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #13219 +/- ##
=========================================
Coverage 40.60% 40.60%
- Complexity 6373 6375 +2
=========================================
Files 1345 1345
Lines 77330 77343 +13
Branches 10618 10620 +2
=========================================
+ Hits 31397 31407 +10
- Misses 43171 43172 +1
- Partials 2762 2764 +2 ☔ View full report in Codecov by Sentry. |
| val limit = maxSelectionCount ?: return true | ||
| return !(nextState && currentSelectionCount >= limit) |
There was a problem hiding this comment.
The official documentation recommends to set the limit logic here:
We could theoretically manage selection state better in the ViewModel, controlling what can be selected/deselected and passing this to the selection tracker. We'd need to track selection and deselection eligibility and maintain selection state, then pass both to the predicate, but this feels like a complex solution for what's ultimately an edge case.
Since bulk updating 100 items on mobile feels like a rare action given the limited screen space, this current approach where we have limits at both UI and API layers feels sufficient enough to me without spending too much time on it.
There was a problem hiding this comment.
Thanks for explaining this! 👍🏻 I think this is a better and cleaner solution.
WooCommerce/src/test/kotlin/com/woocommerce/android/ui/orders/OrderListViewModelTest.kt
Outdated
Show resolved
Hide resolved
| val limit = maxSelectionCount ?: return true | ||
| return !(nextState && currentSelectionCount >= limit) |
There was a problem hiding this comment.
Thanks for explaining this! 👍🏻 I think this is a better and cleaner solution.
Closes: #13217
Description
The API for bulk updating Orders only support doing 100 Orders at once, and we want to add limiter on the UI side as well.
Known issue: When selecting Orders, then scrolling down to load more Orders, then the selection state gets removed. Thus truly selecting 100 items at once will not be possible in this PR. I recommend setting a smaller limit temporarily for testing purposes. More below. (The issue itself is already fixed in this PR)
Steps to reproduce
OrderListViewModeland updating the value ofBULK_UPDATE_COUNT_LIMITto something reasonable like 2 or 3 then build the app.The tests that have been performed
I tested the steps above, see video below:
Images/gif
(The test below was done with the limit set to 2, to keep things easy)
Screen.Recording.2024-12-30.at.16.33.19.mov
RELEASE-NOTES.txtif 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: