Automattic / Automattic/newspack-rolling-coverage
No test coverage for the push-notification publish path
- Dominant language
- PHP
- Stars
- 1
- Forks
- 1
- Avg merge
- 2d 18h
- Merged PRs (30d)
- 3
Description
## Summary
`tests/` holds `bootstrap.php` and `test-bootstrap.php` and nothing else. The push-notification publish path has no coverage, and it is the part of this feature where a mistake reaches readers' devices and cannot be recalled.
From the [#18](https://github.com/Automattic/newspack-rolling-coverage/pull/18) review.
## Why here first
Every defect found reviewing [#18](https://github.com/Automattic/newspack-rolling-coverage/pull/18) sat in publish-transition plumbing — fires when it should not, silently does not fire, fires twice. That behaviour is awkward to hold in your head and cheap to assert, which is the combination unit tests are good at. It is also the area most likely to shift again: the send is now gated on a mix of `$_POST`, stored intent, and a third-party meta key, and each of those moved during review.
## Cases worth pinning
Send gating, in `Push_Notifications`:
1. Publishing with the checkbox ticked sends once.
2. A second `wp_update_post()` carrying the same `$_POST` does not send again — including when the previous send failed and wrote no `os_notification_id`.
3. A scheduled entry published by cron, with intent stored, sends once.
4. A programmatic publish with no stored intent does not send. `Entry_Ingestion_Service::ingest()` inserts entries directly at `publish` when auto-publish is on, and `Breakout::on_breakout_post_status_change()` calls `wp_update_post()` on an already-published entry; neither should notify.
5. A coverage with no canonical URL does not send.
6. `is_onesignal_configured() === false` no-ops cleanly.
Payload shape, in `override_notification_fields()`:
7. Running the callback over OneSignal's default payload removes `included_segments` and adds the tag filter. This pins the third-party contract, so the next OneSignal upgrade fails a test rather than a publisher's send.
Sanitization, in `Taxonomy::sanitize_canonical_url()`:
8. Same-host URLs pass; off-site hosts, `javascript:` and malformed input are rejected.
## Notes
Cases 1 to 6 need the OneSignal API stubbed rather than called. Filtering `pre_http_request` on `onesignal.com` is enough, and lets a test assert how many requests were made and what they contained — which is how the duplicate-send behaviour was confirmed during review.
A global `WC()`-style mock is not needed here; the only third-party surface is `onesignal_create_notification()` and the `OneSignalWPSetting` option.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with tests/bootstrap.php and test-bootstrap.php, then trace the named Push_Notifications, Entry_Ingestion_Service, Breakout, and Taxonomy methods. Stub OneSignal through pre_http_request and add tests for the six publish cases, payload filtering in override_notification_fields(), and canonical URL sanitization. Done means the specified sends, no-ops, payload shape, and URL acceptance or rejection are asserted without calling OneSignal.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- php, wordpress
- Domain
- backend, testing
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 68/100