Skip to content

Commit 9260e52

Browse files
committed
refactor(deploy): restructure core app deploy and extract shared deploy checks
The app deploy command had grown into one long interleaved function. Restructure it into a single createAppDeployment that validates, prepares and ships, reporting through a shared checks seam, so the command reads as one clear call. This also lands the shared scaffolding the studio deploy will reuse: the checks seam and step helpers (deployChecks), read-only target resolution (resolveDeployTarget), and one home for the auto-update rules (resolveAutoUpdates/getAutoUpdateIssueMessage). Workbench-specific validation moves out of the CLI: viewDeployment goes into @sanity/workbench-cli. Tighten the deploy seams while here: verifyOutputDir takes an isWorkbenchApp boolean instead of a workbench object, checkBuiltOutput becomes a standalone @sanity/workbench-cli/deploy export rather than a method on getWorkbench, and DeployTarget models isExternal as its own field instead of folding it into the type. Adds unit coverage for resolveDeployTarget and resolveTitleUpdate. No behavior change — the integration deploy tests pass unchanged.
1 parent 596baa7 commit 9260e52

21 files changed

Lines changed: 1045 additions & 441 deletions

packages/@sanity/cli/src/actions/build/__tests__/shouldAutoUpdate.test.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import {type Output} from '@sanity/cli-core'
22
import {beforeEach, describe, expect, it, vi} from 'vitest'
33

4-
import {shouldAutoUpdate} from '../shouldAutoUpdate'
4+
import {resolveAutoUpdates, shouldAutoUpdate} from '../shouldAutoUpdate'
55
import {type BuildFlags} from '../types'
66

77
describe('shouldAutoUpdate', () => {
@@ -236,3 +236,55 @@ describe('shouldAutoUpdate', () => {
236236
})
237237
})
238238
})
239+
240+
describe('resolveAutoUpdates', () => {
241+
it('returns disabled with no issue when nothing is configured', () => {
242+
expect(resolveAutoUpdates({cliConfig: {}, flags: {} as BuildFlags})).toEqual({
243+
enabled: false,
244+
issue: null,
245+
})
246+
})
247+
248+
it('reads deployment.autoUpdates without an issue', () => {
249+
expect(
250+
resolveAutoUpdates({cliConfig: {deployment: {autoUpdates: true}}, flags: {} as BuildFlags}),
251+
).toEqual({enabled: true, issue: null})
252+
expect(
253+
resolveAutoUpdates({cliConfig: {deployment: {autoUpdates: false}}, flags: {} as BuildFlags}),
254+
).toEqual({enabled: false, issue: null})
255+
})
256+
257+
it('reports the deprecated top-level config', () => {
258+
expect(resolveAutoUpdates({cliConfig: {autoUpdates: true}, flags: {} as BuildFlags})).toEqual({
259+
enabled: true,
260+
issue: {type: 'deprecated-config'},
261+
})
262+
})
263+
264+
it('reports a conflict when both configs are present, preferring the new value', () => {
265+
expect(
266+
resolveAutoUpdates({
267+
cliConfig: {autoUpdates: true, deployment: {autoUpdates: false}},
268+
flags: {} as BuildFlags,
269+
}),
270+
).toEqual({enabled: false, issue: {type: 'conflicting-config'}})
271+
})
272+
273+
it('reports the deprecated flag and takes its value', () => {
274+
expect(
275+
resolveAutoUpdates({cliConfig: {}, flags: {'auto-updates': true} as BuildFlags}),
276+
).toEqual({enabled: true, issue: {flag: '--auto-updates', type: 'deprecated-flag'}})
277+
expect(
278+
resolveAutoUpdates({cliConfig: {}, flags: {'auto-updates': false} as BuildFlags}),
279+
).toEqual({enabled: false, issue: {flag: '--no-auto-updates', type: 'deprecated-flag'}})
280+
})
281+
282+
it('lets the flag take precedence over config, masking config issues', () => {
283+
expect(
284+
resolveAutoUpdates({
285+
cliConfig: {autoUpdates: true, deployment: {autoUpdates: true}},
286+
flags: {'auto-updates': false} as BuildFlags,
287+
}),
288+
).toEqual({enabled: false, issue: {flag: '--no-auto-updates', type: 'deprecated-flag'}})
289+
})
290+
})

packages/@sanity/cli/src/actions/build/shouldAutoUpdate.ts

Lines changed: 88 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,39 @@ import {type BuildFlags} from './types.js'
99
interface AutoUpdateSources {
1010
cliConfig: CliConfig
1111
flags: BuildFlags | DeployFlags | DevFlags
12-
output: Output
1312
}
1413

1514
/**
16-
* Compares parameters from various sources to determine whether or not to auto-update.
17-
* @remarks Throws an error if both the old and new auto update config are used; throws a warning if the old config is used, or if the auto updates flags are used.
15+
* The single auto-update configuration problem to surface, if any.
16+
* Flag usage takes precedence over config problems: when the deprecated flag
17+
* is passed, config is not consulted, so config issues are not reported.
18+
*/
19+
type AutoUpdateIssue =
20+
| {flag: '--auto-updates' | '--no-auto-updates'; type: 'deprecated-flag'}
21+
| {type: 'conflicting-config'}
22+
| {type: 'deprecated-config'}
23+
24+
export interface AutoUpdateSettings {
25+
enabled: boolean
26+
issue: AutoUpdateIssue | null
27+
}
28+
29+
/**
30+
* Owns the auto-update rules: flag-over-config precedence, the deprecated
31+
* top-level `autoUpdates` config, and the old/new config conflict.
32+
* Returns plain facts so each surface (deploy warnings, dry-run checks)
33+
* decides its own presentation.
34+
*
1835
* @internal
1936
*/
20-
export function shouldAutoUpdate({cliConfig, flags, output}: AutoUpdateSources): boolean {
21-
// Auto updates in flags is deprecated; throw a warning if used
37+
export function resolveAutoUpdates({cliConfig, flags}: AutoUpdateSources): AutoUpdateSettings {
38+
// Flags always take precedence over config
2239
if ('auto-updates' in flags) {
23-
const flagUsed = flags['auto-updates'] ? '--auto-updates' : '--no-auto-updates'
24-
output.warn(
25-
`The ${flagUsed} flag is deprecated for deploy and build commands. Set the autoUpdates option in the deployment section of sanity.cli.ts or sanity.cli.js instead.`,
26-
)
27-
// Flags always take precedence over config
28-
return Boolean(flags['auto-updates'])
40+
const enabled = Boolean(flags['auto-updates'])
41+
return {
42+
enabled,
43+
issue: {flag: enabled ? '--auto-updates' : '--no-auto-updates', type: 'deprecated-flag'},
44+
}
2945
}
3046

3147
const hasOldConfig = cliConfig && 'autoUpdates' in cliConfig
@@ -36,30 +52,74 @@ export function shouldAutoUpdate({cliConfig, flags, output}: AutoUpdateSources):
3652
cliConfig.deployment &&
3753
'autoUpdates' in cliConfig.deployment
3854

39-
if (hasOldConfig && hasNewConfig) {
40-
output.error(
41-
'Found both `autoUpdates` (deprecated) and `deployment.autoUpdates` in sanity.cli.js/.ts. Please remove the deprecated top level `autoUpdates` config.',
42-
{
43-
exit: 1,
44-
},
45-
)
55+
if (hasNewConfig) {
56+
return {
57+
enabled: Boolean(cliConfig?.deployment?.autoUpdates),
58+
issue: hasOldConfig ? {type: 'conflicting-config'} : null,
59+
}
4660
}
4761

4862
if (hasOldConfig) {
49-
output.warn('The autoUpdates config has moved to deployment.autoUpdates.')
50-
output.warn(`Please update sanity.cli.ts or sanity.cli.js and make the following change:
51-
${styleText('red', `- autoUpdates: ${cliConfig.autoUpdates},`)}
52-
${styleText('green', `+ deployment: {autoUpdates: ${cliConfig.autoUpdates}}`)}
53-
`)
63+
return {enabled: Boolean(cliConfig?.autoUpdates), issue: {type: 'deprecated-config'}}
5464
}
5565

56-
if (hasNewConfig) {
57-
return Boolean(cliConfig?.deployment?.autoUpdates)
66+
return {enabled: false, issue: null}
67+
}
68+
69+
/**
70+
* The user-facing message for an auto-update configuration problem.
71+
* Shared by every surface that reports the issue (deploy warnings, dry-run
72+
* checks) so the wording has one home.
73+
*
74+
* @internal
75+
*/
76+
export function getAutoUpdateIssueMessage(issue: AutoUpdateIssue): string {
77+
switch (issue.type) {
78+
case 'conflicting-config': {
79+
return 'Found both `autoUpdates` (deprecated) and `deployment.autoUpdates` in sanity.cli.js/.ts. Please remove the deprecated top level `autoUpdates` config.'
80+
}
81+
case 'deprecated-config': {
82+
return 'The autoUpdates config has moved to deployment.autoUpdates.'
83+
}
84+
case 'deprecated-flag': {
85+
return `The ${issue.flag} flag is deprecated for deploy and build commands. Set the autoUpdates option in the deployment section of sanity.cli.ts or sanity.cli.js instead.`
86+
}
5887
}
88+
}
5989

60-
if (hasOldConfig) {
61-
return Boolean(cliConfig?.autoUpdates)
90+
/**
91+
* Resolves the auto-update setting and prints any configuration problem.
92+
* @remarks Throws an error if both the old and new auto update config are used; throws a warning if the old config is used, or if the auto updates flags are used.
93+
* @internal
94+
*/
95+
export function shouldAutoUpdate({
96+
cliConfig,
97+
flags,
98+
output,
99+
}: AutoUpdateSources & {output: Output}): boolean {
100+
const {enabled, issue} = resolveAutoUpdates({cliConfig, flags})
101+
102+
switch (issue?.type) {
103+
case 'conflicting-config': {
104+
output.error(getAutoUpdateIssueMessage(issue), {exit: 1})
105+
break
106+
}
107+
case 'deprecated-config': {
108+
output.warn(getAutoUpdateIssueMessage(issue))
109+
output.warn(`Please update sanity.cli.ts or sanity.cli.js and make the following change:
110+
${styleText('red', `- autoUpdates: ${cliConfig.autoUpdates},`)}
111+
${styleText('green', `+ deployment: {autoUpdates: ${cliConfig.autoUpdates}}`)}
112+
`)
113+
break
114+
}
115+
case 'deprecated-flag': {
116+
output.warn(getAutoUpdateIssueMessage(issue))
117+
break
118+
}
119+
default: {
120+
break
121+
}
62122
}
63123

64-
return false
124+
return enabled
65125
}
Lines changed: 134 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,134 @@
1+
import {beforeEach, describe, expect, test, vi} from 'vitest'
2+
3+
import {type UserApplication} from '../../../services/userApplications.js'
4+
import {checkAppTarget, createAggregatingChecks} from '../deployChecks.js'
5+
import {resolveAppDeployTarget} from '../resolveDeployTarget.js'
6+
7+
vi.mock('../resolveDeployTarget.js', () => ({
8+
resolveAppDeployTarget: vi.fn(),
9+
}))
10+
11+
const mockResolveApp = vi.mocked(resolveAppDeployTarget)
12+
13+
function application(overrides: Partial<UserApplication> = {}): UserApplication {
14+
return {
15+
appHost: 'my-studio',
16+
createdAt: '2024-01-01T00:00:00Z',
17+
id: 'app-1',
18+
organizationId: null,
19+
projectId: 'project-1',
20+
title: null,
21+
type: 'studio',
22+
updatedAt: '2024-01-01T00:00:00Z',
23+
urlType: 'internal',
24+
...overrides,
25+
}
26+
}
27+
28+
beforeEach(() => vi.clearAllMocks())
29+
30+
describe('createAggregatingChecks', () => {
31+
test('records checks without exiting', () => {
32+
const checks = createAggregatingChecks()
33+
checks.add({message: 'ok', name: 'a', status: 'pass'})
34+
checks.add({message: 'bad', name: 'b', status: 'fail'})
35+
expect(checks.all()).toHaveLength(2)
36+
})
37+
38+
test('a thrown step becomes a fail check', async () => {
39+
const checks = createAggregatingChecks()
40+
const result = await checks.run('boom', async () => {
41+
throw new Error('nope')
42+
})
43+
expect(result).toBeNull()
44+
expect(checks.all()[0]).toMatchObject({name: 'boom', status: 'fail'})
45+
expect(checks.all()[0]?.message).toContain('nope')
46+
})
47+
48+
test('run returns the value when the step succeeds', async () => {
49+
const checks = createAggregatingChecks()
50+
const result = await checks.run('ok', async () => 42)
51+
expect(result).toBe(42)
52+
expect(checks.all()).toHaveLength(0)
53+
})
54+
})
55+
56+
describe('checkAppTarget', () => {
57+
test('found → pass check, existing app and target', async () => {
58+
const app = application({appHost: 'app-host', id: 'core-1', title: 'My App', type: 'coreApp'})
59+
mockResolveApp.mockResolvedValue({application: app, type: 'found'})
60+
const checks = createAggregatingChecks()
61+
62+
const {existingApp, target} = await checkAppTarget(checks, {
63+
appId: 'core-1',
64+
organizationId: 'org-1',
65+
})
66+
67+
expect(existingApp).toBe(app)
68+
expect(target).toEqual({
69+
appId: 'core-1',
70+
exists: true,
71+
host: 'app-host',
72+
isExternal: false,
73+
type: 'coreApp',
74+
})
75+
expect(checks.all()[0]?.message).toContain('Deploys to existing application "My App"')
76+
})
77+
78+
test('would-create → pass check, no existing app', async () => {
79+
mockResolveApp.mockResolvedValue({type: 'would-create'})
80+
const checks = createAggregatingChecks()
81+
82+
const {existingApp, target} = await checkAppTarget(checks, {
83+
appId: undefined,
84+
organizationId: 'org-1',
85+
})
86+
87+
expect(existingApp).toBeNull()
88+
expect(target).toEqual({
89+
appId: null,
90+
exists: false,
91+
host: null,
92+
isExternal: false,
93+
type: 'coreApp',
94+
})
95+
})
96+
97+
test('needs-input → fail check (would prompt)', async () => {
98+
mockResolveApp.mockResolvedValue({
99+
existing: [application(), application()],
100+
type: 'needs-input',
101+
})
102+
const checks = createAggregatingChecks()
103+
104+
const {target} = await checkAppTarget(checks, {appId: undefined, organizationId: 'org-1'})
105+
106+
expect(target).toBeNull()
107+
expect(checks.all()[0]).toMatchObject({name: 'target', status: 'fail'})
108+
expect(checks.all()[0]?.message).toContain('2 existing applications found')
109+
})
110+
111+
test('invalid → fail check', async () => {
112+
mockResolveApp.mockResolvedValue({
113+
message: 'Cannot find app with app ID nope',
114+
reason: 'app-not-found',
115+
type: 'invalid',
116+
})
117+
const checks = createAggregatingChecks()
118+
119+
const {target} = await checkAppTarget(checks, {appId: 'nope', organizationId: 'org-1'})
120+
121+
expect(target).toBeNull()
122+
expect(checks.all()[0]?.status).toBe('fail')
123+
})
124+
125+
test('a 403 becomes a permissions fail check', async () => {
126+
mockResolveApp.mockRejectedValue(Object.assign(new Error('forbidden'), {statusCode: 403}))
127+
const checks = createAggregatingChecks()
128+
129+
const {target} = await checkAppTarget(checks, {appId: undefined, organizationId: 'org-1'})
130+
131+
expect(target).toBeNull()
132+
expect(checks.all()[0]?.message).toContain('don’t have permission')
133+
})
134+
})
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
import {describe, expect, test} from 'vitest'
2+
3+
import {resolveAppDeployTarget} from '../resolveDeployTarget.js'
4+
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.
8+
9+
describe('resolveAppDeployTarget', () => {
10+
test('no appId and no organizationId → blocked', async () => {
11+
const result = await resolveAppDeployTarget({appId: undefined, organizationId: undefined})
12+
13+
expect(result).toEqual({message: 'app.organizationId is missing', type: 'blocked'})
14+
})
15+
})

0 commit comments

Comments
 (0)