diff --git a/internal/push/push.go b/internal/push/push.go index 41ed571..4013fd1 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -115,12 +115,23 @@ type TestSent struct { // notification outbox as a plan_ready event whose tap opens TestURL. Its // key, "plan_ready:test-" and a new id, is never used twice, so it is never // taken for an event already sent. It goes to push alone, never to Google -// Chat. With no subscription it is ErrNoSubscriptions, and with push -// switched off for plan_ready it is notify.ErrPushOff; neither queues -// anything. Delivery happens after SendTest returns, with the outbox's -// retries. +// Chat. With push switched off for plan_ready it is notify.ErrPushOff, +// whether or not any device has turned on push; otherwise with no +// subscription it is ErrNoSubscriptions. Neither queues anything. Delivery +// happens after SendTest returns, with the outbox's retries. func (s *Service) SendTest(ctx context.Context) (TestSent, error) { - subs, err := db.New(s.st.Reader).ListPushSubscriptions(ctx) + q := db.New(s.st.Reader) + pref, err := q.GetNotificationPref(ctx, string(notify.PlanReady)) + if errors.Is(err, sql.ErrNoRows) { + return TestSent{}, fmt.Errorf("push: no notification preference for %s", notify.PlanReady) + } + if err != nil { + return TestSent{}, fmt.Errorf("push: read notification preference for %s: %w", notify.PlanReady, err) + } + if pref.Push != 1 { + return TestSent{}, fmt.Errorf("%w: %s", notify.ErrPushOff, notify.PlanReady) + } + subs, err := q.ListPushSubscriptions(ctx) if err != nil { return TestSent{}, fmt.Errorf("push: list subscriptions: %w", err) } diff --git a/internal/push/push_test.go b/internal/push/push_test.go index 255fb89..3c7c8d2 100644 --- a/internal/push/push_test.go +++ b/internal/push/push_test.go @@ -306,3 +306,20 @@ func TestSendTestWithPushOff(t *testing.T) { t.Fatalf("rows = %+v, want none", rows) } } + +// Push off is reported before the subscriptions are looked at, so with +// both push off and no device subscribed the answer is push off. +func TestSendTestWithPushOffAndNoSubscriptions(t *testing.T) { + t.Parallel() + svc, st := newTestService(t) + if _, err := st.Writer.Exec(`UPDATE notification_prefs SET push = 0 WHERE event = 'plan_ready'`); err != nil { + t.Fatal(err) + } + _, err := svc.SendTest(context.Background()) + if !errors.Is(err, notify.ErrPushOff) || errors.Is(err, ErrNoSubscriptions) { + t.Fatalf("SendTest = %v, want notify.ErrPushOff", err) + } + if rows := notifytest.Rows(t, st, notify.PlanReady); len(rows) != 0 { + t.Fatalf("rows = %+v, want none", rows) + } +} diff --git a/internal/server/push.go b/internal/server/push.go index 5a816fb..b4bc711 100644 --- a/internal/server/push.go +++ b/internal/server/push.go @@ -89,9 +89,10 @@ func deletePushSubscription(logger *slog.Logger, svc *push.Service) http.Handler // postTestPush answers POST /settings/notifications/test-push: it queues // push.TestTitle for every device that turned on push, aimed at /plan, and // answers 202 with the event key and how many subscriptions it goes to. -// Delivery follows through the notification outbox. With no subscription -// it is 409 no_subscriptions, and with push switched off for plan_ready -// 409 push_off; neither sends anything. It never posts to Google Chat. +// Delivery follows through the notification outbox. With push switched +// off for plan_ready it is 409 push_off, whether or not any device has +// turned on push; otherwise with no subscription it is 409 +// no_subscriptions. Neither sends anything. It never posts to Google Chat. func postTestPush(logger *slog.Logger, svc *push.Service) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { sent, err := svc.SendTest(r.Context()) diff --git a/internal/server/push_test.go b/internal/server/push_test.go index 92e4305..9c4f6de 100644 --- a/internal/server/push_test.go +++ b/internal/server/push_test.go @@ -291,8 +291,21 @@ func TestTestPushAPI(t *testing.T) { svc, st, _ := newTestPushWith(t) router := newPushRouter(t, svc) - // With no device subscribed there is nothing to test. + // With push off for plan_ready and no device subscribed, push off is + // what the test says. + if _, err := st.Writer.Exec(`UPDATE notification_prefs SET push = 0 WHERE event = 'plan_ready'`); err != nil { + t.Fatal(err) + } rec, body := serve(t, router, signedIn(http.MethodPost, testPushPath, nil)) + if rec.Code != http.StatusConflict || body["error"].(map[string]any)["code"] != "push_off" { + t.Errorf("POST with push off and no subscriptions = %d %s, want 409 push_off", rec.Code, rec.Body) + } + if _, err := st.Writer.Exec(`UPDATE notification_prefs SET push = 1 WHERE event = 'plan_ready'`); err != nil { + t.Fatal(err) + } + + // With no device subscribed there is nothing to test. + rec, body = serve(t, router, signedIn(http.MethodPost, testPushPath, nil)) if rec.Code != http.StatusConflict || body["error"].(map[string]any)["code"] != "no_subscriptions" { t.Errorf("POST with no subscriptions = %d %s, want 409 no_subscriptions", rec.Code, rec.Body) } diff --git a/web/src/onboarding/NotificationsStep.test.tsx b/web/src/onboarding/NotificationsStep.test.tsx index ee406c4..55ec2a4 100644 --- a/web/src/onboarding/NotificationsStep.test.tsx +++ b/web/src/onboarding/NotificationsStep.test.tsx @@ -18,6 +18,12 @@ import styles from './Onboarding.module.css' const IPHONE_SAFARI = 'Mozilla/5.0 (iPhone; CPU iPhone OS 18_5 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/18.5 Mobile/15E148 Safari/604.1' +const IPHONE_CHROME = + 'Mozilla/5.0 (iPhone; CPU iPhone OS 18_5 like Mac OS X) AppleWebKit/605.1.15 (KHTML, like Gecko) CriOS/138.0.7204.119 Mobile/15E148 Safari/604.1' + +const IOS_UNSUPPORTED = + "Notifications aren't on: on iPhone and iPad they only work once MealQuest is on the Home Screen. Tap Share, then Add to Home Screen, open MealQuest from there, and turn them on in Settings under Notifications." + const COMPLETED_AT = '2026-10-06T21:30:00.000Z' // requestPermission stands in for the browser's Notification API, which @@ -132,7 +138,11 @@ async function finishFromResult(server: ReturnType) { } function useIphoneSafariTab() { - vi.spyOn(navigator, 'userAgent', 'get').mockReturnValue(IPHONE_SAFARI) + useIphoneTab(IPHONE_SAFARI) +} + +function useIphoneTab(userAgent: string) { + vi.spyOn(navigator, 'userAgent', 'get').mockReturnValue(userAgent) Object.defineProperty(navigator, 'maxTouchPoints', { configurable: true, value: 5 }) Object.defineProperty(navigator, 'standalone', { configurable: true, value: false }) vi.stubGlobal('matchMedia', (query: string) => ({ matches: false, media: query }) as MediaQueryList) @@ -265,14 +275,22 @@ describe('onboarding step 3', () => { const server = await renderStep() fireEvent.click(turnOn()) - expect(await screen.findByRole('status')).toHaveTextContent( - "Notifications aren't on: on iPhone and iPad they only work once MealQuest is on the Home Screen. Tap Share, then Add to Home Screen, open MealQuest from there, and turn them on in Settings under Notifications.", - ) + expect(await screen.findByRole('status')).toHaveTextContent(IOS_UNSUPPORTED) expect(screen.getByRole('note')).toHaveTextContent('Tap Share, then Add to Home Screen to get notifications') expect(requestPermission).not.toHaveBeenCalled() await finishFromResult(server) }) + it('"Turn on notifications" in another iOS browser also says to add MealQuest to the Home Screen first', async () => { + useIphoneTab(IPHONE_CHROME) + const server = await renderStep() + fireEvent.click(turnOn()) + + expect(await screen.findByRole('status')).toHaveTextContent(IOS_UNSUPPORTED) + expect(requestPermission).not.toHaveBeenCalled() + await finishFromResult(server) + }) + it('"Turn on notifications" can ask again after the prompt is dismissed', async () => { const browser = stubPushEnvironment({ permission: 'default', answer: 'default' }) const server = await renderStep() diff --git a/web/src/onboarding/NotificationsStep.tsx b/web/src/onboarding/NotificationsStep.tsx index be48c82..0c41a96 100644 --- a/web/src/onboarding/NotificationsStep.tsx +++ b/web/src/onboarding/NotificationsStep.tsx @@ -2,7 +2,7 @@ import { useState } from 'react' import { useQueryClient } from '@tanstack/react-query' import { CalendarDays, ShoppingBasket, Sparkles, UtensilsCrossed, type LucideIcon } from 'lucide-react' import { useNavigate } from 'react-router' -import { InstallHint, isIosSafari, isStandalone } from '../pwa/InstallHint.tsx' +import { InstallHint, isIos, isStandalone } from '../pwa/InstallHint.tsx' import type { SubscribePushResult } from '../push/subscribe.ts' import { usePush } from '../push/usePush.ts' import { PUSH_STATUS_KEY } from '../settings/notificationsApi.ts' @@ -13,7 +13,8 @@ import { OnboardingStep } from './OnboardingStep.tsx' import styles from './Onboarding.module.css' // IOS_UNSUPPORTED says how to get notifications on iPhone and iPad, where -// Web Push reaches only the app installed to the Home Screen. +// Web Push reaches only the app installed to the Home Screen, whichever +// browser the page is open in. const IOS_UNSUPPORTED = "Notifications aren't on: on iPhone and iPad they only work once MealQuest is on the Home Screen. Tap Share, then Add to Home Screen, open MealQuest from there, and turn them on in Settings under Notifications." @@ -26,7 +27,7 @@ function resultText(result: SubscribePushResult): string { case 'denied': return "Notifications aren't on: this browser blocked them. To turn them on later, allow notifications for this site in the browser's site settings, then turn them on in Settings under Notifications." case 'unsupported': - return isIosSafari(navigator) && !isStandalone() + return isIos(navigator) && !isStandalone() ? IOS_UNSUPPORTED : "Notifications aren't on: this browser can't receive them, so nothing was turned on." case 'dismissed': diff --git a/web/src/pwa/InstallHint.tsx b/web/src/pwa/InstallHint.tsx index 92b30d2..dfc48c7 100644 --- a/web/src/pwa/InstallHint.tsx +++ b/web/src/pwa/InstallHint.tsx @@ -16,13 +16,18 @@ export const DISMISSED_KEY = 'mealquest.installHint.dismissed' // ...) or drops Version/ or Safari/, so only this exact tail is Safari. const SAFARI_TAIL = /\(KHTML, like Gecko\) Version\/[\d.]+ (?:Mobile\/\w+ )?Safari\/[\d.]+$/ -// isIosSafari reports whether the page is open in Safari on iPhone, iPod or -// iPad. iPadOS 13+ asks for the desktop site and reports itself as a Mac, so -// a "Macintosh" with a touch screen is an iPad. -export function isIosSafari(nav: Pick): boolean { +// isIos reports whether the page is open on iPhone, iPod or iPad, in any +// browser. iPadOS 13+ asks for the desktop site and reports itself as a Mac, +// so a "Macintosh" with a touch screen is an iPad. +export function isIos(nav: Pick): boolean { const ua = nav.userAgent - const ios = /iPhone|iPod|iPad/.test(ua) || (/Macintosh/.test(ua) && nav.maxTouchPoints > 1) - return ios && SAFARI_TAIL.test(ua) + return /iPhone|iPod|iPad/.test(ua) || (/Macintosh/.test(ua) && nav.maxTouchPoints > 1) +} + +// isIosSafari reports whether the page is open in Safari on iPhone, iPod or +// iPad. +function isIosSafari(nav: Pick): boolean { + return isIos(nav) && SAFARI_TAIL.test(nav.userAgent) } // isStandalone reports whether the app is running from the Home Screen.