Integration fix 3: keep recipe_url out of dinner responses (Feature 12 review F1) #97

18 changed files with 356 additions and 160 deletions
+6 -6
View File
@@ -179,12 +179,12 @@ automatic** (D-021).
response holds the key. `Service.HarnessConfig` reads the settings for
each skill run, so a saved change applies to the next run.
- Dinners (`dinners.Service`). `POST /api/v1/dinners` adds a Regular from
exactly one of `{name}` or `{recipe_url}` (an https:// link, kept in
`recipe_url` for grounding) with status and origin `regular` and
grounding_status `pending`, answering 201 with the dinner. A link's
dinner is named from its last path segment (separators become spaces,
an extension and all-digit words are dropped: `…/12345/beef-stew/` is
"Beef stew"); a link that names nothing is 400 — add it by name. A name
`{name}` or `{name, recipe_url}` (an https:// link, kept in
`recipe_url` for grounding and never returned in a dinner response, as
it may carry a personal token) with status and origin `regular` and
grounding_status `pending`, answering 201 with the dinner. The name is
always the household's: no part of a link is ever used as a dinner's
name, and a link without a name is 400. A name
is trimmed, at most 120 characters and must keep a word after
`harness.Normalize`, which sets `normalized_name`. A name
`harness.Similar` to an unarchived dinner's, or a link one already has
+19 -58
View File
@@ -16,7 +16,6 @@ import (
"slices"
"strings"
"sync"
"unicode"
"unicode/utf8"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness"
@@ -55,7 +54,9 @@ var listable = []string{StatusRegular, StatusQuestLog}
// ErrNotFound is returned for an unknown dinner id.
var ErrNotFound = errors.New("dinner not found")
// Dinner is a dinner as the API shows it.
// Dinner is a dinner as the API shows it. It has no recipe_url: a pasted
// link may carry a personal token, so it stays in the database for
// grounding and never leaves the server.
type Dinner struct {
ID string `json:"id"`
Name string `json:"name"`
@@ -67,7 +68,6 @@ type Dinner struct {
Cuisine *string `json:"cuisine"`
IsFavorite bool `json:"is_favorite"`
RecipeID *string `json:"recipe_id"`
RecipeURL *string `json:"recipe_url"`
PhotoURL *string `json:"photo_url"`
GroundingStatus string `json:"grounding_status"`
// GroundingError is why grounding failed, one of the Reason codes;
@@ -78,7 +78,10 @@ type Dinner struct {
UpdatedAt string `json:"updated_at"`
}
// NewRegular is a Regular to add: exactly one of Name and RecipeURL.
// NewRegular is a Regular to add: its Name, which the household always
// gives, and optionally the RecipeURL to ground it in. The name is never
// taken from the link: a pasted link may carry a personal token anywhere in
// it, and the name is shown in every dinner response.
type NewRegular struct {
Name *string
RecipeURL *string
@@ -170,10 +173,9 @@ type Created struct {
GroundingJobID *string `json:"grounding_job_id"`
}
// CreateRegular adds a Regular named by r.Name, or by the last path segment
// of r.RecipeURL (whose recipe grounding later replaces), with status and
// origin regular and grounding_status pending, recording updatedBy, and
// enqueues the KindGroundDinner job that grounds it. It returns without
// CreateRegular adds a Regular named r.Name, keeping r.RecipeURL for
// grounding when given, with status and origin regular and grounding_status
// pending, recording updatedBy, and enqueues the KindGroundDinner job that grounds it. It returns without
// waiting for the job. A name repeating an unarchived dinner is a
// *DuplicateError. When the job cannot be enqueued, the stored dinner is
// returned with grounding_status failed (ReasonGroundingError) and no job
@@ -223,34 +225,24 @@ func (s *Service) createRegular(ctx context.Context, r NewRegular, updatedBy str
if updatedBy == "" {
return db.Dinner{}, errors.New("dinners: create: no member to record as updated_by")
}
var name string
if r.Name == nil {
return db.Dinner{}, invalid("name", "give the dinner's name, with or without a recipe_url")
}
name, err := checkName(*r.Name)
if err != nil {
return db.Dinner{}, err
}
var recipeURL sql.NullString
switch {
case r.Name != nil && r.RecipeURL != nil:
return db.Dinner{}, invalid("name", "give a name or a recipe_url, not both")
case r.Name != nil:
n, err := checkName(*r.Name)
if err != nil {
return db.Dinner{}, err
}
name = n
case r.RecipeURL != nil:
if r.RecipeURL != nil {
u, err := checkRecipeURL(*r.RecipeURL)
if err != nil {
return db.Dinner{}, err
}
n, ok := nameFromURL(u)
if !ok {
return db.Dinner{}, invalid("recipe_url", "the link does not name a dinner; add it by name instead")
}
name = n
recipeURL = sql.NullString{String: u.String(), Valid: true}
default:
return db.Dinner{}, invalid("name", "give a name or a recipe_url")
}
var row db.Dinner
err := s.st.WithTx(ctx, func(tx *sql.Tx) error {
err = s.st.WithTx(ctx, func(tx *sql.Tx) error {
q := db.New(tx)
if err := checkUnique(ctx, q, "", name, recipeURL); err != nil {
return err
@@ -495,36 +487,6 @@ func checkRecipeURL(raw string) (*url.URL, error) {
return u, nil
}
// nameFromURL names a dinner from the last path segment of u: separators
// become spaces, a file extension and words of only digits are dropped and
// the first letter is capitalized, so
// https://example.com/recipes/12345/chicken-tikka-masala/ is
// "Chicken tikka masala". ok is false when no usable name is left.
func nameFromURL(u *url.URL) (name string, ok bool) {
segments := strings.FieldsFunc(u.Path, func(r rune) bool { return r == '/' })
if len(segments) == 0 {
return "", false
}
slug := segments[len(segments)-1]
if i := strings.LastIndexByte(slug, '.'); i > 0 {
slug = slug[:i]
}
var words []string
for _, w := range strings.FieldsFunc(slug, func(r rune) bool { return r == '-' || r == '_' || r == '+' || unicode.IsSpace(r) }) {
if strings.IndexFunc(w, func(r rune) bool { return !unicode.IsDigit(r) }) >= 0 {
words = append(words, strings.ToLower(w))
}
}
name = strings.Join(words, " ")
if r, size := utf8.DecodeRuneInString(name); r != utf8.RuneError {
name = string(unicode.ToUpper(r)) + name[size:]
}
if _, err := checkName(name); err != nil {
return "", false
}
return name, true
}
// checkUnique returns a *DuplicateError when name is harness.Similar to an
// unarchived dinner other than self, or recipeURL is one's recipe link: the
// source_url of the recipe it is grounded in, else the recipe_url still
@@ -565,7 +527,6 @@ func fromRow(r db.Dinner) Dinner {
Cuisine: nullString(r.Cuisine),
IsFavorite: r.IsFavorite == 1,
RecipeID: nullString(r.RecipeID),
RecipeURL: nullString(r.RecipeUrl),
GroundingStatus: r.GroundingStatus,
GroundingError: nullString(r.GroundingError),
UpdatedBy: nullString(r.UpdatedBy),
+65 -24
View File
@@ -1,33 +1,74 @@
package dinners
package dinners_test
import (
"net/url"
"encoding/json"
"errors"
"strings"
"testing"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/dinners"
)
func TestNameFromURL(t *testing.T) {
// TestCreateRegularNeverNamesFromLink: a Regular added with a recipe link
// is named only by the household, so no part of the link — host, path,
// query or fragment — is stored as its name or shown in a dinner or a
// duplicate error, even a token of plain letters in the path.
func TestCreateRegularNeverNamesFromLink(t *testing.T) {
t.Parallel()
tests := []struct {
link string
want string // "" means no usable name
}{
{"https://example.com/recipes/12345/chicken-tikka-masala/", "Chicken tikka masala"},
{"https://example.com/recipe/beef_stew.html", "Beef stew"},
{"https://example.com/2024/05/best-ever-chili-123", "Best ever chili"},
{"https://example.com/crème-brûlée", "Crème brûlée"},
{"https://example.com/recipes/chili?utm=1#notes", "Chili"},
{"https://example.com/", ""},
{"https://example.com/recipe/98765", ""},
{"https://example.com/the-best-easy-recipe", ""},
}
for _, tt := range tests {
u, err := url.Parse(tt.link)
if err != nil {
t.Fatal(err)
}
got, ok := nameFromURL(u)
if got != tt.want || ok != (tt.want != "") {
t.Errorf("nameFromURL(%s) = %q, %v; want %q", tt.link, got, ok, tt.want)
e := newUnstartedEnv(t)
const link = "https://c.example/stew-key-abcdefghijklmno?sig=zyxwvut#frag"
leaks := []string{"c.example", "key", "abcdefghijklmno", "sig", "zyxwvut", "frag"}
check := func(what, text string) {
t.Helper()
lower := strings.ToLower(text)
for _, leak := range leaks {
if strings.Contains(lower, leak) {
t.Errorf("%s contains %q: %s", what, leak, text)
}
}
}
_, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{RecipeURL: ptr(link)}, member)
var verr *dinners.ValidationError
if !errors.As(err, &verr) {
t.Fatalf("link without a name: err = %v, want a *ValidationError", err)
}
check("validation error", verr.Error())
c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr(" Sunday stew "), RecipeURL: ptr(link)}, member)
if err != nil {
t.Fatal(err)
}
if c.Name != "Sunday stew" {
t.Errorf("name = %q, want the household's %q", c.Name, "Sunday stew")
}
created, err := json.Marshal(c)
if err != nil {
t.Fatal(err)
}
check("create response", string(created))
row := e.dinner(t, c.ID)
check("stored name", row.Name+" "+row.NormalizedName)
if row.RecipeUrl.String != link {
t.Errorf("stored recipe_url = %q, want %q kept for grounding", row.RecipeUrl.String, link)
}
list, err := e.svc.List(t.Context(), dinners.StatusRegular)
if err != nil {
t.Fatal(err)
}
listed, err := json.Marshal(list)
if err != nil {
t.Fatal(err)
}
check("list response", string(listed))
// The same link under another name is still a duplicate, and its error
// names the existing dinner by the household's name.
_, err = e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Weeknight chili"), RecipeURL: ptr(link)}, member)
var dup *dinners.DuplicateError
if !errors.As(err, &dup) || dup.Existing.ID != c.ID {
t.Fatalf("same link again: err = %v, want a duplicate of %s", err, c.ID)
}
check("duplicate error", dup.Error())
}
+17 -11
View File
@@ -194,7 +194,7 @@ func TestCreateByRecipeURLGroundsThatURL(t *testing.T) {
e := newEnv(t)
e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150))
c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{RecipeURL: ptr(stew)}, member)
c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Beef stew"), RecipeURL: ptr(stew)}, member)
if err != nil {
t.Fatal(err)
}
@@ -223,7 +223,7 @@ func TestCreateGroundingFailureRecordsReason(t *testing.T) {
reason string
}{
{"no candidates", dinners.NewRegular{Name: ptr("Unknown dish")}, nil, dinners.ReasonNoRecipe},
{"unusable link", dinners.NewRegular{RecipeURL: ptr("https://gone.example/recipes/lost-soup")}, nil, dinners.ReasonRecipeUnusable},
{"unusable link", dinners.NewRegular{Name: ptr("Lost soup"), RecipeURL: ptr("https://gone.example/recipes/lost-soup")}, nil, dinners.ReasonRecipeUnusable},
{"search fails", dinners.NewRegular{Name: ptr("Chicken curry")}, func(e *env) { e.search.Err = recipes.ErrSearchRateLimited }, dinners.ReasonSearchFailed},
{"model fails", dinners.NewRegular{Name: ptr("Chicken curry")}, func(e *env) {
// One provider failure per candidate tried.
@@ -375,7 +375,7 @@ func TestRenameLinkWhilePendingGroundsNewName(t *testing.T) {
e.search.Results["Beef stew"] = []recipes.Candidate{{URL: stew}}
e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150))
c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{RecipeURL: ptr(curryA)}, member)
c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Chicken curry"), RecipeURL: ptr(curryA)}, member)
if err != nil {
t.Fatal(err)
}
@@ -383,8 +383,11 @@ func TestRenameLinkWhilePendingGroundsNewName(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if d.GroundingStatus != dinners.GroundingPending || d.RecipeURL != nil {
t.Fatalf("renamed = %+v, want still pending with no recipe_url", d)
if d.GroundingStatus != dinners.GroundingPending {
t.Fatalf("renamed = %+v, want still pending", d)
}
if row, err := db.New(e.st.Reader).GetDinner(t.Context(), c.ID); err != nil || row.RecipeUrl.Valid {
t.Fatalf("renamed row = %+v (err %v), want no recipe_url", row, err)
}
e.start(t)
@@ -737,7 +740,7 @@ func TestFixReplacesPendingLink(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
e := newEnv(t)
c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{RecipeURL: ptr(lost)}, member)
c, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Lamb roast"), RecipeURL: ptr(lost)}, member)
if err != nil {
t.Fatal(err)
}
@@ -749,19 +752,22 @@ func TestFixReplacesPendingLink(t *testing.T) {
t.Fatal(err)
}
e.model.QueueGenerate(recipestest.NormalizedReply("stovetop", 25))
d, err := e.svc.Ground(t.Context(), c.ID, tt.fix, member)
if _, err := e.svc.Ground(t.Context(), c.ID, tt.fix, member); err != nil {
t.Fatal(err)
}
row, err := db.New(e.st.Reader).GetDinner(t.Context(), c.ID)
if err != nil {
t.Fatal(err)
}
if d.RecipeURL != nil {
t.Errorf("recipe_url = %q after the fix, want the unusable link dropped", *d.RecipeURL)
if row.RecipeUrl.Valid {
t.Errorf("recipe_url = %q after the fix, want the unusable link dropped", row.RecipeUrl.String)
}
if _, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{RecipeURL: ptr(lost)}, member); err != nil {
if _, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Lentil soup"), RecipeURL: ptr(lost)}, member); err != nil {
t.Errorf("adding the dropped link again: %v, want added", err)
}
if tt.linkTaken != "" {
_, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{RecipeURL: ptr(tt.linkTaken)}, member)
_, err := e.svc.CreateRegular(t.Context(), dinners.NewRegular{Name: ptr("Curry night"), RecipeURL: ptr(tt.linkTaken)}, member)
var dup *dinners.DuplicateError
if !errors.As(err, &dup) || dup.Existing.ID != c.ID {
t.Errorf("adding the fixed dinner's recipe link: err = %v, want a duplicate of %s", err, c.ID)
+2 -1
View File
@@ -19,7 +19,8 @@ const maxDinnerBody = 16 << 10
// manual recipe of dinners.MaxManualLines lines of each kind.
const maxGroundBody = 512 << 10
// dinnerCreate is the POST /dinners body: a name or a recipe_url.
// dinnerCreate is the POST /dinners body: a name, with or without a
// recipe_url.
type dinnerCreate struct {
Name *string `json:"name"`
RecipeURL *string `json:"recipe_url"`
+89 -14
View File
@@ -159,7 +159,6 @@ func TestCreateDinnerByName(t *testing.T) {
"origin": "regular",
"grounding_status": "pending",
"is_favorite": false,
"recipe_url": nil,
"photo_url": nil,
"method": nil,
"minutes_total": nil,
@@ -170,6 +169,9 @@ func TestCreateDinnerByName(t *testing.T) {
t.Errorf("%s = %#v (present %v), want %#v", k, got, ok, v)
}
}
if got, ok := body["recipe_url"]; ok {
t.Errorf("recipe_url = %#v, want no recipe_url in a dinner response", got)
}
var normalized, updatedBy string
if err := st.Reader.QueryRow(`SELECT normalized_name, updated_by FROM dinners WHERE id = ?`, body["id"]).Scan(&normalized, &updatedBy); err != nil {
t.Fatal(err)
@@ -184,13 +186,12 @@ func TestCreateDinnerByURL(t *testing.T) {
router, _ := newDinnersRouter(t)
link := "https://www.example.com/recipes/12345/beef-and-broccoli-stir-fry/"
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"recipe_url":"`+link+`"}`)
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Stir fry night","recipe_url":"`+link+`"}`)
if rec.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body)
}
for k, v := range map[string]any{
"name": "Beef and broccoli stir fry",
"recipe_url": link,
"name": "Stir fry night",
"status": "regular",
"origin": "regular",
"grounding_status": "pending",
@@ -200,24 +201,27 @@ func TestCreateDinnerByURL(t *testing.T) {
t.Errorf("%s = %#v, want %#v", k, body[k], v)
}
}
// The link stays on the server for grounding; responses never carry it.
if got, ok := body["recipe_url"]; ok {
t.Errorf("recipe_url = %#v, want no recipe_url in a dinner response", got)
}
// The same link again is a duplicate even though its name would differ.
rec, body = callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Something else entirely"}`)
if rec.Code != http.StatusCreated {
t.Fatalf("unrelated name = %d: %s", rec.Code, rec.Body)
}
rec, body = callDinners(t, router, http.MethodPost, dinnersPath, `{"recipe_url":"`+link+`"}`)
rec, body = callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Beef and broccoli","recipe_url":"`+link+`"}`)
if rec.Code != http.StatusConflict || errorCode(body) != "duplicate_dinner" {
t.Errorf("same link again = %d %v, want 409 duplicate_dinner", rec.Code, body)
}
for _, tc := range []struct{ name, body string }{
{"plain http", `{"recipe_url":"http://example.com/recipes/chili"}`},
{"not a URL", `{"recipe_url":"chili"}`},
{"credentials", `{"recipe_url":"https://me:pw@example.com/recipes/chili"}`},
{"no name in path", `{"recipe_url":"https://example.com/"}`},
{"numeric slug", `{"recipe_url":"https://example.com/recipe/98765"}`},
{"both fields", `{"name":"Chili","recipe_url":"https://example.com/recipes/chili"}`},
{"plain http", `{"name":"Chili","recipe_url":"http://example.com/recipes/chili"}`},
{"not a URL", `{"name":"Chili","recipe_url":"chili"}`},
{"credentials", `{"name":"Chili","recipe_url":"https://me:pw@example.com/recipes/chili"}`},
{"link without a name", `{"recipe_url":"https://example.com/recipes/chili"}`},
{"filler name with a link", `{"name":"The Best Easy Recipe","recipe_url":"https://example.com/recipes/chili"}`},
{"neither field", `{}`},
} {
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, tc.body)
@@ -227,6 +231,77 @@ func TestCreateDinnerByURL(t *testing.T) {
}
}
// TestDinnerResponsesHideRecipeURL: a pasted link may carry a personal
// token, so neither the create response nor a later list read shows any
// part of it, while the stored link still awaits grounding.
func TestDinnerResponsesHideRecipeURL(t *testing.T) {
t.Parallel()
router, st := newDinnersRouter(t)
const link = "https://c.example/recipes/stew?access_token=personal-token-abc123"
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Stew","recipe_url":"`+link+`"}`)
if rec.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body)
}
created := rec.Body.String()
rec, _ = callDinners(t, router, http.MethodGet, dinnersPath+"?status=regular", "")
if rec.Code != http.StatusOK {
t.Fatalf("GET status=regular = %d: %s", rec.Code, rec.Body)
}
for _, resp := range []struct{ name, body string }{{"create", created}, {"list", rec.Body.String()}} {
for _, secret := range []string{link, "c.example", "access_token", "personal-token-abc123", "recipe_url"} {
if strings.Contains(resp.body, secret) {
t.Errorf("%s response contains %q: %s", resp.name, secret, resp.body)
}
}
}
var stored string
if err := st.Reader.QueryRow(`SELECT recipe_url FROM dinners WHERE id = ?`, body["id"]).Scan(&stored); err != nil {
t.Fatal(err)
}
if stored != link {
t.Errorf("stored recipe_url = %q, want %q kept for grounding", stored, link)
}
}
// TestDinnerResponsesHideTokenInLinkPath: a personal token can sit
// anywhere in a link's path, even as plain letters, so a dinner added from
// a link is named only by the household and neither the create response, a
// later list read nor the duplicate error for the same link shows any of
// the link.
func TestDinnerResponsesHideTokenInLinkPath(t *testing.T) {
t.Parallel()
router, _ := newDinnersRouter(t)
const link = "https://c.example/recipes/stew-key-abcdefghijklmno"
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Sunday stew","recipe_url":"`+link+`"}`)
if rec.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body)
}
if body["name"] != "Sunday stew" {
t.Errorf("name = %v, want Sunday stew", body["name"])
}
created := rec.Body.String()
rec, _ = callDinners(t, router, http.MethodGet, dinnersPath+"?status=regular", "")
if rec.Code != http.StatusOK {
t.Fatalf("GET status=regular = %d: %s", rec.Code, rec.Body)
}
listed := rec.Body.String()
rec, _ = callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Beef stew","recipe_url":"`+link+`"}`)
if rec.Code != http.StatusConflict {
t.Fatalf("same link again = %d, want 409: %s", rec.Code, rec.Body)
}
for _, resp := range []struct{ name, body string }{{"create", created}, {"list", listed}, {"duplicate", rec.Body.String()}} {
lower := strings.ToLower(resp.body)
for _, secret := range []string{"c.example", "recipes", "key", "abcdefghijklmno"} {
if strings.Contains(lower, secret) {
t.Errorf("%s response contains %q: %s", resp.name, secret, resp.body)
}
}
}
}
func TestCreateDinnerDuplicate(t *testing.T) {
t.Parallel()
router, st := newDinnersRouter(t)
@@ -242,10 +317,10 @@ func TestCreateDinnerDuplicate(t *testing.T) {
t.Errorf("%q: %d %v, want 409 duplicate_dinner", name, rec.Code, body)
}
}
// A slug naming the same dish is a duplicate too.
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"recipe_url":"https://example.com/easy-chicken-tikka-masala.html"}`)
// A name repeating the dish is a duplicate with a link too.
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Chicken tikka masala","recipe_url":"https://example.com/other-curry.html"}`)
if rec.Code != http.StatusConflict {
t.Errorf("URL slug duplicate = %d %v, want 409", rec.Code, body)
t.Errorf("named duplicate with a link = %d %v, want 409", rec.Code, body)
}
var n int
+8 -7
View File
@@ -33,8 +33,10 @@ export type Regular = {
pending?: true
}
// NewRegular is a POST /api/v1/dinners body: a name or a recipe link.
export type NewRegular = { name: string } | { recipe_url: string }
// NewRegular is a POST /api/v1/dinners body: the household's name for the
// dinner, with the recipe link to ground it in when one was pasted. A
// dinner is never named from its link, which may carry private details.
export type NewRegular = { name: string; recipe_url?: string }
// RegularPatch is a PATCH /api/v1/dinners/{id} body; every field is optional.
export type RegularPatch = Partial<{ name: string; method: Method; minutes_total: number; difficulty: Difficulty }>
@@ -195,10 +197,9 @@ function settle(queryClient: QueryClient) {
let pendingIds = 0
// useAddRegular adds a Regular by name or recipe link (POST
// /api/v1/dinners). It shows in the list at once — a link under the link
// itself until the server names it — and is taken out again if the server
// refuses it.
// useAddRegular adds a Regular by name, with or without a recipe link (POST
// /api/v1/dinners). It shows in the list at once under its name and is
// taken out again if the server refuses it.
export function useAddRegular() {
const { apiFetch } = useApi()
const queryClient = useQueryClient()
@@ -210,7 +211,7 @@ export function useAddRegular() {
const id = `pending-${++pendingIds}`
const optimistic: CachedRegular = {
id,
name: 'name' in add ? add.name : add.recipe_url,
name: add.name,
method: null,
minutes_total: null,
difficulty: null,
@@ -43,6 +43,13 @@
font-weight: 700;
}
/* The add form: the add row, then the name field a pasted link needs. */
.addForm {
display: flex;
flex-direction: column;
gap: 8px;
}
/* The add field: the input fills the row beside the grape + button. */
.add {
display: flex;
+20 -8
View File
@@ -192,21 +192,33 @@ describe('RegularsSettingsPage', () => {
expect(screen.getByRole('heading', { level: 2, name: 'On rotation · 2' })).toBeInTheDocument()
})
it('adds a Regular from a pasted recipe link', async () => {
const link = 'https://example.com/recipes/12345/beef-stew/'
it('adds a Regular from a pasted recipe link under the name the household gives it', async () => {
const link = 'https://example.com/recipes/stew-key-abcdefghijklmno'
const server = stubServer([], {
'POST /api/v1/dinners': () => {
const created = dinnerBody('d9', 'Beef stew', { grounding_status: 'pending', recipe_url: link })
'POST /api/v1/dinners': (body) => {
const created = dinnerBody('d9', (body as { name: string }).name, { grounding_status: 'pending' })
server.regulars = [created]
return jsonResponse(201, { ...created, grounding_job_id: 'job-1' })
},
})
await renderPage()
add(link)
expect(await screen.findByText('Beef stew')).toBeInTheDocument()
expect(sent(server.fetchMock)).toEqual([{ request: 'POST /api/v1/dinners', body: { recipe_url: link } }])
expect(within(row('Beef stew')).getByText('Finding recipe…')).toBeInTheDocument()
fireEvent.change(addField(), { target: { value: ` ${link} ` } })
// A link alone cannot be added: it never names a dinner.
expect(screen.getByRole('button', { name: 'Add Regular' })).toBeDisabled()
const name = screen.getByRole('textbox', { name: 'Dinner name' })
expect(name).toHaveAccessibleDescription(/never names a dinner/)
fireEvent.change(name, { target: { value: ' Sunday stew ' } })
fireEvent.click(screen.getByRole('button', { name: 'Add Regular' }))
expect(addField()).toHaveValue('')
expect(screen.queryByRole('textbox', { name: 'Dinner name' })).not.toBeInTheDocument()
expect(await screen.findByText('Sunday stew')).toBeInTheDocument()
expect(sent(server.fetchMock)).toEqual([
{ request: 'POST /api/v1/dinners', body: { name: 'Sunday stew', recipe_url: link } },
])
expect(within(row('Sunday stew')).getByText('Finding recipe…')).toBeInTheDocument()
expect(document.body).not.toHaveTextContent(/example\.com|abcdefghijklmno|key/i)
})
it('takes back an add the server refuses and says why', async () => {
+44 -19
View File
@@ -49,12 +49,14 @@ export const methodLabels: Record<Method, string> = {
const difficultyLabels: Record<Difficulty, string> = { easy: 'Easy', medium: 'Medium', hard: 'Hard' }
// A pasted link is added as a recipe link; anything else is a dinner name.
// A pasted link is added as a recipe link under a name the household gives;
// anything else is a dinner name.
const LINK = /^https?:\/\//i
// Settings · Regular dinners: the household's Regulars, each with its photo,
// method and minutes, favorite heart and whether its recipe was found. The
// field at the top adds one by name or pasted recipe link; tapping one
// field at the top adds one by name, or by pasted recipe link with a name
// the household gives it; tapping one
// edits or removes it, or fixes its recipe when none was found.
export function RegularsSettingsPage() {
const navigate = useNavigate()
@@ -85,19 +87,24 @@ function RegularsManager() {
const favorite = useToggleFavorite()
const { refusals, track } = useRefusals()
const [draft, setDraft] = useState('')
const [linkName, setLinkName] = useState('')
const [open, setOpen] = useState<Open>(null)
const headingId = useId()
const linkHintId = useId()
const isLink = LINK.test(draft.trim())
const ready = draft.trim() !== '' && (!isLink || linkName.trim() !== '')
const list = regulars.data ?? []
// A Regular removed meanwhile closes its sheet.
const opened = open && list.find((regular) => regular.id === open.id && !regular.pending)
function onSubmit(event: FormEvent<HTMLFormElement>) {
event.preventDefault()
if (!ready) return
const text = draft.trim()
if (text === '') return
const body: NewRegular = LINK.test(text) ? { recipe_url: text } : { name: text }
track(`add:${text.toLowerCase()}`, `add ${text}`, add.mutateAsync(body))
const body: NewRegular = isLink ? { name: linkName.trim(), recipe_url: text } : { name: text }
track(`add:${body.name.toLowerCase()}`, `add ${body.name}`, add.mutateAsync(body))
setDraft('')
setLinkName('')
}
function onRemove(regular: Regular) {
@@ -111,20 +118,38 @@ function RegularsManager() {
return (
<>
<form className={styles.add} onSubmit={onSubmit}>
<TextInput
label="Add a Regular"
labelHidden
className={styles.addInput}
placeholder="Dinner name or recipe link"
autoComplete="off"
enterKeyHint="done"
value={draft}
onChange={(event) => setDraft(event.target.value)}
/>
<IconButton type="submit" label="Add Regular" tone="accent" disabled={draft.trim() === ''}>
<Icon icon={Plus} size="nav" />
</IconButton>
<form className={styles.addForm} onSubmit={onSubmit}>
<div className={styles.add}>
<TextInput
label="Add a Regular"
labelHidden
className={styles.addInput}
placeholder="Dinner name or recipe link"
autoComplete="off"
enterKeyHint="done"
value={draft}
onChange={(event) => setDraft(event.target.value)}
/>
<IconButton type="submit" label="Add Regular" tone="accent" disabled={!ready}>
<Icon icon={Plus} size="nav" />
</IconButton>
</div>
{isLink && (
<>
<TextInput
label="Dinner name"
placeholder="What do you call this dinner?"
autoComplete="off"
enterKeyHint="done"
aria-describedby={linkHintId}
value={linkName}
onChange={(event) => setLinkName(event.target.value)}
/>
<p id={linkHintId} className={styles.hint}>
Name it yourself: a link can carry private details, so it never names a dinner.
</p>
</>
)}
</form>
{refusals.map((refusal) => (
<p key={refusal.key} role="alert" className={styles.problem}>
+47
View File
@@ -0,0 +1,47 @@
// Runs the service worker's activate handler against mocked Cache Storage,
// with the Workbox precache and router stubbed out. tsconfig.sw.json checks
// this file with the worker's types, like src/sw.ts itself.
import { afterEach, describe, expect, it, vi } from 'vitest'
vi.mock('workbox-precaching', () => ({
PrecacheFallbackPlugin: class {},
matchPrecache: vi.fn(),
precacheAndRoute: vi.fn(),
}))
vi.mock('workbox-routing', () => ({ registerRoute: vi.fn() }))
describe('service worker activation', () => {
afterEach(() => {
vi.unstubAllGlobals()
})
it('deletes the v1 API cache and keeps the precache, photo and font caches', async () => {
// A household on the v1 worker: its API cache held dinners with the
// pasted recipe_url; its photos and fonts are not in the precache.
const names = [
'mealquest-precache-v1',
'mealquest-api-v1',
'mealquest-media-v1',
'mealquest-fonts-v1',
'other-app-cache',
]
const cacheStorage = {
keys: vi.fn(async () => names),
delete: vi.fn(async () => true),
}
vi.stubGlobal('caches', cacheStorage)
vi.stubGlobal('__WB_MANIFEST', [])
await import('./sw')
const pending: Promise<unknown>[] = []
const activate = Object.assign(new Event('activate'), {
waitUntil: (promise: Promise<unknown>) => pending.push(promise),
})
self.dispatchEvent(activate)
await Promise.all(pending)
expect(pending).toHaveLength(1)
expect(cacheStorage.delete).toHaveBeenCalledWith('mealquest-api-v1')
expect(cacheStorage.delete).toHaveBeenCalledTimes(1)
})
})
+2 -2
View File
@@ -18,7 +18,7 @@ import { CacheFirst, NetworkOnly, type Strategy } from 'workbox-strategies'
import { createApiStrategy } from './sw/apiStrategy'
import {
cacheNames,
cacheVersion,
cacheVersions,
routeFor,
staleCaches,
type Route,
@@ -29,7 +29,7 @@ declare const self: ServiceWorkerGlobalScope & {
}
// The precache becomes mealquest-precache-v1.
setCacheNameDetails({ prefix: 'mealquest', precache: 'precache', suffix: cacheVersion })
setCacheNameDetails({ prefix: 'mealquest', precache: 'precache', suffix: cacheVersions.precache })
const day = 24 * 60 * 60
+13 -1
View File
@@ -1,5 +1,5 @@
import { describe, expect, it } from 'vitest'
import { cacheFallbackHeader, cacheNames, markCacheFallback, routeFor, staleCaches, type Route } from './routes'
import { cacheFallbackHeader, cacheNames, cacheVersions, markCacheFallback, routeFor, staleCaches, type Route } from './routes'
const origin = 'https://mealquest.test'
@@ -146,6 +146,18 @@ describe('cache names', () => {
it('all start with mealquest-', () => {
for (const name of Object.values(cacheNames)) expect(name).toMatch(/^mealquest-/)
})
it('retire only the v1 API cache', () => {
// v1 API responses held dinners with the pasted recipe_url, which can
// carry a personal token; offline, the worker would still serve them.
// The precache, photos and fonts keep their v1 caches.
expect(cacheNames).toEqual({
api: 'mealquest-api-v2',
media: 'mealquest-media-v1',
fonts: 'mealquest-fonts-v1',
})
expect(cacheVersions.precache).toBe('v1')
})
})
describe('staleCaches', () => {
+14 -5
View File
@@ -24,14 +24,23 @@ export interface RouteRequest {
mode: RequestMode
}
// Bumping the version renames every cache; activate then deletes the old ones.
export const cacheVersion = 'v1'
// Each cache carries its own version. Bumping one renames only that cache;
// activate then deletes its old name and keeps every other cache, so saved
// photos and fonts stay available offline.
export const cacheVersions = {
precache: 'v1',
// v2 retires the v1 API cache, whose dinner responses still carried the
// pasted recipe_url.
api: 'v2',
media: 'v1',
fonts: 'v1',
} as const
export const cachePrefix = 'mealquest-'
export const cacheNames = {
api: `${cachePrefix}api-${cacheVersion}`,
media: `${cachePrefix}media-${cacheVersion}`,
fonts: `${cachePrefix}fonts-${cacheVersion}`,
api: `${cachePrefix}api-${cacheVersions.api}`,
media: `${cachePrefix}media-${cacheVersions.media}`,
fonts: `${cachePrefix}fonts-${cacheVersions.fonts}`,
} as const
export const networkTimeoutSeconds = 3
+1
View File
@@ -2,6 +2,7 @@
// source: they pin the parts routes.ts cannot express — the API cache's
// strategy, the font strategy's expiration, the font route's place ahead of
// the precache route and network-only requests left to the browser.
// src/sw.test.ts runs the worker's activation against mocked Cache Storage.
import { describe, expect, it } from 'vitest'
import source from '../sw.ts?raw'
-2
View File
@@ -10,7 +10,6 @@ export type DinnerBody = {
cuisine: string | null
is_favorite: boolean
recipe_id: string | null
recipe_url: string | null
photo_url: string | null
grounding_status: string
grounding_error: string | null
@@ -33,7 +32,6 @@ export function dinnerBody(id: string, name: string, overrides: Partial<DinnerBo
cuisine: null,
is_favorite: false,
recipe_id: null,
recipe_url: null,
photo_url: null,
grounding_status: 'grounded',
grounding_error: null,
+1 -1
View File
@@ -19,5 +19,5 @@
"noEmit": true
},
"include": ["src", "vite.config.ts", "manifest.json"],
"exclude": ["src/sw.ts"]
"exclude": ["src/sw.ts", "src/sw.test.ts"]
}
+1 -1
View File
@@ -4,6 +4,6 @@
"lib": ["ES2022", "WebWorker"],
"types": []
},
"include": ["src/sw.ts"],
"include": ["src/sw.ts", "src/sw.test.ts"],
"exclude": []
}