Skip to content

Commit c04911a

Browse files
alfonso-noriegaalfonso-noriega
authored
Merge pull request #7087 from Shopify/03-24-add_install_count_warning_for_safer_deploys
Add install count warning for safer deploys
2 parents abc5566 + c4f4b2b commit c04911a

11 files changed

Lines changed: 321 additions & 2 deletions

File tree

‎packages/app/src/cli/api/graphql/app-management/generated/app-install-count.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/* eslint-disable @typescript-eslint/consistent-type-definitions */
2+
import * as Types from './types.js'
3+
4+
import {TypedDocumentNode as DocumentNode} from '@graphql-typed-document-node/core'
5+
6+
export type AppInstallCountQueryVariables = Types.Exact<{
7+
appId: Types.Scalars['ID']['input']
8+
}>
9+
10+
export type AppInstallCountQuery = {app: {installCount?: number | null}}
11+
12+
export const AppInstallCount = {
13+
kind: 'Document',
14+
definitions: [
15+
{
16+
kind: 'OperationDefinition',
17+
operation: 'query',
18+
name: {kind: 'Name', value: 'AppInstallCount'},
19+
variableDefinitions: [
20+
{
21+
kind: 'VariableDefinition',
22+
variable: {kind: 'Variable', name: {kind: 'Name', value: 'appId'}},
23+
type: {kind: 'NonNullType', type: {kind: 'NamedType', name: {kind: 'Name', value: 'ID'}}},
24+
},
25+
],
26+
selectionSet: {
27+
kind: 'SelectionSet',
28+
selections: [
29+
{
30+
kind: 'Field',
31+
name: {kind: 'Name', value: 'app'},
32+
arguments: [
33+
{
34+
kind: 'Argument',
35+
name: {kind: 'Name', value: 'id'},
36+
value: {kind: 'Variable', name: {kind: 'Name', value: 'appId'}},
37+
},
38+
],
39+
selectionSet: {
40+
kind: 'SelectionSet',
41+
selections: [
42+
{kind: 'Field', name: {kind: 'Name', value: 'installCount'}},
43+
{kind: 'Field', name: {kind: 'Name', value: '__typename'}},
44+
],
45+
},
46+
},
47+
],
48+
},
49+
},
50+
],
51+
} as unknown as DocumentNode<AppInstallCountQuery, AppInstallCountQueryVariables>

‎packages/app/src/cli/api/graphql/app-management/queries/app-install-count.graphql‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
query AppInstallCount($appId: ID!) {
2+
app(id: $appId) {
3+
installCount
4+
}
5+
}

‎packages/app/src/cli/models/app/app.test-data.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1376,6 +1376,7 @@ export function testDeveloperPlatformClient(stubs: Partial<DeveloperPlatformClie
13761376
orgFromId: (_organizationId: string) => Promise.resolve(testOrganization()),
13771377
appsForOrg: (_organizationId: string) => Promise.resolve({apps: [testOrganizationApp()], hasMorePages: false}),
13781378
specifications: (_app: MinimalAppIdentifiers) => Promise.resolve(testRemoteSpecifications),
1379+
appInstallCount: (_app: MinimalAppIdentifiers) => Promise.resolve(0),
13791380
templateSpecifications: (_app: MinimalAppIdentifiers) =>
13801381
Promise.resolve({templates: testRemoteExtensionTemplates, groupOrder: []}),
13811382
orgAndApps: (_orgId: string) =>

‎packages/app/src/cli/prompts/deploy-release.test.ts‎

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,60 @@ describe('deployOrReleaseConfirmationPrompt', () => {
206206
expect(result).toBe(true)
207207
})
208208

209+
test('and no force with deleted extensions and installCount should pass installCount to danger prompt', async () => {
210+
// Given
211+
const breakdownInfo = buildCompleteBreakdownInfo()
212+
const renderDangerousConfirmationPromptSpyOn = vi
213+
.spyOn(ui, 'renderDangerousConfirmationPrompt')
214+
.mockResolvedValue(true)
215+
vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {})
216+
const appTitle = 'app title'
217+
218+
// When
219+
const result = await deployOrReleaseConfirmationPrompt({
220+
...breakdownInfo,
221+
appTitle,
222+
release: true,
223+
force: false,
224+
installCount: 1243,
225+
})
226+
227+
// Then
228+
expect(renderDangerousConfirmationPromptSpyOn).toHaveBeenCalledWith(
229+
expect.objectContaining({
230+
warningItem: expect.arrayContaining([{error: '1243'}]),
231+
}),
232+
)
233+
expect(result).toBe(true)
234+
})
235+
236+
test('and no force with deleted extensions but installCount 0 should not pass warningItem to danger prompt', async () => {
237+
// Given
238+
const breakdownInfo = buildCompleteBreakdownInfo()
239+
const renderDangerousConfirmationPromptSpyOn = vi
240+
.spyOn(ui, 'renderDangerousConfirmationPrompt')
241+
.mockResolvedValue(true)
242+
vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {})
243+
const appTitle = 'app title'
244+
245+
// When
246+
const result = await deployOrReleaseConfirmationPrompt({
247+
...breakdownInfo,
248+
appTitle,
249+
release: true,
250+
force: false,
251+
installCount: 0,
252+
})
253+
254+
// Then
255+
expect(renderDangerousConfirmationPromptSpyOn).toHaveBeenCalledWith(
256+
expect.not.objectContaining({
257+
warningItem: expect.anything(),
258+
}),
259+
)
260+
expect(result).toBe(true)
261+
})
262+
209263
test('and no force with deleted extensions but without app title should display the complete confirmation prompt', async () => {
210264
// Given
211265
const breakdownInfo = buildCompleteBreakdownInfo()

‎packages/app/src/cli/prompts/deploy-release.ts‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ interface DeployOrReleaseConfirmationPromptOptions {
2424
/** If true, allow removing extensions and configuration without user confirmation */
2525
allowDeletes?: boolean
2626
showConfig?: boolean
27+
installCount?: number
2728
}
2829

2930
interface DeployConfirmationPromptOptions {
@@ -36,6 +37,7 @@ interface DeployConfirmationPromptOptions {
3637
configInfoTable: InfoTableSection
3738
}
3839
release: boolean
40+
installCount?: number
3941
}
4042

4143
/**
@@ -97,6 +99,7 @@ export async function deployOrReleaseConfirmationPrompt({
9799
configExtensionIdentifiersBreakdown,
98100
appTitle,
99101
release,
102+
installCount,
100103
}: DeployOrReleaseConfirmationPromptOptions): Promise<boolean> {
101104
await metadata.addPublicMetadata(() => buildConfigurationBreakdownMetadata(configExtensionIdentifiersBreakdown))
102105

@@ -117,6 +120,7 @@ export async function deployOrReleaseConfirmationPrompt({
117120
extensionsContentPrompt,
118121
configContentPrompt,
119122
release,
123+
installCount,
120124
})
121125
}
122126

@@ -125,6 +129,7 @@ async function deployConfirmationPrompt({
125129
extensionsContentPrompt: {extensionsInfoTable, hasDeletedExtensions},
126130
configContentPrompt,
127131
release,
132+
installCount,
128133
}: DeployConfirmationPromptOptions): Promise<boolean> {
129134
const timeBeforeConfirmationMs = new Date().valueOf()
130135
let confirmationResponse = true
@@ -149,11 +154,21 @@ async function deployConfirmationPrompt({
149154
}
150155

151156
const question = `${release ? 'Release' : 'Create'} a new version${appTitle ? ` of ${appTitle}` : ''}?`
157+
const showInstallCountWarning = hasDeletedExtensions && installCount !== undefined && installCount > 0
152158
if (isDangerous) {
153159
confirmationResponse = await renderDangerousConfirmationPrompt({
154160
message: question,
155161
infoTable,
156162
confirmation: appTitle,
163+
...(showInstallCountWarning
164+
? {
165+
warningItem: [
166+
'This release removes extensions and related data from',
167+
{error: installCount.toString()},
168+
'app installations.\nUse caution as this may include production data on live stores.',
169+
],
170+
}
171+
: {}),
157172
})
158173
} else {
159174
confirmationResponse = await renderConfirmationPrompt({

‎packages/app/src/cli/services/context/identifiers.test.ts‎

Lines changed: 153 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,9 @@
1-
import {configExtensionsIdentifiersBreakdown, extensionsIdentifiersDeployBreakdown} from './breakdown-extensions.js'
1+
import {
2+
buildExtensionBreakdownInfo,
3+
configExtensionsIdentifiersBreakdown,
4+
ExtensionIdentifierBreakdownInfo,
5+
extensionsIdentifiersDeployBreakdown,
6+
} from './breakdown-extensions.js'
27
import {ensureDeploymentIdsPresence} from './identifiers.js'
38
import {deployConfirmed} from './identifiers-extensions.js'
49
import {deployOrReleaseConfirmationPrompt} from '../../prompts/deploy-release.js'
@@ -34,6 +39,152 @@ describe('ensureDeploymentIdsPresence', () => {
3439
await expect(ensureDeploymentIdsPresence(params)).rejects.toThrow(AbortSilentError)
3540
})
3641

42+
test('when there are remote-only extensions and not forced, appInstallCount is called with remoteApp.id', async () => {
43+
// Given
44+
const breakdown = buildExtensionsBreakdown()
45+
breakdown.extensionIdentifiersBreakdown.onlyRemote = [buildExtensionBreakdownInfo('removed', 'uuid-1')]
46+
vi.mocked(extensionsIdentifiersDeployBreakdown).mockResolvedValue(breakdown)
47+
vi.mocked(configExtensionsIdentifiersBreakdown).mockResolvedValue(buildConfigBreakdown())
48+
vi.mocked(deployOrReleaseConfirmationPrompt).mockResolvedValue(false)
49+
50+
const remoteApp = testOrganizationApp({id: 'real-app-id', apiKey: 'api-key-different'})
51+
const client = testDeveloperPlatformClient({
52+
appInstallCount: vi.fn().mockResolvedValue(42),
53+
})
54+
55+
const params = {
56+
app: testApp(),
57+
developerPlatformClient: client,
58+
appId: 'api-key-different',
59+
appName: 'appName',
60+
remoteApp,
61+
envIdentifiers: {},
62+
force: false,
63+
release: true,
64+
}
65+
66+
// When
67+
await expect(ensureDeploymentIdsPresence(params)).rejects.toThrow()
68+
69+
// Then
70+
expect(client.appInstallCount).toHaveBeenCalledWith({
71+
id: 'real-app-id',
72+
apiKey: 'api-key-different',
73+
organizationId: remoteApp.organizationId,
74+
})
75+
})
76+
77+
test('when force is true, appInstallCount is not called even with remote-only extensions', async () => {
78+
// Given
79+
const breakdown = buildExtensionsBreakdown()
80+
breakdown.extensionIdentifiersBreakdown.onlyRemote = [buildExtensionBreakdownInfo('removed', 'uuid-1')]
81+
vi.mocked(extensionsIdentifiersDeployBreakdown).mockResolvedValue(breakdown)
82+
vi.mocked(configExtensionsIdentifiersBreakdown).mockResolvedValue(buildConfigBreakdown())
83+
vi.mocked(deployOrReleaseConfirmationPrompt).mockResolvedValue(true)
84+
vi.mocked(deployConfirmed).mockResolvedValue({
85+
extensions: {},
86+
extensionIds: {},
87+
extensionsNonUuidManaged: {},
88+
})
89+
90+
const client = testDeveloperPlatformClient({
91+
appInstallCount: vi.fn().mockResolvedValue(42),
92+
})
93+
94+
const params = {
95+
app: testApp(),
96+
developerPlatformClient: client,
97+
appId: 'appId',
98+
appName: 'appName',
99+
remoteApp: testOrganizationApp(),
100+
envIdentifiers: {},
101+
force: true,
102+
release: true,
103+
}
104+
105+
// When
106+
await ensureDeploymentIdsPresence(params)
107+
108+
// Then
109+
expect(client.appInstallCount).not.toHaveBeenCalled()
110+
})
111+
112+
test('when allowUpdates and allowDeletes are both true, appInstallCount is not called', async () => {
113+
// Given
114+
const breakdown = buildExtensionsBreakdown()
115+
breakdown.extensionIdentifiersBreakdown.onlyRemote = [buildExtensionBreakdownInfo('removed', 'uuid-1')]
116+
vi.mocked(extensionsIdentifiersDeployBreakdown).mockResolvedValue(breakdown)
117+
vi.mocked(configExtensionsIdentifiersBreakdown).mockResolvedValue(buildConfigBreakdown())
118+
vi.mocked(deployOrReleaseConfirmationPrompt).mockResolvedValue(true)
119+
vi.mocked(deployConfirmed).mockResolvedValue({
120+
extensions: {},
121+
extensionIds: {},
122+
extensionsNonUuidManaged: {},
123+
})
124+
125+
const client = testDeveloperPlatformClient({
126+
appInstallCount: vi.fn().mockResolvedValue(42),
127+
})
128+
129+
const params = {
130+
app: testApp(),
131+
developerPlatformClient: client,
132+
appId: 'appId',
133+
appName: 'appName',
134+
remoteApp: testOrganizationApp(),
135+
envIdentifiers: {},
136+
force: false,
137+
allowUpdates: true,
138+
allowDeletes: true,
139+
release: true,
140+
}
141+
142+
// When
143+
await ensureDeploymentIdsPresence(params)
144+
145+
// Then
146+
expect(client.appInstallCount).not.toHaveBeenCalled()
147+
})
148+
149+
test('when appInstallCount throws, installCount is undefined and deploy proceeds', async () => {
150+
// Given
151+
const breakdown = buildExtensionsBreakdown()
152+
breakdown.extensionIdentifiersBreakdown.onlyRemote = [buildExtensionBreakdownInfo('removed', 'uuid-1')]
153+
vi.mocked(extensionsIdentifiersDeployBreakdown).mockResolvedValue(breakdown)
154+
vi.mocked(configExtensionsIdentifiersBreakdown).mockResolvedValue(buildConfigBreakdown())
155+
vi.mocked(deployOrReleaseConfirmationPrompt).mockResolvedValue(true)
156+
vi.mocked(deployConfirmed).mockResolvedValue({
157+
extensions: {},
158+
extensionIds: {},
159+
extensionsNonUuidManaged: {},
160+
})
161+
162+
const client = testDeveloperPlatformClient({
163+
appInstallCount: vi.fn().mockRejectedValue(new Error('API error')),
164+
})
165+
166+
const params = {
167+
app: testApp(),
168+
developerPlatformClient: client,
169+
appId: 'appId',
170+
appName: 'appName',
171+
remoteApp: testOrganizationApp(),
172+
envIdentifiers: {},
173+
force: false,
174+
release: true,
175+
}
176+
177+
// When
178+
await ensureDeploymentIdsPresence(params)
179+
180+
// Then - installCount should be undefined in the prompt call
181+
expect(deployOrReleaseConfirmationPrompt).toHaveBeenCalledWith(
182+
expect.objectContaining({
183+
installCount: undefined,
184+
}),
185+
)
186+
})
187+
37188
test('when the prompt is confirmed post-confirmation actions as run and the result is returned', async () => {
38189
// Given
39190
vi.mocked(extensionsIdentifiersDeployBreakdown).mockResolvedValue(buildExtensionsBreakdown())
@@ -75,7 +226,7 @@ describe('ensureDeploymentIdsPresence', () => {
75226
function buildExtensionsBreakdown() {
76227
return {
77228
extensionIdentifiersBreakdown: {
78-
onlyRemote: [],
229+
onlyRemote: [] as ExtensionIdentifierBreakdownInfo[],
79230
toCreate: [],
80231
toUpdate: [],
81232
fromDashboard: [],

‎packages/app/src/cli/services/context/identifiers.ts‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,25 @@ export async function ensureDeploymentIdsPresence(options: EnsureDeploymentIdsPr
5858
activeAppVersion: options.activeAppVersion,
5959
})
6060

61+
const shouldFetchInstallCount =
62+
extensionIdentifiersBreakdown.onlyRemote.length > 0 &&
63+
!options.force &&
64+
!(options.allowUpdates && options.allowDeletes)
65+
66+
let installCount: number | undefined
67+
if (shouldFetchInstallCount) {
68+
try {
69+
installCount = await options.developerPlatformClient.appInstallCount({
70+
id: options.remoteApp.id,
71+
apiKey: options.remoteApp.apiKey,
72+
organizationId: options.remoteApp.organizationId,
73+
})
74+
// eslint-disable-next-line no-catch-all/no-catch-all
75+
} catch (_error) {
76+
installCount = undefined
77+
}
78+
}
79+
6180
const confirmed = await deployOrReleaseConfirmationPrompt({
6281
extensionIdentifiersBreakdown,
6382
configExtensionIdentifiersBreakdown,
@@ -66,6 +85,7 @@ export async function ensureDeploymentIdsPresence(options: EnsureDeploymentIdsPr
6685
force: options.force,
6786
allowUpdates: options.allowUpdates,
6887
allowDeletes: options.allowDeletes,
88+
installCount,
6989
})
7090
if (!confirmed) throw new AbortSilentError()
7191

0 commit comments

Comments
 (0)