Fix missing dispatch calls for recordEvent thunk

[ilyasfoo]

Changes proposed in this Pull Request

This PR attempts to fix the issue with the following tracks are not consistently recorded:

• calypso_plugin_installed_success
• calypso_plugin_installed_error
• calypso_plugin_activated_success
• calypso_plugin_activated_error

Upon my investigation, it seemed that refactoring done for reduxifying actions somewhen in December 2020 might have affected the tracking done. It was found that:

• Dispatch call was missing for recordEvent
• site parameter was used instead of siteId

This change also attempts to fix possible similar issue for the following tracks and mcstats:

• calypso_plugin_autoupdate_enabled_*
• calypso_plugin_autoupdate_disabled_*
• calypso_plugin_removed_*

Testing instructions

Unfortunately, I do not know if there are ways to test all of the changes, but I think testing one of these should verify that the change works across the board, since the pattern used is the same.

The following is the instruction to test with plugin installation and activation, please let me know if you do know how to test others:

1. Run calypso in your development environment.
2. Create a new business site.
3. Install Hello Dolly plugin to turn it into Atomic site.
4. Navigate to /woocommerce-installation/{your new site}.
5. Open up developer console, type in localStorage.setItem( 'debug', 'calypso:analytics*' ) to debug tracks.
6. Click on Set up my store!.
7. The site will redirect once setup is completed, so it's good to select Preserve log so that it stays after redirect.
8. If you successfully set up WooCommerce, notice the calypso_plugin_installed_success track appears in console

Sample screenshot of my developer console:

[matticbot]

Test live: https://calypso.live/?branch=fix/plugin-actions-missing-dispatch

[matticbot]

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~241 bytes removed 📉 [gzipped])

name                      parsed_size           gzip_size
zoninator                       -36 B  (-0.0%)      -40 B  (-0.1%)
wp-super-cache                  -36 B  (-0.0%)      -40 B  (-0.0%)
woocommerce-installation        -36 B  (-0.0%)      -38 B  (-0.1%)
settings-performance            -36 B  (-0.0%)      -41 B  (-0.0%)
settings                        -36 B  (-0.0%)      -43 B  (-0.0%)
plugins                         -36 B  (-0.0%)      -38 B  (-0.0%)
plans                           -36 B  (-0.0%)      -38 B  (-0.0%)
activity                        -36 B  (-0.0%)      -41 B  (-0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Async-loaded Components (~113 bytes removed 📉 [gzipped])

name                                                         parsed_size           gzip_size
async-load-design-playground                                       -36 B  (-0.0%)      -37 B  (-0.0%)
async-load-design                                                  -36 B  (-0.0%)      -37 B  (-0.0%)
async-load-calypso-my-sites-site-settings-seo-settings-form        -36 B  (-0.0%)      -39 B  (-0.1%)
async-load-calypso-my-sites-sidebar-unified                        -36 B  (-0.0%)      -37 B  (-0.1%)

React components that are loaded lazily, when a certain part of UI is displayed for the first time.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

[tyxla]

Great catch @ilyasfoo! Looks and works great.

Thanks a bunch for fixing. 🙌

[mattsherman]

@ilyasfoo I never got any calypso_plugin_activated_success events logged. Any ideas why that would be?

[ilyasfoo]

@mattsherman Hmm, that sounds like the behaviour prior this fix. I'm suspecting it could be your cache? If possible, can you look if the code does run in your local, maybe attaching a debugger statement?

[mattsherman]

@mattsherman Hmm, that sounds like the behaviour prior this fix. I'm suspecting it could be your cache? If possible, can you look if the code does run in your local, maybe attaching a debugger statement?

I'm baffled. I have added a number of console.log() statements to the file in this PR. recordInstallPluginEvent gets called (where calypso_plugin_installed_* is recorded) -- so, it is definitely using the updated file from this PR (I usually run with "Disable cache" enabled to make sure that I don't encounter caching issues when testing). But, activatePlugin() is never called (where calypso_plugin_activated_* is recorded). But, I tracing through the code I don't think it ever would (when going through the /woocommerce-installation flow), since it looks like it would be using the SitePlugin.activate() method to activate. So, I don't understand how you are getting the calypso_plugin_activated_success event in that flow!

I don't want to hold up the merge of this PR if I am just doing something silly in my testing. But, I also want to make sure that there is not a different code path than what you expect that is somehow being activated.

[ilyasfoo]

@mattsherman Ah, you're right on that one! My bad, during my testing I deleted WooCommerce, WooCommerce services, and WooCommerce stripe plugin in order to re-test. I think woocommerce-gateway-stripe plugin did not end up being deleted because it was loading for a while but I chose to just ignore it.. I only had calypso_plugin_activated_success recorded because I think the plugin was only deactivated, not deleted at the time!

So the correct flow would only record calypso_plugin_installed_success. Thank you for testing so thoroughly 🙇

[tyxla]

For the protocol, I tested this in the regular plugin management flows.

[mattsherman]

For the protocol, I tested this in the regular plugin management flows.

Thanks for the clarification @tyxla -- I was pretty certain you must have tested with different flows, but it is good to know that for sure!

[mattsherman]

So the correct flow would only record calypso_plugin_installed_success. Thank you for testing so thoroughly 🙇

Thanks for confirming this @ilyasfoo -- I'm glad to see that I wasn't off base.