feat(alert-manager): route alerts to Slack via newspack_log#4723
Conversation
There was a problem hiding this comment.
Pull request overview
Adds alert forwarding from newspack_alert to newspack_log so Newspack Manager can route alert-level logs to Slack.
Changes:
- Registers
Alert_Manager::forward_alert_to_log()onnewspack_alert. - Emits
newspack_logentries withlog_level => 3. - Adds unit tests for forwarding and malformed alert payloads.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
includes/class-alert-manager.php |
Adds the alert-to-log forwarding hook and implementation. |
tests/unit-tests/alert-manager.php |
Adds cleanup and tests for alert log forwarding behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
LGTM — local checks (lint, PHPUnit for Alert_Manager: 18 tests / 72 assertions) and CI are all green. The newspack_log contract — ( $code, $message, $params ) with type ∈ error|debug and log_level ∈ 1..4 — matches Logger::log and the precedent at newspack-manager's class-data-report.php:352. Severity-to-level mapping aligns; dropping context from the forwarded params is correct (passing as data would JSON-encode reader identifiers into the log payload).
Approving — none of the inline comments are blockers, just refinements worth considering before merge.
|
Thanks, @chickenn00dle! I've addressed the suggestions. |
|
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! ❤️ |
All Submissions:
Changes proposed in this Pull Request:
Adds a
forward_alert_to_loglistener onnewspack_alertinAlert_Manager. The listener emitsdo_action( 'newspack_log', $code, $message, [ 'log_level' => 3, ... ] ), which Newspack Manager'sLoggeralready routes through Jetpack to the Newspack Slack alerts channel.Existing precedent for
log_level => 3→ Slack alert is innewspack-manager'sclass-data-report.php:352.Closes NPPD-1443.
How to test the changes in this Pull Request:
Check out this branch on a site with
newspack-manageractive and connected to a hub (sonewspack_logactually round-trips through Jetpack).From
wp shell(or a quick snippet), fire a synthetic alert:Confirm a level 3 log is fired with the alert message
Other information: