Skip to content

Conversation

@malinajirka
Copy link
Contributor

@malinajirka malinajirka commented Oct 27, 2025

#WOOMOB-1589

Do not merge label - target branch must be trunk.

Description

Replace fetching by offset with fetching by pages to bring /products endpoint to consistency with /variations endpoint.

Test Steps

Smoke test local catalog - verify all products are fetched during full sync.

Images/gif

N/A

  • I have considered if this change warrants release notes and have added them to RELEASE-NOTES.txt if necessary. Use the "[Internal]" label for non-user-facing changes.

…roducts-disappear-after' into issue/woomob-1589-woo-poslocal-catalog-use-pages-instead-of-offset-for

# Conflicts:
#	WooCommerce/src/test/kotlin/com/woocommerce/android/ui/woopos/localcatalog/WooPosSyncProductsActionTest.kt
#	libs/fluxc-plugin/src/main/kotlin/org/wordpress/android/fluxc/store/pos/localcatalog/WooPosLocalCatalogStore.kt
…roducts-disappear-after' into issue/woomob-1589-woo-poslocal-catalog-use-pages-instead-of-offset-for
@malinajirka malinajirka added this to the 23.6 milestone Oct 27, 2025
@malinajirka malinajirka requested a review from Copilot October 27, 2025 13:53
@malinajirka malinajirka added feature: POS status: do not merge Dependent on another PR, ready for review but not ready for merge. labels Oct 27, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR migrates the /products endpoint from offset-based pagination to page-based pagination to align with the existing /variations endpoint implementation.

Key Changes:

  • Replaced offset-based pagination parameters with page-based parameters throughout the products fetching flow
  • Updated API client to use page numbers instead of offsets
  • Consolidated pagination parameter building by reusing buildBaseParams for variations

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
WooPosProductRestClient.kt Updated fetchProducts to use page parameter instead of offset; refactored fetchVariations to use shared buildBaseParams method
WooPosLocalCatalogStore.kt Changed pagination from offset to page-based in fetchRecentlyModifiedProducts; updated next page calculation logic
WooPosLocalCatalogFetchProductsResult.kt Renamed nextOffset field to nextPage
WooPosSyncProductsAction.kt Updated pagination tracking from currentOffset to currentPage
WooPosLocalCatalogStoreTest.kt Updated all test expectations to use page-based pagination
WooPosSyncProductsActionTest.kt Updated test mocks and assertions to use page-based pagination

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 27, 2025

📲 You can test the changes from this Pull Request in WooCommerce-Wear Android by scanning the QR code below to install the corresponding build.
App NameWooCommerce-Wear Android
Platform⌚️ Wear OS
FlavorJalapeno
Build TypeDebug
Commit229638e
Direct Downloadwoocommerce-wear-prototype-build-pr14830-229638e.apk

@malinajirka malinajirka marked this pull request as ready for review October 27, 2025 14:19
@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 27, 2025

📲 You can test the changes from this Pull Request in WooCommerce Android by scanning the QR code below to install the corresponding build.

App NameWooCommerce Android
Platform📱 Mobile
FlavorJalapeno
Build TypeDebug
Commit229638e
Direct Downloadwoocommerce-prototype-build-pr14830-229638e.apk

Base automatically changed from issue/woomob-1551-woo-poslocal-catalog-ensure-trashed-products-disappear-after to trunk October 27, 2025 16:09
@samiuelson samiuelson self-assigned this Oct 28, 2025
Copy link
Contributor

@samiuelson samiuelson left a comment

Choose a reason for hiding this comment

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

LGTM!

@samiuelson samiuelson removed the status: do not merge Dependent on another PR, ready for review but not ready for merge. label Oct 28, 2025
@samiuelson samiuelson enabled auto-merge October 28, 2025 09:56
@samiuelson samiuelson merged commit 43baf3b into trunk Oct 28, 2025
17 checks passed
@samiuelson samiuelson deleted the issue/woomob-1589-woo-poslocal-catalog-use-pages-instead-of-offset-for branch October 28, 2025 10:31
@codecov-commenter
Copy link

Codecov Report

❌ Patch coverage is 66.66667% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 38.27%. Comparing base (c734c98) to head (229638e).
⚠️ Report is 12 commits behind head on trunk.

Files with missing lines Patch % Lines
...st/wpcom/wc/product/pos/WooPosProductRestClient.kt 0.00% 3 Missing ⚠️
.../store/pos/localcatalog/WooPosLocalCatalogStore.kt 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff            @@
##              trunk   #14830   +/-   ##
=========================================
  Coverage     38.26%   38.27%           
- Complexity    10100    10101    +1     
=========================================
  Files          2131     2131           
  Lines        120600   120593    -7     
  Branches      16529    16528    -1     
=========================================
+ Hits          46152    46154    +2     
+ Misses        69771    69764    -7     
+ Partials       4677     4675    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants