-
Notifications
You must be signed in to change notification settings - Fork 51
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
perf(data-events): queue dispatches to execute on shutdown #3616
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the following ways to trigger syncs and all are working except for the CLI command:
- Reader action (purchasing a new subscription, renewing an existing one) ✔️
- Admin action (manually renewing a subscription or changing its status) ✔️
- Admin bulk sync in Users table ✔️
- CLI command
wp newspack esp sync --user-ids=127
❌ — in this case the CLI command fails to read the associative args whether in dry-run or live mode. The command works as expected intrunk
.
Once we fix things for the CLI command, I think this will be a good performance improvement!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Working after the merge from trunk
!
Thank you for the review, @dkoo! |
Hey @miguelpeixe, good job getting this PR merged! 🎉 Now, the Please check if this PR needs to be included in the "Upcoming Changes" and "Release Notes" doc. If it doesn't, simply remove the label. If it does, please add an entry to our shared document, with screenshots and testing instructions if applicable, then remove the label. Thank you! ❤️ |
## [5.10.7-alpha.1](v5.10.6...v5.10.7-alpha.1) (2025-01-10) ### Bug Fixes * **cli:** verify-reader CLI command ([#3660](#3660)) ([c639af7](c639af7)) * **recaptcha:** replace alerts with generic errors ([#3627](#3627)) ([44ef2d2](44ef2d2)) ### Performance Improvements * **data-events:** queue dispatches to execute on shutdown ([#3616](#3616)) ([510a1a0](510a1a0))
🎉 This PR is included in version 5.10.7-alpha.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [5.12.0-alpha.1](v5.11.1...v5.12.0-alpha.1) (2025-01-14) ### Bug Fixes * **cli:** verify-reader CLI command ([#3660](#3660)) ([c639af7](c639af7)) * **recaptcha:** replace alerts with generic errors ([#3627](#3627)) ([44ef2d2](44ef2d2)) * remove newspack_corrections_ids meta ([#3675](#3675)) ([dad258b](dad258b)) ### Features * **corrections:** add corrections and clarifications behind feature flag ([#3638](#3638)) ([ea745cf](ea745cf)) ### Performance Improvements * **data-events:** queue dispatches to execute on shutdown ([#3616](#3616)) ([510a1a0](510a1a0))
🎉 This PR is included in version 5.12.0-alpha.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
# [5.12.0](v5.11.3...v5.12.0) (2025-01-20) ### Bug Fixes * **cli:** verify-reader CLI command ([#3660](#3660)) ([c639af7](c639af7)) * **recaptcha:** replace alerts with generic errors ([#3627](#3627)) ([44ef2d2](44ef2d2)) * remove newspack_corrections_ids meta ([#3675](#3675)) ([dad258b](dad258b)) * **wcs:** migrate-expired-subscriptions handle manual subscriptions ([#3663](#3663)) ([e0f32e8](e0f32e8)) ### Features * **corrections:** add corrections and clarifications behind feature flag ([#3638](#3638)) ([ea745cf](ea745cf)) ### Performance Improvements * **data-events:** queue dispatches to execute on shutdown ([#3616](#3616)) ([510a1a0](510a1a0))
🎉 This PR is included in version 5.12.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
p1733922470397489-slack-C055SRT7WN4
Depending on the setup, a site can execute 5 dispatches when a reader registers:
membership_status_active
membership_saved
reader_logged_in
reader_registered
network_user_updated
The list grows if the registration happens during a subscription purchase.
Other actions on the site can also dispatch multiple events.
Each dispatch runs a
wp_remote_post()
. Even though it's a non-blocking execution with a timeout of 0.01, calling this function for every dispatch is unnecessary and costs performance.This PR proposes queueing the dispatches to be executed in a single
wp_remote_post()
called on theshutdown
hook. This hook runs right before PHP shuts down execution, including on fatal errors.How to test the changes in this Pull Request:
NEWSPACK_LOG_LEVEL
set to at least2
onwp-config.php
reader_registered
andreader_logged_in
in the same runOther information: