diff --git a/api/src/models/item.model.js b/api/src/models/item.model.js index 358e8f481..eafe59a45 100644 --- a/api/src/models/item.model.js +++ b/api/src/models/item.model.js @@ -156,6 +156,17 @@ const itemSchema = new mongoose.Schema( */ nutrition_source: { type: String, trim: true, default: '' }, + /* + * May a customer say how hot they want this one? + * + * Off unless the shop turns it on, dish by dish. Not because chillies look + * silly on a dessert - because a kitchen that batch-cooks its gravy CANNOT + * make one portion mild, and a customer who asked for mild and got hot is + * worse off than one who never asked. Only the kitchen knows which dishes + * it can really vary. See utils/spice-level.js. + */ + spice_choice: { type: Boolean, default: false }, + /* * What is and is not IN the dish: plant based, Jain, gluten free, nut * free, organic, no added sugar. See FOOD_TAGS in utils/dish-facts.js. @@ -277,6 +288,7 @@ class ItemModel { prep_minutes: { type: 'Number', select: true }, nutrition: { type: 'Object', select: true }, nutrition_source: { type: 'String', select: true }, + spice_choice: { type: 'Boolean', select: true }, food_tags: { type: 'Array', select: true }, menu_marks: { type: 'Array', select: true }, isAvailable: { type: 'Boolean', select: true }, diff --git a/api/src/repositories/item.repository.js b/api/src/repositories/item.repository.js index 89a111c8e..9c59166a1 100644 --- a/api/src/repositories/item.repository.js +++ b/api/src/repositories/item.repository.js @@ -1748,6 +1748,8 @@ class ItemRepository extends BaseModel { * Sync replaces whole documents, so all three are written on every * save or the next one deletes them. */ + /* Whether a customer may say how hot they want it. */ + spice_choice: Boolean(data.spice_choice), nutrition: dishFacts.cleanNutrition(data.nutrition), /* Only the one word means anything; everything else is a person. A client that omits it is the item screen, where a person is looking @@ -3971,6 +3973,7 @@ class ItemRepository extends BaseModel { /* What is on the plate and what is in it. The health badges are NOT read - they are derived from these below, so a dish can never carry a claim its own nutrition contradicts. */ + spice_choice: 1, nutrition: 1, nutrition_source: 1, food_tags: 1, @@ -4458,6 +4461,7 @@ class ItemRepository extends BaseModel { prep_minutes: '$prep_minutes', /* What is on the plate and what is in it. Folded into facts and CLAIMS below and do not travel raw. */ + spice_choice: '$spice_choice', nutrition: '$nutrition', nutrition_source: '$nutrition_source', food_tags: '$food_tags', diff --git a/api/src/repositories/sale.repository.js b/api/src/repositories/sale.repository.js index 60e26b9e6..7f9d36f83 100644 --- a/api/src/repositories/sale.repository.js +++ b/api/src/repositories/sale.repository.js @@ -40,6 +40,7 @@ async function withDayparts(shop) { const { notifyOrderAttention } = require('../helpers/order-attention'); const orderApproval = require('../utils/order-approval'); +const spiceLevel = require('../utils/spice-level'); const StockLogsRepository = require('./stock-log.repository'); const { PAYMENT_STATUS } = require('../constants'); const moment = require('moment-timezone'); @@ -8034,6 +8035,9 @@ class SalesRepository { /* See the note at the cancel flow below: this list is what the kitchen ticket is printed from. */ item_description: String(si.item_description || ''), + /* And for the same reason: a level stored on the sale and absent + from the change record never reaches the paper. */ + spice_level: spiceLevel.levelOf(si.spice_level), }; }) .filter((it) => it.item_id && it.item_quantity > 0); @@ -8560,6 +8564,13 @@ class SalesRepository { on the sale and printed on the ticket, and never shown to the person deciding whether to accept the order. */ note: item.item_description || '', + /* + * And how hot they asked for it, for the same reason. This is the + * screen where an order is refused, and "we cannot make that one + * mild" is a reason to refuse it - which nobody can act on if the + * request is only visible on the paper in the kitchen. + */ + spice: spiceLevel.levelOf(item.spice_level), })), total: Number(row.total) || 0, delivery_fee: Number(row.delivery_fee) || 0, @@ -9078,6 +9089,7 @@ class SalesRepository { item_name: String(ex.item_name || ''), item_quantity: qty, item_description: String(ex.item_description || ''), + spice_level: spiceLevel.levelOf(ex.spice_level), process: 'cancel', item_code: String(ex.item_sku || ''), unit: String(ex.item_unit || 'qty'), @@ -9120,8 +9132,10 @@ class SalesRepository { quantity: parseFloat(ex.item_quantity || 0), name: String(ex.item_name || ''), /* Carried so a REMOVED line can still say which one it was. Two of - the same dish on one table are told apart by the note. */ + the same dish on one table are told apart by the note, and by how + hot each of them was to be. */ description: String(ex.item_description || ''), + spice_level: spiceLevel.levelOf(ex.spice_level), item_code: String(ex.item_sku || ''), price: parseFloat(ex.item_price || 0), unit: String(ex.item_unit || 'qty'), @@ -9167,6 +9181,11 @@ class SalesRepository { ...(item.item_description != null ? { item_description: String(item.item_description) } : {}), + /* A KOT line the catalogue no longer holds still belongs to + somebody who may have changed their mind about the chillies. */ + ...(item.spice_level != null + ? { spice_level: spiceLevel.levelOf(item.spice_level) } + : {}), }; incomingProductIds.push(productId); } @@ -9191,6 +9210,16 @@ class SalesRepository { /* From the request first: an amendment carries the note the person just typed, and the stored copy is the one before it. */ item_description: String(item.item_note || item.item_description || ''), + /* + * Same order for the spice level, and the stored line is read + * through existingIndex rather than oldItemsData because that map + * has had this id deleted from it a few lines above. + */ + spice_level: spiceLevel.levelOf( + item.spice_level != null + ? item.spice_level + : (updatedItems[existingIndex[productId]] || {}).spice_level + ), process: changeProcess, item_code: String(itemDoc.itemid || ''), unit: String(itemDoc.item_unit || itemDoc.unit || 'qty'), @@ -9236,6 +9265,11 @@ class SalesRepository { }; if (item.item_description) updatedItems[i].item_description = String(item.item_description); + /* The ticket is printed from the change record above; THIS is what + the customer sees back on their own order and what a shop counts + later, so a change of mind has to land on both. */ + if (item.spice_level != null) + updatedItems[i].spice_level = spiceLevel.levelOf(item.spice_level); } else { const itemQuantity = qty; const sellingPrice = price; @@ -9287,6 +9321,7 @@ class SalesRepository { tax_amount: taxAmount, tax_fields: itemDoc.tax_fields || [], item_description: String(item.item_description || itemDoc.description || ''), + spice_level: spiceLevel.levelOf(item.spice_level), track_inventory: itemDoc.track_inventory || false, negative_stock: itemDoc.negative_stock || false, }); @@ -9302,6 +9337,7 @@ class SalesRepository { item_name: String(remItemData.name || ''), item_quantity: remQty, item_description: String(remItemData.description || ''), + spice_level: spiceLevel.levelOf(remItemData.spice_level), process: 'cancel', item_code: String(remItemData.item_code || ''), unit: String(remItemData.unit || 'qty'), @@ -9688,6 +9724,20 @@ class SalesRepository { item_description: String(item.item_note || item.item_description || '') .trim() .slice(0, 200), + /* + * HOW HOT, AS A NUMBER AND NOT AS A SENTENCE. + * + * The obvious build appends "less spicy" to the note above, and it is + * wrong twice: a customer reading a Tamil menu writes Tamil, so the + * ticket carries prose the kitchen may misread, and prose cannot be + * counted afterwards. A level prints identically on every ticket + * whatever language the order was placed in, and a shop can learn that + * four orders in ten ask for mild. + * + * levelOf refuses anything that is not 1, 2 or 3, so a device sending + * nonsense gets no promise made about somebody's food. + */ + spice_level: spiceLevel.levelOf(item.spice_level), // receipt-facing fields item_base_price: round(baseUnitPrice), item_quantity: qty, @@ -9821,6 +9871,7 @@ class SalesRepository { name: String(line.item_name || line.name || ''), quantity: Number(line.item_quantity != null ? line.item_quantity : line.quantity || 0), note: String(line.item_description || ''), + spice: spiceLevel.levelOf(line.spice_level), total: Number(line.total != null ? line.total : line.item_total || 0), })), total: Number(order.total != null ? order.total : order.sales_total || 0), diff --git a/api/src/utils/spice-level.js b/api/src/utils/spice-level.js new file mode 100644 index 000000000..371ce7b65 --- /dev/null +++ b/api/src/utils/spice-level.js @@ -0,0 +1,104 @@ +'use strict'; +/* + * How hot, when the kitchen can decide. + * + * Owner: "when user order if food is speci food. we can have simple option + * like low, medium high with number chilly image like one, two, three chilli + * icons user can customize easy. we can add those into kitchen note." + * + * Spice is the thing an Indian restaurant is asked to change more often than + * anything else on its menu, and until now the only way to ask was typing it + * into the free-text note - in whatever words, in whatever language, for a + * kitchen that then has to read prose off a ticket at speed. + * + * THREE LEVELS, AND NOT CHOOSING IS ONE TOO. + * + * Mild, medium, spicy. "However the kitchen makes it" is simply not picking, + * which is why there is no fourth button for it: a default that has to be + * selected is a question, and this has to stay one tap. Somebody who wants no + * chilli at all still has the note, which has not gone anywhere. + * + * WHY IT IS A FIELD AND NOT A SENTENCE IN THE NOTE. + * + * The obvious build is to append "less spicy" to the kitchen note, and it is + * wrong twice over. A customer reading a Tamil menu writes Tamil, and the + * ticket then carries prose the kitchen may misread; a LEVEL prints the same + * on every ticket whatever language the order was placed in. And a number can + * be counted afterwards - a shop can learn that four orders in ten ask for + * mild, and cook accordingly - which no amount of free text will ever tell it. + * + * NOT EVERY DISH. See item.spice_choice: the shop says which dishes take one. + * That is not about chillies looking silly on a gulab jamun. A kitchen that + * batch-cooks its gravy CANNOT make one portion mild, and a customer who asked + * for mild and got hot is worse off than one who never asked - so the offer + * exists only where the kitchen can honour it, and only the kitchen knows + * where that is. + * + * THE TICKET SAYS IT IN ASCII. The chillies are drawn on the customer's screen + * and nowhere else: escpos-receipt.js puts every character through ascii() and + * latin1, so an emoji on a kitchen ticket prints as a question mark or as + * nothing. The paper gets the word and the count instead, and the count is + * there for a cook who does not read the word. + * + * NO DATABASE IMPORTS. + */ + +/* Stored as the number of chillies, because that is what the customer taps + and what the ticket counts. 0 means nobody chose. */ +const SPICE = Object.freeze({ + NOT_SAID: 0, + MILD: 1, + MEDIUM: 2, + SPICY: 3, +}); + +const LEVELS = Object.freeze([SPICE.MILD, SPICE.MEDIUM, SPICE.SPICY]); + +/* The words the paper prints. English, because the ticket is the kitchen's + and the kitchen is the shop's; the customer's own screen says it in the + customer's language, from the ordering dictionary. */ +const WORDS = Object.freeze({ + [SPICE.MILD]: 'MILD', + [SPICE.MEDIUM]: 'MEDIUM', + [SPICE.SPICY]: 'SPICY', +}); + +/** + * A level somebody actually chose, or nothing. + * + * Anything that is not one of the three reads as NOT SAID, deliberately: a + * value nobody recognises must never become a promise about somebody's food. + */ +function levelOf(value) { + /* + * A NUMBER OR THE TEXT OF ONE, AND NOTHING ELSE. + * + * Number(true) is 1, so a client sending spice_level: true would have had + * MILD printed on a real ticket for a request nobody made. Caught by the + * test rather than by anybody reading this, which is the point of the test. + * A string is still allowed: a handset that stringifies its form sends "2", + * and refusing that would silently drop the request. + */ + if (typeof value !== 'number' && typeof value !== 'string') return SPICE.NOT_SAID; + const n = Number(value); + return LEVELS.includes(n) ? n : SPICE.NOT_SAID; +} + +/** Does this dish offer the choice at all? */ +function offersChoice(item) { + return Boolean(item && item.spice_choice === true); +} + +/** + * What the kitchen ticket says, or '' when nobody asked. + * + * The count is not decoration. A cook who does not read English still reads + * 2 of 3, and a ticket that printed only a word would be useless to them. + */ +function ticketLine(value) { + const level = levelOf(value); + if (!level) return ''; + return 'SPICE: ' + WORDS[level] + ' (' + level + ' of ' + LEVELS.length + ')'; +} + +module.exports = { SPICE, LEVELS, WORDS, levelOf, offersChoice, ticketLine }; diff --git a/api/tests/unit/models/item.model.test.js b/api/tests/unit/models/item.model.test.js index e7b939f35..3521d70f1 100644 --- a/api/tests/unit/models/item.model.test.js +++ b/api/tests/unit/models/item.model.test.js @@ -791,7 +791,7 @@ describe('Item.LegacyItemModel › class identity', () => { // deliberately NOT fields, so a dish can never store one). // + nutrition_source (who said so: a person, or a machine that guessed; // dish-facts publishes nothing derived from an estimate). - expect(Object.keys(LegacyItemModel.fields)).toHaveLength(77); + expect(Object.keys(LegacyItemModel.fields)).toHaveLength(78); expect(LegacyItemModel.fields).toEqual( expect.objectContaining({ /* Named as well as counted: a count alone passes if one field is diff --git a/api/tests/unit/services/customer-order.service.test.js b/api/tests/unit/services/customer-order.service.test.js index 35de0e6da..58ecd398b 100644 --- a/api/tests/unit/services/customer-order.service.test.js +++ b/api/tests/unit/services/customer-order.service.test.js @@ -279,8 +279,10 @@ describe('the order, read back by the phone that placed it', () => { can_change: false, why_not: 'already_paid', }); + /* `spice` is 0 because nobody chose one: the level is only ever a number + the customer tapped, and a dish nobody asked about carries none. */ expect(out.data.items).toEqual([ - { item_id: 'm1', name: 'Chicken Biryani', quantity: 2, note: '', total: 660 }, + { item_id: 'm1', name: 'Chicken Biryani', quantity: 2, note: '', spice: 0, total: 660 }, ]); }); diff --git a/api/tests/unit/the-kitchen-reads-how-hot.test.js b/api/tests/unit/the-kitchen-reads-how-hot.test.js new file mode 100644 index 000000000..8b9fbf1bc --- /dev/null +++ b/api/tests/unit/the-kitchen-reads-how-hot.test.js @@ -0,0 +1,138 @@ +'use strict'; + +/* + * "Two chillies" reaches the kitchen, by the road "less spicy" already took. + * + * Owner: "when user order if food is speci food. we can have simple option + * like low, medium high with number chilly image like one, two, three chilli + * icons user can customize easy. we can add those into kitchen note." + * + * THE ROAD IS THE POINT, and the-kitchen-reads-less-spicy.test.js is beside + * this file because it paid for the map. The kitchen ticket is NOT printed + * from `sale.items`: the poller builds its print jobs out of `changes[].items`, + * the record of what changed. The note was stored perfectly on the sale, both + * ticket builders printed it, and it still never reached paper - because three + * separate places build that change list and all three left it out. + * + * A spice level stored on the sale and absent from the change record would be + * the same bug, shipped again, in the same week the last one was fixed. So + * this asserts the RULE rather than one spelling: every change list that + * carries the customer's note carries the level beside it. + */ + +const fs = require('fs'); +const path = require('path'); + +const ROOT = path.join(__dirname, '..', '..'); +const REPO = fs.readFileSync(path.join(ROOT, 'src', 'repositories', 'sale.repository.js'), 'utf8'); + +/** + * Every object literal in the repository that carries the customer's note. + * + * Found by the note rather than by function name: the note is on the sale + * line, on three change lists and on the map a removed line is rebuilt from, + * and those are exactly the places a level has to travel too. Anchored this + * way, a fourth one added tomorrow is caught the day it appears. + */ +function placesCarryingTheNote() { + const out = []; + const re = /(item_description|description):\s*String\(/g; + let m; + while ((m = re.exec(REPO))) { + /* + * The literal this line sits in: back to its opening brace, forward to + * the matching close. Counted rather than regexed, because these objects + * hold nested ternaries and template strings. + * + * And out again while the literal is a ONE-LINE fragment. Two of these + * sites are conditional spreads - `...(x != null ? { item_description: … + * } : {})` - whose own braces enclose nothing but the note; the level + * sits beside them in the object those spreads build, which is the object + * this test means by "place". + */ + let open = m.index; + let close; + do { + open = REPO.lastIndexOf('{', open - 1); + let depth = 0; + close = open; + while (close < REPO.length) { + if (REPO[close] === '{') depth += 1; + else if (REPO[close] === '}') { + depth -= 1; + if (depth === 0) break; + } + close += 1; + } + } while (open > 0 && !REPO.slice(open, close).includes('\n')); + out.push({ at: m.index, body: REPO.slice(open, close + 1) }); + } + return out; +} + +test('every place that carries the note carries the level too', () => { + /* + * The whole test. A level that stops at any one of these is a level the + * kitchen never sees, and nothing anywhere fails: the order goes through, + * the ticket prints, and the food comes out wrong. + */ + const places = placesCarryingTheNote(); + expect(places.length).toBeGreaterThanOrEqual(5); + + const without = places.filter((p) => !/spice_level:/.test(p.body)); + expect(without.map((p) => REPO.slice(p.at, p.at + 60))).toEqual([]); +}); + +test('an amendment prefers the level just chosen over the stored one', () => { + /* + * Same rule as the note, for the same reason: the request carries what the + * person has only now said and the stored copy is what they said before. + * Reading the stored one first would quietly ignore a change of mind. + * + * Read through existingIndex rather than oldItemsData, because that map has + * had this id deleted from it a few lines above - which is the sort of thing + * that only shows up when somebody actually changes an order. + */ + expect(REPO).toMatch(/item\.spice_level != null\s*\?\s*item\.spice_level/); + expect(REPO).toMatch(/updatedItems\[existingIndex\[productId\]\] \|\| \{\}\)\.spice_level/); +}); + +test('every level is cleaned on the way in, never trusted', () => { + /* + * This arrives as JSON from a customer's own phone over the open internet. + * levelOf refuses anything that is not 1, 2 or 3 - a stray true included, + * because Number(true) is 1 and would have printed MILD on real paper. + */ + const raw = REPO.match(/spice_level:\s*([^\n]*)/g) || []; + expect(raw.length).toBeGreaterThanOrEqual(6); + for (const line of raw) { + expect(line).toMatch(/spiceLevel\.levelOf\(/); + } + expect(REPO).toMatch(/require\('\.\.\/utils\/spice-level'\)/); +}); + +test('the shop sees how hot before it decides whether to accept', () => { + /* + * The approval queue is where an order is REFUSED, and "we cannot make that + * one mild" is a reason to refuse it. A request visible only on the paper in + * the kitchen reaches the person who has to cook it and not the person who + * has to agree to it. + */ + expect(REPO).toMatch(/spice: spiceLevel\.levelOf\(item\.spice_level\)/); +}); + +test('the customer can see it back on their own order', () => { + /* Somebody who asked for mild and is waiting should be able to check that + the restaurant heard them, without ringing the counter. */ + expect(REPO).toMatch(/spice: spiceLevel\.levelOf\(line\.spice_level\)/); +}); + +test('the reason is written where the next person will look', () => { + /* + * The note bug was invisible because every layer worked and the ticket was + * still wrong. Whoever adds the next per-line field deserves to be told, in + * the file, that the ticket is printed from the CHANGE record. + */ + expect(REPO).toMatch(/THE NOTE TRAVELS WITH THE ITEM/); + expect(REPO).toMatch(/never reaches the paper/); +}); diff --git a/api/tests/unit/utils/spice-level.test.js b/api/tests/unit/utils/spice-level.test.js new file mode 100644 index 000000000..2f16937b3 --- /dev/null +++ b/api/tests/unit/utils/spice-level.test.js @@ -0,0 +1,99 @@ +'use strict'; + +/* + * How hot, and what may be said about it. + * + * Owner: "when user order if food is speci food. we can have simple option + * like low, medium high with number chilly image like one, two, three chilli + * icons user can customize easy. we can add those into kitchen note." + * + * The rule this file guards is small and worth stating plainly: a level that + * nobody chose must never become a level somebody did. Everything a customer + * sends arrives as JSON over the open internet, so "2" from a phone, 2 from a + * handset, 7 from a broken client and "medium" from somebody experimenting + * all land in the same field, and exactly three of those may reach a cook. + */ + +const spice = require('../../../src/utils/spice-level'); + +describe('a level somebody actually chose', () => { + test('the three the customer can tap are the three that count', () => { + expect(spice.levelOf(1)).toBe(1); + expect(spice.levelOf(2)).toBe(2); + expect(spice.levelOf(3)).toBe(3); + }); + + test('a number in a string is still that number, because JSON is JSON', () => { + /* The ordering page sends a number; a handset that stringifies its form + sends "2". Refusing the second would silently drop the request. */ + expect(spice.levelOf('2')).toBe(2); + }); + + test('anything else is nobody asked', () => { + /* + * Not an error, and not a guess. A value nobody recognises must not become + * a promise about somebody's food, and it must not stop the order either: + * the dish is still wanted, the request simply cannot be honoured. + */ + [0, 4, -1, 99, '', null, undefined, NaN, 'medium', {}, [], true].forEach((bad) => { + expect(spice.levelOf(bad)).toBe(0); + }); + }); +}); + +describe('which dishes offer the choice', () => { + test('only a dish the shop ticked', () => { + /* + * Off by default and per dish, because a kitchen that batch-cooks its + * gravy cannot make one portion mild - and a customer who asked for mild + * and got hot is worse off than one who never asked. + */ + expect(spice.offersChoice({ spice_choice: true })).toBe(true); + expect(spice.offersChoice({ spice_choice: false })).toBe(false); + expect(spice.offersChoice({})).toBe(false); + expect(spice.offersChoice(null)).toBe(false); + }); + + test('a truthy value that is not true does not count', () => { + /* Settings have arrived as the string "false" in this codebase before, + and read as ON through a loose check. A boolean field is a boolean. */ + expect(spice.offersChoice({ spice_choice: 'false' })).toBe(false); + expect(spice.offersChoice({ spice_choice: 1 })).toBe(false); + }); +}); + +describe('what the paper says', () => { + test('the word and the count, in that order', () => { + expect(spice.ticketLine(1)).toBe('SPICE: MILD (1 of 3)'); + expect(spice.ticketLine(2)).toBe('SPICE: MEDIUM (2 of 3)'); + expect(spice.ticketLine(3)).toBe('SPICE: SPICY (3 of 3)'); + }); + + test('nobody asked, nothing printed', () => { + /* A ticket line reading "SPICE: NOT SAID" is a line every cook learns to + skip, on every ticket, forever. */ + expect(spice.ticketLine(0)).toBe(''); + expect(spice.ticketLine(undefined)).toBe(''); + expect(spice.ticketLine('hot')).toBe(''); + }); + + test('every character survives a thermal printer', () => { + /* + * escpos-receipt.js puts every character through ascii() and latin1. A + * chilli emoji on this line would reach the kitchen as a question mark or + * as nothing, which is why the chillies are drawn on the customer's phone + * and the paper gets a word and a number instead. + */ + for (const level of spice.LEVELS) { + const line = spice.ticketLine(level); + + expect(line).toMatch(/^[\x20-\x7e]+$/); + } + }); + + test('the count is there for a cook who does not read the word', () => { + /* The whole reason this is a level and not a sentence: 2 of 3 means the + same thing to a cook in any language. */ + expect(spice.ticketLine(2)).toContain('2 of 3'); + }); +}); diff --git a/frontend/modules/items_write.html b/frontend/modules/items_write.html index da4210e77..561d5983f 100644 --- a/frontend/modules/items_write.html +++ b/frontend/modules/items_write.html @@ -1220,6 +1220,19 @@