add graphql operation validation

Author: ericlee878

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

@@ -33,7 +33,7 @@ describe('executeBulkOperation', () => {
     vi.mocked(ensureAuthenticatedAdmin).mockResolvedValue(mockAdminSession)
   })
 
-  test('runs query operation when GraphQL starts with query or curly brace', async () => {
+  test('calls runBulkOperationQuery when GraphQL operation starts with query keyword or curly brace', async () => {
     const query = '{ products { edges { node { id } } } }'
     const mockResponse = {
       bulkOperation: successfulBulkOperation,
@@ -55,7 +55,7 @@ describe('executeBulkOperation', () => {
     expect(runBulkOperationMutation).not.toHaveBeenCalled()
   })
 
-  test('runs mutation operation when GraphQL starts with mutation', async () => {
+  test('calls runBulkOperationMutation when GraphQL operation starts with mutation keyword', async () => {
     const mutation = 'mutation productUpdate($input: ProductInput!) { productUpdate(input: $input) { product { id } } }'
     const mockResponse = {
       bulkOperation: successfulBulkOperation,
@@ -77,7 +77,7 @@ describe('executeBulkOperation', () => {
     expect(runBulkOperationQuery).not.toHaveBeenCalled()
   })
 
-  test('passes variables to mutation when provided with `--variables` flag', async () => {
+  test('passes variables parameter to runBulkOperationMutation when variables are provided', async () => {
     const mutation = 'mutation productUpdate($input: ProductInput!) { productUpdate(input: $input) { product { id } } }'
     const variables = '[{"input":{"id":"gid://shopify/Product/123","tags":["test"]}}]'
     const mockResponse = {
@@ -100,7 +100,7 @@ describe('executeBulkOperation', () => {
     })
   })
 
-  test('renders success message when bulk operation is created', async () => {
+  test('renders success message when bulk operation returns without user errors', async () => {
     const query = '{ products { edges { node { id } } } }'
     const mockResponse = {
       bulkOperation: successfulBulkOperation,
@@ -119,7 +119,7 @@ describe('executeBulkOperation', () => {
     })
   })
 
-  test('renders warning when user errors are present', async () => {
+  test('renders warning with formatted field errors when bulk operation returns user errors', async () => {
     const query = '{ products { edges { node { id } } } }'
     const mockResponse = {
       bulkOperation: null,
@@ -143,4 +143,55 @@ describe('executeBulkOperation', () => {
 
     expect(renderSuccess).not.toHaveBeenCalled()
   })
+
+  test('throws GraphQL syntax error when given malformed GraphQL document', async () => {
+    const malformedQuery = '{ products { edges { node { id } }'
+
+    await expect(
+      executeBulkOperation({
+        app: mockApp,
+        storeFqdn,
+        query: malformedQuery,
+      }),
+    ).rejects.toThrow('Syntax Error')
+
+    expect(runBulkOperationQuery).not.toHaveBeenCalled()
+    expect(runBulkOperationMutation).not.toHaveBeenCalled()
+  })
+
+  test('throws error when GraphQL document contains multiple operation definitions', async () => {
+    const multipleOperations =
+      'mutation { productUpdate(input: {}) { product { id } } } mutation { productDelete(input: {}) { deletedProductId } }'
+
+    await expect(
+      executeBulkOperation({
+        app: mockApp,
+        storeFqdn,
+        query: multipleOperations,
+      }),
+    ).rejects.toThrow('Multiple operations are not supported')
+
+    expect(runBulkOperationQuery).not.toHaveBeenCalled()
+    expect(runBulkOperationMutation).not.toHaveBeenCalled()
+  })
+
+  test('throws error when GraphQL document contains no operation definitions', async () => {
+    const noOperations = `
+      fragment ProductFields on Product {
+        id
+        title
+      }
+    `
+
+    await expect(
+      executeBulkOperation({
+        app: mockApp,
+        storeFqdn,
+        query: noOperations,
+      }),
+    ).rejects.toThrow('must contain exactly one operation definition')
+
+    expect(runBulkOperationQuery).not.toHaveBeenCalled()
+    expect(runBulkOperationMutation).not.toHaveBeenCalled()
+  })
 })

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

@@ -24,12 +24,7 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr
 
   const adminSession = await ensureAuthenticatedAdmin(storeFqdn)
 
-  const operationIsMutation = isMutation(query)
-  if (!operationIsMutation && variables) {
-    throw new AbortError(
-      outputContent`The ${outputToken.yellow('--variables')} flag can only be used with mutations, not queries.`,
-    )
-  }
+  const operationIsMutation = validateGraphQLDocument(query, variables)
 
   const bulkOperationResponse = operationIsMutation
     ? await runBulkOperationMutation({adminSession, query, variables})
@@ -77,9 +72,24 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr
   }
 }
 
-function isMutation(graphqlOperation: string): boolean {
+function validateGraphQLDocument(graphqlOperation: string, variables?: string): boolean {
   const document = parse(graphqlOperation)
-  const firstOperation = document.definitions.find((def) => def.kind === 'OperationDefinition')
+  const operationDefinitions = document.definitions.filter((def) => def.kind === 'OperationDefinition')
+
+  if (operationDefinitions.length !== 1) {
+    throw new AbortError(
+      'GraphQL document must contain exactly one operation definition. Multiple operations are not supported.',
+    )
+  }
+
+  const operation = operationDefinitions[0]
+  const operationIsMutation = operation?.kind === 'OperationDefinition' && operation.operation === 'mutation'
+
+  if (!operationIsMutation && variables) {
+    throw new AbortError(
+      outputContent`The ${outputToken.yellow('--variables')} flag can only be used with mutations, not queries.`,
+    )
+  }
 
-  return firstOperation?.kind === 'OperationDefinition' && firstOperation.operation === 'mutation'
+  return operationIsMutation
 }