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

Conversation

jaredrethman
Copy link
Collaborator

@jaredrethman jaredrethman commented Feb 22, 2024

All Submissions:

Changes proposed in this Pull Request:

This PR introduces a WP CLI command for processing np_webhook_requests with post-meta status in pending state.

How to test the changes in this Pull Request:

  1. Ensure a Hub and Node relationship exists and is configured correctly.
  2. Node: Add incorrect "Hub URL" - WP Admin > Newspack Network > Node Settings
  3. Node: Perform individual (as many as you can, the more the better) user related updates i.e. update; name, lastname etc.
  4. Node: Add correct "Hub URL" - WP Admin > Newspack Network > Node Settings
  5. Open terminal and run wp newspack-network process-webhooks to process and delete pending requests

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@jaredrethman jaredrethman marked this pull request as ready for review February 22, 2024 20:27
@jaredrethman jaredrethman requested a review from a team as a code owner February 22, 2024 20:27
@jaredrethman jaredrethman changed the title feat(webhook): added wp cli command - process pending requests feat(node): added wp cli command - process pending requests Feb 22, 2024
Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Works great @jaredrethman! Nice work 🎉

I left two small comments regarding output, then I think this is good to go.

includes/node/class-webhook.php Outdated Show resolved Hide resolved
includes/node/class-webhook.php Outdated Show resolved Hide resolved
pr feedback adds better output when no requests are processed
Copy link
Contributor

@chickenn00dle chickenn00dle left a comment

Choose a reason for hiding this comment

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

Thanks for the quick turnaround! I have one small nit, but since the current messaging works and I don't wanna hold this up, gonna go ahead and approve.


if ( ! empty( $unprocessed_request_ids ) ) {
WP_CLI::error( "The following requests could not be processed:\n" . wp_json_encode( $unprocessed_request_ids ) );
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

includes/node/class-webhook.php Outdated Show resolved Hide resolved
@adekbadek adekbadek merged commit 7dbd8dc into trunk Feb 23, 2024
3 checks passed
@adekbadek adekbadek deleted the feat/wpcli-process-pending branch February 23, 2024 15:09
matticbot pushed a commit that referenced this pull request Feb 23, 2024
# [1.3.0-alpha.1](v1.2.0...v1.3.0-alpha.1) (2024-02-23)

### Bug Fixes

* memberships sync ([#63](#63)) ([0a54f1d](0a54f1d))

### Features

* add hub bookmarks to nodes ([#56](#56)) ([54700a0](54700a0))
* add option to manually sync users  ([#53](#53)) ([3ec1b19](3ec1b19))
* display membership plans from the network ([#59](#59)) ([6fa01fd](6fa01fd))
* enhance network subscriptions and orders search ([#55](#55)) ([bcb0615](bcb0615))
* Node connection using a link ([#58](#58)) ([721f8b2](721f8b2))
* **wp-cli:** add command to backfill events ([#51](#51)) ([13d2498](13d2498))
* **wp-cli:** add process pending webhook requests command ([#67](#67)) ([7dbd8dc](7dbd8dc))
* **wp-cli:** sync all events from the Hub ([#65](#65)) ([dc595ca](dc595ca))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.3.0-alpha.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

matticbot pushed a commit that referenced this pull request Feb 28, 2024
# [1.3.0](v1.2.0...v1.3.0) (2024-02-28)

### Bug Fixes

* backfill duplicate handling; woo links in wp-admin-bar ([#71](#71), [#72](#72)) ([bbce13b](bbce13b))
* cli commands ([#73](#73)) ([ff563ac](ff563ac))
* memberships sync ([#63](#63)) ([0a54f1d](0a54f1d))
* missing woocommerce-memberships handling ([76dbdf7](76dbdf7))

### Features

* add hub bookmarks to nodes ([#56](#56)) ([54700a0](54700a0))
* add option to manually sync users  ([#53](#53)) ([3ec1b19](3ec1b19))
* display membership plans from the network ([#59](#59)) ([6fa01fd](6fa01fd))
* enhance network subscriptions and orders search ([#55](#55)) ([bcb0615](bcb0615))
* Node connection using a link ([#58](#58)) ([721f8b2](721f8b2))
* **wp-cli:** add command to backfill events ([#51](#51)) ([13d2498](13d2498))
* **wp-cli:** add process pending webhook requests command ([#67](#67)) ([7dbd8dc](7dbd8dc))
* **wp-cli:** sync all events from the Hub ([#65](#65)) ([dc595ca](dc595ca))
@matticbot
Copy link
Contributor

🎉 This PR is included in version 1.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants