fix: 🐛 address pr comments

This commit is contained in:
amianthus
2026-04-28 19:18:28 +01:00
parent eb2c063a6f
commit 8528f9dcd7
2 changed files with 112 additions and 15 deletions

View File

@@ -1,4 +1,5 @@
import {
cusProductToPrices,
cusProductToProcessorType,
type FullCusProduct,
type FullProduct,
@@ -13,15 +14,19 @@ import { pricesOnlyOneOff } from "@/internal/products/prices/priceUtils.js";
*
* For `update`: validates the specific cusProduct being modified.
*
* For `attach`: scans the customer's existing products. Throws if any are
* managed by a non-Stripe processor — UNLESS the product being attached is a
* true one-off (no recurring prices). True one-off attaches are safe across
* processors because they create a parallel cus_product without replacing
* the customer's existing subscription, and they never spin up a new Stripe
* subscription that could conflict with an RC-managed plan.
* For `attach`: scans the customer's existing products. Throws if any
* RECURRING product is managed by a non-Stripe processor — UNLESS the product
* being attached is itself a true one-off (no recurring prices). One-off
* cross-processor purchases (in either direction) are safe: they create a
* parallel cus_product, never spin up or reuse a recurring Stripe subscription,
* and so cannot conflict with the existing external subscription.
*
* Recurring add-ons are NOT exempt — they create a Stripe subscription that
* would coexist with the RC-managed main product, leading to incorrect billing.
* Concretely:
* - external recurring + attaching anything → throw (would create / mutate
* a Stripe sub that coexists or collides with the external sub).
* - external recurring + attaching one-off → bypass (parallel one-off only).
* - external one-off only + attaching anything → bypass (no external sub
* exists to conflict with).
*/
export const handleExternalPSPErrors = ({
customerProduct,
@@ -60,13 +65,25 @@ export const handleExternalPSPErrors = ({
return;
}
const externalCusProduct = customerProducts.find(
(cp) => cusProductToProcessorType(cp) !== ProcessorType.Stripe,
);
// Only block on EXTERNAL RECURRING products. External one-off-only products
// (e.g. a previously-purchased RC one-off pack) don't have a recurring
// subscription and so can't conflict with the new Stripe attach.
const conflictingExternalCusProduct = customerProducts.find((cp) => {
const isExternal =
cusProductToProcessorType(cp) !== ProcessorType.Stripe;
if (!isExternal) return false;
if (externalCusProduct) {
// Skip external products that are pure one-offs — they have no
// recurring sub to conflict with. Prices live on customer_prices
// (FullCusProduct.product is the bare Product without prices).
const cpPrices = cusProductToPrices({ cusProduct: cp });
const cpIsOneOffOnly = pricesOnlyOneOff(cpPrices);
return !cpIsOneOffOnly;
});
if (conflictingExternalCusProduct) {
throw new RecaseError({
message: `Cannot attach because the customer's current product '${externalCusProduct.product.name}' is managed by RevenueCat.`,
message: `Cannot attach because the customer's current product '${conflictingExternalCusProduct.product.name}' is managed by RevenueCat.`,
});
}
};

View File

@@ -82,13 +82,34 @@ const buildMixedIntervalProduct = (id: string): FullProduct =>
],
});
const buildRcCusProduct = (id = "cus_prod_rc"): FullCusProduct =>
customerProducts.create({
const buildRcCusProduct = (id = "cus_prod_rc"): FullCusProduct => {
const product = buildRecurringProduct("rc_main", false);
return customerProducts.create({
id,
productId: "rc_main",
product,
customerPrices: product.prices.map((price) =>
priceFixtures.createCustomer({ price, customerProductId: id }),
),
processorType: ProcessorType.RevenueCat,
subscriptionIds: [],
});
};
/** RC-managed cus product whose underlying product has only one-off prices. */
const buildRcOneOffCusProduct = (id = "cus_prod_rc_oneoff"): FullCusProduct => {
const product = buildOneOffProduct("rc_oneoff", true);
return customerProducts.create({
id,
productId: "rc_oneoff",
product,
customerPrices: product.prices.map((price) =>
priceFixtures.createCustomer({ price, customerProductId: id }),
),
processorType: ProcessorType.RevenueCat,
subscriptionIds: [],
});
};
const buildStripeCusProduct = (id = "cus_prod_stripe"): FullCusProduct =>
customerProducts.create({
@@ -259,5 +280,64 @@ describe(
"managed by RevenueCat",
);
});
// ─── External one-off-only products are NOT a conflict ──────────────────
test("BYPASS: customer has only an RC ONE-OFF product, attaching a Stripe recurring", () => {
// RC one-off (e.g. an in-app topup) doesn't have a recurring sub —
// nothing to conflict with the new Stripe attach.
const rcOneOff = buildRcOneOffCusProduct();
const recurringMain = buildRecurringProduct("pro_25_monthly", false);
expect(() =>
handleExternalPSPErrors({
customerProducts: [rcOneOff],
attachProduct: recurringMain,
action: "attach",
}),
).not.toThrow();
});
test("BYPASS: customer has only an RC ONE-OFF product, attaching a Stripe recurring add-on", () => {
const rcOneOff = buildRcOneOffCusProduct();
const recurringAddOn = buildRecurringProduct("recurring_addon", true);
expect(() =>
handleExternalPSPErrors({
customerProducts: [rcOneOff],
attachProduct: recurringAddOn,
action: "attach",
}),
).not.toThrow();
});
test("THROWS: customer has BOTH RC recurring and RC one-off, attaching a Stripe recurring", () => {
// The one-off is benign but the recurring product still conflicts.
const rcRecurring = buildRcCusProduct("cus_prod_rc_recurring");
const rcOneOff = buildRcOneOffCusProduct("cus_prod_rc_oneoff");
const recurringMain = buildRecurringProduct("pro_50_monthly", false);
expectThrows(
() =>
handleExternalPSPErrors({
customerProducts: [rcOneOff, rcRecurring],
attachProduct: recurringMain,
action: "attach",
}),
"managed by RevenueCat",
);
});
test("BYPASS: customer has only an RC ONE-OFF, no attachProduct provided", () => {
// Defensive: even without attachProduct, an RC one-off shouldn't
// trigger the guard since there's no recurring conflict.
const rcOneOff = buildRcOneOffCusProduct();
expect(() =>
handleExternalPSPErrors({
customerProducts: [rcOneOff],
action: "attach",
}),
).not.toThrow();
});
},
);