Block Directory: Use plugin API for installing & deleting block-plugins #23219
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ryelle
added
the
[Feature] Block Directory
ryelle
commented
Jun 16, 2020
| }, | ||
| method: 'POST', | ||
| } ); | ||
| yield addInstalledBlockType( block ); | ||
| yield addInstalledBlockType( { ...block, plugin: response.plugin } ); |
There was a problem hiding this comment.
This is a bit of a workaround, since we need the main plugin file for uninstallation (ex, blissful-buttons/blissful-buttons.php, or wp-p5js-block/plugin.php). I'm not sure if we can/should try adding this to the block directory response (from wp.org), or leave this as-is.
There was a problem hiding this comment.
I don't think the block directory response could return that info nor should it.
I'm fine with this approach for now, but since plugin is a pretty generic property we could run into collisions in the future.
TimothyBJacobs
approved these changes
Jun 16, 2020
| @@ -94,18 +95,19 @@ export function* installBlockType( block ) { | |||
| * @param {Object} block The blockType object. | |||
| */ | |||
| export function* uninstallBlockType( block ) { | |||
| const { id } = block; | |||
| const plugin = block.plugin.replace( '.php', '' ); | |||
There was a problem hiding this comment.
This is somewhat unexpected, is the plugin file the only way to get the slug?
There was a problem hiding this comment.
Right now the plugins controller returns the file. This could be bypassed by using the self link.
I don't know if the controller should return the file ie my-plugin/init.php or my-plugin/init for the plugin field. The former is the "real" value, but the latter is what will get used in the URL. I'm very much open to thoughts.
|
Ran this locally. Works fine. |
StevenDufresne
approved these changes
Jun 17, 2020
|
Could this be rebased? Not sure if the test failures are legitimate. |
ellatrix
force-pushed
the
update/block-dir-use-plugin-eps
branch
from
6c9ac20 to
fa992a7
Compare
|
Ok, I rebased it. |
|
Size Change: +640 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
The install and uninstall routes no longer exist on this controller, as they've moved to the plugin controller.
|
Looks like there's some e2e test failures. |
noisysocks
pushed a commit
that referenced
this pull request
Jun 24, 2020
…ns (#23219) * Block Directory: Use plugin API for installing & deleting block-plugins * Remove the install & uninstall endpoints from the block-directory * Set the endpoint for the block by using the self link property * Remove erroneous route tests The install and uninstall routes no longer exist on this controller, as they've moved to the plugin controller. * Update e2e install mock url to fix tests. Co-authored-by: tellyworth <[email protected]> Co-authored-by: dufresnesteven <[email protected]>
This was referenced Jun 24, 2020
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
The
/pluginsendpoints were introduced in #22454, and while/block-directory/installand/block-directory/uninstallwere updated to use those endpoints in PHP, we can actually remove those endpoints and call thepluginsmethods directly. See #22454 (comment)This PR updates the install flow to call
POST __experimental/plugins, and the uninstall flow to callPUT __experimental/plugins/${ plugin }(to deactivate) andDELETE __experimental/plugins/${ plugin }to remove the files.It also removes the install & uninstall endpoints from
__experimental/block-directory, and updates all the tests.How has this been tested?
The php & js unit tests have been updated. I've also tested the block directory flow in browser (chrome):
Checklist: