Skip to content
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

[GTIN] Add WP CLI command for migration #2662

Open
wants to merge 6 commits into
base: add/support-core-gtin-field
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 5 additions & 0 deletions src/Internal/DependencyManagement/CoreServiceProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@
use Automattic\WooCommerce\GoogleListingsAndAds\Utility\DateTimeUtility;
use Automattic\WooCommerce\GoogleListingsAndAds\Utility\ImageUtility;
use Automattic\WooCommerce\GoogleListingsAndAds\Utility\ISOUtility;
use Automattic\WooCommerce\GoogleListingsAndAds\Utility\WPCLIMigrationGTIN;
use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\League\ISO3166\ISO3166DataProvider;
use Automattic\WooCommerce\GoogleListingsAndAds\Vendor\Psr\Container\ContainerInterface;
use Automattic\WooCommerce\GoogleListingsAndAds\View\PHPViewFactory;
Expand Down Expand Up @@ -216,6 +217,7 @@
AttributeMapping::class => true,
MarketingChannelRegistrar::class => true,
OAuthService::class => true,
WPCLIMigrationGTIN::class => true,
];

/**
Expand Down Expand Up @@ -431,5 +433,8 @@
$this->share_with_tags( GLAChannel::class, MerchantCenterService::class, AdsCampaign::class, Ads::class, MerchantStatuses::class, ProductSyncStats::class );
$this->share_with_tags( MarketingChannelRegistrar::class, GLAChannel::class, WC::class );
}

// ClI Classes
$this->share_with_tags( WPCLIMigrationGTIN::class, ProductRepository::class, AttributeManager::class );

Check warning on line 438 in src/Internal/DependencyManagement/CoreServiceProvider.php

View check run for this annotation

Codecov / codecov/patch

src/Internal/DependencyManagement/CoreServiceProvider.php#L438

Added line #L438 was not covered by tests
}
}
212 changes: 212 additions & 0 deletions src/Utility/WPCLIMigrationGTIN.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,212 @@
<?php
declare( strict_types=1 );

namespace Automattic\WooCommerce\GoogleListingsAndAds\Utility;

use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Conditional;
use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Registerable;
use Automattic\WooCommerce\GoogleListingsAndAds\Infrastructure\Service;
use Automattic\WooCommerce\GoogleListingsAndAds\Product\Attributes\AttributeManager;
use Automattic\WooCommerce\GoogleListingsAndAds\Product\ProductRepository;
use Exception;
use WP_CLI;

defined( 'ABSPATH' ) || exit;

/**
* Class WP_CLI_GTIN_Migration
* Creates a set of utility commands in WP CLI for GTIN Migration
*
* @package Automattic\WooCommerce\GoogleListingsAndAds\Utility
*
* @since x.x.x
*/
class WPCLIMigrationGTIN implements Service, Registerable, Conditional {


/** @var AttributeManager */
public AttributeManager $attribute_manager;

/** @var ProductRepository */
public ProductRepository $product_repository;

/**
* Constructor
*
* @param ProductRepository $product_repository
* @param AttributeManager $attribute_manager
*/
public function __construct( ProductRepository $product_repository, AttributeManager $attribute_manager ) {
$this->product_repository = $product_repository;
$this->attribute_manager = $attribute_manager;

Check warning on line 41 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L39-L41

Added lines #L39 - L41 were not covered by tests
}

/**
* Register service and initialize hooks.
*/
public function register(): void {
WP_CLI::add_hook( 'after_wp_load', [ $this, 'register_commands' ] );

Check warning on line 48 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L47-L48

Added lines #L47 - L48 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure how we can setup a environment without WP_CLI, but based on the doc here it sounds like it might not always be available. And since we aren't doing any conditional checks it seems like it would fatal here.

Should we make this class implement Conditional and add a is_needed function to check if WP_CLI is available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tweaked here 173824b

}

/**
* Register the commands
*/
public function register_commands(): void {
WP_CLI::add_command( 'wc g4wc gtin-migration start', [ $this, 'gtin_migration_start' ] );

Check warning on line 55 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L54-L55

Added lines #L54 - L55 were not covered by tests
}

/**
* Starts the GTIN migration in batches
*/
public function gtin_migration_start(): void {
$batch_size = $this->get_batch_size();
$num_products = $this->get_total_products_count();
WP_CLI::log( sprintf( 'Starting GTIN migration for %s products in the store.', $num_products ) );
$progress = WP_CLI\Utils\make_progress_bar( 'GTIN Migration', $num_products / $batch_size );
$processed = 0;
$batch_number = 1;
$start_time = microtime( true );

Check warning on line 68 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L61-L68

Added lines #L61 - L68 were not covered by tests

// First batch
$items = $this->get_items( $batch_number );
$processed += $this->process_items( $items );
$progress->tick();

Check warning on line 73 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L71-L73

Added lines #L71 - L73 were not covered by tests

// Next batches
while ( ! empty( $items ) ) {
++$batch_number;
$items = $this->get_items( $batch_number );
$processed += $this->process_items( $items );
$progress->tick();

Check warning on line 80 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L76-L80

Added lines #L76 - L80 were not covered by tests
}

$progress->finish();
$total_time = microtime( true ) - $start_time;

Check warning on line 84 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L83-L84

Added lines #L83 - L84 were not covered by tests

// Issue a warning if nothing is migrated.
if ( ! $processed ) {
WP_CLI::warning( __( 'No GTIN were migrated.', 'google-listings-and-ads' ) );
return;

Check warning on line 89 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L87-L89

Added lines #L87 - L89 were not covered by tests
}

WP_CLI::success(
sprintf(

Check warning on line 93 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L92-L93

Added lines #L92 - L93 were not covered by tests
/* Translators: %1$d is the number of migrated GTINS and %2$d is the execution time in seconds. */
_n(
'%1$d GTIN was migrated in %2$d seconds.',
'%1$d GTIN were migrated in %2$d seconds.',
$processed,
'google-listings-and-ads'
),
$processed,
$total_time
)
);

Check warning on line 104 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L95-L104

Added lines #L95 - L104 were not covered by tests
}

/**
* Get total of products in the store to be migrated.
*
* @return int The total number of products.
*/
private function get_total_products_count(): int {
$args = [
'status' => 'publish',
'return' => 'ids',
'type' => [ 'simple', 'variation' ],
];

Check warning on line 117 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L112-L117

Added lines #L112 - L117 were not covered by tests

return count( $this->product_repository->find_ids( $args ) );

Check warning on line 119 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L119

Added line #L119 was not covered by tests
}


/**
* Get the items for the current batch
*
* @param int $batch_number
* @return int[] Array of WooCommerce product IDs
*/
private function get_items( int $batch_number ): array {
return $this->product_repository->find_all_product_ids( $this->get_batch_size(), $this->get_query_offset( $batch_number ) );

Check warning on line 130 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L129-L130

Added lines #L129 - L130 were not covered by tests
}

/**
* Get the query offset based on a given batch number and the specified batch size.
*
* @param int $batch_number
*
* @return int
*/
protected function get_query_offset( int $batch_number ): int {
return $this->get_batch_size() * ( $batch_number - 1 );

Check warning on line 141 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L140-L141

Added lines #L140 - L141 were not covered by tests
}

/**
* Get the batch size. By default, 100.
*
* @return int The batch size.
*/
private function get_batch_size(): int {
return apply_filters( 'woocommerce_gla_batched_cli_size', 100 );

Check warning on line 150 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L149-L150

Added lines #L149 - L150 were not covered by tests
}

/**
* Process batch items.
*
* @param int[] $items A single batch of WooCommerce product IDs from the get_batch() method.
* @return int The number of items processed.
*/
protected function process_items( array $items ): int {

Check warning on line 159 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L159

Added line #L159 was not covered by tests
// todo: refactor avoiding duplication with MigrateGTIN Job after https://github.com/woocommerce/google-listings-and-ads/pull/2656 gets merged.
// update the product core GTIN using G4W GTIN
$products = $this->product_repository->find_by_ids( $items );
$processed = 0;

Check warning on line 163 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L162-L163

Added lines #L162 - L163 were not covered by tests

foreach ( $products as $product ) {

Check warning on line 165 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L165

Added line #L165 was not covered by tests
// void if core GTIN is already set.
if ( $product->get_global_unique_id() ) {
WP_CLI::debug( sprintf( 'GTIN has been skipped for Product ID: %s - %s. GTIN was found in Product Inventory tab.', $product->get_id(), $product->get_name() ) );
continue;

Check warning on line 169 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L167-L169

Added lines #L167 - L169 were not covered by tests
}

// process variations
if ( $product instanceof \WC_Product_Variable ) {
$variations = $product->get_children();
$processed += $this->process_items( $variations );
continue;

Check warning on line 176 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L173-L176

Added lines #L173 - L176 were not covered by tests
}

$gtin = $this->attribute_manager->get_value( $product, 'gtin' );
if ( ! $gtin ) {
WP_CLI::debug( sprintf( 'GTIN has been skipped for Product ID: %s - %s. No GTIN was found', $product->get_id(), $product->get_name() ) );
continue;

Check warning on line 182 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L179-L182

Added lines #L179 - L182 were not covered by tests
}

$gtin = str_replace( '-', '', $gtin );
if ( is_numeric( $gtin ) ) {

Check warning on line 186 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L185-L186

Added lines #L185 - L186 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nitpick, but I would have used ! is_numeric here with a continue like is done for the error checking above. That way we can keep the condition and error message right next to each other. And we don't have to indent the try and catch another time.

try {
$product->set_global_unique_id( $gtin );
$product->save();
WP_CLI::debug( sprintf( 'GTIN [ %s ] has been migrated for Product ID: %s - %s', $gtin, $product->get_id(), $product->get_name() ) );
++$processed;
} catch ( Exception $e ) {
WP_CLI::error( $e->getMessage(), false );

Check warning on line 193 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L188-L193

Added lines #L188 - L193 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we combine this error with the product ID, so we can know what to fix? Right now I'm getting this as output:

Starting GTIN migration for 2290 products in the store.
GTIN Migration  13 % [============>                                                                            ] 0:01 / 0:15
Error: Invalid or duplicated GTIN, UPC, EAN or ISBN.
GTIN Migration  100% [=========================================================================================] 0:01 / 0:15
Warning: No GTIN were migrated.

Which leaves me having to manually figure out which product I should fix.

}
} else {
WP_CLI::debug( sprintf( 'GTIN [ %s ] has been skipped for Product ID: %s - %s. Invalid GTIN was found.', $gtin, $product->get_id(), $product->get_name() ) );

Check warning on line 196 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L196

Added line #L196 was not covered by tests
}
}

return $processed;

Check warning on line 200 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L200

Added line #L200 was not covered by tests
}

/**
* Check if this Service is needed.
*
* @see https://make.wordpress.org/cli/handbook/guides/commands-cookbook/#include-in-a-plugin-or-theme
* @return bool
*/
public static function is_needed(): bool {
return defined( 'WP_CLI' ) && WP_CLI;

Check warning on line 210 in src/Utility/WPCLIMigrationGTIN.php

View check run for this annotation

Codecov / codecov/patch

src/Utility/WPCLIMigrationGTIN.php#L209-L210

Added lines #L209 - L210 were not covered by tests
}
}