Beta plugin: Support managing multiple plugins#20168
Conversation
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available. Once your PR is ready for review, check one last time that all required checks (other than "Required review") appearing at the bottom of this PR are passing or skipped. Beta plugin:
|
22952e0 to
8c6ee25
Compare
8d2b819 to
73a755f
Compare
JPBETA__PLUGIN_FILE and JPBETA_DEFAULT_BRANCH are unused. Several others assume Jetpack; group them for deletion.
Eventually all the stuff that varies by plugin will live here.
This one was pretty ok to start with. - Declare a field that was being used. - Fix up doc blocks. - Remove some unnecessary two-line functions, which lets us get rid of another field. - Remove some unneeded `empty()` calls. - Simplify a weirdly overcomplicated conditional. - Privatize methods that aren't used externally. - Rename a misspelled method, `has_never_version` → `has_newer_version`. - Fix semantics in `plugins_api` filter method `get_plugin_info()`. - Document why `upgrader_source_selection` is being filtered at all.
The `activate` command won't work yet, because I haven't written the method to actually do the installation. Other than that it should work.
If we're going to try to delete transients, let's try to get them all. So rename the ones from the self-autoupdater to use a common prefix with the rest of the extension, then select the list of transients from the database.
The menu option wasn't being added if Jetpack (not Beta) was network-activated, I guess for access control. In a multiple-plugin world, we'll need to do that access control inside the admin page based on the plugin selected. Similarly, Utils::admin_url() was generating links to regular Admin or Network Admin based on whether Jetpack (not Beta) was network-activated. That too will need to depend on if it's generating a link to the "manage" page for a network-activated plugin or not. Several hooks needed updating for a multiple-plugin world where some might be network-activated and some might not be. To avoid potential network hits on every page load from those hooks if transients are being particularly transient, cache a map of known plugin files in the options. There's still one use left in Utils::replace_active_plugin(); that needs refactoring anyway, so I'll get rid of the last use when I do that.
Just return false, the standard handler does the right thing (since PHP 4.3.10!).
Creates a PluginInstaller class to hold that logic instead. One call remains, from the UI. This also gets rid of Utils::update_plugin(), also in the UI. And we clean up Hooks::deactivate() to handle multiple plugins.
And link to Network Admin if applicable.
Changes of note: * Got rid of the JETPACK_GREEN constant, because we need different greens in different places. * Logo changed to Green 40, per https://git.io/JcWou * Admin bar highlight color changed to Green 50, per pcdRpT-if-p2 * The admin bar menu gets an extra level. * The autoupdater email loses the attempt at "what changed?" and testing instructions. It would be too much if multiple plugins get updated. Instead, each plugin listed has a link to the PR or mirror repo branch. * Pulling in composer/semver to compare versions, since PHP's `version_compare()` doesn't really work right.
I decided to more heavily use "template" files rather than embedding `<?php ?>` in the class file. I still need to do something about Admin::to_test_content() and Utils::what_changed(), and go through to remove now-unused methods.
And merge Utils::what_changed() into it. Also, instead of using Jetpack's markdown library when it happens to be available, just pull in one that explicitly handles GitHub-Flavored Markdown.
It didn't turn out to be as big as I thought.
0f73b06 to
469750c
Compare
469750c to
2ef4293
Compare
For example, if the -dev dir was left from an older version of Jetpack Beta.
8cd5063 to
3eeb04e
Compare
Mostly to prevent the possibility of accidentally managing a network-enabled plugin when our hook for preventing duplicate versions can't run in time.
kraftbj
left a comment
There was a problem hiding this comment.
It would be nice to have a gentle fail if someone activates the plugin without running composer install (like we do for jetpack itself), but that isn't blocking. Let's merge it!
After PR: #20168 There were a few element not found errors since the plugin layout was adjusted; this PR cleans those errors up.
After PR: #20168 There were a few element not found errors since the plugin layout was adjusted; this PR cleans those errors up.
After PR: Automattic/jetpack#20168 There were a few element not found errors since the plugin layout was adjusted; this PR cleans those errors up. Committed via a GitHub action: https://github.com/Automattic/jetpack/actions/runs/1045899249
Changes proposed in this Pull Request:
We want to be able to support testing plugins other than Jetpack, such as the upcoming Jetpack Backup plugin. We've already changed the build, and the change to the Beta Download website is pending. This updates the plugin itself.
Jetpack product discussion
p9dueE-362-p2
Does this pull request change what data or activity we track or use?
No.
Testing instructions:
git clone --branch add/beta-plugin-multiple-plugins --depth=20 https://github.com/Automattic/jetpackcd jetpack/projects/plugins/beta/composer installcd ~/apps/$USER/public/wp-content/pluginsrm -rf jetpack-betaln -s ~/jetpack/projects/plugins/beta jetpack-beta