Skip to content

Commit cda2a51

Browse files
Merge pull request #6755 from Shopify/jtv/bulk-mutations-require-variables
Update `app bulk execute` to fail if given a mutation document but no variables
2 parents 13d76df + c0d93db commit cda2a51

2 files changed

Lines changed: 27 additions & 4 deletions

File tree

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

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ describe('executeBulkOperation', () => {
134134

135135
test('runs mutation operation when GraphQL document starts with mutation', async () => {
136136
const mutation = 'mutation productUpdate($input: ProductInput!) { productUpdate(input: $input) { product { id } } }'
137+
const variables = ['{"input":{"id":"gid://shopify/Product/123"}}']
137138
const mockResponse: BulkOperationRunMutationMutation['bulkOperationRunMutation'] = {
138139
bulkOperation: createdBulkOperation,
139140
userErrors: [],
@@ -145,12 +146,13 @@ describe('executeBulkOperation', () => {
145146
remoteApp: mockRemoteApp,
146147
store: mockStore,
147148
query: mutation,
149+
variables,
148150
})
149151

150152
expect(runBulkOperationMutation).toHaveBeenCalledWith({
151153
adminSession: mockAdminSession,
152154
query: mutation,
153-
variablesJsonl: undefined,
155+
variablesJsonl: '{"input":{"id":"gid://shopify/Product/123"}}',
154156
version: BULK_OPERATIONS_MIN_API_VERSION,
155157
})
156158
expect(runBulkOperationQuery).not.toHaveBeenCalled()
@@ -327,6 +329,22 @@ describe('executeBulkOperation', () => {
327329
})
328330
})
329331

332+
test('throws error when mutation is provided without variables', async () => {
333+
const mutation = 'mutation productUpdate($input: ProductInput!) { productUpdate(input: $input) { product { id } } }'
334+
335+
await expect(
336+
executeBulkOperation({
337+
organization: mockOrganization,
338+
remoteApp: mockRemoteApp,
339+
store: mockStore,
340+
query: mutation,
341+
}),
342+
).rejects.toThrow('Bulk mutations require variables')
343+
344+
expect(runBulkOperationQuery).not.toHaveBeenCalled()
345+
expect(runBulkOperationMutation).not.toHaveBeenCalled()
346+
})
347+
330348
test('uses watchBulkOperation (not quickWatchBulkOperation) when watch flag is true', async () => {
331349
const query = '{ products { edges { node { id } } } }'
332350
const initialResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = {

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -216,10 +216,15 @@ function resultsContainUserErrors(results: string): boolean {
216216
})
217217
}
218218

219-
/**
220-
* Validates bulk operation-specific constraints for variables.
221-
*/
222219
function validateBulkOperationVariables(graphqlOperation: string, variablesJsonl?: string): void {
220+
if (isMutation(graphqlOperation) && !variablesJsonl) {
221+
throw new AbortError(
222+
outputContent`Bulk mutations require variables. Provide a JSONL file with ${outputToken.yellow(
223+
'--variable-file',
224+
)} or individual JSON objects with ${outputToken.yellow('--variables')}.`,
225+
)
226+
}
227+
223228
if (!isMutation(graphqlOperation) && variablesJsonl) {
224229
throw new AbortError(
225230
outputContent`The ${outputToken.yellow('--variables')} and ${outputToken.yellow(

0 commit comments

Comments
 (0)