From 88239424f355ece8cfc7c52e53bfc22f95a4c73f Mon Sep 17 00:00:00 2001 From: Adam Moussa Date: Thu, 30 Jul 2026 18:53:04 -0400 Subject: [PATCH] fix(form): resolve remaining review nitpicks --- src/server/templates/admin.js | 21 +++++++++++++-------- src/server/templates/form.css | 4 ++-- tests/fixtures/menu.json | 12 +++++++++++- tests/test_generate_form.py | 25 ++++--------------------- 4 files changed, 30 insertions(+), 32 deletions(-) diff --git a/src/server/templates/admin.js b/src/server/templates/admin.js index de4e676..fee9ff4 100644 --- a/src/server/templates/admin.js +++ b/src/server/templates/admin.js @@ -115,10 +115,10 @@ function buildEditActions(email) { return wrap; } -function buildViewActions(idx, email, name) { +function buildViewActions(email, name) { const wrap = document.createElement('div'); wrap.className = 'admin-actions'; - const editBtn = buildActionButton('Edit', 'start-edit', { index: String(idx), email }); + const editBtn = buildActionButton('Edit', 'start-edit', { email }); editBtn.className = 'btn btn-sm btn-edit'; const deleteBtn = buildActionButton('Delete', 'delete', { email, name }); deleteBtn.className = 'btn btn-sm btn-delete'; @@ -229,7 +229,7 @@ function renderAdminTable(data) { if (isEditing) { actionsTd.appendChild(buildEditActions(order.employee_email)); } else { - actionsTd.appendChild(buildViewActions(idx, order.employee_email, order.employee_name)); + actionsTd.appendChild(buildViewActions(order.employee_email, order.employee_name)); } tr.appendChild(totalTd); @@ -267,7 +267,8 @@ function downloadOrderList() { const names = Object.keys(totals).sort((a, b) => a.localeCompare(b)); const esc = v => { const s = String(v); - return /[",\n]/.test(s) ? '"' + s.replace(/"/g, '""') + '"' : s; + const safe = /^[=+\-@\t\r]/.test(s) ? "'" + s : s; + return /[",\n]/.test(safe) ? '"' + safe.replace(/"/g, '""') + '"' : safe; }; const rows = [['Item', 'Quantity']].concat(names.map(n => [n, totals[n]])); const csv = rows.map(r => r.map(esc).join(',')).join('\r\n'); @@ -307,9 +308,12 @@ function downloadSummaryPdf() { .finally(() => { if (btn) btn.disabled = false; }); } -function adminStartEdit(idx, email) { +function adminStartEdit(email) { fetch(ADMIN_URL + '?week=' + encodeURIComponent(currentAdminWeek), { headers: adminHeaders() }) - .then(r => r.json()) + .then(r => { + if (!r.ok) throw new Error('HTTP ' + r.status); + return r.json(); + }) .then(data => { lastAdminData = data; const order = data.orders.find(o => o.employee_email === email); @@ -318,7 +322,8 @@ function adminStartEdit(idx, email) { editQuantities = {}; order.items.forEach(i => { editQuantities[i.name] = i.quantity; }); renderAdminTable(data); - }); + }) + .catch(() => alert('Failed to load this order for editing.')); } function adminCancelEdit() { @@ -376,7 +381,7 @@ function handleAdminPanelClick(e) { if (btn) { const action = btn.dataset.adminAction; if (action === 'start-edit') { - adminStartEdit(parseInt(btn.dataset.index, 10), btn.dataset.email); + adminStartEdit(btn.dataset.email); } else if (action === 'delete') { adminDeleteOrder(btn.dataset.email, btn.dataset.name || ''); } else if (action === 'save-edit') { diff --git a/src/server/templates/form.css b/src/server/templates/form.css index b4c73b7..f4c02c6 100644 --- a/src/server/templates/form.css +++ b/src/server/templates/form.css @@ -341,7 +341,7 @@ header p { font-size: 0.85rem; opacity: 0.8; } @media (max-width: 480px) { .meal-card { display: grid; - grid-template-columns: 80px minmax(0, 1fr); + grid-template-columns: auto minmax(0, 1fr); align-items: start; } .qty-control { @@ -469,7 +469,7 @@ header p { font-size: 0.85rem; opacity: 0.8; } font-size: 0.9rem; } -.admin-toolbar .back-btn { margin: 0; padding: 8px 16px; font-size: 0.85rem; margin-top: 0; } +.admin-toolbar .back-btn { margin: 0; padding: 8px 16px; font-size: 0.85rem; } @media (max-width: 600px) { .admin-toolbar { width: 100%; } diff --git a/tests/fixtures/menu.json b/tests/fixtures/menu.json index ee0671b..921739b 100644 --- a/tests/fixtures/menu.json +++ b/tests/fixtures/menu.json @@ -1,6 +1,6 @@ { "scraped_at": "2026-07-27T12:00:00+00:00", - "meal_count": 4, + "meal_count": 5, "meals": [ { "name": "Chicken Bowl", @@ -45,6 +45,16 @@ "dietary_tags": [], "image_url": "", "is_new": false + }, + { + "name": "", + "price": 9.0, + "calories": "300", + "protein": "10g", + "description": "Hostile script comment", + "dietary_tags": [], + "image_url": "", + "is_new": false } ] } \ No newline at end of file diff --git a/tests/test_generate_form.py b/tests/test_generate_form.py index 4c4bfc8..05d4b93 100644 --- a/tests/test_generate_form.py +++ b/tests/test_generate_form.py @@ -138,7 +138,6 @@ class TestGenerateFormStructural: def test_no_onclick_admin_data_embedding(self): html = _render(google=True) assert 'onclick="admin' not in html - assert 'onclick="admin' not in html # Static markup must not interpolate runtime data into onclick assert not re.search(r'onclick="[^"]*\$\{', html) @@ -162,6 +161,7 @@ class TestGenerateFormStructural: assert "Luis' Lomo Saltado" in names assert 'Meal with "quotes"' in names assert "" in names + assert "" in names def test_meal_card_template_and_qty_input(self): html = _render() @@ -187,14 +187,10 @@ class TestGenerateFormStructural: class TestGenerateFormPlaywright: - @pytest.fixture(scope="class") + @pytest.fixture def browser_page(self, tmp_path_factory): - try: - from playwright.sync_api import sync_playwright - except ImportError as exc: # pragma: no cover - raise RuntimeError( - "playwright package is required; install requirements.txt" - ) from exc + sync_api = pytest.importorskip("playwright.sync_api") + sync_playwright = sync_api.sync_playwright html = _render(google=False, api_key="") # Stub status/roster so the page does not hang on network; roster fallback @@ -285,19 +281,6 @@ class TestGenerateFormPlaywright: "() => document.querySelector('[data-meal-index=\"0\"] .qty').value === '0'" ) assert qty.input_value() == "0" - # Total may still include other meals from prior test if class-scoped page - # is shared — set all known cards to 0 then set one meal to 2 via type-in. - page.evaluate( - """() => { - document.querySelectorAll('[data-meal-index] .qty').forEach((input) => { - input.value = '0'; - input.dispatchEvent(new Event('input', { bubbles: true })); - }); - }""" - ) - page.wait_for_function( - "() => document.getElementById('count-display').textContent === '0 meals'" - ) qty.fill("2") qty.blur() page.wait_for_function(