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

Bypass scheduled actions for customer updates #37265

Merged
merged 10 commits into from Mar 17, 2023

Conversation

mattsherman
Copy link
Contributor

@mattsherman mattsherman commented Mar 16, 2023

All Submissions:

Changes proposed in this Pull Request:

This PR changes the handling of various types of customer updates to update the Analytics lookup tables immediately instead of using Action Scheduler, to decrease the load on Action Scheduler and improve overall site performance.

  • New customer is created (actions: woocommerce_new_customer)
  • Customer updated (actions: woocommerce_update_customer, updated_user_meta)
  • Customer deleted (actions delete_user, remove_user_from_blog)
  • Personal data removed (action: woocommerce_privacy_remove_order_personal_data)

Closes #36330.

How to test the changes in this Pull Request:

General store settings to configure prior to testing

  • WooCommerce > Settings
    • Accounts & Privacy
      • Enable: Allow customers to create an account during checkout
      • Enable: Allow customers to create an account on the "My account" page
      • Enable: When creating an account, automatically generate an account username for the customer based on their name, surname, or email
      • Disable: When creating an account, send the new user a link to set their password
    • Account erasure requests
      • Enable: Remove personal data from orders on request

New customer is created (actions: woocommerce_new_customer)

  1. Either create a new customer via code (you can put this in a mu-plugin to get a customer created automatically when any store page loads)...
function test_create_customer() {
	try {
		$customer = new WC_Customer();
		$customer->set_username( 'customer' );
		$customer->set_password( 'abc123' );
		$customer->set_email( 'customer@example.com' );
		$customer->save();
	} catch ( Exception ) {}
}

add_action( 'admin_init', 'test_create_customer' );

or use the REST API (doc) to create a new customer...

POST /wp-json/wc/w3/customers
  1. Verify the new customer appears instantly on the Customers report.
  2. Verify no wc-admin_import_customers appears in Tools > Scheduled Actions.

Customer updated (actions: woocommerce_update_customer, updated_user_meta, added_user_meta)

  1. Create a new customer account on the shop store front by going to My account and registering a new account.
  2. Update last name of your new account.
  3. Verify the updated customer appears instantly on the Customers report.
  4. Verify no wc-admin_import_customers appears in Tools > Scheduled Actions.
  5. Manually remove the wc_last_active entry from wp_usermeta for the customer's record.
  6. Refresh the logged in My account page.
  7. Verify the customer's last active appears correctly on the Customers report.
  8. Verify no wc-admin_import_customers appears in Tools > Scheduled Actions.

Customer deleted (actions delete_user, remove_user_from_blog)

  1. Delete customer's user account from Users > All Users.
  2. Verify no wc-admin_delete_user_customers appears in Tools > Scheduled Actions.

Personal data removed (action: woocommerce_privacy_remove_order_personal_data)

  1. Go to Tools > Erase Personal Data.
  2. Enter the email address of the customer account.
  3. Uncheck "Send personal data erasure confirmation email".
  4. Click "Send Request".
  5. Click "Erase personal data" on the row in the table.
  6. After it completes, verify customer data has been removed from Customers report.

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you created a changelog file for each project being changed, ie pnpm --filter=<project> changelog add?
  • Have you included testing instructions?

FOR PR REVIEWER ONLY:

  • I have reviewed that everything is sanitized/escaped appropriately for any SQL or XSS injection possibilities. I made sure Linting is not ignored or disabled.

@mattsherman mattsherman force-pushed the update/bypass-actions-for-customer-updates branch from 5753924 to 4356684 Compare March 17, 2023 03:12
@mattsherman mattsherman added focus: performance The issue/PR is related to performance. and removed focus: react admin [team:Ghidorah] labels Mar 17, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Mar 17, 2023

Test Results Summary

Commit SHA: 26aa5a3

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202611m 1s
E2E Tests1910010020113m 24s

To view the full API test report, click here.
To view the full E2E test report, click here.
To view all test reports, visit the WooCommerce Test Reports Dashboard.

@mattsherman mattsherman added the type: enhancement The issue is a request for an enhancement. label Mar 17, 2023
@mattsherman mattsherman requested review from a team March 17, 2023 04:40
@mattsherman mattsherman marked this pull request as ready for review March 17, 2023 04:51
@codecov
Copy link

codecov bot commented Mar 17, 2023

Codecov Report

Merging #37265 (26aa5a3) into trunk (0cf5677) will decrease coverage by 0.0%.
The diff coverage is 19.6%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             trunk   #37265     +/-   ##
==========================================
- Coverage     46.7%    46.7%   -0.0%     
- Complexity   17191    17195      +4     
==========================================
  Files          429      429             
  Lines        64845    64878     +33     
==========================================
+ Hits         30275    30284      +9     
- Misses       34570    34594     +24     
Impacted Files Coverage Δ
plugins/woocommerce/includes/class-wc-ajax.php 4.3% <0.0%> (-<0.1%) ⬇️
...ugins/woocommerce/includes/class-wc-post-types.php 2.7% <0.0%> (-0.1%) ⬇️
...ludes/tracks/events/class-wc-settings-tracking.php 0.0% <0.0%> (ø)
...ocommerce/includes/abstracts/abstract-wc-order.php 86.6% <50.0%> (-0.1%) ⬇️
...gins/woocommerce/includes/wc-product-functions.php 44.7% <100.0%> (+1.0%) ⬆️

... and 1 file with indirect coverage changes

Copy link
Contributor

@nathanss nathanss left a comment

Choose a reason for hiding this comment

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

Tested fine and code looks good, nice job!. Just left a suggestion on one part.

@@ -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.

@mattsherman mattsherman merged commit ffc5b91 into trunk Mar 17, 2023
18 of 20 checks passed
@mattsherman mattsherman deleted the update/bypass-actions-for-customer-updates branch March 17, 2023 14:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
focus: analytics focus: performance The issue/PR is related to performance. plugin: woocommerce Issues related to the WooCommerce Core plugin. type: enhancement The issue is a request for an enhancement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Analytics: Bypass scheduled actions for customer updates
2 participants