From 875a0faaae1130b76e919932381413f5b959dc23 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Fri, 9 Oct 2026 16:54:23 -0400 Subject: [PATCH 1/2] Turn on push from onboarding step 3 and add a test-push endpoint (Feature 19) Onboarding step 3's "Turn on notifications" now calls usePush().subscribePush straight from that tap (the only place onboarding asks for permission) and shows how it ended: on; blocked, with how to allow it in the browser's site settings; not supported, with how to add MealQuest to the Home Screen on iPhone; or a dismissed prompt that can be asked again. "Maybe later" still finishes without asking. The interim notificationsUnavailable is gone. POST /api/v1/settings/notifications/test-push queues "MealQuest test notification" through the notification outbox as a plan_ready event with a fresh plan_ready:test- key, opening /plan, on the push channel only (new notify.Service.EmitPush), never Google Chat. It answers 202 {event_key, subscriptions}, or 409 no_subscriptions / push_off rather than reporting success when nothing would be sent. Maestro task f89279b8-b92c-4d9f-a6c2-fd87fccda509. Co-Authored-By: Claude Opus 5.5 --- cmd/server/main.go | 2 +- docs/ARCHITECTURE.md | 11 + internal/notify/notify.go | 41 +++- internal/notify/notify_test.go | 48 ++++ internal/push/push.go | 62 ++++- internal/push/push_test.go | 90 +++++++- internal/server/push.go | 27 +++ internal/server/push_test.go | 48 +++- internal/server/server.go | 6 +- web/src/onboarding/NotificationsStep.test.tsx | 214 +++++++++++------- web/src/onboarding/NotificationsStep.tsx | 91 ++++---- web/src/onboarding/Onboarding.module.css | 4 +- 12 files changed, 505 insertions(+), 139 deletions(-) diff --git a/cmd/server/main.go b/cmd/server/main.go index 536cfdd..bac4f85 100644 --- a/cmd/server/main.go +++ b/cmd/server/main.go @@ -160,7 +160,7 @@ func run(cfg config.Config, logger *slog.Logger) error { return err } - pushSvc, err := push.NewService(db, cfg.VAPID.PublicKey) + pushSvc, err := push.NewService(db, cfg.VAPID.PublicKey, notifier) if err != nil { return err } diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 7ab3287..9606f6a 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -706,6 +706,17 @@ Runs are recorded in `schedule_runs` and executed through `jobs`. `{"endpoint"}` and answers 204 whether or not it was stored. - An invalid body is 400 `invalid_request` (not a JSON object of the known fields) or `invalid_subscription` (a field fails validation). + - `POST /api/v1/settings/notifications/test-push` (CSRF-checked) checks + delivery to the household's devices. It queues "MealQuest test + notification" through `notification_outbox`, so it takes the real + delivery path with its retries. It is a `plan_ready` event whose tap + opens `/plan`. Its key is `plan_ready:test-`, new on every call, + so `Emit` never deduplicates it. It goes to the `push` channel only + (`notify.Service.EmitPush`), never to the family Google Chat space. + It answers 202 `{event_key, subscriptions}`. Two cases send nothing + and answer 409 rather than report success: no device has subscribed + (`no_subscriptions`), or push is switched off for `plan_ready` in + the preferences (`push_off`). - `internal/chat` keeps the Google Chat incoming webhook in `chat_settings` and is the outbox's sender for the `chat` channel. The webhook URL holds its own key and token, so it is sealed with `secretbox` in diff --git a/internal/notify/notify.go b/internal/notify/notify.go index d3b61ab..4254e5b 100644 --- a/internal/notify/notify.go +++ b/internal/notify/notify.go @@ -8,6 +8,8 @@ // nothing the second time and never touches a row already stored, whatever // its status. EmitTx does the same inside the caller's transaction, so an // event is stored with the change it is about; the caller then calls Wake. +// EmitPush stores an event for the push channel alone, for a test of push +// delivery that must not reach Google Chat. // // The worker started by Start delivers due rows through the Sender // registered for their channel. A delivered row is marked sent. A failed @@ -103,6 +105,10 @@ type Payload struct { // stored. var ErrInvalidEvent = errors.New("notify: invalid event") +// ErrPushOff is returned by EmitPush when the event type's preference +// switches push off; nothing is stored. +var ErrPushOff = errors.New("notify: push is switched off for this event") + // Service emits notifications and runs the worker that delivers them. type Service struct { st *store.Store @@ -153,10 +159,39 @@ func (s *Service) Emit(ctx context.Context, ev Event) error { return nil } +// EmitPush stores one pending push notification of ev and wakes the +// worker, as Emit does but never for Google Chat, whatever the preference +// says of it. When the preference of ev's type switches push off it is +// ErrPushOff and stores nothing. It is for a test of push delivery, which +// must not post to the household's Chat space. +func (s *Service) EmitPush(ctx context.Context, ev Event) error { + err := s.st.WithTx(ctx, func(tx *sql.Tx) error { + return s.emitTx(ctx, tx, ev, func(pref db.NotificationPref) ([]Channel, error) { + if pref.Push != 1 { + return nil, fmt.Errorf("%w: %s", ErrPushOff, ev.Type) + } + return []Channel{ChannelPush}, nil + }) + }) + if err != nil { + return err + } + s.Wake() + return nil +} + // EmitTx stores ev's notifications as Emit does, in tx, so they are // committed with the change the event is about or not at all. The caller // calls Wake once tx has committed. func (s *Service) EmitTx(ctx context.Context, tx *sql.Tx, ev Event) error { + return s.emitTx(ctx, tx, ev, func(pref db.NotificationPref) ([]Channel, error) { + return enabledChannels(pref), nil + }) +} + +// emitTx stores ev in tx for each channel that channels chooses from the +// preference of ev's type. +func (s *Service) emitTx(ctx context.Context, tx *sql.Tx, ev Event, channels func(db.NotificationPref) ([]Channel, error)) error { if err := validate(ev); err != nil { return err } @@ -177,9 +212,13 @@ func (s *Service) EmitTx(ctx context.Context, tx *sql.Tx, ev Event) error { if err != nil { return fmt.Errorf("notify: read notification preference for %s: %w", ev.Type, err) } + chs, err := channels(pref) + if err != nil { + return err + } now := timestamp(s.now()) var queued []Channel - for _, ch := range enabledChannels(pref) { + for _, ch := range chs { n, err := q.InsertNotification(ctx, db.InsertNotificationParams{ ID: store.NewID(), EventKey: ev.Key, diff --git a/internal/notify/notify_test.go b/internal/notify/notify_test.go index f5b99a7..de2b91e 100644 --- a/internal/notify/notify_test.go +++ b/internal/notify/notify_test.go @@ -259,6 +259,54 @@ func TestEmitIgnoresDuplicateKey(t *testing.T) { } } +func TestEmitPushStoresOnlyAPushRow(t *testing.T) { + ctx := context.Background() + svc, st, _ := newTestService(t) + setPref(t, st, PlanReady, true, true) + const key = "plan_ready:test-1" + if err := svc.EmitPush(ctx, planReady(key)); err != nil { + t.Fatal(err) + } + r := onlyRow(t, st, key, ChannelPush) + if r.Status != "pending" || r.NextAttemptAt.String != timestamp(start) { + t.Errorf("row = %+v, want pending, due now", r) + } + select { + case <-svc.wake: + default: + t.Error("EmitPush did not wake the worker") + } + + // The same key again is ignored, as by Emit. + if err := svc.EmitPush(ctx, planReady(key)); err != nil { + t.Fatalf("duplicate EmitPush: %v", err) + } + if got := onlyRow(t, st, key, ChannelPush); got != r { + t.Fatalf("duplicate EmitPush changed the row:\n got %+v\nwant %+v", got, r) + } +} + +func TestEmitPushRefusesWhenPushIsOff(t *testing.T) { + svc, st, _ := newTestService(t) + setPref(t, st, PlanReady, false, true) + const key = "plan_ready:test-1" + if err := svc.EmitPush(context.Background(), planReady(key)); !errors.Is(err, ErrPushOff) { + t.Fatalf("EmitPush = %v, want ErrPushOff", err) + } + if rows := outbox(t, st, key); len(rows) != 0 { + t.Fatalf("rows = %v, want none", rows) + } +} + +func TestEmitPushRejectsInvalidEvents(t *testing.T) { + svc, _, _ := newTestService(t) + ev := planReady("plan_ready:test-1") + ev.URL = "https://evil.example/plan" + if err := svc.EmitPush(context.Background(), ev); !errors.Is(err, ErrInvalidEvent) { + t.Fatalf("EmitPush = %v, want ErrInvalidEvent", err) + } +} + func TestEmitRejectsInvalidEvents(t *testing.T) { svc, st, _ := newTestService(t) valid := planReady("plan_ready:2026-10-12") diff --git a/internal/push/push.go b/internal/push/push.go index 1daaed1..41ed571 100644 --- a/internal/push/push.go +++ b/internal/push/push.go @@ -14,6 +14,7 @@ import ( "strings" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/db" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/vapid" ) @@ -69,22 +70,75 @@ func (e *ValidationError) Error() string { return "invalid push subscription: " + strings.Join(parts, "; ") } -// Service stores push subscriptions. +// Service stores push subscriptions and sends a test notification to them. type Service struct { st *store.Store publicKey string + notifier *notify.Service } // NewService returns a Service over st handing out publicKey, the VAPID -// public key in vapid's encoding. -func NewService(st *store.Store, publicKey string) (*Service, error) { +// public key in vapid's encoding, that sends its test notification through +// notifier. +func NewService(st *store.Store, publicKey string, notifier *notify.Service) (*Service, error) { if st == nil { return nil, errors.New("push: nil store") } if _, err := vapid.ParsePublicKey(publicKey); err != nil { return nil, fmt.Errorf("push: VAPID public key: %w", err) } - return &Service{st: st, publicKey: publicKey}, nil + if notifier == nil { + return nil, errors.New("push: nil notifier") + } + return &Service{st: st, publicKey: publicKey, notifier: notifier}, nil +} + +// The test notification SendTest sends. +const ( + TestTitle = "MealQuest test notification" + TestBody = "Push works on this device. Tap to open the plan." + TestURL = "/plan" +) + +// ErrNoSubscriptions is returned by SendTest when no device has turned on +// push: there is nothing to send the test to. +var ErrNoSubscriptions = errors.New("push: no device has turned on notifications") + +// TestSent is a test notification queued for delivery: its event key and +// how many subscriptions were stored when it was queued. +type TestSent struct { + EventKey string `json:"event_key"` + Subscriptions int `json:"subscriptions"` +} + +// SendTest queues TestTitle for every stored subscription, through the +// 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. +func (s *Service) SendTest(ctx context.Context) (TestSent, error) { + subs, err := db.New(s.st.Reader).ListPushSubscriptions(ctx) + if err != nil { + return TestSent{}, fmt.Errorf("push: list subscriptions: %w", err) + } + if len(subs) == 0 { + return TestSent{}, ErrNoSubscriptions + } + key := string(notify.PlanReady) + ":test-" + store.NewID() + err = s.notifier.EmitPush(ctx, notify.Event{ + Type: notify.PlanReady, + Key: key, + Title: TestTitle, + Body: TestBody, + URL: TestURL, + }) + if err != nil { + return TestSent{}, fmt.Errorf("push: send test notification: %w", err) + } + return TestSent{EventKey: key, Subscriptions: len(subs)}, nil } // PublicKey returns the VAPID public key, for applicationServerKey. diff --git a/internal/push/push_test.go b/internal/push/push_test.go index a07eb05..255fb89 100644 --- a/internal/push/push_test.go +++ b/internal/push/push_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify/notifytest" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/vapid" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/migrations" @@ -35,7 +37,7 @@ func newTestService(t *testing.T) (*Service, *store.Store) { if err != nil { t.Fatal(err) } - svc, err := NewService(st, pair.PublicKey) + svc, err := NewService(st, pair.PublicKey, notifytest.New(t, st)) if err != nil { t.Fatal(err) } @@ -86,12 +88,20 @@ func rows(t *testing.T, st *store.Store) map[string]storedRow { func TestNewServiceRefusesBadPublicKey(t *testing.T) { t.Parallel() _, st := newTestService(t) - if _, err := NewService(st, "not-a-key"); err == nil { + notifier := notifytest.New(t, st) + if _, err := NewService(st, "not-a-key", notifier); err == nil { t.Error("NewService accepted a malformed public key") } - if _, err := NewService(nil, ""); err == nil { + if _, err := NewService(nil, "", notifier); err == nil { t.Error("NewService accepted a nil store") } + pair, err := vapid.Generate() + if err != nil { + t.Fatal(err) + } + if _, err := NewService(st, pair.PublicKey, nil); err == nil { + t.Error("NewService accepted a nil notifier") + } } func TestSubscribeUpsertsByEndpoint(t *testing.T) { @@ -222,3 +232,77 @@ func TestSubscribeValidates(t *testing.T) { t.Errorf("invalid requests stored %d rows", len(got)) } } + +func TestSendTestQueuesOnePushToPlan(t *testing.T) { + t.Parallel() + svc, st := newTestService(t) + p256dh, auth := browserKeys(t) + for _, ep := range []string{endpoint, endpoint + "-2"} { + if _, err := svc.Subscribe(context.Background(), SubscribeRequest{Endpoint: ep, P256dh: p256dh, Auth: auth}); err != nil { + t.Fatal(err) + } + } + + first, err := svc.SendTest(context.Background()) + if err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(first.EventKey, "plan_ready:test-") || first.Subscriptions != 2 { + t.Errorf("SendTest = %+v, want a plan_ready:test- key and 2 subscriptions", first) + } + // A second test is a new event, not taken for the first. + second, err := svc.SendTest(context.Background()) + if err != nil { + t.Fatal(err) + } + if second.EventKey == first.EventKey { + t.Fatalf("two tests share the key %s", first.EventKey) + } + + // Seeded prefs send plan_ready to Google Chat too; the test never does. + rows := notifytest.Rows(t, st, notify.PlanReady) + if len(rows) != 2 { + t.Fatalf("rows = %+v, want one per test", rows) + } + byKey := map[string]notifytest.Row{} + for _, r := range rows { + byKey[r.Key] = r + } + for _, sent := range []TestSent{first, second} { + want := notifytest.Row{Key: sent.EventKey, Channel: notify.ChannelPush, Payload: notify.Payload{ + Event: notify.PlanReady, Title: "MealQuest test notification", Body: TestBody, URL: "/plan", Tag: sent.EventKey, + }} + if got := byKey[sent.EventKey]; got != want { + t.Errorf("row of %s = %+v, want %+v", sent.EventKey, got, want) + } + } +} + +func TestSendTestWithoutSubscriptions(t *testing.T) { + t.Parallel() + svc, st := newTestService(t) + if _, err := svc.SendTest(context.Background()); !errors.Is(err, ErrNoSubscriptions) { + t.Fatalf("SendTest = %v, want ErrNoSubscriptions", err) + } + if rows := notifytest.Rows(t, st, notify.PlanReady); len(rows) != 0 { + t.Fatalf("rows = %+v, want none", rows) + } +} + +func TestSendTestWithPushOff(t *testing.T) { + t.Parallel() + svc, st := newTestService(t) + p256dh, auth := browserKeys(t) + if _, err := svc.Subscribe(context.Background(), SubscribeRequest{Endpoint: endpoint, P256dh: p256dh, Auth: auth}); err != nil { + t.Fatal(err) + } + if _, err := st.Writer.Exec(`UPDATE notification_prefs SET push = 0 WHERE event = 'plan_ready'`); err != nil { + t.Fatal(err) + } + if _, err := svc.SendTest(context.Background()); !errors.Is(err, notify.ErrPushOff) { + 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 c78f413..5a816fb 100644 --- a/internal/server/push.go +++ b/internal/server/push.go @@ -5,6 +5,7 @@ import ( "log/slog" "net/http" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/push" ) @@ -85,6 +86,32 @@ 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. +func postTestPush(logger *slog.Logger, svc *push.Service) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + sent, err := svc.SendTest(r.Context()) + switch { + case errors.Is(err, push.ErrNoSubscriptions): + writeError(w, r, logger, http.StatusConflict, "no_subscriptions", + "no device has turned on notifications, so there is nothing to send the test to") + case errors.Is(err, notify.ErrPushOff): + writeError(w, r, logger, http.StatusConflict, "push_off", + "push is switched off for New weekly plan ready in Notifications settings, so no test was sent") + case err != nil: + logger.ErrorContext(r.Context(), "send test push", "error", err) + writeError(w, r, logger, http.StatusInternalServerError, "internal", "internal error") + default: + logger.InfoContext(r.Context(), "test push queued", "event_key", sent.EventKey, "subscriptions", sent.Subscriptions) + writeJSON(w, r, logger, http.StatusAccepted, sent) + } + } +} + // writePushError maps a push.Service error to its API error: an invalid // subscription is 400 invalid_subscription, anything else 500. func writePushError(w http.ResponseWriter, r *http.Request, logger *slog.Logger, action string, err error) { diff --git a/internal/server/push_test.go b/internal/server/push_test.go index 25e38a7..92e4305 100644 --- a/internal/server/push_test.go +++ b/internal/server/push_test.go @@ -14,6 +14,8 @@ import ( "testing" "testing/fstest" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify" + "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/notify/notifytest" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/push" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/store" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/vapid" @@ -22,6 +24,7 @@ import ( const ( vapidPublicKeyPath = "/api/v1/push/vapid-public-key" pushSubscriptionsPath = "/api/v1/push/subscriptions" + testPushPath = "/api/v1/settings/notifications/test-push" testPushEndpoint = "https://updates.push.services.mozilla.com/wpush/v2/gAAAAABk" ) @@ -41,7 +44,7 @@ func newTestPushWith(t *testing.T) (*push.Service, *store.Store, vapid.KeyPair) t.Fatal(err) } st := newTestStore(t) - svc, err := push.NewService(st, pair.PublicKey) + svc, err := push.NewService(st, pair.PublicKey, notifytest.New(t, st)) if err != nil { t.Fatal(err) } @@ -282,3 +285,46 @@ func mustP256dh(t *testing.T) string { } return base64.RawURLEncoding.EncodeToString(k.PublicKey().Bytes()) } + +func TestTestPushAPI(t *testing.T) { + t.Parallel() + svc, st, _ := newTestPushWith(t) + router := newPushRouter(t, svc) + + // 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) + } + + for _, ep := range []string{testPushEndpoint, testPushEndpoint + "-phone"} { + req := signedIn(http.MethodPost, pushSubscriptionsPath, strings.NewReader(subscriptionJSON(t, ep, ""))) + if rec, _ := serve(t, router, req); rec.Code != http.StatusOK { + t.Fatalf("POST subscription = %d %s", rec.Code, rec.Body) + } + } + rec, body = serve(t, router, signedIn(http.MethodPost, testPushPath, nil)) + key, _ := body["event_key"].(string) + if rec.Code != http.StatusAccepted || !strings.HasPrefix(key, "plan_ready:test-") || body["subscriptions"] != float64(2) { + t.Fatalf("POST test-push = %d %s, want 202 with a plan_ready:test- key and 2 subscriptions", rec.Code, rec.Body) + } + // One push row, none for the household's Google Chat space, opening /plan. + want := []notifytest.Row{{Key: key, Channel: notify.ChannelPush, Payload: notify.Payload{ + Event: notify.PlanReady, Title: "MealQuest test notification", Body: push.TestBody, URL: "/plan", Tag: key, + }}} + if got := notifytest.Rows(t, st, notify.PlanReady); len(got) != 1 || got[0] != want[0] { + t.Errorf("outbox = %+v, want %+v", got, want) + } + + // With push off for plan_ready, the test says so and sends nothing. + 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 = %d %s, want 409 push_off", rec.Code, rec.Body) + } + if got := notifytest.Rows(t, st, notify.PlanReady); len(got) != 1 { + t.Errorf("outbox after push off = %+v, want still one row", got) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index af7a20c..ed4fa91 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -60,8 +60,9 @@ type RouteMounter interface { // /api/v1/integrations/google and its /connect, /callback and /calendars, // PUT /api/v1/integrations/google/calendar and DELETE // /api/v1/integrations/google; pushSvc serves GET -// /api/v1/push/vapid-public-key and POST and DELETE -// /api/v1/push/subscriptions; runner serves +// /api/v1/push/vapid-public-key, POST and DELETE +// /api/v1/push/subscriptions and POST +// /api/v1/settings/notifications/test-push; runner serves // POST /api/v1/jobs and GET /api/v1/jobs/{id}; photos serves // GET /media/photos/{id}-{size}.jpg; assets is the web build // served for every other GET. /health, /auth and the web build are public. @@ -133,6 +134,7 @@ func NewRouter(logger *slog.Logger, version string, trusted forwarded.Trusted, a apiV1.Patch("/settings/household", patchHouseholdSettings(logger, settings)) apiV1.Get("/settings/notifications", getNotificationSettings(logger, settings)) apiV1.Put("/settings/notifications", putNotificationSettings(logger, settings)) + apiV1.Post("/settings/notifications/test-push", postTestPush(logger, pushSvc)) apiV1.Get("/settings/recipe-search", getRecipeSearchSettings(logger, search)) apiV1.Patch("/settings/recipe-search", patchRecipeSearchSettings(logger, search)) apiV1.Post("/settings/recipe-search/test", postRecipeSearchTest(logger, search)) diff --git a/web/src/onboarding/NotificationsStep.test.tsx b/web/src/onboarding/NotificationsStep.test.tsx index f3f3632..ee406c4 100644 --- a/web/src/onboarding/NotificationsStep.test.tsx +++ b/web/src/onboarding/NotificationsStep.test.tsx @@ -11,7 +11,8 @@ import { InDataRouter } from '../test/dataRouter.tsx' import { errorResponse, fakeLocation, jsonResponse, ME } from '../test/fetchStub.ts' import { expectMinHitArea } from '../test/hitArea.ts' import { mockViewport } from '../test/matchMedia.ts' -import { NotificationsStep, notificationsUnavailable, type EnableNotifications } from './NotificationsStep.tsx' +import { PUSH_ENDPOINT, restorePushEnvironment, stubPushEnvironment, VAPID_PUBLIC_KEY } from '../test/push.ts' +import { NotificationsStep } from './NotificationsStep.tsx' import styles from './Onboarding.module.css' const IPHONE_SAFARI = @@ -29,6 +30,7 @@ beforeEach(() => { }) afterEach(() => { + restorePushEnvironment() vi.restoreAllMocks() vi.unstubAllGlobals() Reflect.deleteProperty(navigator, 'maxTouchPoints') @@ -41,13 +43,20 @@ type Held = { body: unknown; answer: (response: Response) => void } // stubFirstRunServer stands in for the server on first run: signed in, // onboarding not finished. Each PATCH of the household settings is held in -// `held` until the test answers it. -function stubFirstRunServer() { +// `held` until the test answers it. It hands out the VAPID public key with +// vapidKey and stores push subscriptions in `subscriptions`. +function stubFirstRunServer(vapidKey: () => Response = () => jsonResponse(200, VAPID_PUBLIC_KEY)) { const held: Held[] = [] + const subscriptions: unknown[] = [] const fetchMock = vi.fn((input, init) => { const path = String(input) const method = init?.method ?? 'GET' if (method === 'GET' && path === '/api/v1/me') return Promise.resolve(jsonResponse(200, ME)) + if (method === 'GET' && path === '/api/v1/push/vapid-public-key') return Promise.resolve(vapidKey()) + if (method === 'POST' && path === '/api/v1/push/subscriptions') { + subscriptions.push(JSON.parse(String(init?.body))) + return Promise.resolve(jsonResponse(200, { id: 'sub-1', endpoint: PUSH_ENDPOINT })) + } if (method === 'GET' && path === '/api/v1/settings/household') { return Promise.resolve(jsonResponse(200, { ...HOUSEHOLD_SETTINGS, onboarding_completed_at: null })) } @@ -59,7 +68,7 @@ function stubFirstRunServer() { throw new Error(`unexpected fetch: ${method} ${path}`) }) vi.stubGlobal('fetch', fetchMock) - return { fetchMock, held } + return { fetchMock, held, subscriptions } } function completed(request: Held) { @@ -85,20 +94,16 @@ async function renderApp() { return server } -// renderStep renders step 3 alone with the given enableNotifications, and a -// Home that says it was reached. AuthBootstrap reads /api/v1/me for the CSRF -// token every write needs. -async function renderStep(enableNotifications?: EnableNotifications) { - const server = stubFirstRunServer() +// renderStep renders step 3 alone, and a Home that says it was reached. +// AuthBootstrap reads /api/v1/me for the CSRF token every write needs. +async function renderStep(vapidKey?: () => Response) { + const server = stubFirstRunServer(vapidKey) render( - } - /> + } /> Home} /> Regulars step} /> @@ -110,6 +115,29 @@ async function renderStep(enableNotifications?: EnableNotifications) { return server } +// finishFromResult taps "Finish setting up", the only action once a result +// shows, and expects onboarding finished and Home reached. +async function finishFromResult(server: ReturnType) { + expect(screen.queryByRole('button', { name: 'Turn on notifications' })).not.toBeInTheDocument() + expect(screen.queryByRole('button', { name: 'Maybe later' })).not.toBeInTheDocument() + const finish = screen.getByRole('button', { name: 'Finish setting up' }) + expectMinHitArea(finish) + // Not a silent success: onboarding is not finished behind the household's back. + expect(server.held).toHaveLength(0) + fireEvent.click(finish) + await vi.waitFor(() => expect(server.held).toHaveLength(1)) + expect(server.held[0]!.body).toEqual({ onboarding_completed: true }) + completed(server.held[0]!) + expect(await screen.findByRole('heading', { level: 1, name: 'Home' })).toBeInTheDocument() +} + +function useIphoneSafariTab() { + vi.spyOn(navigator, 'userAgent', 'get').mockReturnValue(IPHONE_SAFARI) + 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) +} + function turnOn() { return screen.getByRole('button', { name: 'Turn on notifications' }) } @@ -157,10 +185,7 @@ describe('onboarding step 3', () => { }) it('shows the install hint in an iOS Safari tab', async () => { - vi.spyOn(navigator, 'userAgent', 'get').mockReturnValue(IPHONE_SAFARI) - 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) + useIphoneSafariTab() await renderStep() expect(screen.getByRole('note')).toHaveTextContent('Tap Share, then Add to Home Screen to get notifications') }) @@ -180,91 +205,127 @@ describe('onboarding step 3', () => { expect(requestPermission).not.toHaveBeenCalled() }) - it('"Turn on notifications" says plainly that nothing was turned on while notifications are unavailable', async () => { + it('"Turn on notifications" asks permission from the tap, subscribes this device and says it is on', async () => { + const browser = stubPushEnvironment({ permission: 'default', answer: 'granted' }) + const server = await renderStep() + expect(browser.notification.requestPermission).not.toHaveBeenCalled() + + fireEvent.click(turnOn()) + expect(browser.notification.requestPermission).toHaveBeenCalledOnce() + expect(turnOn()).toBeDisabled() + expect(later()).toBeDisabled() + + expect(await screen.findByRole('status')).toHaveTextContent('Notifications are on for this device.') + expect(browser.pushManager.subscribe).toHaveBeenCalledOnce() + expect(server.subscriptions).toEqual([expect.objectContaining({ endpoint: PUSH_ENDPOINT })]) + await finishFromResult(server) + expect(browser.notification.requestPermission).toHaveBeenCalledOnce() + }) + + it('"Turn on notifications" says the browser blocked them, and how to allow them later', async () => { + const browser = stubPushEnvironment({ permission: 'default', answer: 'denied' }) + const server = await renderStep() + fireEvent.click(turnOn()) + + const status = await screen.findByRole('status') + expect(status).toHaveTextContent("Notifications aren't on: this browser blocked them.") + expect(status).toHaveTextContent("allow notifications for this site in the browser's site settings") + expect(status).toHaveTextContent('turn them on in Settings under Notifications') + expect(browser.pushManager.subscribe).not.toHaveBeenCalled() + expect(server.subscriptions).toEqual([]) + await finishFromResult(server) + }) + + it('"Turn on notifications" says blocked without asking again when the browser already denied them', async () => { + const browser = stubPushEnvironment({ permission: 'denied' }) + const server = await renderStep() + fireEvent.click(turnOn()) + + expect(await screen.findByRole('status')).toHaveTextContent("Notifications aren't on: this browser blocked them.") + expect(browser.notification.requestPermission).not.toHaveBeenCalled() + await finishFromResult(server) + }) + + it('"Turn on notifications" says plainly that a browser without Web Push turned nothing on', async () => { + // The app as a whole: jsdom has no service worker or PushManager. const server = await renderApp() fireEvent.click(turnOn()) expect(await screen.findByRole('status')).toHaveTextContent( - "Notifications aren't on yet: MealQuest can't send them until the notification feature ships, so nothing was turned on.", + "Notifications aren't on: this browser can't receive them, so nothing was turned on.", ) - // Not a silent success: onboarding is not finished behind the household's back. - expect(server.held).toHaveLength(0) expect(screen.getByRole('heading', { level: 1, name: 'Stay in the loop' })).toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Turn on notifications' })).not.toBeInTheDocument() - expect(screen.queryByRole('button', { name: 'Maybe later' })).not.toBeInTheDocument() - - const finish = screen.getByRole('button', { name: 'Finish setting up' }) - expectMinHitArea(finish) - fireEvent.click(finish) - await vi.waitFor(() => expect(server.held).toHaveLength(1)) - expect(server.held[0]!.body).toEqual({ onboarding_completed: true }) - completed(server.held[0]!) - - expect(await screen.findByRole('heading', { level: 1, name: 'Home' })).toBeInTheDocument() + await finishFromResult(server) expect(screen.getByRole('navigation', { name: 'Primary' })).toBeInTheDocument() expect(requestPermission).not.toHaveBeenCalled() }) - it('"Turn on notifications" says the browser blocked them when denied, then finishes on request', async () => { - const server = await renderStep(() => Promise.resolve('denied')) + it('"Turn on notifications" in an iOS Safari tab says to add MealQuest to the Home Screen first', async () => { + useIphoneSafariTab() + const server = await renderStep() fireEvent.click(turnOn()) - expect(await screen.findByRole('status')).toHaveTextContent("Notifications aren't on: this browser blocked them.") - expect(server.held).toHaveLength(0) - - fireEvent.click(screen.getByRole('button', { name: 'Finish setting up' })) - await vi.waitFor(() => expect(server.held).toHaveLength(1)) - completed(server.held[0]!) - expect(await screen.findByRole('heading', { level: 1, name: 'Home' })).toBeInTheDocument() - }) - - it('"Turn on notifications" asks the injected enableNotifications first, then finishes', async () => { - let answer: (result: 'enabled') => void = () => {} - const enable = vi.fn( - () => - new Promise((resolve) => { - answer = resolve - }), + 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.", ) - const server = await renderStep(enable) - expect(enable).not.toHaveBeenCalled() - - fireEvent.click(turnOn()) - expect(enable).toHaveBeenCalledOnce() - expect(turnOn()).toBeDisabled() - expect(later()).toBeDisabled() - // Onboarding is not finished until enableNotifications has answered. - await Promise.resolve() - expect(server.held).toHaveLength(0) - - answer('enabled') - await vi.waitFor(() => expect(server.held).toHaveLength(1)) - expect(server.held[0]!.body).toEqual({ onboarding_completed: true }) - completed(server.held[0]!) - expect(await screen.findByRole('heading', { level: 1, name: 'Home' })).toBeInTheDocument() + expect(screen.getByRole('note')).toHaveTextContent('Tap Share, then Add to Home Screen to get notifications') + expect(requestPermission).not.toHaveBeenCalled() + await finishFromResult(server) }) - it('"Maybe later" does not ask the injected enableNotifications', async () => { - const enable = vi.fn(() => Promise.resolve('enabled')) - const server = await renderStep(enable) + it('"Turn on notifications" can ask again after the prompt is dismissed', async () => { + const browser = stubPushEnvironment({ permission: 'default', answer: 'default' }) + const server = await renderStep() + fireEvent.click(turnOn()) + + expect(await screen.findByRole('status')).toHaveTextContent( + 'The permission prompt was closed without an answer, so notifications are still off.', + ) + expect(server.held).toHaveLength(0) + await vi.waitFor(() => expect(turnOn()).toBeEnabled()) + expect(later()).toBeEnabled() + + fireEvent.click(turnOn()) + await vi.waitFor(() => expect(browser.notification.requestPermission).toHaveBeenCalledTimes(2)) + }) + + it('"Maybe later" finishes without asking for permission or subscribing', async () => { + const browser = stubPushEnvironment({ permission: 'default', answer: 'granted' }) + const server = await renderStep() fireEvent.click(later()) await vi.waitFor(() => expect(server.held).toHaveLength(1)) completed(server.held[0]!) expect(await screen.findByRole('heading', { level: 1, name: 'Home' })).toBeInTheDocument() - expect(enable).not.toHaveBeenCalled() + expect(browser.notification.requestPermission).not.toHaveBeenCalled() + expect(browser.pushManager.subscribe).not.toHaveBeenCalled() + expect(server.subscriptions).toEqual([]) }) - it('says why and stays when enableNotifications fails', async () => { - const server = await renderStep(() => Promise.reject(new Error('push service down'))) + it('says why and stays when turning on fails', async () => { + stubPushEnvironment({ permission: 'granted' }) + const server = await renderStep(() => errorResponse(503, 'push_unavailable')) fireEvent.click(turnOn()) - expect(await screen.findByRole('alert')).toHaveTextContent('Could not turn on notifications: push service down') + expect(await screen.findByRole('alert')).toHaveTextContent('Could not turn on notifications: push_unavailable message') + expect(screen.queryByRole('status')).not.toBeInTheDocument() expect(server.held).toHaveLength(0) expect(turnOn()).toBeEnabled() expect(screen.getByRole('heading', { level: 1, name: 'Stay in the loop' })).toBeInTheDocument() }) + it('refuses to ask for permission outside a tap, and says so', async () => { + const browser = stubPushEnvironment({ permission: 'default' }) + Object.defineProperty(navigator, 'userActivation', { configurable: true, value: { isActive: false } }) + await renderStep() + fireEvent.click(turnOn()) + + expect(await screen.findByRole('alert')).toHaveTextContent( + 'Could not turn on notifications: subscribePush: notification permission can only be asked for from a tap on a button', + ) + expect(browser.notification.requestPermission).not.toHaveBeenCalled() + }) + it('says why and stays when the server refuses to finish onboarding, then finishes on retry', async () => { const server = await renderApp() fireEvent.click(later()) @@ -317,10 +378,3 @@ describe('onboarding step 3 with too few Regulars', () => { expect(screen.queryByRole('button', { name: 'Add Regulars' })).not.toBeInTheDocument() }) }) - -describe('notificationsUnavailable', () => { - it('answers "unavailable" without asking for permission', async () => { - await expect(notificationsUnavailable()).resolves.toBe('unavailable') - expect(requestPermission).not.toHaveBeenCalled() - }) -}) diff --git a/web/src/onboarding/NotificationsStep.tsx b/web/src/onboarding/NotificationsStep.tsx index 4c43f7b..be48c82 100644 --- a/web/src/onboarding/NotificationsStep.tsx +++ b/web/src/onboarding/NotificationsStep.tsx @@ -1,32 +1,37 @@ 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 } from '../pwa/InstallHint.tsx' +import { InstallHint, isIosSafari, 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' import { needsMoreRegulars, useCompleteOnboarding } from '../settings/api.ts' import { Button } from '../ui/Button.tsx' import { Icon } from '../ui/Icon.tsx' import { OnboardingStep } from './OnboardingStep.tsx' import styles from './Onboarding.module.css' -// NotificationsResult is how asking to turn on notifications ended. -export type NotificationsResult = 'enabled' | 'denied' | 'unavailable' +// IOS_UNSUPPORTED says how to get notifications on iPhone and iPad, where +// Web Push reaches only the app installed to the Home Screen. +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." -export type EnableNotifications = () => Promise - -// TODO(Feature 19): Web Push. Until it is wired, notifications cannot be -// turned on, and nothing asks the browser for permission. This placeholder -// is the interim state: "Turn on notifications" answers 'unavailable', and -// the step tells the household plainly that nothing was turned on. -export function notificationsUnavailable(): Promise { - return Promise.resolve('unavailable') -} - -// notOnReasons says why notifications are not on after asking, for each -// result other than 'enabled'. -const notOnReasons: Record, string> = { - unavailable: - "Notifications aren't on yet: MealQuest can't send them until the notification feature ships, so nothing was turned on. You can finish setting up now.", - denied: "Notifications aren't on: this browser blocked them. You can allow them in the browser's settings later.", +// resultText says how turning on notifications ended. 'dismissed' leaves the +// choice open, so it is the only result that keeps "Turn on notifications". +function resultText(result: SubscribePushResult): string { + switch (result) { + case 'subscribed': + return "Notifications are on for this device. MealQuest will let the household know when there's something to look at." + 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() + ? IOS_UNSUPPORTED + : "Notifications aren't on: this browser can't receive them, so nothing was turned on." + case 'dismissed': + return "The permission prompt was closed without an answer, so notifications are still off. Turn them on to be asked again, or choose Maybe later." + } } const topics: { icon: LucideIcon; name: string; meaning: string }[] = [ @@ -52,27 +57,26 @@ const topics: { icon: LucideIcon; name: string; meaning: string }[] = [ }, ] -export type NotificationsStepProps = { - // Asks to turn on notifications when the household taps "Turn on - // notifications" — never on render. - enableNotifications?: EnableNotifications -} - // NotificationsStep is onboarding step 3: what MealQuest notifies about, the -// iOS install hint, and finishing onboarding. "Maybe later", and "Turn on -// notifications" once they are enabled, record that onboarding is finished -// and go Home. When they are not enabled, the step says so and offers only -// "Finish setting up". When the server refuses because the household has -// fewer than three Regulars, the reason shows with a way back to step 2 to -// add them. -export function NotificationsStep({ enableNotifications = notificationsUnavailable }: NotificationsStepProps) { +// iOS install hint, and finishing onboarding. "Turn on notifications" turns +// on Web Push on this device from that tap — the only place onboarding asks +// for permission — and says how it ended: on, blocked with how to allow it +// later, or not supported with how to get it on iPhone. Then only "Finish +// setting up" is offered, except after a dismissed prompt, which can be +// asked again. "Maybe later" finishes without asking. When the server +// refuses to finish because the household has fewer than three Regulars, +// the reason shows with a way back to step 2 to add them. +export function NotificationsStep() { const navigate = useNavigate() + const push = usePush() + const queryClient = useQueryClient() const complete = useCompleteOnboarding() const [enabling, setEnabling] = useState(false) const [problem, setProblem] = useState(null) const [fewRegulars, setFewRegulars] = useState(false) - const [notOn, setNotOn] = useState(null) + const [result, setResult] = useState(null) const busy = enabling || complete.isPending + const decided = result !== null && result !== 'dismissed' async function finish() { try { @@ -90,23 +94,20 @@ export function NotificationsStep({ enableNotifications = notificationsUnavailab setFewRegulars(false) } + // turnOn calls subscribePush straight from the tap, before anything is + // awaited, so the browser lets it ask for permission. async function turnOn() { clearProblem() + setResult(null) setEnabling(true) - let result: NotificationsResult try { - result = await enableNotifications() + setResult(await push.subscribePush()) } catch (error) { setProblem(`Could not turn on notifications: ${(error as Error).message}`) - return } finally { setEnabling(false) + void queryClient.invalidateQueries({ queryKey: PUSH_STATUS_KEY }) } - if (result !== 'enabled') { - setNotOn(notOnReasons[result]) - return - } - await finish() } function later() { @@ -119,7 +120,7 @@ export function NotificationsStep({ enableNotifications = notificationsUnavailab step={3} title="Stay in the loop" action={ - notOn ? ( + decided ? (