Skip to content

Conversation

@malinajirka
Copy link
Contributor

Do not merge label - target branch needs to be trunk.

Description

This PR is just preparation for refactoring the WooPosSyncProductsAction and WooPosSyncVariationsAction into a single action. Since moving and refactoring the classes within a single PR could easily get out of hand and would be difficult to review, this PR simply moves the actions into a single file and a shared class.

Test Steps

No need to test anything - it's just a simple move.

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.

@malinajirka malinajirka added this to the 23.6 milestone Oct 27, 2025
@malinajirka malinajirka added status: do not merge Dependent on another PR, ready for review but not ready for merge. feature: POS labels Oct 27, 2025
@malinajirka malinajirka requested a review from Copilot October 27, 2025 15:13
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 consolidates the WooPosSyncProductsAction and WooPosSyncVariationsAction classes into a single file by introducing a new WooPosSyncAction wrapper class. The wrapper delegates to the existing actions, which have been moved from separate files into the same file.

  • The WooPosSyncVariationsAction class is moved from its own file into WooPosSyncAction.kt
  • A new WooPosSyncAction wrapper class is introduced to provide syncProducts() and syncVariations() methods
  • All references to the separate action classes are updated to use the new wrapper

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
WooPosLocalCatalogSyncRepositoryTest.kt Updates test mocks and method calls to use the new WooPosSyncAction wrapper class
WooPosSyncVariationsAction.kt Deleted file - variations action moved to WooPosSyncAction.kt
WooPosSyncAction.kt Adds wrapper class and consolidates both products and variations actions into single file
WooPosLocalCatalogSyncRepository.kt Updates constructor and method calls to use the new wrapper class

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

maxPages = maxPages
)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

📓 All the code within this file below this line was copy-pasted from the separate files.

@wpmobilebot
Copy link
Collaborator

📲 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
Commit79635b3
Direct Downloadwoocommerce-wear-prototype-build-pr14831-79635b3.apk

@wpmobilebot
Copy link
Collaborator

📲 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
Commit79635b3
Direct Downloadwoocommerce-prototype-build-pr14831-79635b3.apk

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!

Base automatically changed from issue/woomob-1589-woo-poslocal-catalog-use-pages-instead-of-offset-for to trunk October 28, 2025 10:31
@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 15:00
@samiuelson samiuelson merged commit 33dfdb4 into trunk Oct 28, 2025
22 of 24 checks passed
@samiuelson samiuelson deleted the issue/woomob-1334-woo-poslocal-catalog-run-both-products-and-variations-within-v2 branch October 28, 2025 15:02
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.

4 participants