fix: preserve automatic tax preflight defaults
This commit is contained in:
95
MIGRATIONS_FIX_TARGET_LIMIT.md
Normal file
95
MIGRATIONS_FIX_TARGET_LIMIT.md
Normal file
@@ -0,0 +1,95 @@
|
||||
# Migration Runs `target_limit` Dev DB Note
|
||||
|
||||
While writing the automatic-tax missing-address regression test, the integration
|
||||
harness could not reach billing setup because the dev database schema was behind
|
||||
the checked-in Drizzle model.
|
||||
|
||||
## Symptom
|
||||
|
||||
Running:
|
||||
|
||||
```bash
|
||||
cd server
|
||||
./run.sh /Users/amianthus/.superset/worktrees/06ef6d27-730a-4eec-a0b1-3f2853221478/fix/automatic-tax-retry/server/tests/integration/billing/tax/automatic-tax-no-address-error.test.ts
|
||||
```
|
||||
|
||||
failed during setup with:
|
||||
|
||||
```text
|
||||
error: column organizations_migration_runs.target_limit does not exist
|
||||
code: "internal_error"
|
||||
```
|
||||
|
||||
This happened before the test reached customer/product billing behavior.
|
||||
|
||||
## Investigation
|
||||
|
||||
The checked-in table model is:
|
||||
|
||||
```text
|
||||
shared/models/migrationV2Models/migrationRunTable.ts
|
||||
```
|
||||
|
||||
It defines:
|
||||
|
||||
```ts
|
||||
export const migrationRuns = pgTable(
|
||||
"migration_runs",
|
||||
{
|
||||
...
|
||||
target_limit: numeric({ mode: "number" }),
|
||||
...
|
||||
},
|
||||
);
|
||||
```
|
||||
|
||||
The actual dev DB had `migration_runs`, not `organizations_migration_runs`.
|
||||
I verified with:
|
||||
|
||||
```bash
|
||||
infisical run --env=dev --recursive -- bun -e 'import postgres from "postgres"; const sql = postgres(process.env.DATABASE_URL!); const rows = await sql`select table_schema, table_name from information_schema.tables where table_name like ${"%migration%run%"} order by table_schema, table_name`; console.log(rows); await sql.end();'
|
||||
```
|
||||
|
||||
which returned:
|
||||
|
||||
```text
|
||||
public.migration_item_runs
|
||||
public.migration_runs
|
||||
```
|
||||
|
||||
The `organizations_migration_runs.target_limit` wording appears to be a SQL
|
||||
alias/prefix in the failing query, not the physical table name.
|
||||
|
||||
## Temporary Local Fix Applied
|
||||
|
||||
I first tried the alias-looking table name:
|
||||
|
||||
```bash
|
||||
infisical run --env=dev --recursive -- bun -e 'import postgres from "postgres"; const sql = postgres(process.env.DATABASE_URL!); await sql`ALTER TABLE organizations_migration_runs ADD COLUMN IF NOT EXISTS target_limit numeric`; await sql.end(); console.log("added target_limit if missing");'
|
||||
```
|
||||
|
||||
That failed with:
|
||||
|
||||
```text
|
||||
PostgresError: relation "organizations_migration_runs" does not exist
|
||||
code: "42P01"
|
||||
```
|
||||
|
||||
Then I applied the actual checked-in table name:
|
||||
|
||||
```bash
|
||||
infisical run --env=dev --recursive -- bun -e 'import postgres from "postgres"; const sql = postgres(process.env.DATABASE_URL!); await sql`ALTER TABLE migration_runs ADD COLUMN IF NOT EXISTS target_limit numeric`; await sql.end(); console.log("added migration_runs.target_limit if missing");'
|
||||
```
|
||||
|
||||
That succeeded:
|
||||
|
||||
```text
|
||||
added migration_runs.target_limit if missing
|
||||
```
|
||||
|
||||
After that, the automatic-tax test reached the intended billing failure.
|
||||
|
||||
## Follow-up Needed
|
||||
|
||||
Create/apply the real migration for `migration_runs.target_limit` so future
|
||||
agents and dev environments do not need the manual `ALTER TABLE`.
|
||||
@@ -14,9 +14,7 @@ import { CusService } from "@/internal/customers/CusService";
|
||||
export const getOrCreateStripeCustomer = async ({
|
||||
ctx,
|
||||
customer,
|
||||
options = {
|
||||
updateDb: true,
|
||||
},
|
||||
options,
|
||||
}: {
|
||||
ctx: AutumnContext;
|
||||
customer: Customer;
|
||||
@@ -26,11 +24,15 @@ export const getOrCreateStripeCustomer = async ({
|
||||
};
|
||||
}): Promise<ExpandedStripeCustomer | undefined> => {
|
||||
const { logger } = ctx;
|
||||
const resolvedOptions = {
|
||||
updateDb: true,
|
||||
...options,
|
||||
};
|
||||
|
||||
const currentStripeCustomer = await getExpandedStripeCustomer({
|
||||
ctx,
|
||||
stripeCustomerId: customer.processor?.id,
|
||||
expandTax: options.expandTax,
|
||||
expandTax: resolvedOptions.expandTax,
|
||||
});
|
||||
|
||||
if (currentStripeCustomer) return currentStripeCustomer;
|
||||
@@ -43,11 +45,11 @@ export const getOrCreateStripeCustomer = async ({
|
||||
ctx,
|
||||
customer,
|
||||
options: {
|
||||
expandTax: options.expandTax,
|
||||
expandTax: resolvedOptions.expandTax,
|
||||
},
|
||||
});
|
||||
|
||||
if (options.updateDb) {
|
||||
if (resolvedOptions.updateDb) {
|
||||
await CusService.update({
|
||||
ctx,
|
||||
idOrInternalId: customer.id || customer.internal_id,
|
||||
|
||||
@@ -27,6 +27,7 @@ export const fetchStripeCustomerForBilling = async ({
|
||||
customer: fullCus,
|
||||
options: {
|
||||
expandTax,
|
||||
updateDb: true,
|
||||
},
|
||||
})
|
||||
: await getExpandedStripeCustomer({
|
||||
|
||||
@@ -6,8 +6,10 @@ const hasUsableTaxAddress = (address?: Stripe.Address | null) => {
|
||||
return Boolean(address?.country);
|
||||
};
|
||||
|
||||
const customerHasUsableTaxLocation = (stripeCustomer?: Stripe.Customer) => {
|
||||
if (!stripeCustomer) return true;
|
||||
export const customerHasUsableTaxLocationForStripeTax = (
|
||||
stripeCustomer?: Stripe.Customer,
|
||||
) => {
|
||||
if (!stripeCustomer) return false;
|
||||
|
||||
if (stripeCustomer.tax?.automatic_tax) {
|
||||
return ["supported", "not_collecting"].includes(
|
||||
@@ -35,7 +37,7 @@ export const shouldEnableStripeAutomaticTax = ({
|
||||
|
||||
// Use only the already-fetched Stripe customer. If setup did not fetch one,
|
||||
// do not fetch again on the write path.
|
||||
if (!customerHasUsableTaxLocation(billingContext.stripeCustomer)) {
|
||||
if (!customerHasUsableTaxLocationForStripeTax(billingContext.stripeCustomer)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ import {
|
||||
} from "@/external/stripe/stripeSubUtils/convertSubUtils.js";
|
||||
import { sanitizeSubItems } from "@/external/stripe/stripeSubUtils/getStripeSubItems.js";
|
||||
import { buildAutumnSubscriptionMetadata } from "@/internal/billing/v2/providers/stripe/utils/common/autumnStripeMetadata.js";
|
||||
import { customerHasUsableTaxLocationForStripeTax } from "@/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax.js";
|
||||
import type { AttachParams } from "@/internal/customers/cusProducts/AttachParams.js";
|
||||
import { buildInvoiceMemoFromEntitlements } from "@/internal/invoices/invoiceMemoUtils.js";
|
||||
import { freeTrialToStripeTimestamp } from "@/internal/products/free-trials/freeTrialUtils.js";
|
||||
@@ -63,7 +64,9 @@ export const createStripeSub2 = async ({
|
||||
// Skip auto_tax in invoice mode: send_invoice has no
|
||||
// address-collection UI so Stripe Tax rejects.
|
||||
const wantsAutoTax =
|
||||
!!org.config.automatic_tax && !attachParams.invoiceOnly;
|
||||
!!org.config.automatic_tax &&
|
||||
!attachParams.invoiceOnly &&
|
||||
customerHasUsableTaxLocationForStripeTax(attachParams.stripeCus);
|
||||
|
||||
const subscription = await stripeCli.subscriptions.create({
|
||||
...paymentMethodData,
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
import { addMinutes } from "date-fns";
|
||||
import { Decimal } from "decimal.js";
|
||||
import { payForInvoice } from "@/external/stripe/stripeInvoiceUtils.js";
|
||||
import { customerHasUsableTaxLocationForStripeTax } from "@/internal/billing/v2/providers/stripe/utils/tax/shouldEnableStripeAutomaticTax.js";
|
||||
import { createFullCusProduct } from "@/internal/customers/add-product/createFullCusProduct.js";
|
||||
import { handleCreateCheckout } from "@/internal/customers/add-product/handleCreateCheckout.js";
|
||||
import type { AttachParams } from "@/internal/customers/cusProducts/AttachParams.js";
|
||||
@@ -135,8 +136,10 @@ export const handleOneOffFunction = async ({
|
||||
|
||||
// Skip auto_tax in invoice mode: send_invoice has no
|
||||
// address-collection UI so Stripe Tax rejects.
|
||||
const wantsAutoTax =
|
||||
!!org.config.automatic_tax && !attachParams.invoiceOnly;
|
||||
const wantsAutoTax =
|
||||
!!org.config.automatic_tax &&
|
||||
!attachParams.invoiceOnly &&
|
||||
customerHasUsableTaxLocationForStripeTax(attachParams.stripeCus);
|
||||
|
||||
let stripeInvoice = await stripeCli.invoices.create({
|
||||
customer: customer.processor.id!,
|
||||
|
||||
@@ -24,6 +24,9 @@ export const getStripeCusData = async ({
|
||||
const stripeCus = await getOrCreateStripeCustomer({
|
||||
ctx,
|
||||
customer,
|
||||
options: {
|
||||
expandTax: !!ctx.org.config.automatic_tax,
|
||||
},
|
||||
});
|
||||
|
||||
if (!stripeCus) {
|
||||
|
||||
Reference in New Issue
Block a user