Repository navigation
fix(aggregator): alerts push UX — permission flow, quiet-hours off, test button, primary nav - #3707
Conversation
…est button, primary nav (#3676) Drew's phone report, six defects: - unsupported-browser line was a dead end; now plain steps (iOS: Safari → Share → Add to Home Screen → open from the icon; in-app browser: open in the real browser), variant picked by UA. - Enable threw 'notification permission: default' and the page alert()ed it. The client now throws coded errors only and the page renders plain words per state (blocked / prompt dismissed / ...); the request still runs inside the tap handler, before subscribing. - quiet hours gain an OFF switch (quiet jsonb {enabled:false}, no migration); inQuietHours returns false when disabled and the time inputs disable with it. - every control on the panel carries a one-line explanation in plain language; the bare 'quiet' label is gone. - 'Send a test alert' button → POST /api/push/test sends a real push (class 'test') to all subscriptions and records notification_sends rows; the result is reported in plain words. - /alerts promoted to the primary nav (Drew's ruling supersedes the post-#1497 dropdown placement) + a link from /attention.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 56 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aaef86894f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| <table><caption>Recent alert-ledger transitions</caption><tr><th scope="col">at</th><th scope="col">kind</th><th scope="col">incident</th><th scope="col">source · key</th></tr>${events}</table>`; | ||
| const script = `<script> | ||
| const VAPID = ${JSON.stringify(vapidPublicKey)}; | ||
| const SUBS = ${JSON.stringify(view.subscriptions.map((s) => ({ endpoint: s.endpoint, classes: s.classes, quiet: s.quiet })))}; |
There was a problem hiding this comment.
A subscription submitted through /api/push/subscriptions can contain a quiet.tz value such as </script><script>alert(document.domain)</script>: validateNewSubscription passes it through mergeQuiet, which accepts any nonempty string. Embedding that stored value here with only JSON.stringify lets the HTML parser terminate the script and execute attacker-controlled JavaScript whenever someone visits /alerts. Previously these values were HTML-escaped in subscription rows. Escape < in the serialized JSON before inserting it into the script.
Useful? React with 👍 / 👎.
| if (!window.pmPush || !window.pmPush.supported()) { | ||
| status.textContent = "Push isn't available in this browser yet — the steps below get it working."; | ||
| guide.hidden = false; | ||
| const pf = window.pmPush && window.pmPush.platform ? window.pmPush.platform() : { ios: false }; | ||
| document.getElementById("push-guide-ios").hidden = !pf.ios; |
There was a problem hiding this comment.
On a normal /alerts load, layout() places the deferred push-client.js before this non-deferred inline script, so the immediate refreshStatus() call runs before window.pmPush exists. This branch therefore always selects the generic unsupported-browser guide, even on iPhone, and returns without ever applying the newly added saved preferences. Nothing refreshes the panel when the client subsequently loads, so saved incident/quiet-hours choices remain displayed as checked defaults and can be overwritten by Save preferences. Run the initial refresh after the deferred client has executed, such as on DOMContentLoaded.
Useful? React with 👍 / 👎.
Fix build on #3676 from Drew's phone report (2026-11-01) — six defects:
notification permission: <raw value>and the page alert()ed it. The client now throws coded errors only (denied / dismissed / unsupported / subscribe-failed / save-failed); the page renders plain words per state. The permission request still runs inside the Enable tap, before subscribing.enabled:falsein the existing quiet jsonb (no migration);inQuietHoursreturns false when disabled; the time inputs disable while off.POST /api/push/testsends a real Web Push (clearly a test) to all current subscriptions and recordsalerts.notification_sendsrows with classtest; the outcome is reported in plain words ('Test sent to N subscriptions' / 'No subscriptions yet — enable push first').Also: the alerts page opens with this browser's saved preferences applied to the controls, and push-client.js is cache-busted (
?v=push-ux-2026-11-01) so phones pick up the fixed permission flow without waiting out the old 1h asset cache.Tests: full aggregator suite 882 pass / 0 fail (new: quiet-off merge + inQuietHours-disabled, test-payload shape, sendTestPush sends-row recording incl. the zero-subscription case; curation table flipped).
Need help on this PR? Tag
@codesmith-botwith what you need.