Skip to content

Commit bce09b0

Browse files
committed
add graphql operation validation
1 parent 742d51c commit bce09b0

2 files changed

Lines changed: 85 additions & 14 deletions

File tree

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

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ describe('executeBulkOperation', () => {
3333
vi.mocked(ensureAuthenticatedAdmin).mockResolvedValue(mockAdminSession)
3434
})
3535

36-
test('runs query operation when GraphQL starts with query or curly brace', async () => {
36+
test('calls runBulkOperationQuery when GraphQL operation starts with query keyword or curly brace', async () => {
3737
const query = '{ products { edges { node { id } } } }'
3838
const mockResponse = {
3939
bulkOperation: successfulBulkOperation,
@@ -55,7 +55,7 @@ describe('executeBulkOperation', () => {
5555
expect(runBulkOperationMutation).not.toHaveBeenCalled()
5656
})
5757

58-
test('runs mutation operation when GraphQL starts with mutation', async () => {
58+
test('calls runBulkOperationMutation when GraphQL operation starts with mutation keyword', async () => {
5959
const mutation = 'mutation productUpdate($input: ProductInput!) { productUpdate(input: $input) { product { id } } }'
6060
const mockResponse = {
6161
bulkOperation: successfulBulkOperation,
@@ -77,7 +77,7 @@ describe('executeBulkOperation', () => {
7777
expect(runBulkOperationQuery).not.toHaveBeenCalled()
7878
})
7979

80-
test('passes variables to mutation when provided with `--variables` flag', async () => {
80+
test('passes variables parameter to runBulkOperationMutation when variables are provided', async () => {
8181
const mutation = 'mutation productUpdate($input: ProductInput!) { productUpdate(input: $input) { product { id } } }'
8282
const variables = '[{"input":{"id":"gid://shopify/Product/123","tags":["test"]}}]'
8383
const mockResponse = {
@@ -100,7 +100,7 @@ describe('executeBulkOperation', () => {
100100
})
101101
})
102102

103-
test('renders success message when bulk operation is created', async () => {
103+
test('renders success message when bulk operation returns without user errors', async () => {
104104
const query = '{ products { edges { node { id } } } }'
105105
const mockResponse = {
106106
bulkOperation: successfulBulkOperation,
@@ -119,7 +119,7 @@ describe('executeBulkOperation', () => {
119119
})
120120
})
121121

122-
test('renders warning when user errors are present', async () => {
122+
test('renders warning with formatted field errors when bulk operation returns user errors', async () => {
123123
const query = '{ products { edges { node { id } } } }'
124124
const mockResponse = {
125125
bulkOperation: null,
@@ -143,4 +143,65 @@ describe('executeBulkOperation', () => {
143143

144144
expect(renderSuccess).not.toHaveBeenCalled()
145145
})
146+
147+
test('throws GraphQL syntax error when given malformed GraphQL document', async () => {
148+
const malformedQuery = '{ products { edges { node { id } }'
149+
150+
await expect(
151+
executeBulkOperation({
152+
app: mockApp,
153+
storeFqdn,
154+
query: malformedQuery,
155+
}),
156+
).rejects.toThrow('Syntax Error')
157+
158+
expect(runBulkOperationQuery).not.toHaveBeenCalled()
159+
expect(runBulkOperationMutation).not.toHaveBeenCalled()
160+
})
161+
162+
test('throws error when GraphQL document contains multiple operation definitions', async () => {
163+
const multipleOperations = `
164+
mutation {
165+
productUpdate(input: {}) {
166+
product { id }
167+
}
168+
}
169+
mutation {
170+
productDelete(input: {}) {
171+
deletedProductId
172+
}
173+
}
174+
`
175+
176+
await expect(
177+
executeBulkOperation({
178+
app: mockApp,
179+
storeFqdn,
180+
query: multipleOperations,
181+
}),
182+
).rejects.toThrow('Multiple operations are not supported')
183+
184+
expect(runBulkOperationQuery).not.toHaveBeenCalled()
185+
expect(runBulkOperationMutation).not.toHaveBeenCalled()
186+
})
187+
188+
test('throws error when GraphQL document contains no operation definitions', async () => {
189+
const noOperations = `
190+
fragment ProductFields on Product {
191+
id
192+
title
193+
}
194+
`
195+
196+
await expect(
197+
executeBulkOperation({
198+
app: mockApp,
199+
storeFqdn,
200+
query: noOperations,
201+
}),
202+
).rejects.toThrow('must contain exactly one operation definition')
203+
204+
expect(runBulkOperationQuery).not.toHaveBeenCalled()
205+
expect(runBulkOperationMutation).not.toHaveBeenCalled()
206+
})
146207
})

‎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)