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.

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

Add WP-CLI compatibility #423

Merged
merged 19 commits into from Apr 23, 2014

Conversation

Projects
None yet
3 participants
@westonruter
Copy link
Contributor

commented Apr 22, 2014

Without this PR, the plugin is not able to be activated with WP-CLI as there is a fatal error regarding a class not being defined.

  • Log actions performed by WP-CLI anonymously (without --user) as entries with user=0
  • Note in Stream meta any actions performed by WP-CLI with a --user supplied.
  • Show admin notices via the WP-CLI logger instead of via the all_admin_notices hook
  • Add WP-CLI in the author drop-down
  • Attribute actions by WP-CLI with --user as “via WP-CLI” in the table.
  • Fix uninstall routine and DB version

westonruter added some commits Apr 21, 2014

Eliminate WP_CLI short-circuit
Stream needs to be more thoughtful with regards to how it behaves when WP-CLI is activated. With this short-circuit in place, plugin activation was prevented via WP-CLI:

> PHP Fatal error:  Class 'WP_Stream_Admin' not found in /srv/www/wordpress-develop/src/wp-content/plugins/stream/includes/install.php on line 83
Merge branch 'develop' into wp-cli
Conflicts:
	stream.php
@@ -254,6 +256,9 @@ function column_default( $item, $column_name ) {
if ( $user ) {
$author_name = isset( $user->display_name ) ? $user->display_name : $user->user_login;
$author_avatar = get_avatar( $author_ID, 40 );
} else if ( $is_wp_cli ) {

This comment has been minimized.

Copy link
@fjarrett
@@ -121,7 +121,7 @@ public function query( $args ) {
$where .= $wpdb->prepare( " AND $wpdb->stream.summary LIKE %s", "%{$args['search']}%" );
}
if ( $args['author'] ) {
if ( $args['author'] || $args['author'] === '0' ) {

This comment has been minimized.

Copy link
@fjarrett

fjarrett Apr 23, 2014

Contributor

@westonruter Yoda style needed here.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2014

stream settings

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2014

stream list-table

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2014

wp-cli-dashboard

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2014

(The most recent commit passes Travis CI except for one build where there was an HTTP error.)

@shadyvb

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2014

@westonruter Shouldn't we make WP-CLI a generic name ? I think there would be other tools used to manage WP from CLI other than WP-CLI.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2014

Maybe eventually. We'd need to add support for detecting the tool that is
making the change. Perhaps out of scope here?

@shadyvb

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2014

It'd be hard to detect the tool, a think a something along the lines of 'Server/CLI Operation' would be informative.

<?php
$author_name = 'WP-CLI';
$avatar_url = WP_STREAM_URL . 'ui/stream-icons/wp-cli.png';
$author_avatar = sprintf( '<img alt="%s" src="%s" class="avatar avatar-80 photo" height="36" width="36">', esc_attr( $author_name ), esc_url( $avatar_url ) );

This comment has been minimized.

Copy link
@shadyvb

shadyvb Apr 23, 2014

Contributor

What about printf ?

This comment has been minimized.

Copy link
@westonruter

westonruter Apr 23, 2014

Author Contributor

Make it so.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2014

Hummm. Wouldn't this also come into play when a Cron action updates
something? Are we making sure to disable Stream when DOING_CRON?

@shadyvb

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2014

I'm not sure if we do anything about that, at least not that i know of.
But i guess the normal request-activated cron will still be attributed to the user who initiated the request, or is it done via async requests ?

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2014

Async request yes. No user would be logged in.

@westonruter

This comment has been minimized.

Copy link
Contributor Author

commented Apr 23, 2014

Should create an issue for that, to turn off Stream logging during Cron.

But we can detect when WP-CLI is being used via the constant WP_CLI. This is getting stored in stream meta too.

@shadyvb

This comment has been minimized.

Copy link
Contributor

commented Apr 23, 2014

Opened #431

@@ -272,7 +278,8 @@ function column_default( $item, $column_name ) {
$author_avatar,
$author_name,
$user_deleted ? sprintf( '<br /><small class="deleted">%s</small>', esc_html__( 'Deleted User', 'stream' ) ) : '',
$author_role ? sprintf( '<br /><small>%s</small>', $author_role ) : ''
$author_role ? sprintf( '<br /><small>%s</small>', $author_role ) : '',
$user && $is_wp_cli ? sprintf( '<br /><small>%s</small>', __( 'via WP-CLI', 'stream' ) ) : ''

This comment has been minimized.

Copy link
@shadyvb

shadyvb Apr 23, 2014

Contributor

@westonruter I'm not sure i get the need for $user && part here.

This comment has been minimized.

Copy link
@westonruter

westonruter Apr 23, 2014

Author Contributor

We only want to include this if the user is not 0, as in that case it shows WP-CLI as the user.

This comment has been minimized.

Copy link
@shadyvb

shadyvb Apr 23, 2014

Contributor

I'm having difficult time trying to wrap my head around this one 😄 #morning-blockers

This comment has been minimized.

Copy link
@westonruter

westonruter Apr 23, 2014

Author Contributor

If WP-CLI was invoked with --user and we have a user ID available to us. In this case, we want to show "via WP-CLI". Otherwise, if the user ID was 0 then we show "WP-CLI":

stream list-table

This comment has been minimized.

Copy link
@shadyvb

shadyvb Apr 23, 2014

Contributor

Oh, now i understand! Thanks for the explanation!

@fjarrett fjarrett merged commit d7c6213 into develop Apr 23, 2014

1 check failed

continuous-integration/travis-ci The Travis CI build could not complete due to an error
Details

@fjarrett fjarrett deleted the wp-cli branch Apr 23, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.