Skip to content

Commit 71ee5db

Browse files
committed
add graphql operation validation
1 parent 1e7de33 commit 71ee5db

File tree

2 files changed

+73
-12
lines changed

2 files changed

+73
-12
lines changed

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

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ describe('executeBulkOperation', () => {
9999
expect(runBulkOperationQuery).not.toHaveBeenCalled()
100100
})
101101

102-
test('passes variables to mutation when provided with `--variables` flag', async () => {
102+
test('passes variables parameter to runBulkOperationMutation when variables are provided', async () => {
103103
const mutation = 'mutation productUpdate($input: ProductInput!) { productUpdate(input: $input) { product { id } } }'
104104
const variables = '[{"input":{"id":"gid://shopify/Product/123","tags":["test"]}}]'
105105
const mockResponse = {
@@ -122,7 +122,7 @@ describe('executeBulkOperation', () => {
122122
})
123123
})
124124

125-
test('renders success message when bulk operation is created', async () => {
125+
test('renders success message when bulk operation returns without user errors', async () => {
126126
const query = '{ products { edges { node { id } } } }'
127127
const mockResponse = {
128128
bulkOperation: successfulBulkOperation,
@@ -141,7 +141,7 @@ describe('executeBulkOperation', () => {
141141
})
142142
})
143143

144-
test('renders warning when user errors are present', async () => {
144+
test('renders warning with formatted field errors when bulk operation returns user errors', async () => {
145145
const query = '{ products { edges { node { id } } } }'
146146
const mockResponse = {
147147
bulkOperation: null,
@@ -165,4 +165,55 @@ describe('executeBulkOperation', () => {
165165

166166
expect(renderSuccess).not.toHaveBeenCalled()
167167
})
168+
169+
test('throws GraphQL syntax error when given malformed GraphQL document', async () => {
170+
const malformedQuery = '{ products { edges { node { id } }'
171+
172+
await expect(
173+
executeBulkOperation({
174+
app: mockApp,
175+
storeFqdn,
176+
query: malformedQuery,
177+
}),
178+
).rejects.toThrow('Syntax Error')
179+
180+
expect(runBulkOperationQuery).not.toHaveBeenCalled()
181+
expect(runBulkOperationMutation).not.toHaveBeenCalled()
182+
})
183+
184+
test('throws error when GraphQL document contains multiple operation definitions', async () => {
185+
const multipleOperations =
186+
'mutation { productUpdate(input: {}) { product { id } } } mutation { productDelete(input: {}) { deletedProductId } }'
187+
188+
await expect(
189+
executeBulkOperation({
190+
app: mockApp,
191+
storeFqdn,
192+
query: multipleOperations,
193+
}),
194+
).rejects.toThrow('Multiple operations are not supported')
195+
196+
expect(runBulkOperationQuery).not.toHaveBeenCalled()
197+
expect(runBulkOperationMutation).not.toHaveBeenCalled()
198+
})
199+
200+
test('throws error when GraphQL document contains no operation definitions', async () => {
201+
const noOperations = `
202+
fragment ProductFields on Product {
203+
id
204+
title
205+
}
206+
`
207+
208+
await expect(
209+
executeBulkOperation({
210+
app: mockApp,
211+
storeFqdn,
212+
query: noOperations,
213+
}),
214+
).rejects.toThrow('must contain exactly one operation definition')
215+
216+
expect(runBulkOperationQuery).not.toHaveBeenCalled()
217+
expect(runBulkOperationMutation).not.toHaveBeenCalled()
218+
})
168219
})

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

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -24,12 +24,7 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr
2424

2525
const adminSession = await ensureAuthenticatedAdmin(storeFqdn)
2626

27-
const operationIsMutation = isMutation(query)
28-
if (!operationIsMutation && variables) {
29-
throw new AbortError(
30-
outputContent`The ${outputToken.yellow('--variables')} flag can only be used with mutations, not queries.`,
31-
)
32-
}
27+
const operationIsMutation = validateGraphQLDocument(query, variables)
3328

3429
const bulkOperationResponse = operationIsMutation
3530
? await runBulkOperationMutation({adminSession, query, variables})
@@ -77,9 +72,24 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr
7772
}
7873
}
7974

80-
function isMutation(graphqlOperation: string): boolean {
75+
function validateGraphQLDocument(graphqlOperation: string, variables?: string): boolean {
8176
const document = parse(graphqlOperation)
82-
const firstOperation = document.definitions.find((def) => def.kind === 'OperationDefinition')
77+
const operationDefinitions = document.definitions.filter((def) => def.kind === 'OperationDefinition')
78+
79+
if (operationDefinitions.length !== 1) {
80+
throw new AbortError(
81+
'GraphQL document must contain exactly one operation definition. Multiple operations are not supported.',
82+
)
83+
}
84+
85+
const operation = operationDefinitions[0]
86+
const operationIsMutation = operation?.kind === 'OperationDefinition' && operation.operation === 'mutation'
87+
88+
if (!operationIsMutation && variables) {
89+
throw new AbortError(
90+
outputContent`The ${outputToken.yellow('--variables')} flag can only be used with mutations, not queries.`,
91+
)
92+
}
8393

84-
return firstOperation?.kind === 'OperationDefinition' && firstOperation.operation === 'mutation'
94+
return operationIsMutation
8595
}

0 commit comments

Comments
 (0)