From 52bc3767ffd112b7bc0582ba060f8d5ccee0587a Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 01:26:51 -0400 Subject: [PATCH 1/4] fix(clusters): explain the two saves a region swap takes before the update fails Central manager refuses a cluster update that leaves every region the cluster runs in while adding another ("Cannot delete all region plans from a cluster and add new region plan in the same update"). Changing an existing cluster's only region went all the way through payment review and then failed with that error. Flag that selection on an edit, under the first region, with the steps: keep the current region, add the new one and save, then remove the old one. A tier that allows a single region cannot do that, so it says the cluster cannot move in place instead. Submitting stays blocked until a current region is kept. Regions compare the way central manager matches them: a catalog region by name, whatever its latency tier, and a custom region by id (regionCohortKey), since its name is free text. Refs #1311 Co-Authored-By: Claude Opus 5.5 --- src/features/clusters/DESIGN.md | 8 ++ .../upsert/ClusterForm.regions.test.tsx | 104 +++++++++++++++++- .../clusters/upsert/ClusterForm.test.tsx | 1 + src/features/clusters/upsert/ClusterForm.tsx | 15 +++ src/features/clusters/upsert/index.tsx | 25 ++++- .../upsert/lib/describeRegionSwap.test.ts | 57 ++++++++++ .../clusters/upsert/lib/describeRegionSwap.ts | 35 ++++++ 7 files changed, 240 insertions(+), 5 deletions(-) create mode 100644 src/features/clusters/upsert/lib/describeRegionSwap.test.ts create mode 100644 src/features/clusters/upsert/lib/describeRegionSwap.ts diff --git a/src/features/clusters/DESIGN.md b/src/features/clusters/DESIGN.md index 14d54caaa..c1825cd14 100644 --- a/src/features/clusters/DESIGN.md +++ b/src/features/clusters/DESIGN.md @@ -20,3 +20,11 @@ which includes the moment a second region is removed. A custom region is never c 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). + +Central manager refuses a cluster update that leaves every region the cluster runs in while adding another, so moving a +cluster takes two saves. It matches a catalog region by name, whatever its latency tier, and a custom region by id +(`regionCohortKey`). `lib/describeRegionSwap.ts` blocks that selection on an edit with the steps instead of letting the +update fail (#1311). It mirrors only the case the server is certain to refuse, so it can never block an update the +server would accept. The server also counts shrinking a kept region as leaving it (a smaller tier of a catalog region, +or fewer units of a custom one); a selection that shrinks every current region while adding one is not mirrored and +still gets the server's error. diff --git a/src/features/clusters/upsert/ClusterForm.regions.test.tsx b/src/features/clusters/upsert/ClusterForm.regions.test.tsx index 8b4291854..8a2c7768d 100644 --- a/src/features/clusters/upsert/ClusterForm.regions.test.tsx +++ b/src/features/clusters/upsert/ClusterForm.regions.test.tsx @@ -1,11 +1,12 @@ /** @vitest-environment jsdom */ import type { SchemaPlan, SchemaRegion } from '@/integrations/api/api.gen'; -import type { Organization } from '@/integrations/api/api.patch'; +import type { Organization, OrganizationRegion } 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 { RegionCohort } from './lib/describeRegionSwap'; import type { UpsertClusterSchemaType } from './upsertClusterSchema'; vi.mock('@tanstack/react-router', () => { @@ -48,12 +49,22 @@ beforeAll(() => { }); afterEach(cleanup); -function renderForm(defaultValues: Partial) { +function renderForm( + defaultValues: Partial, + { clusterId, currentRegions = [], organizationRegions = [], canUseCustomRegions = false }: { + clusterId?: string; + currentRegions?: RegionCohort[]; + organizationRegions?: OrganizationRegion[]; + canUseCustomRegions?: boolean; + } = {}, +) { const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); return render( ) { mode={undefined} organization={organization} organizationId="org-a" - organizationRegions={[]} - canUseCustomRegions={false} + organizationRegions={organizationRegions} + canUseCustomRegions={canUseCustomRegions} canDefineCustomRegions={false} lockedOrganizationRegionIds={[]} partialUpgrade={null} @@ -175,3 +186,88 @@ describe('ClusterForm regions', () => { expect(selectedRegionPlans()).toEqual(['US / 90ms, small distribution']); }); }); + +async function pickOption(currentText: string, option: string) { + const trigger = screen.getAllByRole('combobox').find(combobox => combobox.textContent === currentText); + if (!trigger) { + throw new Error(`No select is showing ${currentText}`); + } + await act(async () => { + fireEvent.click(trigger); + }); + await act(async () => { + fireEvent.click(screen.getByRole('option', { name: option })); + }); +} + +const submitButton = () => + screen.getByRole('button', { name: /Confirm Payment Details|Edit Cluster/ }); +const swapGuidance = () => screen.queryByText(/can't move out of all its current regions/); + +describe('ClusterForm editing regions', () => { + const editing = { clusterId: 'clu-a', currentRegions: [{ key: 'US', name: 'US' }] }; + const runningInUs = { regionPlans: [{ regionId: 'us-small' }] }; + + it('explains the two saves a region swap takes instead of letting the update fail (#1311)', async () => { + renderForm(runningInUs, editing); + + await pickOption('US', 'Europe'); + + expect(selectedRegionPlans()).toEqual(['Europe / 60ms, small distribution']); + expect(swapGuidance()?.textContent).toBe( + "A cluster can't move out of all its current regions in one update. Keep US, add Europe as an additional region and save, then remove US once that update has finished.", + ); + expect(submitButton().disabled).toBe(true); + }); + + it('clears the guidance once a current region is kept alongside the new one', async () => { + renderForm(runningInUs, editing); + await pickOption('US', 'Europe'); + expect(swapGuidance()).not.toBeNull(); + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Add Additional Region Usage' })); + }); + await pickOption('Asia', 'US'); + + expect(swapGuidance()).toBeNull(); + expect(submitButton().disabled).toBe(false); + }); + + it('says a single-region tier cannot move in place', async () => { + renderForm({ ...runningInUs, performanceDescription: 'Free tier' }, editing); + + await pickOption('US', 'Europe'); + + expect(swapGuidance()?.textContent).toBe( + "A cluster can't move out of all its current regions in one update, and this tier allows only one region, so it can't move from US to Europe. Create a new cluster in Europe instead.", + ); + }); + + it('matches a custom region by id, so a region added beside it needs no guidance', async () => { + renderForm({ regionPlans: [{ regionId: 'oreg-edge', quantity: 1 }] }, { + clusterId: 'clu-a', + currentRegions: [{ key: 'oreg-edge', name: 'Edge' }], + organizationRegions: [ + { id: 'oreg-edge', organizationId: 'org-a', name: 'Edge', placement: { gcp: ['us-east1', 'us-west1'] } }, + ], + canUseCustomRegions: true, + }); + + await act(async () => { + fireEvent.click(screen.getByRole('button', { name: 'Add Additional Region Usage' })); + }); + + expect(swapGuidance()).toBeNull(); + expect(submitButton().disabled).toBe(false); + }); + + it('never applies to a new cluster', async () => { + renderForm(runningInUs); + + await pickOption('US', 'Europe'); + + expect(selectedRegionPlans()).toEqual(['Europe / 60ms, small distribution']); + expect(swapGuidance()).toBeNull(); + }); +}); diff --git a/src/features/clusters/upsert/ClusterForm.test.tsx b/src/features/clusters/upsert/ClusterForm.test.tsx index dc659a8df..a20da94e7 100644 --- a/src/features/clusters/upsert/ClusterForm.test.tsx +++ b/src/features/clusters/upsert/ClusterForm.test.tsx @@ -57,6 +57,7 @@ function renderVersionForm() { >; harperVersions: HarperVersionsResponse | undefined; @@ -64,6 +67,7 @@ interface ClusterFormProps { export function ClusterForm({ alreadyUsingFree, clusterId, + currentRegions, defaultValues, deploymentToPerformanceToPlan, harperVersions, @@ -157,6 +161,7 @@ export function ClusterForm({ : dedicatedRegionLookup; let totalInstances = 0; let firstOrganizationRegionIndex = -1; + const selectedRegions: RegionCohort[] = []; for (let i = 0; i < data.regionPlans.length; i++) { const regionPlan = data.regionPlans[i]; const region = regionLookup.get(regionPlan.regionId); @@ -171,6 +176,7 @@ export function ClusterForm({ continue; } const cohort = regionCohortKey(region); + selectedRegions.push({ key: cohort, name: region.name }); if (!names.has(cohort)) { names.add(cohort); } else { @@ -236,11 +242,20 @@ export function ClusterForm({ message: `A cluster needs at least ${MIN_CLUSTER_INSTANCES} instances in total.`, }); } + const regionSwap = describeRegionSwap({ + currentRegions, + selectedRegions, + singleRegionTier: !selectedPlan?.priceUsd || !!selectedPlan.allowedRegionIds?.length, + }); + if (regionSwap) { + ctx.addIssue({ code: 'custom', path: ['regionPlans.0.regionId'], message: regionSwap }); + } } }, [ alreadyUsingFree, canUseCustomRegions, colocatedRegionLookup, + currentRegions, dedicatedRegionLookup, deploymentToPerformanceToPlan, mode, diff --git a/src/features/clusters/upsert/index.tsx b/src/features/clusters/upsert/index.tsx index 3c2aab600..cacbde18f 100644 --- a/src/features/clusters/upsert/index.tsx +++ b/src/features/clusters/upsert/index.tsx @@ -29,8 +29,9 @@ import { buildUpgradeVersionOptions } from './lib/buildUpgradeVersionOptions'; import { calculateDefaultDeploymentPerformanceAndRegionPlans, } from './lib/calculateDefaultDeploymentPerformanceAndRegionPlans'; +import { RegionCohort } from './lib/describeRegionSwap'; import { detectPartialUpgrade } from './lib/detectPartialUpgrade'; -import { buildRegionLookup, isOrganizationRegionId } from './lib/regionLookup'; +import { buildRegionLookup, isOrganizationRegionId, regionCohortKey } from './lib/regionLookup'; import { buildRegionPlanDefaults, migrateDraftRegionPlans } from './lib/regionPlanDefaults'; import { RegionPlanEntry, UpsertClusterSchema, UpsertClusterSchemaType } from './upsertClusterSchema'; @@ -137,6 +138,27 @@ export function UpsertCluster() { return false; }, [cluster?.id, organization?.clusters, planTypes]); + const currentRegions = useMemo(() => { + if (!clusterId) { + return []; + } + const lookup = buildRegionLookup( + [...(regionLocationsColocated ?? []), ...(regionLocationsDedicated ?? [])], + organizationRegions, + cloudProvider, + ); + return (cluster?.plans ?? []).flatMap(plan => { + const region = plan.regionId ? lookup.get(plan.regionId) : undefined; + if (region) { + return [{ key: regionCohortKey(region), name: region.name }]; + } + if (isOrganizationRegionId(plan.regionId)) { + return [{ key: plan.regionId!, name: plan.region ?? plan.regionId! }]; + } + return plan.region ? [{ key: plan.region, name: plan.region }] : []; + }); + }, [cloudProvider, cluster, clusterId, organizationRegions, regionLocationsColocated, regionLocationsDedicated]); + const deploymentToPerformanceToPlan = useMemo>>( () => groupThenKeyBy(planTypes?.sort(sortByField('priceUsd')) || [], 'deploymentDescription', 'performanceDescription'), @@ -337,6 +359,7 @@ export function UpsertCluster() { names.map(name => ({ key: name, name })); +const swap = (currentRegionNames: string[], selectedRegionNames: string[], singleRegionTier = false) => + describeRegionSwap({ + currentRegions: catalog(currentRegionNames), + selectedRegions: catalog(selectedRegionNames), + singleRegionTier, + }); + +describe('describeRegionSwap', () => { + it('stays quiet for a new cluster, which has no current regions', () => { + expect(swap([], ['Europe'])).toBeNull(); + }); + + it('stays quiet while any current region is kept', () => { + expect(swap(['US'], ['US'])).toBeNull(); + expect(swap(['US'], ['Europe', 'US'])).toBeNull(); + expect(swap(['US', 'Asia'], ['Asia', 'Europe'])).toBeNull(); + }); + + it('stays quiet until a region is actually chosen', () => { + expect(swap(['US'], [])).toBeNull(); + expect(swap(['US'], [''])).toBeNull(); + }); + + it('names the region to keep and the one to add', () => { + expect(swap(['US'], ['', 'Europe'])).toBe( + "A cluster can't move out of all its current regions in one update. Keep US, add Europe as an additional region and save, then remove US once that update has finished.", + ); + }); + + it('asks to keep one of several current regions', () => { + expect(swap(['US', 'Asia', 'US'], ['Europe', 'Canada'])).toBe( + "A cluster can't move out of all its current regions in one update. Keep one of US and Asia, add Europe as an additional region and save, then remove the others once that update has finished.", + ); + }); + + it('does not suggest a second region on a tier that allows only one', () => { + expect(swap(['US'], ['Europe'], true)).toBe( + "A cluster can't move out of all its current regions in one update, and this tier allows only one region, so it can't move from US to Europe. Create a new cluster in Europe instead.", + ); + }); + + it('matches a custom region by id, as central manager does, not by its name', () => { + const edge = { key: 'oreg-1', name: 'US' }; + expect(describeRegionSwap({ + currentRegions: [edge], + selectedRegions: [{ ...edge, name: 'US, renamed' }, ...catalog(['Europe'])], + singleRegionTier: false, + })).toBeNull(); + expect(describeRegionSwap({ currentRegions: [edge], selectedRegions: catalog(['US']), singleRegionTier: false })) + .not.toBeNull(); + }); +}); diff --git a/src/features/clusters/upsert/lib/describeRegionSwap.ts b/src/features/clusters/upsert/lib/describeRegionSwap.ts new file mode 100644 index 000000000..726f58866 --- /dev/null +++ b/src/features/clusters/upsert/lib/describeRegionSwap.ts @@ -0,0 +1,35 @@ +const regionList = new Intl.ListFormat('en', { style: 'long', type: 'conjunction' }); + +/** + * A region as central manager matches it across an update: a catalog region by name, whatever its latency tier, and a + * custom region by id (`regionCohortKey`), since its name is free text. + */ +export interface RegionCohort { + key: string; + name: string; +} + +/** + * Central manager refuses an update that leaves every region a cluster runs in while adding a new one, so moving a + * cluster takes two saves. Returns guidance for a selection it would refuse, or null. + */ +export function describeRegionSwap({ currentRegions, selectedRegions, singleRegionTier }: { + currentRegions: readonly RegionCohort[]; + selectedRegions: readonly RegionCohort[]; + singleRegionTier: boolean; +}): string | null { + const current = new Map(currentRegions.filter(({ key }) => key).map(({ key, name }) => [key, name])); + const selected = selectedRegions.filter(({ key }) => key); + if (!current.size || !selected.length || selected.some(({ key }) => current.has(key))) { + return null; + } + const from = regionList.format([...current.values()]); + const to = selected[0].name; + const rule = "A cluster can't move out of all its current regions in one update"; + if (singleRegionTier) { + return `${rule}, and this tier allows only one region, so it can't move from ${from} to ${to}. Create a new cluster in ${to} instead.`; + } + return current.size === 1 + ? `${rule}. Keep ${from}, add ${to} as an additional region and save, then remove ${from} once that update has finished.` + : `${rule}. Keep one of ${from}, add ${to} as an additional region and save, then remove the others once that update has finished.`; +} From 7502e12dae88739dd358127a6886147e0bcd7313 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 01:32:47 -0400 Subject: [PATCH 2/4] fix(clusters): say which regions to remove after a multi-region swap With several current regions the guidance said "remove the others" after the first save, but the regions left by then are the ones the user kept. Name those instead, skip the work entirely when creating, and test how a cluster's plans resolve to the regions the check compares: a catalog plan by its region name, a custom region by its id. Refs #1311 Co-Authored-By: Claude Opus 5.5 --- src/features/clusters/upsert/index.tsx | 38 ++++++++----------- .../upsert/lib/describeRegionSwap.test.ts | 37 ++++++++++++++++-- .../clusters/upsert/lib/describeRegionSwap.ts | 22 ++++++++++- 3 files changed, 71 insertions(+), 26 deletions(-) diff --git a/src/features/clusters/upsert/index.tsx b/src/features/clusters/upsert/index.tsx index cacbde18f..21d7bbbdb 100644 --- a/src/features/clusters/upsert/index.tsx +++ b/src/features/clusters/upsert/index.tsx @@ -29,9 +29,9 @@ import { buildUpgradeVersionOptions } from './lib/buildUpgradeVersionOptions'; import { calculateDefaultDeploymentPerformanceAndRegionPlans, } from './lib/calculateDefaultDeploymentPerformanceAndRegionPlans'; -import { RegionCohort } from './lib/describeRegionSwap'; +import { regionCohortsOfPlans } from './lib/describeRegionSwap'; import { detectPartialUpgrade } from './lib/detectPartialUpgrade'; -import { buildRegionLookup, isOrganizationRegionId, regionCohortKey } from './lib/regionLookup'; +import { buildRegionLookup, isOrganizationRegionId } from './lib/regionLookup'; import { buildRegionPlanDefaults, migrateDraftRegionPlans } from './lib/regionPlanDefaults'; import { RegionPlanEntry, UpsertClusterSchema, UpsertClusterSchemaType } from './upsertClusterSchema'; @@ -138,26 +138,20 @@ export function UpsertCluster() { return false; }, [cluster?.id, organization?.clusters, planTypes]); - const currentRegions = useMemo(() => { - if (!clusterId) { - return []; - } - const lookup = buildRegionLookup( - [...(regionLocationsColocated ?? []), ...(regionLocationsDedicated ?? [])], - organizationRegions, - cloudProvider, - ); - return (cluster?.plans ?? []).flatMap(plan => { - const region = plan.regionId ? lookup.get(plan.regionId) : undefined; - if (region) { - return [{ key: regionCohortKey(region), name: region.name }]; - } - if (isOrganizationRegionId(plan.regionId)) { - return [{ key: plan.regionId!, name: plan.region ?? plan.regionId! }]; - } - return plan.region ? [{ key: plan.region, name: plan.region }] : []; - }); - }, [cloudProvider, cluster, clusterId, organizationRegions, regionLocationsColocated, regionLocationsDedicated]); + const currentRegions = useMemo( + () => + clusterId + ? regionCohortsOfPlans( + cluster?.plans ?? [], + buildRegionLookup( + [...(regionLocationsColocated ?? []), ...(regionLocationsDedicated ?? [])], + organizationRegions, + cloudProvider, + ), + ) + : [], + [cloudProvider, cluster, clusterId, organizationRegions, regionLocationsColocated, regionLocationsDedicated], + ); const deploymentToPerformanceToPlan = useMemo>>( () => diff --git a/src/features/clusters/upsert/lib/describeRegionSwap.test.ts b/src/features/clusters/upsert/lib/describeRegionSwap.test.ts index e321ea026..5147d910e 100644 --- a/src/features/clusters/upsert/lib/describeRegionSwap.test.ts +++ b/src/features/clusters/upsert/lib/describeRegionSwap.test.ts @@ -1,5 +1,7 @@ +import type { SchemaRegion } from '@/integrations/api/api.gen'; import { describe, expect, it } from 'vitest'; -import { describeRegionSwap, RegionCohort } from './describeRegionSwap'; +import { describeRegionSwap, RegionCohort, regionCohortsOfPlans } from './describeRegionSwap'; +import { buildRegionLookup } from './regionLookup'; // A catalog region's cohort is its name. const catalog = (names: string[]): RegionCohort[] => names.map(name => ({ key: name, name })); @@ -10,6 +12,35 @@ const swap = (currentRegionNames: string[], selectedRegionNames: string[], singl singleRegionTier, }); +describe('regionCohortsOfPlans', () => { + const lookup = buildRegionLookup( + [ + { id: 'us-1', region: 'US', latencyDescription: '83ms', instanceCount: 2 }, + { id: 'eu-1', region: 'Europe', latencyDescription: '58ms', instanceCount: 2 }, + ] as SchemaRegion[], + [{ id: 'oreg-1', organizationId: 'org-a', name: 'Edge', placement: { gcp: ['us-east1', 'us-west1'] } }], + undefined, + ); + + it('keys a catalog plan by its region name and a custom region by its id', () => { + expect(regionCohortsOfPlans([ + { planId: 'shared', regionId: 'eu-1' }, + { planId: 'shared', regionId: 'oreg-1', quantity: 2 }, + ], lookup)).toEqual([{ key: 'Europe', name: 'Europe' }, { key: 'oreg-1', name: 'Edge' }]); + }); + + it('falls back to what a plan the lookup lacks carries', () => { + expect(regionCohortsOfPlans([ + { planId: 'shared', regionId: 'retired-1', region: 'Asia' }, + { planId: 'shared', regionId: 'oreg-gone', region: 'Old edge' }, + ], lookup)).toEqual([{ key: 'Asia', name: 'Asia' }, { key: 'oreg-gone', name: 'Old edge' }]); + }); + + it('skips plans with no region, such as self-hosted instances', () => { + expect(regionCohortsOfPlans([{ planId: 'self-hosted-0' }], lookup)).toEqual([]); + }); +}); + describe('describeRegionSwap', () => { it('stays quiet for a new cluster, which has no current regions', () => { expect(swap([], ['Europe'])).toBeNull(); @@ -32,9 +63,9 @@ describe('describeRegionSwap', () => { ); }); - it('asks to keep one of several current regions', () => { + it('asks to keep at least one of several current regions', () => { expect(swap(['US', 'Asia', 'US'], ['Europe', 'Canada'])).toBe( - "A cluster can't move out of all its current regions in one update. Keep one of US and Asia, add Europe as an additional region and save, then remove the others once that update has finished.", + "A cluster can't move out of all its current regions in one update. Keep at least one of US and Asia, add Europe as an additional region and save, then remove the old regions that are left once that update has finished.", ); }); diff --git a/src/features/clusters/upsert/lib/describeRegionSwap.ts b/src/features/clusters/upsert/lib/describeRegionSwap.ts index 726f58866..8242183d2 100644 --- a/src/features/clusters/upsert/lib/describeRegionSwap.ts +++ b/src/features/clusters/upsert/lib/describeRegionSwap.ts @@ -1,3 +1,6 @@ +import { RegionPlan } from '@/integrations/api/api.patch'; +import { isOrganizationRegionId, regionCohortKey, RegionLookup } from './regionLookup'; + const regionList = new Intl.ListFormat('en', { style: 'long', type: 'conjunction' }); /** @@ -9,6 +12,20 @@ export interface RegionCohort { name: string; } +/** A cluster's plans as central manager matches them; a plan the lookup lacks still matches by what it carries. */ +export function regionCohortsOfPlans(plans: readonly RegionPlan[], lookup: RegionLookup): RegionCohort[] { + return plans.flatMap(plan => { + const region = plan.regionId ? lookup.get(plan.regionId) : undefined; + if (region) { + return [{ key: regionCohortKey(region), name: region.name }]; + } + if (plan.regionId && isOrganizationRegionId(plan.regionId)) { + return [{ key: plan.regionId, name: plan.region ?? plan.regionId }]; + } + return plan.region ? [{ key: plan.region, name: plan.region }] : []; + }); +} + /** * Central manager refuses an update that leaves every region a cluster runs in while adding a new one, so moving a * cluster takes two saves. Returns guidance for a selection it would refuse, or null. @@ -18,6 +35,9 @@ export function describeRegionSwap({ currentRegions, selectedRegions, singleRegi selectedRegions: readonly RegionCohort[]; singleRegionTier: boolean; }): string | null { + if (!currentRegions.length) { + return null; + } const current = new Map(currentRegions.filter(({ key }) => key).map(({ key, name }) => [key, name])); const selected = selectedRegions.filter(({ key }) => key); if (!current.size || !selected.length || selected.some(({ key }) => current.has(key))) { @@ -31,5 +51,5 @@ export function describeRegionSwap({ currentRegions, selectedRegions, singleRegi } return current.size === 1 ? `${rule}. Keep ${from}, add ${to} as an additional region and save, then remove ${from} once that update has finished.` - : `${rule}. Keep one of ${from}, add ${to} as an additional region and save, then remove the others once that update has finished.`; + : `${rule}. Keep at least one of ${from}, add ${to} as an additional region and save, then remove the old regions that are left once that update has finished.`; } From c215cde7b6456855407058bd7304a378e01fee81 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 12 Oct 2026 11:59:29 -0400 Subject: [PATCH 3/4] test(clusters): pin the one-region swap guidance for a tier missing from the catalog With no plan for the selected tier the price is 0, so the form hides Add Additional Region Usage. The guidance has to say to create a new cluster, not to add a region the form won't let the user add. Refs #1311 Co-Authored-By: Claude Opus 5.5 --- .../clusters/upsert/ClusterForm.regions.test.tsx | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/features/clusters/upsert/ClusterForm.regions.test.tsx b/src/features/clusters/upsert/ClusterForm.regions.test.tsx index 8a2c7768d..c252ceed5 100644 --- a/src/features/clusters/upsert/ClusterForm.regions.test.tsx +++ b/src/features/clusters/upsert/ClusterForm.regions.test.tsx @@ -262,6 +262,17 @@ describe('ClusterForm editing regions', () => { expect(submitButton().disabled).toBe(false); }); + it('treats a tier missing from the catalog as one region, as the form does', async () => { + renderForm({ ...runningInUs, deploymentDescription: 'Retired', performanceDescription: 'Retired tier' }, editing); + + await pickOption('US', 'Europe'); + + expect(screen.queryByRole('button', { name: 'Add Additional Region Usage' })).toBeNull(); + expect(swapGuidance()?.textContent).toBe( + "A cluster can't move out of all its current regions in one update, and this tier allows only one region, so it can't move from US to Europe. Create a new cluster in Europe instead.", + ); + }); + it('never applies to a new cluster', async () => { renderForm(runningInUs); From c81ec95553eb8429b3cd4eadd0df4a9a6c9e7cda Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 12 Oct 2026 14:13:32 -0400 Subject: [PATCH 4/4] fix(clusters): name every region a swap adds in the two-save steps With Europe and Canada chosen to replace US, the steps said to add Europe and left Canada out, so following them to the letter dropped a region. Name each region being added, and drop two comments that only restated the code. Refs #1311 Co-Authored-By: Claude Opus 5.5 --- .../clusters/upsert/lib/describeRegionSwap.test.ts | 3 +-- .../clusters/upsert/lib/describeRegionSwap.ts | 11 +++++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/src/features/clusters/upsert/lib/describeRegionSwap.test.ts b/src/features/clusters/upsert/lib/describeRegionSwap.test.ts index 5147d910e..044b3ed06 100644 --- a/src/features/clusters/upsert/lib/describeRegionSwap.test.ts +++ b/src/features/clusters/upsert/lib/describeRegionSwap.test.ts @@ -3,7 +3,6 @@ import { describe, expect, it } from 'vitest'; import { describeRegionSwap, RegionCohort, regionCohortsOfPlans } from './describeRegionSwap'; import { buildRegionLookup } from './regionLookup'; -// A catalog region's cohort is its name. const catalog = (names: string[]): RegionCohort[] => names.map(name => ({ key: name, name })); const swap = (currentRegionNames: string[], selectedRegionNames: string[], singleRegionTier = false) => describeRegionSwap({ @@ -65,7 +64,7 @@ describe('describeRegionSwap', () => { it('asks to keep at least one of several current regions', () => { expect(swap(['US', 'Asia', 'US'], ['Europe', 'Canada'])).toBe( - "A cluster can't move out of all its current regions in one update. Keep at least one of US and Asia, add Europe as an additional region and save, then remove the old regions that are left once that update has finished.", + "A cluster can't move out of all its current regions in one update. Keep at least one of US and Asia, add Europe and Canada as additional regions and save, then remove the old regions that are left once that update has finished.", ); }); diff --git a/src/features/clusters/upsert/lib/describeRegionSwap.ts b/src/features/clusters/upsert/lib/describeRegionSwap.ts index 8242183d2..f491d2a65 100644 --- a/src/features/clusters/upsert/lib/describeRegionSwap.ts +++ b/src/features/clusters/upsert/lib/describeRegionSwap.ts @@ -28,7 +28,7 @@ export function regionCohortsOfPlans(plans: readonly RegionPlan[], lookup: Regio /** * Central manager refuses an update that leaves every region a cluster runs in while adding a new one, so moving a - * cluster takes two saves. Returns guidance for a selection it would refuse, or null. + * cluster takes two saves. */ export function describeRegionSwap({ currentRegions, selectedRegions, singleRegionTier }: { currentRegions: readonly RegionCohort[]; @@ -44,12 +44,15 @@ export function describeRegionSwap({ currentRegions, selectedRegions, singleRegi return null; } const from = regionList.format([...current.values()]); - const to = selected[0].name; + const added = [...new Map(selected.map(({ key, name }) => [key, name])).values()]; const rule = "A cluster can't move out of all its current regions in one update"; if (singleRegionTier) { + const [to] = added; return `${rule}, and this tier allows only one region, so it can't move from ${from} to ${to}. Create a new cluster in ${to} instead.`; } + const additional = added.length === 1 ? 'an additional region' : 'additional regions'; + const add = `add ${regionList.format(added)} as ${additional}`; return current.size === 1 - ? `${rule}. Keep ${from}, add ${to} as an additional region and save, then remove ${from} once that update has finished.` - : `${rule}. Keep at least one of ${from}, add ${to} as an additional region and save, then remove the old regions that are left once that update has finished.`; + ? `${rule}. Keep ${from}, ${add} and save, then remove ${from} once that update has finished.` + : `${rule}. Keep at least one of ${from}, ${add} and save, then remove the old regions that are left once that update has finished.`; }