Skip to content

Commit f5681d1

Browse files
irfanoirfano
authored
Merge pull request #14687 from woocommerce/issue/WOOMOB-1237-booking-list-sorting-logic
[Bookings] Add "Sort by" logic to booking list screen
2 parents 55e165a + 3f2b31a commit f5681d1

File tree

10 files changed

+151
-101
lines changed

10 files changed

+151
-101
lines changed

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

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import com.woocommerce.android.WooException
44
import com.woocommerce.android.tools.SelectedSite
55
import kotlinx.coroutines.flow.Flow
66
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption
7+
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsOrderOption
78
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsStore
89
import org.wordpress.android.fluxc.persistence.entity.BookingEntity
910
import javax.inject.Inject
@@ -16,14 +17,16 @@ class BookingsRepository @Inject constructor(
1617
page: Int,
1718
perPage: Int,
1819
query: String? = null,
19-
filters: List<BookingsFilterOption> = emptyList()
20+
filters: List<BookingsFilterOption> = emptyList(),
21+
order: BookingsOrderOption
2022
): Result<FetchResult> {
2123
val result = bookingsStore.fetchBookings(
2224
site = selectedSite.get(),
2325
perPage = perPage,
2426
page = page,
2527
query = query,
26-
filters = filters
28+
filters = filters,
29+
order = order
2730
)
2831
return if (result.isError) {
2932
Result.failure(WooException(result.error))
@@ -41,12 +44,14 @@ class BookingsRepository @Inject constructor(
4144

4245
fun observeBookings(
4346
limit: Int? = null,
44-
filters: List<BookingsFilterOption> = emptyList()
47+
filters: List<BookingsFilterOption> = emptyList(),
48+
order: BookingsOrderOption
4549
): Flow<List<Booking>> =
4650
bookingsStore.observeBookings(
4751
site = selectedSite.get(),
4852
limit = limit,
49-
filters = filters
53+
filters = filters,
54+
order = order
5055
)
5156

5257
fun observeBooking(bookingId: Long): Flow<Booking?> =

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/list/BookingListHandler.kt‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import kotlinx.coroutines.flow.update
1212
import kotlinx.coroutines.sync.Mutex
1313
import kotlinx.coroutines.sync.withLock
1414
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption
15+
import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsOrderOption
1516
import java.util.concurrent.atomic.AtomicBoolean
1617
import javax.inject.Inject
1718

@@ -29,28 +30,40 @@ class BookingListHandler @Inject constructor(
2930

3031
private val searchQuery = MutableStateFlow<String?>(null)
3132
private val filters = MutableStateFlow<List<BookingsFilterOption>>(emptyList())
33+
private val sortBy = MutableStateFlow(BookingListSortOption.NewestToOldest)
3234

3335
private val searchResults = MutableStateFlow(emptyList<Booking>())
3436

3537
@OptIn(ExperimentalCoroutinesApi::class)
36-
val bookingsFlow: Flow<List<Booking>> = combine(searchQuery, filters, page) { query, filters, page ->
38+
val bookingsFlow: Flow<List<Booking>> = combine(
39+
searchQuery,
40+
filters,
41+
page,
42+
sortBy
43+
) { query, filters, page, sortBy ->
3744
if (query.isNullOrEmpty()) {
38-
bookingsRepository.observeBookings(limit = page * PAGE_SIZE, filters)
45+
bookingsRepository.observeBookings(
46+
limit = page * PAGE_SIZE,
47+
filters = filters,
48+
order = sortBy.toBookingsOrderOption()
49+
)
3950
} else {
4051
searchResults
4152
}
4253
}.flatMapLatest { it }
4354

4455
suspend fun loadBookings(
4556
searchQuery: String? = null,
46-
filters: List<BookingsFilterOption> = emptyList()
57+
filters: List<BookingsFilterOption> = emptyList(),
58+
sortBy: BookingListSortOption
4759
): Result<Unit> = mutex.withLock {
4860
// Reset pagination attributes
4961
page.value = 1
5062
canLoadMore.set(true)
5163

5264
this.searchQuery.value = searchQuery
5365
this.filters.value = filters
66+
this.sortBy.value = sortBy
5467

5568
return@withLock if (searchQuery == null) {
5669
fetchBookings()
@@ -73,11 +86,13 @@ class BookingListHandler @Inject constructor(
7386

7487
private suspend fun fetchBookings(): Result<Unit> {
7588
val isSearching = !searchQuery.value.isNullOrEmpty()
89+
val order = sortBy.value.toBookingsOrderOption()
7690
return bookingsRepository.fetchBookings(
7791
page = page.value,
7892
perPage = PAGE_SIZE,
7993
query = searchQuery.value,
80-
filters = filters.value
94+
filters = filters.value,
95+
order = order
8196
).onSuccess { result ->
8297
canLoadMore.set(result.hasMorePages)
8398
if (result.hasMorePages) {
@@ -88,4 +103,9 @@ class BookingListHandler @Inject constructor(
88103
}
89104
}.map { }
90105
}
106+
107+
private fun BookingListSortOption.toBookingsOrderOption() = when (this) {
108+
BookingListSortOption.NewestToOldest -> BookingsOrderOption.DESC
109+
BookingListSortOption.OldestToNewest -> BookingsOrderOption.ASC
110+
}
91111
}

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/list/BookingListScreen.kt‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ import com.woocommerce.android.R
4949
import com.woocommerce.android.ui.bookings.compose.BookingAttendanceStatus
5050
import com.woocommerce.android.ui.bookings.compose.BookingStatus
5151
import com.woocommerce.android.ui.bookings.compose.BookingSummary
52+
import com.woocommerce.android.ui.bookings.compose.BookingSummaryModel
5253
import com.woocommerce.android.ui.compose.component.InfiniteListHandler
5354
import com.woocommerce.android.ui.compose.component.Toolbar
5455
import com.woocommerce.android.ui.compose.component.WCPrimaryTabRow
@@ -300,7 +301,7 @@ private fun BookingListPreview() {
300301
bookings = List(20) {
301302
BookingListItem(
302303
id = it.toLong(),
303-
summary = com.woocommerce.android.ui.bookings.compose.BookingSummaryModel(
304+
summary = BookingSummaryModel(
304305
date = "Aug 20, 2024",
305306
name = "Women’s Haircut",
306307
customerName = "Margarita Nikolaevna",

‎WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/list/BookingListViewModel.kt‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -40,13 +40,7 @@ class BookingListViewModel @Inject constructor(
4040
key = "searchQuery"
4141
)
4242

43-
private val sortOptionsByTab = MutableStateFlow(
44-
mapOf(
45-
BookingListTab.Today to BookingListSortOption.NewestToOldest,
46-
BookingListTab.Upcoming to BookingListSortOption.NewestToOldest,
47-
BookingListTab.All to BookingListSortOption.NewestToOldest,
48-
)
49-
)
43+
private val sortOption = savedStateHandle.getStateFlow(viewModelScope, BookingListSortOption.NewestToOldest)
5044

5145
private val isSortSheetVisible = MutableStateFlow(false)
5246

@@ -79,11 +73,10 @@ class BookingListViewModel @Inject constructor(
7973
val state = combine(
8074
contentState,
8175
selectedTab,
82-
sortOptionsByTab,
76+
sortOption,
8377
isSortSheetVisible,
8478
searchState
85-
) { contentState, selectedTab, sortOptionsByTab, sheetVisible, searchState ->
86-
val sortOption = sortOptionsByTab[selectedTab] ?: BookingListSortOption.NewestToOldest
79+
) { contentState, selectedTab, sortOption, sheetVisible, searchState ->
8780
BookingListViewState(
8881
contentState = contentState,
8982
tabState = BookingListTabState(
@@ -123,23 +116,30 @@ class BookingListViewModel @Inject constructor(
123116
.debounce {
124117
if (it.isNullOrEmpty()) 0L else AppConstants.SEARCH_TYPING_DELAY_MS
125118
}
126-
127-
merge(selectedTab, queryFlow)
128-
.collectLatest {
129-
// Cancel any ongoing fetch or load more operations
130-
bookingsFetchJob?.cancel()
131-
bookingsLoadMoreJob?.cancel()
132-
133-
bookingsFetchJob = fetchBookings(BookingListLoadingState.Loading)
134-
}
119+
val sortFlow = sortOption.drop(1) // Skip the initial value to avoid double fetch on init
120+
121+
merge(selectedTab, queryFlow, sortFlow).collectLatest {
122+
// Cancel any ongoing fetch or load more operations
123+
bookingsFetchJob?.cancel()
124+
bookingsLoadMoreJob?.cancel()
125+
126+
bookingsFetchJob = fetchBookings(
127+
initialLoadingState = if (it is BookingListSortOption) {
128+
BookingListLoadingState.Refreshing
129+
} else {
130+
BookingListLoadingState.Loading
131+
}
132+
)
133+
}
135134
}
136135
}
137136

138137
private fun fetchBookings(initialLoadingState: BookingListLoadingState) = launch {
139138
loadingState.value = initialLoadingState
140139
bookingListHandler.loadBookings(
141140
searchQuery = searchQuery.value,
142-
filters = prepareFilters()
141+
filters = prepareFilters(),
142+
sortBy = sortOption.value
143143
).onFailure {
144144
triggerEvent(MultiLiveEvent.Event.ShowSnackbar(R.string.bookings_fetch_error))
145145
}
@@ -174,11 +174,8 @@ class BookingListViewModel @Inject constructor(
174174
}
175175

176176
private fun onSortOptionSelected(option: BookingListSortOption) {
177-
val tab = selectedTab.value
178-
sortOptionsByTab.value = sortOptionsByTab.value.toMutableMap()
179-
.also { it[tab] = option }
177+
sortOption.value = option
180178
isSortSheetVisible.value = false
181-
// TODO Apply the selected sorting to the data for the active tab
182179
}
183180

184181
private fun onSortDismiss() {

‎WooCommerce/src/test/kotlin/com/woocommerce/android/ui/bookings/list/BookingListHandlerTest.kt‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -33,11 +33,19 @@ class BookingListHandlerTest : BaseUnitTest() {
3333
private val availablePages = 3
3434
private val bookingsRepository: BookingsRepository = mock {
3535
val results = MutableStateFlow(emptyList<Booking>())
36-
on { observeBookings(any(), any()) } doAnswer { invocation ->
36+
on { observeBookings(any(), any(), any()) } doAnswer { invocation ->
3737
val limit = invocation.getArgument<Int>(0)
3838
results.map { it.take(limit) }
3939
}
40-
onBlocking { fetchBookings(any(), any(), anyOrNull(), any()) } doAnswer InlineClassesAnswer { invocation ->
40+
onBlocking {
41+
fetchBookings(
42+
any(),
43+
any(),
44+
anyOrNull(),
45+
any(),
46+
any()
47+
)
48+
} doAnswer InlineClassesAnswer { invocation ->
4149
val page = invocation.getArgument<Int>(0)
4250
val perPage = invocation.getArgument<Int>(1)
4351
val canLoadMore = page < availablePages
@@ -57,7 +65,7 @@ class BookingListHandlerTest : BaseUnitTest() {
5765
@Test
5866
fun `given repository returns bookings, when observing bookings flow, then returns bookings`() = testBlocking {
5967
val sampleBookings = List(10) { getSampleBooking(it) }
60-
given(bookingsRepository.observeBookings(any(), any())).willReturn(flowOf(sampleBookings))
68+
given(bookingsRepository.observeBookings(any(), any(), any())).willReturn(flowOf(sampleBookings))
6169

6270
val bookings = bookingListHandler.bookingsFlow.first()
6371

@@ -67,7 +75,10 @@ class BookingListHandlerTest : BaseUnitTest() {
6775
@Test
6876
fun `given no search query and force refresh, when loading bookings, then fetches from repository`() =
6977
testBlocking {
70-
val result = bookingListHandler.loadBookings(searchQuery = null)
78+
val result = bookingListHandler.loadBookings(
79+
searchQuery = null,
80+
sortBy = BookingListSortOption.NewestToOldest
81+
)
7182
val bookings = bookingListHandler.bookingsFlow.first()
7283

7384
assertThat(result.isSuccess).isTrue()
@@ -78,10 +89,21 @@ class BookingListHandlerTest : BaseUnitTest() {
7889
fun `given repository fetch fails, when loading bookings with force refresh, then returns failure`() =
7990
testBlocking {
8091
val exception = Exception("Network error")
81-
given(bookingsRepository.fetchBookings(page = any(), perPage = any(), query = anyOrNull(), filters = any()))
92+
given(
93+
bookingsRepository.fetchBookings(
94+
page = any(),
95+
perPage = any(),
96+
query = anyOrNull(),
97+
filters = any(),
98+
order = any()
99+
)
100+
)
82101
.willReturn(Result.failure(exception))
83102

84-
val result = bookingListHandler.loadBookings(searchQuery = null)
103+
val result = bookingListHandler.loadBookings(
104+
searchQuery = null,
105+
sortBy = BookingListSortOption.NewestToOldest
106+
)
85107

86108
assertThat(result.isFailure).isTrue()
87109
assertThat(result.exceptionOrNull()).isEqualTo(exception)
@@ -97,13 +119,14 @@ class BookingListHandlerTest : BaseUnitTest() {
97119
page = any(),
98120
perPage = any(),
99121
query = anyOrNull(),
100-
filters = any()
122+
filters = any(),
123+
order = any()
101124
)
102125
}
103126

104127
@Test
105128
fun `when load more is called and can load more is true, then fetches next page`() = testBlocking {
106-
bookingListHandler.loadBookings()
129+
bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
107130

108131
val result = bookingListHandler.loadMore()
109132
val bookings = bookingListHandler.bookingsFlow.first()
@@ -114,7 +137,7 @@ class BookingListHandlerTest : BaseUnitTest() {
114137

115138
@Test
116139
fun `when last page is reached, then can load more becomes false`() = testBlocking {
117-
bookingListHandler.loadBookings()
140+
bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
118141

119142
var result: Result<Unit>? = null
120143
repeat(availablePages - 1) {
@@ -126,26 +149,27 @@ class BookingListHandlerTest : BaseUnitTest() {
126149
page = intThat { it > availablePages },
127150
perPage = any(),
128151
query = anyOrNull(),
129-
filters = any()
152+
filters = any(),
153+
order = any()
130154
)
131155
}
132156

133157
@Test
134158
fun `when load bookings is called, then pagination resets`() = testBlocking {
135159
// First load and load more to advance page
136-
bookingListHandler.loadBookings()
160+
bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
137161
bookingListHandler.loadMore()
138162

139163
// Load bookings again - should reset to page 1
140-
bookingListHandler.loadBookings()
164+
bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
141165
val bookings = bookingListHandler.bookingsFlow.first()
142166

143167
assertThat(bookings).hasSize(BookingListHandler.PAGE_SIZE)
144168
}
145169

146170
@Test
147171
fun `when bookings flow is observed with pagination, then limit increases correctly`() = testBlocking {
148-
bookingListHandler.loadBookings()
172+
bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
149173

150174
val initialBookings = bookingListHandler.bookingsFlow.first()
151175
assertThat(initialBookings).hasSize(BookingListHandler.PAGE_SIZE)
@@ -154,15 +178,19 @@ class BookingListHandlerTest : BaseUnitTest() {
154178
val moreBookings = bookingListHandler.bookingsFlow.first()
155179

156180
@Suppress("UnusedFlow")
157-
verify(bookingsRepository).observeBookings(limit = eq(2 * BookingListHandler.PAGE_SIZE), filters = any())
181+
verify(bookingsRepository).observeBookings(
182+
limit = eq(2 * BookingListHandler.PAGE_SIZE),
183+
filters = any(),
184+
order = any()
185+
)
158186
assertThat(moreBookings).hasSize(2 * BookingListHandler.PAGE_SIZE)
159187
}
160188

161189
@Test
162190
fun `when concurrent load operations occur, then operations are synchronized`() = testBlocking {
163191
// Launch multiple concurrent load operations
164-
val job1 = launch { bookingListHandler.loadBookings() }
165-
val job2 = launch { bookingListHandler.loadBookings() }
192+
val job1 = launch { bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest) }
193+
val job2 = launch { bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest) }
166194
val job3 = launch { bookingListHandler.loadMore() }
167195

168196
job1.join()

0 commit comments

Comments
 (0)