Hide candidate URLs, report unavailable notifications, clear grounded links
CI / ci (push) Successful in 2m41s

Release corrections from the review of promotion pull request #98:

- MQ-98-01: recipe-candidates returns an opaque candidate_id (an HMAC of
  the dinner and page under a per-process key) that the server resolves
  to the page it last offered that dinner. Candidate URLs, which may
  carry a token, no longer reach responses or the service-worker cache.
- MQ-98-02: "Turn on notifications" no longer finishes onboarding
  silently when notifications are unavailable (or denied); the step says
  plainly that nothing was turned on and offers "Finish setting up".
  The placeholder stays until Feature 19 wires Web Push.
- MQ-98-03: GroundPendingDinner clears the pasted recipe_url when
  background grounding succeeds, as manual grounding already did.

Maestro task 6819ce33-3715-4bd4-9208-a54a8b5cee02.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-06 22:16:44 -04:00
co-authored by Claude Opus 5.5
parent a3648397ee
commit e9e9087b48
12 changed files with 356 additions and 52 deletions
+14 -9
View File
@@ -209,7 +209,8 @@ automatic** (D-021).
and `grounding_job_id` null. The job grounds the dinner's and `grounding_job_id` null. The job grounds the dinner's
`recipe_url` directly with `GroundURL`, else its name with `Ground` `recipe_url` directly with `GroundURL`, else its name with `Ground`
(see Recipe grounding), and sets grounding_status `grounded` with the (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 `failed` with `grounding_error` — also the job's failure code — one of
`no_recipe_found`, `recipe_unusable` (the pasted link), `search_not_configured`, `no_recipe_found`, `recipe_unusable` (the pasted link), `search_not_configured`,
`search_failed`, `ai_failed`, `interrupted` (the server stopped) or `search_failed`, `ai_failed`, `interrupted` (the server stopped) or
@@ -233,9 +234,11 @@ automatic** (D-021).
searches for the dinner's name and returns `{candidates: [{candidate_id, searches for the dinner's name and returns `{candidates: [{candidate_id,
title, site_name, photo_url, minutes}]}`, at most 3, ranked as `Ground` 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, 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` so `photo_url` is set only for a page already stored. `candidate_id` is
is the page's URL. `POST /api/v1/dinners/{id}/ground` takes exactly one an opaque id the server resolves to the page it last offered that dinner
of `{candidate_id}`, `{recipe_url}` (both grounded with `GroundURL`, (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, setting `grounded`) or `{manual: {ingredients[], steps[], minutes,
method}}` (1–100 ingredient lines, up to 100 steps, each non-blank and at 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 most 500 characters; minutes 1–1440). A manual recipe is stored as typed
@@ -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, same table as the server's, and runs the same cases,
`web/src/dinners/testdata/dedupe_cases.json`, as the server's) and `web/src/dinners/testdata/dedupe_cases.json`, as the server's) and
notifications (`/onboarding/notifications`: what MealQuest notifies about, notifications (`/onboarding/notifications`: what MealQuest notifies about,
the iOS install hint, "Turn on notifications" and "Maybe later"). Either the iOS install hint, "Turn on notifications" and "Maybe later"). "Maybe
button sends `PATCH /api/v1/settings/household` with later", and "Turn on notifications" once notifications are enabled, send
`{"onboarding_completed": true}`; the server sets `onboarding_completed_at` `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; 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. the field itself is read-only and `false` is refused) and the app goes Home.
Completing needs at least three Regulars (`household.MinRegulars`); with 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 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. 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 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 wires Web Push, "Turn on notifications" asks for nothing and says plainly
settings cannot be read (the service worker never caches `/api/v1/settings`), 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. so the saved pages open as they are.
Regular dinners (`/settings/regulars`) lists the Regulars from Regular dinners (`/settings/regulars`) lists the Regulars from
+3 -1
View File
@@ -347,6 +347,7 @@ func (q *Queries) GroundDinner(ctx context.Context, arg GroundDinnerParams) (Din
const groundPendingDinner = `-- name: GroundPendingDinner :execrows const groundPendingDinner = `-- name: GroundPendingDinner :execrows
UPDATE dinners UPDATE dinners
SET recipe_id = ?1, SET recipe_id = ?1,
recipe_url = NULL,
photo_id = ?2, photo_id = ?2,
method = COALESCE(method, ?3), method = COALESCE(method, ?3),
minutes_total = COALESCE(minutes_total, ?4), 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 // 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, // 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 // 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) { func (q *Queries) GroundPendingDinner(ctx context.Context, arg GroundPendingDinnerParams) (int64, error) {
result, err := q.db.ExecContext(ctx, groundPendingDinner, result, err := q.db.ExecContext(ctx, groundPendingDinner,
arg.RecipeID, arg.RecipeID,
+2 -1
View File
@@ -80,7 +80,8 @@ type Querier interface {
// the grounding_revision it was grounded for; a dinner grounded some other // 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, // 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 // 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) GroundPendingDinner(ctx context.Context, arg GroundPendingDinnerParams) (int64, error)
ListDinnersByStatus(ctx context.Context, status string) ([]Dinner, error) ListDinnersByStatus(ctx context.Context, status string) ([]Dinner, error)
// The planned dinners dated from from_date up to but not including // The planned dinners dated from from_date up to but not including
+12 -1
View File
@@ -7,6 +7,7 @@ package dinners
import ( import (
"context" "context"
"crypto/rand"
"database/sql" "database/sql"
"errors" "errors"
"fmt" "fmt"
@@ -140,6 +141,15 @@ type Service struct {
// by dinner id, so archiving or renaming a dinner can stop them. // by dinner id, so archiving or renaming a dinner can stop them.
groundingsMu sync.Mutex groundingsMu sync.Mutex
groundings map[string]map[*runningGrounding]struct{} 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: by dinner id, the page URL of each candidate
// id last offered by Candidates, so a candidate id is resolved here
// and its URL never leaves the server.
offersMu sync.Mutex
offers map[string]map[string]string
} }
// NewService returns a service over st that grounds dinners with grounder // NewService returns a service over st that grounds dinners with grounder
@@ -156,7 +166,8 @@ func NewService(st *store.Store, grounder *recipes.Grounder, runner *jobs.Runner
case logger == nil: case logger == nil:
return nil, errors.New("dinners: nil logger") 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]map[string]string)}
rand.Read(s.offerKey) // never fails: it crashes the program instead
kind, err := jobs.Register(runner, KindGroundDinner, s.runGroundJob, Reasons...) kind, err := jobs.Register(runner, KindGroundDinner, s.runGroundJob, Reasons...)
if err != nil { if err != nil {
return nil, err return nil, err
+51 -9
View File
@@ -2,7 +2,10 @@ package dinners
import ( import (
"context" "context"
"crypto/hmac"
"crypto/sha256"
"database/sql" "database/sql"
"encoding/base64"
"encoding/json" "encoding/json"
"errors" "errors"
"fmt" "fmt"
@@ -282,7 +285,9 @@ func reasonFor(err error, fromURL bool) string {
// Candidate is a recipe page offered to ground a dinner in. // Candidate is a recipe page offered to ground a dinner in.
type Candidate struct { 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"` CandidateID string `json:"candidate_id"`
Title string `json:"title"` Title string `json:"title"`
SiteName string `json:"site_name"` SiteName string `json:"site_name"`
@@ -292,8 +297,9 @@ type Candidate struct {
} }
// Candidates searches for dinner id's name and returns up to MaxCandidates // 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 // recipe pages to ground it in, best first; the list may be empty. They
// search that fails wraps recipes.ErrSearchFailed. // replace the dinner's earlier candidates: one not offered again no longer
// grounds it. A search that fails wraps recipes.ErrSearchFailed.
func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error) { func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error) {
row, err := s.get(ctx, id) row, err := s.get(ctx, id)
if err != nil { if err != nil {
@@ -304,16 +310,47 @@ func (s *Service) Candidates(ctx context.Context, id string) ([]Candidate, error
return nil, err return nil, err
} }
out := make([]Candidate, len(offers)) out := make([]Candidate, len(offers))
urls := make(map[string]string, len(offers))
for i, o := range 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 != "" { if o.PhotoID != "" {
u := PhotoURL(o.PhotoID) u := PhotoURL(o.PhotoID)
out[i].PhotoURL = &u out[i].PhotoURL = &u
} }
} }
s.offersMu.Lock()
s.offers[row.ID] = urls
s.offersMu.Unlock()
return out, nil return out, nil
} }
// 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 candidates last offered since the service
// started, or whose page is not a valid recipe link, is a
// *ValidationError.
func (s *Service) candidateURL(id, cid string) (string, error) {
s.offersMu.Lock()
raw, ok := s.offers[id][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 // Fix grounds a dinner by hand: exactly one of a candidate from
// Candidates, a pasted recipe link or a recipe typed in. // Candidates, a pasted recipe link or a recipe typed in.
type Fix struct { type Fix struct {
@@ -367,6 +404,11 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
if cur.Status == StatusArchived { if cur.Status == StatusArchived {
return Dinner{}, ErrArchived 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) groundCtx, cancel := context.WithTimeout(ctx, groundTimeout)
defer cancel() defer cancel()
gr, err := s.grounder.GroundURL(groundCtx, link) gr, err := s.grounder.GroundURL(groundCtx, link)
@@ -401,6 +443,9 @@ func (s *Service) Ground(ctx context.Context, id string, f Fix, updatedBy string
if err != nil { if err != nil {
return Dinner{}, fmt.Errorf("%w: dinner %s: record: %w", ErrGroundFailed, id, err) return Dinner{}, fmt.Errorf("%w: dinner %s: record: %w", ErrGroundFailed, id, err)
} }
s.offersMu.Lock()
delete(s.offers, cur.ID)
s.offersMu.Unlock()
return fromRow(row), nil return fromRow(row), nil
} }
@@ -550,11 +595,8 @@ func (f Fix) validate() (link string, manual *ManualRecipe, err error) {
} }
switch { switch {
case f.CandidateID != nil: case f.CandidateID != nil:
u, err := checkRecipeURL(*f.CandidateID) // Ground resolves the candidate to its page.
if err != nil { return "", nil, nil
return "", nil, invalid("candidate_id", "is not a candidate from recipe-candidates")
}
return u.String(), nil, nil
case f.RecipeURL != nil: case f.RecipeURL != nil:
u, err := checkRecipeURL(*f.RecipeURL) u, err := checkRecipeURL(*f.RecipeURL)
if err != nil { if err != nil {
+82 -8
View File
@@ -9,6 +9,7 @@ import (
"io/fs" "io/fs"
"log/slog" "log/slog"
"slices" "slices"
"strings"
"testing" "testing"
"testing/fstest" "testing/fstest"
"time" "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) { func TestCreateGroundingFailureRecordsReason(t *testing.T) {
t.Parallel() t.Parallel()
tests := []struct { tests := []struct {
@@ -642,6 +676,26 @@ func failedDinner(t *testing.T, e *env, name string) string {
return c.ID 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) { func TestCandidates(t *testing.T) {
t.Parallel() t.Parallel()
e := newEnv(t) e := newEnv(t)
@@ -664,14 +718,25 @@ func TestCandidates(t *testing.T) {
} }
first, second := got[0], got[1] first, second := got[0], got[1]
wantPhoto := dinners.PhotoURL(e.dinner(t, c.ID).PhotoID.String) 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 { 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) 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 { second.PhotoURL != nil || second.Minutes == nil || *second.Minutes != 25 {
t.Errorf("second = %+v, want curryB without a photo", second) 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) { if _, err := e.svc.Candidates(t.Context(), "nope"); !errors.Is(err, dinners.ErrNotFound) {
t.Errorf("unknown dinner: err = %v, want ErrNotFound", err) t.Errorf("unknown dinner: err = %v, want ErrNotFound", err)
@@ -702,9 +767,11 @@ func TestFixViaCandidate(t *testing.T) {
t.Parallel() t.Parallel()
e := newEnv(t) e := newEnv(t)
id := failedDinner(t, e, "Mystery curry") 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)) 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -729,12 +796,18 @@ func TestFixReplacesPendingLink(t *testing.T) {
const lost = "https://gone.example/recipes/lamb-tagine" const lost = "https://gone.example/recipes/lamb-tagine"
tests := []struct { tests := []struct {
name string 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 is the recipe link the fixed dinner now has, if any.
linkTaken string linkTaken string
}{ }{
{"candidate", dinners.Fix{CandidateID: ptr(curryB)}, curryB}, {"candidate", func(t *testing.T, e *env, id string) dinners.Fix {
{"manual", dinners.Fix{Manual: &dinners.ManualRecipe{Ingredients: []string{"1 lamb"}, Minutes: ptr(90), Method: ptr("oven")}}, ""}, 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 { for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
@@ -752,7 +825,7 @@ func TestFixReplacesPendingLink(t *testing.T) {
t.Fatal(err) t.Fatal(err)
} }
e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) 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) t.Fatal(err)
} }
row, err := db.New(e.st.Reader).GetDinner(t.Context(), c.ID) row, err := db.New(e.st.Reader).GetDinner(t.Context(), c.ID)
@@ -855,7 +928,8 @@ func TestFixValidation(t *testing.T) {
tests := map[string]dinners.Fix{ tests := map[string]dinners.Fix{
"nothing": {}, "nothing": {},
"two": {CandidateID: ptr(curryA), RecipeURL: ptr(stew)}, "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")}, "http link": {RecipeURL: ptr("http://a.example/x")},
"no ingredients": {Manual: &dinners.ManualRecipe{Minutes: ptr(20), Method: ptr("oven")}}, "no ingredients": {Manual: &dinners.ManualRecipe{Minutes: ptr(20), Method: ptr("oven")}},
"blank ingredient": {Manual: &dinners.ManualRecipe{Ingredients: []string{" "}, Minutes: ptr(20), Method: ptr("oven")}}, "blank ingredient": {Manual: &dinners.ManualRecipe{Ingredients: []string{" "}, Minutes: ptr(20), Method: ptr("oven")}},
+7 -3
View File
@@ -2,10 +2,14 @@
<html lang="en"> <html lang="en">
<head> <head>
<meta charset="UTF-8" /> <meta charset="UTF-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<meta name="theme-color" content="#5a358c" />
<link rel="apple-touch-icon" href="/apple-touch-icon.png" />
<title>MealQuest</title> <title>MealQuest</title>
</head> <script type="module" crossorigin src="/assets/index-CtlEgrTB.js"></script>
<link rel="stylesheet" crossorigin href="/assets/index-CbjluR9l.css">
<link rel="manifest" href="/manifest.webmanifest"><script id="vite-plugin-pwa:register-sw" src="/registerSW.js"></script></head>
<body> <body>
<!-- Placeholder so the Go embed compiles. The build replaces this directory with web/dist. --> <div id="root"></div>
<p>MealQuest web assets have not been built.</p>
</body> </body>
</html> </html>
+99 -4
View File
@@ -68,6 +68,27 @@ func listedDinner(t *testing.T, h http.Handler, id string) map[string]any {
return nil 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) { func TestPostDinnerReturnsBeforeGroundingCompletes(t *testing.T) {
t.Parallel() t.Parallel()
e := newDinnersEnv(t).withRecipes(t) e := newDinnersEnv(t).withRecipes(t)
@@ -138,8 +159,11 @@ func TestGetRecipeCandidates(t *testing.T) {
t.Fatalf("candidates = %v, want 2", body) t.Fatalf("candidates = %v, want 2", body)
} }
first := list[0].(map[string]any) 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{ want := map[string]any{
"candidate_id": curryPage, "candidate_id": first["candidate_id"],
"title": "Chicken Curry", "title": "Chicken Curry",
"site_name": "Curry House", "site_name": "Curry House",
"photo_url": nil, "photo_url": nil,
@@ -186,7 +210,8 @@ func TestPostGroundViaCandidate(t *testing.T) {
id := createDinner(t, e.h, `{"name":"Chicken curry"}`) id := createDinner(t, e.h, `{"name":"Chicken curry"}`)
e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25)) 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 { if rec.Code != http.StatusOK {
t.Fatalf("POST = %d: %s", rec.Code, rec.Body) 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. // The candidate is now stored, so it is offered with its photo.
_, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "") _, body := callDinners(t, e.h, http.MethodGet, dinnersPath+"/"+id+"/recipe-candidates", "")
for _, c := range body["candidates"].([]any) { 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) 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) { func TestPostGroundViaRecipeURL(t *testing.T) {
t.Parallel() t.Parallel()
e := newDinnersEnv(t).withRecipes(t) e := newDinnersEnv(t).withRecipes(t)
@@ -365,7 +459,8 @@ func TestPostGroundRejects(t *testing.T) {
for _, tc := range []struct{ name, body, code string }{ for _, tc := range []struct{ name, body, code string }{
{"empty", `{}`, "invalid_dinner"}, {"empty", `{}`, "invalid_dinner"},
{"two kinds", `{"candidate_id":"https://a.example/x","recipe_url":"https://a.example/x"}`, "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"}, {"http link", `{"recipe_url":"http://a.example/x"}`, "invalid_dinner"},
{"null", `{"recipe_url":null}`, "invalid_request"}, {"null", `{"recipe_url":null}`, "invalid_request"},
{"unknown field", `{"url":"https://a.example/x"}`, "invalid_request"}, {"unknown field", `{"url":"https://a.example/x"}`, "invalid_request"},
+3 -1
View File
@@ -118,9 +118,11 @@ RETURNING *;
-- the grounding_revision it was grounded for; a dinner grounded some other -- 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, -- 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 -- 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 UPDATE dinners
SET recipe_id = sqlc.arg(recipe_id), SET recipe_id = sqlc.arg(recipe_id),
recipe_url = NULL,
photo_id = sqlc.arg(photo_id), photo_id = sqlc.arg(photo_id),
method = COALESCE(method, sqlc.arg(method)), method = COALESCE(method, sqlc.arg(method)),
minutes_total = COALESCE(minutes_total, sqlc.arg(minutes_total)), minutes_total = COALESCE(minutes_total, sqlc.arg(minutes_total)),
+26 -1
View File
@@ -180,10 +180,22 @@ describe('onboarding step 3', () => {
expect(requestPermission).not.toHaveBeenCalled() 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() const server = await renderApp()
fireEvent.click(turnOn()) 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)) await vi.waitFor(() => expect(server.held).toHaveLength(1))
expect(server.held[0]!.body).toEqual({ onboarding_completed: true }) expect(server.held[0]!.body).toEqual({ onboarding_completed: true })
completed(server.held[0]!) completed(server.held[0]!)
@@ -193,6 +205,19 @@ describe('onboarding step 3', () => {
expect(requestPermission).not.toHaveBeenCalled() 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 () => { it('"Turn on notifications" asks the injected enableNotifications first, then finishes', async () => {
let answer: (result: 'enabled') => void = () => {} let answer: (result: 'enabled') => void = () => {}
const enable = vi.fn<EnableNotifications>( const enable = vi.fn<EnableNotifications>(
+45 -14
View File
@@ -14,11 +14,21 @@ export type NotificationsResult = 'enabled' | 'denied' | 'unavailable'
export type EnableNotifications = () => Promise<NotificationsResult> export type EnableNotifications = () => Promise<NotificationsResult>
// TODO(Feature 19): Web Push. Until it is wired, notifications cannot be // 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<NotificationsResult> { export function notificationsUnavailable(): Promise<NotificationsResult> {
return Promise.resolve('unavailable') return Promise.resolve('unavailable')
} }
// notOnReasons says why notifications are not on after asking, for each
// result other than 'enabled'.
const notOnReasons: Record<Exclude<NotificationsResult, 'enabled'>, 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 }[] = [ const topics: { icon: LucideIcon; name: string; meaning: string }[] = [
{ {
icon: CalendarDays, icon: CalendarDays,
@@ -49,16 +59,19 @@ export type NotificationsStepProps = {
} }
// NotificationsStep is onboarding step 3: what MealQuest notifies about, the // NotificationsStep is onboarding step 3: what MealQuest notifies about, the
// iOS install hint, and finishing onboarding. Either button records that // iOS install hint, and finishing onboarding. "Maybe later", and "Turn on
// onboarding is finished and goes Home. When the server refuses because the // notifications" once they are enabled, record that onboarding is finished
// household has fewer than three Regulars, the reason shows with a way back // and go Home. When they are not enabled, the step says so and offers only
// to step 2 to add them. // "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) { export function NotificationsStep({ enableNotifications = notificationsUnavailable }: NotificationsStepProps) {
const navigate = useNavigate() const navigate = useNavigate()
const complete = useCompleteOnboarding() const complete = useCompleteOnboarding()
const [enabling, setEnabling] = useState(false) const [enabling, setEnabling] = useState(false)
const [problem, setProblem] = useState<string | null>(null) const [problem, setProblem] = useState<string | null>(null)
const [fewRegulars, setFewRegulars] = useState(false) const [fewRegulars, setFewRegulars] = useState(false)
const [notOn, setNotOn] = useState<string | null>(null)
const busy = enabling || complete.isPending const busy = enabling || complete.isPending
async function finish() { async function finish() {
@@ -80,14 +93,19 @@ export function NotificationsStep({ enableNotifications = notificationsUnavailab
async function turnOn() { async function turnOn() {
clearProblem() clearProblem()
setEnabling(true) setEnabling(true)
let result: NotificationsResult
try { try {
await enableNotifications() result = await enableNotifications()
} catch (error) { } catch (error) {
setProblem(`Could not turn on notifications: ${(error as Error).message}`) setProblem(`Could not turn on notifications: ${(error as Error).message}`)
return return
} finally { } finally {
setEnabling(false) setEnabling(false)
} }
if (result !== 'enabled') {
setNotOn(notOnReasons[result])
return
}
await finish() await finish()
} }
@@ -101,14 +119,22 @@ export function NotificationsStep({ enableNotifications = notificationsUnavailab
step={3} step={3}
title="Stay in the loop" title="Stay in the loop"
action={ action={
<div className={styles.actions}> notOn ? (
<Button className={styles.action} disabled={busy} onClick={() => void turnOn()}> <div className={styles.actions}>
Turn on notifications <Button className={styles.action} disabled={busy} onClick={later}>
</Button> Finish setting up
<Button variant="outline" className={styles.action} disabled={busy} onClick={later}> </Button>
Maybe later </div>
</Button> ) : (
</div> <div className={styles.actions}>
<Button className={styles.action} disabled={busy} onClick={() => void turnOn()}>
Turn on notifications
</Button>
<Button variant="outline" className={styles.action} disabled={busy} onClick={later}>
Maybe later
</Button>
</div>
)
} }
> >
<p className={styles.helper}>MealQuest can let the household know when there&apos;s something to look at:</p> <p className={styles.helper}>MealQuest can let the household know when there&apos;s something to look at:</p>
@@ -128,6 +154,11 @@ export function NotificationsStep({ enableNotifications = notificationsUnavailab
<div className={styles.hint}> <div className={styles.hint}>
<InstallHint /> <InstallHint />
</div> </div>
{notOn && (
<p role="status" className={styles.notOn}>
{notOn}
</p>
)}
{problem && ( {problem && (
<p role="alert" className={styles.problem}> <p role="alert" className={styles.problem}>
{problem} {problem}
+12
View File
@@ -229,6 +229,18 @@
color: var(--color-accent-700); 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. */ /* Step 3: back to step 2 when the server wants more Regulars. */
.recover { .recover {
margin-top: var(--space-3); margin-top: var(--space-3);