Automattic / woocommerce-subscriptions-core

Subscriptions core package for WooCommerce
Other
85 stars 31 forks source link

Fix PHPCS violations in `WC_Subscriptions_Tracker` to improve code quality #377

Closed Jinksi closed 1 year ago

Jinksi commented 1 year ago

Based on #374. Please wait for it to be merged before reviewing/merging.

Description

This PR only fixes PHPCS violations in the WC_Subscriptions_Tracker class to improve code quality. No changes to the underlying logic are expected.

How to test this PR

  1. Enable WC Tracking in WC Settings → Advanced → WooCommerce.com → Enable Tracking.
  2. Run WC_Tracker::get_tracking_data()['extensions']['wc_subscriptions']['settings'] with the WP Console plugin. The output should be identical on trunk and this branch. Toggle settings such as enable_early_renewal_via_modal to check the enabled/disabled output.
  3. Run ./vendor/bin/phpcs includes/class-wc-subscriptions-tracker.php in your terminal. On trunk you should see errors/warnings. On this branch, you should not see any violations. Note: running phpcs directly from ./vendor/bin/ skips the "only changed LOC" rule that we use for PHPCS GH checks.

Product impact

Jinksi commented 1 year ago

@shendy-a8c Hmmm 😕 I can't see any obvious reason for this error based on the changes in this branch.

Do you get this error on trunk?

shendy-a8c commented 1 year ago

I tried trunk and this PR's branch, with HPOS disabled and enabled. I got that error on all. Let me try to investigate why.

Jinksi commented 1 year ago

This could be due to #374, which has been merged. Do you see this error when using subs-core 5.1.0?

Otherwise, maybe it has something to do with WC version or PHP version. 🤔

Jinksi commented 1 year ago

It looks like @shendy-a8c's error was due to the WC Tracking setting being disabled.

I've added the following to the testing instructions:

  1. Enable WC Tracking in WC Settings → Advanced → WooCommerce.com → Enable Tracking.