diff --git a/docs/NUTRITION_DATA.md b/docs/NUTRITION_DATA.md new file mode 100644 index 0000000..3933db3 --- /dev/null +++ b/docs/NUTRITION_DATA.md @@ -0,0 +1,136 @@ +# Nutrition data — the contract between the agent team, Fiesta and the app + +The customer app shows a nutrition panel on the product screen. The data comes +from the global catalogue, which the agent team fills. This is what each side +has to produce and can rely on. + +--- + +## 1. What the app receives + +`GET /live/api/v1/mob/products/getproductbyvariant?tenantid=&productid=&variantid=` + +Each product in `details[]` carries: + +```json +"nutrition": { + "per": "100g", + "servingsize": "30g", + "items": [ + { "name": "Energy", "value": 520, "unit": "kcal" }, + { "name": "Protein", "value": 6.5, "unit": "g" }, + { "name": "Total Sugars", "value": 22, "unit": "g" }, + { "name": "Sodium", "value": 310, "unit": "mg" } + ] +} +``` + +Rules the app can build on: + +- **The key is ABSENT when nothing is known.** Not `null`, not `{}`. A missing + key means "we do not know", never "this food has no nutrition". +- **`items` is never empty when `nutrition` is present.** A panel with no rows + is not sent, because an empty box on a product page reads as a claim. +- **`per` and `servingsize` are each optional** and often absent — see §3. Show + the basis only when it is there. Do not default it to `100g`: a wrong basis + makes every figure beneath it a misstatement rather than an unknown. +- **`value` is a number, and may be a decimal.** Saturated fat is 11.5 g as + often as 11 g. +- **`unit` is free text and may be absent.** "Servings per pack 4" has a figure + and no unit. +- **A row may have a `name` and a `value` of 0 with no unit.** That is real + label text with no number in it — "Contains permitted natural colour". Render + the name and leave the figure column blank; do not print `0`. +- Every member of a variant group carries its own panel, so switching from + 500 ml to 1 L does not blank the screen. + +## 2. What the agent team fills + +A `nutrition` column of type **`jsonb`** on each `brand_*` table in the +catalogue database, holding exactly the object above. + +```sql +ALTER TABLE brand_patanjali ADD COLUMN IF NOT EXISTS nutrition jsonb; +``` + +Notes: + +- **The column is optional and Fiesta already handles its absence.** Catalogue + reads discover columns per table and substitute NULL for any that are missing, + so brands can be filled one at a time and a table without the column keeps + working. Nothing has to be co-ordinated with a deploy. +- **Spelling is `servingsize`**, one word, lowercase. It is the shape the app + was written against. +- **Write what the label says.** If the pack states per 100 g, `per` is `"100g"`. + If it states per serving, say so. If it states neither, omit the field rather + than assuming. +- `nutrition` and the older `nutrients` may both exist on a row. Keep both — + the console shows the lines, the app shows the panel, and they are different + readers with different needs. + +## 3. Where today's data comes from, and why it is thinner + +Every catalogue row currently holds nutrition as `nutrients`, a `text[]` of +display lines: + +``` +{"Energy 350kcal", "Protein 7g"} +``` + +Fiesta parses those into the same panel shape when no structured `nutrition` +exists, so the app shows figures for products the agent team has not reached +yet. That derived panel has **no `per` and no `servingsize`** — the strings +never carried them — which is the visible difference between a filled brand and +one still waiting. + +Parsing rules, in `models/nutrition.go`: + +| line | becomes | +|---|---| +| `Energy 350kcal` | `{Energy, 350, kcal}` | +| `Protein: 6.5 g` | `{Protein, 6.5, g}` | +| `Energy - 520 kcal` | `{Energy, 520, kcal}` | +| `Vitamin B12 1.2µg` | `{Vitamin B12, 1.2, µg}` | +| `Servings per pack 4` | `{Servings per pack, 4, ""}` | +| `Contains permitted colour` | `{Contains permitted colour, 0, ""}` — kept whole | +| `100` | dropped — a figure with no label is noise | + +Structured always wins over derived where both exist. + +## 4. How it reaches a shop's product + +The catalogue is the source; a tenant's product is a **snapshot** taken at +import, because the catalogue is re-scraped and rows retire. Chain: + +``` +brand_*.nutrition (agent team) + └─ catalogue read ─ repositories/catalogueRepository.go: nutritionOf + └─ import ─ services/productService.go: catalogueFactsOf + └─ products.cataloguefacts → {"nutrition": {...}, "nutrients": [...]} + └─ endpoint ─ services/productService.go: decorateNutrition +``` + +`getproductbyvariant` does **not** query the catalogue database. That would be a +cross-database lookup per product on a screen a shopper is waiting on. It reads +the snapshot only. + +## 5. Products imported before the snapshot existed + +**This is the one thing that must be done before any of it shows.** + +`cataloguefacts` is empty on every product imported before that column existed — +measured on tenant 1147: 17 products, 15 catalogue-imported, **0 with facts**. +Those products will show no nutrition no matter what the catalogue holds. + +The fix is a one-off backfill, which also restores their FSSAI licence, +highlights and provider list: + +```sh +go run ./scratch/cataloguefactsbackfill # dry run — prints every change +go run ./scratch/cataloguefactsbackfill apply # writes, then prints the undo +``` + +It touches only products with an `imageid` and a NULL `cataloguefacts`, so +re-running it is a no-op rather than a second opinion. Run it **after** the agent +team fills a brand to pick up that brand's structured panels; it is safe to run +repeatedly as more brands are filled. diff --git a/models/catalogue.go b/models/catalogue.go index cef55c9..5b96654 100644 --- a/models/catalogue.go +++ b/models/catalogue.go @@ -26,9 +26,12 @@ type CatalogueProduct struct { FSSAILicense string `json:"fssai_license,omitempty"` Highlights PGStringArray `json:"highlights,omitempty"` Nutrients PGStringArray `json:"nutrients,omitempty"` - SearchQuery string `json:"search_query,omitempty"` - CreatedAt time.Time `json:"created_at,omitempty"` - UpdatedAt time.Time `json:"updated_at,omitempty"` + // The structured panel, when the catalogue carries one. Nil falls back to + // parsing Nutrients above — see NutritionFromLines. + Nutrition *NutritionPanel `json:"nutrition,omitempty"` + SearchQuery string `json:"search_query,omitempty"` + CreatedAt time.Time `json:"created_at,omitempty"` + UpdatedAt time.Time `json:"updated_at,omitempty"` } // CatalogueBrand describes a brand available in the catalogue DB. diff --git a/models/nutrition.go b/models/nutrition.go new file mode 100644 index 0000000..5ab52ee --- /dev/null +++ b/models/nutrition.go @@ -0,0 +1,137 @@ +package models + +import ( + "regexp" + "strconv" + "strings" +) + +/* +The nutrition panel, as the customer app renders it. + +── Why this is a type and not a list of strings ──────────────────────────── + +The catalogue already carries `nutrients`, a text[] of display lines like +"Energy 350kcal". That is enough to print bullets, which is what the console +does with it today, and not enough for an app: it cannot sort by a value, show +a per-serving column beside a per-100g one, or put the unit in a different +style from the number. It also has nowhere to say what the figures are PER, +which is the one piece of context that makes the rest meaningful — 520 kcal is +a fact about a quantity, and without "per 100g" it is a fact about nothing. + +So this is the shape the agent team fills and the app reads. See +docs/NUTRITION_DATA.md for the contract. + +── Why the field names are ugly ──────────────────────────────────────────── + +`servingsize`, not `serving_size` or `servingSize`. This is the shape the app +developer asked for, and an API is a promise to a client that has already been +written against it. Consistency with the rest of Fiesta — which is itself +inconsistent, `productid` beside `image_id` beside `sku_source` — is worth less +than not breaking the caller. +*/ +type NutritionPanel struct { + // What the figures are measured against: "100g", "100ml", "1 serving". + Per string `json:"per,omitempty"` + // What the pack calls one serving: "30g". Separate from `Per` because a + // label routinely states both, and the app shows them in different places. + Servingsize string `json:"servingsize,omitempty"` + // Never nil when this panel exists — see `HasValues`. An app that receives + // `items: null` has to branch; one that receives `[]` does not, and a panel + // with no rows should not have been sent at all. + Items []NutritionItem `json:"items"` +} + +// NutritionItem is one line of the panel. +type NutritionItem struct { + Name string `json:"name"` + // The figure. A float because saturated fat is 11.5g as often as it is 11g, + // and rounding it to please a type would be changing a label. + Value float64 `json:"value"` + // "kcal", "g", "mg". Free text on purpose: the label is the authority and a + // closed list here would mean refusing to carry whatever it actually says. + Unit string `json:"unit,omitempty"` +} + +// HasValues reports whether this panel is worth sending. +// +// A panel with no rows is not a panel — it is an empty box on the product page, +// which reads as "this product has no nutrition" rather than "we do not know +// yet". The endpoint omits it instead. +func (p *NutritionPanel) HasValues() bool { + return p != nil && len(p.Items) > 0 +} + +/* +nutrientLine pulls "Energy 350kcal" apart. + +── Why parsing exists at all ──────────────────────────────────────────────── + +Every catalogue row on the platform today holds nutrition as those display +strings and nothing else. Waiting for the agent team to refill all of them +before the app can show anything would mean shipping a field that is null for +every product, for as long as that takes. + +So a structured panel is used when one exists, and one is derived from the +strings when it does not. The derived panel is strictly worse — it has no `per` +and no serving size, because the strings never carried them — and it is still +the difference between an app screen with values on it and an empty one. + +── What it refuses to do ─────────────────────────────────────────────────── + +A line it cannot read is kept WHOLE as the name, with no value and no unit, +rather than dropped or guessed at. "Contains permitted natural colour" is a +real nutrition line and it has no number in it; binning it would quietly lose +label text, and forcing a 0 into it would state something false about the food. +*/ +var nutrientLine = regexp.MustCompile(`^(.*?)[\s:]*(-?\d+(?:\.\d+)?)\s*([a-zA-Zµ%]*)$`) + +// NutritionFromLines derives a panel from the catalogue's display strings. +// +// Returns nil when nothing usable is found, so a caller can tell "no nutrition" +// from "a panel with no numbers in it". +func NutritionFromLines(lines []string) *NutritionPanel { + items := make([]NutritionItem, 0, len(lines)) + + for _, raw := range lines { + line := strings.TrimSpace(raw) + if line == "" { + continue + } + + match := nutrientLine.FindStringSubmatch(line) + if match == nil { + // No number anywhere. Kept as written — see above. + items = append(items, NutritionItem{Name: line}) + continue + } + + name := strings.TrimSpace(strings.Trim(match[1], "-–—:")) + if name == "" { + // The whole line was a number. Nothing sensible to label it with, + // and an unnamed row on a nutrition panel is noise. + continue + } + + value, err := strconv.ParseFloat(match[2], 64) + if err != nil { + items = append(items, NutritionItem{Name: line}) + continue + } + + items = append(items, NutritionItem{ + Name: name, + Value: value, + Unit: strings.TrimSpace(match[3]), + }) + } + + if len(items) == 0 { + return nil + } + // No `per` and no serving size, deliberately left empty rather than guessed + // at. "100g" is the common case and it is not the only one, and a wrong + // basis is worse than an absent one — it makes every figure beneath it a + // misstatement rather than an unknown. + return &NutritionPanel{Items: items} +} diff --git a/models/nutrition_test.go b/models/nutrition_test.go new file mode 100644 index 0000000..144df66 --- /dev/null +++ b/models/nutrition_test.go @@ -0,0 +1,141 @@ +package models + +import "testing" + +/* +Reading the catalogue's nutrition strings. + +The catalogue holds nutrition as display lines — "Energy 350kcal" — and the app +needs figures. These are the shapes those lines actually come in, and the ones +they come in when a scrape goes sideways. + +The rule throughout: never invent a number, and never lose label text. A line +that cannot be read is carried whole rather than dropped, because it is +something a manufacturer printed on a packet and this code is not the authority +on what belongs on a food label. +*/ + +func TestAPlainLineBecomesAFigure(t *testing.T) { + panel := NutritionFromLines([]string{"Energy 350kcal"}) + if !panel.HasValues() { + t.Fatal("nothing parsed") + } + got := panel.Items[0] + if got.Name != "Energy" || got.Value != 350 || got.Unit != "kcal" { + t.Fatalf("got %+v", got) + } +} + +func TestTheSeparatorsThatActuallyOccur(t *testing.T) { + // Colons, multi-word names and a space before the unit are all in the wild, + // and each one used to be a whole line lost. + for _, tc := range []struct { + line string + name string + value float64 + unit string + }{ + {"Protein: 6.5 g", "Protein", 6.5, "g"}, + {"Total Sugars 22g", "Total Sugars", 22, "g"}, + {"Saturated Fat 11.5 g", "Saturated Fat", 11.5, "g"}, + {"Sodium 310mg", "Sodium", 310, "mg"}, + {"Energy - 520 kcal", "Energy", 520, "kcal"}, + } { + panel := NutritionFromLines([]string{tc.line}) + if !panel.HasValues() { + t.Errorf("%q parsed to nothing", tc.line) + continue + } + got := panel.Items[0] + if got.Name != tc.name || got.Value != tc.value || got.Unit != tc.unit { + t.Errorf("%q → %+v, want {%s %v %s}", tc.line, got, tc.name, tc.value, tc.unit) + } + } +} + +func TestADecimalSurvives(t *testing.T) { + // Saturated fat is 11.5g as often as 11g. Rounding to please a type would be + // editing a food label. + panel := NutritionFromLines([]string{"Saturated Fat 11.5g"}) + if panel.Items[0].Value != 11.5 { + t.Fatalf("got %v, want 11.5", panel.Items[0].Value) + } +} + +func TestALineWithNoNumberIsKeptWhole(t *testing.T) { + // Real label text. Dropping it loses something a manufacturer printed; + // forcing a 0 into it states something false about the food. + panel := NutritionFromLines([]string{"Contains permitted natural colour"}) + if !panel.HasValues() { + t.Fatal("the line was dropped") + } + got := panel.Items[0] + if got.Name != "Contains permitted natural colour" { + t.Fatalf("the text was mangled: %+v", got) + } + if got.Value != 0 || got.Unit != "" { + t.Fatalf("a figure was invented for a line that had none: %+v", got) + } +} + +func TestAUnitlessFigureKeepsItsNumber(t *testing.T) { + // "Servings per pack 4" has a real number and no unit. + panel := NutritionFromLines([]string{"Servings per pack 4"}) + got := panel.Items[0] + if got.Name != "Servings per pack" || got.Value != 4 || got.Unit != "" { + t.Fatalf("got %+v", got) + } +} + +func TestPercentAndMicrogramsAreUnits(t *testing.T) { + for _, tc := range []struct{ line, unit string }{ + {"Vitamin C 45%", "%"}, + {"Vitamin B12 1.2µg", "µg"}, + } { + panel := NutritionFromLines([]string{tc.line}) + if !panel.HasValues() || panel.Items[0].Unit != tc.unit { + t.Errorf("%q → %+v, want unit %q", tc.line, panel.Items[0], tc.unit) + } + } +} + +func TestBlanksAndBareNumbersAreNotRows(t *testing.T) { + // A blank is nothing. A bare "100" has no label, and an unnamed row on a + // nutrition panel is noise a shopper cannot use. + if panel := NutritionFromLines([]string{"", " ", "100"}); panel != nil { + t.Fatalf("made a panel out of nothing: %+v", panel) + } +} + +func TestNoLinesMeansNoPanel(t *testing.T) { + // nil rather than an empty panel, so a caller can tell "no nutrition known" + // from "a panel that happens to be empty" — the endpoint omits the first. + if NutritionFromLines(nil) != nil { + t.Fatal("an absent panel was reported as present") + } + if NutritionFromLines([]string{}) != nil { + t.Fatal("an absent panel was reported as present") + } +} + +func TestAnEmptyPanelIsNotWorthSending(t *testing.T) { + // `items: []` on a product page renders as an empty box, which reads as + // "this food has no nutrition" rather than "we do not know yet". + var absent *NutritionPanel + if absent.HasValues() { + t.Fatal("nil reported as having values") + } + if (&NutritionPanel{Per: "100g"}).HasValues() { + t.Fatal("a panel with a basis and no rows reported as having values") + } +} + +func TestTheDerivedPanelDoesNotGuessItsBasis(t *testing.T) { + // The strings never carried one. "100g" is the common case and not the only + // one, and a wrong basis makes every figure beneath it a misstatement rather + // than an unknown. + panel := NutritionFromLines([]string{"Energy 350kcal"}) + if panel.Per != "" || panel.Servingsize != "" { + t.Fatalf("invented a basis: per=%q servingsize=%q", panel.Per, panel.Servingsize) + } +} diff --git a/models/product.go b/models/product.go index 88bb49c..95651ab 100644 --- a/models/product.go +++ b/models/product.go @@ -172,6 +172,20 @@ type Products struct { // scan-into-struct silently drops slice- and map-kind destination fields. Cataloguefacts string `json:"cataloguefacts,omitempty" gorm:"column:cataloguefacts;type:jsonb"` + // The nutrition panel, for the product screen in the customer app. + // + // `gorm:"-"`: not a column. It is unpacked from Cataloguefacts above, which + // is where the import snapshots it — a second column holding the same facts + // is a second thing to keep in step, and this one has no writer of its own. + // + // ABSENT rather than null when a product has no nutrition. Most products on + // the platform have none today, and `"nutrition": null` on every row of a + // mobile response is payload spent saying nothing. An app should read a + // missing key as "not known", never as "this food has no nutrition". + // + // Set by the service, not the repository — see decorateNutrition. + Nutrition *NutritionPanel `json:"nutrition,omitempty" gorm:"-"` + Productdesc string `json:"productdesc,omitempty"` Productsku string `json:"productsku,omitempty"` Brandid int `json:"brandid,omitempty"` @@ -226,22 +240,22 @@ type Products struct { } type Locationproducts struct { - Productid int `json:"productid"` - AppLocationid int `json:"applocationid" gorm:"column:applocationid"` - Productlocationid int `json:"productlocationid" gorm:"->"` - Tenantid int `json:"tenantid,omitempty"` - Categoryid int `json:"categoryid"` - Categoryname string `json:"categoryname" gorm:"->"` - Subcategoryid int `json:"subcategoryid,omitempty"` - Subcategoryname string `json:"Subcategoryname" gorm:"->"` - Catalogueid int `json:"catalogueid,omitempty"` - Addonid int `json:"addonid,omitempty"` - Discountid int `json:"discountid,omitempty"` - Pricingid int `json:"pricingid,omitempty"` - Productname string `json:"productname,omitempty"` - Productimage string `json:"productimage,omitempty"` - Productdesc string `json:"productdesc,omitempty"` - Productsku string `json:"productsku,omitempty"` + Productid int `json:"productid"` + AppLocationid int `json:"applocationid" gorm:"column:applocationid"` + Productlocationid int `json:"productlocationid" gorm:"->"` + Tenantid int `json:"tenantid,omitempty"` + Categoryid int `json:"categoryid"` + Categoryname string `json:"categoryname" gorm:"->"` + Subcategoryid int `json:"subcategoryid,omitempty"` + Subcategoryname string `json:"Subcategoryname" gorm:"->"` + Catalogueid int `json:"catalogueid,omitempty"` + Addonid int `json:"addonid,omitempty"` + Discountid int `json:"discountid,omitempty"` + Pricingid int `json:"pricingid,omitempty"` + Productname string `json:"productname,omitempty"` + Productimage string `json:"productimage,omitempty"` + Productdesc string `json:"productdesc,omitempty"` + Productsku string `json:"productsku,omitempty"` // Three columns this read used to leave in the table. // @@ -264,19 +278,19 @@ type Locationproducts struct { Productimages string `json:"productimages,omitempty"` Cataloguefacts string `json:"cataloguefacts,omitempty"` - Brandid int `json:"brandid,omitempty"` - Productbrand string `json:"productbrand,omitempty"` - Productunit string `json:"productunit"` - Unitvalue string `json:"unitvalue"` - Toppicks string `json:"toppicks,omitempty"` - Productcost float64 `json:"productcost,omitempty"` - Taxamount float64 `json:"taxamount,omitempty"` - Taxpercent float64 `json:"taxpercent,omitempty"` - Producttax int `json:"producttax" gorm:"default:0"` - Productstock int `json:"productstock" gorm:"default:0"` - Productcombo int `json:"productcombo" gorm:"default:0"` - Variants int `json:"variants" gorm:"default:0"` - Quantity int `json:"quantity"` + Brandid int `json:"brandid,omitempty"` + Productbrand string `json:"productbrand,omitempty"` + Productunit string `json:"productunit"` + Unitvalue string `json:"unitvalue"` + Toppicks string `json:"toppicks,omitempty"` + Productcost float64 `json:"productcost,omitempty"` + Taxamount float64 `json:"taxamount,omitempty"` + Taxpercent float64 `json:"taxpercent,omitempty"` + Producttax int `json:"producttax" gorm:"default:0"` + Productstock int `json:"productstock" gorm:"default:0"` + Productcombo int `json:"productcombo" gorm:"default:0"` + Variants int `json:"variants" gorm:"default:0"` + Quantity int `json:"quantity"` // Price is the per-store selling price from productlocations.price — the one // CreateProductLocation upserts. Read-only here: it comes from the joined // productlocations row, not from products. Without it a store could set a diff --git a/repositories/catalogueRepository.go b/repositories/catalogueRepository.go index fd8b69b..aa92ffb 100644 --- a/repositories/catalogueRepository.go +++ b/repositories/catalogueRepository.go @@ -37,6 +37,12 @@ var ErrCatalogueDBUnavailable = errors.New("catalogue database is not configured // catalogueProductColumns casts the text[] columns to text: GORM's raw // scan-into-struct silently drops slice-kind destination fields, so they // are read as text here and parsed into []string in scanProductRow. +// +// DELIBERATELY WITHOUT `nutrition`. This list is the fallback for when column +// discovery has not run, and it asserts that every column it names exists — so +// naming one that no brand table has yet would fail every read on this path +// rather than cost one field. `nutrition` lives in catalogueOptionalColumns +// below, which is what lets it ship before the agent team creates it. const catalogueProductColumns = `id, product_name, title, description, category, image_id, size, variant_key, product_sku, sku_source, price_range, providers::text AS providers, fssai_license, highlights::text AS highlights, nutrients::text AS nutrients, search_query, created_at, updated_at` @@ -67,6 +73,12 @@ var catalogueOptionalColumns = []struct{ Name, Present, Absent string }{ {"fssai_license", "fssai_license", "NULL::text AS fssai_license"}, {"highlights", "highlights::text AS highlights", "NULL::text AS highlights"}, {"nutrients", "nutrients::text AS nutrients", "NULL::text AS nutrients"}, + + // The structured panel the agent team fills: per, serving size and typed + // rows. Optional like everything else here, and that is what makes it + // shippable before the column exists — a brand table without it reads NULL + // and falls back to parsing `nutrients` above, rather than going dark. + {"nutrition", "nutrition::text AS nutrition", "NULL::text AS nutrition"}, {"search_query", "search_query", "NULL::text AS search_query"}, {"created_at", "created_at", "NULL::timestamptz AS created_at"}, {"updated_at", "updated_at", "NULL::timestamptz AS updated_at"}, @@ -134,6 +146,7 @@ type catalogueProductRow struct { FSSAILicense string Highlights string Nutrients string + Nutrition string SearchQuery string CreatedAt time.Time UpdatedAt time.Time @@ -217,6 +230,7 @@ func (row catalogueProductRow) toModel(brand string) models.CatalogueProduct { FSSAILicense: row.FSSAILicense, Highlights: models.ParsePGArray(row.Highlights), Nutrients: models.ParsePGArray(row.Nutrients), + Nutrition: nutritionOf(row), SearchQuery: row.SearchQuery, CreatedAt: row.CreatedAt, UpdatedAt: row.UpdatedAt, @@ -727,3 +741,29 @@ func (r *catalogueRepository) GetProductByID(brand string, id int64) (*models.Ca product := row.toModel(strings.ToLower(brand)) return &product, nil } + +// nutritionOf decides which nutrition a catalogue row actually has. +// +// Two sources, and the structured one wins wherever it exists. The agent team +// fills `nutrition` — a JSON panel with the basis, the serving size and typed +// rows — and that is the one the app wants. Everything already in the catalogue +// has only `nutrients`, a text[] of display lines, so those are parsed into the +// same shape rather than left unusable. +// +// The derived panel is strictly worse: the strings never carried a basis, so it +// has no "per 100g" on it. It is still the difference between a product page +// with figures and an empty one, for every row on the platform today. +// +// A malformed `nutrition` value falls back rather than failing the read. One +// bad JSON blob in one row should cost that row its panel, not take out the +// brand listing it appears in. +func nutritionOf(row catalogueProductRow) *models.NutritionPanel { + if raw := strings.TrimSpace(row.Nutrition); raw != "" && raw != "null" { + var panel models.NutritionPanel + if err := json.Unmarshal([]byte(raw), &panel); err == nil && panel.HasValues() { + return &panel + } + log.Printf("catalogue: product %d has nutrition that could not be read; falling back to its nutrient lines", row.ID) + } + return models.NutritionFromLines(models.ParsePGArray(row.Nutrients)) +} diff --git a/scratch/cataloguefactsbackfill/main.go b/scratch/cataloguefactsbackfill/main.go index 0a30aa3..1e4664a 100644 --- a/scratch/cataloguefactsbackfill/main.go +++ b/scratch/cataloguefactsbackfill/main.go @@ -64,6 +64,17 @@ var scalarFacts = []string{ var arrayFacts = []string{"providers", "highlights", "nutrients"} +// The structured nutrition panel: a jsonb column the agent team fills, which +// the customer app renders. Its own category because it is neither a scalar +// nor a text[] — stored as a string it would reach the app as an escaped blob +// inside the snapshot instead of an object, and every reader would have to +// decode it twice. +// +// Absent on every brand table today, which costs this fact and nothing else. +// The lines in `nutrients` above are parsed into a panel at read time, so a +// product backfilled before the column exists still shows figures. +var jsonFacts = []string{"nutrition"} + type product struct { Productid int Productbrand string @@ -312,7 +323,7 @@ func factColumnsOf(db *gorm.DB, table string) []string { } var keep []string - for _, c := range append(append([]string{}, scalarFacts...), arrayFacts...) { + for _, c := range append(append(append([]string{}, scalarFacts...), arrayFacts...), jsonFacts...) { if present[c] { keep = append(keep, c) } @@ -332,7 +343,9 @@ func factsFor(db *gorm.DB, table string, cols []string, imageID string) (map[str selects := make([]string, 0, len(cols)) for _, c := range cols { - if isArrayFact(c) { + // jsonb and text[] are both cast to text for the same reason: GORM's + // raw scan cannot land either one in a Go value directly. + if isArrayFact(c) || isJSONFact(c) { selects = append(selects, c+"::text AS "+c) continue } @@ -356,6 +369,21 @@ func factsFor(db *gorm.DB, table string, cols []string, imageID string) (map[str if text == "" { continue } + if isJSONFact(c) { + // Embedded as an object, not as the text the column scanned to. + // Stored as a string it would reach the app as an escaped blob + // inside the snapshot, and every reader would decode it twice. + // + // Unreadable JSON is skipped rather than stored: an absent panel + // falls back to the nutrient lines at read time, a broken one does + // not. + var panel models.NutritionPanel + if err := json.Unmarshal([]byte(text), &panel); err != nil || !panel.HasValues() { + continue + } + facts[c] = panel + continue + } if isArrayFact(c) { values := models.ParsePGArray(text) kept := make([]string, 0, len(values)) @@ -374,6 +402,15 @@ func factsFor(db *gorm.DB, table string, cols []string, imageID string) (map[str return facts, true } +func isJSONFact(name string) bool { + for _, c := range jsonFacts { + if c == name { + return true + } + } + return false +} + func isArrayFact(name string) bool { for _, c := range arrayFacts { if c == name { diff --git a/services/nutritionResponse_test.go b/services/nutritionResponse_test.go new file mode 100644 index 0000000..33b9ca4 --- /dev/null +++ b/services/nutritionResponse_test.go @@ -0,0 +1,148 @@ +package services + +import ( + "encoding/json" + "testing" + + "nearle/models" +) + +/* +What the product screen receives. + +The app renders a nutrition panel off `getproductbyvariant`. These are about the +contract that screen is built on — the exact field names, and what happens for +the overwhelming majority of products, which have no structured nutrition at all +and are not going to for a while. +*/ + +func TestTheStructuredPanelIsUsedWhereItExists(t *testing.T) { + facts := `{"nutrition":{"per":"100g","servingsize":"30g","items":[ + {"name":"Energy","value":520,"unit":"kcal"}, + {"name":"Protein","value":6.5,"unit":"g"}]}}` + + panel := nutritionFromFacts(facts) + if !panel.HasValues() { + t.Fatal("no panel") + } + if panel.Per != "100g" || panel.Servingsize != "30g" { + t.Fatalf("basis lost: per=%q servingsize=%q", panel.Per, panel.Servingsize) + } + if len(panel.Items) != 2 || panel.Items[1].Value != 6.5 { + t.Fatalf("items wrong: %+v", panel.Items) + } +} + +func TestTheOldNutrientLinesStillProduceAPanel(t *testing.T) { + // This is the case that actually matters on day one: every product on the + // platform carries display lines and nothing else. Without this the app + // ships a nutrition screen that is empty for every product in every shop. + panel := nutritionFromFacts(`{"nutrients":["Energy 350kcal","Protein 7g"]}`) + if !panel.HasValues() { + t.Fatal("the lines were not used") + } + if panel.Items[0].Name != "Energy" || panel.Items[0].Value != 350 { + t.Fatalf("got %+v", panel.Items[0]) + } +} + +func TestStructuredBeatsLinesWhenBothArePresent(t *testing.T) { + // The import keeps both — the console shows the lines, the app shows the + // panel. The structured one is the agent team's work and carries a basis, + // so it wins. + facts := `{"nutrients":["Energy 350kcal"], + "nutrition":{"per":"100g","items":[{"name":"Energy","value":520,"unit":"kcal"}]}}` + + panel := nutritionFromFacts(facts) + if panel.Per != "100g" || panel.Items[0].Value != 520 { + t.Fatalf("the derived panel won: %+v", panel) + } +} + +func TestNothingKnownMeansNoPanel(t *testing.T) { + for _, facts := range []string{"", " ", "{}", `{"highlights":["Crunchy"]}`} { + if p := nutritionFromFacts(facts); p != nil { + t.Errorf("%q produced a panel: %+v", facts, p) + } + } +} + +func TestAMalformedSnapshotCostsThePanelAndNothingElse(t *testing.T) { + // One bad blob must not fail the response for the variant group it is in — + // the shopper tapped a product and is owed the screen. + if p := nutritionFromFacts(`{"nutrition":{"items":`); p != nil { + t.Fatalf("read a panel out of broken JSON: %+v", p) + } +} + +func TestAPanelWithNoRowsIsNotSent(t *testing.T) { + // `items: []` renders as an empty box, which a shopper reads as "this food + // has no nutrition" rather than "we do not know". + if p := nutritionFromFacts(`{"nutrition":{"per":"100g","items":[]}}`); p != nil { + t.Fatalf("sent an empty panel: %+v", p) + } +} + +func TestEveryRowInAVariantGroupIsDecorated(t *testing.T) { + // A shopper switching from 500ml to 1L must not watch the nutrition vanish. + rows := []models.Products{ + {Productid: 1, Cataloguefacts: `{"nutrients":["Energy 350kcal"]}`}, + {Productid: 2, Cataloguefacts: `{"nutrition":{"per":"100g","items":[{"name":"Energy","value":700,"unit":"kcal"}]}}`}, + {Productid: 3}, + } + decorateNutrition(rows) + + if !rows[0].Nutrition.HasValues() || !rows[1].Nutrition.HasValues() { + t.Fatal("a sibling was left without its panel") + } + if rows[2].Nutrition != nil { + t.Fatal("invented a panel for a product with no facts") + } +} + +func TestTheWireNamesAreTheOnesTheAppAskedFor(t *testing.T) { + // An API is a promise to a client already written against it. `servingsize` + // is not how the rest of this file would spell it, and it is what was asked + // for, which outranks house style. + rows := []models.Products{{ + Productid: 7093, + Cataloguefacts: `{"nutrition":{"per":"100g","servingsize":"30g","items":[{"name":"Energy","value":520,"unit":"kcal"}]}}`, + }} + decorateNutrition(rows) + + encoded, err := json.Marshal(rows[0]) + if err != nil { + t.Fatalf("marshal: %v", err) + } + var out map[string]any + if err := json.Unmarshal(encoded, &out); err != nil { + t.Fatalf("unmarshal: %v", err) + } + + nutrition, ok := out["nutrition"].(map[string]any) + if !ok { + t.Fatalf("no `nutrition` key on the product: %s", encoded) + } + for _, key := range []string{"per", "servingsize", "items"} { + if _, ok := nutrition[key]; !ok { + t.Errorf("missing %q: %v", key, nutrition) + } + } + item := nutrition["items"].([]any)[0].(map[string]any) + for _, key := range []string{"name", "value", "unit"} { + if _, ok := item[key]; !ok { + t.Errorf("item missing %q: %v", key, item) + } + } +} + +func TestAProductWithNoNutritionHasNoKeyAtAll(t *testing.T) { + // Not `"nutrition": null`. Most products have none, and a null on every row + // of a mobile response is payload spent saying nothing. + encoded, _ := json.Marshal(models.Products{Productid: 1}) + var out map[string]any + _ = json.Unmarshal(encoded, &out) + if _, present := out["nutrition"]; present { + t.Fatalf("an absent panel was sent as a key: %s", encoded) + } +} diff --git a/services/productService.go b/services/productService.go index 23291fc..c9470b0 100644 --- a/services/productService.go +++ b/services/productService.go @@ -251,6 +251,7 @@ func (s *productService) GetProductByVariant(tenantid, variantid, locationid, pr // `productstock` on each row is what the app disables on. A single ungrouped // product comes back as a one-member group, so the app has one code path: // count the rows, and skip the picker when there is only one. + decorateNutrition(result) return result, nil } @@ -374,6 +375,19 @@ func catalogueFactsOf(p *models.CatalogueProduct) map[string]any { putList("highlights", p.Highlights) putList("nutrients", p.Nutrients) + // The structured panel, snapshotted like everything else here. + // + // Kept BESIDE `nutrients` rather than instead of it. The lines are what the + // console has always shown and what a re-scrape can still change; the panel + // is what the app renders. Dropping either would break a reader that exists. + // + // `Nutrition` is already the resolved one — structured where the catalogue + // has it, derived from the lines where it does not — so a product imported + // today carries a panel whether or not the agent team has reached its brand. + if p.Nutrition.HasValues() { + facts["nutrition"] = p.Nutrition + } + return facts } @@ -618,3 +632,63 @@ func (s *productService) PublishProduct(tenantID, productID int, price, taxPerce func (s *productService) UnpublishProduct(tenantID, productID int) (int, error) { return s.repo.UnpublishProduct(tenantID, productID) } + +/* +decorateNutrition unpacks each product's nutrition panel for the app. + +── Why the endpoint does this and the query does not ─────────────────────── + +The panel lives inside `cataloguefacts`, a jsonb blob the import writes to hold +everything the catalogue knew that `products` has no column for. Reading it in +SQL would mean a jsonb path expression inside a query that already carries four +correlated subqueries, and it would have to be repeated in every read that ever +wants nutrition. Unpacking it once, here, keeps the query about the shelf. + +── The two shapes it accepts ─────────────────────────────────────────────── + +`nutrition` is the structured panel and is used wherever it exists. `nutrients` +is the older text[] of display lines — "Energy 350kcal" — which is what every +product on the platform actually carries today, and it is parsed into the same +shape rather than left unreadable by the app. + +So a product shows figures if ANYTHING is known about it, and the field is +absent only when genuinely nothing is. + +── What it will not do ───────────────────────────────────────────────────── + +It does not go back to the catalogue DB for a product whose snapshot is empty. +That is a cross-database lookup on a hot mobile endpoint, per product, on a +screen a shopper is waiting for. Products imported before `cataloguefacts` +existed have empty snapshots — measured: all 15 catalogue-imported products on +tenant 1147 — and the fix for those is the backfill in +`scratch/nutritionbackfill`, run once, not a lookup paid for on every tap. +*/ +func decorateNutrition(products []models.Products) { + for i := range products { + products[i].Nutrition = nutritionFromFacts(products[i].Cataloguefacts) + } +} + +// nutritionFromFacts reads a panel out of one product's catalogue snapshot. +// +// Every failure returns nil rather than an error: the product screen is worth +// drawing without a nutrition panel, and a malformed blob on one row must not +// fail the response for the variant group it belongs to. +func nutritionFromFacts(raw string) *models.NutritionPanel { + if strings.TrimSpace(raw) == "" { + return nil + } + + var facts struct { + Nutrition *models.NutritionPanel `json:"nutrition"` + Nutrients []string `json:"nutrients"` + } + if err := json.Unmarshal([]byte(raw), &facts); err != nil { + return nil + } + + if facts.Nutrition.HasValues() { + return facts.Nutrition + } + return models.NutritionFromLines(facts.Nutrients) +}