Skip to content

Prevent unnecessary booking fetches on search close #15092

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.

Sign up for GitHub

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

Open
irfano wants to merge 3 commits into
base: trunk
Choose a base branch
from
Open

Prevent unnecessary booking fetches on search close #15092

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f3c0f33
Verify unnecessary fetch prevention on search close
irfano Dec 12, 2025
737242b
Prevent unnecessary API calls on search close
irfano Dec 12, 2025
4c6c778
Simplify initial loading state logic
irfano Dec 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions WooCommerce/src/main/kotlin/com/woocommerce/android/ui/bookings/list/BookingListViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ class BookingListViewModel @Inject constructor(
filters
) { tab, query, sort, filters ->
FetchParams(
searchQuery = query,
searchQuery = query?.takeUnless { it.isEmpty() },
Copy link
Contributor Author

@irfano irfano Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Empty strings are now treated as null.

sortOption = sort,
selectedTab = tab,
filters = filters
Expand All @@ -193,17 +193,14 @@ class BookingListViewModel @Inject constructor(
bookingsFetchJob?.cancel()
bookingsLoadMoreJob?.cancel()

val initialLoadingState = if (isRefreshing) {
if (!isRefreshing && lastFetchParams == fetchParams) return@collectLatest
Copy link
Contributor Author

@irfano irfano Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a check to skip data fetching if parameters remain unchanged.


val initialLoadingState = if (isRefreshing ||
(lastFetchParams != null && lastFetchParams?.sortOption != fetchParams.sortOption)
) {
BookingListLoadingState.Refreshing
} else {
lastFetchParams.let { lastFetchParams ->
if (lastFetchParams != null && lastFetchParams.sortOption != fetchParams.sortOption) {
// When sort option changes, force refreshing state to indicate data reload
BookingListLoadingState.Refreshing
} else {
BookingListLoadingState.Loading
}
}
BookingListLoadingState.Loading
Comment on lines +198 to +203
Copy link
Contributor Author

@irfano irfano Dec 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified initialLoadingState logic.

}

lastFetchParams = fetchParams
Expand Down
28 changes: 28 additions & 0 deletions ...erce/src/test/kotlin/com/woocommerce/android/ui/bookings/list/BookingListViewModelTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.woocommerce.android.ui.bookings.list

import androidx.lifecycle.SavedStateHandle
import com.woocommerce.android.AppConstants
import com.woocommerce.android.R
import com.woocommerce.android.model.GetLocations
import com.woocommerce.android.ui.bookings.Booking
Expand All @@ -16,6 +17,7 @@ import com.woocommerce.android.viewmodel.ResourceProvider
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.flowOf
import kotlinx.coroutines.test.advanceTimeBy
import kotlinx.coroutines.test.advanceUntilIdle
import org.assertj.core.api.Assertions.assertThat
import org.mockito.kotlin.any
Expand Down Expand Up @@ -299,6 +301,7 @@ class BookingListViewModelTest : BaseUnitTest() {
// WHEN
val initialState = viewModel.state.getOrAwaitValue()
initialState.searchState.onQueryChanged("test query")
advanceTimeBy(AppConstants.SEARCH_TYPING_DELAY_MS + 1)
val stateWithQuery = viewModel.state.getOrAwaitValue()
stateWithQuery.searchState.onQueryChanged(null)
advanceUntilIdle()
Expand Down Expand Up @@ -408,6 +411,31 @@ class BookingListViewModelTest : BaseUnitTest() {
assertThat(savedStateHandle.get<Long>(BookingListViewModel.KEY_BOOKING_SELECTED_ON_BIG_SCREEN)).isEqualTo(1234L)
}

@Test
fun `when search query is opened and closed without typing, then bookings are NOT refetched`() = testBlocking {
// GIVEN
setup()

// WHEN
val initialState = viewModel.state.getOrAwaitValue()
// Search opened -> query starts empty
initialState.searchState.onQueryChanged("")
advanceUntilIdle()

val stateWithEmptyQuery = viewModel.state.getOrAwaitValue()
// Search closed -> query becomes null
stateWithEmptyQuery.searchState.onQueryChanged(null)
advanceUntilIdle()

// THEN
// Should only be called once for the initial load
verify(bookingListHandler, times(1)).loadBookings(
searchQuery = anyOrNull(),
filters = any(),
sortBy = any()
)
}

private fun getSampleBooking(id: Int): Booking {
return BookingEntity(
id = LocalOrRemoteId.RemoteId(id.toLong()),
Expand Down
Loading