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

Add plugin installation request track for core profiler #39533

Merged
merged 6 commits into from Aug 17, 2023

Conversation

ilyasfoo
Copy link
Contributor

@ilyasfoo ilyasfoo commented Aug 2, 2023

Changes proposed in this Pull Request:

Closes #39484.

This PR adds the track wcadmin_coreprofiler_store_extensions_continue with props shown, selected, and unselected when a user clicks on continue in the plugins step in core profiler.

  • shown are plugins that are shown to the user whether it's installed & activated or not
  • selected are plugins that are selected
  • unselected are plugins that are unselected

How to test the changes in this Pull Request:

Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:

  1. In a fresh site, open your browser developer console and run localStorage.setItem( 'debug', 'wc-admin:*' ); to set debug for tracks.
  2. Ensure no the following plugins are not already installed & activated: Jetpack, Pinterest, MailPoet, Google Listing & Ads
  3. Go to Onboarding wizard > Business Info step (/wp-admin/admin.php?page=wc-admin&path=%2Fsetup-wizard&step=business-info)
  4. Choose Afghanistan as the store country to have limited plugins
  5. Click on Continue
  6. Deselect Jetpack and MailPoet and click on Continue
  7. Observe that the track debug is shown along with its props
    image
  8. Wait until the installation finishes and press the back button to go back to the plugins step
  9. Leave everything checked and click on Continue
  10. Observe that the track debug is shown along with its props
    image

Changelog entry

  • Automatically create a changelog entry from the details below.

Significance

  • Patch
  • Minor
  • Major

Type

  • Fix - Fixes an existing bug
  • Add - Adds functionality
  • Update - Update existing functionality
  • Dev - Development related task
  • Tweak - A minor adjustment to the codebase
  • Performance - Address performance issues
  • Enhancement - Improvement to existing functionality

Message

Comment

@github-actions github-actions bot added the plugin: woocommerce Issues related to the WooCommerce Core plugin. label Aug 2, 2023
@ilyasfoo ilyasfoo requested review from a team, chihsuan and moon0326 August 2, 2023 10:22
@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

Hi @chihsuan, @moon0326, @woocommerce/ghidorah

Apart from reviewing the code changes, please make sure to review the testing instructions as well.

You can follow this guide to find out what good testing instructions should look like:
https://github.com/woocommerce/woocommerce/wiki/Writing-high-quality-testing-instructions

recordEvent( 'coreprofiler_store_extensions_continue', {
shown: event.payload.pluginsShown || [],
selected: event.payload.pluginsSelected || [],
unselected: event.payload.pluginsUnselected || [],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even though it's possible to infer some values, we have both selected and unselected so that it's easy to query in analytics.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 2, 2023

Test Results Summary

Commit SHA: 1a0ab2b

Test 🧪Passed ✅Failed 🚨Broken 🚧Skipped ⏭️Unknown ❔Total 📊Duration ⏱️
API Tests25900202610m 49s
E2E Tests1950015021021m 8s

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.

Copy link
Member

@chihsuan chihsuan left a comment

Choose a reason for hiding this comment

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

LGTM and tested well. 👍

@rjchow
Copy link
Contributor

rjchow commented Aug 3, 2023

can you build funnels with nested properties?

{
    shown:
        {
            jetpack: true,
            woopayments: false,
        },
    selected:
        {
            jetpack: true,
        }
}

and split on shown.jetpack == true, etc?

I'm thinking it would be quite troublesome to do funnel analysis with the comma separated tuple sets

alternatively, maybe

{
    jetpack: "shown, selected",
    woopayments: "shown",
}

@ilyasfoo
Copy link
Contributor Author

ilyasfoo commented Aug 4, 2023

alternatively, maybe

{
    jetpack: "shown, selected",
    woopayments: "shown",
}

@rjchow I think your alternative proposal might make it possible to make some limited analysis with tracks funnel, where not possible with the current design. However, I'm unsure:

  • It'll be structured differently than what the other track wcadmin_coreprofiler_store_extensions_installed_and_activated has with plugins.
  • If the query to be used on looker is going to be more complicated

@moon0326 Do you have any opinion on whether using slightly different prop structure for this track?

@moon0326
Copy link
Contributor

moon0326 commented Aug 9, 2023

alternatively, maybe

{
    jetpack: "shown, selected",
    woopayments: "shown",
}

@rjchow I think your alternative proposal might make it possible to make some limited analysis with tracks funnel, where not possible with the current design. However, I'm unsure:

  • It'll be structured differently than what the other track wcadmin_coreprofiler_store_extensions_installed_and_activated has with plugins.
  • If the query to be used on looker is going to be more complicated

@moon0326 Do you have any opinion on whether using slightly different prop structure for this track?

Sorry for the delay 🙏

Do you have any opinion on whether using slightly different prop structure for this track?

WIth my limited experience with funnel, what about we use the following name pattern?

:plugin_name_shown: true
:plugin_name_selected:true

jetpack_shown: true
jetpack_selected: true
and so on

@ilyasfoo
Copy link
Contributor Author

ilyasfoo commented Aug 15, 2023

@moon0326 It would be up to 24 props in total for our plugins, however I think it might work

Edit: actually, plugin slugs are not valid for track prop name since there's dashes, i.e: google-listings-and-ads.. I'm thinking that might not be possible without bludgeoning plugin slug.. Maybe we can use my original PR instead. What do you think? cc @rjchow

@rjchow
Copy link
Contributor

rjchow commented Aug 16, 2023

As long as it satisfies the needs of whoever might use it, I'm not hung up about it 😆

@ilyasfoo ilyasfoo merged commit 3e9c14d into trunk Aug 17, 2023
26 checks passed
@ilyasfoo ilyasfoo deleted the add/39484-additional-plugins-step-tracks branch August 17, 2023 14:34
@github-actions github-actions bot added this to the 8.1.0 milestone Aug 17, 2023
@github-actions github-actions bot added the needs: analysis Indicates if the PR requires a PR testing scrub session. label Aug 17, 2023
samueljseay pushed a commit that referenced this pull request Aug 18, 2023
* Add track to plugin installation in core profiler

* Rename available to shown

* Add changelog

* Lint
@nigeljamesstevenson nigeljamesstevenson added status: analysis complete Indicates if a PR has been analysed by Solaris and removed needs: analysis Indicates if the PR requires a PR testing scrub session. labels Aug 18, 2023
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. status: analysis complete Indicates if a PR has been analysed by Solaris
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Additional track changes on Core Profiler extensions step
5 participants