Skip to content

Conversation

@chihsuan
Copy link
Member

@chihsuan chihsuan commented Oct 31, 2022

All Submissions:

Changes proposed in this Pull Request:

Closes 103-gh-woocommerce/team-ghidorah.

we're passing packed objects to tracks, which causes it to have object names:

Instead of passing the objects, this PR change to pass strings to tracks. Besides, I also fixed the error_message translation string and docblock lint issues.

  • This PR is a very minor change/addition and does not require testing instructions (if checked you can ignore/remove the next section).

How to test the changes in this Pull Request:

  1. Install Code Snippets and woocommerce-beta-tester
  2. Create a new snipeet with the following code
function pre_install_plugins( $value ) {
    return array( 'invalid-plugin-slug' );
}

add_filter( 'woocommerce_admin_plugins_pre_install', 'pre_install_plugins', 999 );
  1. Go to WooCommerce > Home
  2. Go to marketing task
  3. Click "Activate" button on Google Listings & Ads
  4. Visit the WooCommerce->Status->Logs page wp-admin/admin.php?page=wc-status&tab=logs in a new tab
  5. Select a tracks-* log from the dropdown
  6. Search the wcadmin_install_plugin_error string
  7. Observe that all event props are in snake_case like below:
2022-10-31T08:38:13+00:00 DEBUG wcadmin_install_plugin_error
2022-10-31T08:38:13+00:00 DEBUG   - error_message: The requested plugin `invalid-plugin-slug` could not be installed. Plugin API call failed.
2022-10-31T08:38:13+00:00 DEBUG   - api: Plugin not found.
2022-10-31T08:38:13+00:00 DEBUG   - slug: invalid-plugin-slug
  1. Update the snippets with the following code
function plugins_api_result( $res, $action, $args ) {
	$res->download_link = 'invalid link';
    return $res;
}

add_filter( 'plugins_api_result', 'plugins_api_result', 999, 3 );
  1. Repeat 4~9 steps
  2. Observe that all event props are in snake_case like below:
2022-10-31T08:44:41+00:00 DEBUG wcadmin_install_plugin_error
2022-10-31T08:44:41+00:00 DEBUG   - error_message: The requested plugin `google-listings-and-ads` could not be installed.
2022-10-31T08:44:41+00:00 DEBUG   - slug: google-listings-and-ads
2022-10-31T08:44:41+00:00 DEBUG   - api_version: 2.2.0
2022-10-31T08:44:41+00:00 DEBUG   - api_download_link: invalid link
2022-10-31T08:44:41+00:00 DEBUG   - upgrader_skin_message: Downloading installation package from invalid link…,Download failed. A valid URL was not provided.
2022-10-31T08:44:41+00:00 DEBUG   - result: 

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?

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.

@github-actions github-actions bot added focus: react admin plugin: woocommerce Issues related to the WooCommerce Core plugin. labels Oct 31, 2022
@chihsuan chihsuan self-assigned this Oct 31, 2022
@chihsuan chihsuan requested a review from a team October 31, 2022 08:54
@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2022

Test Results Summary

Commit SHA: 28bcf9b

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests24800302510m 48s
E2E Tests186006019214m 21s

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.

@chihsuan chihsuan requested review from a team, SamratBiswas1, ilyasfoo and rjchow and removed request for a team and SamratBiswas1 November 1, 2022 06:21
@chihsuan chihsuan force-pushed the fix/103-install-plugin-error-track branch from 03cc1f0 to 28bcf9b Compare November 1, 2022 06:32
Copy link
Contributor

@ilyasfoo ilyasfoo left a comment

Choose a reason for hiding this comment

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

Tested well! I've confirmed both tracks are recorded without reject in backend. I dont think this requires another review, LGTM!

);
wc_admin_record_tracks_event( 'install_plugin_error', $properties );

/**
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding these comments!

@chihsuan chihsuan merged commit e60be03 into trunk Nov 2, 2022
@chihsuan chihsuan deleted the fix/103-install-plugin-error-track branch November 2, 2022 07:55
@github-actions github-actions bot added this to the 7.2.0 milestone Nov 2, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Nov 2, 2022

Hi @chihsuan, thanks for merging this pull request. Please take a look at these follow-up tasks you may need to perform:

  • Add the release: add testing instructions label

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

plugin: woocommerce Issues related to the WooCommerce Core plugin.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants