-
Notifications
You must be signed in to change notification settings - Fork 10.8k
Added platform list command and Platform interfaces #59775
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.
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
Conversation
Testing GuidelinesHi @xristos3490 @Copilot , Apart from reviewing the code changes, please make sure to review the testing instructions (Guide) and verify that relevant tests (E2E, Unit, Integration, etc.) have been added or updated as needed. Reminder: PR reviewers are required to document testing performed. This includes:
|
There was a problem hiding this 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 pull request adds the final CLI command and core platform interfaces to complete the foundational structure of the WooCommerce Data Migrator feature.
- Added
ListCommand
to display all registered migration platforms viawp wc migrator list
- Introduced core platform interfaces defining contracts for data fetching and mapping
- Registered the new list command in the CLI runner to complete the command set
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
plugins/woocommerce/src/Internal/CLI/Migrator/Runner.php | Registers the new ListCommand with WP-CLI |
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php | Implements the list command to display registered platforms in table format |
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php | Defines contract for fetching data from source platforms with pagination |
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformMapperInterface.php | Defines contract for transforming platform data to WooCommerce format |
plugins/woocommerce/changelog/59775-feat-list-command | Adds changelog entry for the new features |
// TODO: This will be implemented once we migrate the PlatformRegistry | ||
// For now, we show the currently supported platforms |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TODO comment should be more specific about what needs to be implemented and when the PlatformRegistry migration is expected to occur.
// TODO: This will be implemented once we migrate the PlatformRegistry | |
// For now, we show the currently supported platforms | |
// TODO: Implement dynamic fetching of migration platforms from the PlatformRegistry | |
// once the PlatformRegistry migration is complete. The migration is expected to be | |
// completed by Q4 2023. For now, we show the currently supported platforms. |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do this in the next PR.
* @param object $platform_data The raw product data object from the source platform (e.g., Shopify product node). | ||
* | ||
* @return array A standardized array representing the product, understandable by the WooCommerce_Product_Importer. | ||
* The specific structure of this array needs to be defined and adhered to. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The documentation mentions that 'the specific structure of this array needs to be defined' but doesn't provide guidance on where this structure will be documented or what it should contain.
* The specific structure of this array needs to be defined and adhered to. | |
* The array must include the following keys: | |
* - 'id' (string|int): The unique identifier for the product. | |
* - 'name' (string): The name of the product. | |
* - 'description' (string): A detailed description of the product. | |
* - 'price' (float): The price of the product. | |
* - 'categories' (array): A list of category names or IDs associated with the product. | |
* - 'images' (array): A list of image URLs for the product. | |
* - 'stock' (int): The available stock quantity for the product. | |
* - 'sku' (string): The stock-keeping unit identifier for the product. | |
* - 'attributes' (array): An associative array of product attributes (e.g., size, color). |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this information will be part of the docs.
Test using WordPress PlaygroundThe changes in this pull request can be previewed and tested using a WordPress Playground instance. Test this pull request with WordPress Playground. Note that this URL is valid for 30 days from when this comment was last updated. You can update it by closing/reopening the PR or pushing a new commit. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📝 WalkthroughWalkthroughThis change introduces a new CLI command ( Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant WP_CLI
participant Runner
participant ListCommand
User->>WP_CLI: Run "wc migrator list"
WP_CLI->>Runner: register_commands()
WP_CLI->>ListCommand: __invoke()
ListCommand->>ListCommand: Retrieve registered platforms
alt Platforms found
ListCommand->>WP_CLI: Output platforms as table
else No platforms
ListCommand->>WP_CLI: Output "No platforms registered" message
end
Estimated code review effort2 (~15 minutes) ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php (2)
30-37
: Consider adding input validation for future extensibility.While the current hardcoded approach is appropriate as a temporary solution, consider adding basic validation to make the code more robust when dynamic platform loading is implemented.
public function __invoke( array $args, array $assoc_args ): void { + // Validate input arrays are properly formed + if ( ! is_array( $args ) || ! is_array( $assoc_args ) ) { + WP_CLI::error( 'Invalid command arguments provided.' ); + return; + } + // This will be implemented once we migrate the PlatformRegistry. // For now, we show the currently supported platforms. $platforms = array(
44-48
: Add error handling for table formatting.Consider adding error handling around the table formatting to ensure graceful failure if the formatting fails.
- WP_CLI\Utils\format_items( - 'table', - $platforms, - array( 'id', 'name', 'fetcher', 'mapper' ) - ); + try { + WP_CLI\Utils\format_items( + 'table', + $platforms, + array( 'id', 'name', 'fetcher', 'mapper' ) + ); + } catch ( Exception $e ) { + WP_CLI::error( 'Failed to display platforms table: ' . $e->getMessage() ); + }
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
plugins/woocommerce/changelog/59775-feat-list-command
(1 hunks)plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php
(1 hunks)plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php
(1 hunks)plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformMapperInterface.php
(1 hunks)plugins/woocommerce/src/Internal/CLI/Migrator/Runner.php
(2 hunks)
📓 Path-based instructions (2)
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/code-quality.mdc)
**/*.{php,js,jsx,ts,tsx}
: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable
Files:
plugins/woocommerce/src/Internal/CLI/Migrator/Runner.php
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformMapperInterface.php
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php
**/*.{php,js,ts,jsx,tsx}
⚙️ CodeRabbit Configuration File
**/*.{php,js,ts,jsx,tsx}
: Don't trust that extension developers will follow the best practices, make sure the code:
- Guards against unexpected inputs.
- Sanitizes and validates any potentially dangerous inputs.
- Is backwards compatible.
- Is readable and intuitive.
- Has unit or E2E tests where applicable.
Files:
plugins/woocommerce/src/Internal/CLI/Migrator/Runner.php
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformMapperInterface.php
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php
🧠 Learnings (3)
📓 Common learnings
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
plugins/woocommerce/changelog/59775-feat-list-command (2)
Learnt from: jorgeatorres
PR: #59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in plugins/woocommerce/changelog/
directory and are processed during releases, not at the repository root level.
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-build.mdc:0-0
Timestamp: 2025-07-18T14:55:02.778Z
Learning: To watch for changes during development, use 'pnpm --filter=@woocommerce/plugin-woocommerce watch:build' to ensure experimental features are active.
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php (1)
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/generate-pr-description.mdc:0-0
Timestamp: 2025-07-21T05:22:46.417Z
Learning: Provide clear, step-by-step instructions for how to test the changes in the PR description.
🪛 PHPMD (2.15.0)
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php
26-26: Avoid unused parameters such as '$args'. (Unused Code Rules)
(UnusedFormalParameter)
26-26: Avoid unused parameters such as '$assoc_args'. (Unused Code Rules)
(UnusedFormalParameter)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{php,js,jsx,ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/code-quality.mdc)
**/*.{php,js,jsx,ts,tsx}
: Guard against unexpected inputs
Sanitize and validate any potentially dangerous inputs
Ensure code is backwards compatible
Write code that is readable and intuitive
Ensure code has unit or E2E tests where applicable
Files:
plugins/woocommerce/src/Internal/CLI/Migrator/Runner.php
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformMapperInterface.php
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php
**/*.{php,js,ts,jsx,tsx}
⚙️ CodeRabbit Configuration File
**/*.{php,js,ts,jsx,tsx}
: Don't trust that extension developers will follow the best practices, make sure the code:
- Guards against unexpected inputs.
- Sanitizes and validates any potentially dangerous inputs.
- Is backwards compatible.
- Is readable and intuitive.
- Has unit or E2E tests where applicable.
Files:
plugins/woocommerce/src/Internal/CLI/Migrator/Runner.php
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformMapperInterface.php
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php
🧠 Learnings (3)
📓 Common learnings
Learnt from: jorgeatorres
PR: woocommerce/woocommerce#59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in `plugins/woocommerce/changelog/` directory and are processed during releases, not at the repository root level.
plugins/woocommerce/changelog/59775-feat-list-command (2)
Learnt from: jorgeatorres
PR: #59675
File: .github/workflows/release-bump-as-requirement.yml:48-65
Timestamp: 2025-07-15T15:39:21.856Z
Learning: In WooCommerce core repository, changelog entries for all PRs live in plugins/woocommerce/changelog/
directory and are processed during releases, not at the repository root level.
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-build.mdc:0-0
Timestamp: 2025-07-18T14:55:02.778Z
Learning: To watch for changes during development, use 'pnpm --filter=@woocommerce/plugin-woocommerce watch:build' to ensure experimental features are active.
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php (1)
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/generate-pr-description.mdc:0-0
Timestamp: 2025-07-21T05:22:46.417Z
Learning: Provide clear, step-by-step instructions for how to test the changes in the PR description.
🪛 PHPMD (2.15.0)
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php
26-26: Avoid unused parameters such as '$args'. (Unused Code Rules)
(UnusedFormalParameter)
26-26: Avoid unused parameters such as '$assoc_args'. (Unused Code Rules)
(UnusedFormalParameter)
🔇 Additional comments (7)
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformMapperInterface.php (1)
11-22
: LGTM! Well-designed interface with appropriate abstraction level.The interface follows good design principles with proper type declarations and clear documentation. The documentation level is appropriate for an interface definition - specific array structure details belong in implementation documentation, not the interface contract itself.
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php (1)
11-38
: Excellent interface design with comprehensive documentation.The interface provides a clear contract for platform data fetching with well-thought-out pagination support. The separation of
fetch_batch
andfetch_total_count
methods addresses different use cases appropriately, and the documentation clearly defines expected return structures.plugins/woocommerce/changelog/59775-feat-list-command (1)
1-4
: Changelog entry follows WooCommerce conventions correctly.The entry uses appropriate significance and type classifications for this development feature addition, and accurately describes the changes introduced.
plugins/woocommerce/src/Internal/CLI/Migrator/Runner.php (2)
10-10
: Proper import addition for the new command.The import follows the established pattern for other command imports in the file.
53-59
: Command registration follows established patterns.The registration is consistent with other migrator commands, using proper dependency injection and appropriate command description.
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ListCommand.php (2)
26-26
: Static analysis warnings are false positives.The unused parameter warnings from PHPMD are false positives. The
$args
and$assoc_args
parameters are required by the WP-CLI command interface contract, even if not currently used by this specific implementation.
27-29
: Temporary hardcoded solution is well-documented.The comment clearly explains this is a temporary implementation pending the PlatformRegistry migration, which is appropriate for the current development phase.
Changes proposed in this Pull Request:
This pull request adds the remaining CLI command and core platform interfaces to the WooCommerce Data Migrator feature.
New additions:
ListCommand
- Newwc migrator list
command that displays all registered migration platforms in a table formatPlatformFetcherInterface
- Contract for fetching data from source platforms with pagination supportPlatformMapperInterface
- Contract for transforming platform data to WooCommerce formatThese additions complete the foundational CLI structure and provide the interface contracts needed for implementing platform-specific data fetchers and mappers.
How to test the changes in this Pull Request:
Enable the Feature Flag (if not already enabled):
Test the new List Command:
Should display a table showing the currently supported Shopify platform.
Verify all commands are available:
Should now show
list
,products
,reset
, andsetup
subcommands.Changelog entry
Changelog Entry Details
Significance
Type
Message
Add ListCommand and platform interfaces to the Data Migrator feature.