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:
parent
076e80ecb4
commit
0cab704840
1 changed files with 29 additions and 10 deletions
|
|
@ -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 };
|
||||
|
|
|
|||
Loading…
Reference in a new issue