Require the household's name for a Regular added from a link (PR #97 review R4-F1)
CI / ci (push) Successful in 2m38s
CI / ci (pull_request) Successful in 2m25s

A pasted link may carry a personal token anywhere in its path, so no part
of it can safely name a dinner: the slug heuristic always let some plain
words through. POST /api/v1/dinners now takes {name} or {name, recipe_url};
a link without a name is 400 invalid_dinner, and nameFromURL is gone. The
link is still kept only for grounding and for the recipe-link duplicate
rule. Name-created Regulars, grounding, and the rename rules are unchanged.

On the Regulars screen, pasting a link shows a "Dinner name" field, and
Add stays disabled until it is filled. The optimistic row shows that name,
never the link.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-06 21:16:16 -04:00
co-authored by Claude Opus 5.5
parent 9574d1920e
commit 51bae14b7b
10 changed files with 200 additions and 182 deletions
+4 -5
View File
@@ -179,13 +179,12 @@ automatic** (D-021).
response holds the key. `Service.HarnessConfig` reads the settings for response holds the key. `Service.HarnessConfig` reads the settings for
each skill run, so a saved change applies to the next run. each skill run, so a saved change applies to the next run.
- Dinners (`dinners.Service`). `POST /api/v1/dinners` adds a Regular from - Dinners (`dinners.Service`). `POST /api/v1/dinners` adds a Regular from
exactly one of `{name}` or `{recipe_url}` (an https:// link, kept in `{name}` or `{name, recipe_url}` (an https:// link, kept in
`recipe_url` for grounding and never returned in a dinner response, as `recipe_url` for grounding and never returned in a dinner response, as
it may carry a personal token) with status and origin `regular` and it may carry a personal token) with status and origin `regular` and
grounding_status `pending`, answering 201 with the dinner. A link's grounding_status `pending`, answering 201 with the dinner. The name is
dinner is named from its last path segment (separators become spaces, always the household's: no part of a link is ever used as a dinner's
an extension and all-digit words are dropped: `…/12345/beef-stew/` is name, and a link without a name is 400. A name
"Beef stew"); a link that names nothing is 400 — add it by name. A name
is trimmed, at most 120 characters and must keep a word after is trimmed, at most 120 characters and must keep a word after
`harness.Normalize`, which sets `normalized_name`. A name `harness.Normalize`, which sets `normalized_name`. A name
`harness.Similar` to an unarchived dinner's, or a link one already has `harness.Similar` to an unarchived dinner's, or a link one already has
+14 -77
View File
@@ -16,7 +16,6 @@ import (
"slices" "slices"
"strings" "strings"
"sync" "sync"
"unicode"
"unicode/utf8" "unicode/utf8"
"code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness" "code.cubecraftlabs.com/Maniacs_Mansion/Meal-Quest/internal/ai/harness"
@@ -79,7 +78,10 @@ type Dinner struct {
UpdatedAt string `json:"updated_at"` 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 { type NewRegular struct {
Name *string Name *string
RecipeURL *string RecipeURL *string
@@ -171,10 +173,9 @@ type Created struct {
GroundingJobID *string `json:"grounding_job_id"` GroundingJobID *string `json:"grounding_job_id"`
} }
// CreateRegular adds a Regular named by r.Name, or by the last path segment // CreateRegular adds a Regular named r.Name, keeping r.RecipeURL for
// of r.RecipeURL (whose recipe grounding later replaces), with status and // grounding when given, with status and origin regular and grounding_status
// origin regular and grounding_status pending, recording updatedBy, and // pending, recording updatedBy, and enqueues the KindGroundDinner job that grounds it. It returns without
// enqueues the KindGroundDinner job that grounds it. It returns without
// waiting for the job. A name repeating an unarchived dinner is a // waiting for the job. A name repeating an unarchived dinner is a
// *DuplicateError. When the job cannot be enqueued, the stored dinner is // *DuplicateError. When the job cannot be enqueued, the stored dinner is
// returned with grounding_status failed (ReasonGroundingError) and no job // returned with grounding_status failed (ReasonGroundingError) and no job
@@ -224,34 +225,24 @@ func (s *Service) createRegular(ctx context.Context, r NewRegular, updatedBy str
if updatedBy == "" { if updatedBy == "" {
return db.Dinner{}, errors.New("dinners: create: no member to record as updated_by") return db.Dinner{}, errors.New("dinners: create: no member to record as updated_by")
} }
var name string if r.Name == nil {
var recipeURL sql.NullString return db.Dinner{}, invalid("name", "give the dinner's name, with or without a recipe_url")
switch { }
case r.Name != nil && r.RecipeURL != nil: name, err := checkName(*r.Name)
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 { if err != nil {
return db.Dinner{}, err return db.Dinner{}, err
} }
name = n var recipeURL sql.NullString
case r.RecipeURL != nil: if r.RecipeURL != nil {
u, err := checkRecipeURL(*r.RecipeURL) u, err := checkRecipeURL(*r.RecipeURL)
if err != nil { if err != nil {
return db.Dinner{}, err 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} 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 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) q := db.New(tx)
if err := checkUnique(ctx, q, "", name, recipeURL); err != nil { if err := checkUnique(ctx, q, "", name, recipeURL); err != nil {
return err return err
@@ -496,60 +487,6 @@ func checkRecipeURL(raw string) (*url.URL, error) {
return u, nil return u, nil
} }
// credentialWords end the name nameFromURL takes from a slug: a pasted link
// can carry a personal token in its path, as in /stew-access_token-abc123,
// and the name is shown in every dinner response.
var credentialWords = map[string]bool{
"access": true, "accesstoken": true, "api": true, "apikey": true, "auth": true,
"authtoken": true, "bearer": true, "credential": true, "credentials": true,
"jwt": true, "passcode": true, "password": true, "passwd": true, "secret": true,
"session": true, "sig": true, "signature": true, "token": true,
}
// maxSlugWordChars bounds a word nameFromURL keeps: a longer run of letters
// is more likely a token than a dish.
const maxSlugWordChars = 20
// nameFromURL names a dinner from the last path segment of u: separators
// become spaces, a file extension is dropped, only words of at most
// maxSlugWordChars letters are kept, the name ends at the first of
// credentialWords 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) }) {
w = strings.ToLower(w)
if credentialWords[w] {
break
}
if utf8.RuneCountInString(w) <= maxSlugWordChars && strings.IndexFunc(w, notLetter) < 0 {
words = append(words, 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
}
// notLetter reports whether r is neither a letter nor a combining mark.
func notLetter(r rune) bool {
return !unicode.IsLetter(r) && !unicode.IsMark(r)
}
// checkUnique returns a *DuplicateError when name is harness.Similar to an // checkUnique returns a *DuplicateError when name is harness.Similar to an
// unarchived dinner other than self, or recipeURL is one's recipe link: the // 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 // source_url of the recipe it is grounded in, else the recipe_url still
+61 -31
View File
@@ -1,44 +1,74 @@
package dinners package dinners_test
import ( import (
"net/url" "encoding/json"
"errors"
"strings"
"testing" "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() t.Parallel()
tests := []struct { e := newUnstartedEnv(t)
link string const link = "https://c.example/stew-key-abcdefghijklmno?sig=zyxwvut#frag"
want string // "" means no usable name leaks := []string{"c.example", "key", "abcdefghijklmno", "sig", "zyxwvut", "frag"}
}{ check := func(what, text string) {
{"https://example.com/recipes/12345/chicken-tikka-masala/", "Chicken tikka masala"}, t.Helper()
{"https://example.com/recipe/beef_stew.html", "Beef stew"}, lower := strings.ToLower(text)
{"https://example.com/2024/05/best-ever-chili-123", "Best ever chili"}, for _, leak := range leaks {
{"https://example.com/crème-brûlée", "Crème brûlée"}, if strings.Contains(lower, leak) {
{"https://example.com/recipes/chili?utm=1#notes", "Chili"}, t.Errorf("%s contains %q: %s", what, leak, text)
{"https://example.com/", ""},
{"https://example.com/recipe/98765", ""},
{"https://example.com/the-best-easy-recipe", ""},
{"https://example.com/best-chili-v2", "Best chili"},
{"https://example.com/stew-access_token-personaltokenabc123", "Stew"},
{"https://example.com/stew-token-abcdefghijk", "Stew"},
{"https://example.com/chili-api-key-zyxwvut", "Chili"},
{"https://example.com/stew-x7k2p9q4", "Stew"},
{"https://example.com/recipes/token-abcdefghijk", ""},
{"https://example.com/key-lime-pie", "Key lime pie"},
{"https://example.com/stew-t=abcdef", "Stew"},
{"https://example.com/stew-accesstoken-abcdefghijk", "Stew"},
{"https://example.com/stew-abcdefghijklmnopqrstuvwxyz", "Stew"},
{"https://example.com/cre%CC%80me-bru%CC%82le%CC%81e", "Cre\u0300me bru\u0302le\u0301e"},
} }
for _, tt := range tests { }
u, err := url.Parse(tt.link) }
_, 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
got, ok := nameFromURL(u) if c.Name != "Sunday stew" {
if got != tt.want || ok != (tt.want != "") { t.Errorf("name = %q, want the household's %q", c.Name, "Sunday stew")
t.Errorf("nameFromURL(%s) = %q, %v; want %q", tt.link, got, ok, tt.want)
} }
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())
} }
+6 -6
View File
@@ -194,7 +194,7 @@ func TestCreateByRecipeURLGroundsThatURL(t *testing.T) {
e := newEnv(t) e := newEnv(t)
e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150)) 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -223,7 +223,7 @@ func TestCreateGroundingFailureRecordsReason(t *testing.T) {
reason string reason string
}{ }{
{"no candidates", dinners.NewRegular{Name: ptr("Unknown dish")}, nil, dinners.ReasonNoRecipe}, {"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}, {"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) { {"model fails", dinners.NewRegular{Name: ptr("Chicken curry")}, func(e *env) {
// One provider failure per candidate tried. // 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.search.Results["Beef stew"] = []recipes.Candidate{{URL: stew}}
e.model.QueueGenerate(recipestest.NormalizedReply("oven", 150)) 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -740,7 +740,7 @@ func TestFixReplacesPendingLink(t *testing.T) {
t.Run(tt.name, func(t *testing.T) { t.Run(tt.name, func(t *testing.T) {
t.Parallel() t.Parallel()
e := newEnv(t) 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 { if err != nil {
t.Fatal(err) t.Fatal(err)
} }
@@ -763,11 +763,11 @@ func TestFixReplacesPendingLink(t *testing.T) {
t.Errorf("recipe_url = %q after the fix, want the unusable link dropped", row.RecipeUrl.String) 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) t.Errorf("adding the dropped link again: %v, want added", err)
} }
if tt.linkTaken != "" { 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 var dup *dinners.DuplicateError
if !errors.As(err, &dup) || dup.Existing.ID != c.ID { 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) 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. // manual recipe of dinners.MaxManualLines lines of each kind.
const maxGroundBody = 512 << 10 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 { type dinnerCreate struct {
Name *string `json:"name"` Name *string `json:"name"`
RecipeURL *string `json:"recipe_url"` RecipeURL *string `json:"recipe_url"`
+28 -22
View File
@@ -186,12 +186,12 @@ func TestCreateDinnerByURL(t *testing.T) {
router, _ := newDinnersRouter(t) router, _ := newDinnersRouter(t)
link := "https://www.example.com/recipes/12345/beef-and-broccoli-stir-fry/" 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 { if rec.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body) t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body)
} }
for k, v := range map[string]any{ for k, v := range map[string]any{
"name": "Beef and broccoli stir fry", "name": "Stir fry night",
"status": "regular", "status": "regular",
"origin": "regular", "origin": "regular",
"grounding_status": "pending", "grounding_status": "pending",
@@ -211,18 +211,17 @@ func TestCreateDinnerByURL(t *testing.T) {
if rec.Code != http.StatusCreated { if rec.Code != http.StatusCreated {
t.Fatalf("unrelated name = %d: %s", rec.Code, rec.Body) 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" { if rec.Code != http.StatusConflict || errorCode(body) != "duplicate_dinner" {
t.Errorf("same link again = %d %v, want 409 duplicate_dinner", rec.Code, body) t.Errorf("same link again = %d %v, want 409 duplicate_dinner", rec.Code, body)
} }
for _, tc := range []struct{ name, body string }{ for _, tc := range []struct{ name, body string }{
{"plain http", `{"recipe_url":"http://example.com/recipes/chili"}`}, {"plain http", `{"name":"Chili","recipe_url":"http://example.com/recipes/chili"}`},
{"not a URL", `{"recipe_url":"chili"}`}, {"not a URL", `{"name":"Chili","recipe_url":"chili"}`},
{"credentials", `{"recipe_url":"https://me:pw@example.com/recipes/chili"}`}, {"credentials", `{"name":"Chili","recipe_url":"https://me:pw@example.com/recipes/chili"}`},
{"no name in path", `{"recipe_url":"https://example.com/"}`}, {"link without a name", `{"recipe_url":"https://example.com/recipes/chili"}`},
{"numeric slug", `{"recipe_url":"https://example.com/recipe/98765"}`}, {"filler name with a link", `{"name":"The Best Easy Recipe","recipe_url":"https://example.com/recipes/chili"}`},
{"both fields", `{"name":"Chili","recipe_url":"https://example.com/recipes/chili"}`},
{"neither field", `{}`}, {"neither field", `{}`},
} { } {
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, tc.body) rec, body := callDinners(t, router, http.MethodPost, dinnersPath, tc.body)
@@ -240,7 +239,7 @@ func TestDinnerResponsesHideRecipeURL(t *testing.T) {
router, st := newDinnersRouter(t) router, st := newDinnersRouter(t)
const link = "https://c.example/recipes/stew?access_token=personal-token-abc123" const link = "https://c.example/recipes/stew?access_token=personal-token-abc123"
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"recipe_url":"`+link+`"}`) rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Stew","recipe_url":"`+link+`"}`)
if rec.Code != http.StatusCreated { if rec.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body) t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body)
} }
@@ -266,29 +265,36 @@ func TestDinnerResponsesHideRecipeURL(t *testing.T) {
} }
} }
// TestDinnerResponsesHideTokenInLinkPath: a personal token can sit in the // TestDinnerResponsesHideTokenInLinkPath: a personal token can sit
// link's last path segment, which names the dinner, so the name stops before // anywhere in a link's path, even as plain letters, so a dinner added from
// it and neither the create response nor a later list read shows it. // 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) { func TestDinnerResponsesHideTokenInLinkPath(t *testing.T) {
t.Parallel() t.Parallel()
router, _ := newDinnersRouter(t) router, _ := newDinnersRouter(t)
const link = "https://c.example/recipes/stew-access_token-personaltokenabc123" const link = "https://c.example/recipes/stew-key-abcdefghijklmno"
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"recipe_url":"`+link+`"}`) rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"name":"Sunday stew","recipe_url":"`+link+`"}`)
if rec.Code != http.StatusCreated { if rec.Code != http.StatusCreated {
t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body) t.Fatalf("status = %d, want 201: %s", rec.Code, rec.Body)
} }
if body["name"] != "Stew" { if body["name"] != "Sunday stew" {
t.Errorf("name = %v, want Stew", body["name"]) t.Errorf("name = %v, want Sunday stew", body["name"])
} }
created := rec.Body.String() created := rec.Body.String()
rec, _ = callDinners(t, router, http.MethodGet, dinnersPath+"?status=regular", "") rec, _ = callDinners(t, router, http.MethodGet, dinnersPath+"?status=regular", "")
if rec.Code != http.StatusOK { if rec.Code != http.StatusOK {
t.Fatalf("GET status=regular = %d: %s", rec.Code, rec.Body) t.Fatalf("GET status=regular = %d: %s", rec.Code, rec.Body)
} }
for _, resp := range []struct{ name, body string }{{"create", created}, {"list", rec.Body.String()}} { 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) lower := strings.ToLower(resp.body)
for _, secret := range []string{"personaltokenabc123", "abc123", "access", "token"} { for _, secret := range []string{"c.example", "recipes", "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)
} }
@@ -311,10 +317,10 @@ func TestCreateDinnerDuplicate(t *testing.T) {
t.Errorf("%q: %d %v, want 409 duplicate_dinner", name, rec.Code, body) t.Errorf("%q: %d %v, want 409 duplicate_dinner", name, rec.Code, body)
} }
} }
// A slug naming the same dish is a duplicate too. // A name repeating the dish is a duplicate with a link too.
rec, body := callDinners(t, router, http.MethodPost, dinnersPath, `{"recipe_url":"https://example.com/easy-chicken-tikka-masala.html"}`) 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 { 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 var n int
+8 -7
View File
@@ -33,8 +33,10 @@ export type Regular = {
pending?: true pending?: true
} }
// NewRegular is a POST /api/v1/dinners body: a name or a recipe link. // NewRegular is a POST /api/v1/dinners body: the household's name for the
export type NewRegular = { name: string } | { recipe_url: string } // 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. // 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 }> export type RegularPatch = Partial<{ name: string; method: Method; minutes_total: number; difficulty: Difficulty }>
@@ -195,10 +197,9 @@ function settle(queryClient: QueryClient) {
let pendingIds = 0 let pendingIds = 0
// useAddRegular adds a Regular by name or recipe link (POST // useAddRegular adds a Regular by name, with or without a recipe link (POST
// /api/v1/dinners). It shows in the list at once — a link under the link // /api/v1/dinners). It shows in the list at once under its name and is
// itself until the server names it — and is taken out again if the server // taken out again if the server refuses it.
// refuses it.
export function useAddRegular() { export function useAddRegular() {
const { apiFetch } = useApi() const { apiFetch } = useApi()
const queryClient = useQueryClient() const queryClient = useQueryClient()
@@ -210,7 +211,7 @@ export function useAddRegular() {
const id = `pending-${++pendingIds}` const id = `pending-${++pendingIds}`
const optimistic: CachedRegular = { const optimistic: CachedRegular = {
id, id,
name: 'name' in add ? add.name : add.recipe_url, name: add.name,
method: null, method: null,
minutes_total: null, minutes_total: null,
difficulty: null, difficulty: null,
@@ -43,6 +43,13 @@
font-weight: 700; 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. */ /* The add field: the input fills the row beside the grape + button. */
.add { .add {
display: flex; display: flex;
+20 -8
View File
@@ -192,21 +192,33 @@ describe('RegularsSettingsPage', () => {
expect(screen.getByRole('heading', { level: 2, name: 'On rotation · 2' })).toBeInTheDocument() expect(screen.getByRole('heading', { level: 2, name: 'On rotation · 2' })).toBeInTheDocument()
}) })
it('adds a Regular from a pasted recipe link', async () => { it('adds a Regular from a pasted recipe link under the name the household gives it', async () => {
const link = 'https://example.com/recipes/12345/beef-stew/' const link = 'https://example.com/recipes/stew-key-abcdefghijklmno'
const server = stubServer([], { const server = stubServer([], {
'POST /api/v1/dinners': () => { 'POST /api/v1/dinners': (body) => {
const created = dinnerBody('d9', 'Beef stew', { grounding_status: 'pending' }) const created = dinnerBody('d9', (body as { name: string }).name, { grounding_status: 'pending' })
server.regulars = [created] server.regulars = [created]
return jsonResponse(201, { ...created, grounding_job_id: 'job-1' }) return jsonResponse(201, { ...created, grounding_job_id: 'job-1' })
}, },
}) })
await renderPage() await renderPage()
add(link) fireEvent.change(addField(), { target: { value: ` ${link} ` } })
expect(await screen.findByText('Beef stew')).toBeInTheDocument() // A link alone cannot be added: it never names a dinner.
expect(sent(server.fetchMock)).toEqual([{ request: 'POST /api/v1/dinners', body: { recipe_url: link } }]) expect(screen.getByRole('button', { name: 'Add Regular' })).toBeDisabled()
expect(within(row('Beef stew')).getByText('Finding recipe…')).toBeInTheDocument() 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 () => { it('takes back an add the server refuses and says why', async () => {
+32 -7
View File
@@ -49,12 +49,14 @@ export const methodLabels: Record<Method, string> = {
const difficultyLabels: Record<Difficulty, string> = { easy: 'Easy', medium: 'Medium', hard: 'Hard' } 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 const LINK = /^https?:\/\//i
// Settings · Regular dinners: the household's Regulars, each with its photo, // Settings · Regular dinners: the household's Regulars, each with its photo,
// method and minutes, favorite heart and whether its recipe was found. The // 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. // edits or removes it, or fixes its recipe when none was found.
export function RegularsSettingsPage() { export function RegularsSettingsPage() {
const navigate = useNavigate() const navigate = useNavigate()
@@ -85,19 +87,24 @@ function RegularsManager() {
const favorite = useToggleFavorite() const favorite = useToggleFavorite()
const { refusals, track } = useRefusals() const { refusals, track } = useRefusals()
const [draft, setDraft] = useState('') const [draft, setDraft] = useState('')
const [linkName, setLinkName] = useState('')
const [open, setOpen] = useState<Open>(null) const [open, setOpen] = useState<Open>(null)
const headingId = useId() const headingId = useId()
const linkHintId = useId()
const isLink = LINK.test(draft.trim())
const ready = draft.trim() !== '' && (!isLink || linkName.trim() !== '')
const list = regulars.data ?? [] const list = regulars.data ?? []
// A Regular removed meanwhile closes its sheet. // A Regular removed meanwhile closes its sheet.
const opened = open && list.find((regular) => regular.id === open.id && !regular.pending) const opened = open && list.find((regular) => regular.id === open.id && !regular.pending)
function onSubmit(event: FormEvent<HTMLFormElement>) { function onSubmit(event: FormEvent<HTMLFormElement>) {
event.preventDefault() event.preventDefault()
if (!ready) return
const text = draft.trim() const text = draft.trim()
if (text === '') return const body: NewRegular = isLink ? { name: linkName.trim(), recipe_url: text } : { name: text }
const body: NewRegular = LINK.test(text) ? { recipe_url: text } : { name: text } track(`add:${body.name.toLowerCase()}`, `add ${body.name}`, add.mutateAsync(body))
track(`add:${text.toLowerCase()}`, `add ${text}`, add.mutateAsync(body))
setDraft('') setDraft('')
setLinkName('')
} }
function onRemove(regular: Regular) { function onRemove(regular: Regular) {
@@ -111,7 +118,8 @@ function RegularsManager() {
return ( return (
<> <>
<form className={styles.add} onSubmit={onSubmit}> <form className={styles.addForm} onSubmit={onSubmit}>
<div className={styles.add}>
<TextInput <TextInput
label="Add a Regular" label="Add a Regular"
labelHidden labelHidden
@@ -122,9 +130,26 @@ function RegularsManager() {
value={draft} value={draft}
onChange={(event) => setDraft(event.target.value)} onChange={(event) => setDraft(event.target.value)}
/> />
<IconButton type="submit" label="Add Regular" tone="accent" disabled={draft.trim() === ''}> <IconButton type="submit" label="Add Regular" tone="accent" disabled={!ready}>
<Icon icon={Plus} size="nav" /> <Icon icon={Plus} size="nav" />
</IconButton> </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> </form>
{refusals.map((refusal) => ( {refusals.map((refusal) => (
<p key={refusal.key} role="alert" className={styles.problem}> <p key={refusal.key} role="alert" className={styles.problem}>