worker: atomic quote claim, pipeline-wide revenue, image validation on update, crawler grid order

src/admin.js

@@ -610,12 +610,34 @@ export async function updateProduct(env, id, body) {
   if ("personalise_required" in body) {
     put("personalise_required", body.personalise_required ? 1 : 0);
   }
-  if ("image" in body) {
-    const image = clip(body.image, MAXLEN.image);
-    if (!image) return bad("Image path cannot be empty.");
-    put("image", image);
+  // Same check as createProduct: the client names a file, the manifest decides
+  // whether it exists. This path used to accept any string — an external URL,
+  // a traversal, a typo — and the shop would render whatever it was told.
+  if ("image" in body || "images" in body) {
+    const manifest = await readManifest(env);
+    if (!manifest) return bad("Image manifest unavailable — run `npm run images` and deploy.", 503);
+    const known = new Set(manifest.images.map((i) => i.file));
+
+    if ("image" in body) {
+      const image = clip(body.image, MAXLEN.image);
+      if (!image) return bad("Image path cannot be empty.");
+      const named = image.replace(/^.*\//, "");
+      if (!known.has(named) || /^https?:|^\/\/|\.\./i.test(image)) {
+        return bad("That image is not in assets/images. Push the photo, run `npm run images`, and try again.");
+      }
+      put("image", `assets/images/${named}`);
+    }
+    if ("images" in body) {
+      const extras = [];
+      for (const part of clip(body.images, MAXLEN.images).split(",")) {
+        const f = part.trim().replace(/^.*\//, "");
+        if (!f) continue;
+        if (!known.has(f)) return bad(`Extra image "${f}" is not in assets/images.`);
+        extras.push(`assets/images/${f}`);
+      }
+      put("images", extras.join(","));
+    }
   }
-  if ("images" in body) put("images", clip(body.images, MAXLEN.images));
   if ("category" in body) put("category", clip(body.category, MAXLEN.category));
   if ("visible" in body) put("visible", body.visible ? 1 : 0);
   // Leads the catalogue. A toggle, so coerced rather than validated — there is
@@ -1080,9 +1102,13 @@ export async function refundOrder(env, id, body) {
 
 // ── dashboard summary ─────────────────────────────────────────────
 export async function stats(env) {
+  // Every stage from paid onwards is money that has arrived. This counted only
+  // 'paid' and 'shipped', so the moment an order was marked "in production" it
+  // fell out of the revenue figure and came back when it shipped — the number on
+  // the dashboard went DOWN as work progressed.
   const paid = await env.DB.prepare(
     `SELECT COUNT(*) AS orders, COALESCE(SUM(total_paise),0) AS revenue
-       FROM orders WHERE status IN ('paid','shipped')`
+       FROM orders WHERE status IN ('paid','in_production','ready','shipped','delivered')`
   ).first();
   const pending = await env.DB.prepare(
     `SELECT COUNT(*) AS n FROM orders WHERE status = 'pending'`

src/index.js

@@ -167,10 +167,14 @@ export default {
         && (request.method === "GET" || request.method === "HEAD")) {
       const page = await env.ASSETS.fetch(request);
       try {
+        // The SAME order listProducts() in shop.js produces, or the grid a
+        // crawler indexes (and a visitor sees for the first moment) is a
+        // different grid from the one main.js draws a second later.
         const [{ results }, promo] = await Promise.all([
           env.DB.prepare(
-            `SELECT slug, name, description, price_paise, image
-               FROM products WHERE visible = 1 ORDER BY sort ASC, name ASC`
+            `SELECT slug, name, description, price_paise, image, pinned
+               FROM products WHERE visible = 1
+              ORDER BY pinned DESC, created_at DESC, (sort = 0), sort ASC, name ASC`
           ).all(),
           // Non-fatal: a banner that fails to render server-side just appears
           // the old way, from /api/products, once main.js runs.
@@ -179,8 +183,11 @@ export default {
             return null;
           }),
         ]);
+        const ordered = (results || []).slice().sort((a, b) =>
+          Number(Boolean(b.pinned)) - Number(Boolean(a.pinned)) ||
+          Number(!(a.price_paise > 0)) - Number(!(b.price_paise > 0)));
 
-        const rendered = rewriteHome(env, page, results || [], url, promo);
+        const rendered = rewriteHome(env, page, ordered, url, promo);
 
         // Edge-cached, because this added a D1 query to the hot path.
         //

src/orders.js

@@ -400,35 +400,54 @@ async function handleQuotePaid(env, ctx, evt) {
   const orderId = uid();
   const orderReceipt = "AP-" + orderId.replace(/-/g, "").slice(0, 8).toUpperCase();
   const t = now();
+  const priorStatus = quote.status;
+
+  // CLAIM THE QUOTE FIRST, conditionally. The read above and the writes below
+  // are two round trips, and two deliveries of the same payment — Razorpay
+  // retries, and the event-id header is not guaranteed — could both read
+  // order_id = NULL and both build an order. `AND order_id IS NULL` makes the
+  // claim atomic: exactly one delivery changes a row; the other sees 0 and
+  // stops, before it has created anything.
+  const claim = await env.DB.prepare(
+    `UPDATE quotes SET status = 'paid', order_id = ?, updated_at = ?
+      WHERE id = ? AND order_id IS NULL`
+  ).bind(orderId, t, quote.id).run();
+  if (claim.meta?.changes === 0) return;
 
   // The address is deliberately blank: a quote request never asks for one. The
   // dashboard flags this order as needing an address, and Aswin is already in an
   // email thread with the customer by the time it is paid.
-  await env.DB.batch([
-    env.DB.prepare(
-      `INSERT INTO orders (id, receipt, rzp_order_id, rzp_payment_id, status,
-                           subtotal_paise, shipping_paise, total_paise, currency,
-                           delivery, cust_name, cust_email, cust_phone,
-                           notes, created_at, paid_at)
-       VALUES (?,?,?,?,'paid',?,0,?,'INR','ship',?,?,?,?,?,?)`
-    ).bind(orderId, orderReceipt, rzpOrder.id || payment.order_id || null,
-           payment.id || null, paid, paid,
-           quote.cust_name, quote.cust_email, quote.cust_phone,
-           `Quotation ${quote.receipt}`, t, t),
-
-    // product_id NULL — the column is nullable for exactly this: a line that
-    // never came from the catalogue.
-    env.DB.prepare(
-      `INSERT INTO order_items
-         (id, order_id, product_id, name, price_paise, qty, personalisation, pos)
-       VALUES (?,?,NULL,?,?,1,?,0)`
-    ).bind(uid(), orderId, `Custom print — ${quote.receipt}`, paid,
-           String(quote.description || "").slice(0, 500)),
-
-    env.DB.prepare(
-      `UPDATE quotes SET status = 'paid', order_id = ?, updated_at = ? WHERE id = ?`
-    ).bind(orderId, t, quote.id),
-  ]);
+  try {
+    await env.DB.batch([
+      env.DB.prepare(
+        `INSERT INTO orders (id, receipt, rzp_order_id, rzp_payment_id, status,
+                             subtotal_paise, shipping_paise, total_paise, currency,
+                             delivery, cust_name, cust_email, cust_phone,
+                             notes, created_at, paid_at)
+         VALUES (?,?,?,?,'paid',?,0,?,'INR','ship',?,?,?,?,?,?)`
+      ).bind(orderId, orderReceipt, rzpOrder.id || payment.order_id || null,
+             payment.id || null, paid, paid,
+             quote.cust_name, quote.cust_email, quote.cust_phone,
+             `Quotation ${quote.receipt}`, t, t),
+
+      // product_id NULL — the column is nullable for exactly this: a line that
+      // never came from the catalogue.
+      env.DB.prepare(
+        `INSERT INTO order_items
+           (id, order_id, product_id, name, price_paise, qty, personalisation, pos)
+         VALUES (?,?,NULL,?,?,1,?,0)`
+      ).bind(uid(), orderId, `Custom print — ${quote.receipt}`, paid,
+             String(quote.description || "").slice(0, 500)),
+    ]);
+  } catch (e) {
+    // Release the claim so Razorpay's retry can convert the quote, rather than
+    // leaving it marked paid and pointing at an order that was never written.
+    await env.DB.prepare(
+      `UPDATE quotes SET status = ?, order_id = NULL, updated_at = ?
+        WHERE id = ? AND order_id = ?`
+    ).bind(priorStatus, now(), quote.id, orderId).run().catch(() => {});
+    throw e;
+  }
 
   const order = await env.DB.prepare(`SELECT * FROM orders WHERE id = ?`).bind(orderId).first();
   const { results: items } = await env.DB.prepare(

test/admin.mjs

@@ -349,7 +349,11 @@ function makeDB(seed = {}) {
       return { meta: { changes: o ? 1 : 0 } };
     }
     if (s.startsWith("SELECT COUNT(*) AS orders, COALESCE(SUM(total_paise),0) AS revenue")) {
-      const rows = db.orders.filter((o) => ["paid", "shipped"].includes(o.status));
+      // Parse the IN (...) list out of the SQL rather than hardcoding it, so
+      // the fake cannot agree with the test while disagreeing with the query.
+      const list = (s.match(/status IN \(([^)]*)\)/) || [])[1] || "";
+      const statuses = list.split(",").map((x) => x.trim().replace(/^'|'$/g, ""));
+      const rows = db.orders.filter((o) => statuses.includes(o.status));
       return { first: { orders: rows.length, revenue: rows.reduce((n, o) => n + o.total_paise, 0) } };
     }
     if (s.startsWith("SELECT COUNT(*) AS n FROM orders WHERE status = 'pending'")) {
@@ -442,6 +446,22 @@ section("admin products — list includes hidden rows");
   ok("exposes the visible flag", out.products.some((p) => p.visible === 0));
 }
 
+section("admin products — update checks the image against the manifest");
+{
+  const env = envDB({ products: [PRODUCT] });
+  for (const image of ["https://evil.example/x.jpg", "//evil.example/x.jpg", "../../etc/passwd", "not-in-manifest.jpg"]) {
+    const [status] = await read(await updateProduct(env, PRODUCT.id, { image }));
+    ok(`rejects ${image}`, status === 400, String(status));
+  }
+  const [status] = await read(await updateProduct(env, PRODUCT.id, { image: "dragon.jpg", images: "extra1.jpg, assets/images/extra2.jpg" }));
+  ok("accepts a file that exists", status === 200, String(status));
+  const row = env.DB._db.products[0];
+  ok("stores the canonical path", row.image === "assets/images/dragon.jpg", row.image);
+  ok("extras are canonical too", row.images === "assets/images/extra1.jpg,assets/images/extra2.jpg", row.images);
+  const [bad] = await read(await updateProduct(env, PRODUCT.id, { images: "extra1.jpg,ghost.jpg" }));
+  ok("an unknown extra is refused", bad === 400);
+}
+
 section("admin products — price validation");
 {
   for (const [label, price] of [
@@ -1304,13 +1324,19 @@ section("admin stats");
       { ...ORDER, id: "o2", status: "shipped", total_paise: 100000 },
       { ...ORDER, id: "o3", status: "pending", total_paise: 50000 },
       { ...ORDER, id: "o4", status: "cancelled", total_paise: 70000 },
+      // Mid-pipeline. These used to drop out of revenue the moment an order was
+      // marked "in production" and reappear when it shipped.
+      { ...ORDER, id: "o5", status: "in_production", total_paise: 20000 },
+      { ...ORDER, id: "o6", status: "ready", total_paise: 10000 },
+      { ...ORDER, id: "o7", status: "delivered", total_paise: 5000 },
+      { ...ORDER, id: "o8", status: "refunded", total_paise: 90000 },
     ],
     products: [PRODUCT, { ...PRODUCT, id: "p2", visible: 0 }],
   });
   const [, s] = await read(await stats(env));
-  ok("counts paid + shipped as revenue", s.revenue_paise === 144800, String(s.revenue_paise));
-  ok("excludes pending and cancelled from revenue", s.revenue_paise !== 264800);
-  ok("paid order count", s.paid_orders === 2);
+  ok("counts every stage from paid to delivered as revenue", s.revenue_paise === 179800, String(s.revenue_paise));
+  ok("excludes pending, cancelled and refunded from revenue", s.revenue_paise !== 264800 && s.revenue_paise < 269800);
+  ok("paid order count spans the pipeline", s.paid_orders === 5, String(s.paid_orders));
   ok("pending count", s.pending_orders === 1);
   ok("product totals", s.products_total === 2 && s.products_visible === 1);
 }

test/orders.mjs

@@ -57,9 +57,19 @@ function makeDB() {
       return { first: db.quotes.find((q) => q.receipt === args[0]) || null };
     }
     if (s.startsWith("UPDATE quotes SET status = 'paid'")) {
+      // The conditional claim. `AND order_id IS NULL` is what makes a second
+      // delivery a no-op, so the fake has to honour it or the race test below
+      // passes for the wrong reason.
       const q = db.quotes.find((x) => x.id === args[2]);
-      if (q) { q.status = "paid"; q.order_id = args[0]; q.updated_at = args[1]; }
-      return { meta: { changes: q ? 1 : 0 } };
+      if (!q || (s.includes("order_id IS NULL") && q.order_id)) return { meta: { changes: 0 } };
+      q.status = "paid"; q.order_id = args[0]; q.updated_at = args[1];
+      return { meta: { changes: 1 } };
+    }
+    if (s.startsWith("UPDATE quotes SET status = ?, order_id = NULL")) {
+      const q = db.quotes.find((x) => x.id === args[2] && x.order_id === args[3]);
+      if (!q) return { meta: { changes: 0 } };
+      q.status = args[0]; q.order_id = null; q.updated_at = args[1];
+      return { meta: { changes: 1 } };
     }
     if (s.startsWith("SELECT * FROM orders WHERE id = ?")) {
       return { first: db.orders.find((o) => o.id === args[0]) || null };
@@ -872,6 +882,58 @@ section("payment_link.paid — a redelivery does nothing twice");
   ok("exactly one line", env.DB._db.order_items.length === 1);
 }
 
+section("payment_link.paid — two deliveries with NO event id race for one quote");
+{
+  // No x-razorpay-event-id header at all, so webhook_events cannot help, and the
+  // two deliveries interleave: both read the quote before either has written.
+  // The conditional UPDATE is the only thing between this and two orders, two
+  // receipts and two invoices for one payment.
+  const env = ENV(); stubFetch();
+  env.DB._db.quotes.push({ ...QUOTE_ROW });
+  const raw = linkPaidBody();
+  const sig = await hmacHex(raw, WEBHOOK_SECRET);
+
+  const c1 = makeCtx(), c2 = makeCtx();
+  const noId = (r) => { const h = new Headers(r.headers); h.delete("x-razorpay-event-id"); return new Request(r.url, { method: "POST", headers: h, body: raw }); };
+  await Promise.all([
+    razorpayWebhook(noId(webhookReq(raw, sig, "")), env, c1),
+    razorpayWebhook(noId(webhookReq(raw, sig, "")), env, c2),
+  ]);
+  await settle(c1); await settle(c2);
+
+  ok("exactly one order despite the race", env.DB._db.orders.length === 1, String(env.DB._db.orders.length));
+  ok("exactly one line", env.DB._db.order_items.length === 1);
+  ok("the quote points at the order that exists",
+     env.DB._db.quotes[0].order_id === env.DB._db.orders[0]?.id);
+}
+
+section("payment_link.paid — a failed order write releases the claim");
+{
+  const env = ENV(); stubFetch();
+  env.DB._db.quotes.push({ ...QUOTE_ROW });
+  const raw = linkPaidBody();
+  const sig = await hmacHex(raw, WEBHOOK_SECRET);
+
+  // D1 falls over on the batch, once.
+  const realBatch = env.DB.batch.bind(env.DB);
+  let failed = false;
+  env.DB.batch = async (stmts) => {
+    if (!failed) { failed = true; throw new Error("D1 hiccup"); }
+    return realBatch(stmts);
+  };
+  const c1 = makeCtx();
+  let threw = false;
+  try { await razorpayWebhook(webhookReq(raw, sig, "evt_pl_fail1"), env, c1); } catch { threw = true; }
+  ok("the first delivery fails loudly so Razorpay retries", threw);
+  ok("no order was written", env.DB._db.orders.length === 0);
+  ok("the quote is NOT left marked paid", env.DB._db.quotes[0].status !== "paid" && env.DB._db.quotes[0].order_id == null,
+     JSON.stringify(env.DB._db.quotes[0]));
+
+  const c2 = makeCtx();
+  await razorpayWebhook(webhookReq(raw, sig, "evt_pl_fail2"), env, c2); await settle(c2);
+  ok("the retry converts the quote", env.DB._db.orders.length === 1 && env.DB._db.quotes[0].status === "paid");
+}
+
 section("payment_link.paid — an unknown reference is refused quietly");
 {
   const env = ENV(); stubFetch();