fix(tariff): reject stepped base combined with time/seasonal tiers
A stepped ("up-to") default card prices the whole stay as one total, so the V2
engine short-circuits to steppedFee and NEVER consults windowed cards — any
time/seasonal tiers would silently never fire. Found live: an active tariff had a
stepped base plus weekday-night + weekend tiers, and every 3h stay priced 600 ALL
regardless of hour/day because the tiers were dead.
- validateTariffV2 now rejects a stepped defaultCard combined with windowedCards,
with an actionable message (switch the base to ladder/flat, or remove the tiers).
- Composer shows an inline red warning the moment base mode is stepped and tiers
exist; publishing is blocked server-side regardless.
- ApiError now carries the server's problems[], so the publish error surfaces the
SPECIFIC reason instead of a generic "invalid tariff structure".
- 2 new validation tests (55 pass).
Wiki: tariff, log.
Claude-Session: https://claude.ai/code/session_01Xcm6ikLgGoCxxHrxtjkk5V
This commit is contained in:
@@ -341,8 +341,8 @@ export function TariffComposer() {
|
|||||||
setMsg({ kind: "ok", text: t("tariff.publishedOk") });
|
setMsg({ kind: "ok", text: t("tariff.publishedOk") });
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
const text =
|
const text =
|
||||||
e instanceof ApiError && (e as ApiError & { problems?: string[] }).problems
|
e instanceof ApiError && e.problems?.length
|
||||||
? `${e.message}: ${((e as ApiError & { problems?: string[] }).problems ?? []).join("; ")}`
|
? `${e.message}: ${e.problems.join("; ")}`
|
||||||
: (e as Error).message;
|
: (e as Error).message;
|
||||||
setMsg({ kind: "err", text });
|
setMsg({ kind: "err", text });
|
||||||
} finally {
|
} finally {
|
||||||
@@ -404,6 +404,13 @@ export function TariffComposer() {
|
|||||||
<details className="mt-6" open={form.tiers.length > 0}>
|
<details className="mt-6" open={form.tiers.length > 0}>
|
||||||
<summary className="cursor-pointer text-h6 font-semibold uppercase tracking-wider text-term-text">{t("tariff.tiersAdvanced")}</summary>
|
<summary className="cursor-pointer text-h6 font-semibold uppercase tracking-wider text-term-text">{t("tariff.tiersAdvanced")}</summary>
|
||||||
<p className="hint mt-1.5 mb-2">{t("tariff.tiersHint")}</p>
|
<p className="hint mt-1.5 mb-2">{t("tariff.tiersHint")}</p>
|
||||||
|
{/* A stepped ("up-to") base rate cannot be combined with time tiers — the
|
||||||
|
engine would ignore them. Warn up-front; publishing is also blocked server-side. */}
|
||||||
|
{form.base.mode === "stepped" && form.tiers.length > 0 && (
|
||||||
|
<p className="mb-3 rounded-term border border-term-red/50 bg-term-red/10 px-3 py-2 text-[12px] text-term-red">
|
||||||
|
{t("tariff.steppedTiersConflict")}
|
||||||
|
</p>
|
||||||
|
)}
|
||||||
{form.tiers.map((tr, i) => (
|
{form.tiers.map((tr, i) => (
|
||||||
<fieldset key={i} className="card mb-3 p-4">
|
<fieldset key={i} className="card mb-3 p-4">
|
||||||
<legend className="flex items-center gap-2 px-1">
|
<legend className="flex items-center gap-2 px-1">
|
||||||
|
|||||||
+4
-2
@@ -29,7 +29,7 @@ export async function apiFetch<T>(path: string, init: RequestInit = {}): Promise
|
|||||||
}
|
}
|
||||||
const res = await fetch(path, { ...init, headers, credentials: "include" });
|
const res = await fetch(path, { ...init, headers, credentials: "include" });
|
||||||
if (!res.ok) {
|
if (!res.ok) {
|
||||||
const msg = (await res.json().catch(() => ({}))) as { error?: string };
|
const msg = (await res.json().catch(() => ({}))) as { error?: string; problems?: string[] };
|
||||||
const error = msg.error ?? `${path}: ${res.status}`;
|
const error = msg.error ?? `${path}: ${res.status}`;
|
||||||
// Ship the failed request to the backend log store (best-effort, loop-safe — the
|
// Ship the failed request to the backend log store (best-effort, loop-safe — the
|
||||||
// logger itself never logs the /api/logs call). 401s are normal pre-login churn,
|
// logger itself never logs the /api/logs call). 401s are normal pre-login churn,
|
||||||
@@ -37,7 +37,7 @@ export async function apiFetch<T>(path: string, init: RequestInit = {}): Promise
|
|||||||
if (res.status !== 401) {
|
if (res.status !== 401) {
|
||||||
logFailedRequest({ path, method, status: res.status, error });
|
logFailedRequest({ path, method, status: res.status, error });
|
||||||
}
|
}
|
||||||
throw new ApiError(error, res.status);
|
throw new ApiError(error, res.status, msg.problems);
|
||||||
}
|
}
|
||||||
if (res.status === 204) return undefined as T;
|
if (res.status === 204) return undefined as T;
|
||||||
return res.json() as Promise<T>;
|
return res.json() as Promise<T>;
|
||||||
@@ -47,6 +47,8 @@ export class ApiError extends Error {
|
|||||||
constructor(
|
constructor(
|
||||||
message: string,
|
message: string,
|
||||||
readonly status: number,
|
readonly status: number,
|
||||||
|
/** Field-level problems from a validation error (e.g. tariff publish), if any. */
|
||||||
|
readonly problems?: string[],
|
||||||
) {
|
) {
|
||||||
super(message);
|
super(message);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -243,6 +243,8 @@ export const en: Catalog = {
|
|||||||
stepUpTo: "Up to",
|
stepUpTo: "Up to",
|
||||||
stepTotal: "Total price",
|
stepTotal: "Total price",
|
||||||
addStep: "+ Add row",
|
addStep: "+ Add row",
|
||||||
|
steppedTiersConflict:
|
||||||
|
"⚠ Time/seasonal tiers do NOT apply when the base rate is 'By duration (up-to)' — the engine ignores them entirely. Remove the tiers, or switch the base rate to 'Hourly ladder' or 'Flat price'. Publishing is blocked until this is fixed.",
|
||||||
tiersAdvanced: "Advanced: time & seasonal tiers",
|
tiersAdvanced: "Advanced: time & seasonal tiers",
|
||||||
tiersHint: "Optional. Add tiers that apply only at certain hours/days/dates or for a category (e.g. happy hour, night rate, weekend, bus). With no tiers, just the base rate is published.",
|
tiersHint: "Optional. Add tiers that apply only at certain hours/days/dates or for a category (e.g. happy hour, night rate, weekend, bus). With no tiers, just the base rate is published.",
|
||||||
tierName: "Name",
|
tierName: "Name",
|
||||||
|
|||||||
@@ -246,6 +246,8 @@ export const sq = {
|
|||||||
stepUpTo: "Deri në",
|
stepUpTo: "Deri në",
|
||||||
stepTotal: "Çmimi total",
|
stepTotal: "Çmimi total",
|
||||||
addStep: "+ Shto rresht",
|
addStep: "+ Shto rresht",
|
||||||
|
steppedTiersConflict:
|
||||||
|
"⚠ Nivelet kohore/sezonale NUK zbatohen kur tarifa bazë është 'Sipas kohëzgjatjes (deri-në)' — motori i shpërfill plotësisht. Hiqi nivelet, ose ndrysho tarifën bazë në 'Shkallë orësh' a 'Çmim fiks'. Publikimi bllokohet derisa kjo të rregullohet.",
|
||||||
tiersAdvanced: "Të avancuara: nivele kohore & sezonale",
|
tiersAdvanced: "Të avancuara: nivele kohore & sezonale",
|
||||||
tiersHint: "Opsionale. Shto nivele tarifore që vlejnë vetëm në orë/ditë/data ose kategori të caktuara (p.sh. orë e lirë, tarifë nate, fundjavë, autobus). Pa nivele, publikohet vetëm tarifa bazë.",
|
tiersHint: "Opsionale. Shto nivele tarifore që vlejnë vetëm në orë/ditë/data ose kategori të caktuara (p.sh. orë e lirë, tarifë nate, fundjavë, autobus). Pa nivele, publikohet vetëm tarifa bazë.",
|
||||||
tierName: "Emri",
|
tierName: "Emri",
|
||||||
|
|||||||
@@ -833,6 +833,17 @@ function validateTariffV2(t: Partial<TariffStructureV2>): string[] {
|
|||||||
cards.forEach((c, i) => validateCard(c, `windowedCards[${i}]`, false, errs));
|
cards.forEach((c, i) => validateCard(c, `windowedCards[${i}]`, false, errs));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A STEPPED base (an "up-to" total-by-duration table) prices the WHOLE stay as one
|
||||||
|
// number — it cannot be sliced per-increment, so windowed (time/seasonal) tiers have
|
||||||
|
// nothing to override and the engine ignores them entirely. Forbid the combination
|
||||||
|
// rather than let an operator publish tiers that silently never fire. (Switch the base
|
||||||
|
// to an hourly ladder / flat rate to use tiers, or remove the tiers.)
|
||||||
|
if (t.defaultCard != null && hasSteps(t.defaultCard) && cards.length > 0) {
|
||||||
|
errs.push(
|
||||||
|
"time/seasonal tiers do not apply to an up-to-duration (stepped) base rate — remove the tiers, or switch the base rate to an hourly ladder or flat price",
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// Precedence determinism: reject two cards (same category bucket) that tie on
|
// Precedence determinism: reject two cards (same category bucket) that tie on
|
||||||
// (specificity, priority) with overlapping windows — the operator must break the
|
// (specificity, priority) with overlapping windows — the operator must break the
|
||||||
// tie with priority rather than relying silently on the name tiebreak.
|
// tie with priority rather than relying silently on the name tiebreak.
|
||||||
|
|||||||
@@ -229,6 +229,19 @@ describe("validate V2", () => {
|
|||||||
const errs = validateTariffStructure({ ...base, defaultCard: { ...okDefault, window: { dow: [1] } } });
|
const errs = validateTariffStructure({ ...base, defaultCard: { ...okDefault, window: { dow: [1] } } });
|
||||||
expect(errs).toContain("defaultCard must not have a window (it is the always-active fallback)");
|
expect(errs).toContain("defaultCard must not have a window (it is the always-active fallback)");
|
||||||
});
|
});
|
||||||
|
it("rejects a STEPPED base card combined with windowed tiers (they would be ignored)", () => {
|
||||||
|
const steppedDefault: TariffCard = { name: "d", priority: 0, steps: [{ uptoMin: 60, totalMinor: 200 }] };
|
||||||
|
const errs = validateTariffStructure({
|
||||||
|
...base,
|
||||||
|
defaultCard: steppedDefault,
|
||||||
|
windowedCards: [{ name: "night", priority: 1, window: { dow: [1] }, blocks: ladder(5000) }],
|
||||||
|
});
|
||||||
|
expect(errs.some((e) => /up-to-duration \(stepped\) base/.test(e))).toBe(true);
|
||||||
|
});
|
||||||
|
it("accepts a STEPPED base card with NO tiers", () => {
|
||||||
|
const steppedDefault: TariffCard = { name: "d", priority: 0, steps: [{ uptoMin: 60, totalMinor: 200 }] };
|
||||||
|
expect(validateTariffStructure({ ...base, defaultCard: steppedDefault })).toEqual([]);
|
||||||
|
});
|
||||||
it("rejects a bad hour format", () => {
|
it("rejects a bad hour format", () => {
|
||||||
const errs = validateTariffStructure({ ...base, defaultCard: okDefault, windowedCards: [{ name: "w", priority: 1, window: { fromHour: "25:00", toHour: "26:00" }, blocks: ladder(5000) }] });
|
const errs = validateTariffStructure({ ...base, defaultCard: okDefault, windowedCards: [{ name: "w", priority: 1, window: { fromHour: "25:00", toHour: "26:00" }, blocks: ladder(5000) }] });
|
||||||
expect(errs.some((e) => e.includes("fromHour"))).toBe(true);
|
expect(errs.some((e) => e.includes("fromHour"))).toBe(true);
|
||||||
|
|||||||
@@ -94,6 +94,14 @@ A card's body is **one of three mutually-exclusive shapes** — `flatMinor`, `bl
|
|||||||
- A `steps` table **replaces** the `blocks` ladder and **forbids `dailyCapMinor`** (the top tier IS
|
- A `steps` table **replaces** the `blocks` ladder and **forbids `dailyCapMinor`** (the top tier IS
|
||||||
the per-day cap). In V2 it is allowed **only on the `defaultCard`** — a whole-stay total can't be
|
the per-day cap). In V2 it is allowed **only on the `defaultCard`** — a whole-stay total can't be
|
||||||
sliced per-increment by a windowed card, so windowed/stepped don't compose.
|
sliced per-increment by a windowed card, so windowed/stepped don't compose.
|
||||||
|
- **A stepped base + time/seasonal tiers is REJECTED** (`validateTariffV2`, 2026-06-20). The engine
|
||||||
|
short-circuits to `steppedFee` on a stepped default card and never consults windowed cards, so any
|
||||||
|
tiers would **silently never fire**. Rather than publish dead tiers, validation refuses the combo
|
||||||
|
("time/seasonal tiers do not apply to an up-to-duration (stepped) base rate — remove the tiers, or
|
||||||
|
switch the base rate to an hourly ladder or flat price"); the composer also shows an inline red
|
||||||
|
warning the moment both are present. (Discovered live: an active version had a stepped base AND
|
||||||
|
weekday-night + weekend tiers; the tiers priced nothing — every 3h stay was the stepped 600
|
||||||
|
regardless of time. The `problems[]` array now surfaces through `ApiError` to the publish message.)
|
||||||
- Validation: ≥1 row, strictly-ascending positive `uptoMin`, non-negative integer totals (totals
|
- Validation: ≥1 row, strictly-ascending positive `uptoMin`, non-negative integer totals (totals
|
||||||
need not be monotonic — an owner *may* price a longer stay cheaper).
|
need not be monotonic — an owner *may* price a longer stay cheaper).
|
||||||
|
|
||||||
|
|||||||
+13
@@ -1040,3 +1040,16 @@ addStep (sq+en). 8 new unit tests incl. the exact owner matrix + multi-day + ove
|
|||||||
+ validation (53 pass). VERIFIED end-to-end via the real UI: authored the matrix in the
|
+ validation (53 pass). VERIFIED end-to-end via the real UI: authored the matrix in the
|
||||||
composer, published, Tariff Lab priced it exactly (3h->500, 6h->800, 12h->1000,
|
composer, published, Tariff Lab priced it exactly (3h->500, 6h->800, 12h->1000,
|
||||||
2d->2000). Build+lint green. Updated [[tariff]].
|
2d->2000). Build+lint green. Updated [[tariff]].
|
||||||
|
|
||||||
|
## [2026-06-20] fix | Reject stepped base + time tiers (silently-ignored tiers)
|
||||||
|
|
||||||
|
Found live: the active tariff had a STEPPED ("up-to") base card AND two windowed tiers
|
||||||
|
(weekday-night "Nata gjate javes", weekend "Fundjava"). computeFeeV2 short-circuits to
|
||||||
|
steppedFee on a stepped default card, so the tiers NEVER fired — a 3h stay was 600 ALL
|
||||||
|
at every hour/day. The composer happily let this contradictory combo be built + published.
|
||||||
|
Fix: validateTariffV2 now rejects a stepped defaultCard combined with windowedCards
|
||||||
|
(clear message: switch base to ladder/flat or remove tiers); the composer shows an inline
|
||||||
|
red warning when base.mode==="stepped" && tiers>0. Also: ApiError now carries the
|
||||||
|
server's problems[] so the publish error shows the SPECIFIC reason (was generic "invalid
|
||||||
|
tariff structure"). 2 new validation tests (55 pass). Verified live: warning renders +
|
||||||
|
publish blocked with the full message. Build+lint green. Updated [[tariff]].
|
||||||
|
|||||||
Reference in New Issue
Block a user