Skip to content

Commit 6dddb52

Browse files
gu-stavgu-stav
committed
refactor(deploy): restructure studio deploy on the shared deploy checks
Builds on the core app refactor: deployStudio now runs through a single createStudioDeployment using the shared checks seam, with studio target resolution (resolveStudioDeployTarget) and the studio target check added to the shared modules. With both deploy paths refactored, the two interactive adapters merge into one findUserApplication module and the two creators into createUserApplication, and cannotPromptForStudioHost gets one home. deployStudio carries the tightened seams through: it asks getWorkbench for an isWorkbenchApp boolean, and the studio target models isExternal as its own field. No behavior change — the integration deploy tests pass unchanged.
1 parent 9260e52 commit 6dddb52

12 files changed

Lines changed: 956 additions & 609 deletions

‎packages/@sanity/cli/src/actions/deploy/__tests__/deployChecks.test.ts‎

Lines changed: 118 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
import {beforeEach, describe, expect, test, vi} from 'vitest'
22

33
import {type UserApplication} from '../../../services/userApplications.js'
4-
import {checkAppTarget, createAggregatingChecks} from '../deployChecks.js'
5-
import {resolveAppDeployTarget} from '../resolveDeployTarget.js'
4+
import {
5+
checkAppTarget,
6+
checkStudioTarget,
7+
createAggregatingChecks,
8+
type DeployTarget,
9+
} from '../deployChecks.js'
10+
import {resolveAppDeployTarget, resolveStudioDeployTarget} from '../resolveDeployTarget.js'
611

712
vi.mock('../resolveDeployTarget.js', () => ({
813
resolveAppDeployTarget: vi.fn(),
14+
resolveStudioDeployTarget: vi.fn(),
915
}))
1016

17+
const mockResolveStudio = vi.mocked(resolveStudioDeployTarget)
1118
const mockResolveApp = vi.mocked(resolveAppDeployTarget)
1219

1320
function application(overrides: Partial<UserApplication> = {}): UserApplication {
@@ -53,6 +60,115 @@ describe('createAggregatingChecks', () => {
5360
})
5461
})
5562

63+
const studioArgs = {
64+
appId: undefined,
65+
isExternal: false,
66+
projectId: 'project-1',
67+
studioHost: undefined,
68+
urlFlag: undefined,
69+
}
70+
71+
describe('checkStudioTarget', () => {
72+
test('found → pass check and an existing target', async () => {
73+
mockResolveStudio.mockResolvedValue({application: application(), type: 'found'})
74+
const checks = createAggregatingChecks()
75+
76+
const target: DeployTarget | null = await checkStudioTarget(checks, studioArgs)
77+
78+
expect(target).toEqual({
79+
appId: 'app-1',
80+
exists: true,
81+
host: 'my-studio',
82+
isExternal: false,
83+
type: 'studio',
84+
})
85+
expect(checks.all()).toContainEqual(expect.objectContaining({name: 'target', status: 'pass'}))
86+
})
87+
88+
test('would-create → pass check and a to-be-created target', async () => {
89+
mockResolveStudio.mockResolvedValue({appHost: 'new-studio', type: 'would-create'})
90+
const checks = createAggregatingChecks()
91+
92+
const target = await checkStudioTarget(checks, studioArgs)
93+
94+
expect(target).toEqual({
95+
appId: null,
96+
exists: false,
97+
host: 'new-studio',
98+
isExternal: false,
99+
type: 'studio',
100+
})
101+
expect(checks.all()[0]?.status).toBe('pass')
102+
})
103+
104+
test('external would-create → external studio target', async () => {
105+
mockResolveStudio.mockResolvedValue({
106+
appHost: 'https://studio.example.com',
107+
type: 'would-create',
108+
})
109+
const checks = createAggregatingChecks()
110+
111+
const target = await checkStudioTarget(checks, {...studioArgs, isExternal: true})
112+
113+
expect(target).toEqual({
114+
appId: null,
115+
exists: false,
116+
host: 'https://studio.example.com',
117+
isExternal: true,
118+
type: 'studio',
119+
})
120+
expect(checks.all()[0]?.status).toBe('pass')
121+
})
122+
123+
test('needs-input → fail check (unattended cannot prompt)', async () => {
124+
mockResolveStudio.mockResolvedValue({existing: [], type: 'needs-input'})
125+
const checks = createAggregatingChecks()
126+
127+
const target = await checkStudioTarget(checks, studioArgs)
128+
129+
expect(target).toBeNull()
130+
expect(checks.all()[0]).toMatchObject({name: 'target', status: 'fail'})
131+
expect(checks.all()[0]?.message).toContain('Cannot prompt for studio hostname')
132+
})
133+
134+
test('invalid → fail check with the resolution message', async () => {
135+
mockResolveStudio.mockResolvedValue({
136+
message: 'Cannot find app with app ID nope',
137+
reason: 'app-not-found',
138+
type: 'invalid',
139+
})
140+
const checks = createAggregatingChecks()
141+
142+
const target = await checkStudioTarget(checks, {...studioArgs, appId: 'nope'})
143+
144+
expect(target).toBeNull()
145+
expect(checks.all()[0]).toMatchObject({
146+
message: 'Cannot find app with app ID nope',
147+
status: 'fail',
148+
})
149+
})
150+
151+
test('blocked → skip check', async () => {
152+
mockResolveStudio.mockResolvedValue({message: 'api.projectId is missing', type: 'blocked'})
153+
const checks = createAggregatingChecks()
154+
155+
const target = await checkStudioTarget(checks, studioArgs)
156+
157+
expect(target).toBeNull()
158+
expect(checks.all()[0]?.status).toBe('skip')
159+
})
160+
161+
test('a thrown resolution becomes a fail check', async () => {
162+
mockResolveStudio.mockRejectedValue(new Error('network down'))
163+
const checks = createAggregatingChecks()
164+
165+
const target = await checkStudioTarget(checks, studioArgs)
166+
167+
expect(target).toBeNull()
168+
expect(checks.all()[0]?.message).toContain('Failed to resolve deploy target: network down')
169+
})
170+
})
171+
56172
describe('checkAppTarget', () => {
57173
test('found → pass check, existing app and target', async () => {
58174
const app = application({appHost: 'app-host', id: 'core-1', title: 'My App', type: 'coreApp'})

‎packages/@sanity/cli/src/actions/deploy/__tests__/resolveDeployTarget.test.ts‎

Lines changed: 94 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,100 @@
11
import {describe, expect, test} from 'vitest'
22

3-
import {resolveAppDeployTarget} from '../resolveDeployTarget.js'
3+
import {resolveAppDeployTarget, resolveStudioDeployTarget} from '../resolveDeployTarget.js'
44

5-
// The verdicts that hit the API (found / would-create / needs-input) are
6-
// exercised end-to-end by the deploy integration tests; this covers the guard
7-
// that short-circuits before any lookup.
5+
// These cases all short-circuit before any user-application lookup — they cover
6+
// the host/URL validation and the missing-config guards, no API access needed.
7+
// The verdicts that do hit the API (found / would-create / needs-input) are
8+
// exercised end-to-end by the deploy integration tests.
9+
10+
const studioBase = {
11+
appId: undefined,
12+
isExternal: false,
13+
projectId: 'project-1',
14+
studioHost: undefined,
15+
urlFlag: undefined,
16+
}
17+
18+
describe('resolveStudioDeployTarget', () => {
19+
test('internal --url that looks like a URL → invalid, suggests --external', async () => {
20+
const result = await resolveStudioDeployTarget({...studioBase, urlFlag: 'my-studio.com'})
21+
22+
expect(result).toMatchObject({reason: 'invalid-host', type: 'invalid'})
23+
expect(result).toHaveProperty(
24+
'message',
25+
expect.stringContaining('Did you mean to use --external'),
26+
)
27+
})
28+
29+
test('internal --url with illegal hostname characters → invalid', async () => {
30+
const result = await resolveStudioDeployTarget({...studioBase, urlFlag: 'bad_host'})
31+
32+
expect(result).toMatchObject({reason: 'invalid-host', type: 'invalid'})
33+
expect(result).toHaveProperty('message', expect.stringContaining('Invalid studio hostname'))
34+
})
35+
36+
test('external --url that is not a URL → invalid', async () => {
37+
const result = await resolveStudioDeployTarget({
38+
...studioBase,
39+
isExternal: true,
40+
urlFlag: 'not a url',
41+
})
42+
43+
expect(result).toMatchObject({reason: 'invalid-host', type: 'invalid'})
44+
})
45+
46+
test('external --url with a non-http protocol → invalid', async () => {
47+
const result = await resolveStudioDeployTarget({
48+
...studioBase,
49+
isExternal: true,
50+
urlFlag: 'ftp://example.com',
51+
})
52+
53+
expect(result).toMatchObject({reason: 'invalid-host', type: 'invalid'})
54+
expect(result).toHaveProperty('message', expect.stringContaining('http or https'))
55+
})
56+
57+
test('an invalid external studioHost from config is still validated → invalid', async () => {
58+
const result = await resolveStudioDeployTarget({
59+
...studioBase,
60+
isExternal: true,
61+
studioHost: 'bad url',
62+
})
63+
64+
expect(result).toMatchObject({reason: 'invalid-host', type: 'invalid'})
65+
})
66+
67+
test('appId without a projectId → blocked', async () => {
68+
const result = await resolveStudioDeployTarget({
69+
...studioBase,
70+
appId: 'app-1',
71+
projectId: undefined,
72+
})
73+
74+
expect(result).toEqual({message: 'api.projectId is missing', type: 'blocked'})
75+
})
76+
77+
test('a configured studioHost without a projectId → blocked', async () => {
78+
const result = await resolveStudioDeployTarget({
79+
...studioBase,
80+
projectId: undefined,
81+
studioHost: 'my-studio',
82+
})
83+
84+
expect(result).toEqual({message: 'api.projectId is missing', type: 'blocked'})
85+
})
86+
87+
test('a valid external studioHost still needs a projectId → blocked', async () => {
88+
const result = await resolveStudioDeployTarget({
89+
...studioBase,
90+
isExternal: true,
91+
projectId: undefined,
92+
studioHost: 'https://studio.example.com',
93+
})
94+
95+
expect(result).toEqual({message: 'api.projectId is missing', type: 'blocked'})
96+
})
97+
})
898

999
describe('resolveAppDeployTarget', () => {
10100
test('no appId and no organizationId → blocked', async () => {

packages/@sanity/cli/src/actions/deploy/createStudioUserApplication.ts renamed to packages/@sanity/cli/src/actions/deploy/createUserApplication.ts

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import {CLIError} from '@oclif/core/errors'
2-
import {input} from '@sanity/cli-core/ux'
2+
import {input, spinner} from '@sanity/cli-core/ux'
3+
import {customAlphabet} from 'nanoid'
34

45
import {createUserApplication, type UserApplication} from '../../services/userApplications.js'
6+
import {NO_ORGANIZATION_ID} from '../../util/errorMessages.js'
57
import {deployDebug} from './deployDebug.js'
68
import {normalizeUrl, validateUrl} from './urlUtils.js'
79

@@ -22,6 +24,7 @@ interface CreateStudioUserApplicationOptions {
2224
urlType?: 'external' | 'internal'
2325
}
2426

27+
/** Prompts for a studio hostname (or external URL) and registers it. */
2528
export async function createStudioUserApplication(options: CreateStudioUserApplicationOptions) {
2629
const {projectId, urlType = 'internal'} = options
2730
const {promise, resolve} = promiseWithResolvers<UserApplication>()
@@ -49,11 +52,7 @@ export async function createStudioUserApplication(options: CreateStudioUserAppli
4952
try {
5053
const response = await createUserApplication({
5154
appType: 'studio',
52-
body: {
53-
appHost,
54-
type: 'studio',
55-
urlType,
56-
},
55+
body: {appHost, type: 'studio', urlType},
5756
projectId,
5857
})
5958
resolve(response)
@@ -73,3 +72,57 @@ export async function createStudioUserApplication(options: CreateStudioUserAppli
7372

7473
return await promise
7574
}
75+
76+
/** Prompts for a title and creates a core application, retrying if the host is taken. */
77+
export async function createUserApplicationForApp(
78+
organizationId?: string,
79+
): Promise<UserApplication> {
80+
if (!organizationId) {
81+
throw new Error(NO_ORGANIZATION_ID)
82+
}
83+
84+
// First get the title from the user
85+
const title = await input({
86+
message: 'Enter a title for your application:',
87+
validate: (input: string) => input.length > 0 || 'Title is required',
88+
})
89+
90+
return tryCreateApp(title, organizationId)
91+
}
92+
93+
// appHosts have some restrictions (no uppercase, must start with a letter)
94+
const generateId = () => {
95+
const letters = 'abcdefghijklmnopqrstuvwxyz'
96+
const firstChar = customAlphabet(letters, 1)()
97+
const rest = customAlphabet('abcdefghijklmnopqrstuvwxyz0123456789', 11)()
98+
return `${firstChar}${rest}`
99+
}
100+
101+
const tryCreateApp = async (title: string, organizationId: string) => {
102+
// we will likely prepend this with an org ID or other parameter in the future
103+
const appHost = generateId()
104+
105+
const spin = spinner('Creating application').start()
106+
107+
try {
108+
const response = await createUserApplication({
109+
appType: 'coreApp',
110+
body: {appHost, title, type: 'coreApp', urlType: 'internal'},
111+
organizationId,
112+
})
113+
114+
spin.succeed()
115+
return response
116+
} catch (e) {
117+
// if the name is taken, generate a new one and try again
118+
if ([402, 409].includes(e?.statusCode)) {
119+
deployDebug('App host taken, retrying with new host')
120+
return tryCreateApp(title, organizationId)
121+
}
122+
123+
spin.fail()
124+
125+
deployDebug('Error creating core application', e)
126+
throw e
127+
}
128+
}

‎packages/@sanity/cli/src/actions/deploy/createUserApplicationForApp.ts‎

Lines changed: 0 additions & 64 deletions
This file was deleted.

0 commit comments

Comments
 (0)