Skip to content

Commit aa19356

Browse files
Merge pull request #6697 from Shopify/bulk_operation_error_message
Use minimum API version for bulk operations
2 parents a170b5a + a96993e commit aa19356

File tree

11 files changed

+346
-133
lines changed

11 files changed

+346
-133
lines changed

packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,16 +4,25 @@ import {
44
normalizeBulkOperationId,
55
extractBulkOperationId,
66
} from './bulk-operation-status.js'
7+
import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js'
78
import {GetBulkOperationByIdQuery} from '../../api/graphql/bulk-operations/generated/get-bulk-operation-by-id.js'
89
import {OrganizationApp, Organization, OrganizationSource} from '../../models/organization.js'
910
import {ListBulkOperationsQuery} from '../../api/graphql/bulk-operations/generated/list-bulk-operations.js'
11+
import {resolveApiVersion} from '../graphql/common.js'
1012
import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
1113
import {ensureAuthenticatedAdminAsApp} from '@shopify/cli-kit/node/session'
1214
import {adminRequestDoc} from '@shopify/cli-kit/node/api/admin'
1315
import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
1416

1517
vi.mock('@shopify/cli-kit/node/session')
1618
vi.mock('@shopify/cli-kit/node/api/admin')
19+
vi.mock('../graphql/common.js', async () => {
20+
const actual = await vi.importActual('../graphql/common.js')
21+
return {
22+
...actual,
23+
resolveApiVersion: vi.fn(),
24+
}
25+
})
1726

1827
const storeFqdn = 'test-store.myshopify.com'
1928
const operationId = 'gid://shopify/BulkOperation/123'
@@ -35,6 +44,7 @@ const remoteApp = {
3544

3645
beforeEach(() => {
3746
vi.mocked(ensureAuthenticatedAdminAsApp).mockResolvedValue({token: 'test-token', storeFqdn})
47+
vi.mocked(resolveApiVersion).mockResolvedValue(BULK_OPERATIONS_MIN_API_VERSION)
3848
})
3949

4050
afterEach(() => {
@@ -169,6 +179,30 @@ describe('getBulkOperationStatus', () => {
169179
expect(output.info()).toContain('Bulk operation canceled.')
170180
})
171181

182+
test('calls resolveApiVersion with minimum API version constant', async () => {
183+
vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'}))
184+
185+
await getBulkOperationStatus({organization: mockOrganization, storeFqdn, operationId, remoteApp})
186+
187+
expect(resolveApiVersion).toHaveBeenCalledWith({
188+
adminSession: {token: 'test-token', storeFqdn},
189+
minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION,
190+
})
191+
})
192+
193+
test('uses resolved API version in admin request', async () => {
194+
vi.mocked(resolveApiVersion).mockResolvedValue('test-api-version')
195+
vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'}))
196+
197+
await getBulkOperationStatus({organization: mockOrganization, storeFqdn, operationId, remoteApp})
198+
199+
expect(adminRequestDoc).toHaveBeenCalledWith(
200+
expect.objectContaining({
201+
version: 'test-api-version',
202+
}),
203+
)
204+
})
205+
172206
describe('time formatting', () => {
173207
test('uses "Started" for running operations', async () => {
174208
vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'}))
@@ -328,4 +362,28 @@ describe('listBulkOperations', () => {
328362
expect(output.info()).toContain('Listing bulk operations.')
329363
expect(output.info()).toContain('No bulk operations found in the last 7 days.')
330364
})
365+
366+
test('calls resolveApiVersion with minimum API version constant', async () => {
367+
vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperationsList([]))
368+
369+
await listBulkOperations({organization: mockOrganization, storeFqdn, remoteApp})
370+
371+
expect(resolveApiVersion).toHaveBeenCalledWith({
372+
adminSession: {token: 'test-token', storeFqdn},
373+
minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION,
374+
})
375+
})
376+
377+
test('uses resolved API version in admin request', async () => {
378+
vi.mocked(resolveApiVersion).mockResolvedValue('test-api-version')
379+
vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperationsList([]))
380+
381+
await listBulkOperations({organization: mockOrganization, storeFqdn, remoteApp})
382+
383+
expect(adminRequestDoc).toHaveBeenCalledWith(
384+
expect.objectContaining({
385+
version: 'test-api-version',
386+
}),
387+
)
388+
})
331389
})

packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import {BulkOperation} from './watch-bulk-operation.js'
22
import {formatBulkOperationStatus} from './format-bulk-operation-status.js'
3+
import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js'
34
import {
45
GetBulkOperationById,
56
GetBulkOperationByIdQuery,
67
} from '../../api/graphql/bulk-operations/generated/get-bulk-operation-by-id.js'
7-
import {formatOperationInfo} from '../graphql/common.js'
8+
import {formatOperationInfo, resolveApiVersion} from '../graphql/common.js'
89
import {OrganizationApp, Organization} from '../../models/organization.js'
910
import {
1011
ListBulkOperations,
@@ -19,8 +20,6 @@ import {timeAgo, formatDate} from '@shopify/cli-kit/common/string'
1920
import {BugError} from '@shopify/cli-kit/node/error'
2021
import colors from '@shopify/cli-kit/node/colors'
2122

22-
const API_VERSION = '2026-01'
23-
2423
export function normalizeBulkOperationId(id: string): string {
2524
// If already a GID, return as-is
2625
if (id.startsWith('gid://')) {
@@ -63,7 +62,7 @@ export async function getBulkOperationStatus(options: GetBulkOperationStatusOpti
6362
body: [
6463
{
6564
list: {
66-
items: formatOperationInfo({organization, remoteApp, storeFqdn, showVersion: false}),
65+
items: formatOperationInfo({organization, remoteApp, storeFqdn}),
6766
},
6867
},
6968
],
@@ -78,7 +77,10 @@ export async function getBulkOperationStatus(options: GetBulkOperationStatusOpti
7877
query: GetBulkOperationById,
7978
session: adminSession,
8079
variables: {id: operationId},
81-
version: API_VERSION,
80+
version: await resolveApiVersion({
81+
adminSession,
82+
minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION,
83+
}),
8284
})
8385

8486
if (response.bulkOperation) {
@@ -99,7 +101,7 @@ export async function listBulkOperations(options: ListBulkOperationsOptions): Pr
99101
body: [
100102
{
101103
list: {
102-
items: formatOperationInfo({organization, remoteApp, storeFqdn, showVersion: false}),
104+
items: formatOperationInfo({organization, remoteApp, storeFqdn}),
103105
},
104106
},
105107
],
@@ -121,7 +123,10 @@ export async function listBulkOperations(options: ListBulkOperationsOptions): Pr
121123
sortKey: 'CREATED_AT',
122124
reverse: true,
123125
},
124-
version: API_VERSION,
126+
version: await resolveApiVersion({
127+
adminSession,
128+
minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION,
129+
}),
125130
})
126131

127132
const operations = response.bulkOperations.nodes.map((operation) => ({
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
/**
2+
* Minimum API version for bulk operations.
3+
* This ensures bulk operation features work correctly across all operations.
4+
*/
5+
export const BULK_OPERATIONS_MIN_API_VERSION = '2026-01'

packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts

Lines changed: 60 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import {runBulkOperationQuery} from './run-query.js'
33
import {runBulkOperationMutation} from './run-mutation.js'
44
import {watchBulkOperation, shortBulkOperationPoll} from './watch-bulk-operation.js'
55
import {downloadBulkOperationResults} from './download-bulk-operation-results.js'
6-
import {validateApiVersion} from '../graphql/common.js'
6+
import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js'
7+
import {resolveApiVersion} from '../graphql/common.js'
78
import {BulkOperationRunQueryMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-query.js'
89
import {BulkOperationRunMutationMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-mutation.js'
910
import {OrganizationApp, OrganizationSource} from '../../models/organization.js'
@@ -22,7 +23,7 @@ vi.mock('../graphql/common.js', async () => {
2223
const actual = await vi.importActual('../graphql/common.js')
2324
return {
2425
...actual,
25-
validateApiVersion: vi.fn(),
26+
resolveApiVersion: vi.fn(),
2627
}
2728
})
2829
vi.mock('@shopify/cli-kit/node/ui')
@@ -68,6 +69,7 @@ describe('executeBulkOperation', () => {
6869
beforeEach(() => {
6970
vi.mocked(ensureAuthenticatedAdminAsApp).mockResolvedValue(mockAdminSession)
7071
vi.mocked(shortBulkOperationPoll).mockResolvedValue(createdBulkOperation)
72+
vi.mocked(resolveApiVersion).mockResolvedValue(BULK_OPERATIONS_MIN_API_VERSION)
7173
})
7274

7375
afterEach(() => {
@@ -92,6 +94,7 @@ describe('executeBulkOperation', () => {
9294
expect(runBulkOperationQuery).toHaveBeenCalledWith({
9395
adminSession: mockAdminSession,
9496
query,
97+
version: BULK_OPERATIONS_MIN_API_VERSION,
9598
})
9699
expect(runBulkOperationMutation).not.toHaveBeenCalled()
97100
})
@@ -114,6 +117,7 @@ describe('executeBulkOperation', () => {
114117
expect(runBulkOperationQuery).toHaveBeenCalledWith({
115118
adminSession: mockAdminSession,
116119
query,
120+
version: BULK_OPERATIONS_MIN_API_VERSION,
117121
})
118122
expect(runBulkOperationMutation).not.toHaveBeenCalled()
119123
})
@@ -137,6 +141,7 @@ describe('executeBulkOperation', () => {
137141
adminSession: mockAdminSession,
138142
query: mutation,
139143
variablesJsonl: undefined,
144+
version: BULK_OPERATIONS_MIN_API_VERSION,
140145
})
141146
expect(runBulkOperationQuery).not.toHaveBeenCalled()
142147
})
@@ -162,6 +167,7 @@ describe('executeBulkOperation', () => {
162167
adminSession: mockAdminSession,
163168
query: mutation,
164169
variablesJsonl: '{"input":{"id":"gid://shopify/Product/123","tags":["test"]}}',
170+
version: BULK_OPERATIONS_MIN_API_VERSION,
165171
})
166172
})
167173

@@ -204,9 +210,13 @@ describe('executeBulkOperation', () => {
204210
query,
205211
})
206212

207-
expect(renderWarning).toHaveBeenCalledWith({
208-
headline: 'Bulk operation errors.',
209-
body: 'query: Invalid query syntax\nunknown: Another error',
213+
expect(renderError).toHaveBeenCalledWith({
214+
headline: 'Error creating bulk operation.',
215+
body: {
216+
list: {
217+
items: ['query: Invalid query syntax', 'Another error'],
218+
},
219+
},
210220
})
211221

212222
expect(renderSuccess).not.toHaveBeenCalled()
@@ -558,51 +568,6 @@ describe('executeBulkOperation', () => {
558568
expect(renderSuccess).not.toHaveBeenCalled()
559569
})
560570

561-
test('validates API version when provided', async () => {
562-
const query = '{ products { edges { node { id } } } }'
563-
const version = '2025-01'
564-
const mockResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = {
565-
bulkOperation: createdBulkOperation,
566-
userErrors: [],
567-
}
568-
vi.mocked(runBulkOperationQuery).mockResolvedValue(mockResponse)
569-
vi.mocked(validateApiVersion).mockResolvedValue()
570-
571-
await executeBulkOperation({
572-
organization: mockOrganization,
573-
remoteApp: mockRemoteApp,
574-
storeFqdn,
575-
query,
576-
version,
577-
})
578-
579-
expect(validateApiVersion).toHaveBeenCalledWith(mockAdminSession, version)
580-
expect(runBulkOperationQuery).toHaveBeenCalledWith({
581-
adminSession: mockAdminSession,
582-
query,
583-
version,
584-
})
585-
})
586-
587-
test('does not validate version when not provided', async () => {
588-
const query = '{ products { edges { node { id } } } }'
589-
const mockResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = {
590-
bulkOperation: createdBulkOperation,
591-
userErrors: [],
592-
}
593-
vi.mocked(runBulkOperationQuery).mockResolvedValue(mockResponse)
594-
vi.mocked(validateApiVersion).mockClear()
595-
596-
await executeBulkOperation({
597-
organization: mockOrganization,
598-
remoteApp: mockRemoteApp,
599-
storeFqdn,
600-
query,
601-
})
602-
603-
expect(validateApiVersion).not.toHaveBeenCalled()
604-
})
605-
606571
test('renders warning when completed operation results contain userErrors', async () => {
607572
const query = '{ products { edges { node { id } } } }'
608573
const resultsWithErrors = '{"data":{"productUpdate":{"userErrors":[{"message":"invalid input"}]}},"__lineNumber":0}'
@@ -711,4 +676,49 @@ describe('executeBulkOperation', () => {
711676
}),
712677
)
713678
})
679+
680+
test('calls resolveApiVersion with minimum API version constant', async () => {
681+
const query = '{ products { edges { node { id } } } }'
682+
const mockResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = {
683+
bulkOperation: createdBulkOperation,
684+
userErrors: [],
685+
}
686+
vi.mocked(runBulkOperationQuery).mockResolvedValue(mockResponse)
687+
688+
await executeBulkOperation({
689+
organization: mockOrganization,
690+
remoteApp: mockRemoteApp,
691+
storeFqdn,
692+
query,
693+
})
694+
695+
expect(resolveApiVersion).toHaveBeenCalledWith({
696+
adminSession: mockAdminSession,
697+
userSpecifiedVersion: undefined,
698+
minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION,
699+
})
700+
})
701+
702+
test('uses resolved API version when running bulk operation', async () => {
703+
vi.mocked(resolveApiVersion).mockResolvedValue('test-api-version')
704+
const query = '{ products { edges { node { id } } } }'
705+
const mockResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = {
706+
bulkOperation: createdBulkOperation,
707+
userErrors: [],
708+
}
709+
vi.mocked(runBulkOperationQuery).mockResolvedValue(mockResponse)
710+
711+
await executeBulkOperation({
712+
organization: mockOrganization,
713+
remoteApp: mockRemoteApp,
714+
storeFqdn,
715+
query,
716+
})
717+
718+
expect(runBulkOperationQuery).toHaveBeenCalledWith({
719+
adminSession: mockAdminSession,
720+
query,
721+
version: 'test-api-version',
722+
})
723+
})
714724
})

0 commit comments

Comments
 (0)