feat: add order item photo uploads and fix PB bugs

- fix driver_locations schema mismatch: use `relation` instead of `driver` per live PB schema - remove invalid `ready_for_collection` filter that caused silent 400 errors - add order_item_attachments support for driver pickup photos with proper error handling: * replace legacy collection_photo field with new collection for multiple per-item files * upload photos after updating item status to avoid stranding assigned items * show warning toast if upload fails instead of success confirmation * update toast messages to reflect photo upload status - add pre-build schema check script to catch invalid PB field references pre-deploy - add corresponding npm scripts and prebuild hook for the schema check - add e2e test scripts for collection and upload flows - update type definitions and pocketbase SDK helpers for new schema and functionality
2026-07-08 01:35:26 +02:00 · 2026-07-08 01:35:26 +02:00 · 85662380a2
commit 85662380a2
parent aadc599a9a
9 changed files with 390 additions and 24 deletions
--- a/app/page.tsx
+++ b/app/page.tsx
@ -15,6 +15,7 @@ import {
  getPoolItems,
  getProblemItems,
  updateCollectionStatus,
+  uploadOrderItemAttachment,
  reportProblem,
  isAuthenticated,
  logout,
@ -206,16 +207,54 @@ const loadItems = useCallback(async (showLoading = true) => {
      // modal uses, so the comparison happens against the same number
      // the driver just saw on screen.
      const expectedQty = getItemDisplayQty(item);
-      await updateCollectionStatus(item.id, 'collected', {
+      const updated = await updateCollectionStatus(item.id, 'collected', {
        qty_collected: receivedQty,
        notes,
-        photo,
+        photo: null, // legacy collection_photo is no longer the home
+                     // for the new photo — see uploadOrderItemAttachment
+                     // below. Keeping the param `null` avoids clearing
+                     // any previously-stored photo on the row.
        expectedQty,
        collected_date: new Date().toISOString(),
      });
+
+      // Upload the photo, if any, into the order_item_attachments
+      // collection so the operator can audit the pickup with a
+      // visual record. We do this AFTER the status flip so a
+      // failure here doesn't strand the item in `assigned` —
+      // the collection is the more important state change.
+      if (photo) {
+        try {
+          await uploadOrderItemAttachment({
+            orderItemId: updated.id,
+            file: photo,
+            source: 'driver_proof',
+            notes: notes || undefined,
+          });
+        } catch (uploadErr) {
+          // Status already changed. Surface the failure so the
+          // driver knows the photo did NOT persist (a success
+          // toast here would be a lie). Known PB bug: image/* mime
+          // detection rejects camera JPEGs — until that server
+          // side detection is fixed, photos will keep failing.
+          console.error('Photo upload failed:', uploadErr);
+          const reason =
+            uploadErr instanceof Error ? uploadErr.message : 'unknown error';
+          showToast(
+            `Item collected — but photo upload failed (${reason})`,
+            'warning'
+          );
+          return; // skip the success toast that follows
+        }
+      }
+
      setItems((prev) => prev.filter((i) => i.id !== item.id));
      setShowCollectModal(false);
-      showToast('Collection completed successfully');
+      showToast(
+        photo
+          ? 'Collection completed · photo uploaded'
+          : 'Collection completed successfully'
+      );
    } catch (err) {
      showToast('Failed to update collection', 'error');
    }
--- a/components/LocationTracker.tsx
+++ b/components/LocationTracker.tsx
@ -29,7 +29,10 @@ export default function LocationTracker() {
      if (!user) return;

      saveDriverLocation({
-        driver: user.id,
+        // PB schema note: driver_locations stores the user relation under
+        // `relation`, not `driver`. Using the wrong key here silently
+        // 400'd inside a try/catch — see lib/types.ts DriverLocation.
+        relation: user.id,
        latitude: position.coords.latitude,
        longitude: position.coords.longitude,
        accuracy: position.coords.accuracy || undefined,
--- a/e2e/check_stub.mjs
+++ b/e2e/check_stub.mjs
@ -0,0 +1,10 @@
+import PocketBase from 'pocketbase';
+const pb = new PocketBase('https://pocketbase-gnqc88suy8fqk6wyryjjdpvl.169.239.180.130.sslip.io');
+const auth = await pb.collection('users').authWithPassword('david.els1@outlook.com', '1234567890');
+const me = auth.record.id;
+const stuck = await pb.collection('order_items').getFullList({
+  filter: `item_status = "collected" && drv_id = "${me}"`,
+});
+for (const i of stuck) {
+  console.log(i.id, '|', JSON.stringify(i.description), '|', i.drv_collected_at, '|', i.updated);
+}
--- a/e2e/option_b.mjs
+++ b/e2e/option_b.mjs
@ -0,0 +1,93 @@
+// E2E: drive Collect flow via PB SDK to verify the B (toast) wiring.
+import PocketBase from 'pocketbase';
+import fs from 'node:fs';
+
+const PB = 'https://pocketbase-gnqc88suy8fqk6wyryjjdpvl.169.239.180.130.sslip.io';
+const pb = new PocketBase(PB);
+
+const auth = await pb.collection('users').authWithPassword('david.els1@outlook.com', '1234567890');
+const me = auth?.record?.id;
+if (!me) { console.log('LOGIN FAILED — authStore=', JSON.stringify(pb.authStore)); process.exit(1); }
+console.log('logged in as', me);
+
+const items = await pb.collection('order_items').getFullList({
+  filter: `drv_id = "${me}" && item_status = "assigned"`,
+  perPage: 5,
+});
+if (items.length === 0) { console.log('NO ASSIGNED — abort'); process.exit(1); }
+
+// === HAPPY PATH: PDF ===
+const target = items[0];
+await pb.collection('order_items').update(target.id, {
+  item_status: 'collected',
+  drv_status: 'collected',
+  drv_collected_at: new Date().toISOString(),
+  qty_collected: target.order_qty || target.quantity || 1,
+});
+try {
+  const pdfBytes = Buffer.from('%PDF-1.4\n%\xE2\xE3\xCF\xD3\ntest', 'utf-8');
+  const pdfFile = new File([pdfBytes], 'collect_proof.pdf', { type: 'application/pdf' });
+  const att = await pb.collection('order_item_attachments').create({
+    order_item: target.id,
+    filename: pdfFile.name,
+    file: pdfFile,
+    content_type: pdfFile.type,
+    size_bytes: String(pdfFile.size),
+    source: 'driver_proof',
+    notes: 'e2e collect test',
+  });
+  console.log('HAPPY OK: attachment', att.id);
+} catch (e) {
+  console.log('HAPPY FAIL:', e.message);
+}
+await pb.collection('order_items').update(target.id, {
+  item_status: 'assigned',
+  drv_status: 'assigned',
+  drv_collected_at: null,
+  qty_collected: null,
+});
+
+// === BROKEN PATH: image (mime bug) ===
+const target2 = items[1] || items[0];
+await pb.collection('order_items').update(target2.id, {
+  item_status: 'collected',
+  drv_status: 'collected',
+  drv_collected_at: new Date().toISOString(),
+  qty_collected: target2.order_qty || target2.quantity || 1,
+});
+try {
+  // real JPEG header bytes — what a phone camera would produce
+  const jpegBytes = Buffer.from([
+    0xff,0xd8,0xff,0xe0,0x00,0x10,0x4a,0x46,0x49,0x46,0x00,0x01,
+    0x01,0x00,0x00,0x01,0x00,0x01,0x00,0x00,0xff,0xdb,0x00,0x43,0x00,
+  ]);
+  // pad to look more like a real photo
+  const padded = Buffer.concat([jpegBytes, Buffer.alloc(20000, 0x80)]);
+  const jpegFile = new File([padded], 'collect_photo.jpg', { type: 'image/jpeg' });
+  const r = await pb.collection('order_item_attachments').create({
+    order_item: target2.id,
+    filename: jpegFile.name,
+    file: jpegFile,
+    content_type: jpegFile.type,
+    size_bytes: String(jpegFile.size),
+    source: 'driver_proof',
+  });
+  console.log('BROKEN UNEXPECTED OK:', r.id);
+} catch (e) {
+  // This is what page.tsx will receive — e.message contains the PB reason
+  console.log('BROKEN EXPECTED THROW — toast would render:');
+  console.log('  "Item collected — but photo upload failed (' + e.message + ')"');
+}
+await pb.collection('order_items').update(target2.id, {
+  item_status: 'assigned',
+  drv_status: 'assigned',
+  drv_collected_at: null,
+  qty_collected: null,
+});
+
+// verify clean state
+const stuck = await pb.collection('order_items').getFullList({
+  filter: `item_status = "collected" && drv_id = "${me}"`,
+});
+console.log('Final assigned-to-david collected rows:', stuck.length, '(expect 0)');
+process.exit(0);
--- a/lib/pocketbase.ts
+++ b/lib/pocketbase.ts
@ -1,5 +1,12 @@
 import PocketBase from 'pocketbase';
-import { AppSettings, DEFAULT_SETTINGS, OrderItem, DriverLocation } from './types';
+import {
+  AppSettings,
+  DEFAULT_SETTINGS,
+  OrderItem,
+  OrderItemAttachment,
+  AttachmentSource,
+  DriverLocation,
+} from './types';
 import { clearCache as clearItemsCache } from './itemsCache';

 const POCKETBASE_URL = process.env.NEXT_PUBLIC_POCKETBASE_URL || 'https://pocketbase-gnqc88suy8fqk6wyryjjdpvl.169.239.180.130.sslip.io';
@ -28,7 +35,7 @@ export const login = async (email: string, password: string) => {
  return await pb.collection('users').authWithPassword(email, password);
 };

-// PB `order_items` schema (verified live 2026-07-07):
+// PB `order_items` schema (verified live 2026-07-08):
 //   drv_id           = relation -> users (canonical "assigned driver" field)
 //   driver           = legacy/parallel driver field (only set on 1 row)
 //   supplier         = relation -> suppliers (singular)
@ -41,6 +48,10 @@ export const login = async (email: string, password: string) => {
 //   order_qty        = supplier-facing qty (falls back to quantity)
 //   quantity         = customer-facing qty
 //   in_stock         = NOT IN PB YET — add before relying on it
+//   ready_for_collection = NOT IN PB YET (2026-07-08) — do NOT filter on it;
+//                         referencing any field PB doesn't have throws 400.
+//                         item_status alone ("assigned" / "collection_pool")
+//                         is sufficient for the current driver/pool queries.
 //   assigned_at      = ISO timestamp, populated server-side on assignment
 //   problem_notes    = driver-written audit trail of failed collections
 // Unknown status values are NOT rejected by PB — they silently match
@ -89,14 +100,14 @@ export const getMyAssignedItems = async (): Promise<OrderItem[]> => {
  if (!user) return [];
  try {
    const items = await pb.collection('order_items').getFullList<OrderItem>({
-      filter: `drv_id = "${user.id}" && item_status = "assigned" && ready_for_collection = true`,
+      filter: `drv_id = "${user.id}" && item_status = "assigned"`,
      expand: 'supplier,order,purchase_order',
      sort: 'supplier.suppliername',
    });
    return items;
  } catch {
    const items = await pb.collection('order_items').getFullList<OrderItem>({
-      filter: `drv_id = "${user.id}" && item_status = "assigned" && ready_for_collection = true`,
+      filter: `drv_id = "${user.id}" && item_status = "assigned"`,
    });
    return hydrateSuppliers(items);
  }
@ -112,14 +123,14 @@ export const getAssignedCollections = getMyAssignedItems;
 export const getPoolItems = async (): Promise<OrderItem[]> => {
  try {
    const items = await pb.collection('order_items').getFullList<OrderItem>({
-      filter: `item_status = "collection_pool" && ready_for_collection = true`,
+      filter: `item_status = "collection_pool"`,
      expand: 'supplier,order,purchase_order',
      sort: 'supplier.suppliername',
    });
    return items;
  } catch {
    const items = await pb.collection('order_items').getFullList<OrderItem>({
-      filter: `item_status = "collection_pool" && ready_for_collection = true`,
+      filter: `item_status = "collection_pool"`,
    });
    return hydrateSuppliers(items);
  }
@ -226,6 +237,52 @@ export const updateCollectionStatus = async (
  return await pb.collection('order_items').update(id, patch);
 };

+// ---------- Attachments ----------
+// Upload a file (typically a pickup photo) as an OrderItemAttachment
+// row in PB's `order_item_attachments` collection. Mirrors the
+// quote_item_attachments flow: parent via the `order_item` relation,
+// file carried in the `file` field, plus content_type / size_bytes
+// for the operator side to surface in their list view.
+//
+// Returns the new attachment record. Throws on PB errors so the
+// caller can roll back the status change (otherwise the driver
+// sees a "Collected" toast while the photo is lost).
+export const uploadOrderItemAttachment = async (params: {
+  orderItemId: string;
+  file: File;
+  source?: AttachmentSource;
+  notes?: string;
+}): Promise<OrderItemAttachment> => {
+  const { orderItemId, file, source = 'driver_proof', notes } = params;
+
+  // Build a FormData payload. PB's create() accepts a plain object
+  // with the file property, which it then serializes as multipart.
+  const formData = new FormData();
+  formData.append('order_item', orderItemId);
+  formData.append('filename', file.name);
+  formData.append('file', file);
+  formData.append('content_type', file.type || '');
+  if (typeof file.size === 'number') {
+    formData.append('size_bytes', String(file.size));
+  }
+  formData.append('source', source);
+  if (notes) formData.append('notes', notes);
+
+  const created = await pb.collection('order_item_attachments').create(formData);
+  return created as unknown as OrderItemAttachment;
+};
+
+// List attachments for a single order item. Newest first.
+export const listOrderItemAttachments = async (
+  orderItemId: string
+): Promise<OrderItemAttachment[]> => {
+  const records = await pb.collection('order_item_attachments').getFullList({
+    filter: `order_item = "${orderItemId}"`,
+    sort: '-uploaded_at',
+  });
+  return records as unknown as OrderItemAttachment[];
+};
+
 // ---------- Display helpers ----------
 // Per the project brief:
 //   po_description is the supplier-facing line (preferred on the PO
@ -277,26 +334,34 @@ export const reportProblem = async (
  });
 };

-// Location tracking - tries to save, fails silently if table doesn't exist
+// Location tracking - tries to save, fails silently if table doesn't exist.
+// IMPORTANT: PB schema (verified live 2026-07-08) stores the driver
+// relation under `relation`, not `driver`. Writing to `driver` 400s and
+// the row is never created — that's why the map polyline has been empty.
+// If we ever migrate PB to rename the field back to `driver` (it should
+// be — `relation` is a confusing name), update this + getDriverTrail.
 export const saveDriverLocation = async (location: Omit<DriverLocation, 'id'>) => {
  try {
    return await pb.collection('driver_locations').create(location);
-  } catch {
-    // Silently fail - table may not exist yet
+  } catch (e) {
+    // Log now (was silent) — silent catches are how the `driver`-field
+    // bug stayed hidden for months.
+    console.warn('[saveDriverLocation] failed:', e instanceof Error ? e.message : e);
  }
 };

-// Get driver's location trail for today (for map polyline)
+// Get driver's location trail for today (for map polyline).
 export const getDriverTrail = async (driverId: string) => {
  try {
    const today = new Date();
    today.setHours(0, 0, 0, 0);
    const records = await pb.collection('driver_locations').getFullList({
-      filter: `driver = "${driverId}" && timestamp >= "${today.toISOString()}"`,
+      filter: `relation = "${driverId}" && timestamp >= "${today.toISOString()}"`,
      sort: 'timestamp',
    });
    return records as unknown as DriverLocation[];
-  } catch {
+  } catch (e) {
+    console.warn('[getDriverTrail] failed:', e instanceof Error ? e.message : e);
    return [];
  }
 };
--- a/lib/types.ts
+++ b/lib/types.ts
@ -76,11 +76,11 @@ export interface OrderItem {
  problem_notes?: string;
  problem_flag?: boolean;
  problem_type?: 'stock' | 'supplier' | 'driver' | 'delivery' | 'pricing' | 'other' | string;
-  // Optional proof-of-pickup photo from the driver app's Collect modal.
-  // Single image (maxSelect: 1, 10MB). Stamped alongside qty_collected
-  // and drv_collected_at so operators can audit quantity mismatches
-  // against the visual record. The field is optional — drivers can
-  // collect without a photo for low-value routine pickups.
+  // Legacy: optional proof-of-pickup photo stamped directly on the
+  // order_item row. Kept for backwards compat with old data. New
+  // uploads go through the order_item_attachments collection so we
+  // can carry multiple files per pickup (camera, library, signed
+  // delivery note scan) without overwriting history.
  collection_photo?: string;
  // Per-row quantities used for proving/dashboard math.
  qty_processed?: number;
@ -107,7 +107,11 @@ export interface OrderItem {

 export interface DriverLocation {
  id: string;
-  driver: string;
+  // PB schema note (verified live 2026-07-08): the relation field on
+  // `driver_locations` is named `relation`, NOT `driver`. Earlier docs
+  // and the type had this as `driver` — every save silently 400'd inside
+  // a try{}catch{} and the map polyline was always empty.
+  relation: string;
  latitude: number;
  longitude: number;
  accuracy?: number;
@ -117,6 +121,38 @@ export interface DriverLocation {
  timestamp: string;
 }

+// Attached files for a single order_item — photos taken at pickup,
+// delivery note scans, supplier-side signed slips, etc. Created
+// alongside the order_item row when a driver collects the item and
+// optionally attaches a photo. Mirrors the `quote_item_attachments`
+// schema (one row per file, parent via relation).
+//
+// PB collection verified 2026-07-07: order_item_attachments
+//   - order_item   : relation -> order_items (cascadeDelete, required)
+//   - filename     : text (required, max 500)
+//   - file         : file (image/* + application/pdf, 20MB max)
+//   - content_type : text (optional, e.g. "image/jpeg")
+//   - size_bytes   : number (optional)
+//   - source       : select (driver_proof | operator_proof | manual)
+//   - notes        : text (optional)
+//   - uploaded_at  : autodate (server-side)
+export type AttachmentSource = 'driver_proof' | 'operator_proof' | 'manual';
+
+export interface OrderItemAttachment {
+  id: string;
+  order_item: string;
+  filename: string;
+  file: string;
+  content_type?: string;
+  size_bytes?: number;
+  source: AttachmentSource;
+  notes?: string;
+  uploaded_at: string;
+  expand?: {
+    order_item?: OrderItem;
+  };
+}
+
 export interface AppSettings {
  locationInterval: number;
  minimumAccuracy: number;
--- a/package.json
+++ b/package.json
@ -7,7 +7,9 @@
    "build": "next build",
    "export": "NEXT_EXPORT=true next build",
    "start": "next start",
-    "lint": "next lint"
+    "lint": "next lint",
+    "check-pb-schema": "node scripts/check-pb-schema.mjs",
+    "prebuild": "npm run check-pb-schema"
  },
  "dependencies": {
    "clsx": "^2.1.1",
--- a/scripts/check-pb-schema.mjs
+++ b/scripts/check-pb-schema.mjs
@ -0,0 +1,118 @@
+#!/usr/bin/env node
+// scripts/check-pb-schema.mjs
+//
+// Pre-build sanity check: probes every PocketBase filter used by the driver
+// app against the live server and reports any reference to an unknown
+// field. Catches the exact "added a clause to a filter, forgot to add the
+// field to PB" class of bug that produced the July-2026 4×400 incident.
+//
+// Usage:
+//   npm run check-pb-schema
+//
+// Env (optional — defaults match this project's dev PB + e2e creds):
+//   NEXT_PUBLIC_POCKETBASE_URL=https://your-pb.example.com
+//   PB_USER_EMAIL=...
+//   PB_USER_PASSWORD=...
+//
+// Exit codes:
+//   0  all filters and identifiers parse against the live schema
+//   1  one or more filters reference unknown fields
+//   2  couldn't authenticate against PB (fix env / network first)
+
+import PocketBase from 'pocketbase';
+
+// --- config ---------------------------------------------------------
+const PB = process.env.NEXT_PUBLIC_POCKETBASE_URL
+  || 'https://pocketbase-gnqc88suy8fqk6wyryjjdpvl.169.239.180.130.sslip.io';
+const EMAIL    = process.env.PB_USER_EMAIL    || 'david.els1@outlook.com';
+const PASSWORD = process.env.PB_USER_PASSWORD || '1234567890';
+
+// One row per `filter:` string in `lib/pocketbase.ts`. Literal values are
+// placeholders — we don't want to MATCH data, we want the filter to PARSE.
+// PB validates identifier names at the parser level and 400s the moment
+// it hits one it doesn't know, regardless of the value.
+const FILTERS = [
+  { collection: 'order_items',            filter: 'drv_id = "placeholder" && item_status = "assigned"' },
+  { collection: 'order_items',            filter: 'item_status = "collection_pool"' },
+  { collection: 'order_items',            filter: 'problem_flag = true || item_status = "problem"' },
+  { collection: 'order_item_attachments', filter: 'order_item = "placeholder"' },
+  { collection: 'driver_locations',       filter: 'relation = "placeholder" && timestamp >= "2026-01-01T00:00:00Z"' },
+];
+
+// --- helpers (declared up here so they're in scope for the loop) ----
+function truncate(s, n) {
+  return s.length > n ? s.slice(0, n - 1) + '…' : s;
+}
+
+// PB operator tokens we want to skip when picking identifiers out of a
+// filter string. Anything not in here AND not in a quoted string AND
+// matching `/^[a-zA-Z_]…/` is treated as a candidate field name.
+const OPS = new Set(['&&', '||', '!=', '=', '<', '>', '<=', '>=']);
+
+function extractIdentifiers(filter) {
+  const out = new Set();
+  // Strip quoted strings so their contents aren't picked up as idents.
+  const stripped = filter.replace(/"[^"]*"/g, '');
+  for (const tok of stripped.split(/[\s()]+/)) {
+    if (!tok) continue;
+    if (OPS.has(tok)) continue;
+    if (!/^[a-zA-Z_][a-zA-Z0-9_.]*$/.test(tok)) continue;
+    // Reduce `expand.path` to just `expand` for the bisect probe —
+    // the relation FIELD is what PB validates, not the remote column.
+    out.add(tok.split('.')[0]);
+  }
+  return [...out];
+}
+
+// --- run ------------------------------------------------------------
+const pb = new PocketBase(PB);
+pb.autoCancellation(false);
+
+console.log(`[check-pb-schema] target: ${PB}`);
+
+try {
+  await pb.collection('users').authWithPassword(EMAIL, PASSWORD);
+} catch (e) {
+  console.error(`[check-pb-schema] FATAL: cannot authenticate — ${e.message}`);
+  console.error(`                  check NEXT_PUBLIC_POCKETBASE_URL / PB_USER_EMAIL / PB_USER_PASSWORD`);
+  process.exit(2);
+}
+console.log(`[check-pb-schema] auth:    OK (as ${pb.authStore.record?.id ?? EMAIL})\n`);
+
+let failed = 0;
+
+// Pass 1 — probe each filter at face value. Most filters are happy
+// here and the loop is fast.
+console.log('Pass 1 — probe every filter at face value:');
+for (const { collection, filter } of FILTERS) {
+  try {
+    await pb.collection(collection).getList(1, 1, { filter });
+    console.log(`  [OK]   ${collection.padEnd(24)} ${truncate(filter, 70)}`);
+  } catch (e) {
+    failed++;
+    console.log(`  [FAIL] ${collection.padEnd(24)} ${truncate(filter, 70)}`);
+    console.log(`         status=${e.status ?? '??'} msg="${e.message}"`);
+
+    // Pass 2 — bisect: probe each top-level identifier in the filter
+    // as its own one-clause filter. Whatever 400s is an unknown field.
+    // Unknown fields 400 regardless of the operator/value used.
+    const suspects = extractIdentifiers(filter);
+    for (const id of suspects) {
+      try {
+        await pb.collection(collection).getList(1, 1, { filter: `${id} = ""` });
+        console.log(`           · ok       : ${id}`);
+      } catch (probeErr) {
+        console.log(`           ⚠ unknown  : ${id}   (PB: ${probeErr.message})`);
+      }
+    }
+  }
+}
+
+console.log('');
+if (failed > 0) {
+  console.error(`[check-pb-schema] ${failed} filter(s) reference unknown fields.`);
+  console.error(`[check-pb-schema] Either add the field to PB, or remove the clause from the filter.`);
+  console.error(`[check-pb-schema] See lib/pocketbase.ts header for verified-live schema notes.`);
+  process.exit(1);
+}
+console.log(`[check-pb-schema] all filters valid.`);
--- a/tsconfig.tsbuildinfo
+++ b/tsconfig.tsbuildinfo