Skip to content

Commit e7b0d80

Browse files
AdamGrzybkowskiAdamGrzybkowski
committed
Simplify AttendanceStatuses to single-value AttendanceStatus
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.
1 parent ff38406 commit e7b0d80

File tree

12 files changed

+78
-120
lines changed

12 files changed

+78
-120
lines changed

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/BookingFilterListScreen.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,8 +162,8 @@ private fun FiltersNavHost(
162162
}
163163
composable(BookingFilterPage.AttendanceStatus.route) {
164164
BookingAttendanceStatusFilterRoute(
165-
initialAttendanceStatuses = state.updatedBookingFilters.attendanceStatuses
166-
) { attendanceStatuses -> state.onUpdateFilterOption(attendanceStatuses) }
165+
initialAttendanceStatus = state.updatedBookingFilters.attendanceStatus
166+
) { attendanceStatus -> state.onUpdateFilterOption(attendanceStatus) }
167167
}
168168
composable(BookingFilterPage.PaymentStatus.route) {
169169
}

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/BookingFilterListUiState.kt‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -69,13 +69,7 @@ data class BookingFilterListUiState(
6969
}
7070

7171
BookingFilterPage.AttendanceStatus -> {
72-
updatedBookingFilters.attendanceStatuses.values.takeIf { it.isNotEmpty() }?.let { list ->
73-
if (list.size > 1) {
74-
UiString.UiStringText(list.size.toString())
75-
} else {
76-
UiString.UiStringRes(list.first().titleRes)
77-
}
78-
}
72+
updatedBookingFilters.attendanceStatus.value?.let { UiString.UiStringRes(it.titleRes) }
7973
}
8074

8175
BookingFilterPage.Customer -> updatedBookingFilters.customer?.customerName?.let { name ->
@@ -151,7 +145,7 @@ fun BookingFilters.updateFilterOption(bookingsFilterOption: BookingsFilterOption
151145
is BookingsFilterOption.DateRange -> copy(dateRange = bookingsFilterOption)
152146
is BookingsFilterOption.Customer -> copy(customer = bookingsFilterOption)
153147
is BookingsFilterOption.TeamMembers -> copy(teamMembers = bookingsFilterOption)
154-
is BookingsFilterOption.AttendanceStatuses -> copy(attendanceStatuses = bookingsFilterOption)
148+
is BookingsFilterOption.AttendanceStatus -> copy(attendanceStatus = bookingsFilterOption)
155149
is BookingsFilterOption.PaymentStatus -> copy(paymentStatus = bookingsFilterOption)
156150
is BookingsFilterOption.BookingType -> copy(bookingType = bookingsFilterOption)
157151
is BookingsFilterOption.Location -> copy(location = bookingsFilterOption)

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/BookingFilterListViewModel.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ class BookingFilterListViewModel @Inject constructor(
151151
if (teamMembers != BookingsFilterOption.TeamMembers.DEFAULT) add("team_member")
152152
if (bookingType?.value != null) add("booking_type")
153153
if (serviceEvents != BookingsFilterOption.ServiceEvents.DEFAULT) add("service_events")
154-
if (attendanceStatuses != BookingsFilterOption.AttendanceStatuses.DEFAULT) add("attendance_status")
154+
if (attendanceStatus != BookingsFilterOption.AttendanceStatus.DEFAULT) add("attendance_status")
155155
if (paymentStatus != null) add("payment_status")
156156
if (customer != null) add("customer")
157157
if (dateRange != BookingsFilterOption.DateRange.DEFAULT) add("date_time")

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/attendancestatus/BookingAttendanceStatusFilterPage.kt‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,13 +9,13 @@ import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilter
99

1010
@Composable
1111
fun BookingAttendanceStatusFilterRoute(
12-
initialAttendanceStatuses: BookingsFilterOption.AttendanceStatuses?,
13-
onAttendanceStatusesFilterChanged: (BookingsFilterOption.AttendanceStatuses) -> Unit,
12+
initialAttendanceStatus: BookingsFilterOption.AttendanceStatus?,
13+
onAttendanceStatusFilterChanged: (BookingsFilterOption.AttendanceStatus) -> Unit,
1414
) {
1515
val viewModel =
1616
hiltViewModel<BookingAttendanceStatusFilterViewModel, BookingAttendanceStatusFilterViewModel.Factory>
1717
{ factory ->
18-
factory.create(initialAttendanceStatuses, onAttendanceStatusesFilterChanged)
18+
factory.create(initialAttendanceStatus, onAttendanceStatusFilterChanged)
1919
}
2020
val uiState by viewModel.uiState.observeAsState()
2121
uiState?.let { BookingAttendanceStatusFilterPage(it) }

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/attendancestatus/BookingAttendanceStatusFilterViewModel.kt‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,19 +10,19 @@ import dagger.assisted.AssistedInject
1010
import dagger.hilt.android.lifecycle.HiltViewModel
1111
import kotlinx.coroutines.flow.MutableStateFlow
1212
import kotlinx.coroutines.flow.update
13-
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption.AttendanceStatuses
13+
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption
1414
import org.wordpress.android.fluxc.persistence.entity.BookingEntity.AttendanceStatus
1515

1616
@HiltViewModel(assistedFactory = BookingAttendanceStatusFilterViewModel.Factory::class)
1717
class BookingAttendanceStatusFilterViewModel @AssistedInject constructor(
18-
@Assisted private val initialStatuses: AttendanceStatuses?,
19-
@Assisted private val onFilterChanged: (AttendanceStatuses) -> Unit,
18+
@Assisted private val initialStatus: BookingsFilterOption.AttendanceStatus?,
19+
@Assisted private val onFilterChanged: (BookingsFilterOption.AttendanceStatus) -> Unit,
2020
savedStateHandle: SavedStateHandle,
2121
) : ScopedViewModel(savedStateHandle) {
2222

2323
private val _uiState = MutableStateFlow(
2424
BookingAttendanceStatusFilterUiState(
25-
selectedStatus = initialStatuses?.values?.singleOrNull(),
25+
selectedStatus = initialStatus?.value,
2626
onStatusSelected = ::onStatusSelected
2727
)
2828
)
@@ -35,14 +35,14 @@ class BookingAttendanceStatusFilterViewModel @AssistedInject constructor(
3535
}
3636

3737
_uiState.update { it.copy(selectedStatus = newSelectedStatus) }
38-
onFilterChanged(AttendanceStatuses(setOfNotNull(newSelectedStatus)))
38+
onFilterChanged(BookingsFilterOption.AttendanceStatus(newSelectedStatus))
3939
}
4040

4141
@AssistedFactory
4242
interface Factory {
4343
fun create(
44-
initial: AttendanceStatuses?,
45-
onFilterChanged: (AttendanceStatuses) -> Unit
44+
initial: BookingsFilterOption.AttendanceStatus?,
45+
onFilterChanged: (BookingsFilterOption.AttendanceStatus) -> Unit
4646
): BookingAttendanceStatusFilterViewModel
4747
}
4848
}

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/filter/data/BookingFilterRepository.kt‎

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,12 @@ class BookingFilterRepository @Inject constructor(
2929
// Keys are built per-site to keep selections isolated across sites
3030
private fun teamMembersKey(siteId: Int) = stringSetPreferencesKey("bfilters_${siteId}_team_members")
3131
private fun bookingTypeKey(siteId: Int) = stringPreferencesKey("bfilters_${siteId}_booking_type")
32-
private fun attendanceStatusesKey(siteId: Int) = stringSetPreferencesKey("bfilters_${siteId}_attendance_statuses")
32+
private fun attendanceStatusKey(siteId: Int) = stringPreferencesKey("bfilters_${siteId}_attendance_status")
33+
34+
@Deprecated("Legacy key for migration")
35+
private fun legacyAttendanceStatusesKey(siteId: Int) =
36+
stringSetPreferencesKey("bfilters_${siteId}_attendance_statuses")
37+
3338
private fun customerIdKey(siteId: Int) = longPreferencesKey("bfilters_${siteId}_customer_id")
3439
private fun customerNameKey(siteId: Int) = stringPreferencesKey("bfilters_${siteId}_customer_name")
3540
private fun dateBeforeKey(siteId: Int) = longPreferencesKey("bfilters_${siteId}_date_before")
@@ -43,8 +48,7 @@ class BookingFilterRepository @Inject constructor(
4348
BookingFilters(
4449
teamMembers = prefs.getTeamMembers(siteId) ?: BookingsFilterOption.TeamMembers.DEFAULT,
4550
bookingType = prefs.getBookingType(siteId),
46-
attendanceStatuses = prefs.getAttendanceStatuses(siteId)
47-
?: BookingsFilterOption.AttendanceStatuses.DEFAULT,
51+
attendanceStatus = prefs.getAttendanceStatus(siteId),
4852
customer = prefs.getCustomerValue(siteId),
4953
dateRange = prefs.getDateRangeValue(siteId),
5054
serviceEvents = prefs.getServiceEventsValue(siteId)
@@ -56,7 +60,6 @@ class BookingFilterRepository @Inject constructor(
5660
suspend fun save(bookingFilters: BookingFilters) {
5761
val siteId = selectedSite.getSelectedSiteId()
5862
dataStore.edit { prefs ->
59-
// Team member
6063
val teamMembersKey = teamMembersKey(siteId)
6164
val teamMembersValues = bookingFilters.teamMembers.values
6265
if (teamMembersValues.isEmpty()) {
@@ -65,34 +68,29 @@ class BookingFilterRepository @Inject constructor(
6568
prefs[teamMembersKey] = teamMembersValues.map { it.value.toString() }.toSet()
6669
}
6770

68-
// Booking type
6971
val bookingTypeKey = bookingTypeKey(siteId)
7072
val bookingTypeValue = bookingFilters.bookingType?.value?.name
7173
if (bookingTypeValue != null) {
7274
prefs[bookingTypeKey] = bookingTypeValue
7375
} else {
74-
// Clear if not provided
7576
prefs.remove(bookingTypeKey)
7677
}
7778

78-
// Attendance statuses
79-
val attendanceStatusesKey = attendanceStatusesKey(siteId)
80-
val attendanceStatusesValues = bookingFilters.attendanceStatuses.values
81-
if (attendanceStatusesValues.isEmpty()) {
82-
prefs.remove(attendanceStatusesKey)
79+
val attendanceStatusValue = bookingFilters.attendanceStatus.value?.key
80+
if (attendanceStatusValue != null) {
81+
prefs[attendanceStatusKey(siteId)] = attendanceStatusValue
8382
} else {
84-
prefs[attendanceStatusesKey] = attendanceStatusesValues.map { it.key }.toSet()
83+
prefs.remove(attendanceStatusKey(siteId))
8584
}
85+
@Suppress("DEPRECATION")
86+
prefs.remove(legacyAttendanceStatusesKey(siteId))
8687

87-
// Customer
8888
val customerIdKey = customerIdKey(siteId)
8989
val customerNameKey = customerNameKey(siteId)
9090
val customer = bookingFilters.customer
9191
if (customer != null) {
92-
val id = customer.customerId
93-
val name = customer.customerName
94-
prefs[customerIdKey] = id
95-
prefs[customerNameKey] = name
92+
prefs[customerIdKey] = customer.customerId
93+
prefs[customerNameKey] = customer.customerName
9694
} else {
9795
// Clear if not provided
9896
prefs.remove(customerIdKey)
@@ -124,7 +122,6 @@ class BookingFilterRepository @Inject constructor(
124122
.map { "${it.productId}${SERVICE_EVENTS_PRODUCT_DELIMITER}${it.productName}" }
125123
.toSet()
126124
}
127-
// Other filters currently have no persisted payload; ignore for now
128125
}
129126
}
130127

@@ -141,13 +138,23 @@ class BookingFilterRepository @Inject constructor(
141138
return value?.let { BookingsFilterOption.BookingType(value = it) }
142139
}
143140

144-
private fun Preferences.getAttendanceStatuses(siteId: Int): BookingsFilterOption.AttendanceStatuses? {
145-
val stored = this[attendanceStatusesKey(siteId)] ?: return null
146-
val statuses = stored.mapNotNull { key ->
141+
private fun Preferences.getAttendanceStatus(siteId: Int): BookingsFilterOption.AttendanceStatus {
142+
val stored = this[attendanceStatusKey(siteId)]
143+
?: return migrateLegacyAttendanceStatus(siteId)
144+
val status = runCatching { BookingEntity.AttendanceStatus.fromKey(stored) }.getOrNull()
145+
?.takeIf { it !is BookingEntity.AttendanceStatus.Unknown }
146+
return BookingsFilterOption.AttendanceStatus(status)
147+
}
148+
149+
@Suppress("DEPRECATION")
150+
private fun Preferences.migrateLegacyAttendanceStatus(siteId: Int): BookingsFilterOption.AttendanceStatus {
151+
val legacy = this[legacyAttendanceStatusesKey(siteId)]
152+
?: return BookingsFilterOption.AttendanceStatus.DEFAULT
153+
val status = legacy.firstNotNullOfOrNull { key ->
147154
runCatching { BookingEntity.AttendanceStatus.fromKey(key) }.getOrNull()
148155
?.takeIf { it !is BookingEntity.AttendanceStatus.Unknown }
149-
}.toSet()
150-
return statuses.singleOrNull()?.let { BookingsFilterOption.AttendanceStatuses(setOf(it)) }
156+
}
157+
return BookingsFilterOption.AttendanceStatus(status)
151158
}
152159

153160
private fun Preferences.getCustomerValue(siteId: Int): BookingsFilterOption.Customer? {

‎WooCommerce/src/test/kotlin/com/woocommerce/android/ui/bookings/filter/BookingFilterListViewModelTest.kt‎

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import androidx.lifecycle.SavedStateHandle
44
import com.woocommerce.android.R
55
import com.woocommerce.android.analytics.AnalyticsEvent
66
import com.woocommerce.android.analytics.AnalyticsTrackerWrapper
7-
import com.woocommerce.android.model.UiString
87
import com.woocommerce.android.model.UiString.UiStringRes
98
import com.woocommerce.android.ui.bookings.filter.data.BookingFilterRepository
109
import com.woocommerce.android.util.getOrAwaitValue
@@ -221,7 +220,7 @@ class BookingFilterListViewModelTest : BaseUnitTest() {
221220
val onPage = viewModel.uiState.getOrAwaitValue()
222221

223222
// Select one attendance status (Attended)
224-
onPage.onUpdateFilterOption(BookingsFilterOption.AttendanceStatuses(values = setOf(AttendanceStatus.Attended)))
223+
onPage.onUpdateFilterOption(BookingsFilterOption.AttendanceStatus(AttendanceStatus.Attended))
225224

226225
// WHEN: leave the page (go back to root list)
227226
viewModel.uiState.getOrAwaitValue().onClose()
@@ -236,39 +235,12 @@ class BookingFilterListViewModelTest : BaseUnitTest() {
236235
assertThat(value).isEqualTo(R.string.booking_attendance_status_attended)
237236
}
238237

239-
@Test
240-
fun `when two attendance statuses selected and leaving page, then root shows selected both statuses`() {
241-
// GIVEN: navigate to Attendance Status page and select two statuses
242-
val initial = viewModel.uiState.getOrAwaitValue()
243-
initial.openPage(BookingFilterPage.AttendanceStatus)
244-
val onPage = viewModel.uiState.getOrAwaitValue()
245-
246-
// Select two attendance statuses (Attended and Unattended)
247-
onPage.onUpdateFilterOption(
248-
BookingsFilterOption.AttendanceStatuses(
249-
values = setOf(AttendanceStatus.Attended, AttendanceStatus.Unattended)
250-
)
251-
)
252-
253-
// WHEN: leave the page (go back to root list)
254-
viewModel.uiState.getOrAwaitValue().onClose()
255-
256-
// THEN: root list shows both selected statuses in subtitle values (order agnostic)
257-
val root = viewModel.uiState.getOrAwaitValue()
258-
259-
val attendanceItem =
260-
root.items.first { (it.title as UiStringRes).stringRes == R.string.bookings_filter_title_attendance_status }
261-
val value = (attendanceItem.value as UiString.UiStringText).text
262-
263-
assertThat(value).isEqualTo("2")
264-
}
265-
266238
@Test
267239
fun `when onShowBookings called with filters, then BOOKING_LIST_APPLY_FILTERS is tracked`() {
268240
val state = viewModel.uiState.getOrAwaitValue()
269241
state.onUpdateFilterOption(BookingType(BookingType.Type.SERVICE))
270242
state.onUpdateFilterOption(
271-
BookingsFilterOption.AttendanceStatuses(values = setOf(AttendanceStatus.Attended))
243+
BookingsFilterOption.AttendanceStatus(AttendanceStatus.Attended)
272244
)
273245

274246
viewModel.uiState.getOrAwaitValue().onShowBookings()

‎WooCommerce/src/test/kotlin/com/woocommerce/android/ui/bookings/filter/attendancestatus/BookingAttendanceStatusFilterViewModelTest.kt‎

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,20 @@ import com.woocommerce.android.viewmodel.BaseUnitTest
66
import kotlinx.coroutines.ExperimentalCoroutinesApi
77
import org.assertj.core.api.Assertions.assertThat
88
import org.junit.Test
9-
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption.AttendanceStatuses
9+
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption
1010
import org.wordpress.android.fluxc.persistence.entity.BookingEntity.AttendanceStatus
1111

1212
@OptIn(ExperimentalCoroutinesApi::class)
1313
class BookingAttendanceStatusFilterViewModelTest : BaseUnitTest() {
1414

15-
private var lastFilterResult: AttendanceStatuses? = null
15+
private var lastFilterResult: BookingsFilterOption.AttendanceStatus? = null
1616

1717
private fun createViewModel(
18-
initialStatuses: AttendanceStatuses? = null
18+
initialStatus: BookingsFilterOption.AttendanceStatus? = null
1919
): BookingAttendanceStatusFilterViewModel {
2020
lastFilterResult = null
2121
return BookingAttendanceStatusFilterViewModel(
22-
initialStatuses = initialStatuses,
22+
initialStatus = initialStatus,
2323
onFilterChanged = { lastFilterResult = it },
2424
savedStateHandle = SavedStateHandle()
2525
)
@@ -36,50 +36,50 @@ class BookingAttendanceStatusFilterViewModelTest : BaseUnitTest() {
3636
// THEN
3737
val state = viewModel.uiState.getOrAwaitValue()
3838
assertThat(state.selectedStatus).isEqualTo(AttendanceStatus.Attended)
39-
assertThat(lastFilterResult).isEqualTo(AttendanceStatuses(setOf(AttendanceStatus.Attended)))
39+
assertThat(lastFilterResult).isEqualTo(BookingsFilterOption.AttendanceStatus(AttendanceStatus.Attended))
4040
}
4141

4242
@Test
4343
fun `given Attended selected, when Unattended is selected, then only Unattended is in the filter`() =
4444
testBlocking {
4545
// GIVEN
46-
val viewModel = createViewModel(AttendanceStatuses(setOf(AttendanceStatus.Attended)))
46+
val viewModel = createViewModel(BookingsFilterOption.AttendanceStatus(AttendanceStatus.Attended))
4747

4848
// WHEN
4949
viewModel.uiState.getOrAwaitValue().items[UNATTENDED_INDEX].onClick()
5050

5151
// THEN
5252
val state = viewModel.uiState.getOrAwaitValue()
5353
assertThat(state.selectedStatus).isEqualTo(AttendanceStatus.Unattended)
54-
assertThat(lastFilterResult).isEqualTo(AttendanceStatuses(setOf(AttendanceStatus.Unattended)))
54+
assertThat(lastFilterResult).isEqualTo(BookingsFilterOption.AttendanceStatus(AttendanceStatus.Unattended))
5555
}
5656

5757
@Test
5858
fun `given Attended selected, when Any is selected, then filter is cleared`() = testBlocking {
5959
// GIVEN
60-
val viewModel = createViewModel(AttendanceStatuses(setOf(AttendanceStatus.Attended)))
60+
val viewModel = createViewModel(BookingsFilterOption.AttendanceStatus(AttendanceStatus.Attended))
6161

6262
// WHEN
6363
viewModel.uiState.getOrAwaitValue().items[ANY_INDEX].onClick()
6464

6565
// THEN
6666
val state = viewModel.uiState.getOrAwaitValue()
6767
assertThat(state.selectedStatus).isNull()
68-
assertThat(lastFilterResult).isEqualTo(AttendanceStatuses(emptySet()))
68+
assertThat(lastFilterResult).isEqualTo(BookingsFilterOption.AttendanceStatus(null))
6969
}
7070

7171
@Test
7272
fun `given Attended selected, when Attended is selected again, then Attended remains selected`() = testBlocking {
7373
// GIVEN
74-
val viewModel = createViewModel(AttendanceStatuses(setOf(AttendanceStatus.Attended)))
74+
val viewModel = createViewModel(BookingsFilterOption.AttendanceStatus(AttendanceStatus.Attended))
7575

7676
// WHEN
7777
viewModel.uiState.getOrAwaitValue().items[ATTENDED_INDEX].onClick()
7878

7979
// THEN
8080
val state = viewModel.uiState.getOrAwaitValue()
8181
assertThat(state.selectedStatus).isEqualTo(AttendanceStatus.Attended)
82-
assertThat(lastFilterResult).isEqualTo(AttendanceStatuses(setOf(AttendanceStatus.Attended)))
82+
assertThat(lastFilterResult).isEqualTo(BookingsFilterOption.AttendanceStatus(AttendanceStatus.Attended))
8383
}
8484

8585
companion object {

0 commit comments

Comments
 (0)