From bef9310febd81b13f26ae1b191cf092ceaad0405 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 00:54:43 -0400 Subject: [PATCH 1/2] fix(clusters): keep the chosen region when a tier's region limit corrects it A tier with allowedRegionIds (the free tier) takes a single region, and the form corrects that region whenever it is the only one left. The correction always jumped to the default US region, so removing the second region, which is what the tier's "only one region" error asks the user to do, also threw away the first region and its latency. Prefer a latency the tier allows in the region the user chose, and fall back to the default region only when the tier offers nothing there. Refs #1275 Co-Authored-By: Claude Opus 5.5 --- src/features/clusters/DESIGN.md | 6 + .../upsert/ClusterForm.regions.test.tsx | 164 ++++++++++++++++++ src/features/clusters/upsert/ClusterForm.tsx | 4 +- 3 files changed, 173 insertions(+), 1 deletion(-) create mode 100644 src/features/clusters/upsert/ClusterForm.regions.test.tsx diff --git a/src/features/clusters/DESIGN.md b/src/features/clusters/DESIGN.md index f5d88357b..14d54caaa 100644 --- a/src/features/clusters/DESIGN.md +++ b/src/features/clusters/DESIGN.md @@ -14,3 +14,9 @@ create prefilled from it, and once that create succeeds the form terminates the saved form values can bring it back after a billing redirect. So `onClusterSavedCallback` gates the terminate on `creating`, not on `sourceClusterId` alone. Without that gate, an update that failed straight away made an edit delete the cluster it was editing. + +A tier with `allowedRegionIds` takes one region, and the form corrects that region whenever it is the only one left, +which includes the moment a second region is removed. A custom region is never corrected, since the allow-list describes +the catalog. The correction keeps the chosen region, matching the catalog entry's name for the stored region id, and +moves only its latency when the tier offers that region at all. It falls back to the default region only when the tier +offers nothing there (#1275). diff --git a/src/features/clusters/upsert/ClusterForm.regions.test.tsx b/src/features/clusters/upsert/ClusterForm.regions.test.tsx new file mode 100644 index 000000000..eccdc68e0 --- /dev/null +++ b/src/features/clusters/upsert/ClusterForm.regions.test.tsx @@ -0,0 +1,164 @@ +/** @vitest-environment jsdom */ +import type { SchemaPlan, SchemaRegion } from '@/integrations/api/api.gen'; +import type { Organization } from '@/integrations/api/api.patch'; +import { groupThenKeyBy } from '@/lib/groupThenKeyBy'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, cleanup, fireEvent, render, screen } from '@testing-library/react'; +import { afterEach, beforeAll, describe, expect, it, vi } from 'vitest'; +import { ClusterForm } from './ClusterForm'; +import type { UpsertClusterSchemaType } from './upsertClusterSchema'; + +vi.mock('@tanstack/react-router', () => { + const router = { invalidate: vi.fn() }; + const navigate = vi.fn(); + return { useNavigate: () => navigate, useRouter: () => router }; +}); + +const regions: SchemaRegion[] = [ + { id: 'us-small', region: 'US', latencyDescription: '90ms, small distribution', instanceCount: 2 }, + { id: 'us-medium', region: 'US', latencyDescription: '70ms, medium distribution', instanceCount: 4 }, + { id: 'eu-small', region: 'Europe', latencyDescription: '60ms, small distribution', instanceCount: 2 }, + { id: 'eu-medium', region: 'Europe', latencyDescription: '45ms, medium distribution', instanceCount: 4 }, + { id: 'ap-small', region: 'Asia', latencyDescription: '170ms, small distribution', instanceCount: 2 }, + { id: 'ap-medium', region: 'Asia', latencyDescription: '140ms, medium distribution', instanceCount: 4 }, +] as SchemaRegion[]; + +const colocated = { deploymentType: 'colocated', deploymentDescription: 'Colocated' }; +const plans = [ + { + ...colocated, + id: 'free', + performanceDescription: 'Free tier', + priceUsd: 0, + allowedRegionIds: ['us-small', 'eu-small'], + }, + { ...colocated, id: 'medium', performanceDescription: 'Medium tier', priceUsd: 250 }, +] as SchemaPlan[]; + +const organization = { id: 'org-a', name: 'Org A', subdomain: 'org-a' } as Organization; + +beforeAll(() => { + Object.assign(Element.prototype, { + hasPointerCapture: () => false, + setPointerCapture: () => {}, + releasePointerCapture: () => {}, + scrollIntoView: () => {}, + }); + window.PointerEvent = class extends MouseEvent {} as typeof PointerEvent; +}); +afterEach(cleanup); + +function renderForm(defaultValues: Partial) { + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + return render( + + + , + ); +} + +// The form stores a region id, so read each row as it shows: the select under its "Region" (or +// "Region N") label, then the latency select that follows it. +function selectedRegionPlans() { + const comboboxes = screen.getAllByRole('combobox'); + return screen.getAllByText(/^Region( \d+)?$/, { selector: 'label' }).map(label => { + const region = label.closest('[data-slot="form-item"]')!.querySelector('[role="combobox"]')!; + const latency = comboboxes[comboboxes.indexOf(region as HTMLElement) + 1]; + return `${region.textContent} / ${latency?.textContent}`; + }); +} + +async function removeRegion(index: number) { + await act(async () => { + fireEvent.click(screen.getAllByRole('button', { name: 'Remove' })[index]); + }); +} + +describe('ClusterForm regions', () => { + it('leaves the first region alone when the second is removed', async () => { + renderForm({ + regionPlans: [ + { regionId: 'eu-medium' }, + { regionId: 'ap-medium' }, + ], + }); + + await removeRegion(1); + + expect(selectedRegionPlans()).toEqual(['Europe / 45ms, medium distribution']); + }); + + it('keeps the first region when the second is removed on a tier that limits regions (#1275)', async () => { + renderForm({ + performanceDescription: 'Free tier', + regionPlans: [ + { regionId: 'eu-medium' }, + { regionId: 'ap-medium' }, + ], + }); + expect(selectedRegionPlans()).toEqual([ + 'Europe / 45ms, medium distribution', + 'Asia / 140ms, medium distribution', + ]); + + await removeRegion(1); + + expect(selectedRegionPlans()).toEqual(['Europe / 60ms, small distribution']); + }); + + it('leaves an allowed first region untouched on a tier that limits regions', async () => { + renderForm({ + performanceDescription: 'Free tier', + regionPlans: [ + { regionId: 'eu-small' }, + { regionId: 'ap-medium' }, + ], + }); + + await removeRegion(1); + + expect(selectedRegionPlans()).toEqual(['Europe / 60ms, small distribution']); + }); + + it('falls back to the default region only when the tier offers nothing in the chosen one', async () => { + renderForm({ + performanceDescription: 'Free tier', + regionPlans: [ + { regionId: 'ap-medium' }, + { regionId: 'eu-medium' }, + ], + }); + + await removeRegion(1); + + expect(selectedRegionPlans()).toEqual(['US / 90ms, small distribution']); + }); +}); diff --git a/src/features/clusters/upsert/ClusterForm.tsx b/src/features/clusters/upsert/ClusterForm.tsx index af3b07c91..f8033843f 100644 --- a/src/features/clusters/upsert/ClusterForm.tsx +++ b/src/features/clusters/upsert/ClusterForm.tsx @@ -337,7 +337,9 @@ export function ClusterForm({ const firstRegionId = selectedRegionPlans[0].regionId; if (!isOrganizationRegionId(firstRegionId) && !allowedRegionIds.includes(firstRegionId)) { const possibleRegions = regionLocations?.filter(r => allowedRegionIds.includes(r.id)); - const regionToSelect = possibleRegions?.find(r => r.region === 'US') || possibleRegions?.[0]; + const chosenRegionName = regionLookup.get(firstRegionId)?.name; + const regionToSelect = possibleRegions?.find(r => r.region === chosenRegionName) + || possibleRegions?.find(r => r.region === 'US') || possibleRegions?.[0]; if (regionToSelect) { form.setValue('regionPlans.0.regionId', regionToSelect.id); void form.trigger(); From d9703d2ccddc23c41ef5b4689079d3172a2ac6ef Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 01:00:38 -0400 Subject: [PATCH 2/2] test(clusters): cover removing the first region on a tier that limits regions Removing the first row shifts the survivor to index 0 before the correction runs, so pin that it keeps its region there too. Refs #1275 Co-Authored-By: Claude Opus 5.5 --- .../upsert/ClusterForm.regions.test.tsx | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/features/clusters/upsert/ClusterForm.regions.test.tsx b/src/features/clusters/upsert/ClusterForm.regions.test.tsx index eccdc68e0..8b4291854 100644 --- a/src/features/clusters/upsert/ClusterForm.regions.test.tsx +++ b/src/features/clusters/upsert/ClusterForm.regions.test.tsx @@ -85,8 +85,7 @@ function renderForm(defaultValues: Partial) { ); } -// The form stores a region id, so read each row as it shows: the select under its "Region" (or -// "Region N") label, then the latency select that follows it. +// The form stores a region id, so read each row as it is shown. function selectedRegionPlans() { const comboboxes = screen.getAllByRole('combobox'); return screen.getAllByText(/^Region( \d+)?$/, { selector: 'label' }).map(label => { @@ -134,6 +133,20 @@ describe('ClusterForm regions', () => { expect(selectedRegionPlans()).toEqual(['Europe / 60ms, small distribution']); }); + it('keeps the surviving region when the first is removed on a tier that limits regions', async () => { + renderForm({ + performanceDescription: 'Free tier', + regionPlans: [ + { regionId: 'ap-medium' }, + { regionId: 'eu-medium' }, + ], + }); + + await removeRegion(0); + + expect(selectedRegionPlans()).toEqual(['Europe / 60ms, small distribution']); + }); + it('leaves an allowed first region untouched on a tier that limits regions', async () => { renderForm({ performanceDescription: 'Free tier',