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.
This commit is contained in:
Claude 2026-08-31 16:41:34 +02:00
parent 076e80ecb4
commit 0cab704840

View file

@ -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 = export type UpdateStockItemsPricingResultRow =
| { | {
id: string; id: string;
@ -29,7 +40,12 @@ export type UpdateStockItemsPricingResultRow =
item: StockItem; item: StockItem;
previous: StockItemPricingSnapshot; previous: StockItemPricingSnapshot;
} }
| { id: string; ok: false; reason: string; message: string }; | {
id: string;
ok: false;
reason: UpdateStockItemsPricingFailureReason;
message: string;
};
export type StockItemPricingSnapshot = { export type StockItemPricingSnapshot = {
cost_price?: number; cost_price?: number;
@ -42,7 +58,6 @@ export type UpdateStockItemsPricingResult = {
summary: { total: number; succeeded: number; failed: number }; 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; const PATCHABLE_KEYS = ["costPrice", "sellingPrice", "markup"] as const;
export async function updateStockItemsPricing( export async function updateStockItemsPricing(
@ -51,6 +66,8 @@ export async function updateStockItemsPricing(
const results: UpdateStockItemsPricingResultRow[] = []; const results: UpdateStockItemsPricingResultRow[] = [];
for (const update of input.updates) { for (const update of input.updates) {
// Whitelist check (route also enforces — service is defensive). // 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)[]) { for (const key of Object.keys(update) as (keyof StockPricingUpdate)[]) {
if ( if (
key !== "id" && key !== "id" &&
@ -62,23 +79,28 @@ export async function updateStockItemsPricing(
reason: "forbidden_field", reason: "forbidden_field",
message: `forbidden_field: ${key}`, message: `forbidden_field: ${key}`,
}); });
validationFailed = true;
continue; continue;
} }
const value = (update as Record<string, unknown>)[key]; 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({ results.push({
id: update.id, id: update.id,
ok: false, ok: false,
reason: "validation_error", reason: "validation_error",
message: `${key} must be a finite number >= 0`, message: `${key} must be a finite number >= 0`,
}); });
validationFailed = true;
continue; continue;
} }
} }
// Continue only if validation above did not push a failure for this row. if (validationFailed) continue;
if (results.length > 0 && results[results.length - 1]?.id === update.id) {
continue;
}
// 1. Load existing row for previous-snapshot + not_found detection. // 1. Load existing row for previous-snapshot + not_found detection.
let before: StockItem; let before: StockItem;
@ -146,6 +168,3 @@ export async function updateStockItemsPricing(
}; };
return { results, summary }; return { results, summary };
} }
// Silence unused-import warning — kept available for the bulk endpoint.
export { FORBIDDEN_KEYS };