isset( $data['installWooCommerce'] ) && $data['installWooCommerce']

can be simplified to:

! empty( $data['installWooCommerce'] )

Several things here:

• this duplicates a lot of what Jetpack_JSON_API_Plugins_Install_Endpoint::install() does 😞 it would be really nice if we can abstract the installation functionality and reuse it in the plugins install endpoint and here as well.
• why the underscore in the method name?

why the underscore in the method name?

I was mimicking the (private) _process_onboarding() method's nomenclature here (since _install_plugin, while not private, is only used by that private method). I'm perfectly fine with dropping it (especially if I move it to a different file and it becomes a public function).

Ah, makes sense. Anyway, I still don't see why we should be prefixing with an underscore - why not just making the method private or protected? That applies to _process_onboarding() too.

I think the idea is just to signal "this method is private" as part of its name (so its immediately visible at call sites). A bit like including type information in a variable name (can't think of anything really ugly now, maybe something like counter_int or __super_ugly_struct_t or CFooBarSubClass 😛).

I still don't follow why prefixing with an underscore would be more immediately visible than having the method defined with protected or private visibility. After all, visibility is specified before the method name. Not to mention that if a method should be considered private, it is beneficial that it should be implemented as such, so it's not inadvertently used as a public one.

Should be bool|WP_Error instead.

This essentially duplicates Jetpack_JSON_API_Plugins_Install_Endpoint::generate_wordpress_org_plugin_download_link() 😞

If we were to use code from there, we'd have to move it to a shared file; not sure that makes sense, since the install endpoint code also included bulk install logic which we don't need.

I this it makes complete sense to use a shared file; the fact that it was done like that in the install endpoint doesn't make it the best solution IMHO, especially in cases like this, where we're forced to duplicate code because of this decision.

I would assume that any plugin installation/activation/deactivation helper methods would not be defined in the endpoint itself. Rationale: see how the same is done in Jetpack_JSON_API_Plugins_Install_Endpoint, and that prevents us from reusing this functionality.

I'd prefer moving these methods in their own library class, just like Jetpack_Widgets, and load that library with jetpack_require_lib() when we need it. We could eventually migrate the endpoint to use these helpers afterwards.

Started de-duping PR #8464, to be merged before this one.

Naming nit: if we have those defined as variables, let's lowercase them in order to conform to the coding standards.

These could make a good use of class constants if we move those methods to a separate library class - in that case we could keep the uppercase names.

activate_plugin will return null on successful activation (yeah, it's weird, I agree), we need to account for that and return true in that case.

activate_plugin will return null on successful activation (yeah, it's weird, I agree), we need to account for that and return true in that case.

Perhaps we can alter both return activate_plugin cases to $activated = activate_plugin( $WOOCOMMERCE_ID );, and then have a single check that relies on ! is_wp_error( $activated ) before actually returning true at the end of the method.

Maybe a good idea to have a ! current_user_can( 'install_plugins' ) permission check too (for both multisites and single sites)?

activate_plugin does not check for any capabilities, so before calling it, we should check whether the user can actually activate plugins (current_user_can( 'activate_plugins' )).