JSON API: Add new `action_links` field to plugins endpoint #6267

roundhill · 2017-02-02T20:11:50Z

Over in Automattic/wp-calypso#82 we'd like to show any action links that the plugin uses in the Calypso plugins page. This PR adds support for returning these action links via the /sites/example.com/plugins endpoint.

The only way to get the links is in an HTML tag format, so this parses out the url and link name and returns them in an array. Because the plugin developer can return anything they want in these links, I used DOMDocument to handle the parsing to be safest. Other ideas are welcome!

To test

Apply Diff D4247 to your wp.com sandbox to whitelist the new field.
Do a get to the /sites/your-jetpack-dev-site/plugins endpoint.
You should see an empty action_link array if no action links were added, or an array of data if it exists for the plugin:

The only way to get the links in an HTML tag format, so this parses out the url and link name and returns them in an array.

jeherve · 2017-02-03T18:07:07Z

json-endpoints/jetpack/class.jetpack-json-api-plugins-endpoint.php

+	 protected function get_plugin_action_links( $plugin_file ) {
+		 $formatted_action_links = array();
+
+		 $action_links = apply_filters( 'plugin_action_links', null, $plugin_file, null, 'all' );


Could you add a docblock here? Something like /** This filter is documented in src/wp-admin/includes/class-wp-plugins-list-table.php */ should work and will make our parser happier.

Thanks!

Added exactly that in 309f823

roundhill · 2017-02-07T17:03:17Z

I wasn't sure about API tests for Jetpack endpoints, do we still run them from wp.com?

enejb · 2017-02-07T18:33:55Z

Why not use the full url instead of relying on a partial url?

roundhill · 2017-02-07T22:23:52Z

I've realized this isn't working so well because most plugins will hook into admin_init which isn't available when this endpoint runs. Exploring solutions...

enejb · 2017-02-08T00:09:02Z

The work done by @artpi in this PR would be a good alternative solution see #5757

… action links. Could be scary for a number of reasons including performance of the endpoint. Also run the new plugin_actoin_links filter that keys on the filename of the plugin.

roundhill · 2017-02-08T23:06:58Z

I've got a solution working to capture the links, but it requires performing the admin_init action in the endpoint. I'm quite concerned about this though as it seems like not a very "normal" WordPress thing to do. Performance-wise it seems to add about 500ms to the /site/%s/plugins endpoint.

If anyone has attempted something like this before, please let me know how crazy this is :)

roundhill · 2017-02-08T23:07:43Z

Why not use the full url instead of relying on a partial url?

I added support for this in 7944833

roccotripaldi

This works well. I'm sorry it took me so long to review.
I cannot think of a way to get action_links without calling admin_init.

I'm quite concerned about this though as it seems like not a very "normal" WordPress thing to do.

Perhaps not, but it does make sense if you think about it. With this endpoint, we are exposing a list of plugins and their meta data, information which is only available in wp-admin.

I do appreciate you considering performance, but as you said, many plugins will hook to admin_init to provide this information, and we would be excluding them.

Also, we should probably add the same action_link field to the endpoint for a single plugin: /sites/$site/plugins/jetpack%2Fjetpack for example.

roundhill · 2017-02-21T19:10:16Z

Not sure when the next beta is going out, but for Automated Transfer's sake it'd be great to get this API merged soon.

jeherve · 2017-02-21T21:35:52Z

@roundhill The Beta will be available next week. We'll see about reviewing this PR and merging it until then! 👍

singerb · 2017-02-22T11:55:25Z

LGTM. Can't speak to performance much, but given that we seem to be stuck running admin_init for this to work, I don't think there's much else we can do there.

zinigor · 2017-02-22T15:33:28Z

json-endpoints/jetpack/class.jetpack-json-api-plugins-endpoint.php

+		 /** This filter is documented in src/wp-admin/includes/class-wp-plugins-list-table.php */
+		 $action_links = apply_filters( "plugin_action_links_{$plugin_file}", $action_links, $plugin_file, null, 'all' );
+		 if ( count( $action_links ) > 0 ) {
+			 $dom_doc = new DOMDocument;


This is great, I love seeing documents parsed with DOMDocument, but the problem is that sometimes it may be unavailable if the XML module is disabled. While it's probably a rather small edge case, it would cause a fatal for the whole endpoint response. What do you think can be done about it? I'd hate to resort to regex parsing in that case, can we just omit action links in case DOMDocument isn't available?

I suppose it's not the end of the world if we can't get the links for sites with the XML module disabled. Would a try/catch be appropriate here to check that DOMDocument is available?

@zinigor Adding a class_exists check seemed to do the trick. Added in aa526d0.

Do we have a way of checking how many Jetpack sites have DOM/XML disabled?

No, I don't think we have any means of knowing that percentage. My hope is that it will only affect sites that are running on manually configured servers. Thanks for adding that change!

@see

* Changelog: update stable tag and move changelog to changelog.txt Also remove old releases from readme.txt to keep the changelog tab short. * Changelog: add #5883 Also update the filter's docblock to match new version. * Changelog: add #5938 * Changelog: add #6298 * Changelog: add #3405 * Changelog: add #5941 * Changelog: add #6239 * Changelog: add #6281 * Changelog: add #6303 * Changelog: add #6018 * Changelog: add #6300 * Changelog: add #6296 * Changelog: add #6130 * Changelog: add #6292 * Readme: remove extra "on". * Changelog: add #6307 * Changelog: add #3297 * Changelog: add #6275 * Changelog: add #6321 * Changelog: add #6297 * Readme: update the support forum link anchor. Anchor changed when WordPress.org forums were updated to bbPress 2 * Readme: update list of a12s, it wasn't up to date anymore! * Changelog: add #6338 * Changelog: add #6337 * Changelog: add #6335 * Changelog: add #6333 * Testing List: first version of the 4.7 testing list. * Changelog: add #6332 * Changelog: add #6325 * Changelog: add #6326 * Changelog: add #6339 * Changelog: add #6342 * Changelog: add #6343 * Changelog: add #6346 * Changelog: add #6347 * Changelog: add #6279 * Changelog: add #6306 * Changelog: add #6312 * Changelog: add #6316 * Changelog: add #6171 * Changelog: add #6317 * Changelog: add #6246 * Changelog: add #6263 * Changelog: add #4220 * Changelog: add #5888 * Changelog: add #3406 * Changelog: add #3637 * Changelog: add #6320 * Changelog: add #5992 * Changelog: add #6322 * Changelog: add #6324 * Changelog: add #6352 * Changelog: add #6355 * Changelog: add #6360 * Changelog: add #6362 * Changelog: add #6369, #6382 * Changelog: add #6370 * Changelog: add #6375 * Changelog: add #6383 * Changelog: add #6384 * Changelog: add #6386 * Changelog: add #6395 * Changelog: add #6403 * Changelog: add #6406 * Changelog: add #6418 * Changelog: add #6419 * Changelog: add #6434 * Changelog: add #6446 * Changelog: add #6006 * Changelog: add #6096 * Changelog: add #6399 * Changelog: fix typo. @see #6331 (comment) * Changelog: add #6440 * Changelog: add #6443 * Changelog: add #6445 * Changelog: add #6463 * Changelog: add #6468 * Changelog: add #6471 * Changelog: add #6474 * Changelog: add #6480 * Changelog: add #6497 * Changelog: add #6499 * Changelog: add #6514 * Changelog: add #6267 * Changelog: add #5940 * Changelog: add #6492 * Changelog: add #5281 * Changelog: add #6327 * Changelog: add #6451 * Changelog: add #6525 * Changelog: add #6530

… and #6267 Merges r151791-wpcom.

Added new action_links field to plugins endpoint.

aec5f7b

The only way to get the links in an HTML tag format, so this parses out the url and link name and returns them in an array.

roundhill added [Feature] WPCOM API [Type] Enhancement Changes to an existing feature — removing, adding, or changing parts of it [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 2, 2017

jeherve added the [Pri] Normal label Feb 3, 2017

jeherve requested changes Feb 3, 2017

View reviewed changes

jeherve added [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 3, 2017

Adding a doc comment for the plugin_action_links filter.

309f823

jeherve added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. [Pri] High and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! [Pri] Normal labels Feb 3, 2017

roundhill requested review from roccotripaldi, enejb and lezama February 7, 2017 17:02

roundhill added [Status] In Progress and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 7, 2017

roundhill removed the request for review from lezama February 7, 2017 22:48

roundhill added 2 commits February 8, 2017 14:45

Perform the admin_init action in order to capture all of the plugin…

19d52c2

… action links. Could be scary for a number of reasons including performance of the endpoint. Also run the new plugin_actoin_links filter that keys on the filename of the plugin.

Prepend the full admin url if the link doesn’t have a scheme set.

7944833

roundhill added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] In Progress labels Feb 8, 2017

roccotripaldi requested changes Feb 9, 2017

View reviewed changes

Adding doc comment for parser.

a515d1e

roundhill added [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. and removed [Status] Needs Author Reply We would need you to make some changes or provide some more details about your PR. Thank you! labels Feb 13, 2017

jeherve approved these changes Feb 13, 2017

View reviewed changes

jeherve removed the [Status] In Progress label Feb 21, 2017

jeherve requested a review from dereksmart February 21, 2017 21:36

jeherve added [Pri] BLOCKER and removed [Pri] High labels Feb 21, 2017

singerb added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 22, 2017

zinigor requested changes Feb 22, 2017

View reviewed changes

Add protection against the DOMDocument class not being available.

aa526d0

zinigor approved these changes Feb 28, 2017

View reviewed changes

zinigor added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review To request a review from fellow Jetpack developers. Label will be renamed soon. labels Feb 28, 2017

dereksmart merged commit d5039f5 into master Feb 28, 2017

dereksmart deleted the add/json-api-plugins-action-links branch February 28, 2017 13:39

dereksmart removed the [Status] Ready to Merge Go ahead, you can push that green button! label Feb 28, 2017

jeherve added the [Status] Has Changelog label Feb 28, 2017

jeherve added a commit that referenced this pull request Feb 28, 2017

Changelog: add #6267

23cf177

tyxla mentioned this pull request May 5, 2017

JSON API: Plugins endpoint breaks characters in plugin action links #7127

Closed

zinigor pushed a commit that referenced this pull request May 11, 2017

REST API: Adding action_links member to plugin endpoints. Refs: D4247…

145ce2c

… and #6267 Merges r151791-wpcom.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

JSON API: Add new `action_links` field to plugins endpoint #6267

JSON API: Add new `action_links` field to plugins endpoint #6267

roundhill commented Feb 2, 2017 •

edited

Loading

jeherve Feb 3, 2017

roundhill Feb 3, 2017

roundhill commented Feb 7, 2017

enejb commented Feb 7, 2017

roundhill commented Feb 7, 2017

enejb commented Feb 8, 2017 •

edited

Loading

roundhill commented Feb 8, 2017

roundhill commented Feb 8, 2017

roccotripaldi left a comment

roundhill commented Feb 21, 2017 •

edited

Loading

jeherve commented Feb 21, 2017

singerb commented Feb 22, 2017

zinigor Feb 22, 2017

roundhill Feb 22, 2017

roundhill Feb 22, 2017

zinigor Feb 28, 2017

JSON API: Add new action_links field to plugins endpoint #6267

JSON API: Add new action_links field to plugins endpoint #6267

Conversation

roundhill commented Feb 2, 2017 • edited Loading

jeherve Feb 3, 2017

Choose a reason for hiding this comment

roundhill Feb 3, 2017

Choose a reason for hiding this comment

roundhill commented Feb 7, 2017

enejb commented Feb 7, 2017

roundhill commented Feb 7, 2017

enejb commented Feb 8, 2017 • edited Loading

roundhill commented Feb 8, 2017

roundhill commented Feb 8, 2017

roccotripaldi left a comment

Choose a reason for hiding this comment

roundhill commented Feb 21, 2017 • edited Loading

jeherve commented Feb 21, 2017

singerb commented Feb 22, 2017

zinigor Feb 22, 2017

Choose a reason for hiding this comment

roundhill Feb 22, 2017

Choose a reason for hiding this comment

roundhill Feb 22, 2017

Choose a reason for hiding this comment

zinigor Feb 28, 2017

Choose a reason for hiding this comment

JSON API: Add new `action_links` field to plugins endpoint #6267

JSON API: Add new `action_links` field to plugins endpoint #6267

roundhill commented Feb 2, 2017 •

edited

Loading

enejb commented Feb 8, 2017 •

edited

Loading

roundhill commented Feb 21, 2017 •

edited

Loading