Add activation option workaround to trigger woocommerce_newly_installed action

[martynmjones]

Changes proposed in this Pull Request:

This PR adds an option workaround for new installations of WooCommerce so that the woocommerce_newly_installed hook is triggered the first time admin_init runs after activation.

Additionally, the action name used in CouponPageMoved.php is updated to the newer woocommerce_newly_installed.

Closes #38682

How to test the changes in this Pull Request:

1. Setup a new environment that WooCommerce has not previously been installed on
2. Activate WooCommerce
3. Confirm that the legacy menu WooCommerce > Coupons is not added
4. Go to wp-admin/options.php, search for woocommerce_newly_installed, and confirm the value is no

[github-actions]

Hi @jorgeatorres, @Konamiman,

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

[Konamiman]

Hi @martynmjones, thanks for your contribution. I'd like to request some changes, though.

As we state in our documentation, ideally any new code for WooCommerce should go into the src directory, and changes to code in includes should be kept to the minimum. I think your changes in class-wc-install.php are small enough and reasonable, so no issues here.

However, something we tend to be more strict about (but we don't have properly documented, and that's something we need to improve) is that we don't want to add new public methods to existing classes unless it's absolutely necessary, because once these are implemented we are forced to keep backwards compatibility with it.

However when adding a hook handler to a class the handler method needs to be externally accessible. To this end we have implemented the AccessiblePrivateMethods trait, which allows to use private class methods as hook handlers.

So my request for you is to please use this trait so that the newly_installed method can be private. It's quite easy, you just need to do these adjustments to your changes in class-wc-install.php:

1. Add use Automattic\WooCommerce\Internal\Traits\AccessiblePrivateMethods; to the class file.
2. Add use AccessiblePrivateMethods; right below class WC_Install {.
3. Change the add_action( 'admin_init', array( __CLASS__, 'newly_installed' ) ); so that it's preprended with self::. So: self::add_action( 'admin_init', array( __CLASS__, 'newly_installed' ) );
4. Change the newly_installed method so that it's private.

Please ping me once these changes are implemented. Thank you!

[Konamiman]

By the way, I see that after firing woocommerce_newly_installed you set the "newly installed" option to "no"; wouldn't it be enough to just delete the option? Or am I missing something?

[martynmjones]

Hey @Konamiman, thanks for the review and guidance!

I've made the changes you requests so that AccessiblePrivateMethods is now being used instead of the method being public so it's ready for a second look.

By the way, I see that after firing woocommerce_newly_installed you set the "newly installed" option to "no"; wouldn't it be enough to just delete the option? Or am I missing something?

The option could be deleted and it's unlikely there would be any issues, however, as WC_Install::is_new_install() isn't guaranteed to only return true for brand new installs, setting the newly installed option to no ensures that the woocommerce_newly_installed hook will only run once and then never again in the future.

[Konamiman]

@martynmjones Thanks, just a couple more of minor changes are needed, I tried to do these myself but looks like I don't have edit permissions in your fork:

1. The // Register the woocommerce_newly_installed action using AccessiblePrivateMethods::add_action(). comment is redundant and can be removed (the presence of use AccessiblePrivateMethods should be enough, and the code of the trait itself has already all the relevant information about why it's used).
2. Please change the @since x.x.x in newly_installed to @since 8.0.0.

[martynmjones]

Thanks for the second look, @Konamiman! I've updated the comments as requested in ea8645b.

[jorgeatorres]

LGTM. Thank you @martynmjones!

Changes have been implemented