Fix Upgrade and Subsite Registration Redirects

[mdawaffe]

Fixes #5081.

Changes proposed in this Pull Request:

By default, Jetpack::activate_default_modules() redirects on error and queues an eventual redirect on success. This method is called in a few places where those redirects are not desired.

• Never redirect during automatic upgrades. Automatic upgrades can be triggered by any visitor on init.
• Never redirect during a Jetpack_Network->do_subsiteregister() call. Some of these calls are hooked to Core hooks, which can result in unexpected redirect behavior. Calls to Jetpack_Network->do_subsiteregister() should do their own error/success handling.
• Default to not redirecting during non-wp-admin/ requests.
• Default to not redirecting during REST, XML-RPC, AJAX, CRON, CLI requests.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

No: Bug fix.

Testing instructions:

It's a huge pain :(

For do_subsiteregister(), create a custom CLI command as in #5081.

For upgrade requests, you have to:

1. Deactivate the Protect module.
2. Set the "First Introduced" header in the Protect module to "7.6.5".
3. Open a new browser tab. In that browser tab, open the Network dev tools pane.
4. wp jetpack options update version 7.6
5. Quickly load the homepage (before some other request goes to your site).
6. Inspect the network request for the homepage load.

Prior to this PR, you'll see a Location header. Depending on what else is going on on your site, the request will 200, so there won't actually be a redirect, though you can still see the header. (It may 301 is some circumstances, in which case, you will see the redirect.)

After this PR, there's no Location header and no redirect.

Proposed changelog entry for your changes:

• Do not redirect during the automatic upgrades.

[jetpackbot]

	Warnings
⚠️	The PR is missing at least one [Status] label. Suggestions: [Status] In Progress, [Status] Needs Review

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 383f9e4

[mdawaffe]

I think we should get rid of Jetpack::activate_default_modules() success redirect entirely:

jetpack/class.jetpack.php

Line 2865 in f892020

wp_safe_redirect( Jetpack::admin_url( 'page=jetpack' ) );

It was originally one of several failure redirects. As the code evolved, it (I think accidentally) got attached to the success state as well.

I think all calling code handles that redirect independently, so it serves no purpose.

I didn't want to do that in this PR, though, so close to 7.7.

Much of that module activation code needs cleaning up. It needs better ways to return errors: I don't think the Jetpack::state() code is used much anymore.

[jeherve]

This seems to work, so I'll be merging this now. I'd love to get more eyes on this during the Beta though.

@iandunn @nickdaugherty @GaryJones Any chance you could test that change in your respective environments during the Beta?

Thank you!

[nickdaugherty]

Apologies @jeherve - I missed your message. 7.8 has been deployed to VIP Go and I didn't notice any redirect issues this time. I believe it only happens when new default modules are activated and I'm not sure if 7.8 had any, but it was a smooth upgrade.

Thanks for working on this!