[WOOMOB-2449] Fix attendance status filter to be single-select#15504
[WOOMOB-2449] Fix attendance status filter to be single-select#15504AdamGrzybkowski merged 4 commits intotrunkfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses WOOMOB-2449 by changing the Bookings “Attendance status” filter from multi-select to single-select to prevent invalid API requests (attendance_status must be a single string).
Changes:
- Updated attendance status UI state + ViewModel to track a single selected status (or “Any”).
- Adjusted filter persistence to read only one stored status for backward compatibility with legacy multi-select data.
- Updated REST query serialization to send a single
attendance_statusvalue.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsRestClient.kt | Serializes attendance_status as a single string instead of a comma-separated list. |
| WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/data/BookingFilterRepository.kt | Reads only one stored attendance status from DataStore for backward compatibility. |
| WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/attendancestatus/BookingAttendanceStatusFilterViewModel.kt | Enforces single-select selection behavior and emits a single-value filter set. |
| WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/attendancestatus/BookingAttendanceStatusFilterUiState.kt | Replaces selectedStatuses set with a single selectedStatus for UI rendering. |
| WooCommerce/src/test/kotlin/com/woocommerce/android/ui/bookings/filter/attendancestatus/BookingAttendanceStatusFilterViewModelTest.kt | Adds unit tests validating single-select behavior (including “Any” and no toggle-off). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...e/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/data/BookingFilterRepository.kt
Outdated
Show resolved
Hide resolved
...e/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/data/BookingFilterRepository.kt
Outdated
Show resolved
Hide resolved
...main/kotlin/org/wordpress/android/fluxc/network/rest/wpcom/wc/bookings/BookingsRestClient.kt
Outdated
Show resolved
Hide resolved
...mmerce/android/ui/bookings/filter/attendancestatus/BookingAttendanceStatusFilterViewModel.kt
Outdated
Show resolved
Hide resolved
|
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## trunk #15504 +/- ##
============================================
+ Coverage 39.54% 39.57% +0.02%
- Complexity 11237 11248 +11
============================================
Files 2251 2251
Lines 129761 129749 -12
Branches 18170 18175 +5
============================================
+ Hits 51316 51347 +31
+ Misses 73207 73162 -45
- Partials 5238 5240 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
hichamboushaba
left a comment
There was a problem hiding this comment.
Nice work @AdamGrzybkowski, I'm approving, I left a single comment, but it's not a blocker, you can ignore it if you don't agree.
| set( | ||
| "attendance_status", | ||
| attendanceStatuses.values.joinToString(FILTER_QUERY_PARAMETER_SEPERATOR) { it.key }) | ||
| attendanceStatuses.values.singleOrNull()?.let { |
There was a problem hiding this comment.
Note: I've decided to keep AttendanceStatuses since it's heavily used and in case we need to add multiselect again, it should be significanlty easier.
I believe it's unlikely we will have more attendance statuses than the two we currently have, as they are logically the only valid options. From a YAGNI perspective, I would also suggest changing attendanceStatuses to a single value in the networking layer to make the code easier to understand.
If, in the unlikely event, we do add more attendance statuses and the API supports multi-select options, we can reintroduce the logic. I think that would be a simple change.
There was a problem hiding this comment.
Yeah, I was 50/50 on that. Done in e7b0d80
fc3878b to
ff38406
Compare
The booking attendance status filter allowed multi-select, causing a 400 API error when multiple statuses were sent. Changed the UI state and ViewModel to single-select, updated the DataStore reader to only use the first stored value for backwards compatibility, and simplified the API serialization to send a plain string.
Address review: replace firstOrNull with singleOrNull so that legacy multi-value data is treated as unfiltered rather than picking an arbitrary status. Update existing repository test to save a single status.
Refactor the Set-based AttendanceStatuses wrapper to a single nullable AttendanceStatus value across networking, persistence, and UI layers. Since attendance status is mutually exclusive (attended/unattended), a Set adds unnecessary complexity. Simplifies DAO query from IN clause to nullable equality comparison and updates DataStore persistence with legacy migration support.
e7b0d80 to
bc0ab39
Compare

Description
Fixes WOOMOB-2449
The attendance status filter in the bookings list allowed multi-select, which caused a 400 API error when multiple statuses were sent (
attendance_status is not of type string). This PR makes the filter single-select so only one attendance status (or "Any") can be active at a time.Changes:
BookingAttendanceStatusFilterUiStateto hold a singleAttendanceStatus?instead of aSetBookingFilterRepositoryto read only the first stored value from DataStore (backwards compatible with previously stored multi-value data)BookingsRestClientto send a plain stringNote: I've decided to keep
AttendanceStatusessince it's heavily used and in case we need to add multiselect again, it should be significanlty easier.Test Steps
Images/gif
Screen_recording_20260310_110701.mp4
RELEASE-NOTES.txtif necessary. Use the "[Internal]" label for non-user-facing changes.