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.

Sign up for GitHub

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

Jump to bottom

Bypass scheduled actions for customer updates #37265

Merged
merged 10 commits into from Mar 17, 2023
Merged

Bypass scheduled actions for customer updates #37265

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8bc8400
Bypass scheduler for customer update
mattsherman Mar 15, 2023
18866b4
Handle case when wc_last_active user meta doesn't exist at all
mattsherman Mar 16, 2023
4c09f07
Bypass scheduler for wc_last_active customer update
mattsherman Mar 16, 2023
8a65779
Changelog
mattsherman Mar 16, 2023
d6fb8a6
Bypass scheduler for delete_user and remove_user_from_blog
mattsherman Mar 16, 2023
1bc03bb
Bypass scheduler for woocommerce_privacy_remove_order_personal_data
mattsherman Mar 16, 2023
4356684
Bypass scheduler for woocommerce_new_customer
mattsherman Mar 17, 2023
33254b1
Fix linting issue
mattsherman Mar 17, 2023
c8e5fe7
Remove obsolete test for last_active_update sync
mattsherman Mar 17, 2023
26aa5a3
Remove assertions for pending wc-admin_import_customers
mattsherman Mar 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions plugins/woocommerce/changelog/update-bypass-actions-for-customer-updates
View file
Open in desktop
@@ -0,0 +1,4 @@
Significance: patch
Type: performance

Bypass Action Scheduler for customer updates.
84 changes: 84 additions & 0 deletions plugins/woocommerce/src/Admin/API/Reports/Customers/DataStore.php
View file
Open in desktop
Expand Up @@ -84,7 +84,19 @@ protected function assign_report_columns() {
* Set up all the hooks for maintaining and populating table data.
*/
public static function init() {
add_action( 'woocommerce_new_customer', array( __CLASS__, 'update_registered_customer' ) );

add_action( 'woocommerce_update_customer', array( __CLASS__, 'update_registered_customer' ) );
add_action( 'profile_update', array( __CLASS__, 'update_registered_customer' ) );

add_action( 'added_user_meta', array( __CLASS__, 'update_registered_customer_via_last_active' ), 10, 3 );
add_action( 'updated_user_meta', array( __CLASS__, 'update_registered_customer_via_last_active' ), 10, 3 );

add_action( 'delete_user', array( __CLASS__, 'delete_customer_by_user_id' ) );
add_action( 'remove_user_from_blog', array( __CLASS__, 'delete_customer_by_user_id' ) );

add_action( 'woocommerce_privacy_remove_order_personal_data', array( __CLASS__, 'anonymize_customer' ) );

add_action( 'woocommerce_analytics_delete_order_stats', array( __CLASS__, 'sync_on_order_delete' ), 15, 2 );
}

Expand Down Expand Up @@ -775,6 +787,20 @@ public static function update_registered_customer( $user_id ) {
return $results;
}

/**
* Update the database if the "last active" meta value was changed.
* Function expects to be hooked into the `added_user_meta` and `updated_user_meta` actions.
*
* @param int $meta_id ID of updated metadata entry.
* @param int $user_id ID of the user being updated.
* @param string $meta_key Meta key being updated.
*/
public static function update_registered_customer_via_last_active( $meta_id, $user_id, $meta_key ) {
if ( 'wc_last_active' === $meta_key ) {
self::update_registered_customer( $user_id );
}
}

/**
* Check if a user ID is a valid customer or other user role with past orders.
*
Expand Down Expand Up @@ -835,6 +861,11 @@ public static function delete_customer( $customer_id ) {
public static function delete_customer_by_user_id( $user_id ) {
global $wpdb;

if ( (int) $user_id < 1 || doing_action( 'wp_uninitialize_site' ) ) {
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 wondering why this had to be added. In which scenario would this be called with user_id < 1? Maybe it's worth adding a comment?

EDIT: I see you only moved this from the scheduler. Feel free to leave it like this if you think it's OK.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A user ID of 0 would be an unauthenticated request, or some error up the chain that resulted in user ID 0. Whatever the case, proceeding with the DELETE would be unnecessary, so the early return seems most robust.

// Skip the deletion.
return;
}

$user_id = (int) $user_id;
$num_deleted = $wpdb->delete( self::get_db_table_name(), array( 'user_id' => $user_id ) );

Expand All @@ -843,6 +874,59 @@ public static function delete_customer_by_user_id( $user_id ) {
}
}

/**
* Anonymize the customer data for a single order.
*
* @internal
* @param int $order_id Order id.
* @return void
*/
public static function anonymize_customer( $order_id ) {
global $wpdb;

$customer_id = $wpdb->get_var(
$wpdb->prepare( "SELECT customer_id FROM {$wpdb->prefix}wc_order_stats WHERE order_id = %d", $order_id )
);

if ( ! $customer_id ) {
return;
}

// Long form query because $wpdb->update rejects [deleted].
$deleted_text = __( '[deleted]', 'woocommerce' );
$updated = $wpdb->query(
$wpdb->prepare(
"UPDATE {$wpdb->prefix}wc_customer_lookup
SET
user_id = NULL,
username = %s,
first_name = %s,
last_name = %s,
email = %s,
country = '',
postcode = %s,
city = %s,
state = %s
WHERE
customer_id = %d",
array(
$deleted_text,
$deleted_text,
$deleted_text,
'deleted@site.invalid',
$deleted_text,
$deleted_text,
$deleted_text,
$customer_id,
)
)
);
// If the customer row was anonymized, flush the cache.
if ( $updated ) {
ReportsCache::invalidate();
}
}

/**
* Initialize query objects.
*/
Expand Down
141 changes: 0 additions & 141 deletions plugins/woocommerce/src/Internal/Admin/Schedulers/CustomersScheduler.php
View file
Open in desktop
Expand Up @@ -27,13 +27,6 @@ class CustomersScheduler extends ImportScheduler {
* @internal
*/
public static function init() {
add_action( 'woocommerce_new_customer', array( __CLASS__, 'schedule_import' ) );
add_action( 'woocommerce_update_customer', array( __CLASS__, 'schedule_import' ) );
add_action( 'updated_user_meta', array( __CLASS__, 'schedule_import_via_last_active' ), 10, 3 );
add_action( 'woocommerce_privacy_remove_order_personal_data', array( __CLASS__, 'schedule_anonymize' ) );
add_action( 'delete_user', array( __CLASS__, 'schedule_user_delete' ) );
add_action( 'remove_user_from_blog', array( __CLASS__, 'schedule_user_delete' ) );

CustomersDataStore::init();
parent::init();
}
Expand All @@ -47,8 +40,6 @@ public static function init() {
public static function get_dependencies() {
return array(
'delete_batch_init' => OrdersScheduler::get_action( 'delete_batch_init' ),
'anonymize' => self::get_action( 'import' ),
'delete_user' => self::get_action( 'import' ),
);
}

Expand Down Expand Up @@ -120,74 +111,6 @@ public static function get_total_imported() {
return $wpdb->get_var( "SELECT COUNT(*) FROM {$wpdb->prefix}wc_customer_lookup" );
}

/**
* Get all available scheduling actions.
* Used to determine action hook names and clear events.
*
* @internal
* @return array
*/
public static function get_scheduler_actions() {
$actions = parent::get_scheduler_actions();
$actions['anonymize'] = 'wc-admin_anonymize_' . static::$name;
$actions['delete_user'] = 'wc-admin_delete_user_' . static::$name;
return $actions;
}

/**
* Schedule import.
*
* @internal
* @param int $user_id User ID.
* @return void
*/
public static function schedule_import( $user_id ) {
self::schedule_action( 'import', array( $user_id ) );
}

/**
* Schedule an import if the "last active" meta value was changed.
* Function expects to be hooked into the `updated_user_meta` action.
*
* @internal
* @param int $meta_id ID of updated metadata entry.
* @param int $user_id ID of the user being updated.
* @param string $meta_key Meta key being updated.
*/
public static function schedule_import_via_last_active( $meta_id, $user_id, $meta_key ) {
if ( 'wc_last_active' === $meta_key ) {
self::schedule_import( $user_id );
}
}

/**
* Schedule an action to anonymize a single Order.
*
* @internal
* @param WC_Order $order Order object.
* @return void
*/
public static function schedule_anonymize( $order ) {
if ( is_a( $order, 'WC_Order' ) ) {
// Postpone until any pending updates are completed.
self::schedule_action( 'anonymize', array( $order->get_id() ) );
}
}

/**
* Schedule an action to delete a single User.
*
* @internal
* @param int $user_id User ID.
* @return void
*/
public static function schedule_user_delete( $user_id ) {
if ( (int) $user_id > 0 && ! doing_action( 'wp_uninitialize_site' ) ) {
// Postpone until any pending updates are completed.
self::schedule_action( 'delete_user', array( $user_id ) );
}
}

/**
* Imports a single customer.
*
Expand Down Expand Up @@ -220,68 +143,4 @@ public static function delete( $batch_size ) {
CustomersDataStore::delete_customer( $customer_id );
}
}

/**
* Anonymize the customer data for a single order.
*
* @internal
* @param int $order_id Order id.
* @return void
*/
public static function anonymize( $order_id ) {
global $wpdb;

$customer_id = $wpdb->get_var(
$wpdb->prepare( "SELECT customer_id FROM {$wpdb->prefix}wc_order_stats WHERE order_id = %d", $order_id )
);

if ( ! $customer_id ) {
return;
}

// Long form query because $wpdb->update rejects [deleted].
$deleted_text = __( '[deleted]', 'woocommerce' );
$updated = $wpdb->query(
$wpdb->prepare(
"UPDATE {$wpdb->prefix}wc_customer_lookup
SET
user_id = NULL,
username = %s,
first_name = %s,
last_name = %s,
email = %s,
country = '',
postcode = %s,
city = %s,
state = %s
WHERE
customer_id = %d",
array(
$deleted_text,
$deleted_text,
$deleted_text,
'deleted@site.invalid',
$deleted_text,
$deleted_text,
$deleted_text,
$customer_id,
)
)
);
// If the customer row was anonymized, flush the cache.
if ( $updated ) {
ReportsCache::invalidate();
}
}

/**
* Delete the customer data for a single user.
*
* @internal
* @param int $user_id User ID.
* @return void
*/
public static function delete_user( $user_id ) {
CustomersDataStore::delete_customer_by_user_id( $user_id );
}
}
22 changes: 0 additions & 22 deletions plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api-init.php
View file
Open in desktop
Expand Up @@ -104,26 +104,4 @@ public function test_other_user_meta_update_no_customer_sync() {

$this->assertEmpty( $this->queue->actions );
}

/**
* Test that updating wc_last_active triggers a customer sync.
*
* @return void
*/
public function test_other_last_active_update_customer_sync() {
// First call creates the meta key.
// These don't use wc_update_user_last_active() because the timestamps will be the same.
update_user_meta( 1, 'wc_last_active', time() - 10 );
// Second call updates it which triggers the sync.
update_user_meta( 1, 'wc_last_active', time() );

$this->assertCount( 1, $this->queue->actions );
$this->assertArraySubset(
array(
'hook' => CustomersScheduler::get_action( 'import' ),
'args' => array( 1 ),
),
$this->queue->actions[0]
);
}
}
2 changes: 0 additions & 2 deletions plugins/woocommerce/tests/legacy/unit-tests/woocommerce-admin/api/reports-import.php
View file
Open in desktop
Expand Up @@ -207,7 +207,6 @@ function ( $action ) {
$pending_actions
);
$this->assertContains( 'wc-admin_import_orders', $pending_hooks );
$this->assertContains( 'wc-admin_import_customers', $pending_hooks );

// Cancel outstanding actions.
$request = new WP_REST_Request( 'POST', $this->endpoint . '/cancel' );
Expand All @@ -226,7 +225,6 @@ function ( $action ) {
$pending_actions
);
$this->assertNotContains( 'wc-admin_import_orders', $pending_hooks );
$this->assertNotContains( 'wc-admin_import_customers', $pending_hooks );
}

/**
Expand Down