-
Notifications
You must be signed in to change notification settings - Fork 136
[WOOMOB-1469] Booking Customer Filter #14834
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[WOOMOB-1469] Booking Customer Filter #14834
Conversation
Generated by 🚫 Danger |
📲 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 Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## trunk #14834 +/- ##
============================================
- Coverage 38.27% 38.26% -0.01%
Complexity 10117 10117
============================================
Files 2140 2142 +2
Lines 121101 121124 +23
Branches 16599 16603 +4
============================================
+ Hits 46346 46347 +1
- Misses 70046 70067 +21
- Partials 4709 4710 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fde7bb4 to
27c1262
Compare
27c1262 to
8a548c9
Compare
8a548c9 to
42df91a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the customer selection functionality to be reusable and implements a customer filter for bookings. The changes rename components from order-specific names to generic customer selection names and extract the exit logic to enable different behaviors (navigation vs. callbacks).
Key Changes:
- Renamed
OrderCustomerListScreentoCustomerListSelectionScreenfor broader reusability - Made
CustomerListSelectionViewModelopen and extractedexitWithCustomermethod to support customization - Implemented booking customer filter using the refactored customer selection components
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| strings.xml | Shortened string resource from bookings_filter_customer_name to bookings_filter_customer |
| OrderCustomerListFragment.kt | Updated to use renamed CustomerListSelectionScreen |
| CustomerListSelectionViewModel.kt | Made class open and extracted exitWithCustomer method for customization |
| CustomerListSelectionScreen.kt | Renamed composable and added showToolbar parameter for flexible UI |
| CustomerListDialogFragment.kt | Updated to use renamed CustomerListSelectionScreen |
| BookingCustomerFilterViewModel.kt | New ViewModel extending CustomerListSelectionViewModel with callback-based exit |
| BookingCustomerFilterPage.kt | New composable page for booking customer filter |
| BookingFilterListUiState.kt | Updated to use new string resource and added missing trailing comma |
| BookingFilterListScreen.kt | Implemented customer filter page with customer selection logic |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| state.onUpdateFilterOption( | ||
| BookingsFilterOption.Customer( | ||
| customerId = customer.customerId ?: 0L, | ||
| customerName = "${customer.firstName} ${customer.lastName}".trim() | ||
| .ifBlank { customer.email }.orEmpty() | ||
| ) | ||
| ) |
Copilot
AI
Nov 5, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using a fallback of 0L for a null customerId could lead to incorrect filtering behavior if 0 is a valid customer ID in the system. Consider handling the null case explicitly by either preventing selection when customerId is null, or documenting that 0 represents 'no customer'.
| state.onUpdateFilterOption( | |
| BookingsFilterOption.Customer( | |
| customerId = customer.customerId ?: 0L, | |
| customerName = "${customer.firstName} ${customer.lastName}".trim() | |
| .ifBlank { customer.email }.orEmpty() | |
| ) | |
| ) | |
| customer.customerId?.let { id -> | |
| state.onUpdateFilterOption( | |
| BookingsFilterOption.Customer( | |
| customerId = id, | |
| customerName = "${customer.firstName} ${customer.lastName}".trim() | |
| .ifBlank { customer.email }.orEmpty() | |
| ) | |
| ) | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AdamGrzybkowski, I think this is a good suggestion. We can commit Copilot’s code suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and no, although technically that's possible because the model has a nullable ID, when you look at the code, that should never happen. I will commit it since it won't hurt us.
...mmerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/BookingFilterListScreen.kt
Outdated
Show resolved
Hide resolved
irfano
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! We can add additional features, such as multiple selection, in a separate PR.
WOOMOB-1469
Description
Since Hicham did most of the work, I'm opening this PR to have at least some sort of Customer filter ready until we create new UI for the Bookings with multi-select option.
Test Steps
FiltersCustomerShow bookingsImages/gif
Screen_recording_20251105_134834.mp4
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.