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

feat(node): added wp cli command - process pending requests #67

Merged
merged 12 commits into from
Feb 23, 2024
13 changes: 12 additions & 1 deletion DEV_NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,17 @@ Examples:

If you want to customize how this new event looks in the `Event Log`, go to `hub/stores/event-log-items` and create a new class named after the class you informed in `ACCEPTED_ACTIONS`. Implement the `get_summary` method to display the information the way you need.

## WP CLI

Available CLI commands are (add `--help` flag to learn more about each command):

### `wp newspack-network process-webhooks`
* Will process `pending` `np_webhook_request`s and delete after processing.
* `--per-page=1000` to process x amount of requests. Default is `-1`.
* `--status='killed'` to process requests of a different status. Default is `'pending'`
* `--dry-run` enabled. Will run through process without deleting.
* `--yes` enabled. Will bypass confirmations.

## Troubleshooting

Here's how to debug and follow each event while they travel around.
Expand Down Expand Up @@ -141,4 +152,4 @@ Pulls are scheduled in CRON for every 5 minutes. If you want to trigger a pull n

In the Node's log you will see detailed information about the pull attempt, starting with a `Pulling data` message.

In the Hub's log, you will also see detailed information about the pull request, starting with a `Pull request received` message.
In the Hub's log, you will also see detailed information about the pull request, starting with a `Pull request received` message.
133 changes: 133 additions & 0 deletions includes/node/class-webhook.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

namespace Newspack_Network\Node;

use WP_CLI;
use WP_CLI\Utils as WP_CLI_Utils;

use Newspack_Network\Accepted_Actions;
use Newspack_Network\Crypto;
use Newspack\Data_Events\Webhooks as Newspack_Webhooks;
Expand Down Expand Up @@ -38,6 +41,9 @@ class Webhook {
public static function init() {
add_action( 'init', [ __CLASS__, 'register_endpoint' ] );
add_filter( 'newspack_webhooks_request_body', [ __CLASS__, 'filter_webhook_body' ], 10, 2 );
if ( defined( 'WP_CLI' ) && WP_CLI ) {
WP_CLI::add_command( 'newspack-network process-webhooks', [ __CLASS__, 'cli_process_webhooks' ] );
}
}

/**
Expand Down Expand Up @@ -104,4 +110,131 @@ public static function sign( $data, $nonce, $secret_key = null ) {

return Crypto::encrypt_message( $data, $secret_key, $nonce );
}

/**
* Process network requests
*
* @param array $args Indexed array of args.
* @param array $assoc_args Associative array of args.
* @return void
*
* ## OPTIONS
*
* [--per-page=<number>]
* : How many requests to process.
* ---
* default: -1
* ---
*
* [--status=<string>]
* : Filters requests to process by status.
* ---
* default: 'pending'
* ---
*
* [--dry-run]
* : Run the command in dry run mode. No requests (with status killed) will be processed.
*
* [--yes]
* : Run the command without confirmations, use with caution.
*
* ## EXAMPLES
*
* wp newspack-network process-webhooks
* wp newspack-network process-webhooks --per-page=200
* wp newspack-network process-webhooks --per-page=200 --status='killed' --dry-run
* wp newspack-network process-webhooks --per-page=200 --status='killed' --dry-run --yes
*
* @when after_wp_load
*/
public function cli_process_webhooks( array $args, array $assoc_args ): void {
$per_page = (int) ( $assoc_args['per-page'] ?? -1 );
$dry_run = isset( $assoc_args['dry-run'] );
$status = $assoc_args['status'] ?? 'pending';

/**
* Get requests by 'status'
*/
$requests = array_filter(
Newspack_Webhooks::get_endpoint_requests( static::ENDPOINT_ID, $per_page ),
fn ( $r ) => $r['status'] === $status
);
usort(
$requests,
// OrderBy: id, Order: ASC.
fn ( $a, $b ) => $a['id'] <=> $b['id']
);

// No requests, bail.
if ( empty( $requests ) ) {
WP_CLI::error( "No '{$status}' requests exist, exiting!" );
}

$request_ids = array_column( $requests, 'id' );

$counts = [
'total' => count( $requests ),
'failed' => 0,
'success' => 0,
];

$errors = [];

if ( $dry_run ) {
WP_CLI::log( '==== DRY - RUN ====' );
} else {
WP_CLI::confirm( "Confirm processing of {$counts['total']} requests?", $assoc_args );
}

$progress = WP_CLI_Utils\make_progress_bar( 'Processing requests', $counts['total'] );

foreach ( $request_ids as $request_id ) {
if ( ! $dry_run ) {
Newspack_Webhooks::process_request( $request_id );
if ( 'finished' !== get_post_meta( $request_id, 'status', true ) ) {
$errors[ $request_id ] = '<unknown_error>';
// Get last stored error.
$request_errors = get_post_meta( $request_id, 'errors', true );
if ( (array) $request_errors === $request_errors ) {
$errors[ $request_id ] = end( $request_errors );
}
++$counts['failed'];
continue;
}
// Cleanup successfully processed requests.
$deleted = wp_delete_post( $request_id, true );
if ( false === $deleted || null === $deleted ) {
WP_CLI::warning( "There was an error deleting {$request_id}!" );
}
}
++$counts['success'];
$progress->tick();
}

$progress->finish();
WP_CLI::log( '' );

/**
* If all requests have been processed, output success and return.
*/
if ( $counts['success'] === $counts['total'] ) {
WP_CLI::success( "Successfully processed {$counts['success']}/{$counts['total']} '{$status}' requests.\n" );
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit. Feel free to ignore. I think we were okay with the warning here. For large batches of events where some fail and some succeed, I would expect to see information about what failed and what succeeded.

Thinking about this some more, something like the following would be ideal IMO:

success - all events processed
warning - some events processed, some events not processed
error - no events processed

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some solid suggestions there @chickenn00dle! I have implemented your suggestions, capture last error (i.e. the one that caused the error to be captured) and added some additional comments/docs etc 36049d4, all-in-all your suggestion adds a lot of value to the command, thank you 🤜 🤛

Success:

Screenshot 2024-02-22 at 17 37 05

Warning: (hard to test)

Screenshot 2024-02-22 at 17 39 10

Errors:

Screenshot 2024-02-22 at 17 40 29

}

// Last 100 errors.
$errors = wp_json_encode( array_slice( $errors, -100, 100, true ), JSON_PRETTY_PRINT );
/**
* All request processing failed.
*/
if ( $counts['failed'] === $counts['total'] ) {
WP_CLI::error( "0/{$counts['total']} '{$status}' request were processed. \nErrors: {$errors}\n" );
return;
}

WP_CLI::warning( "Not all '{$status}' requests have been processed:" );
WP_CLI::log( "- Success: {$counts['success']}/{$counts['total']}" );
WP_CLI::log( "- Failed: {$counts['success']}/{$counts['failed']}" );
WP_CLI::log( "- Errors: {$errors}\n" );
}
}