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

6 changed files with 58 additions and 14 deletions
Showing only changes of commit c15c8ec284 - Show all commits
+2 -1
View File
@@ -180,7 +180,8 @@ automatic** (D-021).
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
`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. 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
+3 -3
View File
@@ -55,7 +55,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 +69,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;
@@ -565,7 +566,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),
+11 -5
View File
@@ -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)
@@ -749,12 +752,15 @@ 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 {
+41 -2
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)
@@ -190,7 +192,6 @@ func TestCreateDinnerByURL(t *testing.T) {
}
for k, v := range map[string]any{
"name": "Beef and broccoli stir fry",
"recipe_url": link,
"status": "regular",
"origin": "regular",
"grounding_status": "pending",
@@ -200,6 +201,10 @@ 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"}`)
@@ -227,6 +232,40 @@ 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, `{"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)
}
}
func TestCreateDinnerDuplicate(t *testing.T) {
t.Parallel()
router, st := newDinnersRouter(t)
@@ -196,7 +196,7 @@ describe('RegularsSettingsPage', () => {
const link = 'https://example.com/recipes/12345/beef-stew/'
const server = stubServer([], {
'POST /api/v1/dinners': () => {
const created = dinnerBody('d9', 'Beef stew', { grounding_status: 'pending', recipe_url: link })
const created = dinnerBody('d9', 'Beef stew', { grounding_status: 'pending' })
server.regulars = [created]
return jsonResponse(201, { ...created, grounding_job_id: 'job-1' })
},
-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,