fix(clusters): explain the two saves a region change takes instead of failing the update

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.

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<UpsertClusterSchemaType>) {
+function renderForm(
+	defaultValues: Partial<UpsertClusterSchemaType>,
+	{ clusterId, currentRegions = [], organizationRegions = [], canUseCustomRegions = false }: {
+		clusterId?: string;
+		currentRegions?: RegionCohort[];
+		organizationRegions?: OrganizationRegion[];
+		canUseCustomRegions?: boolean;
+	} = {},
+) {
 	const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } });
 	return render(
 		<QueryClientProvider client={queryClient}>
 			<ClusterForm
 				alreadyUsingFree={false}
+				clusterId={clusterId}
+				currentRegions={currentRegions}
 				defaultValues={{
 					clusterName: 'Preview',
 					abbreviatedName: '',
@@ -70,8 +81,8 @@ function renderForm(defaultValues: Partial<UpsertClusterSchemaType>) {
 				mode={undefined}
 				organization={organization}
 				organizationId="org-a"
-				organizationRegions={[]}
-				canUseCustomRegions={false}
+				organizationRegions={organizationRegions}
+				canUseCustomRegions={canUseCustomRegions}
 				canDefineCustomRegions={false}
 				lockedOrganizationRegionIds={[]}
 				partialUpgrade={null}
@@ -175,3 +186,99 @@ 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<HTMLButtonElement>('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('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);
+
+		await pickOption('US', 'Europe');
+
+		expect(selectedRegionPlans()).toEqual(['Europe / 60ms, small distribution']);
+		expect(swapGuidance()).toBeNull();
+	});
+});

src/features/clusters/upsert/ClusterForm.test.tsx

@@ -57,6 +57,7 @@ function renderVersionForm() {
 			<ClusterForm
 				alreadyUsingFree={false}
 				clusterId="clu-1"
+				currentRegions={[]}
 				defaultValues={{
 					clusterName: 'my-cluster',
 					abbreviatedName: '',

src/features/clusters/upsert/ClusterForm.tsx

@@ -32,6 +32,7 @@ import { z } from 'zod';
 import { ClusterBilling } from './ClusterBilling';
 import { ClusterDetails } from './ClusterDetails';
 import { calculateInstanceFQDN } from './lib/calculateInstanceFQDN';
+import { describeRegionSwap, RegionCohort } from './lib/describeRegionSwap';
 import { PartialUpgrade } from './lib/detectPartialUpgrade';
 import { pickDefaultDeploymentPerformanceAndRegionPlans } from './lib/pickDefaultDeploymentPerformanceAndRegionPlans';
 import { buildRegionLookup, isOrganizationRegionId, MIN_CLUSTER_INSTANCES, regionCohortKey } from './lib/regionLookup';
@@ -41,6 +42,8 @@ import { specifiedAbbreviatedName, UpsertClusterSchema, UpsertClusterSchemaType
 interface ClusterFormProps {
 	alreadyUsingFree: boolean;
 	clusterId?: string;
+	/** The regions the cluster being edited runs in now, as central manager matches them; empty when creating. */
+	currentRegions: readonly RegionCohort[];
 	defaultValues: UpsertClusterSchemaType;
 	deploymentToPerformanceToPlan: Record<string, Record<string, SchemaPlan>>;
 	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,

src/features/clusters/upsert/index.tsx

@@ -29,6 +29,7 @@ import { buildUpgradeVersionOptions } from './lib/buildUpgradeVersionOptions';
 import {
 	calculateDefaultDeploymentPerformanceAndRegionPlans,
 } from './lib/calculateDefaultDeploymentPerformanceAndRegionPlans';
+import { regionCohortsOfPlans } from './lib/describeRegionSwap';
 import { detectPartialUpgrade } from './lib/detectPartialUpgrade';
 import { buildRegionLookup, isOrganizationRegionId } from './lib/regionLookup';
 import { buildRegionPlanDefaults, migrateDraftRegionPlans } from './lib/regionPlanDefaults';
@@ -137,6 +138,21 @@ export function UpsertCluster() {
 		return false;
 	}, [cluster?.id, organization?.clusters, planTypes]);
 
+	const currentRegions = useMemo(
+		() =>
+			clusterId
+				? regionCohortsOfPlans(
+					cluster?.plans ?? [],
+					buildRegionLookup(
+						[...(regionLocationsColocated ?? []), ...(regionLocationsDedicated ?? [])],
+						organizationRegions,
+						cloudProvider,
+					),
+				)
+				: [],
+		[cloudProvider, cluster, clusterId, organizationRegions, regionLocationsColocated, regionLocationsDedicated],
+	);
+
 	const deploymentToPerformanceToPlan = useMemo<Record<string, Record<string, SchemaPlan>>>(
 		() =>
 			groupThenKeyBy(planTypes?.sort(sortByField('priceUsd')) || [], 'deploymentDescription', 'performanceDescription'),
@@ -337,6 +353,7 @@ export function UpsertCluster() {
 			<ClusterForm
 				alreadyUsingFree={alreadyUsingFree}
 				clusterId={clusterId}
+				currentRegions={currentRegions}
 				defaultValues={defaultValues}
 				deploymentToPerformanceToPlan={deploymentToPerformanceToPlan}
 				harperVersions={harperVersions}

src/features/clusters/upsert/lib/describeRegionSwap.test.ts

@@ -0,0 +1,87 @@
+import type { SchemaRegion } from '@/integrations/api/api.gen';
+import { describe, expect, it } from 'vitest';
+import { describeRegionSwap, RegionCohort, regionCohortsOfPlans } from './describeRegionSwap';
+import { buildRegionLookup } from './regionLookup';
+
+const catalog = (names: string[]): RegionCohort[] => names.map(name => ({ key: name, name }));
+const swap = (currentRegionNames: string[], selectedRegionNames: string[], singleRegionTier = false) =>
+	describeRegionSwap({
+		currentRegions: catalog(currentRegionNames),
+		selectedRegions: catalog(selectedRegionNames),
+		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();
+	});
+
+	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 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 and Canada as additional regions and save, then remove the old regions that are left 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();
+	});
+});