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 =
|
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 };
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue