[Bookings] Add "Sort by" logic to booking list screen

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

@@ -4,6 +4,7 @@ import com.woocommerce.android.WooException
 import com.woocommerce.android.tools.SelectedSite
 import kotlinx.coroutines.flow.Flow
 import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption
+import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsOrderOption
 import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsStore
 import org.wordpress.android.fluxc.persistence.entity.BookingEntity
 import javax.inject.Inject
@@ -16,14 +17,16 @@ class BookingsRepository @Inject constructor(
         page: Int,
         perPage: Int,
         query: String? = null,
-        filters: List<BookingsFilterOption> = emptyList()
+        filters: List<BookingsFilterOption> = emptyList(),
+        order: BookingsOrderOption
     ): Result<FetchResult> {
         val result = bookingsStore.fetchBookings(
             site = selectedSite.get(),
             perPage = perPage,
             page = page,
             query = query,
-            filters = filters
+            filters = filters,
+            order = order
         )
         return if (result.isError) {
             Result.failure(WooException(result.error))
@@ -41,12 +44,14 @@ class BookingsRepository @Inject constructor(
 
     fun observeBookings(
         limit: Int? = null,
-        filters: List<BookingsFilterOption> = emptyList()
+        filters: List<BookingsFilterOption> = emptyList(),
+        order: BookingsOrderOption
     ): Flow<List<Booking>> =
         bookingsStore.observeBookings(
             site = selectedSite.get(),
             limit = limit,
-            filters = filters
+            filters = filters,
+            order = order
         )
 
     data class FetchResult(

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

@@ -12,6 +12,7 @@ import kotlinx.coroutines.flow.update
 import kotlinx.coroutines.sync.Mutex
 import kotlinx.coroutines.sync.withLock
 import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsFilterOption
+import org.wordpress.android.fluxc.network.rest.wpcom.wc.bookings.BookingsOrderOption
 import java.util.concurrent.atomic.AtomicBoolean
 import javax.inject.Inject
 
@@ -29,28 +30,40 @@ class BookingListHandler @Inject constructor(
 
     private val searchQuery = MutableStateFlow<String?>(null)
     private val filters = MutableStateFlow<List<BookingsFilterOption>>(emptyList())
+    private val sortBy = MutableStateFlow(BookingListSortOption.NewestToOldest)
 
     private val searchResults = MutableStateFlow(emptyList<Booking>())
 
     @OptIn(ExperimentalCoroutinesApi::class)
-    val bookingsFlow: Flow<List<Booking>> = combine(searchQuery, filters, page) { query, filters, page ->
+    val bookingsFlow: Flow<List<Booking>> = combine(
+        searchQuery,
+        filters,
+        page,
+        sortBy
+    ) { query, filters, page, sortBy ->
         if (query.isNullOrEmpty()) {
-            bookingsRepository.observeBookings(limit = page * PAGE_SIZE, filters)
+            bookingsRepository.observeBookings(
+                limit = page * PAGE_SIZE,
+                filters = filters,
+                order = sortBy.toBookingsOrderOption()
+            )
         } else {
             searchResults
         }
     }.flatMapLatest { it }
 
     suspend fun loadBookings(
         searchQuery: String? = null,
-        filters: List<BookingsFilterOption> = emptyList()
+        filters: List<BookingsFilterOption> = emptyList(),
+        sortBy: BookingListSortOption
     ): Result<Unit> = mutex.withLock {
         // Reset pagination attributes
         page.value = 1
         canLoadMore.set(true)
 
         this.searchQuery.value = searchQuery
         this.filters.value = filters
+        this.sortBy.value = sortBy
 
         return@withLock if (searchQuery == null) {
             fetchBookings()
@@ -73,11 +86,13 @@ class BookingListHandler @Inject constructor(
 
     private suspend fun fetchBookings(): Result<Unit> {
         val isSearching = !searchQuery.value.isNullOrEmpty()
+        val order = sortBy.value.toBookingsOrderOption()
         return bookingsRepository.fetchBookings(
             page = page.value,
             perPage = PAGE_SIZE,
             query = searchQuery.value,
-            filters = filters.value
+            filters = filters.value,
+            order = order
         ).onSuccess { result ->
             canLoadMore.set(result.hasMorePages)
             if (result.hasMorePages) {
@@ -88,4 +103,9 @@ class BookingListHandler @Inject constructor(
             }
         }.map { }
     }
+
+    private fun BookingListSortOption.toBookingsOrderOption() = when (this) {
+        BookingListSortOption.NewestToOldest -> BookingsOrderOption.DESC
+        BookingListSortOption.OldestToNewest -> BookingsOrderOption.ASC
+    }
 }

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

@@ -49,6 +49,7 @@ import com.woocommerce.android.R
 import com.woocommerce.android.ui.bookings.compose.BookingAttendanceStatus
 import com.woocommerce.android.ui.bookings.compose.BookingStatus
 import com.woocommerce.android.ui.bookings.compose.BookingSummary
+import com.woocommerce.android.ui.bookings.compose.BookingSummaryModel
 import com.woocommerce.android.ui.compose.component.InfiniteListHandler
 import com.woocommerce.android.ui.compose.component.Toolbar
 import com.woocommerce.android.ui.compose.component.WCPrimaryTabRow
@@ -300,7 +301,7 @@ private fun BookingListPreview() {
                     bookings = List(20) {
                         BookingListItem(
                             id = it.toLong(),
-                            summary = com.woocommerce.android.ui.bookings.compose.BookingSummaryModel(
+                            summary = BookingSummaryModel(
                                 date = "Aug 20, 2024",
                                 name = "Women’s Haircut",
                                 customerName = "Margarita Nikolaevna",

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

@@ -38,13 +38,7 @@ class BookingListViewModel @Inject constructor(
         key = "searchQuery"
     )
 
-    private val sortOptionsByTab = MutableStateFlow(
-        mapOf(
-            BookingListTab.Today to BookingListSortOption.NewestToOldest,
-            BookingListTab.Upcoming to BookingListSortOption.NewestToOldest,
-            BookingListTab.All to BookingListSortOption.NewestToOldest,
-        )
-    )
+    private val sortOption = savedStateHandle.getStateFlow(viewModelScope, BookingListSortOption.NewestToOldest)
 
     private val isSortSheetVisible = MutableStateFlow(false)
 
@@ -75,11 +69,10 @@ class BookingListViewModel @Inject constructor(
     val state = combine(
         contentState,
         selectedTab,
-        sortOptionsByTab,
+        sortOption,
         isSortSheetVisible,
         searchState
-    ) { contentState, selectedTab, sortOptionsByTab, sheetVisible, searchState ->
-        val sortOption = sortOptionsByTab[selectedTab] ?: BookingListSortOption.NewestToOldest
+    ) { contentState, selectedTab, sortOption, sheetVisible, searchState ->
         BookingListViewState(
             contentState = contentState,
             tabState = BookingListTabState(
@@ -119,23 +112,30 @@ class BookingListViewModel @Inject constructor(
                 .debounce {
                     if (it.isNullOrEmpty()) 0L else AppConstants.SEARCH_TYPING_DELAY_MS
                 }
-
-            merge(selectedTab, queryFlow)
-                .collectLatest {
-                    // Cancel any ongoing fetch or load more operations
-                    bookingsFetchJob?.cancel()
-                    bookingsLoadMoreJob?.cancel()
-
-                    bookingsFetchJob = fetchBookings(BookingListLoadingState.Loading)
-                }
+            val sortFlow = sortOption.drop(1) // Skip the initial value to avoid double fetch on init
+
+            merge(selectedTab, queryFlow, sortFlow).collectLatest {
+                // Cancel any ongoing fetch or load more operations
+                bookingsFetchJob?.cancel()
+                bookingsLoadMoreJob?.cancel()
+
+                bookingsFetchJob = fetchBookings(
+                    initialLoadingState = if (it is BookingListSortOption) {
+                        BookingListLoadingState.Refreshing
+                    } else {
+                        BookingListLoadingState.Loading
+                    }
+                )
+            }
         }
     }
 
     private fun fetchBookings(initialLoadingState: BookingListLoadingState) = launch {
         loadingState.value = initialLoadingState
         bookingListHandler.loadBookings(
             searchQuery = searchQuery.value,
-            filters = prepareFilters()
+            filters = prepareFilters(),
+            sortBy = sortOption.value
         ).onFailure {
             triggerEvent(MultiLiveEvent.Event.ShowSnackbar(R.string.bookings_fetch_error))
         }
@@ -170,11 +170,8 @@ class BookingListViewModel @Inject constructor(
     }
 
     private fun onSortOptionSelected(option: BookingListSortOption) {
-        val tab = selectedTab.value
-        sortOptionsByTab.value = sortOptionsByTab.value.toMutableMap()
-            .also { it[tab] = option }
+        sortOption.value = option
         isSortSheetVisible.value = false
-        // TODO Apply the selected sorting to the data for the active tab
     }
 
     private fun onSortDismiss() {

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

@@ -33,11 +33,19 @@ class BookingListHandlerTest : BaseUnitTest() {
     private val availablePages = 3
     private val bookingsRepository: BookingsRepository = mock {
         val results = MutableStateFlow(emptyList<Booking>())
-        on { observeBookings(any(), any()) } doAnswer { invocation ->
+        on { observeBookings(any(), any(), any()) } doAnswer { invocation ->
             val limit = invocation.getArgument<Int>(0)
             results.map { it.take(limit) }
         }
-        onBlocking { fetchBookings(any(), any(), anyOrNull(), any()) } doAnswer InlineClassesAnswer { invocation ->
+        onBlocking {
+            fetchBookings(
+                any(),
+                any(),
+                anyOrNull(),
+                any(),
+                any()
+            )
+        } doAnswer InlineClassesAnswer { invocation ->
             val page = invocation.getArgument<Int>(0)
             val perPage = invocation.getArgument<Int>(1)
             val canLoadMore = page < availablePages
@@ -57,7 +65,7 @@ class BookingListHandlerTest : BaseUnitTest() {
     @Test
     fun `given repository returns bookings, when observing bookings flow, then returns bookings`() = testBlocking {
         val sampleBookings = List(10) { getSampleBooking(it) }
-        given(bookingsRepository.observeBookings(any(), any())).willReturn(flowOf(sampleBookings))
+        given(bookingsRepository.observeBookings(any(), any(), any())).willReturn(flowOf(sampleBookings))
 
         val bookings = bookingListHandler.bookingsFlow.first()
 
@@ -67,7 +75,10 @@ class BookingListHandlerTest : BaseUnitTest() {
     @Test
     fun `given no search query and force refresh, when loading bookings, then fetches from repository`() =
         testBlocking {
-            val result = bookingListHandler.loadBookings(searchQuery = null)
+            val result = bookingListHandler.loadBookings(
+                searchQuery = null,
+                sortBy = BookingListSortOption.NewestToOldest
+            )
             val bookings = bookingListHandler.bookingsFlow.first()
 
             assertThat(result.isSuccess).isTrue()
@@ -78,10 +89,21 @@ class BookingListHandlerTest : BaseUnitTest() {
     fun `given repository fetch fails, when loading bookings with force refresh, then returns failure`() =
         testBlocking {
             val exception = Exception("Network error")
-            given(bookingsRepository.fetchBookings(page = any(), perPage = any(), query = anyOrNull(), filters = any()))
+            given(
+                bookingsRepository.fetchBookings(
+                    page = any(),
+                    perPage = any(),
+                    query = anyOrNull(),
+                    filters = any(),
+                    order = any()
+                )
+            )
                 .willReturn(Result.failure(exception))
 
-            val result = bookingListHandler.loadBookings(searchQuery = null)
+            val result = bookingListHandler.loadBookings(
+                searchQuery = null,
+                sortBy = BookingListSortOption.NewestToOldest
+            )
 
             assertThat(result.isFailure).isTrue()
             assertThat(result.exceptionOrNull()).isEqualTo(exception)
@@ -97,13 +119,14 @@ class BookingListHandlerTest : BaseUnitTest() {
                 page = any(),
                 perPage = any(),
                 query = anyOrNull(),
-                filters = any()
+                filters = any(),
+                order = any()
             )
         }
 
     @Test
     fun `when load more is called and can load more is true, then fetches next page`() = testBlocking {
-        bookingListHandler.loadBookings()
+        bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
 
         val result = bookingListHandler.loadMore()
         val bookings = bookingListHandler.bookingsFlow.first()
@@ -114,7 +137,7 @@ class BookingListHandlerTest : BaseUnitTest() {
 
     @Test
     fun `when last page is reached, then can load more becomes false`() = testBlocking {
-        bookingListHandler.loadBookings()
+        bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
 
         var result: Result<Unit>? = null
         repeat(availablePages - 1) {
@@ -126,26 +149,27 @@ class BookingListHandlerTest : BaseUnitTest() {
             page = intThat { it > availablePages },
             perPage = any(),
             query = anyOrNull(),
-            filters = any()
+            filters = any(),
+            order = any()
         )
     }
 
     @Test
     fun `when load bookings is called, then pagination resets`() = testBlocking {
         // First load and load more to advance page
-        bookingListHandler.loadBookings()
+        bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
         bookingListHandler.loadMore()
 
         // Load bookings again - should reset to page 1
-        bookingListHandler.loadBookings()
+        bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
         val bookings = bookingListHandler.bookingsFlow.first()
 
         assertThat(bookings).hasSize(BookingListHandler.PAGE_SIZE)
     }
 
     @Test
     fun `when bookings flow is observed with pagination, then limit increases correctly`() = testBlocking {
-        bookingListHandler.loadBookings()
+        bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest)
 
         val initialBookings = bookingListHandler.bookingsFlow.first()
         assertThat(initialBookings).hasSize(BookingListHandler.PAGE_SIZE)
@@ -154,15 +178,19 @@ class BookingListHandlerTest : BaseUnitTest() {
         val moreBookings = bookingListHandler.bookingsFlow.first()
 
         @Suppress("UnusedFlow")
-        verify(bookingsRepository).observeBookings(limit = eq(2 * BookingListHandler.PAGE_SIZE), filters = any())
+        verify(bookingsRepository).observeBookings(
+            limit = eq(2 * BookingListHandler.PAGE_SIZE),
+            filters = any(),
+            order = any()
+        )
         assertThat(moreBookings).hasSize(2 * BookingListHandler.PAGE_SIZE)
     }
 
     @Test
     fun `when concurrent load operations occur, then operations are synchronized`() = testBlocking {
         // Launch multiple concurrent load operations
-        val job1 = launch { bookingListHandler.loadBookings() }
-        val job2 = launch { bookingListHandler.loadBookings() }
+        val job1 = launch { bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest) }
+        val job2 = launch { bookingListHandler.loadBookings(sortBy = BookingListSortOption.NewestToOldest) }
         val job3 = launch { bookingListHandler.loadMore() }
 
         job1.join()