From 0cab704840822558c7714aadaa607064cd0431e8 Mon Sep 17 00:00:00 2001
From: Claude <claude@anthropic.com>
Date: Mon, 31 Aug 2026 16:41:34 +0200
Subject: [PATCH] refactor(stock): tighten pricing-service reason type + drop
 dead FORBIDDEN_KEYS

Code-review feedback from Task 1:
- Drop the unused FORBIDDEN_KEYS constant + its export. The whitelist
  PATCHABLE_KEYS already covers what was meant; nothing else reads this.
- Replace UpdateStockItemsPricingResultRow's reason: string with a tight
  UpdateStockItemsPricingFailureReason union so the route can map to HTTP
  status without parsing the message string. Matches UpdateStockItemResult
  in stock-items-service.ts:113.
- Replace the array-tail-fragile 'last push matches current id' guard
  with an explicit validationFailed boolean per row, decoupling from
  push order in the inner loop.
---
 src/lib/server/stock-pricing-service.ts | 39 ++++++++++++++++++-------
 1 file changed, 29 insertions(+), 10 deletions(-)

diff --git a/src/lib/server/stock-pricing-service.ts b/src/lib/server/stock-pricing-service.ts
index b1dc10c..0eb4bb0 100644
--- a/src/lib/server/stock-pricing-service.ts
+++ b/src/lib/server/stock-pricing-service.ts
@@ -22,6 +22,17 @@ export type StockItemsPb = {
   };
 };
 
+/**
+ * Each failure has a stable reason code so the route can map to HTTP status
+ * (not_found → 404, forbidden_field / validation_error → 400, pb_error → 422)
+ * without parsing the message string.
+ */
+export type UpdateStockItemsPricingFailureReason =
+  | "forbidden_field"
+  | "validation_error"
+  | "not_found"
+  | "pb_error";
+
 export type UpdateStockItemsPricingResultRow =
   | {
       id: string;
@@ -29,7 +40,12 @@ export type UpdateStockItemsPricingResultRow =
       item: StockItem;
       previous: StockItemPricingSnapshot;
     }
-  | { id: string; ok: false; reason: string; message: string };
+  | {
+      id: string;
+      ok: false;
+      reason: UpdateStockItemsPricingFailureReason;
+      message: string;
+    };
 
 export type StockItemPricingSnapshot = {
   cost_price?: number;
@@ -42,7 +58,6 @@ export type UpdateStockItemsPricingResult = {
   summary: { total: number; succeeded: number; failed: number };
 };
 
-const FORBIDDEN_KEYS = ["id", "quantity", "created", "updated", "is_active"] as const;
 const PATCHABLE_KEYS = ["costPrice", "sellingPrice", "markup"] as const;
 
 export async function updateStockItemsPricing(
@@ -51,6 +66,8 @@ export async function updateStockItemsPricing(
   const results: UpdateStockItemsPricingResultRow[] = [];
   for (const update of input.updates) {
     // Whitelist check (route also enforces — service is defensive).
+    // Single explicit failure flag per row avoids coupling to push order.
+    let validationFailed = false;
     for (const key of Object.keys(update) as (keyof StockPricingUpdate)[]) {
       if (
         key !== "id" &&
@@ -62,23 +79,28 @@ export async function updateStockItemsPricing(
           reason: "forbidden_field",
           message: `forbidden_field: ${key}`,
         });
+        validationFailed = true;
         continue;
       }
       const value = (update as Record<string, unknown>)[key];
-      if (key !== "id" && value !== undefined && (typeof value !== "number" || !Number.isFinite(value) || (value as number) < 0)) {
+      if (
+        key !== "id" &&
+        value !== undefined &&
+        (typeof value !== "number" ||
+          !Number.isFinite(value) ||
+          (value as number) < 0)
+      ) {
         results.push({
           id: update.id,
           ok: false,
           reason: "validation_error",
           message: `${key} must be a finite number >= 0`,
         });
+        validationFailed = true;
         continue;
       }
     }
-    // Continue only if validation above did not push a failure for this row.
-    if (results.length > 0 && results[results.length - 1]?.id === update.id) {
-      continue;
-    }
+    if (validationFailed) continue;
 
     // 1. Load existing row for previous-snapshot + not_found detection.
     let before: StockItem;
@@ -146,6 +168,3 @@ export async function updateStockItemsPricing(
   };
   return { results, summary };
 }
-
-// Silence unused-import warning — kept available for the bulk endpoint.
-export { FORBIDDEN_KEYS };