Skip to content

[WOOMOB-1589] Replace fetching by offset with fetching by page for /products endpoint #14830

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

Merged
merged 7 commits into from
Oct 28, 2025
Merged

[WOOMOB-1589] Replace fetching by offset with fetching by page for /products endpoint #14830

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cc93810
Use pages instead of offset when fetching products
malinajirka Oct 27, 2025
0eb64b5
Use pages instead of offset when fetching products
malinajirka Oct 27, 2025
bad5ac2
Merge branch 'issue/woomob-1551-woo-poslocal-catalog-ensure-trashed-p…
malinajirka Oct 27, 2025
6441161
Merge branch 'issue/woomob-1551-woo-poslocal-catalog-ensure-trashed-p…
malinajirka Oct 27, 2025
4dac5fc
Fix fetch trashed products action
malinajirka Oct 27, 2025
41a400d
Fix tests
malinajirka Oct 27, 2025
229638e
Merge branch 'trunk' into issue/woomob-1589-woo-poslocal-catalog-use-…
samiuelson Oct 28, 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
6 changes: 3 additions & 3 deletions ...rc/main/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosSyncProductsAction.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ class WooPosSyncProductsAction @Inject constructor(
pageSize: Int,
maxPages: Int
): Pair<List<WooPosProductEntity>, ServerDate> {
var currentOffset = 0
var currentPage = 1
var pagesSynced = 0
var totalPages = maxPages
var firstPageServerDate: String? = null
Expand All @@ -105,7 +105,7 @@ class WooPosSyncProductsAction @Inject constructor(
site = site,
pageSize = pageSize,
modifiedAfterGmt = modifiedAfterGmt,
offset = currentOffset,
page = currentPage,
)

result.fold(
Expand All @@ -121,7 +121,7 @@ class WooPosSyncProductsAction @Inject constructor(
if (!syncResult.hasMore || syncResult.syncedCount == 0) {
logger.d("Local Catalog: No more products to sync")
} else {
currentOffset = syncResult.nextOffset
currentPage = syncResult.nextPage
}
},
onFailure = { error ->
Expand Down
36 changes: 13 additions & 23 deletions ...est/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosSyncProductsActionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,10 @@ class WooPosSyncProductsActionTest {
val mockProducts = createMockProducts(1, productsCount)

mockFetchRecentlyModifiedProductsSuccess(
offset = 0,
page = 1,
mockProducts = mockProducts,
syncedCount = productsCount,
hasMore = false,
nextOffset = 0,
totalPages = 1,
)
}
Expand All @@ -448,40 +447,36 @@ class WooPosSyncProductsActionTest {
)

mockFetchRecentlyModifiedProductsSuccess(
offset = 0,
page = 1,
mockProducts = mockPage1Products,
syncedCount = page1Count,
hasMore = true,
nextOffset = page1Count,
totalPages = totalPages,
)

mockFetchRecentlyModifiedProductsSuccess(
offset = page1Count,
page = 2,
mockProducts = mockPage2Products,
syncedCount = page2Count,
hasMore = true,
nextOffset = page1Count + page2Count,
totalPages = totalPages,
)

mockFetchRecentlyModifiedProductsSuccess(
offset = page1Count + page2Count,
page = 3,
mockProducts = mockPage3Products,
syncedCount = page3Count,
hasMore = false,
nextOffset = 0,
totalPages = totalPages,
)
}

private suspend fun givenEmptyCatalog() {
mockFetchRecentlyModifiedProductsSuccess(
offset = 0,
page = 1,
mockProducts = emptyList(),
syncedCount = 0,
hasMore = false,
nextOffset = 0,
totalPages = 1,
)
}
Expand All @@ -500,36 +495,33 @@ class WooPosSyncProductsActionTest {
val mockPage1Products = createMockProducts(1, 100)

mockFetchRecentlyModifiedProductsSuccess(
offset = 0,
page = 1,
mockProducts = mockPage1Products,
syncedCount = 100,
hasMore = true,
nextOffset = 100,
totalPages = 2,
)

whenever(posLocalCatalogStore.fetchRecentlyModifiedProducts(any(), anyOrNull(), eq(100), any(), eq(null)))
whenever(posLocalCatalogStore.fetchRecentlyModifiedProducts(any(), anyOrNull(), eq(2), any(), eq(null)))
.thenReturn(KotlinResult.failure(Exception(errorMessage)))
}

private suspend fun givenPageWithZeroProductsButHasMore() {
val mockPage2Products = createMockProducts(1, 50)

mockFetchRecentlyModifiedProductsSuccess(
offset = 0,
page = 1,
mockProducts = emptyList(),
syncedCount = 0,
hasMore = true,
nextOffset = 100,
totalPages = 2,
)

mockFetchRecentlyModifiedProductsSuccess(
offset = 100,
page = 2,
mockProducts = mockPage2Products,
syncedCount = 50,
hasMore = true,
nextOffset = 0,
totalPages = 2,
)
}
Expand All @@ -538,29 +530,27 @@ class WooPosSyncProductsActionTest {
val mockProducts = createMockProducts(1, PAGE_SIZE)

mockFetchRecentlyModifiedProductsSuccess(
offset = 0,
page = 1,
mockProducts,
totalPages,
nextOffset = PAGE_SIZE,
syncedCount = PAGE_SIZE,
hasMore = true
)
}

@Suppress("LongParameterList")
private suspend fun mockFetchRecentlyModifiedProductsSuccess(
offset: Int,
page: Int,
mockProducts: List<WooPosProductEntity>,
totalPages: Int,
nextOffset: Int,
syncedCount: Int,
hasMore: Boolean
) {
whenever(
posLocalCatalogStore.fetchRecentlyModifiedProducts(
site = any(),
modifiedAfterGmt = anyOrNull(),
offset = eq(offset),
page = eq(page),
pageSize = any(),
includeStatus = eq(null)
)
Expand All @@ -571,7 +561,7 @@ class WooPosSyncProductsActionTest {
products = mockProducts,
syncedCount = syncedCount,
hasMore = hasMore,
nextOffset = nextOffset,
nextPage = page + 1,
totalPages = totalPages,
serverDate = ""
)
Expand Down
18 changes: 5 additions & 13 deletions .../org/wordpress/android/fluxc/network/rest/wpcom/wc/product/pos/WooPosProductRestClient.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ class WooPosProductRestClient @Inject constructor(
suspend fun fetchProducts(
site: SiteModel,
modifiedAfter: String? = null,
offset: Int,
page: Int,
pageSize: Int,
includeStatus: List<CoreProductStatus>? = null,
): WooResult<Array<ProductApiResponse>> {
val url = WOOCOMMERCE.products.pathV3
val params = buildBaseParams(
pageSize = pageSize,
offset = offset,
page = page,
modifiedAfter = modifiedAfter,
fields = PRODUCT_FIELDS,
includeStatus = includeStatus
Expand Down Expand Up @@ -68,15 +68,7 @@ class WooPosProductRestClient @Inject constructor(
pageSize: Int,
): WooResult<Array<WooPosVariationApiResponse>> {
val url = WOOCOMMERCE.variations.pathV3
val params = mutableMapOf(
"per_page" to pageSize.toString(),
"page" to page.toString(),
"_fields" to VARIATIONS_FIELDS,
).also {
if (modifiedAfter.isNullOrBlank().not()) {
it["modified_after"] = modifiedAfter
}
}
val params = buildBaseParams(pageSize, page, VARIATIONS_FIELDS, modifiedAfter)

val response = wooNetwork.executeGetGsonRequest(
site = site,
Expand Down Expand Up @@ -151,14 +143,14 @@ class WooPosProductRestClient @Inject constructor(

private fun buildBaseParams(
pageSize: Int,
offset: Int,
page: Int,
fields: String,
modifiedAfter: String?,
includeStatus: List<CoreProductStatus>? = null,
): MutableMap<String, String> {
return mutableMapOf(
"per_page" to pageSize.toString(),
"offset" to offset.toString(),
"page" to page.toString(),
"_fields" to fields,
).also {
modifiedAfter?.let { modified ->
Expand Down
2 changes: 1 addition & 1 deletion ...g/wordpress/android/fluxc/store/pos/localcatalog/WooPosLocalCatalogFetchProductsResult.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ data class WooPosLocalCatalogFetchProductsResult(
val products: List<WooPosProductEntity>,
val syncedCount: Int,
val hasMore: Boolean,
val nextOffset: Int,
val nextPage: Int,
val totalPages: Int,
val serverDate: String,
)
12 changes: 6 additions & 6 deletions ...main/kotlin/org/wordpress/android/fluxc/store/pos/localcatalog/WooPosLocalCatalogStore.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ class WooPosLocalCatalogStore @Inject constructor(
val response = posProductRestClient.fetchProducts(
site = site,
modifiedAfter = modifiedAfterGmt,
offset = 0,
page = 1,
pageSize = 1,
includeStatus = null
)
Expand Down Expand Up @@ -153,14 +153,14 @@ class WooPosLocalCatalogStore @Inject constructor(
*
* @param [site] The site to sync products for
* @param [modifiedAfterGmt] ISO 8601 formatted date string (GMT)
* @param [offset] Starting offset for pagination
* @param [page] Page for pagination.
* @param [pageSize] Number of products to fetch per page (default: 100, max: 100)
* @return Result containing SyncResponse with pagination info or error
*/
suspend fun fetchRecentlyModifiedProducts(
site: SiteModel,
modifiedAfterGmt: String?,
offset: Int = 0,
page: Int = 1,
pageSize: Int = DEFAULT_PAGE_SIZE,
includeStatus: List<CoreProductStatus>? = null,
): Result<WooPosLocalCatalogFetchProductsResult> =
Expand All @@ -170,7 +170,7 @@ class WooPosLocalCatalogStore @Inject constructor(
val response = posProductRestClient.fetchProducts(
site = site,
modifiedAfter = modifiedAfterGmt,
offset = offset,
page = page,
pageSize = validPageSize,
includeStatus = includeStatus
)
Expand All @@ -189,7 +189,7 @@ class WooPosLocalCatalogStore @Inject constructor(
products = emptyList(),
syncedCount = 0,
hasMore = false,
nextOffset = offset,
nextPage = page,
totalPages = 0,
serverDate = serverDate
)
Expand All @@ -215,7 +215,7 @@ class WooPosLocalCatalogStore @Inject constructor(
products = products,
syncedCount = products.size,
hasMore = hasMore,
nextOffset = if (hasMore) offset + products.size else offset,
nextPage = if (hasMore) page + 1 else page,
totalPages = totalPages,
serverDate = serverDate,
)
Expand Down
24 changes: 12 additions & 12 deletions ...plugin/src/test/java/org/wordpress/android/fluxc/store/pos/WooPosLocalCatalogStoreTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -145,17 +145,17 @@ class WooPosLocalCatalogStoreTest {
createTestApiResponse(id = 1L, name = "Coffee Mug"),
createTestApiResponse(id = 2L, name = "Laptop Stand")
)
whenever(posProductRestClient.fetchProducts(testSite, validDateString, 0, 100))
whenever(posProductRestClient.fetchProducts(testSite, validDateString, 1, 100))
.thenReturn(WooResult(remoteProducts))

// WHEN
val syncResult = store.fetchRecentlyModifiedProducts(testSite, validDateString, 0, 100).getOrThrow()
val syncResult = store.fetchRecentlyModifiedProducts(testSite, validDateString, 1, 100).getOrThrow()

// THEN
assertThat(syncResult.products.size).isEqualTo(remoteProducts.size)
assertThat(syncResult.syncedCount).isEqualTo(2)
assertThat(syncResult.hasMore).isFalse()
assertThat(syncResult.nextOffset).isEqualTo(0)
assertThat(syncResult.nextPage).isEqualTo(1)
}

@Test
Expand All @@ -164,31 +164,31 @@ class WooPosLocalCatalogStoreTest {
val fullPageOfProducts = Array(100) { index ->
createTestApiResponse(id = index.toLong() + 1, name = "Product ${index + 1}")
}
whenever(posProductRestClient.fetchProducts(testSite, validDateString, 0, 100))
whenever(posProductRestClient.fetchProducts(testSite, validDateString, 1, 100))
.thenReturn(WooResult(fullPageOfProducts))

// WHEN
val syncResult = store.fetchRecentlyModifiedProducts(testSite, validDateString, 0, 100).getOrThrow()
val syncResult = store.fetchRecentlyModifiedProducts(testSite, validDateString, 1, 100).getOrThrow()

// THEN
assertThat(syncResult.hasMore).isTrue()
assertThat(syncResult.nextOffset).isEqualTo(100)
assertThat(syncResult.nextPage).isEqualTo(2)
assertThat(syncResult.syncedCount).isEqualTo(100)
}

@Test
fun `given no remote products, when fetching, then completes`() = runTest {
// GIVEN
whenever(posProductRestClient.fetchProducts(testSite, validDateString, 0, 100))
whenever(posProductRestClient.fetchProducts(testSite, validDateString, 1, 100))
.thenReturn(WooResult(emptyArray()))

// WHEN
val syncResult = store.fetchRecentlyModifiedProducts(testSite, validDateString, 0, 100).getOrThrow()
val syncResult = store.fetchRecentlyModifiedProducts(testSite, validDateString, 1, 100).getOrThrow()

// THEN
assertThat(syncResult.syncedCount).isEqualTo(0)
assertThat(syncResult.hasMore).isFalse()
assertThat(syncResult.nextOffset).isEqualTo(0)
assertThat(syncResult.nextPage).isEqualTo(1)
}

@Test
Expand Down Expand Up @@ -233,15 +233,15 @@ class WooPosLocalCatalogStoreTest {
val partialPageProducts = Array(50) { index ->
createTestApiResponse(id = index.toLong() + 1, name = "Product ${index + 1}")
}
whenever(posProductRestClient.fetchProducts(testSite, validDateString, 0, 100))
whenever(posProductRestClient.fetchProducts(testSite, validDateString, 1, 100))
.thenReturn(WooResult(partialPageProducts))

// WHEN
val syncResult = store.fetchRecentlyModifiedProducts(testSite, validDateString, 0, 100).getOrThrow()
val syncResult = store.fetchRecentlyModifiedProducts(testSite, validDateString, 1, 100).getOrThrow()

// THEN
assertThat(syncResult.hasMore).isFalse()
assertThat(syncResult.nextOffset).isEqualTo(0)
assertThat(syncResult.nextPage).isEqualTo(1)
assertThat(syncResult.syncedCount).isEqualTo(50)
}

Expand Down