Release corrections: candidate URL exposure, onboarding notification honesty, pasted link cleanup #101

Merged
approver_agent merged 5 commits from task/release-corrections_6819ce33-3715-4bd4-9208-a54a8b5cee02 into dev 2026-10-06 23:12:27 -04:00
17 changed files with 712 additions and 68 deletions
+15 -10
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,10 +234,12 @@ 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
setting `grounded`) or `{manual: {ingredients[], steps[], minutes, 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 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
— no model runs — as a `recipes` row whose source_url is — 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, 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
+18 -2
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,18 @@ 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 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 // 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: 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]*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...) kind, err := jobs.Register(runner, KindGroundDinner, s.runGroundJob, Reasons...)
if err != nil { if err != nil {
return nil, err 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 // Archive sets dinner id's status to archived, recording updatedBy. The row
// is kept, so its name still counts for Side quest dedupe. A // 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) { func (s *Service) Archive(ctx context.Context, id, updatedBy string) (Dinner, error) {
if updatedBy == "" { if updatedBy == "" {
return Dinner{}, errors.New("dinners: archive: no member to record as updated_by") 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) return Dinner{}, fmt.Errorf("dinners: archive %s: %w", id, err)
} }
s.stopGroundings(row.ID, math.MaxInt64) s.stopGroundings(row.ID, math.MaxInt64)
s.clearOffers(row.ID)
return fromRow(row), nil return fromRow(row), nil
} }
+4 -2
View File
@@ -16,8 +16,10 @@ import (
func TestCreateRegularNeverNamesFromLink(t *testing.T) { func TestCreateRegularNeverNamesFromLink(t *testing.T) {
t.Parallel() t.Parallel()
e := newUnstartedEnv(t) e := newUnstartedEnv(t)
const link = "https://c.example/stew-key-abcdefghijklmno?sig=zyxwvut#frag" const link = "https://c.example/stew-key-abcdefghijklmno?sig=zyxwvut#lookup"
leaks := []string{"c.example", "key", "abcdefghijklmno", "sig", "zyxwvut", "frag"} // 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) { check := func(what, text string) {
t.Helper() t.Helper()
lower := strings.ToLower(text) lower := strings.ToLower(text)
+33
View File
@@ -14,6 +14,39 @@ func SetGroundTimeout(t *testing.T, d time.Duration) {
t.Cleanup(func() { groundTimeout = old }) 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 // RunGroundJob runs the KindGroundDinner handler for p in the caller's
// goroutine, as the job runner would, and returns the grounding status it // goroutine, as the job runner would, and returns the grounding status it
// reports. // reports.
+148 -12
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"`
@@ -291,10 +296,39 @@ type Candidate struct {
Minutes *int `json:"minutes"` 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 // 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. 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) { 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) row, err := s.get(ctx, id)
if err != nil { if err != nil {
return nil, err return nil, err
@@ -304,16 +338,113 @@ 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
} }
} }
if row.Status != StatusArchived {
keep(urls)
}
return out, nil 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 // 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 +498,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 +537,7 @@ 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.clearOffers(cur.ID)
return fromRow(row), nil 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 // 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 // steps, and records it on the dinner, all in one transaction; the
// or archived dinner is ErrNotFound or ErrArchived; any other failure // dinner's candidates then no longer ground it. A missing or archived
// wraps ErrGroundFailed. // 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) { func (s *Service) groundManual(ctx context.Context, id string, m ManualRecipe, updatedBy string) (Dinner, error) {
var row db.Dinner var row db.Dinner
err := s.st.WithTx(ctx, func(tx *sql.Tx) error { 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 { if err != nil {
return Dinner{}, fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err) return Dinner{}, fmt.Errorf("%w: dinner %s: %w", ErrGroundFailed, id, err)
} }
s.clearOffers(row.ID)
return fromRow(row), nil return fromRow(row), nil
} }
@@ -550,11 +689,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 {
+283 -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)
} }
@@ -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 // TestFixReplacesPendingLink: fixing a Regular added from a link that held
// no recipe drops that link, so it no longer blocks adding the link again, // no recipe drops that link, so it no longer blocks adding the link again,
// while the link of the recipe chosen does. // 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" 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 +1026,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 +1129,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")}},
+3 -1
View File
@@ -292,9 +292,11 @@ func TestDinnerResponsesHideTokenInLinkPath(t *testing.T) {
if rec.Code != http.StatusConflict { if rec.Code != http.StatusConflict {
t.Fatalf("same link again = %d, want 409: %s", rec.Code, rec.Body) 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()}} { for _, resp := range []struct{ name, body string }{{"create", created}, {"list", listed}, {"duplicate", rec.Body.String()}} {
lower := strings.ToLower(resp.body) 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) { if strings.Contains(lower, secret) {
t.Errorf("%s response contains %q: %s", resp.name, secret, resp.body) t.Errorf("%s response contains %q: %s", resp.name, secret, resp.body)
} }
+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);
+9 -4
View File
@@ -15,12 +15,16 @@ describe('service worker activation', () => {
vi.unstubAllGlobals() vi.unstubAllGlobals()
}) })
it('deletes the v1 API cache and keeps the precache, photo and font caches', async () => { it('deletes the v1 and v2 API caches and keeps the current caches', async () => {
// A household on the v1 worker: its API cache held dinners with the // A household that ran the v1 and v2 workers: the v1 API cache held
// pasted recipe_url; its photos and fonts are not in the precache. // 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 = [ const names = [
'mealquest-precache-v1', 'mealquest-precache-v1',
'mealquest-api-v1', 'mealquest-api-v1',
'mealquest-api-v2',
'mealquest-api-v3',
'mealquest-media-v1', 'mealquest-media-v1',
'mealquest-fonts-v1', 'mealquest-fonts-v1',
'other-app-cache', 'other-app-cache',
@@ -42,6 +46,7 @@ describe('service worker activation', () => {
expect(pending).toHaveLength(1) expect(pending).toHaveLength(1)
expect(cacheStorage.delete).toHaveBeenCalledWith('mealquest-api-v1') expect(cacheStorage.delete).toHaveBeenCalledWith('mealquest-api-v1')
expect(cacheStorage.delete).toHaveBeenCalledTimes(1) expect(cacheStorage.delete).toHaveBeenCalledWith('mealquest-api-v2')
expect(cacheStorage.delete).toHaveBeenCalledTimes(2)
}) })
}) })
+5 -4
View File
@@ -147,12 +147,13 @@ describe('cache names', () => {
for (const name of Object.values(cacheNames)) expect(name).toMatch(/^mealquest-/) for (const name of Object.values(cacheNames)) expect(name).toMatch(/^mealquest-/)
}) })
it('retire only the v1 API cache', () => { it('retire only the v1 and v2 API caches', () => {
// v1 API responses held dinners with the pasted recipe_url, which can // v1 API responses held dinners with the pasted recipe_url and v2 ones
// carry a personal token; offline, the worker would still serve them. // 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. // The precache, photos and fonts keep their v1 caches.
expect(cacheNames).toEqual({ expect(cacheNames).toEqual({
api: 'mealquest-api-v2', api: 'mealquest-api-v3',
media: 'mealquest-media-v1', media: 'mealquest-media-v1',
fonts: 'mealquest-fonts-v1', fonts: 'mealquest-fonts-v1',
}) })
+4 -3
View File
@@ -29,9 +29,10 @@ export interface RouteRequest {
// photos and fonts stay available offline. // photos and fonts stay available offline.
export const cacheVersions = { export const cacheVersions = {
precache: 'v1', precache: 'v1',
// v2 retires the v1 API cache, whose dinner responses still carried the // v2 retired the v1 API cache, whose dinner responses still carried the
// pasted recipe_url. // pasted recipe_url; v3 retires the v2 API cache, whose recipe-candidate
api: 'v2', // responses used the full URL as candidate_id.
api: 'v3',
media: 'v1', media: 'v1',
fonts: 'v1', fonts: 'v1',
} as const } as const