Shopify GraphQL Product Data Fetching #59941
Conversation
There was a problem hiding this comment.
Fetching overall looks good and retrieves the product information as expected. I did notice an issue with pagination, though. Check this out:
It's a warning like:
Warning: Undefined property: stdClass::$has_next_page in /Users/chris/Repos/woocommerce/plugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcher.php on line 217
and while the shop has over 1000 products, I can't continue the process. I also left a comment on how the optimal process should be below.
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ProductsCommand.php
Show resolved
Hide resolved
|
Thanks for the review. I've fixed the warning here: ff95efc |
There was a problem hiding this comment.
Thanks for fixing the warning and sharing the plans for re-looping the batches, @naman03malhotra!
Did another test and everything works as expected on my end!
📝 WalkthroughWalkthroughThis change introduces a new Shopify product-fetching feature to the WooCommerce CLI Migrator, enabling product retrieval via Shopify's GraphQL API with cursor-based pagination. The CLI command gains Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI_Command
participant PlatformRegistry
participant ShopifyFetcher
participant ShopifyClient
participant ShopifyGraphQL
User->>CLI_Command: Run `products` command with --fetch, --limit, --after
CLI_Command->>PlatformRegistry: get_fetcher('shopify')
PlatformRegistry-->>CLI_Command: ShopifyFetcher
CLI_Command->>ShopifyFetcher: fetch_batch({limit, after})
ShopifyFetcher->>ShopifyClient: graphql_request(query, variables)
ShopifyClient->>ShopifyGraphQL: POST /graphql.json (query, variables)
ShopifyGraphQL-->>ShopifyClient: GraphQL response (data or errors)
ShopifyClient-->>ShopifyFetcher: Decoded data or WP_Error
ShopifyFetcher-->>CLI_Command: {items, end_cursor, has_next_page}
CLI_Command->>User: Output product info and pagination instructions
Estimated code review effort🎯 4 (Complex) | ⏱️ ~35 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (2)**/*.{php,js,jsx,ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/code-quality.mdc)
Files:
**/*.{php,js,ts,jsx,tsx}⚙️ CodeRabbit Configuration File
Files:
🧠 Learnings (2)📚 Learning: test mocks should accurately simulate the behavior of the functions they replace, including return v...Applied to files:
📚 Learning: vladolaru prefers to keep both direct value comparison (===) and serialized string comparison (maybe...Applied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
🔇 Additional comments (4)
✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
plugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClient.php (1)
218-221: Remove unnecessary phpcs:disable commentThe
$variablesparameter is used in thecompact()call on line 221, so the phpcs:disable comment for unused parameters is not needed.- * - * @phpcs:disable Generic.CodeAnalysis.UnusedFunctionParameter.FoundAfterLastUsed */ private function build_graphql_request_args( string $access_token, string $query, array $variables ): array {plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcherTest.php (1)
473-530: Consider making the helper method protected instead of private.While using reflection to test private methods breaks encapsulation, the current approach is acceptable given the importance of testing the GraphQL variable building logic. However, consider making
build_graphql_variablesprotected instead of private to improve testability without reflection.The test coverage itself is excellent, validating null filtering and different parameter combinations.
📜 Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
plugins/woocommerce/changelog/59941-migrator-shopify-product-fetch(1 hunks)plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ProductsCommand.php(3 hunks)plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php(1 hunks)plugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClient.php(3 hunks)plugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcher.php(2 hunks)plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Commands/ProductsCommandTest.php(2 hunks)plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClientTest.php(2 hunks)plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcherTest.php(2 hunks)
🧰 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/Commands/ProductsCommand.phpplugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClientTest.phpplugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.phpplugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcherTest.phpplugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Commands/ProductsCommandTest.phpplugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcher.phpplugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClient.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/Commands/ProductsCommand.phpplugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClientTest.phpplugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.phpplugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcherTest.phpplugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Commands/ProductsCommandTest.phpplugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcher.phpplugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClient.php
🧠 Learnings (3)
📚 Learning: test mocks should accurately simulate the behavior of the functions they replace, including return v...
Learnt from: vladolaru
PR: woocommerce/woocommerce#59160
File: plugins/woocommerce/tests/php/src/Internal/Admin/Settings/PaymentsProviders/WooPayments/WooPaymentsServiceTest.php:360-373
Timestamp: 2025-06-25T15:39:25.166Z
Learning: Test mocks should accurately simulate the behavior of the functions they replace, including return value semantics. For WordPress functions like `update_option`, the mock should properly return `false` when the value hasn't changed and `true` when it has, to avoid masking logic errors in tests.
Applied to files:
plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcherTest.phpplugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Commands/ProductsCommandTest.php
📚 Learning: in woocommerce core repository, changelog entries for all prs live in `plugins/woocommerce/changelog...
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.
Applied to files:
plugins/woocommerce/changelog/59941-migrator-shopify-product-fetch
📚 Learning: run woocommerce phpunit tests for specific files or directories using the command: pnpm run test:php...
Learnt from: CR
PR: woocommerce/woocommerce#0
File: .cursor/rules/woo-phpunit.mdc:0-0
Timestamp: 2025-07-18T14:55:07.629Z
Learning: Run WooCommerce PHPUnit tests for specific files or directories using the command: pnpm run test:php:env {relative_path} --verbose, and the command must be run in the plugins/woocommerce directory.
Applied to files:
plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Commands/ProductsCommandTest.php
🪛 PHPMD (2.15.0)
plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClientTest.php
369-369: Avoid unused parameters such as '$preempt'. (Unused Code Rules)
(UnusedFormalParameter)
415-415: Avoid unused parameters such as '$preempt'. (Unused Code Rules)
(UnusedFormalParameter)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (14)
- GitHub Check: Core e2e tests 6/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Metrics - @woocommerce/plugin-woocommerce [performance]
- GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.2] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Core API tests - @woocommerce/plugin-woocommerce [api]
- GitHub Check: Core e2e tests 4/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 5/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 2/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 1/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: Core e2e tests 3/6 - @woocommerce/plugin-woocommerce [e2e]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 2/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: PHP: 8.4 WP: latest [WP latest] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: PHP: 7.4 WP: latest - 1 [WP 6.7.2] 1/2 - @woocommerce/plugin-woocommerce [unit:php]
- GitHub Check: Lint - @woocommerce/plugin-woocommerce
- GitHub Check: build
🔇 Additional comments (36)
plugins/woocommerce/src/Internal/CLI/Migrator/Interfaces/PlatformFetcherInterface.php (1)
22-22: LGTM!The PHPDoc update from
'hasNextPage'to'has_next_page'correctly aligns the interface documentation with the snake_case naming convention used in the implementation.plugins/woocommerce/changelog/59941-migrator-shopify-product-fetch (1)
1-4: Changelog entry looks good!The changelog correctly documents this feature addition with appropriate significance level and clear description of the GraphQL product fetching functionality.
plugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClient.php (4)
65-84: GraphQL request method implementation looks solid!The method properly handles credentials validation, delegates to appropriate helper methods, and maintains consistency with the existing REST request pattern.
198-208: URL construction is consistent and correct!The GraphQL URL builder follows the same pattern as the REST URL builder, ensuring protocol presence and using the consistent API version.
234-273: Excellent error handling in GraphQL response processing!The method comprehensively handles all error scenarios including HTTP errors, JSON parsing failures, GraphQL-specific errors, and validates the response structure.
2-8: File header addition follows standards!The PHPDoc file header properly documents the file with the correct package namespace.
plugins/woocommerce/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcher.php (3)
25-144: Well-designed comprehensive GraphQL query!The query efficiently fetches all necessary product data including variants, images, collections, and metadata in a single request. Good use of fragments and query variables for flexibility.
168-220: Robust implementation of batch fetching!The method properly handles GraphQL requests, error scenarios, cursor extraction for pagination, and returns the expected array structure. Good defensive programming with validation of response structure.
222-243: Clean GraphQL variable builder!Good implementation with sensible defaults and proper null value filtering to prevent GraphQL issues.
plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Commands/ProductsCommandTest.php (2)
94-205: Comprehensive test coverage for count requests!The tests properly cover success, filtered, and error scenarios for the count functionality. Good use of mocks and appropriate assertions.
207-460: Excellent test coverage for fetch functionality!The tests comprehensively cover all fetch scenarios including success cases, pagination, errors, and empty results. The mock data correctly represents the GraphQL edge/node structure, and assertions properly validate the expected output.
plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ProductsCommand.php (4)
54-72: LGTM!The CLI documentation is clear and comprehensive, covering all new options and providing helpful usage examples.
103-107: LGTM!The fetch request handling follows the same pattern as the existing count handler, maintaining consistency in the codebase.
54-72: LGTM! Clear CLI parameter documentation.The new CLI parameters are well-documented with appropriate defaults and clear examples showing both basic and advanced usage patterns.
103-107: LGTM! Consistent request handling pattern.The fetch request detection follows the same clean pattern as the existing count request handling.
plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyClientTest.php (9)
327-565: Excellent comprehensive test coverage for GraphQL functionality.The new test methods provide thorough coverage of the
graphql_requestmethod, including:
- Successful GraphQL queries with proper request validation
- Variable handling and request body construction
- Error scenarios (HTTP errors, GraphQL errors, invalid JSON)
- Credential validation
- Proper mocking and cleanup
The tests follow established patterns and testing best practices.
369-369: Static analysis false positive - parameters required by WordPress filter signature.The
$preemptparameter is required by WordPress'spre_http_requestfilter signature, even when unused. Removing it would break the callback contract.Also applies to: 415-415
2-45: LGTM! Proper test class structure and setup.The test class follows standard PHPUnit conventions with proper dependency mocking and initialization.
327-394: LGTM! Comprehensive GraphQL success test.This test thoroughly validates the GraphQL request flow including URL construction, headers, request method, and response parsing with appropriate assertions.
396-437: LGTM! Proper GraphQL variables test.The test correctly validates that GraphQL variables are included in the request body. The static analysis warning about unused
$preemptparameter is a false positive - it's required by the WordPress filter callback signature.
439-472: LGTM! Proper HTTP error handling test.The test correctly validates error handling for HTTP-level failures with appropriate error code and message assertions.
474-516: LGTM! Comprehensive GraphQL error handling test.The test properly validates GraphQL-specific error handling with realistic error structures and appropriate assertions.
518-550: LGTM! Proper invalid JSON handling test.The test correctly validates error handling for malformed JSON responses with appropriate error code assertions.
552-565: LGTM! Proper credentials validation test.The test correctly validates error handling when credentials are missing with appropriate error assertions.
plugins/woocommerce/tests/php/src/Internal/CLI/Migrator/Platforms/Shopify/ShopifyFetcherTest.php (12)
230-233: LGTM!The key naming update to snake_case (
has_next_page) aligns with PHP/WordPress conventions and maintains consistency with the interface changes.
284-468: Comprehensive test coverage for GraphQL batch fetching.The new test methods provide excellent coverage of the
fetch_batchfunctionality:
- Success scenarios with proper data validation
- Cursor-based pagination handling
- Error handling and resilience to malformed responses
- Edge cases like empty results
The tests properly mock the GraphQL client and validate both happy path and error scenarios.
532-612: LGTM!The edge case tests provide valuable coverage of boundary conditions:
- Large batch sizes for bulk operations
- Minimum batch size validation
- Proper response handling in both scenarios
These tests ensure the fetcher behaves correctly across the expected range of input values.
223-234: LGTM! Consistent key naming convention.The update to use
has_next_page(snake_case) is consistent with PHP naming conventions and aligns with the interface changes.
284-356: LGTM! Comprehensive batch fetch success test.This test thoroughly validates the GraphQL product fetching functionality with realistic mock data structures, proper request parameter validation, and comprehensive response parsing assertions.
358-403: LGTM! Proper cursor pagination test.The test correctly validates cursor-based pagination with appropriate parameter passing to GraphQL and proper end-of-results handling.
405-421: LGTM! Proper GraphQL error handling test.The test correctly validates error handling in batch fetching with appropriate fallback to empty results and proper error response structure.
423-442: LGTM! Proper invalid response structure test.The test correctly validates defensive programming against unexpected API response structures with appropriate fallback behavior.
444-468: LGTM! Proper empty results test.The test correctly validates handling of empty result sets with appropriate pagination state and response structure.
470-530: LGTM! Comprehensive GraphQL variables helper test.The test appropriately uses reflection to validate the internal variable building logic with comprehensive scenarios including null filtering and parameter mapping.
532-572: LGTM! Proper large limit test.The test correctly validates handling of large batch sizes with appropriate mock data generation and proper request parameter validation.
574-612: LGTM! Proper minimum limit edge case test.The test correctly validates the edge case of minimum batch size (limit=1) with appropriate single-item result handling and assertions.
| private function handle_fetch_request( string $platform, array $assoc_args ): void { | ||
| $limit = (int) ( $assoc_args['limit'] ?? 5 ); | ||
| $cursor = $assoc_args['after'] ?? null; | ||
|
|
||
| WP_CLI::log( "Fetching {$limit} products from {$platform}..." ); | ||
|
|
||
| $fetcher = $this->platform_registry->get_fetcher( $platform ); | ||
| if ( ! $fetcher ) { | ||
| WP_CLI::error( "Could not get fetcher for platform '{$platform}'" ); | ||
| return; | ||
| } | ||
|
|
||
| // Build fetch arguments. | ||
| $fetch_args = array( | ||
| 'limit' => $limit, | ||
| 'after_cursor' => $cursor, | ||
| ); | ||
|
|
||
| $result = $fetcher->fetch_batch( $fetch_args ); | ||
|
|
||
| if ( empty( $result['items'] ) ) { | ||
| WP_CLI::log( 'No products found or unable to fetch products.' ); | ||
| return; | ||
| } | ||
|
|
||
| WP_CLI::success( sprintf( 'Successfully fetched %d products.', count( $result['items'] ) ) ); | ||
|
|
||
| // Display basic product information. | ||
| foreach ( $result['items'] as $item ) { | ||
| $product = $item->node ?? null; | ||
| if ( ! $product ) { | ||
| continue; | ||
| } | ||
|
|
||
| $title = $product->title ?? 'Unknown Title'; | ||
| $id = $product->id ?? 'Unknown ID'; | ||
| $status = $product->status ?? 'Unknown Status'; | ||
| $variants_count = isset( $product->variants->edges ) ? count( $product->variants->edges ) : 0; | ||
|
|
||
| WP_CLI::log( "- {$title} (ID: {$id}, Status: {$status}, Variants: {$variants_count})" ); | ||
| } | ||
|
|
||
| // Display pagination information. | ||
| if ( $result['has_next_page'] && $result['cursor'] ) { | ||
| WP_CLI::log( '' ); | ||
| WP_CLI::log( 'More products available. To fetch next batch, use:' ); | ||
| WP_CLI::log( "wp wc migrate products --fetch --limit={$limit} --after={$result['cursor']}" ); | ||
| } else { | ||
| WP_CLI::log( '' ); | ||
| WP_CLI::log( 'No more products to fetch.' ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add input validation and sanitization for security.
Several security and validation concerns need to be addressed:
- Validate limit parameter bounds - The
(int)cast doesn't prevent negative or excessively large values - Sanitize cursor parameter - User input is used directly without validation
- Add null safety for object property access - Lines 180-188 access object properties without null checks
- Sanitize output - The cursor value is output directly on line 197
Apply these security improvements:
private function handle_fetch_request( string $platform, array $assoc_args ): void {
- $limit = (int) ( $assoc_args['limit'] ?? 5 );
- $cursor = $assoc_args['after'] ?? null;
+ $limit = max( 1, min( 250, (int) ( $assoc_args['limit'] ?? 5 ) ) );
+ $cursor = isset( $assoc_args['after'] ) ? sanitize_text_field( $assoc_args['after'] ) : null;
WP_CLI::log( "Fetching {$limit} products from {$platform}..." );
$fetcher = $this->platform_registry->get_fetcher( $platform );
if ( ! $fetcher ) {
WP_CLI::error( "Could not get fetcher for platform '{$platform}'" );
return;
}
// Build fetch arguments.
$fetch_args = array(
'limit' => $limit,
'after_cursor' => $cursor,
);
$result = $fetcher->fetch_batch( $fetch_args );
if ( empty( $result['items'] ) ) {
WP_CLI::log( 'No products found or unable to fetch products.' );
return;
}
WP_CLI::success( sprintf( 'Successfully fetched %d products.', count( $result['items'] ) ) );
// Display basic product information.
foreach ( $result['items'] as $item ) {
- $product = $item->node ?? null;
+ $product = isset( $item->node ) ? $item->node : null;
if ( ! $product ) {
continue;
}
- $title = $product->title ?? 'Unknown Title';
- $id = $product->id ?? 'Unknown ID';
- $status = $product->status ?? 'Unknown Status';
- $variants_count = isset( $product->variants->edges ) ? count( $product->variants->edges ) : 0;
+ $title = isset( $product->title ) ? sanitize_text_field( $product->title ) : 'Unknown Title';
+ $id = isset( $product->id ) ? sanitize_text_field( $product->id ) : 'Unknown ID';
+ $status = isset( $product->status ) ? sanitize_text_field( $product->status ) : 'Unknown Status';
+ $variants_count = isset( $product->variants, $product->variants->edges ) && is_array( $product->variants->edges ) ? count( $product->variants->edges ) : 0;
WP_CLI::log( "- {$title} (ID: {$id}, Status: {$status}, Variants: {$variants_count})" );
}
// Display pagination information.
if ( $result['has_next_page'] && $result['cursor'] ) {
WP_CLI::log( '' );
WP_CLI::log( 'More products available. To fetch next batch, use:' );
- WP_CLI::log( "wp wc migrate products --fetch --limit={$limit} --after={$result['cursor']}" );
+ WP_CLI::log( "wp wc migrate products --fetch --limit={$limit} --after=" . sanitize_text_field( $result['cursor'] ) );
} else {
WP_CLI::log( '' );
WP_CLI::log( 'No more products to fetch.' );
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private function handle_fetch_request( string $platform, array $assoc_args ): void { | |
| $limit = (int) ( $assoc_args['limit'] ?? 5 ); | |
| $cursor = $assoc_args['after'] ?? null; | |
| WP_CLI::log( "Fetching {$limit} products from {$platform}..." ); | |
| $fetcher = $this->platform_registry->get_fetcher( $platform ); | |
| if ( ! $fetcher ) { | |
| WP_CLI::error( "Could not get fetcher for platform '{$platform}'" ); | |
| return; | |
| } | |
| // Build fetch arguments. | |
| $fetch_args = array( | |
| 'limit' => $limit, | |
| 'after_cursor' => $cursor, | |
| ); | |
| $result = $fetcher->fetch_batch( $fetch_args ); | |
| if ( empty( $result['items'] ) ) { | |
| WP_CLI::log( 'No products found or unable to fetch products.' ); | |
| return; | |
| } | |
| WP_CLI::success( sprintf( 'Successfully fetched %d products.', count( $result['items'] ) ) ); | |
| // Display basic product information. | |
| foreach ( $result['items'] as $item ) { | |
| $product = $item->node ?? null; | |
| if ( ! $product ) { | |
| continue; | |
| } | |
| $title = $product->title ?? 'Unknown Title'; | |
| $id = $product->id ?? 'Unknown ID'; | |
| $status = $product->status ?? 'Unknown Status'; | |
| $variants_count = isset( $product->variants->edges ) ? count( $product->variants->edges ) : 0; | |
| WP_CLI::log( "- {$title} (ID: {$id}, Status: {$status}, Variants: {$variants_count})" ); | |
| } | |
| // Display pagination information. | |
| if ( $result['has_next_page'] && $result['cursor'] ) { | |
| WP_CLI::log( '' ); | |
| WP_CLI::log( 'More products available. To fetch next batch, use:' ); | |
| WP_CLI::log( "wp wc migrate products --fetch --limit={$limit} --after={$result['cursor']}" ); | |
| } else { | |
| WP_CLI::log( '' ); | |
| WP_CLI::log( 'No more products to fetch.' ); | |
| } | |
| } | |
| private function handle_fetch_request( string $platform, array $assoc_args ): void { | |
| // Validate and bound the limit (1–250) and sanitize the cursor. | |
| $limit = max( 1, min( 250, (int) ( $assoc_args['limit'] ?? 5 ) ) ); | |
| $cursor = isset( $assoc_args['after'] ) | |
| ? sanitize_text_field( $assoc_args['after'] ) | |
| : null; | |
| WP_CLI::log( "Fetching {$limit} products from {$platform}..." ); | |
| $fetcher = $this->platform_registry->get_fetcher( $platform ); | |
| if ( ! $fetcher ) { | |
| WP_CLI::error( "Could not get fetcher for platform '{$platform}'" ); | |
| return; | |
| } | |
| // Build fetch arguments. | |
| $fetch_args = array( | |
| 'limit' => $limit, | |
| 'after_cursor' => $cursor, | |
| ); | |
| $result = $fetcher->fetch_batch( $fetch_args ); | |
| if ( empty( $result['items'] ) ) { | |
| WP_CLI::log( 'No products found or unable to fetch products.' ); | |
| return; | |
| } | |
| WP_CLI::success( sprintf( 'Successfully fetched %d products.', count( $result['items'] ) ) ); | |
| // Display basic product information. | |
| foreach ( $result['items'] as $item ) { | |
| $product = isset( $item->node ) ? $item->node : null; | |
| if ( ! $product ) { | |
| continue; | |
| } | |
| $title = isset( $product->title ) | |
| ? sanitize_text_field( $product->title ) | |
| : 'Unknown Title'; | |
| $id = isset( $product->id ) | |
| ? sanitize_text_field( $product->id ) | |
| : 'Unknown ID'; | |
| $status = isset( $product->status ) | |
| ? sanitize_text_field( $product->status ) | |
| : 'Unknown Status'; | |
| $variants_count = isset( $product->variants, $product->variants->edges ) | |
| && is_array( $product->variants->edges ) | |
| ? count( $product->variants->edges ) | |
| : 0; | |
| WP_CLI::log( "- {$title} (ID: {$id}, Status: {$status}, Variants: {$variants_count})" ); | |
| } | |
| // Display pagination information. | |
| if ( $result['has_next_page'] && $result['cursor'] ) { | |
| WP_CLI::log( '' ); | |
| WP_CLI::log( 'More products available. To fetch next batch, use:' ); | |
| WP_CLI::log( | |
| "wp wc migrate products --fetch --limit={$limit} --after=" | |
| . sanitize_text_field( $result['cursor'] ) | |
| ); | |
| } else { | |
| WP_CLI::log( '' ); | |
| WP_CLI::log( 'No more products to fetch.' ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ProductsCommand.php
between lines 151 and 202, add validation to ensure the 'limit' parameter is
within a reasonable positive range, for example, clamp it between 1 and a
maximum allowed value. Sanitize the 'after' cursor parameter to ensure it
contains only expected characters or format before use. Add null checks before
accessing properties on $product and its nested objects to avoid errors if any
are missing. Sanitize the cursor value before outputting it in the WP_CLI::log
call to prevent injection or formatting issues.
Add input validation and error handling to guard against unexpected inputs.
The method needs better input validation and error handling as required by the coding guidelines:
- Validate limit bounds - Line 152 should validate the limit is within reasonable bounds
- Sanitize cursor input - Line 153 should validate/sanitize the cursor parameter
- Add error handling around fetch_batch - Lines 169 should be wrapped in try-catch
- Validate response structure - Lines 180-188 and 194 access object properties without validation
Apply this diff to improve input validation and error handling:
private function handle_fetch_request( string $platform, array $assoc_args ): void {
- $limit = (int) ( $assoc_args['limit'] ?? 5 );
- $cursor = $assoc_args['after'] ?? null;
+ $limit = max( 1, min( 250, (int) ( $assoc_args['limit'] ?? 5 ) ) );
+ $cursor = isset( $assoc_args['after'] ) ? sanitize_text_field( $assoc_args['after'] ) : null;
WP_CLI::log( "Fetching {$limit} products from {$platform}..." );
$fetcher = $this->platform_registry->get_fetcher( $platform );
if ( ! $fetcher ) {
WP_CLI::error( "Could not get fetcher for platform '{$platform}'" );
return;
}
+ if ( ! method_exists( $fetcher, 'fetch_batch' ) ) {
+ WP_CLI::error( "Fetcher for platform '{$platform}' does not support batch fetching" );
+ return;
+ }
// Build fetch arguments.
$fetch_args = array(
'limit' => $limit,
'after_cursor' => $cursor,
);
- $result = $fetcher->fetch_batch( $fetch_args );
+ try {
+ $result = $fetcher->fetch_batch( $fetch_args );
+ } catch ( Exception $e ) {
+ WP_CLI::error( "Failed to fetch products: " . $e->getMessage() );
+ return;
+ }
- if ( empty( $result['items'] ) ) {
+ if ( ! is_array( $result ) || empty( $result['items'] ) ) {
WP_CLI::log( 'No products found or unable to fetch products.' );
return;
}
WP_CLI::success( sprintf( 'Successfully fetched %d products.', count( $result['items'] ) ) );
// Display basic product information.
foreach ( $result['items'] as $item ) {
- $product = $item->node ?? null;
+ $product = isset( $item->node ) && is_object( $item->node ) ? $item->node : null;
if ( ! $product ) {
continue;
}
- $title = $product->title ?? 'Unknown Title';
- $id = $product->id ?? 'Unknown ID';
- $status = $product->status ?? 'Unknown Status';
- $variants_count = isset( $product->variants->edges ) ? count( $product->variants->edges ) : 0;
+ $title = isset( $product->title ) ? sanitize_text_field( $product->title ) : 'Unknown Title';
+ $id = isset( $product->id ) ? sanitize_text_field( $product->id ) : 'Unknown ID';
+ $status = isset( $product->status ) ? sanitize_text_field( $product->status ) : 'Unknown Status';
+ $variants_count = ( isset( $product->variants->edges ) && is_array( $product->variants->edges ) ) ? count( $product->variants->edges ) : 0;
WP_CLI::log( "- {$title} (ID: {$id}, Status: {$status}, Variants: {$variants_count})" );
}
// Display pagination information.
- if ( $result['has_next_page'] && $result['cursor'] ) {
+ if ( ! empty( $result['has_next_page'] ) && ! empty( $result['cursor'] ) ) {
WP_CLI::log( '' );
WP_CLI::log( 'More products available. To fetch next batch, use:' );
- WP_CLI::log( "wp wc migrate products --fetch --limit={$limit} --after={$result['cursor']}" );
+ WP_CLI::log( "wp wc migrate products --fetch --limit={$limit} --after=" . sanitize_text_field( $result['cursor'] ) );
} else {
WP_CLI::log( '' );
WP_CLI::log( 'No more products to fetch.' );
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private function handle_fetch_request( string $platform, array $assoc_args ): void { | |
| $limit = (int) ( $assoc_args['limit'] ?? 5 ); | |
| $cursor = $assoc_args['after'] ?? null; | |
| WP_CLI::log( "Fetching {$limit} products from {$platform}..." ); | |
| $fetcher = $this->platform_registry->get_fetcher( $platform ); | |
| if ( ! $fetcher ) { | |
| WP_CLI::error( "Could not get fetcher for platform '{$platform}'" ); | |
| return; | |
| } | |
| // Build fetch arguments. | |
| $fetch_args = array( | |
| 'limit' => $limit, | |
| 'after_cursor' => $cursor, | |
| ); | |
| $result = $fetcher->fetch_batch( $fetch_args ); | |
| if ( empty( $result['items'] ) ) { | |
| WP_CLI::log( 'No products found or unable to fetch products.' ); | |
| return; | |
| } | |
| WP_CLI::success( sprintf( 'Successfully fetched %d products.', count( $result['items'] ) ) ); | |
| // Display basic product information. | |
| foreach ( $result['items'] as $item ) { | |
| $product = $item->node ?? null; | |
| if ( ! $product ) { | |
| continue; | |
| } | |
| $title = $product->title ?? 'Unknown Title'; | |
| $id = $product->id ?? 'Unknown ID'; | |
| $status = $product->status ?? 'Unknown Status'; | |
| $variants_count = isset( $product->variants->edges ) ? count( $product->variants->edges ) : 0; | |
| WP_CLI::log( "- {$title} (ID: {$id}, Status: {$status}, Variants: {$variants_count})" ); | |
| } | |
| // Display pagination information. | |
| if ( $result['has_next_page'] && $result['cursor'] ) { | |
| WP_CLI::log( '' ); | |
| WP_CLI::log( 'More products available. To fetch next batch, use:' ); | |
| WP_CLI::log( "wp wc migrate products --fetch --limit={$limit} --after={$result['cursor']}" ); | |
| } else { | |
| WP_CLI::log( '' ); | |
| WP_CLI::log( 'No more products to fetch.' ); | |
| } | |
| } | |
| private function handle_fetch_request( string $platform, array $assoc_args ): void { | |
| // Enforce reasonable bounds on limit and sanitize cursor input | |
| $limit = max( 1, min( 250, (int) ( $assoc_args['limit'] ?? 5 ) ) ); | |
| $cursor = isset( $assoc_args['after'] ) ? sanitize_text_field( $assoc_args['after'] ) : null; | |
| WP_CLI::log( "Fetching {$limit} products from {$platform}..." ); | |
| $fetcher = $this->platform_registry->get_fetcher( $platform ); | |
| if ( ! $fetcher ) { | |
| WP_CLI::error( "Could not get fetcher for platform '{$platform}'" ); | |
| return; | |
| } | |
| // Ensure the fetcher actually implements batch fetching | |
| if ( ! method_exists( $fetcher, 'fetch_batch' ) ) { | |
| WP_CLI::error( "Fetcher for platform '{$platform}' does not support batch fetching" ); | |
| return; | |
| } | |
| // Build fetch arguments. | |
| $fetch_args = array( | |
| 'limit' => $limit, | |
| 'after_cursor' => $cursor, | |
| ); | |
| // Guard against exceptions from the remote platform | |
| try { | |
| $result = $fetcher->fetch_batch( $fetch_args ); | |
| } catch ( Exception $e ) { | |
| WP_CLI::error( "Failed to fetch products: " . $e->getMessage() ); | |
| return; | |
| } | |
| // Validate that we got a well-formed array before proceeding | |
| if ( ! is_array( $result ) || empty( $result['items'] ) ) { | |
| WP_CLI::log( 'No products found or unable to fetch products.' ); | |
| return; | |
| } | |
| WP_CLI::success( sprintf( 'Successfully fetched %d products.', count( $result['items'] ) ) ); | |
| // Display basic product information. | |
| foreach ( $result['items'] as $item ) { | |
| $product = isset( $item->node ) && is_object( $item->node ) ? $item->node : null; | |
| if ( ! $product ) { | |
| continue; | |
| } | |
| $title = isset( $product->title ) ? sanitize_text_field( $product->title ) : 'Unknown Title'; | |
| $id = isset( $product->id ) ? sanitize_text_field( $product->id ) : 'Unknown ID'; | |
| $status = isset( $product->status ) ? sanitize_text_field( $product->status ) : 'Unknown Status'; | |
| $variants_count = ( isset( $product->variants->edges ) && is_array( $product->variants->edges ) ) | |
| ? count( $product->variants->edges ) | |
| : 0; | |
| WP_CLI::log( "- {$title} (ID: {$id}, Status: {$status}, Variants: {$variants_count})" ); | |
| } | |
| // Display pagination information. | |
| if ( ! empty( $result['has_next_page'] ) && ! empty( $result['cursor'] ) ) { | |
| WP_CLI::log( '' ); | |
| WP_CLI::log( 'More products available. To fetch next batch, use:' ); | |
| WP_CLI::log( | |
| "wp wc migrate products --fetch --limit={$limit}" | |
| . " --after=" . sanitize_text_field( $result['cursor'] ) | |
| ); | |
| } else { | |
| WP_CLI::log( '' ); | |
| WP_CLI::log( 'No more products to fetch.' ); | |
| } | |
| } |
🤖 Prompt for AI Agents
In plugins/woocommerce/src/Internal/CLI/Migrator/Commands/ProductsCommand.php
between lines 151 and 202, improve input validation and error handling by first
validating that the 'limit' parameter is an integer within a reasonable range
(e.g., positive and below a max threshold) and sanitizing the 'after' cursor
parameter to ensure it is a valid string or null. Then, wrap the call to
fetch_batch in a try-catch block to handle potential exceptions gracefully.
Finally, before accessing properties on the fetched result and product objects,
add checks to confirm the expected structure and presence of those properties to
avoid errors from undefined or null values.
Submission Review Guidelines:
Changes proposed in this Pull Request:
This PR implements GraphQL product data fetching for the WooCommerce CLI Migrator's Shopify platform support, building upon the existing REST API product counting functionality. This enhancement enables fetching detailed product information with cursor-based pagination for efficient large-scale migrations.
Key Features:
GraphQL API Integration: Added comprehensive GraphQL support to
ShopifyClientfor querying Shopify's Admin APIProduct Data Fetching: Implemented
fetch_batch()method inShopifyFetcherto retrieve detailed product informationCursor Pagination: Full support for GraphQL cursor-based pagination to handle large product catalogs
Rich Product Data: Fetches products with variants, images, collections, metadata, and more
CLI Commands: Enhanced
ProductsCommandwith--fetch,--limit, and--afterflagsComponents Enhanced:
graphql_request()method with proper error handlingfetch_batch()method using comprehensive GraphQL queryScreenshots or screen recordings:
Before (Stubs Only):
wp wc migrate products --fetch # Would return: WP_Error - fetch_batch not implementedAfter (Full GraphQL Implementation):
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
Prerequisites
Basic Product Fetching Tests
Test basic product fetching:
Test cursor pagination:
Cursor is printed at the end of the command.
Test various limit sizes:
Error Handling Tests
Test with invalid credentials:
wp wc migrate products --fetchTest with invalid cursor:
Integration Tests
wp wc migrate products --count # REST API (existing) wp wc migrate products --count --status=activeChangelog entry
Changelog Entry Details
Significance
Type
Message
Add GraphQL product data fetching with cursor pagination to WooCommerce CLI Migrator for Shopify platform. Enables fetching detailed product information including variants, images, and metadata through new --fetch command with --limit and --after flags.