Various improvements to HPOS settings (#47370)

* Properly format order counts

* Remove superfluous tooltip

* Add "is-error" class for admin settings

* Remove unused option in DataSynchronizer

* Use non-persistent group "counts" for caching count of orders pending sync

* Drop redundant method from COT controller

* Standardize usage of <strong> in COT settings

* Drop unused import

* Allow manual sync to be stopped

* Reword messages in HPOS screen

* Make PHPCS happy

* Add changelog

* Fix unit test

* Fully deprecated DataSynchronier::get_sync_status()

* Extract translatable string to prevent duplication
This commit is contained in:
Jorge A. Torres 2024-05-21 08:35:22 -05:00 committed by GitHub
parent bf7204f119
commit 7c4ce70916
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 87 additions and 127 deletions

View File

@ -0,0 +1,4 @@
Significance: minor
Type: update
Improvements to HPOS settings screen.

View File

@ -5137,6 +5137,10 @@ img.help_tip {
p.description {
margin-bottom: 8px;
&.is-error, span.is-error {
color: $red;
}
}
&:first-child {

View File

@ -805,19 +805,18 @@ if ( ! class_exists( 'WC_Admin_Settings', false ) ) :
$description = $value['desc'];
}
$description_is_error = $value['description_is_error'] ?? false;
$extra_description_style = $description_is_error ? " style='color:red'" : '';
$error_class = ( ! empty( $value['description_is_error'] ) ) ? 'is-error' : '';
if ( $description && in_array( $value['type'], array( 'textarea', 'radio' ), true ) ) {
$description = '<p style="margin-top:0">' . wp_kses_post( $description ) . '</p>';
} elseif ( $description && in_array( $value['type'], array( 'checkbox' ), true ) ) {
$description = wp_kses_post( $description );
} elseif ( $description ) {
$description = '<p class="description"' . $extra_description_style . '>' . wp_kses_post( $description ) . '</p>';
$description = '<p class="description ' . $error_class . '">' . wp_kses_post( $description ) . '</p>';
}
if ( $tooltip_html && in_array( $value['type'], array( 'checkbox' ), true ) ) {
$tooltip_html = '<p class="description"' . $extra_description_style . '>' . $tooltip_html . '</p>';
$tooltip_html = '<p class="description ' . $error_class . '">' . $tooltip_html . '</p>';
} elseif ( $tooltip_html ) {
$tooltip_html = wc_help_tip( $tooltip_html );
}

View File

@ -31,6 +31,8 @@ class CustomOrdersTableController {
private const SYNC_QUERY_ARG = 'wc_hpos_sync_now';
private const STOP_SYNC_QUERY_ARG = 'wc_hpos_stop_sync';
/**
* The name of the option for enabling the usage of the custom orders tables
*/
@ -419,17 +421,30 @@ class CustomOrdersTableController {
return;
}
if ( ! filter_input( INPUT_GET, self::SYNC_QUERY_ARG, FILTER_VALIDATE_BOOLEAN ) ) {
if ( filter_input( INPUT_GET, self::SYNC_QUERY_ARG, FILTER_VALIDATE_BOOLEAN ) ) {
$action = 'sync-now';
} elseif ( filter_input( INPUT_GET, self::STOP_SYNC_QUERY_ARG, FILTER_VALIDATE_BOOLEAN ) ) {
$action = 'stop-sync';
} else {
return;
}
if ( ! wp_verify_nonce( sanitize_text_field( wp_unslash( $_GET['_wpnonce'] ?? '' ) ), 'hpos-sync-now' ) ) {
WC_Admin_Settings::add_error( esc_html__( 'Unable to start synchronization. The link you followed may have expired.', 'woocommerce' ) );
if ( ! wp_verify_nonce( sanitize_text_field( wp_unslash( $_GET['_wpnonce'] ?? '' ) ), "hpos-{$action}" ) ) {
WC_Admin_Settings::add_error(
'sync-now' === $action ?
esc_html__( 'Unable to start synchronization. The link you followed may have expired.', 'woocommerce' )
: esc_html__( 'Unable to stop synchronization. The link you followed may have expired.', 'woocommerce' )
);
return;
}
$this->data_cleanup->toggle_flag( false );
if ( 'sync-now' === $action ) {
$this->batch_processing_controller->enqueue_processor( DataSynchronizer::class );
} else {
$this->batch_processing_controller->remove_processor( DataSynchronizer::class );
}
}
/**
@ -441,6 +456,7 @@ class CustomOrdersTableController {
*/
private function register_removable_query_arg( $query_args ) {
$query_args[] = self::SYNC_QUERY_ARG;
$query_args[] = self::STOP_SYNC_QUERY_ARG;
return $query_args;
}
@ -529,7 +545,7 @@ class CustomOrdersTableController {
$get_disabled = function () {
$plugin_compatibility = $this->features_controller->get_compatible_plugins_for_feature( 'custom_order_tables', true );
$sync_complete = 0 === $this->get_orders_pending_sync_count();
$sync_complete = 0 === $this->data_synchronizer->get_current_orders_pending_sync_count();
$disabled = array();
// Changing something here? might also want to look at `enable|disable` functions in CLIRunner.
$incompatible_plugins = array_merge( $plugin_compatibility['uncertain'], $plugin_compatibility['incompatible'] );
@ -575,7 +591,7 @@ class CustomOrdersTableController {
};
$get_sync_message = function () {
$orders_pending_sync_count = $this->get_orders_pending_sync_count();
$orders_pending_sync_count = $this->data_synchronizer->get_current_orders_pending_sync_count( true );
$sync_in_progress = $this->batch_processing_controller->is_enqueued( get_class( $this->data_synchronizer ) );
$sync_enabled = $this->data_synchronizer->data_sync_is_enabled();
$sync_is_pending = $orders_pending_sync_count > 0;
@ -586,15 +602,14 @@ class CustomOrdersTableController {
if ( $is_dangerous ) {
$sync_message[] = wp_kses_data(
sprintf(
// translators: %d: number of pending orders.
_n(
"There's %d order pending sync. <b>Switching data storage while sync is incomplete is dangerous and can lead to order data corruption or loss!</b>",
'There are %d orders pending sync. <b>Switching data storage while sync is incomplete is dangerous and can lead to order data corruption or loss!</b>',
$orders_pending_sync_count,
'woocommerce'
),
$orders_pending_sync_count,
// translators: %s: number of pending orders.
_n( "There's %s order pending sync.", 'There are %s orders pending sync.', $orders_pending_sync_count, 'woocommerce' ),
number_format_i18n( $orders_pending_sync_count ),
)
. ' '
. '<strong>'
. __( 'Switching data storage while sync is incomplete is dangerous and can lead to order data corruption or loss!', 'woocommerce' )
. '</strong>'
);
}
@ -604,10 +619,28 @@ class CustomOrdersTableController {
if ( $sync_in_progress && $sync_is_pending ) {
$sync_message[] = sprintf(
// translators: %d: number of pending orders.
__( 'Currently syncing orders... %d pending', 'woocommerce' ),
$orders_pending_sync_count
// translators: %s: number of pending orders.
__( 'Currently syncing orders... %s pending', 'woocommerce' ),
number_format_i18n( $orders_pending_sync_count )
);
if ( ! $sync_enabled ) {
$stop_sync_url = wp_nonce_url(
add_query_arg(
array(
self::STOP_SYNC_QUERY_ARG => true,
),
wc_get_container()->get( FeaturesController::class )->get_features_page_url()
),
'hpos-stop-sync'
);
$sync_message[] = sprintf(
'<a href="%1$s" class="button button-link">%2$s</a>',
esc_url( $stop_sync_url ),
__( 'Stop sync', 'woocommerce' )
);
}
} elseif ( $sync_is_pending ) {
$sync_now_url = wp_nonce_url(
add_query_arg(
@ -622,26 +655,10 @@ class CustomOrdersTableController {
if ( ! $is_dangerous ) {
$sync_message[] = wp_kses_data(
sprintf(
// translators: %d: number of pending orders.
// translators: %s: number of pending orders.
_n(
"There's %d order pending sync. You can switch order data storage <strong>only when the posts and orders tables are in sync</strong>.",
'There are %d orders pending sync. You can switch order data storage <strong>only when the posts and orders tables are in sync</strong>.',
$orders_pending_sync_count,
'woocommerce'
),
$orders_pending_sync_count
)
);
}
$sync_message[] = sprintf(
'<a href="%1$s" class="button button-link">%2$s</a>',
esc_url( $sync_now_url ),
sprintf(
// translators: %d: number of pending orders.
_n(
'Sync %s pending order',
'Sync %s pending orders',
"You can switch order data storage <strong>only when the posts and orders tables are in sync</strong>. There's currently %s order out of sync.",
'You can switch order data storage <strong>only when the posts and orders tables are in sync</strong>. There are currently %s orders out of sync. ',
$orders_pending_sync_count,
'woocommerce'
),
@ -650,11 +667,18 @@ class CustomOrdersTableController {
);
}
$sync_message[] = sprintf(
'<a href="%1$s" class="button button-link">%2$s</a>',
esc_url( $sync_now_url ),
__( 'Sync orders now', 'woocommerce' )
);
}
return implode( '<br />', $sync_message );
};
$get_description_is_error = function () {
$sync_is_pending = $this->get_orders_pending_sync_count() > 0;
$sync_is_pending = $this->data_synchronizer->get_current_orders_pending_sync_count( true ) > 0;
return $sync_is_pending && $this->changing_data_source_with_sync_pending_is_allowed();
};
@ -691,15 +715,6 @@ class CustomOrdersTableController {
return apply_filters( 'wc_allow_changing_orders_storage_while_sync_is_pending', false );
}
/**
* Returns the count of orders pending synchronization.
*
* @return int
*/
private function get_orders_pending_sync_count(): int {
return $this->data_synchronizer->get_sync_status()['current_pending_count'];
}
/**
* Rewrites post edit links for HPOS placeholder posts so that they go to the HPOS order itself.
* Hooked onto `get_edit_post_link`.

View File

@ -9,7 +9,6 @@ use Automattic\WooCommerce\Caches\OrderCacheController;
use Automattic\WooCommerce\Database\Migrations\CustomOrderTable\PostsToOrdersMigrationController;
use Automattic\WooCommerce\Internal\Admin\Orders\EditLock;
use Automattic\WooCommerce\Internal\BatchProcessing\{ BatchProcessingController, BatchProcessorInterface };
use Automattic\WooCommerce\Internal\Features\FeaturesController;
use Automattic\WooCommerce\Internal\Traits\AccessiblePrivateMethods;
use Automattic\WooCommerce\Internal\Utilities\DatabaseUtil;
use Automattic\WooCommerce\Proxies\LegacyProxy;
@ -27,7 +26,6 @@ class DataSynchronizer implements BatchProcessorInterface {
use AccessiblePrivateMethods;
public const ORDERS_DATA_SYNC_ENABLED_OPTION = 'woocommerce_custom_orders_table_data_sync_enabled';
private const INITIAL_ORDERS_PENDING_SYNC_COUNT_OPTION = 'woocommerce_initial_orders_pending_sync_count';
public const PLACEHOLDER_ORDER_POST_TYPE = 'shop_order_placehold';
public const DELETED_RECORD_META_KEY = '_deleted_from';
@ -119,8 +117,6 @@ class DataSynchronizer implements BatchProcessorInterface {
if ( self::BACKGROUND_SYNC_MODE_CONTINUOUS === $this->get_background_sync_mode() ) {
self::add_action( 'shutdown', array( $this, 'handle_continuous_background_sync' ) );
}
self::add_filter( 'woocommerce_feature_description_tip', array( $this, 'handle_feature_description_tip' ), 10, 3 );
}
/**
@ -429,10 +425,18 @@ class DataSynchronizer implements BatchProcessorInterface {
* The information is meaningful only if pending_data_sync_is_in_progress return true.
*
* @return array
*
* @deprecated 9.0.0
*/
public function get_sync_status() {
wc_deprecated_function(
__METHOD__,
'9.0.0',
'get_current_orders_pending_sync_count()'
);
return array(
'initial_pending_count' => (int) get_option( self::INITIAL_ORDERS_PENDING_SYNC_COUNT_OPTION, 0 ),
'initial_pending_count' => (int) 0,
'current_pending_count' => $this->get_total_pending_count(),
);
}
@ -459,7 +463,7 @@ class DataSynchronizer implements BatchProcessorInterface {
global $wpdb;
if ( $use_cache ) {
$pending_count = wp_cache_get( 'woocommerce_hpos_pending_sync_count' );
$pending_count = wp_cache_get( 'woocommerce_hpos_pending_sync_count', 'counts' );
if ( false !== $pending_count ) {
return (int) $pending_count;
}
@ -551,7 +555,7 @@ SELECT(
);
$pending_count += $deleted_count;
wp_cache_set( 'woocommerce_hpos_pending_sync_count', $pending_count );
wp_cache_set( 'woocommerce_hpos_pending_sync_count', $pending_count, 'counts' );
return $pending_count;
}
@ -688,7 +692,7 @@ ORDER BY orders.id ASC
* or because there's nothing left to synchronize.
*/
public function cleanup_synchronization_state() {
delete_option( self::INITIAL_ORDERS_PENDING_SYNC_COUNT_OPTION );
delete_option( 'woocommerce_initial_orders_pending_sync_count' );
}
/**
@ -1053,69 +1057,4 @@ ORDER BY orders.id ASC
$order->delete( true );
}
}
/**
* Handle the 'woocommerce_feature_description_tip' filter.
*
* When the COT feature is enabled and there are orders pending sync (in either direction),
* show a "you should ync before disabling" warning under the feature in the features page.
* Skip this if the UI prevents changing the feature enable status.
*
* @param string $desc_tip The original description tip for the feature.
* @param string $feature_id The feature id.
* @param bool $ui_disabled True if the UI doesn't allow to enable or disable the feature.
* @return string The new description tip for the feature.
*/
private function handle_feature_description_tip( $desc_tip, $feature_id, $ui_disabled ): string {
if ( 'custom_order_tables' !== $feature_id || $ui_disabled ) {
return $desc_tip;
}
$features_controller = wc_get_container()->get( FeaturesController::class );
$feature_is_enabled = $features_controller->feature_is_enabled( 'custom_order_tables' );
if ( ! $feature_is_enabled ) {
return $desc_tip;
}
$pending_sync_count = $this->get_current_orders_pending_sync_count();
if ( ! $pending_sync_count ) {
return $desc_tip;
}
if ( $this->custom_orders_table_is_authoritative() ) {
$extra_tip = sprintf(
_n(
"⚠ There's one order pending sync from the orders table to the posts table. The feature shouldn't be disabled until this order is synchronized.",
"⚠ There are %1\$d orders pending sync from the orders table to the posts table. The feature shouldn't be disabled until these orders are synchronized.",
$pending_sync_count,
'woocommerce'
),
$pending_sync_count
);
} else {
$extra_tip = sprintf(
_n(
"⚠ There's one order pending sync from the posts table to the orders table. The feature shouldn't be disabled until this order is synchronized.",
"⚠ There are %1\$d orders pending sync from the posts table to the orders table. The feature shouldn't be disabled until these orders are synchronized.",
$pending_sync_count,
'woocommerce'
),
$pending_sync_count
);
}
$cot_settings_url = add_query_arg(
array(
'page' => 'wc-settings',
'tab' => 'advanced',
'section' => 'custom_data_stores',
),
admin_url( 'admin.php' )
);
/* translators: %s = URL of the custom data stores settings page */
$manage_cot_settings_link = sprintf( __( "<a href='%s'>Manage orders synchronization</a>", 'woocommerce' ), $cot_settings_url );
return $desc_tip ? "{$desc_tip}<br/>{$extra_tip} {$manage_cot_settings_link}" : "{$extra_tip} {$manage_cot_settings_link}";
}
}

View File

@ -78,8 +78,7 @@ class COTMigrationUtil {
return false;
}
$sync_status = $this->data_synchronizer->get_sync_status();
return 0 === $sync_status['current_pending_count'];
return 0 === $this->data_synchronizer->get_total_pending_count();
}
/**

View File

@ -664,7 +664,7 @@ class DataSynchronizerTests extends HposTestCase {
);
$sync_setting = array_values( $sync_setting )[0];
$this->assertEquals( $sync_setting['value'], 'no' );
$this->assertTrue( str_contains( $sync_setting['desc_tip'], "There's 1 order pending sync" ) );
$this->assertTrue( str_contains( $sync_setting['desc_tip'], $auth_table_change_allowed_with_sync_pending ? "There's 1 order pending sync" : "There's currently 1 order out of sync" ) );
$this->assertTrue(
str_contains(
$sync_setting['desc_tip'],

View File

@ -92,10 +92,10 @@ class COTMigrationUtilTest extends WC_Unit_Test_Case {
*/
public function test_is_custom_order_tables_in_sync_is_true() {
$data_sync_mock = $this->getMockBuilder( DataSynchronizer::class )
->setMethods( array( 'get_sync_status', 'data_sync_is_enabled' ) )
->setMethods( array( 'get_current_orders_pending_sync_count', 'data_sync_is_enabled' ) )
->getMock();
$data_sync_mock->method( 'get_sync_status' )->willReturn( array( 'current_pending_count' => 0 ) );
$data_sync_mock->method( 'get_current_orders_pending_sync_count' )->willReturn( 0 );
$data_sync_mock->method( 'data_sync_is_enabled' )->willReturn( true );
// This is needed to prevent "Call to private method Mock_DataSynchronizer_xxxx::process_added_option" errors.
@ -113,10 +113,10 @@ class COTMigrationUtilTest extends WC_Unit_Test_Case {
*/
public function test_is_custom_order_tables_in_sync_is_false() {
$data_sync_mock = $this->getMockBuilder( DataSynchronizer::class )
->setMethods( array( 'get_sync_status', 'data_sync_is_enabled' ) )
->setMethods( array( 'get_current_orders_pending_sync_count', 'data_sync_is_enabled' ) )
->getMock();
$data_sync_mock->method( 'get_sync_status' )->willReturn( array( 'current_pending_count' => 0 ) );
$data_sync_mock->method( 'get_current_orders_pending_sync_count' )->willReturn( 0 );
$data_sync_mock->method( 'data_sync_is_enabled' )->willReturn( false );
// This is needed to prevent "Call to private method Mock_DataSynchronizer_xxxx::process_added_option" errors.