Merge pull request #13974 from woocommerce/merge/release-22.2-into-trunk

Merge release/22.2 into trunk

Author: samiuelson

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosProductsCache.kt

@@ -18,4 +18,6 @@ interface WooPosProductsCache {
     suspend fun deleteProduct(productId: Long)
 
     suspend fun clear()
+
+    suspend fun setAll(products: List<Product>)
 }

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/common/data/WooPosProductsInMemoryCache.kt

@@ -19,13 +19,7 @@ class WooPosProductsInMemoryCache @Inject constructor() : WooPosProductsCache {
     private val productsCache = LinkedHashMap<Long, Product>(INITIAL_CAPACITY, LOAD_FACTOR, true)
 
     override suspend fun addAll(products: List<Product>) = mutex.withLock {
-        products.forEach { product ->
-            productsCache[product.remoteId] = product
-            if (productsCache.size > MAX_CACHE_SIZE) {
-                val keysToRemove = productsCache.keys.take(productsCache.size - MAX_CACHE_SIZE)
-                keysToRemove.forEach { productsCache.remove(it) }
-            }
-        }
+        addAllInternal(products)
     }
 
     override suspend fun updateProduct(product: Product) = mutex.withLock {
@@ -49,4 +43,19 @@ class WooPosProductsInMemoryCache @Inject constructor() : WooPosProductsCache {
     override suspend fun clear() = mutex.withLock {
         productsCache.clear()
     }
+
+    override suspend fun setAll(products: List<Product>) = mutex.withLock {
+        productsCache.clear()
+        addAllInternal(products)
+    }
+
+    private fun addAllInternal(products: List<Product>) {
+        products.forEach { product ->
+            productsCache[product.remoteId] = product
+            if (productsCache.size > MAX_CACHE_SIZE) {
+                val keysToRemove = productsCache.keys.take(productsCache.size - MAX_CACHE_SIZE)
+                keysToRemove.forEach { productsCache.remove(it) }
+            }
+        }
+    }
 }

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosItemsList.kt

@@ -402,7 +402,7 @@ private fun InfiniteListHandler(
     state: WooPosContentViewState,
     onEndOfProductsListReached: () -> Unit
 ) {
-    val buffer = 5
+    val buffer = 5 // must be smaller than the page size, otherwise won't call onEndOfProductsListReached
     val loadMore = remember {
         derivedStateOf {
             val layoutInfo = listState.layoutInfo

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsDataSource.kt

@@ -80,17 +80,19 @@ class WooPosProductsDataSource @Inject constructor(
 
     fun loadProducts(forceRefreshProducts: Boolean): Flow<ProductsResult> = flow {
         offset.set(0)
-        if (forceRefreshProducts) {
-            productsCache.clear()
-        }
         productsIndex.clearCache()
 
-        val cachedProducts = sortProducts(productsCache.getAll()).take(NORMAL_PAGE_SIZE)
-        emit(ProductsResult.Cached(cachedProducts))
+        if (!forceRefreshProducts) {
+            val cachedProducts = sortProducts(productsCache.getAll()).take(NORMAL_PAGE_SIZE)
+            emit(ProductsResult.Cached(cachedProducts))
+        }
 
         val fetchResult = fetchProducts()
 
         if (fetchResult.isSuccess) {
+            if (forceRefreshProducts) {
+                productsCache.setAll(fetchResult.getOrThrow())
+            }
             emit(ProductsResult.Remote(Result.success(fetchResult.getOrThrow())))
         } else {
             emit(ProductsResult.Remote(Result.failure(fetchResult.exceptionOrNull() ?: Exception("Unknown error"))))

WooCommerce/src/main/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsViewModel.kt

@@ -35,6 +35,8 @@ class WooPosProductsViewModel @Inject constructor(
     private val analyticsTracker: WooPosAnalyticsTracker,
 ) : ViewModel() {
     private var loadMoreProductsJob: Job? = null
+    private var loadProductsJob: Job? = null
+    private var loadMoreAfterLoadCompletes = false
 
     private val _viewState = MutableStateFlow<WooPosProductsViewState>(
         WooPosProductsViewState.Loading()
@@ -113,7 +115,9 @@ class WooPosProductsViewModel @Inject constructor(
         forceRefreshProducts: Boolean,
         withPullToRefresh: Boolean,
     ) {
-        viewModelScope.launch {
+        loadProductsJob?.cancel()
+        loadMoreProductsJob?.cancel()
+        loadProductsJob = viewModelScope.launch {
             _viewState.value = if (withPullToRefresh) {
                 buildProductsReloadingState()
             } else {
@@ -157,12 +161,25 @@ class WooPosProductsViewModel @Inject constructor(
 
                             else -> WooPosProductsViewState.Error()
                         }
+
+                        if (loadMoreAfterLoadCompletes) {
+                            queueLoadMoreAfterLoadCompletes()
+                        }
                     }
                 }
             }
         }
     }
 
+    private fun queueLoadMoreAfterLoadCompletes() {
+        loadProductsJob?.invokeOnCompletion { throwable ->
+            if (throwable == null && _viewState.value is WooPosProductsViewState.Content) {
+                loadMoreAfterLoadCompletes = false
+                onEndOfProductsListReached()
+            }
+        }
+    }
+
     private fun buildProductsReloadingState() =
         when (val state = viewState.value) {
             is WooPosProductsViewState.Content -> state.copy(pullToRefreshState = WooPosPullToRefreshState.Refreshing)
@@ -199,12 +216,23 @@ class WooPosProductsViewModel @Inject constructor(
         }
     }
 
+    @Suppress("ReturnCount")
     private fun onEndOfProductsListReached() {
         val currentState = _viewState.value
         if (currentState !is WooPosProductsViewState.Content) {
             return
         }
 
+        if (loadProductsJob?.isActive == true) {
+            loadMoreAfterLoadCompletes = true
+            _viewState.value = currentState.copy(paginationState = WooPosPaginationState.Loading)
+            return
+        }
+
+        if (loadMoreProductsJob?.isActive == true) {
+            return
+        }
+
         if (!productsDataSource.hasMorePages) {
             return
         }

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/WooPosProductsDataSourceTest.kt

@@ -91,7 +91,7 @@ class WooPosProductsDataSourceTest {
     private val productsTypesFilterConfig = WooPosProductsTypesFilterConfig()
 
     @Test
-    fun `given force refresh, when loadProducts called, then should clear cache`() = runTest {
+    fun `given force refresh, when loadProducts called, then should clear cache and insert products again`() = runTest {
         // GIVEN
         whenever(productsIndex.getProductList()).thenReturn(emptyList(), sampleProducts)
 
@@ -117,14 +117,13 @@ class WooPosProductsDataSourceTest {
         sut.loadProducts(forceRefreshProducts = true).first()
 
         // THEN
-        verify(productsCache).clear()
+        verify(productsCache).setAll(any())
         verify(productsIndex).clearCache()
     }
 
     @Test
     fun `given cached products, when loadProducts called, then should emit cached products first`() = runTest {
         // GIVEN
-
         whenever(productsCache.getAll()).thenReturn(sampleProducts)
         whenever(
             productStore.fetchProducts(
@@ -154,6 +153,36 @@ class WooPosProductsDataSourceTest {
         assertThat(cachedResult.products).containsExactlyElementsOf(sampleProducts)
     }
 
+    @Test
+    fun `given cached products, when loadProducts called with forceRefresh, then should not emit cached products`() = runTest {
+        // GIVEN
+        whenever(productsCache.getAll()).thenReturn(sampleProducts)
+        whenever(
+            productStore.fetchProducts(
+                site = eq(siteModel),
+                offset = any(),
+                pageSize = any(),
+                sortType = any(),
+                filterOptions = any(),
+                includeTypes = any()
+            )
+        ).thenReturn(WooResult(listOf<WCProductModel>()))
+        whenever(productsIndex.getProductList()).thenReturn(sampleProducts)
+        val sut = WooPosProductsDataSource(
+            productStore,
+            selectedSite,
+            productsCache,
+            productsIndex,
+            productsTypesFilterConfig
+        )
+
+        // WHEN
+        val result = sut.loadProducts(forceRefreshProducts = true).first()
+
+        // THEN
+        assertThat(result).isInstanceOf(WooPosProductsDataSource.ProductsResult.Remote::class.java)
+    }
+
     @Test
     fun `given no products in list cache, when loadProducts called, then should return empty list`() = runTest {
         // GIVEN

WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/home/items/products/WooPosProductsViewModelTest.kt

@@ -16,14 +16,19 @@ import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsEvent.Eve
 import com.woocommerce.android.ui.woopos.util.analytics.WooPosAnalyticsTracker
 import com.woocommerce.android.ui.woopos.util.format.WooPosFormatPrice
 import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.flow.MutableSharedFlow
+import kotlinx.coroutines.flow.flow
 import kotlinx.coroutines.flow.flowOf
+import kotlinx.coroutines.test.advanceUntilIdle
 import kotlinx.coroutines.test.runTest
 import org.assertj.core.api.Assertions.assertThat
 import org.junit.Before
 import org.junit.Rule
 import org.junit.Test
 import org.mockito.kotlin.any
 import org.mockito.kotlin.mock
+import org.mockito.kotlin.never
+import org.mockito.kotlin.times
 import org.mockito.kotlin.verify
 import org.mockito.kotlin.whenever
 import java.math.BigDecimal
@@ -384,6 +389,79 @@ class WooPosProductsViewModelTest {
             }
         }
 
+    @Test
+    @OptIn(ExperimentalCoroutinesApi::class)
+    fun `given initial load in progress, when end of list reached, then pagination state set to loading and queue load more`() = runTest {
+        // GIVEN
+        whenever(productsDataSource.hasMorePages).thenReturn(true)
+        whenever(productsDataSource.loadMore()).thenReturn(
+            Result.success(
+                listOf(
+                    ProductTestUtils.generateProduct(
+                        productId = 2,
+                        productName = "Product 2",
+                        amount = "20.0",
+                        productType = "simple"
+                    )
+                )
+            )
+        )
+
+        val productsFlow = flow {
+            emit(
+                WooPosProductsDataSource.ProductsResult.Remote(
+                    Result.success(
+                        listOf(
+                            ProductTestUtils.generateProduct(
+                                productId = 1,
+                                productName = "Product 1",
+                                amount = "10.0",
+                                productType = "simple"
+                            )
+                        )
+                    )
+                )
+            )
+        }
+
+        whenever(productsDataSource.loadProducts(any())).thenReturn(productsFlow)
+
+        val viewModel = createViewModel()
+
+        // WHEN
+        viewModel.onUIEvent(WooPosProductsUIEvent.EndOfItemsListReached)
+        advanceUntilIdle()
+
+        // THEN
+        verify(productsDataSource, times(1)).loadMore()
+    }
+
+    @Test
+    @OptIn(ExperimentalCoroutinesApi::class)
+    fun `given load more queued, when initial load fails, then load more is not triggered`() = runTest {
+        // GIVEN
+        whenever(productsDataSource.hasMorePages).thenReturn(true)
+
+        val productsFlow = MutableSharedFlow<WooPosProductsDataSource.ProductsResult>()
+        whenever(productsDataSource.loadProducts(any())).thenReturn(productsFlow)
+
+        val viewModel = createViewModel()
+
+        // WHEN
+        viewModel.onUIEvent(WooPosProductsUIEvent.EndOfItemsListReached)
+
+        // Then
+        productsFlow.emit(
+            WooPosProductsDataSource.ProductsResult.Remote(
+                Result.failure(Exception("Test error"))
+            )
+        )
+        advanceUntilIdle()
+
+        // THEN
+        verify(productsDataSource, never()).loadMore()
+    }
+
     private fun createViewModel(): WooPosProductsViewModel {
         return WooPosProductsViewModel(
             productsDataSource = productsDataSource,

version.properties

@@ -1,2 +1,2 @@
-versionName=22.2-rc-1
-versionCode=661
+versionName=22.2-rc-2
+versionCode=662