From cb920714b06cc5e3d064a8f38a9090ba3722b8c8 Mon Sep 17 00:00:00 2001 From: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com> Date: Tue, 6 Oct 2026 23:12:26 -0400 Subject: [PATCH] Release corrections: candidate URL exposure, onboarding notification honesty, pasted link cleanup (#101) Co-authored-by: orchestrator-agent <20+orchestrator-agent@code.cubecraftcreations.com> --- docs/ARCHITECTURE.md | 25 +- internal/db/dinners.sql.go | 4 +- internal/db/querier.go | 3 +- internal/dinners/dinners.go | 20 +- internal/dinners/dinners_test.go | 6 +- internal/dinners/export_test.go | 33 ++ internal/dinners/grounding.go | 160 +++++++++- internal/dinners/grounding_test.go | 291 +++++++++++++++++- internal/server/dinners_test.go | 4 +- internal/server/grounding_test.go | 103 ++++++- migrations/queries/dinners.sql | 4 +- web/src/onboarding/NotificationsStep.test.tsx | 27 +- web/src/onboarding/NotificationsStep.tsx | 59 +++- web/src/onboarding/Onboarding.module.css | 12 + web/src/sw.test.ts | 13 +- web/src/sw/routes.test.ts | 9 +- web/src/sw/routes.ts | 7 +- 17 files changed, 712 insertions(+), 68 deletions(-) diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2c66c03..ac09128 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -209,7 +209,8 @@ automatic** (D-021). and `grounding_job_id` null. The job grounds the dinner's `recipe_url` directly with `GroundURL`, else its name with `Ground` (see Recipe grounding), and sets grounding_status `grounded` with the - recipe's recipe_id, photo_id, method, minutes_total and difficulty, or + recipe's recipe_id, photo_id, method, minutes_total and difficulty, + clearing the pasted `recipe_url` (the recipe replaces it), or `failed` with `grounding_error` — also the job's failure code — one of `no_recipe_found`, `recipe_unusable` (the pasted link), `search_not_configured`, `search_failed`, `ai_failed`, `interrupted` (the server stopped) or @@ -233,10 +234,12 @@ automatic** (D-021). searches for the dinner's name and returns `{candidates: [{candidate_id, title, site_name, photo_url, minutes}]}`, at most 3, ranked as `Ground` ranks them; it fetches pages but downloads no photo and runs no model, - so `photo_url` is set only for a page already stored, and `candidate_id` - is the page's URL. `POST /api/v1/dinners/{id}/ground` takes exactly one - of `{candidate_id}`, `{recipe_url}` (both grounded with `GroundURL`, - setting `grounded`) or `{manual: {ingredients[], steps[], minutes, + so `photo_url` is set only for a page already stored. `candidate_id` is + an opaque id the server resolves to the page it last offered that dinner + (kept in memory, so a restart means fetching the candidates again); the + page's URL, which may carry a token, is never returned or cached. + `POST /api/v1/dinners/{id}/ground` takes exactly one of `{candidate_id}`, + `{recipe_url}` (both grounded with `GroundURL`, setting `grounded`) or `{manual: {ingredients[], steps[], minutes, method}}` (1–100 ingredient lines, up to 100 steps, each non-blank and at most 500 characters; minutes 1–1440). A manual recipe is stored as typed — no model runs — as a `recipes` row whose source_url is @@ -519,9 +522,9 @@ rules in `web/src/dinners/names.ts` folds letters with same table as the server's, and runs the same cases, `web/src/dinners/testdata/dedupe_cases.json`, as the server's) and notifications (`/onboarding/notifications`: what MealQuest notifies about, -the iOS install hint, "Turn on notifications" and "Maybe later"). Either -button sends `PATCH /api/v1/settings/household` with -`{"onboarding_completed": true}`; the server sets `onboarding_completed_at` +the iOS install hint, "Turn on notifications" and "Maybe later"). "Maybe +later", and "Turn on notifications" once notifications are enabled, send +`PATCH /api/v1/settings/household` with `{"onboarding_completed": true}`; the server sets `onboarding_completed_at` to the time of that update (keeping the first time if it is already set; the field itself is read-only and `false` is refused) and the app goes Home. Completing needs at least three Regulars (`household.MinRegulars`); with @@ -530,8 +533,10 @@ fewer the server answers 400 `invalid_settings` naming fewer — by its URL, or after another member removes a Regular — so it then shows the server's reason with an "Add Regulars" button back to step 2. Notification permission is never asked for on page load; until Feature 19 -wires Web Push, "Turn on notifications" asks for nothing. Offline on the saved pages the -settings cannot be read (the service worker never caches `/api/v1/settings`), +wires Web Push, "Turn on notifications" asks for nothing and says plainly +that notifications are not on yet, offering "Finish setting up" (which +completes onboarding as above) instead of a silent success. Offline on the +saved pages the settings cannot be read (the service worker never caches `/api/v1/settings`), so the saved pages open as they are. Regular dinners (`/settings/regulars`) lists the Regulars from diff --git a/internal/db/dinners.sql.go b/internal/db/dinners.sql.go index 74c929d..2530b37 100644 --- a/internal/db/dinners.sql.go +++ b/internal/db/dinners.sql.go @@ -347,6 +347,7 @@ func (q *Queries) GroundDinner(ctx context.Context, arg GroundDinnerParams) (Din const groundPendingDinner = `-- name: GroundPendingDinner :execrows UPDATE dinners SET recipe_id = ?1, + recipe_url = NULL, photo_id = ?2, method = COALESCE(method, ?3), minutes_total = COALESCE(minutes_total, ?4), @@ -375,7 +376,8 @@ type GroundPendingDinnerParams struct { // the grounding_revision it was grounded for; a dinner grounded some other // way, renamed or archived meanwhile is left as it is (0 rows). A method, // minutes_total or difficulty already set is kept: a pending dinner has -// them only when a member edited them while it was grounding. +// them only when a member edited them while it was grounding. Like +// GroundDinner it clears the pasted recipe_url: the recipe replaces the link. func (q *Queries) GroundPendingDinner(ctx context.Context, arg GroundPendingDinnerParams) (int64, error) { result, err := q.db.ExecContext(ctx, groundPendingDinner, arg.RecipeID, diff --git a/internal/db/querier.go b/internal/db/querier.go index 0a27a86..36966f8 100644 --- a/internal/db/querier.go +++ b/internal/db/querier.go @@ -80,7 +80,8 @@ type Querier interface { // the grounding_revision it was grounded for; a dinner grounded some other // way, renamed or archived meanwhile is left as it is (0 rows). A method, // minutes_total or difficulty already set is kept: a pending dinner has - // them only when a member edited them while it was grounding. + // them only when a member edited them while it was grounding. Like + // GroundDinner it clears the pasted recipe_url: the recipe replaces the link. GroundPendingDinner(ctx context.Context, arg GroundPendingDinnerParams) (int64, error) ListDinnersByStatus(ctx context.Context, status string) ([]Dinner, error) // The planned dinners dated from from_date up to but not including diff --git a/internal/dinners/dinners.go b/internal/dinners/dinners.go index ae2a20b..f929396 100644 --- a/internal/dinners/dinners.go +++ b/internal/dinners/dinners.go @@ -7,6 +7,7 @@ package dinners import ( "context" + "crypto/rand" "database/sql" "errors" "fmt" @@ -140,6 +141,18 @@ type Service struct { // by dinner id, so archiving or renaming a dinner can stop them. groundingsMu sync.Mutex groundings map[string]map[*runningGrounding]struct{} + + // offerKey keys the candidate ids Candidates offers, so they are + // opaque and stable for this process. + offerKey []byte + // offersMu guards offers and offerSearches. offers holds, by dinner + // id, the candidates last offered by Candidates until they expire, so + // a candidate id is resolved here and its URL never leaves the server. + // offerSearches holds the Candidates searches running, by dinner id, + // so clearing a dinner's offers keeps them from storing theirs. + offersMu sync.Mutex + offers map[string]*offerSet + offerSearches map[string]map[*offerSearch]struct{} } // NewService returns a service over st that grounds dinners with grounder @@ -156,7 +169,8 @@ func NewService(st *store.Store, grounder *recipes.Grounder, runner *jobs.Runner case logger == nil: return nil, errors.New("dinners: nil logger") } - s := &Service{st: st, grounder: grounder, logger: logger, groundings: make(map[string]map[*runningGrounding]struct{})} + s := &Service{st: st, grounder: grounder, logger: logger, groundings: make(map[string]map[*runningGrounding]struct{}), offerKey: make([]byte, 32), offers: make(map[string]*offerSet), offerSearches: make(map[string]map[*offerSearch]struct{})} + rand.Read(s.offerKey) // never fails: it crashes the program instead kind, err := jobs.Register(runner, KindGroundDinner, s.runGroundJob, Reasons...) if err != nil { return nil, err @@ -384,7 +398,8 @@ func (s *Service) Update(ctx context.Context, id string, p Patch, updatedBy stri // Archive sets dinner id's status to archived, recording updatedBy. The row // is kept, so its name still counts for Side quest dedupe. A -// KindGroundDinner job running for it is stopped. +// KindGroundDinner job running for it is stopped, and its candidates no +// longer ground it. func (s *Service) Archive(ctx context.Context, id, updatedBy string) (Dinner, error) { if updatedBy == "" { return Dinner{}, errors.New("dinners: archive: no member to record as updated_by") @@ -401,6 +416,7 @@ func (s *Service) Archive(ctx context.Context, id, updatedBy string) (Dinner, er return Dinner{}, fmt.Errorf("dinners: archive %s: %w", id, err) } s.stopGroundings(row.ID, math.MaxInt64) + s.clearOffers(row.ID) return fromRow(row), nil } diff --git a/internal/dinners/dinners_test.go b/internal/dinners/dinners_test.go index 620fdc6..764f6cb 100644 --- a/internal/dinners/dinners_test.go +++ b/internal/dinners/dinners_test.go @@ -16,8 +16,10 @@ import ( func TestCreateRegularNeverNamesFromLink(t *testing.T) { t.Parallel() e := newUnstartedEnv(t) - const link = "https://c.example/stew-key-abcdefghijklmno?sig=zyxwvut#frag" - leaks := []string{"c.example", "key", "abcdefghijklmno", "sig", "zyxwvut", "frag"} + const link = "https://c.example/stew-key-abcdefghijklmno?sig=zyxwvut#lookup" + // Each leak holds a character no ULID has (Crockford base32 has no I, L, + // O, U, "-", "." or "/"), so a dinner's id never matches one. + leaks := []string{link, "c.example", "stew-key", "abcdefghijklmno", "sig", "zyxwvut", "lookup"} check := func(what, text string) { t.Helper() lower := strings.ToLower(text) diff --git a/internal/dinners/export_test.go b/internal/dinners/export_test.go index ca06c60..cf7b488 100644 --- a/internal/dinners/export_test.go +++ b/internal/dinners/export_test.go @@ -14,6 +14,39 @@ func SetGroundTimeout(t *testing.T, d time.Duration) { t.Cleanup(func() { groundTimeout = old }) } +// SetOfferLifetime sets how long candidates ground a dinner until t ends. +// A test calling it must not run in parallel. +func SetOfferLifetime(t *testing.T, d time.Duration) { + old := offerLifetime + offerLifetime = d + t.Cleanup(func() { offerLifetime = old }) +} + +// AdvanceClock moves the service's clock d ahead of real time until t +// ends, without firing any timer. A test calling it must not run in +// parallel. +func AdvanceClock(t *testing.T, d time.Duration) { + old := now + now = func() time.Time { return old().Add(d) } + t.Cleanup(func() { now = old }) +} + +// HasOffers reports whether the service holds candidates offered for +// dinner id. +func (s *Service) HasOffers(id string) bool { + s.offersMu.Lock() + defer s.offersMu.Unlock() + _, ok := s.offers[id] + return ok +} + +// OfferCount is the number of dinners whose candidates the service holds. +func (s *Service) OfferCount() int { + s.offersMu.Lock() + defer s.offersMu.Unlock() + return len(s.offers) +} + // RunGroundJob runs the KindGroundDinner handler for p in the caller's // goroutine, as the job runner would, and returns the grounding status it // reports. diff --git a/internal/dinners/grounding.go b/internal/dinners/grounding.go index c978505..60fe2f0 100644 --- a/internal/dinners/grounding.go +++ b/internal/dinners/grounding.go @@ -2,7 +2,10 @@ package dinners import ( "context" + "crypto/hmac" + "crypto/sha256" "database/sql" + "encoding/base64" "encoding/json" "errors" "fmt" @@ -282,7 +285,9 @@ func reasonFor(err error, fromURL bool) string { // Candidate is a recipe page offered to ground a dinner in. type Candidate struct { - // CandidateID is the page's URL; Fix.CandidateID grounds it. + // CandidateID is an opaque id for the page, resolved by the server: + // Fix.CandidateID grounds it. The page's URL is never returned, as it + // may carry a token. CandidateID string `json:"candidate_id"` Title string `json:"title"` SiteName string `json:"site_name"` @@ -291,10 +296,39 @@ type Candidate struct { Minutes *int `json:"minutes"` } +// offerLifetime is how long the candidates Candidates offers a dinner +// ground it; tests shorten it. +var offerLifetime = 30 * time.Minute + +// now is the clock offer lifetimes are measured on; tests advance it. +var now = time.Now + +// offerSet is the candidates last offered for a dinner: the page URL of +// each candidate id. They ground the dinner until deadline; expiry drops +// the set from offers once it has passed. +type offerSet struct { + urls map[string]string + deadline time.Time + expiry *time.Timer +} + +// offerSearch is a Candidates search running for a dinner. It is stale +// once the dinner's offers are cleared, and then stores none. +type offerSearch struct { + stale bool +} + // Candidates searches for dinner id's name and returns up to MaxCandidates -// recipe pages to ground it in, best first; the list may be empty. A -// search that fails wraps recipes.ErrSearchFailed. +// recipe pages to ground it in, best first; the list may be empty. They +// replace the dinner's earlier candidates: one not offered again no longer +// grounds it. They ground it for offerLifetime, and not at all once it is +// grounded by hand or archived, even while this search runs. A search that +// fails wraps recipes.ErrSearchFailed. func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error) { + // Tracked before the dinner is read: grounding or archiving it after + // the read makes this search stale. + keep, done := s.trackOffers(id) + defer done() row, err := s.get(ctx, id) if err != nil { return nil, err @@ -304,16 +338,113 @@ func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error return nil, err } out := make([]Candidate, len(offers)) + urls := make(map[string]string, len(offers)) for i, o := range offers { - out[i] = Candidate{CandidateID: o.URL, Title: o.Title, SiteName: o.SiteName, Minutes: o.Minutes} + cid := s.candidateID(row.ID, o.URL) + urls[cid] = o.URL + out[i] = Candidate{CandidateID: cid, Title: o.Title, SiteName: o.SiteName, Minutes: o.Minutes} if o.PhotoID != "" { u := PhotoURL(o.PhotoID) out[i].PhotoURL = &u } } + if row.Status != StatusArchived { + keep(urls) + } return out, nil } +// trackOffers registers a Candidates search for dinner id. It returns keep, +// which stores urls as the dinner's offers for offerLifetime unless +// clearOffers has run for it since, and done, to call when the search ends. +func (s *Service) trackOffers(id string) (keep func(urls map[string]string), done func()) { + search := &offerSearch{} + s.offersMu.Lock() + if s.offerSearches[id] == nil { + s.offerSearches[id] = make(map[*offerSearch]struct{}) + } + s.offerSearches[id][search] = struct{}{} + s.offersMu.Unlock() + keep = func(urls map[string]string) { + s.offersMu.Lock() + defer s.offersMu.Unlock() + if search.stale { + return + } + s.dropOffers(id) + set := &offerSet{urls: urls, deadline: now().Add(offerLifetime)} + // expiry waits for offersMu, so it finds set stored even when it + // fires at once, and drops set only, never offers stored later. + set.expiry = time.AfterFunc(offerLifetime, func() { + s.offersMu.Lock() + defer s.offersMu.Unlock() + if s.offers[id] == set { + delete(s.offers, id) + } + }) + s.offers[id] = set + } + done = func() { + s.offersMu.Lock() + delete(s.offerSearches[id], search) + if len(s.offerSearches[id]) == 0 { + delete(s.offerSearches, id) + } + s.offersMu.Unlock() + } + return keep, done +} + +// clearOffers drops dinner id's offers and makes every Candidates search +// running for it stale, so no candidate offered before grounds it. Call +// it once the dinner is grounded by hand or archived. +func (s *Service) clearOffers(id string) { + s.offersMu.Lock() + defer s.offersMu.Unlock() + s.dropOffers(id) + for search := range s.offerSearches[id] { + search.stale = true + } +} + +// dropOffers drops dinner id's offers and stops their expiry. The caller +// holds offersMu. +func (s *Service) dropOffers(id string) { + if set := s.offers[id]; set != nil { + set.expiry.Stop() + delete(s.offers, id) + } +} + +// candidateID is the opaque id of the page at rawURL as a candidate for +// dinner id: the same page is offered under the same id until the service +// restarts, and the id reveals nothing of the URL. +func (s *Service) candidateID(id, rawURL string) string { + mac := hmac.New(sha256.New, s.offerKey) + mac.Write([]byte(id + "\x00" + rawURL)) + return base64.RawURLEncoding.EncodeToString(mac.Sum(nil)[:16]) +} + +// candidateURL is the page URL of candidate cid last offered for dinner id. +// A cid not among the dinner's live offers, or whose page is not a valid +// recipe link, is a *ValidationError. Offers past their deadline are not +// live, even before their expiry drops them. +func (s *Service) candidateURL(id, cid string) (string, error) { + var raw string + var ok bool + s.offersMu.Lock() + if set := s.offers[id]; set != nil && now().Before(set.deadline) { + raw, ok = set.urls[cid] + } + s.offersMu.Unlock() + if ok { + if u, err := checkRecipeURL(raw); err == nil { + return u.String(), nil + } + } + return "", invalid("candidate_id", "is not a candidate from recipe-candidates; reload the candidates") +} + // Fix grounds a dinner by hand: exactly one of a candidate from // Candidates, a pasted recipe link or a recipe typed in. type Fix struct { @@ -367,6 +498,11 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string if cur.Status == StatusArchived { return Dinner{}, ErrArchived } + if f.CandidateID != nil { + if link, err = s.candidateURL(cur.ID, *f.CandidateID); err != nil { + return Dinner{}, err + } + } groundCtx, cancel := context.WithTimeout(ctx, groundTimeout) defer cancel() gr, err := s.grounder.GroundURL(groundCtx, link) @@ -401,6 +537,7 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string if err != nil { return Dinner{}, fmt.Errorf("%w: dinner %s: record: %w", ErrGroundFailed, id, err) } + s.clearOffers(cur.ID) return fromRow(row), nil } @@ -422,9 +559,10 @@ func (s *Service) groundRejected(ctx context.Context, id string) error { } // groundManual stores m as dinner id's recipe, with its ingredients and -// steps, and records it on the dinner, all in one transaction. A missing -// or archived dinner is ErrNotFound or ErrArchived; any other failure -// wraps ErrGroundFailed. +// steps, and records it on the dinner, all in one transaction; the +// dinner's candidates then no longer ground it. A missing or archived +// dinner is ErrNotFound or ErrArchived; any other failure wraps +// ErrGroundFailed. func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, updatedBy string) (Dinner, error) { var row db.Dinner err := s.st.WithTx(ctx, func(tx *sql.Tx) error { @@ -522,6 +660,7 @@ func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, u if err != nil { return Dinner{}, fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err) } + s.clearOffers(row.ID) return fromRow(row), nil } @@ -550,11 +689,8 @@ func (f Fix) validate() (link string, manual *ManualRecipe, err error) { } switch { case f.CandidateID != nil: - u, err := checkRecipeURL(*f.CandidateID) - if err != nil { - return "", nil, invalid("candidate_id", "is not a candidate from recipe-candidates") - } - return u.String(), nil, nil + // Ground resolves the candidate to its page. + return "", nil, nil case f.RecipeURL != nil: u, err := checkRecipeURL(*f.RecipeURL) if err != nil { diff --git a/internal/dinners/grounding_test.go b/internal/dinners/grounding_test.go index 5cff022..4604419 100644 --- a/internal/dinners/grounding_test.go +++ b/internal/dinners/grounding_test.go @@ -9,6 +9,7 @@ import ( "io/fs" "log/slog" "slices" + "strings" "testing" "testing/fstest" "time" @@ -214,6 +215,39 @@ func TestCreateByRecipeURLGroundsThatURL(t *testing.T) { } } +// TestCreateByRecipeURLClearsLink: once background grounding resolves a +// pasted link, which may carry a personal token, the dinner no longer +// stores it; the recipe it grounded in records the page. +func TestCreateByRecipeURLClearsLink(t *testing.T) { + t.Parallel() + e := newEnv(t) + const link = "https://c.example/recipes/beef-stew?access_token=personal-token-abc123" + e.pages.Bodies[link] = recipestest.RecipePage("Beef Stew", "Stew Co", "https://c.example/stew.jpg", 150) + e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150)) + + c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Beef stew"), RecipeURL: ptr(link)}, member) + if err != nil { + t.Fatal(err) + } + if job := e.waitJob(t, *c.GroundingJobID); job.Status != jobs.StatusSucceeded { + t.Fatalf("job = %+v", job) + } + row := e.dinner(t, c.ID) + if row.GroundingStatus != "grounded" || !row.RecipeID.Valid { + t.Fatalf("dinner = %+v, want grounded", row) + } + if row.RecipeUrl.Valid { + t.Errorf("recipe_url = %q after grounding, want cleared", row.RecipeUrl.String) + } + rec, err := db.New(e.st.Reader).GetRecipe(t.Context(), row.RecipeID.String) + if err != nil { + t.Fatal(err) + } + if rec.SourceUrl != link { + t.Errorf("recipe source = %q, want %q", rec.SourceUrl, link) + } +} + func TestCreateGroundingFailureRecordsReason(t *testing.T) { t.Parallel() tests := []struct { @@ -642,6 +676,26 @@ func failedDinner(t *testing.T, e *env, name string) string { return c.ID } +// candidateID returns the candidate_id Candidates offers dinner id for the +// page titled title. +func candidateID(t *testing.T, e *env, id, title string) string { + t.Helper() + got, err := e.svc.Candidates(t.Context(), id) + if err != nil { + t.Fatal(err) + } + for _, c := range got { + if c.Title == title { + if strings.Contains(c.CandidateID, "://") { + t.Fatalf("candidate_id = %q, want an opaque id, not the page URL", c.CandidateID) + } + return c.CandidateID + } + } + t.Fatalf("candidates = %+v, want one titled %q", got, title) + return "" +} + func TestCandidates(t *testing.T) { t.Parallel() e := newEnv(t) @@ -664,14 +718,25 @@ func TestCandidates(t *testing.T) { } first, second := got[0], got[1] wantPhoto := dinners.PhotoURL(e.dinner(t, c.ID).PhotoID.String) - if first.CandidateID != curryA || first.Title != "Chicken Curry" || first.SiteName != "Curry House" || + if first.Title != "Chicken Curry" || first.SiteName != "Curry House" || first.PhotoURL == nil || *first.PhotoURL != wantPhoto || first.Minutes == nil || *first.Minutes != 40 { t.Errorf("first = %+v, want the stored curryA with photo %s", first, wantPhoto) } - if second.CandidateID != curryB || second.Title != "Easy Weeknight Curry" || second.SiteName != "B Kitchen" || + if second.Title != "Easy Weeknight Curry" || second.SiteName != "B Kitchen" || second.PhotoURL != nil || second.Minutes == nil || *second.Minutes != 25 { t.Errorf("second = %+v, want curryB without a photo", second) } + if first.CandidateID == "" || first.CandidateID == second.CandidateID || + first.CandidateID == curryA || second.CandidateID == curryB { + t.Errorf("candidate ids = %q, %q, want distinct opaque ids", first.CandidateID, second.CandidateID) + } + again, err := e.svc.Candidates(t.Context(), c.ID) + if err != nil { + t.Fatal(err) + } + if len(again) != 2 || again[0].CandidateID != first.CandidateID || again[1].CandidateID != second.CandidateID { + t.Errorf("candidates again = %+v, want the same ids", again) + } if _, err := e.svc.Candidates(t.Context(), "nope"); !errors.Is(err, dinners.ErrNotFound) { t.Errorf("unknown dinner: err = %v, want ErrNotFound", err) @@ -702,9 +767,11 @@ func TestFixViaCandidate(t *testing.T) { t.Parallel() e := newEnv(t) id := failedDinner(t, e, "Mystery curry") + e.search.Results["Mystery curry"] = []recipes.Candidate{{URL: curryA}, {URL: curryB}} + cid := candidateID(t, e, id, "Easy Weeknight Curry") e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) - d, err := e.svc.Ground(t.Context(), id, dinners.Fix{CandidateID: ptr(curryB)}, "member-2") + d, err := e.svc.Ground(t.Context(), id, dinners.Fix{CandidateID: ptr(cid)}, "member-2") if err != nil { t.Fatal(err) } @@ -721,6 +788,207 @@ func TestFixViaCandidate(t *testing.T) { } } +// wantStaleCandidate checks that candidate cid no longer grounds dinner +// id: Ground refuses it as invalid, and the dinner is unchanged. +func wantStaleCandidate(t *testing.T, e *env, id, cid string) { + t.Helper() + before := e.dinner(t, id) + _, err := e.svc.Ground(t.Context(), id, dinners.Fix{CandidateID: ptr(cid)}, member) + var invalid *dinners.ValidationError + if !errors.As(err, &invalid) { + t.Fatalf("stale candidate: err = %v, want *ValidationError", err) + } + if after := e.dinner(t, id); after != before { + t.Errorf("dinner changed by a stale candidate: %+v, was %+v", after, before) + } +} + +// TestGroundingOrArchivingClearsCandidates: once a dinner is grounded by +// hand — by candidate, link or manual recipe — or archived, the service +// holds none of its candidates' URLs and none of them grounds it. +func TestGroundingOrArchivingClearsCandidates(t *testing.T) { + t.Parallel() + tests := []struct { + name string + // clear grounds or archives dinner id, offered candidate cid. + clear func(t *testing.T, e *env, id, cid string) + // archived says the dinner ends archived, which Ground refuses + // before it reads a candidate. + archived bool + }{ + {"candidate", func(t *testing.T, e *env, id, cid string) { + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 40)) + if _, err := e.svc.Ground(t.Context(), id, dinners.Fix{CandidateID: ptr(cid)}, member); err != nil { + t.Fatal(err) + } + }, false}, + {"link", func(t *testing.T, e *env, id, _ string) { + e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150)) + if _, err := e.svc.Ground(t.Context(), id, dinners.Fix{RecipeURL: ptr(stew)}, member); err != nil { + t.Fatal(err) + } + }, false}, + {"manual", func(t *testing.T, e *env, id, _ string) { + m := &dinners.ManualRecipe{Ingredients: []string{"1 chicken"}, Minutes: ptr(40), Method: ptr("stovetop")} + if _, err := e.svc.Ground(t.Context(), id, dinners.Fix{Manual: m}, member); err != nil { + t.Fatal(err) + } + }, false}, + {"archive", func(t *testing.T, e *env, id, _ string) { + if _, err := e.svc.Archive(t.Context(), id, member); err != nil { + t.Fatal(err) + } + }, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + e := newEnv(t) + id := failedDinner(t, e, "Mystery curry") + e.search.Results["Mystery curry"] = []recipes.Candidate{{URL: curryA}, {URL: curryB}} + cidA := candidateID(t, e, id, "Chicken Curry") + cidB := candidateID(t, e, id, "Easy Weeknight Curry") + + tt.clear(t, e, id, cidA) + if e.svc.HasOffers(id) { + t.Errorf("service still holds the candidates of dinner %s", id) + } + if !tt.archived { + wantStaleCandidate(t, e, id, cidB) + } + }) + } +} + +// TestCandidatesRunningWhenCleared: a candidate search still running when +// its dinner is grounded by hand or archived stores none of the candidates +// it returns, so none of them grounds the dinner. +func TestCandidatesRunningWhenCleared(t *testing.T) { + t.Parallel() + tests := []struct { + name string + clear func(t *testing.T, e *env, id string) + // archived says the dinner ends archived. + archived bool + }{ + {"manual", func(t *testing.T, e *env, id string) { + m := &dinners.ManualRecipe{Ingredients: []string{"1 chicken"}, Minutes: ptr(40), Method: ptr("stovetop")} + if _, err := e.svc.Ground(t.Context(), id, dinners.Fix{Manual: m}, member); err != nil { + t.Fatal(err) + } + }, false}, + {"archive", func(t *testing.T, e *env, id string) { + if _, err := e.svc.Archive(t.Context(), id, member); err != nil { + t.Fatal(err) + } + }, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + e := newEnv(t) + id := failedDinner(t, e, "Mystery curry") + e.search.Results["Mystery curry"] = []recipes.Candidate{{URL: curryA}} + asked := len(e.search.Queries()) + gate := make(chan struct{}) + e.search.Gate = gate + + type result struct { + got []dinners.Candidate + err error + } + done := make(chan result, 1) + go func() { + got, err := e.svc.Candidates(t.Context(), id) + done <- result{got, err} + }() + deadline := time.Now().Add(10 * time.Second) + for len(e.search.Queries()) == asked { + if time.Now().After(deadline) { + t.Fatal("candidate search never started") + } + time.Sleep(5 * time.Millisecond) + } + tt.clear(t, e, id) + close(gate) + r := <-done + if r.err != nil { + t.Fatal(r.err) + } + if len(r.got) != 1 { + t.Fatalf("candidates = %+v, want curryA", r.got) + } + if e.svc.HasOffers(id) { + t.Errorf("a search finishing after dinner %s was cleared stored its candidates", id) + } + if !tt.archived { + wantStaleCandidate(t, e, id, r.got[0].CandidateID) + } + }) + } +} + +// TestCandidatesExpire: once their lifetime passes, a dinner's candidates +// are dropped and no longer ground it, with no other search running. +func TestCandidatesExpire(t *testing.T) { + dinners.SetOfferLifetime(t, 0) + e := newEnv(t) + id := failedDinner(t, e, "Mystery curry") + e.search.Results["Mystery curry"] = []recipes.Candidate{{URL: curryA}} + cid := candidateID(t, e, id, "Chicken Curry") + + waitNoOffers(t, e) + wantStaleCandidate(t, e, id, cid) +} + +// TestCandidatesExpireBeforeEviction: once their lifetime passes, a +// dinner's candidates no longer ground it, even while the service still +// holds them because their eviction has not run yet. +func TestCandidatesExpireBeforeEviction(t *testing.T) { + dinners.SetOfferLifetime(t, time.Hour) + e := newEnv(t) + id := failedDinner(t, e, "Mystery curry") + e.search.Results["Mystery curry"] = []recipes.Candidate{{URL: curryA}} + cid := candidateID(t, e, id, "Chicken Curry") + + dinners.AdvanceClock(t, time.Hour) + if !e.svc.HasOffers(id) { + t.Fatal("candidates evicted already, want them held until their timer fires") + } + wantStaleCandidate(t, e, id, cid) +} + +// TestExpiredCandidatesNotHeld: the service holds no dinner's candidates +// past their lifetime, however many dinners were offered some. +func TestExpiredCandidatesNotHeld(t *testing.T) { + dinners.SetOfferLifetime(t, 50*time.Millisecond) + e := newEnv(t) + names := map[string]string{"Mystery curry": curryA, "Other curry": curryB} + ids := make([]string, 0, len(names)) + for name, u := range names { + ids = append(ids, failedDinner(t, e, name)) + e.search.Results[name] = []recipes.Candidate{{URL: u}} + } + for _, id := range ids { + if _, err := e.svc.Candidates(t.Context(), id); err != nil { + t.Fatal(err) + } + } + waitNoOffers(t, e) +} + +// waitNoOffers waits for the service to hold no dinner's candidates. +func waitNoOffers(t *testing.T, e *env) { + t.Helper() + deadline := time.Now().Add(5 * time.Second) + for n := e.svc.OfferCount(); n > 0; n = e.svc.OfferCount() { + if time.Now().After(deadline) { + t.Fatalf("service still holds the expired candidates of %d dinners", n) + } + time.Sleep(5 * time.Millisecond) + } +} + // TestFixReplacesPendingLink: fixing a Regular added from a link that held // no recipe drops that link, so it no longer blocks adding the link again, // while the link of the recipe chosen does. @@ -729,12 +997,18 @@ func TestFixReplacesPendingLink(t *testing.T) { const lost = "https://gone.example/recipes/lamb-tagine" tests := []struct { name string - fix dinners.Fix + // fix is the fix for dinner id. + fix func(t *testing.T, e *env, id string) dinners.Fix // linkTaken is the recipe link the fixed dinner now has, if any. linkTaken string }{ - {"candidate", dinners.Fix{CandidateID: ptr(curryB)}, curryB}, - {"manual", dinners.Fix{Manual: &dinners.ManualRecipe{Ingredients: []string{"1 lamb"}, Minutes: ptr(90), Method: ptr("oven")}}, ""}, + {"candidate", func(t *testing.T, e *env, id string) dinners.Fix { + e.search.Results["Weeknight dinner"] = []recipes.Candidate{{URL: curryB}} + return dinners.Fix{CandidateID: ptr(candidateID(t, e, id, "Easy Weeknight Curry"))} + }, curryB}, + {"manual", func(*testing.T, *env, string) dinners.Fix { + return dinners.Fix{Manual: &dinners.ManualRecipe{Ingredients: []string{"1 lamb"}, Minutes: ptr(90), Method: ptr("oven")}} + }, ""}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -752,7 +1026,7 @@ func TestFixReplacesPendingLink(t *testing.T) { t.Fatal(err) } e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) - if _, err := e.svc.Ground(t.Context(), c.ID, tt.fix, member); err != nil { + if _, err := e.svc.Ground(t.Context(), c.ID, tt.fix(t, e, c.ID), member); err != nil { t.Fatal(err) } row, err := db.New(e.st.Reader).GetDinner(t.Context(), c.ID) @@ -855,7 +1129,8 @@ func TestFixValidation(t *testing.T) { tests := map[string]dinners.Fix{ "nothing": {}, "two": {CandidateID: ptr(curryA), RecipeURL: ptr(stew)}, - "candidate not URL": {CandidateID: ptr("cand-1")}, + "unknown candidate": {CandidateID: ptr("cand-1")}, + "candidate a URL": {CandidateID: ptr(curryA)}, "http link": {RecipeURL: ptr("http://a.example/x")}, "no ingredients": {Manual: &dinners.ManualRecipe{Minutes: ptr(20), Method: ptr("oven")}}, "blank ingredient": {Manual: &dinners.ManualRecipe{Ingredients: []string{" "}, Minutes: ptr(20), Method: ptr("oven")}}, diff --git a/internal/server/dinners_test.go b/internal/server/dinners_test.go index 49e617c..0a8edbd 100644 --- a/internal/server/dinners_test.go +++ b/internal/server/dinners_test.go @@ -292,9 +292,11 @@ func TestDinnerResponsesHideTokenInLinkPath(t *testing.T) { if rec.Code != http.StatusConflict { t.Fatalf("same link again = %d, want 409: %s", rec.Code, rec.Body) } + // Each secret holds a character no ULID has (Crockford base32 has no I, + // L, O, U, "-", "." or "/"), so the dinner's id never matches one. for _, resp := range []struct{ name, body string }{{"create", created}, {"list", listed}, {"duplicate", rec.Body.String()}} { lower := strings.ToLower(resp.body) - for _, secret := range []string{"c.example", "recipes", "key", "abcdefghijklmno"} { + for _, secret := range []string{link, "c.example", "recipes/stew", "stew-key", "abcdefghijklmno"} { if strings.Contains(lower, secret) { t.Errorf("%s response contains %q: %s", resp.name, secret, resp.body) } diff --git a/internal/server/grounding_test.go b/internal/server/grounding_test.go index e7333d9..1c29afc 100644 --- a/internal/server/grounding_test.go +++ b/internal/server/grounding_test.go @@ -68,6 +68,27 @@ func listedDinner(t *testing.T, h http.Handler, id string) map[string]any { return nil } +// candidateID returns the candidate_id GET recipe-candidates offers dinner +// id for the page titled title. +func candidateID(t *testing.T, h http.Handler, id, title string) string { + t.Helper() + rec, body := callDinners(t, h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "") + if rec.Code != http.StatusOK { + t.Fatalf("GET candidates = %d: %s", rec.Code, rec.Body) + } + for _, c := range body["candidates"].([]any) { + if c := c.(map[string]any); c["title"] == title { + cid := c["candidate_id"].(string) + if strings.Contains(cid, "://") { + t.Fatalf("candidate_id = %q, want an opaque id, not the page URL", cid) + } + return cid + } + } + t.Fatalf("candidates = %v, want one titled %q", body, title) + return "" +} + func TestPostDinnerReturnsBeforeGroundingCompletes(t *testing.T) { t.Parallel() e := newDinnersEnv(t).withRecipes(t) @@ -138,8 +159,11 @@ func TestGetRecipeCandidates(t *testing.T) { t.Fatalf("candidates = %v, want 2", body) } first := list[0].(map[string]any) + if cid, _ := first["candidate_id"].(string); cid == "" || strings.Contains(cid, "a.example") { + t.Errorf("candidate_id = %#v, want an opaque id", first["candidate_id"]) + } want := map[string]any{ - "candidate_id": curryPage, + "candidate_id": first["candidate_id"], "title": "Chicken Curry", "site_name": "Curry House", "photo_url": nil, @@ -186,7 +210,8 @@ func TestPostGroundViaCandidate(t *testing.T) { id := createDinner(t, e.h, `{"name":"Chicken curry"}`) e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) - rec, d := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"candidate_id":"`+curry2Page+`"}`) + cid := candidateID(t, e.h, id, "Weeknight Curry") + rec, d := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"candidate_id":"`+cid+`"}`) if rec.Code != http.StatusOK { t.Fatalf("POST = %d: %s", rec.Code, rec.Body) } @@ -198,12 +223,81 @@ func TestPostGroundViaCandidate(t *testing.T) { // The candidate is now stored, so it is offered with its photo. _, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "") for _, c := range body["candidates"].([]any) { - if c := c.(map[string]any); c["candidate_id"] == curry2Page && c["photo_url"] != photo { + if c := c.(map[string]any); c["candidate_id"] == cid && c["photo_url"] != photo { t.Errorf("grounded candidate photo_url = %v, want %s", c["photo_url"], photo) } } } +// TestRecipeCandidatesHideTokenURLs: a candidate page whose URL carries a +// token in its path or query is offered by an opaque id alone, which +// grounds the dinner in that page. +func TestRecipeCandidatesHideTokenURLs(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t) + const ( + pathToken = "path-token-abc123" + queryToken = "query-token-def456" + pathPage = "https://secret-path.example/u/" + pathToken + "/chicken-curry" + queryPage = "https://secret-query.example/curry?access_token=" + queryToken + ) + e.search.Results["Chicken curry"] = []recipes.Candidate{{URL: pathPage}, {URL: queryPage}} + e.pages.Bodies[pathPage] = recipestest.RecipePage("Chicken Curry", "Curry House", "https://a.example/curry.jpg", 40) + e.pages.Bodies[queryPage] = recipestest.RecipePage("Weeknight Curry", "B Kitchen", "https://b.example/curry.jpg", 25) + e.images.JPEGs["https://b.example/curry.jpg"] = recipestest.JPEG(t, 400, 320, 2) + id := createDinner(t, e.h, `{"name":"Chicken curry"}`) // runner not started: stays pending + + rec, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "") + if rec.Code != http.StatusOK { + t.Fatalf("GET = %d: %s", rec.Code, rec.Body) + } + if list, _ := body["candidates"].([]any); len(list) != 2 { + t.Fatalf("candidates = %v, want 2", body) + } + for _, secret := range []string{pathPage, queryPage, "secret-path.example", "secret-query.example", pathToken, queryToken, "access_token"} { + if strings.Contains(rec.Body.String(), secret) { + t.Errorf("candidates response contains %q: %s", secret, rec.Body) + } + } + + e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) + cid := candidateID(t, e.h, id, "Weeknight Curry") + rec, d := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+id+"/ground", `{"candidate_id":"`+cid+`"}`) + if rec.Code != http.StatusOK { + t.Fatalf("POST = %d: %s", rec.Code, rec.Body) + } + if strings.Contains(rec.Body.String(), queryToken) { + t.Errorf("ground response contains the token: %s", rec.Body) + } + var source string + if err := e.st.Reader.QueryRow(`SELECT source_url FROM recipes WHERE id = ?`, d["recipe_id"]).Scan(&source); err != nil { + t.Fatal(err) + } + if d["grounding_status"] != "grounded" || source != queryPage { + t.Errorf("dinner = %v in recipe %s, want grounded in %s", d, source, queryPage) + } +} + +// TestPostGroundRejectsCandidateNotOffered: a candidate_id is only one +// offered for that dinner; a page URL, or another dinner's candidate, is +// refused. +func TestPostGroundRejectsCandidateNotOffered(t *testing.T) { + t.Parallel() + e := newDinnersEnv(t).withRecipes(t) + id := createDinner(t, e.h, `{"name":"Chicken curry"}`) + other := createDinner(t, e.h, `{"name":"Pad thai"}`) + cid := candidateID(t, e.h, id, "Weeknight Curry") + for _, tc := range []struct{ name, dinner, cid string }{ + {"page URL", id, curry2Page}, + {"another dinner's", other, cid}, + } { + rec, body := callDinners(t, e.h, http.MethodPost, dinnersPath+"/"+tc.dinner+"/ground", `{"candidate_id":"`+tc.cid+`"}`) + if rec.Code != http.StatusBadRequest || errorCode(body) != "invalid_dinner" { + t.Errorf("%s: %d %v, want 400 invalid_dinner", tc.name, rec.Code, body) + } + } +} + func TestPostGroundViaRecipeURL(t *testing.T) { t.Parallel() e := newDinnersEnv(t).withRecipes(t) @@ -365,7 +459,8 @@ func TestPostGroundRejects(t *testing.T) { for _, tc := range []struct{ name, body, code string }{ {"empty", `{}`, "invalid_dinner"}, {"two kinds", `{"candidate_id":"https://a.example/x","recipe_url":"https://a.example/x"}`, "invalid_dinner"}, - {"candidate not a link", `{"candidate_id":"c1"}`, "invalid_dinner"}, + {"candidate not offered", `{"candidate_id":"c1"}`, "invalid_dinner"}, + {"candidate a page URL", `{"candidate_id":"https://a.example/x"}`, "invalid_dinner"}, {"http link", `{"recipe_url":"http://a.example/x"}`, "invalid_dinner"}, {"null", `{"recipe_url":null}`, "invalid_request"}, {"unknown field", `{"url":"https://a.example/x"}`, "invalid_request"}, diff --git a/migrations/queries/dinners.sql b/migrations/queries/dinners.sql index 43e96ae..d2cf5d9 100644 --- a/migrations/queries/dinners.sql +++ b/migrations/queries/dinners.sql @@ -118,9 +118,11 @@ RETURNING *; -- the grounding_revision it was grounded for; a dinner grounded some other -- way, renamed or archived meanwhile is left as it is (0 rows). A method, -- minutes_total or difficulty already set is kept: a pending dinner has --- them only when a member edited them while it was grounding. +-- them only when a member edited them while it was grounding. Like +-- GroundDinner it clears the pasted recipe_url: the recipe replaces the link. UPDATE dinners SET recipe_id = sqlc.arg(recipe_id), + recipe_url = NULL, photo_id = sqlc.arg(photo_id), method = COALESCE(method, sqlc.arg(method)), minutes_total = COALESCE(minutes_total, sqlc.arg(minutes_total)), diff --git a/web/src/onboarding/NotificationsStep.test.tsx b/web/src/onboarding/NotificationsStep.test.tsx index e606aa9..f3f3632 100644 --- a/web/src/onboarding/NotificationsStep.test.tsx +++ b/web/src/onboarding/NotificationsStep.test.tsx @@ -180,10 +180,22 @@ describe('onboarding step 3', () => { expect(requestPermission).not.toHaveBeenCalled() }) - it('"Turn on notifications" finishes onboarding and goes Home while notifications are unavailable', async () => { + it('"Turn on notifications" says plainly that nothing was turned on while notifications are unavailable', async () => { 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.", + ) + // 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]!) @@ -193,6 +205,19 @@ describe('onboarding step 3', () => { 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')) + 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( diff --git a/web/src/onboarding/NotificationsStep.tsx b/web/src/onboarding/NotificationsStep.tsx index f17ddd1..4c43f7b 100644 --- a/web/src/onboarding/NotificationsStep.tsx +++ b/web/src/onboarding/NotificationsStep.tsx @@ -14,11 +14,21 @@ export type NotificationsResult = 'enabled' | 'denied' | 'unavailable' 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. +// 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.", +} + const topics: { icon: LucideIcon; name: string; meaning: string }[] = [ { icon: CalendarDays, @@ -49,16 +59,19 @@ export type NotificationsStepProps = { } // NotificationsStep is onboarding step 3: what MealQuest notifies about, the -// iOS install hint, and finishing onboarding. Either button records that -// onboarding is finished and goes Home. 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. +// 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) { const navigate = useNavigate() const complete = useCompleteOnboarding() const [enabling, setEnabling] = useState(false) const [problem, setProblem] = useState(null) const [fewRegulars, setFewRegulars] = useState(false) + const [notOn, setNotOn] = useState(null) const busy = enabling || complete.isPending async function finish() { @@ -80,14 +93,19 @@ export function NotificationsStep({ enableNotifications = notificationsUnavailab async function turnOn() { clearProblem() setEnabling(true) + let result: NotificationsResult try { - await enableNotifications() + result = await enableNotifications() } catch (error) { setProblem(`Could not turn on notifications: ${(error as Error).message}`) return } finally { setEnabling(false) } + if (result !== 'enabled') { + setNotOn(notOnReasons[result]) + return + } await finish() } @@ -101,14 +119,22 @@ export function NotificationsStep({ enableNotifications = notificationsUnavailab step={3} title="Stay in the loop" action={ -
- - -
+ notOn ? ( +
+ +
+ ) : ( +
+ + +
+ ) } >

MealQuest can let the household know when there's something to look at:

@@ -128,6 +154,11 @@ export function NotificationsStep({ enableNotifications = notificationsUnavailab
+ {notOn && ( +

+ {notOn} +

+ )} {problem && (

{problem} diff --git a/web/src/onboarding/Onboarding.module.css b/web/src/onboarding/Onboarding.module.css index ef2703c..e2ac957 100644 --- a/web/src/onboarding/Onboarding.module.css +++ b/web/src/onboarding/Onboarding.module.css @@ -229,6 +229,18 @@ color: var(--color-accent-700); } +/* Step 3: why notifications are not on after asking for them. */ +.notOn { + margin: var(--space-4) 0 0; + padding: var(--space-4); + border-radius: var(--radius-lg); + background: var(--color-surface); + font-family: var(--font-body); + font-size: 15px; + line-height: 1.4; + color: var(--color-text); +} + /* Step 3: back to step 2 when the server wants more Regulars. */ .recover { margin-top: var(--space-3); diff --git a/web/src/sw.test.ts b/web/src/sw.test.ts index 75e6d20..d2f7b01 100644 --- a/web/src/sw.test.ts +++ b/web/src/sw.test.ts @@ -15,12 +15,16 @@ describe('service worker activation', () => { vi.unstubAllGlobals() }) - it('deletes the v1 API cache and keeps the precache, photo and font caches', async () => { - // A household on the v1 worker: its API cache held dinners with the - // pasted recipe_url; its photos and fonts are not in the precache. + it('deletes the v1 and v2 API caches and keeps the current caches', async () => { + // A household that ran the v1 and v2 workers: the v1 API cache held + // dinners with the pasted recipe_url, the v2 one recipe candidates whose + // candidate_id was the full URL. Its photos and fonts are not in the + // precache, and the current API cache may already exist. const names = [ 'mealquest-precache-v1', 'mealquest-api-v1', + 'mealquest-api-v2', + 'mealquest-api-v3', 'mealquest-media-v1', 'mealquest-fonts-v1', 'other-app-cache', @@ -42,6 +46,7 @@ describe('service worker activation', () => { expect(pending).toHaveLength(1) expect(cacheStorage.delete).toHaveBeenCalledWith('mealquest-api-v1') - expect(cacheStorage.delete).toHaveBeenCalledTimes(1) + expect(cacheStorage.delete).toHaveBeenCalledWith('mealquest-api-v2') + expect(cacheStorage.delete).toHaveBeenCalledTimes(2) }) }) diff --git a/web/src/sw/routes.test.ts b/web/src/sw/routes.test.ts index 62685b3..e4857b8 100644 --- a/web/src/sw/routes.test.ts +++ b/web/src/sw/routes.test.ts @@ -147,12 +147,13 @@ describe('cache names', () => { for (const name of Object.values(cacheNames)) expect(name).toMatch(/^mealquest-/) }) - it('retire only the v1 API cache', () => { - // v1 API responses held dinners with the pasted recipe_url, which can - // carry a personal token; offline, the worker would still serve them. + it('retire only the v1 and v2 API caches', () => { + // v1 API responses held dinners with the pasted recipe_url and v2 ones + // recipe candidates whose candidate_id was the full URL; either can carry + // a personal token, and offline the worker would still serve them. // The precache, photos and fonts keep their v1 caches. expect(cacheNames).toEqual({ - api: 'mealquest-api-v2', + api: 'mealquest-api-v3', media: 'mealquest-media-v1', fonts: 'mealquest-fonts-v1', }) diff --git a/web/src/sw/routes.ts b/web/src/sw/routes.ts index e1f0ea3..c741660 100644 --- a/web/src/sw/routes.ts +++ b/web/src/sw/routes.ts @@ -29,9 +29,10 @@ export interface RouteRequest { // photos and fonts stay available offline. export const cacheVersions = { precache: 'v1', - // v2 retires the v1 API cache, whose dinner responses still carried the - // pasted recipe_url. - api: 'v2', + // v2 retired the v1 API cache, whose dinner responses still carried the + // pasted recipe_url; v3 retires the v2 API cache, whose recipe-candidate + // responses used the full URL as candidate_id. + api: 'v3', media: 'v1', fonts: 'v1', } as const