Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 7 commits into from
Feb 28, 2017
Merged

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

Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
aec5f7b
Added new `action_links` field to plugins endpoint.
roundhill Feb 2, 2017
309f823
Adding a doc comment for the `plugin_action_links` filter.
roundhill Feb 3, 2017
19d52c2
Perform the `admin_init` action in order to capture all of the plugin…
roundhill Feb 8, 2017
7944833
Prepend the full admin url if the link doesn’t have a scheme set.
roundhill Feb 8, 2017
53b1b36
Minor copy correction.
roundhill Feb 9, 2017
a515d1e
Adding doc comment for parser.
roundhill Feb 13, 2017
aa526d0
Add protection against the `DOMDocument` class not being available.
roundhill Feb 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 47 additions & 2 deletions json-endpoints/jetpack/class.jetpack-json-api-plugins-endpoint.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ abstract class Jetpack_JSON_API_Plugins_Endpoint extends Jetpack_JSON_API_Endpoi
'next_autoupdate' => '(string) Y-m-d H:i:s for next scheduled update event',
'log' => '(array:safehtml) An array of update log strings.',
'uninstallable' => '(boolean) Whether the plugin is unistallable.',
'action_links' => '(array) An array of action links that the plugin uses.',
);

protected function result() {
Expand Down Expand Up @@ -69,7 +70,7 @@ protected function validate_input( $plugin ) {
$this->bulk = false;
$this->plugins[] = urldecode( $plugin );
}

if ( is_wp_error( $error = $this->validate_plugins() ) ) {
return $error;
};
Expand Down Expand Up @@ -113,10 +114,11 @@ protected function format_plugin( $plugin_file, $plugin_data ) {
$plugin['network'] = $plugin_data['Network'];
$plugin['update'] = $this->get_plugin_updates( $plugin_file );
$plugin['next_autoupdate'] = date( 'Y-m-d H:i:s', wp_next_scheduled( 'wp_maybe_auto_update' ) );
$plugin['action_links'] = $this->get_plugin_action_links( $plugin_file );

$autoupdate = in_array( $plugin_file, Jetpack_Options::get_option( 'autoupdate_plugins', array() ) );
$plugin['autoupdate'] = $autoupdate;

$autoupdate_translation = in_array( $plugin_file, Jetpack_Options::get_option( 'autoupdate_plugins_translations', array() ) );
$plugin['autoupdate_translation'] = $autoupdate || $autoupdate_translation || Jetpack_Options::get_option( 'autoupdate_translations', false );

Expand All @@ -129,6 +131,9 @@ protected function format_plugin( $plugin_file, $plugin_data ) {
}

protected function get_plugins() {
// Do the admin_init action in order to capture plugin action links.
// See get_plugin_action_links()
do_action( 'admin_init' );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add /** This action is documented in wp-admin/admin.php * here, for the parser?

Thanks!

Copy link
Contributor Author

@roundhill roundhill Feb 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, added in a515d1e. I'm not familiar with the parser, is there documentation about it somewhere?

Copy link
Member

@jeherve jeherve Feb 13, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

Not much I'm afraid, but you can take a look at the result here. The parser itself lives on WordPress.com, and you can check one of the most updates in r130226-wpcom.

$plugins = array();
/** This filter is documented in wp-admin/includes/class-wp-plugins-list-table.php */
$installed_plugins = apply_filters( 'all_plugins', get_plugins() );
Expand Down Expand Up @@ -183,4 +188,44 @@ protected function get_plugin_updates( $plugin_file ) {
}
return null;
}

/**
* Get custom action link tags that the plugin is using
* Ref: https://codex.wordpress.org/Plugin_API/Filter_Reference/plugin_action_links_(plugin_file_name)
* @return array of plugin action links (key: link name value: url)
*/
protected function get_plugin_action_links( $plugin_file ) {
$formatted_action_links = array();

$action_links = array();
/** This filter is documented in src/wp-admin/includes/class-wp-plugins-list-table.php */
$action_links = apply_filters( 'plugin_action_links', $action_links, $plugin_file, null, 'all' );
/** 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

foreach( $action_links as $action_link ) {
$dom_doc->loadHTML( $action_link );
$link_elements = $dom_doc->getElementsByTagName( 'a' );
if ( $link_elements->length == 0 ) {
continue;
}

$link_element = $link_elements->item( 0 );
if ( $link_element->hasAttribute( 'href' ) && $link_element->nodeValue ) {
$link_url = trim( $link_element->getAttribute( 'href' ) );

// Add the full admin path to the url if the plugin did not provide it
$link_url_scheme = wp_parse_url( $link_url, PHP_URL_SCHEME );
if ( empty( $link_url_scheme ) ) {
$link_url = admin_url( $link_url );
}

$formatted_action_links[ $link_element->nodeValue ] = $link_url;
}
}
}

return $formatted_action_links;
}
}