From cdc56e3b71ba8fd13efe670d3fe4610a9f8ca421 Mon Sep 17 00:00:00 2001 From: amianthus <49116958+SirTenzin@users.noreply.github.com> Date: Mon, 25 May 2026 17:33:27 +0100 Subject: [PATCH] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20frontend=20for=20new=20v?= =?UTF-8?q?ercel?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- vite/src/utils/product/productItemUtils.ts | 26 ++- .../configure-vercel/ConfigureVercel.tsx | 43 ----- vite/tests/utils/get-item-id.test.ts | 168 ++++++++++++++++++ 3 files changed, 191 insertions(+), 46 deletions(-) create mode 100644 vite/tests/utils/get-item-id.test.ts diff --git a/vite/src/utils/product/productItemUtils.ts b/vite/src/utils/product/productItemUtils.ts index 9b6ffa60f..212e9a263 100644 --- a/vite/src/utils/product/productItemUtils.ts +++ b/vite/src/utils/product/productItemUtils.ts @@ -100,6 +100,24 @@ const itemsHaveSameInterval = ({ ); }; +/** + * Builds an interval discriminator suffix so two items sharing the same + * feature/entitlement/price-id but differing in billing cadence (e.g. a + * monthly entitlement and a one-off price for the same feature) hash to + * distinct ids. Without this, the plan editor sheet retargets whichever + * item happens to come first in the list and the second item becomes + * uneditable. `null`/`undefined` interval is one-off; `interval_count` of + * `1`/missing is treated the same as omitted. + */ +const intervalSuffix = (item: ProductItem): string => { + if (!item.interval) return "-oneoff"; + const count = + item.interval_count && item.interval_count !== 1 + ? `x${item.interval_count}` + : ""; + return `-${item.interval}${count}`; +}; + export const getItemId = ({ item, itemIndex, @@ -107,12 +125,14 @@ export const getItemId = ({ item: ProductItem; itemIndex: number; }) => { - if (item.entitlement_id) return `ent-${item.entitlement_id}`; - if (item.price_id) return `price-${item.price_id}`; + const interval = intervalSuffix(item); + if (item.entitlement_id) return `ent-${item.entitlement_id}${interval}`; + if (item.price_id) return `price-${item.price_id}${interval}`; if (item.feature_id) { - return item.entity_feature_id + const base = item.entity_feature_id ? `feature-${item.feature_id}-${item.entity_feature_id}` : `feature-${item.feature_id}`; + return `${base}${interval}`; } return `item-${itemIndex}`; }; diff --git a/vite/src/views/developer/configure-vercel/ConfigureVercel.tsx b/vite/src/views/developer/configure-vercel/ConfigureVercel.tsx index 285c59b5f..d63b1bab5 100644 --- a/vite/src/views/developer/configure-vercel/ConfigureVercel.tsx +++ b/vite/src/views/developer/configure-vercel/ConfigureVercel.tsx @@ -39,7 +39,6 @@ type VercelConfigState = { client_integration_id: string; client_secret: string; webhook_url: string; - custom_payment_method: string; marketplace_mode: VercelMarketplaceMode; allowed_product_ids_live: string[]; allowed_product_ids_sandbox: string[]; @@ -59,7 +58,6 @@ export const ConfigureVercel = () => { client_integration_id: "", client_secret: "", webhook_url: "", - custom_payment_method: "", marketplace_mode: "" as VercelMarketplaceMode, allowed_product_ids_live: [], allowed_product_ids_sandbox: [], @@ -105,8 +103,6 @@ export const ConfigureVercel = () => { org?.processor_configs?.vercel?.client_integration_id; const currentClientSecret = org?.processor_configs?.vercel?.client_secret; const currentWebhookUrl = org?.processor_configs?.vercel?.webhook_url; - const currentCustomPaymentMethod = - org?.processor_configs?.vercel?.custom_payment_method; const { isDark } = useTheme(); @@ -144,12 +140,6 @@ export const ConfigureVercel = () => { } } - if (vercelConfig.custom_payment_method?.trim()) { - filteredConfig.custom_payment_method = { - [env]: vercelConfig.custom_payment_method.trim(), - }; - } - if (vercelConfig.marketplace_mode) { filteredConfig.marketplace_mode = vercelConfig.marketplace_mode; } @@ -175,7 +165,6 @@ export const ConfigureVercel = () => { client_integration_id: "", client_secret: "", webhook_url: "", - custom_payment_method: "", marketplace_mode: filteredConfig.marketplace_mode as VercelMarketplaceMode, })); @@ -245,38 +234,6 @@ export const ConfigureVercel = () => { } /> -
- - - Stripe Custom Payment Method ID - - -

- Create a custom payment method in{" "} - - Stripe - - . -

- - setVercelConfig((prev) => ({ - ...prev, - custom_payment_method: e.target.value, - })) - } - placeholder={ - currentCustomPaymentMethod || - "eg. cpmt_Yij7OBT6Fxu0UOa12XguA0vGB" - } - /> -
diff --git a/vite/tests/utils/get-item-id.test.ts b/vite/tests/utils/get-item-id.test.ts new file mode 100644 index 000000000..6c405b42f --- /dev/null +++ b/vite/tests/utils/get-item-id.test.ts @@ -0,0 +1,168 @@ +/** + * Tests for `getItemId` — the single source of truth for plan-editor item + * identity. Two regressions to protect against here: + * + * 1. Same `feature_id` with different intervals must produce DISTINCT ids + * (this turn's bug — the per-item edit sheet retargeted the wrong row). + * 2. Item identity must NOT depend on list position. Charlie's earlier fixes + * (commits 709e1fd23 + 5ee6eeb3a) replaced index-based ids with stable, + * item-intrinsic ids to prevent React from reusing DOM rows on delete + * ("ghost item" bug). The new id format must still be position-invariant. + */ + +import { describe, expect, test } from "bun:test"; +import type { ProductItem } from "@autumn/shared"; +import { getItemId } from "@/utils/product/productItemUtils"; + +const make = (overrides: Partial): ProductItem => + ({ + feature_id: undefined, + entitlement_id: undefined, + price_id: undefined, + entity_feature_id: undefined, + interval: undefined, + interval_count: undefined, + ...overrides, + }) as ProductItem; + +describe("getItemId — interval discriminator (turn fix)", () => { + test("two items with same feature_id but different intervals get distinct ids", () => { + const monthly = make({ feature_id: "action1", interval: "month" }); + const oneOff = make({ feature_id: "action1", interval: undefined }); + + const monthlyId = getItemId({ item: monthly, itemIndex: 0 }); + const oneOffId = getItemId({ item: oneOff, itemIndex: 1 }); + + expect(monthlyId).not.toBe(oneOffId); + expect(monthlyId).toBe("feature-action1-month"); + expect(oneOffId).toBe("feature-action1-oneoff"); + }); + + test("same feature_id + same interval but different interval_count get distinct ids", () => { + const oneMonth = make({ + feature_id: "action1", + interval: "month", + interval_count: 1, + }); + const threeMonths = make({ + feature_id: "action1", + interval: "month", + interval_count: 3, + }); + + const a = getItemId({ item: oneMonth, itemIndex: 0 }); + const b = getItemId({ item: threeMonths, itemIndex: 1 }); + + expect(a).not.toBe(b); + expect(a).toBe("feature-action1-month"); // count=1 is implicit + expect(b).toBe("feature-action1-monthx3"); + }); + + test("entity-scoped feature items still get distinct ids per interval", () => { + const monthly = make({ + feature_id: "seats", + entity_feature_id: "team", + interval: "month", + }); + const oneOff = make({ + feature_id: "seats", + entity_feature_id: "team", + interval: undefined, + }); + + expect(getItemId({ item: monthly, itemIndex: 0 })).toBe( + "feature-seats-team-month", + ); + expect(getItemId({ item: oneOff, itemIndex: 1 })).toBe( + "feature-seats-team-oneoff", + ); + }); + + test("entitlement-keyed items also discriminate by interval", () => { + const monthly = make({ entitlement_id: "ent_x", interval: "month" }); + const oneOff = make({ entitlement_id: "ent_x", interval: undefined }); + + expect(getItemId({ item: monthly, itemIndex: 0 })).toBe( + "ent-ent_x-month", + ); + expect(getItemId({ item: oneOff, itemIndex: 1 })).toBe("ent-ent_x-oneoff"); + }); + + test("price-keyed items also discriminate by interval", () => { + const monthly = make({ price_id: "pr_x", interval: "month" }); + const oneOff = make({ price_id: "pr_x", interval: undefined }); + + expect(getItemId({ item: monthly, itemIndex: 0 })).toBe("price-pr_x-month"); + expect(getItemId({ item: oneOff, itemIndex: 1 })).toBe("price-pr_x-oneoff"); + }); +}); + +describe("getItemId — position-invariance (Charlie's earlier fix)", () => { + test("feature-based id does not depend on itemIndex", () => { + const item = make({ feature_id: "action1", interval: "month" }); + expect(getItemId({ item, itemIndex: 0 })).toBe( + getItemId({ item, itemIndex: 5 }), + ); + }); + + test("entitlement-based id does not depend on itemIndex", () => { + const item = make({ entitlement_id: "ent_x", interval: "month" }); + expect(getItemId({ item, itemIndex: 0 })).toBe( + getItemId({ item, itemIndex: 9 }), + ); + }); + + test("price-based id does not depend on itemIndex", () => { + const item = make({ price_id: "pr_x", interval: "month" }); + expect(getItemId({ item, itemIndex: 0 })).toBe( + getItemId({ item, itemIndex: 9 }), + ); + }); + + test("deleting a sibling does not change other items' ids", () => { + // Simulates the ghost-item scenario: build a list, drop an item from + // the middle, and verify the remaining items still hash to their + // original ids (which is what prevents React from reusing DOM rows). + const items = [ + make({ feature_id: "messages", interval: "month" }), + make({ feature_id: "to-be-deleted", interval: "month" }), + make({ feature_id: "seats", interval: undefined }), + ]; + const idsBefore = items.map((item, i) => + getItemId({ item, itemIndex: i }), + ); + + const afterDelete = items.filter((_, i) => i !== 1); + const idsAfter = afterDelete.map((item, i) => + getItemId({ item, itemIndex: i }), + ); + + expect(idsAfter).toEqual([idsBefore[0]!, idsBefore[2]!]); + }); + + test("index-only fallback fires only when no identifying ids are present", () => { + const orphan = make({ interval: "month" }); + expect(getItemId({ item: orphan, itemIndex: 7 })).toBe("item-7"); + }); +}); + +describe("getItemId — branch precedence", () => { + test("entitlement_id wins over price_id and feature_id", () => { + const item = make({ + entitlement_id: "ent_x", + price_id: "pr_x", + feature_id: "action1", + interval: "month", + }); + expect(getItemId({ item, itemIndex: 0 })).toBe("ent-ent_x-month"); + }); + + test("price_id wins over feature_id when no entitlement_id", () => { + const item = make({ + price_id: "pr_x", + feature_id: "action1", + interval: "month", + }); + expect(getItemId({ item, itemIndex: 0 })).toBe("price-pr_x-month"); + }); +});