From 655f46b21ff69f2e100f59b061b538ab93ccac14 Mon Sep 17 00:00:00 2001 From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com> Date: Tue, 9 Dec 2025 12:17:09 -0500 Subject: [PATCH 1/8] Improve error messaging for failures on bulk operation creation --- .../execute-bulk-operation.test.ts | 10 +++++++--- .../bulk-operations/execute-bulk-operation.ts | 20 ++++++++++--------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts index f4ac7eb5a6..92b6b451c6 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts @@ -204,9 +204,13 @@ describe('executeBulkOperation', () => { query, }) - expect(renderWarning).toHaveBeenCalledWith({ - headline: 'Bulk operation errors.', - body: 'query: Invalid query syntax\nunknown: Another error', + expect(renderError).toHaveBeenCalledWith({ + headline: 'Error creating bulk operation.', + body: { + list: { + items: ['query: Invalid query syntax', 'Another error'], + }, + }, }) expect(renderSuccess).not.toHaveBeenCalled() diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts index 7c4f3df067..8c61ea29a1 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts @@ -74,15 +74,17 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr : await runBulkOperationQuery({adminSession, query, version}) if (bulkOperationResponse?.userErrors?.length) { - const errorMessages = bulkOperationResponse.userErrors - .map( - (error: {field?: string[] | null; message: string}) => - `${error.field?.join('.') ?? 'unknown'}: ${error.message}`, - ) - .join('\n') - renderWarning({ - headline: 'Bulk operation errors.', - body: errorMessages, + const errorMessages = bulkOperationResponse.userErrors.map( + (error: {field?: string[] | null; message: string}) => + `${error.field ? `${error.field.join('.')}: ` : ''}${error.message}`, + ) + renderError({ + headline: 'Error creating bulk operation.', + body: { + list: { + items: errorMessages, + }, + }, }) return } From 8d02b7a1b10b83971b77a10211fbf7888405c706 Mon Sep 17 00:00:00 2001 From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com> Date: Tue, 9 Dec 2025 14:37:16 -0500 Subject: [PATCH 2/8] refactor version validation to allow RCs and default to 2026-01 for bulk operations --- .../bulk-operation-status.test.ts | 34 +++++ .../bulk-operations/bulk-operation-status.ts | 13 +- .../cli/services/bulk-operations/constants.ts | 5 + .../execute-bulk-operation.test.ts | 54 +------- .../bulk-operations/execute-bulk-operation.ts | 17 ++- .../cli/services/execute-operation.test.ts | 26 +--- .../app/src/cli/services/execute-operation.ts | 12 +- .../src/cli/services/graphql/common.test.ts | 123 +++++++++++++++--- .../app/src/cli/services/graphql/common.ts | 51 +++++--- packages/cli-kit/src/public/node/api/admin.ts | 5 +- 10 files changed, 223 insertions(+), 117 deletions(-) create mode 100644 packages/app/src/cli/services/bulk-operations/constants.ts diff --git a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts index fb5bb6f364..b1e763f647 100644 --- a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts +++ b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts @@ -4,9 +4,11 @@ import { normalizeBulkOperationId, extractBulkOperationId, } from './bulk-operation-status.js' +import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js' import {GetBulkOperationByIdQuery} from '../../api/graphql/bulk-operations/generated/get-bulk-operation-by-id.js' import {OrganizationApp, Organization, OrganizationSource} from '../../models/organization.js' import {ListBulkOperationsQuery} from '../../api/graphql/bulk-operations/generated/list-bulk-operations.js' +import {resolveApiVersion} from '../graphql/common.js' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' import {ensureAuthenticatedAdminAsApp} from '@shopify/cli-kit/node/session' import {adminRequestDoc} from '@shopify/cli-kit/node/api/admin' @@ -14,6 +16,13 @@ import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' vi.mock('@shopify/cli-kit/node/session') vi.mock('@shopify/cli-kit/node/api/admin') +vi.mock('../graphql/common.js', async () => { + const actual = await vi.importActual('../graphql/common.js') + return { + ...actual, + resolveApiVersion: vi.fn(), + } +}) const storeFqdn = 'test-store.myshopify.com' const operationId = 'gid://shopify/BulkOperation/123' @@ -35,6 +44,7 @@ const remoteApp = { beforeEach(() => { vi.mocked(ensureAuthenticatedAdminAsApp).mockResolvedValue({token: 'test-token', storeFqdn}) + vi.mocked(resolveApiVersion).mockResolvedValue('2026-01') }) afterEach(() => { @@ -169,6 +179,18 @@ describe('getBulkOperationStatus', () => { expect(output.info()).toContain('Bulk operation canceled.') }) + test('calls resolveApiVersion with minimum API version', async () => { + vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'})) + + await getBulkOperationStatus({organization: mockOrganization, storeFqdn, operationId, remoteApp}) + + expect(resolveApiVersion).toHaveBeenCalledWith( + {token: 'test-token', storeFqdn}, + undefined, + BULK_OPERATIONS_MIN_API_VERSION, + ) + }) + describe('time formatting', () => { test('uses "Started" for running operations', async () => { vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'})) @@ -328,4 +350,16 @@ describe('listBulkOperations', () => { expect(output.info()).toContain('Listing bulk operations.') expect(output.info()).toContain('No bulk operations found in the last 7 days.') }) + + test('calls resolveApiVersion with minimum API version', async () => { + vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperationsList([])) + + await listBulkOperations({organization: mockOrganization, storeFqdn, remoteApp}) + + expect(resolveApiVersion).toHaveBeenCalledWith( + {token: 'test-token', storeFqdn}, + undefined, + BULK_OPERATIONS_MIN_API_VERSION, + ) + }) }) diff --git a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts index 1dca3cabc1..a1d19b1dbc 100644 --- a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts +++ b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts @@ -1,10 +1,11 @@ import {BulkOperation} from './watch-bulk-operation.js' import {formatBulkOperationStatus} from './format-bulk-operation-status.js' +import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js' import { GetBulkOperationById, GetBulkOperationByIdQuery, } from '../../api/graphql/bulk-operations/generated/get-bulk-operation-by-id.js' -import {formatOperationInfo} from '../graphql/common.js' +import {formatOperationInfo, resolveApiVersion} from '../graphql/common.js' import {OrganizationApp, Organization} from '../../models/organization.js' import { ListBulkOperations, @@ -19,8 +20,6 @@ import {timeAgo, formatDate} from '@shopify/cli-kit/common/string' import {BugError} from '@shopify/cli-kit/node/error' import colors from '@shopify/cli-kit/node/colors' -const API_VERSION = '2026-01' - export function normalizeBulkOperationId(id: string): string { // If already a GID, return as-is if (id.startsWith('gid://')) { @@ -63,7 +62,7 @@ export async function getBulkOperationStatus(options: GetBulkOperationStatusOpti body: [ { list: { - items: formatOperationInfo({organization, remoteApp, storeFqdn, showVersion: false}), + items: formatOperationInfo({organization, remoteApp, storeFqdn}), }, }, ], @@ -78,7 +77,7 @@ export async function getBulkOperationStatus(options: GetBulkOperationStatusOpti query: GetBulkOperationById, session: adminSession, variables: {id: operationId}, - version: API_VERSION, + version: await resolveApiVersion(adminSession, undefined, BULK_OPERATIONS_MIN_API_VERSION), }) if (response.bulkOperation) { @@ -99,7 +98,7 @@ export async function listBulkOperations(options: ListBulkOperationsOptions): Pr body: [ { list: { - items: formatOperationInfo({organization, remoteApp, storeFqdn, showVersion: false}), + items: formatOperationInfo({organization, remoteApp, storeFqdn}), }, }, ], @@ -121,7 +120,7 @@ export async function listBulkOperations(options: ListBulkOperationsOptions): Pr sortKey: 'CREATED_AT', reverse: true, }, - version: API_VERSION, + version: await resolveApiVersion(adminSession, undefined, BULK_OPERATIONS_MIN_API_VERSION), }) const operations = response.bulkOperations.nodes.map((operation) => ({ diff --git a/packages/app/src/cli/services/bulk-operations/constants.ts b/packages/app/src/cli/services/bulk-operations/constants.ts new file mode 100644 index 0000000000..3a1bcc32a7 --- /dev/null +++ b/packages/app/src/cli/services/bulk-operations/constants.ts @@ -0,0 +1,5 @@ +/** + * Minimum API version for bulk operations. + * This ensures bulk operation features work correctly across all operations. + */ +export const BULK_OPERATIONS_MIN_API_VERSION = '2026-01' diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts index 92b6b451c6..8cac189891 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts @@ -3,7 +3,7 @@ import {runBulkOperationQuery} from './run-query.js' import {runBulkOperationMutation} from './run-mutation.js' import {watchBulkOperation, shortBulkOperationPoll} from './watch-bulk-operation.js' import {downloadBulkOperationResults} from './download-bulk-operation-results.js' -import {validateApiVersion} from '../graphql/common.js' +import {resolveApiVersion} from '../graphql/common.js' import {BulkOperationRunQueryMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-query.js' import {BulkOperationRunMutationMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-mutation.js' import {OrganizationApp, OrganizationSource} from '../../models/organization.js' @@ -22,7 +22,7 @@ vi.mock('../graphql/common.js', async () => { const actual = await vi.importActual('../graphql/common.js') return { ...actual, - validateApiVersion: vi.fn(), + resolveApiVersion: vi.fn(), } }) vi.mock('@shopify/cli-kit/node/ui') @@ -68,6 +68,7 @@ describe('executeBulkOperation', () => { beforeEach(() => { vi.mocked(ensureAuthenticatedAdminAsApp).mockResolvedValue(mockAdminSession) vi.mocked(shortBulkOperationPoll).mockResolvedValue(createdBulkOperation) + vi.mocked(resolveApiVersion).mockResolvedValue('2026-01') }) afterEach(() => { @@ -92,6 +93,7 @@ describe('executeBulkOperation', () => { expect(runBulkOperationQuery).toHaveBeenCalledWith({ adminSession: mockAdminSession, query, + version: '2026-01', }) expect(runBulkOperationMutation).not.toHaveBeenCalled() }) @@ -114,6 +116,7 @@ describe('executeBulkOperation', () => { expect(runBulkOperationQuery).toHaveBeenCalledWith({ adminSession: mockAdminSession, query, + version: '2026-01', }) expect(runBulkOperationMutation).not.toHaveBeenCalled() }) @@ -137,6 +140,7 @@ describe('executeBulkOperation', () => { adminSession: mockAdminSession, query: mutation, variablesJsonl: undefined, + version: '2026-01', }) expect(runBulkOperationQuery).not.toHaveBeenCalled() }) @@ -162,6 +166,7 @@ describe('executeBulkOperation', () => { adminSession: mockAdminSession, query: mutation, variablesJsonl: '{"input":{"id":"gid://shopify/Product/123","tags":["test"]}}', + version: '2026-01', }) }) @@ -562,51 +567,6 @@ describe('executeBulkOperation', () => { expect(renderSuccess).not.toHaveBeenCalled() }) - test('validates API version when provided', async () => { - const query = '{ products { edges { node { id } } } }' - const version = '2025-01' - const mockResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = { - bulkOperation: createdBulkOperation, - userErrors: [], - } - vi.mocked(runBulkOperationQuery).mockResolvedValue(mockResponse) - vi.mocked(validateApiVersion).mockResolvedValue() - - await executeBulkOperation({ - organization: mockOrganization, - remoteApp: mockRemoteApp, - storeFqdn, - query, - version, - }) - - expect(validateApiVersion).toHaveBeenCalledWith(mockAdminSession, version) - expect(runBulkOperationQuery).toHaveBeenCalledWith({ - adminSession: mockAdminSession, - query, - version, - }) - }) - - test('does not validate version when not provided', async () => { - const query = '{ products { edges { node { id } } } }' - const mockResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = { - bulkOperation: createdBulkOperation, - userErrors: [], - } - vi.mocked(runBulkOperationQuery).mockResolvedValue(mockResponse) - vi.mocked(validateApiVersion).mockClear() - - await executeBulkOperation({ - organization: mockOrganization, - remoteApp: mockRemoteApp, - storeFqdn, - query, - }) - - expect(validateApiVersion).not.toHaveBeenCalled() - }) - test('renders warning when completed operation results contain userErrors', async () => { const query = '{ products { edges { node { id } } } }' const resultsWithErrors = '{"data":{"productUpdate":{"userErrors":[{"message":"invalid input"}]}},"__lineNumber":0}' diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts index 8c61ea29a1..860f2490b8 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts @@ -4,11 +4,12 @@ import {watchBulkOperation, shortBulkOperationPoll, type BulkOperation} from './ import {formatBulkOperationStatus} from './format-bulk-operation-status.js' import {downloadBulkOperationResults} from './download-bulk-operation-results.js' import {extractBulkOperationId} from './bulk-operation-status.js' +import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js' import { createAdminSessionAsApp, validateSingleOperation, - validateApiVersion, formatOperationInfo, + resolveApiVersion, } from '../graphql/common.js' import {OrganizationApp, Organization} from '../../models/organization.js' import {renderSuccess, renderInfo, renderError, renderWarning, TokenItem} from '@shopify/cli-kit/node/ui' @@ -48,11 +49,21 @@ async function parseVariablesToJsonl(variables?: string[], variableFile?: string } export async function executeBulkOperation(input: ExecuteBulkOperationInput): Promise { - const {organization, remoteApp, storeFqdn, query, variables, variableFile, outputFile, watch = false, version} = input + const { + organization, + remoteApp, + storeFqdn, + query, + variables, + variableFile, + outputFile, + watch = false, + version: versionFlag, + } = input const adminSession = await createAdminSessionAsApp(remoteApp, storeFqdn) - if (version) await validateApiVersion(adminSession, version) + const version = await resolveApiVersion(adminSession, versionFlag, BULK_OPERATIONS_MIN_API_VERSION) const variablesJsonl = await parseVariablesToJsonl(variables, variableFile) diff --git a/packages/app/src/cli/services/execute-operation.test.ts b/packages/app/src/cli/services/execute-operation.test.ts index de5571b26d..d22a6a3e8b 100644 --- a/packages/app/src/cli/services/execute-operation.test.ts +++ b/packages/app/src/cli/services/execute-operation.test.ts @@ -1,5 +1,5 @@ import {executeOperation} from './execute-operation.js' -import {createAdminSessionAsApp, validateApiVersion} from './graphql/common.js' +import {createAdminSessionAsApp, resolveApiVersion} from './graphql/common.js' import {OrganizationApp, OrganizationSource} from '../models/organization.js' import {renderSuccess, renderError, renderSingleTask} from '@shopify/cli-kit/node/ui' import {adminRequestDoc} from '@shopify/cli-kit/node/api/admin' @@ -32,6 +32,7 @@ describe('executeOperation', () => { beforeEach(() => { vi.mocked(createAdminSessionAsApp).mockResolvedValue(mockAdminSession) + vi.mocked(resolveApiVersion).mockResolvedValue('2024-07') vi.mocked(renderSingleTask).mockImplementation(async ({task}) => { return task(() => {}) }) @@ -54,12 +55,13 @@ describe('executeOperation', () => { }) expect(createAdminSessionAsApp).toHaveBeenCalledWith(mockRemoteApp, storeFqdn) + expect(resolveApiVersion).toHaveBeenCalledWith(mockAdminSession, undefined) expect(adminRequestDoc).toHaveBeenCalledWith({ // parsed GraphQL document query: expect.any(Object), session: mockAdminSession, variables: undefined, - version: undefined, + version: '2024-07', responseOptions: {handleErrors: false}, }) }) @@ -107,7 +109,7 @@ describe('executeOperation', () => { const version = '2024-01' const mockResult = {data: {shop: {name: 'Test Shop'}}} vi.mocked(adminRequestDoc).mockResolvedValue(mockResult) - vi.mocked(validateApiVersion).mockResolvedValue() + vi.mocked(resolveApiVersion).mockResolvedValue(version) await executeOperation({ organization: mockOrganization, @@ -117,7 +119,7 @@ describe('executeOperation', () => { version, }) - expect(validateApiVersion).toHaveBeenCalledWith(mockAdminSession, version) + expect(resolveApiVersion).toHaveBeenCalledWith(mockAdminSession, version) expect(adminRequestDoc).toHaveBeenCalledWith( expect.objectContaining({ version, @@ -125,22 +127,6 @@ describe('executeOperation', () => { ) }) - test('does not validate version when not provided', async () => { - const query = 'query { shop { name } }' - const mockResult = {data: {shop: {name: 'Test Shop'}}} - vi.mocked(adminRequestDoc).mockResolvedValue(mockResult) - vi.mocked(validateApiVersion).mockClear() - - await executeOperation({ - organization: mockOrganization, - remoteApp: mockRemoteApp, - storeFqdn, - query, - }) - - expect(validateApiVersion).not.toHaveBeenCalled() - }) - test('writes formatted JSON results to stdout by default', async () => { const query = 'query { shop { name } }' const mockResult = {data: {shop: {name: 'Test Shop'}}} diff --git a/packages/app/src/cli/services/execute-operation.ts b/packages/app/src/cli/services/execute-operation.ts index a84112bb54..93be554ba3 100644 --- a/packages/app/src/cli/services/execute-operation.ts +++ b/packages/app/src/cli/services/execute-operation.ts @@ -1,7 +1,7 @@ import { createAdminSessionAsApp, validateSingleOperation, - validateApiVersion, + resolveApiVersion, formatOperationInfo, } from './graphql/common.js' import {OrganizationApp, Organization} from '../models/organization.js' @@ -38,7 +38,11 @@ async function parseVariables(variables?: string): Promise<{[key: string]: unkno } export async function executeOperation(input: ExecuteOperationInput): Promise { - const {organization, remoteApp, storeFqdn, query, variables, version, outputFile} = input + const {organization, remoteApp, storeFqdn, query, variables, version: versionFlag, outputFile} = input + + const adminSession = await createAdminSessionAsApp(remoteApp, storeFqdn) + + const version = await resolveApiVersion(adminSession, versionFlag) renderInfo({ headline: 'Executing GraphQL operation.', @@ -51,10 +55,6 @@ export async function executeOperation(input: ExecuteOperationInput): Promise { @@ -16,7 +16,7 @@ vi.mock('@shopify/cli-kit/node/api/admin', async () => { const actual = await vi.importActual('@shopify/cli-kit/node/api/admin') return { ...actual, - supportedApiVersions: vi.fn(), + fetchApiVersions: vi.fn(), } }) @@ -106,28 +106,121 @@ describe('validateSingleOperation', () => { }) }) -describe('validateApiVersion', () => { +describe('resolveApiVersion', () => { const mockAdminSession = {token: 'test-token', storeFqdn: 'test-store.myshopify.com'} - test('allows unstable version without validation', async () => { - await expect(validateApiVersion(mockAdminSession, 'unstable')).resolves.not.toThrow() + test('returns unstable version without validation', async () => { + const result = await resolveApiVersion(mockAdminSession, 'unstable') - expect(supportedApiVersions).not.toHaveBeenCalled() + expect(result).toBe('unstable') + expect(fetchApiVersions).not.toHaveBeenCalled() }) - test('allows supported API version', async () => { - vi.mocked(supportedApiVersions).mockResolvedValue(['2024-01', '2024-04', '2024-07']) + test('returns user-provided version when allowed', async () => { + vi.mocked(fetchApiVersions).mockResolvedValue([ + {handle: '2024-01', supported: true}, + {handle: '2024-04', supported: true}, + {handle: '2024-07', supported: true}, + ]) - await expect(validateApiVersion(mockAdminSession, '2024-04')).resolves.not.toThrow() + const result = await resolveApiVersion(mockAdminSession, '2024-04') - expect(supportedApiVersions).toHaveBeenCalledWith(mockAdminSession) + expect(result).toBe('2024-04') + expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) }) - test('throws error when API version is not supported', async () => { - vi.mocked(supportedApiVersions).mockResolvedValue(['2024-01', '2024-04', '2024-07']) + test('returns user-provided version even when marked as unsupported', async () => { + vi.mocked(fetchApiVersions).mockResolvedValue([ + {handle: '2024-01', supported: true}, + {handle: '2024-04', supported: false}, + {handle: '2024-07', supported: true}, + ]) - await expect(validateApiVersion(mockAdminSession, '2023-01')).rejects.toThrow('Invalid API version: 2023-01') + const result = await resolveApiVersion(mockAdminSession, '2024-04') - expect(supportedApiVersions).toHaveBeenCalledWith(mockAdminSession) + expect(result).toBe('2024-04') + expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) + }) + + test('throws error when user-provided version is not in API version list', async () => { + vi.mocked(fetchApiVersions).mockResolvedValue([ + {handle: '2024-01', supported: true}, + {handle: '2024-04', supported: true}, + {handle: '2024-07', supported: true}, + ]) + + await expect(resolveApiVersion(mockAdminSession, '2023-01')).rejects.toThrow('Invalid API version: 2023-01') + expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) + }) + + test('returns most recent supported version when no version or minimum version provided', async () => { + vi.mocked(fetchApiVersions).mockResolvedValue([ + {handle: '2024-01', supported: true}, + {handle: '2024-04', supported: true}, + {handle: '2024-07', supported: true}, + {handle: '2025-01', supported: false}, + ]) + + const result = await resolveApiVersion(mockAdminSession) + + expect(result).toBe('2024-07') + expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) + }) + + test('returns minimum version when no version provided and most recent supported is older', async () => { + vi.mocked(fetchApiVersions).mockResolvedValue([ + {handle: '2024-01', supported: true}, + {handle: '2024-04', supported: true}, + {handle: '2025-01', supported: false}, + {handle: '2025-10', supported: true}, + ]) + + const result = await resolveApiVersion(mockAdminSession, undefined, '2026-01') + + expect(result).toBe('2026-01') + expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) + }) + + test('returns most recent supported version when newer than minimum version', async () => { + vi.mocked(fetchApiVersions).mockResolvedValue([ + {handle: '2026-01', supported: true}, + {handle: '2026-04', supported: true}, + {handle: '2026-07', supported: true}, + {handle: '2027-01', supported: false}, + ]) + + const result = await resolveApiVersion(mockAdminSession, undefined, '2026-01') + + expect(result).toBe('2026-07') + expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) + }) +}) + +describe('formatOperationInfo', () => { + const mockOptions = { + organization: {businessName: 'Test Organization'}, + remoteApp: {title: 'Test App'}, + storeFqdn: 'test-store.myshopify.com', + } + + test('includes API version when provided', () => { + const result = formatOperationInfo({ + ...mockOptions, + version: '2024-07', + }) + + expect(result).toEqual([ + 'Organization: Test Organization', + 'App: Test App', + 'Store: test-store.myshopify.com', + 'API version: 2024-07', + ]) + }) + + test('omits API version when not provided', () => { + const result = formatOperationInfo(mockOptions) + + expect(result).toEqual(['Organization: Test Organization', 'App: Test App', 'Store: test-store.myshopify.com']) + expect(result).not.toContain(expect.stringContaining('API version')) }) }) diff --git a/packages/app/src/cli/services/graphql/common.ts b/packages/app/src/cli/services/graphql/common.ts index 503339b0a0..50eeb36f7a 100644 --- a/packages/app/src/cli/services/graphql/common.ts +++ b/packages/app/src/cli/services/graphql/common.ts @@ -2,7 +2,7 @@ import {OrganizationApp} from '../../models/organization.js' import {ensureAuthenticatedAdminAsApp, AdminSession} from '@shopify/cli-kit/node/session' import {AbortError, BugError} from '@shopify/cli-kit/node/error' import {outputContent} from '@shopify/cli-kit/node/output' -import {supportedApiVersions} from '@shopify/cli-kit/node/api/admin' +import {fetchApiVersions} from '@shopify/cli-kit/node/api/admin' import {parse} from 'graphql' /** @@ -46,31 +46,47 @@ export function validateSingleOperation(graphqlOperation: string): void { } /** - * Validates that the specified API version is supported by the store. + * Determines the API version to use based on the user provided version and the available versions. * The 'unstable' version is always allowed without validation. * * @param adminSession - Admin session containing store credentials. - * @param version - The API version to validate. - * @throws AbortError if the version is not supported by the store. + * @param versionFlag - The API version to validate. + * @param minimumVersion - Optional minimum version to use as a fallback when no version is specified. + * @throws AbortError if the provided version is not allowed. */ -export async function validateApiVersion( +export async function resolveApiVersion( adminSession: {token: string; storeFqdn: string}, - version: string, -): Promise { - if (version === 'unstable') return + versionFlag?: string, + minimumVersion?: string, +): Promise { + if (versionFlag === 'unstable') return versionFlag - const supportedVersions = await supportedApiVersions(adminSession) - if (supportedVersions.includes(version)) return + const availableVersions = await fetchApiVersions(adminSession) - const firstLine = outputContent`Invalid API version: ${version}`.value - const secondLine = outputContent`Supported versions: ${supportedVersions.join(', ')}`.value + // Return the most recent supported version, or minimumVersion if specified, whichever is newer + if (!versionFlag) { + const supportedVersions = availableVersions.filter((version) => version.supported).map((version) => version.handle) - throw new AbortError(`${firstLine}\n${secondLine}`) + if (minimumVersion) { + supportedVersions.push(minimumVersion) + } + + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + return supportedVersions.sort().reverse()[0]! + } + + // Check if the user provided version is allowed. Unsupported versions (RC) are allowed here. + const versionList = availableVersions.map((version) => version.handle) + if (versionList.includes(versionFlag)) return versionFlag + + const firstLine = outputContent`Invalid API version: ${versionFlag}`.value + const secondLine = outputContent`Allowed versions: ${versionList.join(', ')}`.value + throw new AbortError(firstLine, secondLine) } /** * Creates formatted info list items for GraphQL operations. - * Includes organization, app, store, and API version information. + * Includes organization, app, store, and optionally API version information. * * @param options - The operation context information * @returns Array of formatted strings for display @@ -80,14 +96,13 @@ export function formatOperationInfo(options: { remoteApp: {title: string} storeFqdn: string version?: string - showVersion?: boolean }): string[] { - const {organization, remoteApp, storeFqdn, version, showVersion = true} = options + const {organization, remoteApp, storeFqdn, version} = options const items = [`Organization: ${organization.businessName}`, `App: ${remoteApp.title}`, `Store: ${storeFqdn}`] - if (showVersion) { - items.push(`API version: ${version ?? 'default (latest stable)'}`) + if (version) { + items.push(`API version: ${version}`) } return items diff --git a/packages/cli-kit/src/public/node/api/admin.ts b/packages/cli-kit/src/public/node/api/admin.ts index 77c12ea647..107d91686f 100644 --- a/packages/cli-kit/src/public/node/api/admin.ts +++ b/packages/cli-kit/src/public/node/api/admin.ts @@ -157,7 +157,10 @@ export async function supportedApiVersions( * @param preferredBehaviour - Custom request behaviour for retries and timeouts. * @returns - An array of supported and unsupported API versions. */ -async function fetchApiVersions(session: AdminSession, preferredBehaviour?: RequestModeInput): Promise { +export async function fetchApiVersions( + session: AdminSession, + preferredBehaviour?: RequestModeInput, +): Promise { try { const response = await adminRequestDoc({ query: PublicApiVersions, From db481d2d22b386afbd312a98757f74b2977dd6a4 Mon Sep 17 00:00:00 2001 From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com> Date: Tue, 9 Dec 2025 15:13:25 -0500 Subject: [PATCH 3/8] PR feedback on function signature --- .../src/cli/services/graphql/common.test.ts | 2 +- .../app/src/cli/services/graphql/common.ts | 24 +++++++++---------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/packages/app/src/cli/services/graphql/common.test.ts b/packages/app/src/cli/services/graphql/common.test.ts index 48de15f726..73c36d98b7 100644 --- a/packages/app/src/cli/services/graphql/common.test.ts +++ b/packages/app/src/cli/services/graphql/common.test.ts @@ -123,7 +123,7 @@ describe('resolveApiVersion', () => { {handle: '2024-07', supported: true}, ]) - const result = await resolveApiVersion(mockAdminSession, '2024-04') + const result = await resolveApiVersion(mockAdminSession, '2024-04', '2026-01') expect(result).toBe('2024-04') expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) diff --git a/packages/app/src/cli/services/graphql/common.ts b/packages/app/src/cli/services/graphql/common.ts index 50eeb36f7a..900548500d 100644 --- a/packages/app/src/cli/services/graphql/common.ts +++ b/packages/app/src/cli/services/graphql/common.ts @@ -50,25 +50,24 @@ export function validateSingleOperation(graphqlOperation: string): void { * The 'unstable' version is always allowed without validation. * * @param adminSession - Admin session containing store credentials. - * @param versionFlag - The API version to validate. - * @param minimumVersion - Optional minimum version to use as a fallback when no version is specified. + * @param userSpecifiedVersion - The API version to validate. + * @param minimumDefaultVersion - Optional minimum version to use as a fallback when no version is specified. * @throws AbortError if the provided version is not allowed. */ export async function resolveApiVersion( adminSession: {token: string; storeFqdn: string}, - versionFlag?: string, - minimumVersion?: string, + userSpecifiedVersion?: string, + minimumDefaultVersion?: string, ): Promise { - if (versionFlag === 'unstable') return versionFlag + if (userSpecifiedVersion === 'unstable') return userSpecifiedVersion const availableVersions = await fetchApiVersions(adminSession) - // Return the most recent supported version, or minimumVersion if specified, whichever is newer - if (!versionFlag) { + if (!userSpecifiedVersion) { + // Return the most recent supported version, or minimumDefaultVersion if specified, whichever is newer. const supportedVersions = availableVersions.filter((version) => version.supported).map((version) => version.handle) - - if (minimumVersion) { - supportedVersions.push(minimumVersion) + if (minimumDefaultVersion) { + supportedVersions.push(minimumDefaultVersion) } // eslint-disable-next-line @typescript-eslint/no-non-null-assertion @@ -77,9 +76,10 @@ export async function resolveApiVersion( // Check if the user provided version is allowed. Unsupported versions (RC) are allowed here. const versionList = availableVersions.map((version) => version.handle) - if (versionList.includes(versionFlag)) return versionFlag + if (versionList.includes(userSpecifiedVersion)) return userSpecifiedVersion - const firstLine = outputContent`Invalid API version: ${versionFlag}`.value + // Invalid user provided version. + const firstLine = outputContent`Invalid API version: ${userSpecifiedVersion}`.value const secondLine = outputContent`Allowed versions: ${versionList.join(', ')}`.value throw new AbortError(firstLine, secondLine) } From 2aeb59668b38f75b7a445111ae670f331131e98f Mon Sep 17 00:00:00 2001 From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com> Date: Tue, 9 Dec 2025 15:30:14 -0500 Subject: [PATCH 4/8] better testing of resolveApiVersion use --- .../bulk-operation-status.test.ts | 30 ++++++++++++- .../execute-bulk-operation.test.ts | 42 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts index b1e763f647..c190a29927 100644 --- a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts +++ b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts @@ -179,7 +179,7 @@ describe('getBulkOperationStatus', () => { expect(output.info()).toContain('Bulk operation canceled.') }) - test('calls resolveApiVersion with minimum API version', async () => { + test('calls resolveApiVersion with minimum API version constant', async () => { vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'})) await getBulkOperationStatus({organization: mockOrganization, storeFqdn, operationId, remoteApp}) @@ -191,6 +191,19 @@ describe('getBulkOperationStatus', () => { ) }) + test('uses resolved API version in admin request', async () => { + vi.mocked(resolveApiVersion).mockResolvedValue('test-api-version') + vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'})) + + await getBulkOperationStatus({organization: mockOrganization, storeFqdn, operationId, remoteApp}) + + expect(adminRequestDoc).toHaveBeenCalledWith( + expect.objectContaining({ + version: 'test-api-version', + }), + ) + }) + describe('time formatting', () => { test('uses "Started" for running operations', async () => { vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperation({status: 'RUNNING'})) @@ -351,7 +364,7 @@ describe('listBulkOperations', () => { expect(output.info()).toContain('No bulk operations found in the last 7 days.') }) - test('calls resolveApiVersion with minimum API version', async () => { + test('calls resolveApiVersion with minimum API version constant', async () => { vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperationsList([])) await listBulkOperations({organization: mockOrganization, storeFqdn, remoteApp}) @@ -362,4 +375,17 @@ describe('listBulkOperations', () => { BULK_OPERATIONS_MIN_API_VERSION, ) }) + + test('uses resolved API version in admin request', async () => { + vi.mocked(resolveApiVersion).mockResolvedValue('test-api-version') + vi.mocked(adminRequestDoc).mockResolvedValue(mockBulkOperationsList([])) + + await listBulkOperations({organization: mockOrganization, storeFqdn, remoteApp}) + + expect(adminRequestDoc).toHaveBeenCalledWith( + expect.objectContaining({ + version: 'test-api-version', + }), + ) + }) }) diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts index 8cac189891..9afd60c0ab 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts @@ -3,6 +3,7 @@ import {runBulkOperationQuery} from './run-query.js' import {runBulkOperationMutation} from './run-mutation.js' import {watchBulkOperation, shortBulkOperationPoll} from './watch-bulk-operation.js' import {downloadBulkOperationResults} from './download-bulk-operation-results.js' +import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js' import {resolveApiVersion} from '../graphql/common.js' import {BulkOperationRunQueryMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-query.js' import {BulkOperationRunMutationMutation} from '../../api/graphql/bulk-operations/generated/bulk-operation-run-mutation.js' @@ -675,4 +676,45 @@ describe('executeBulkOperation', () => { }), ) }) + + test('calls resolveApiVersion with minimum API version constant', async () => { + const query = '{ products { edges { node { id } } } }' + const mockResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = { + bulkOperation: createdBulkOperation, + userErrors: [], + } + vi.mocked(runBulkOperationQuery).mockResolvedValue(mockResponse) + + await executeBulkOperation({ + organization: mockOrganization, + remoteApp: mockRemoteApp, + storeFqdn, + query, + }) + + expect(resolveApiVersion).toHaveBeenCalledWith(mockAdminSession, undefined, BULK_OPERATIONS_MIN_API_VERSION) + }) + + test('uses resolved API version when running bulk operation', async () => { + vi.mocked(resolveApiVersion).mockResolvedValue('test-api-version') + const query = '{ products { edges { node { id } } } }' + const mockResponse: BulkOperationRunQueryMutation['bulkOperationRunQuery'] = { + bulkOperation: createdBulkOperation, + userErrors: [], + } + vi.mocked(runBulkOperationQuery).mockResolvedValue(mockResponse) + + await executeBulkOperation({ + organization: mockOrganization, + remoteApp: mockRemoteApp, + storeFqdn, + query, + }) + + expect(runBulkOperationQuery).toHaveBeenCalledWith({ + adminSession: mockAdminSession, + query, + version: 'test-api-version', + }) + }) }) From d2e908f269bbbf1fbc14ef647147fb4a940c4772 Mon Sep 17 00:00:00 2001 From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com> Date: Fri, 12 Dec 2025 11:00:38 -0500 Subject: [PATCH 5/8] PR feedback: Use constant for API version in tests --- .../bulk-operations/execute-bulk-operation.test.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts index 9afd60c0ab..6ab2d6937b 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts @@ -69,7 +69,7 @@ describe('executeBulkOperation', () => { beforeEach(() => { vi.mocked(ensureAuthenticatedAdminAsApp).mockResolvedValue(mockAdminSession) vi.mocked(shortBulkOperationPoll).mockResolvedValue(createdBulkOperation) - vi.mocked(resolveApiVersion).mockResolvedValue('2026-01') + vi.mocked(resolveApiVersion).mockResolvedValue(BULK_OPERATIONS_MIN_API_VERSION) }) afterEach(() => { @@ -94,7 +94,7 @@ describe('executeBulkOperation', () => { expect(runBulkOperationQuery).toHaveBeenCalledWith({ adminSession: mockAdminSession, query, - version: '2026-01', + version: BULK_OPERATIONS_MIN_API_VERSION, }) expect(runBulkOperationMutation).not.toHaveBeenCalled() }) @@ -117,7 +117,7 @@ describe('executeBulkOperation', () => { expect(runBulkOperationQuery).toHaveBeenCalledWith({ adminSession: mockAdminSession, query, - version: '2026-01', + version: BULK_OPERATIONS_MIN_API_VERSION, }) expect(runBulkOperationMutation).not.toHaveBeenCalled() }) @@ -141,7 +141,7 @@ describe('executeBulkOperation', () => { adminSession: mockAdminSession, query: mutation, variablesJsonl: undefined, - version: '2026-01', + version: BULK_OPERATIONS_MIN_API_VERSION, }) expect(runBulkOperationQuery).not.toHaveBeenCalled() }) @@ -167,7 +167,7 @@ describe('executeBulkOperation', () => { adminSession: mockAdminSession, query: mutation, variablesJsonl: '{"input":{"id":"gid://shopify/Product/123","tags":["test"]}}', - version: '2026-01', + version: BULK_OPERATIONS_MIN_API_VERSION, }) }) From b7caf67bd99cf66e9b9e7f03b73b1b4945a5d88c Mon Sep 17 00:00:00 2001 From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com> Date: Fri, 12 Dec 2025 14:59:11 -0500 Subject: [PATCH 6/8] PR feedback: Added an options type for resolveApiVersion --- .../bulk-operation-status.test.ts | 18 +++++++------- .../bulk-operations/bulk-operation-status.ts | 10 ++++++-- .../execute-bulk-operation.test.ts | 6 ++++- .../bulk-operations/execute-bulk-operation.ts | 8 +++++-- .../cli/services/execute-operation.test.ts | 4 ++-- .../app/src/cli/services/execute-operation.ts | 4 ++-- .../src/cli/services/graphql/common.test.ts | 20 ++++++++++------ .../app/src/cli/services/graphql/common.ts | 24 ++++++++++++------- 8 files changed, 60 insertions(+), 34 deletions(-) diff --git a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts index c190a29927..d3c510b7cb 100644 --- a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts +++ b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts @@ -184,11 +184,10 @@ describe('getBulkOperationStatus', () => { await getBulkOperationStatus({organization: mockOrganization, storeFqdn, operationId, remoteApp}) - expect(resolveApiVersion).toHaveBeenCalledWith( - {token: 'test-token', storeFqdn}, - undefined, - BULK_OPERATIONS_MIN_API_VERSION, - ) + expect(resolveApiVersion).toHaveBeenCalledWith({ + adminSession: {token: 'test-token', storeFqdn}, + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, + }) }) test('uses resolved API version in admin request', async () => { @@ -369,11 +368,10 @@ describe('listBulkOperations', () => { await listBulkOperations({organization: mockOrganization, storeFqdn, remoteApp}) - expect(resolveApiVersion).toHaveBeenCalledWith( - {token: 'test-token', storeFqdn}, - undefined, - BULK_OPERATIONS_MIN_API_VERSION, - ) + expect(resolveApiVersion).toHaveBeenCalledWith({ + adminSession: {token: 'test-token', storeFqdn}, + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, + }) }) test('uses resolved API version in admin request', async () => { diff --git a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts index a1d19b1dbc..54b8c8aa3f 100644 --- a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts +++ b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.ts @@ -77,7 +77,10 @@ export async function getBulkOperationStatus(options: GetBulkOperationStatusOpti query: GetBulkOperationById, session: adminSession, variables: {id: operationId}, - version: await resolveApiVersion(adminSession, undefined, BULK_OPERATIONS_MIN_API_VERSION), + version: await resolveApiVersion({ + adminSession, + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, + }), }) if (response.bulkOperation) { @@ -120,7 +123,10 @@ export async function listBulkOperations(options: ListBulkOperationsOptions): Pr sortKey: 'CREATED_AT', reverse: true, }, - version: await resolveApiVersion(adminSession, undefined, BULK_OPERATIONS_MIN_API_VERSION), + version: await resolveApiVersion({ + adminSession, + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, + }), }) const operations = response.bulkOperations.nodes.map((operation) => ({ diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts index 6ab2d6937b..8d450a17d2 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.test.ts @@ -692,7 +692,11 @@ describe('executeBulkOperation', () => { query, }) - expect(resolveApiVersion).toHaveBeenCalledWith(mockAdminSession, undefined, BULK_OPERATIONS_MIN_API_VERSION) + expect(resolveApiVersion).toHaveBeenCalledWith({ + adminSession: mockAdminSession, + userSpecifiedVersion: undefined, + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, + }) }) test('uses resolved API version when running bulk operation', async () => { diff --git a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts index 860f2490b8..06c5d7ff4c 100644 --- a/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts +++ b/packages/app/src/cli/services/bulk-operations/execute-bulk-operation.ts @@ -58,12 +58,16 @@ export async function executeBulkOperation(input: ExecuteBulkOperationInput): Pr variableFile, outputFile, watch = false, - version: versionFlag, + version: userSpecifiedVersion, } = input const adminSession = await createAdminSessionAsApp(remoteApp, storeFqdn) - const version = await resolveApiVersion(adminSession, versionFlag, BULK_OPERATIONS_MIN_API_VERSION) + const version = await resolveApiVersion({ + adminSession, + userSpecifiedVersion, + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, + }) const variablesJsonl = await parseVariablesToJsonl(variables, variableFile) diff --git a/packages/app/src/cli/services/execute-operation.test.ts b/packages/app/src/cli/services/execute-operation.test.ts index d22a6a3e8b..8f420b7331 100644 --- a/packages/app/src/cli/services/execute-operation.test.ts +++ b/packages/app/src/cli/services/execute-operation.test.ts @@ -55,7 +55,7 @@ describe('executeOperation', () => { }) expect(createAdminSessionAsApp).toHaveBeenCalledWith(mockRemoteApp, storeFqdn) - expect(resolveApiVersion).toHaveBeenCalledWith(mockAdminSession, undefined) + expect(resolveApiVersion).toHaveBeenCalledWith({adminSession: mockAdminSession}) expect(adminRequestDoc).toHaveBeenCalledWith({ // parsed GraphQL document query: expect.any(Object), @@ -119,7 +119,7 @@ describe('executeOperation', () => { version, }) - expect(resolveApiVersion).toHaveBeenCalledWith(mockAdminSession, version) + expect(resolveApiVersion).toHaveBeenCalledWith({adminSession: mockAdminSession, userSpecifiedVersion: version}) expect(adminRequestDoc).toHaveBeenCalledWith( expect.objectContaining({ version, diff --git a/packages/app/src/cli/services/execute-operation.ts b/packages/app/src/cli/services/execute-operation.ts index 93be554ba3..aa3d46149b 100644 --- a/packages/app/src/cli/services/execute-operation.ts +++ b/packages/app/src/cli/services/execute-operation.ts @@ -38,11 +38,11 @@ async function parseVariables(variables?: string): Promise<{[key: string]: unkno } export async function executeOperation(input: ExecuteOperationInput): Promise { - const {organization, remoteApp, storeFqdn, query, variables, version: versionFlag, outputFile} = input + const {organization, remoteApp, storeFqdn, query, variables, version: userSpecifiedVersion, outputFile} = input const adminSession = await createAdminSessionAsApp(remoteApp, storeFqdn) - const version = await resolveApiVersion(adminSession, versionFlag) + const version = await resolveApiVersion({adminSession, userSpecifiedVersion}) renderInfo({ headline: 'Executing GraphQL operation.', diff --git a/packages/app/src/cli/services/graphql/common.test.ts b/packages/app/src/cli/services/graphql/common.test.ts index 73c36d98b7..4e78c80c32 100644 --- a/packages/app/src/cli/services/graphql/common.test.ts +++ b/packages/app/src/cli/services/graphql/common.test.ts @@ -110,7 +110,7 @@ describe('resolveApiVersion', () => { const mockAdminSession = {token: 'test-token', storeFqdn: 'test-store.myshopify.com'} test('returns unstable version without validation', async () => { - const result = await resolveApiVersion(mockAdminSession, 'unstable') + const result = await resolveApiVersion({adminSession: mockAdminSession, userSpecifiedVersion: 'unstable'}) expect(result).toBe('unstable') expect(fetchApiVersions).not.toHaveBeenCalled() @@ -123,7 +123,11 @@ describe('resolveApiVersion', () => { {handle: '2024-07', supported: true}, ]) - const result = await resolveApiVersion(mockAdminSession, '2024-04', '2026-01') + const result = await resolveApiVersion({ + adminSession: mockAdminSession, + userSpecifiedVersion: '2024-04', + minimumDefaultVersion: '2026-01', + }) expect(result).toBe('2024-04') expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) @@ -136,7 +140,7 @@ describe('resolveApiVersion', () => { {handle: '2024-07', supported: true}, ]) - const result = await resolveApiVersion(mockAdminSession, '2024-04') + const result = await resolveApiVersion({adminSession: mockAdminSession, userSpecifiedVersion: '2024-04'}) expect(result).toBe('2024-04') expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) @@ -149,7 +153,9 @@ describe('resolveApiVersion', () => { {handle: '2024-07', supported: true}, ]) - await expect(resolveApiVersion(mockAdminSession, '2023-01')).rejects.toThrow('Invalid API version: 2023-01') + await expect(resolveApiVersion({adminSession: mockAdminSession, userSpecifiedVersion: '2023-01'})).rejects.toThrow( + 'Invalid API version: 2023-01', + ) expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) }) @@ -161,7 +167,7 @@ describe('resolveApiVersion', () => { {handle: '2025-01', supported: false}, ]) - const result = await resolveApiVersion(mockAdminSession) + const result = await resolveApiVersion({adminSession: mockAdminSession}) expect(result).toBe('2024-07') expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) @@ -175,7 +181,7 @@ describe('resolveApiVersion', () => { {handle: '2025-10', supported: true}, ]) - const result = await resolveApiVersion(mockAdminSession, undefined, '2026-01') + const result = await resolveApiVersion({adminSession: mockAdminSession, minimumDefaultVersion: '2026-01'}) expect(result).toBe('2026-01') expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) @@ -189,7 +195,7 @@ describe('resolveApiVersion', () => { {handle: '2027-01', supported: false}, ]) - const result = await resolveApiVersion(mockAdminSession, undefined, '2026-01') + const result = await resolveApiVersion({adminSession: mockAdminSession, minimumDefaultVersion: '2026-01'}) expect(result).toBe('2026-07') expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) diff --git a/packages/app/src/cli/services/graphql/common.ts b/packages/app/src/cli/services/graphql/common.ts index 900548500d..e84b643e92 100644 --- a/packages/app/src/cli/services/graphql/common.ts +++ b/packages/app/src/cli/services/graphql/common.ts @@ -45,20 +45,28 @@ export function validateSingleOperation(graphqlOperation: string): void { } } +/** + * Options for resolving an API version. + */ +export interface ResolveApiVersionOptions { + /** Admin session containing store credentials. */ + adminSession: {token: string; storeFqdn: string} + /** The API version specified by the user. */ + userSpecifiedVersion?: string + /** Optional minimum version to use as a fallback when no version is specified. */ + minimumDefaultVersion?: string +} + /** * Determines the API version to use based on the user provided version and the available versions. * The 'unstable' version is always allowed without validation. * - * @param adminSession - Admin session containing store credentials. - * @param userSpecifiedVersion - The API version to validate. - * @param minimumDefaultVersion - Optional minimum version to use as a fallback when no version is specified. + * @param options - Options for resolving the API version. * @throws AbortError if the provided version is not allowed. */ -export async function resolveApiVersion( - adminSession: {token: string; storeFqdn: string}, - userSpecifiedVersion?: string, - minimumDefaultVersion?: string, -): Promise { +export async function resolveApiVersion(options: ResolveApiVersionOptions): Promise { + const {adminSession, userSpecifiedVersion, minimumDefaultVersion} = options + if (userSpecifiedVersion === 'unstable') return userSpecifiedVersion const availableVersions = await fetchApiVersions(adminSession) From efc420b63b345ef8bb66a6a6773ce4dc887b4caa Mon Sep 17 00:00:00 2001 From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com> Date: Fri, 12 Dec 2025 15:19:55 -0500 Subject: [PATCH 7/8] remove unused export --- packages/app/src/cli/services/graphql/common.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/graphql/common.ts b/packages/app/src/cli/services/graphql/common.ts index e84b643e92..c13a0ab42f 100644 --- a/packages/app/src/cli/services/graphql/common.ts +++ b/packages/app/src/cli/services/graphql/common.ts @@ -48,7 +48,7 @@ export function validateSingleOperation(graphqlOperation: string): void { /** * Options for resolving an API version. */ -export interface ResolveApiVersionOptions { +interface ResolveApiVersionOptions { /** Admin session containing store credentials. */ adminSession: {token: string; storeFqdn: string} /** The API version specified by the user. */ From a96993ec30d130895527045ee195b7e06ddd5c31 Mon Sep 17 00:00:00 2001 From: Nick Wesselman <27013789+nickwesselman@users.noreply.github.com> Date: Fri, 12 Dec 2025 15:42:58 -0500 Subject: [PATCH 8/8] fix more instances of 2026-01 --- .../bulk-operation-status.test.ts | 2 +- .../bulk-operations/watch-bulk-operation.ts | 4 ++-- .../app/src/cli/services/graphql/common.test.ts | 17 ++++++++++++----- 3 files changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts index d3c510b7cb..3b5900b809 100644 --- a/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts +++ b/packages/app/src/cli/services/bulk-operations/bulk-operation-status.test.ts @@ -44,7 +44,7 @@ const remoteApp = { beforeEach(() => { vi.mocked(ensureAuthenticatedAdminAsApp).mockResolvedValue({token: 'test-token', storeFqdn}) - vi.mocked(resolveApiVersion).mockResolvedValue('2026-01') + vi.mocked(resolveApiVersion).mockResolvedValue(BULK_OPERATIONS_MIN_API_VERSION) }) afterEach(() => { diff --git a/packages/app/src/cli/services/bulk-operations/watch-bulk-operation.ts b/packages/app/src/cli/services/bulk-operations/watch-bulk-operation.ts index 0c44c3497f..e13b68ff8c 100644 --- a/packages/app/src/cli/services/bulk-operations/watch-bulk-operation.ts +++ b/packages/app/src/cli/services/bulk-operations/watch-bulk-operation.ts @@ -1,4 +1,5 @@ import {formatBulkOperationStatus} from './format-bulk-operation-status.js' +import {BULK_OPERATIONS_MIN_API_VERSION} from './constants.js' import { GetBulkOperationById, GetBulkOperationByIdQuery, @@ -14,7 +15,6 @@ const TERMINAL_STATUSES = ['COMPLETED', 'FAILED', 'CANCELED', 'EXPIRED'] const INITIAL_POLL_INTERVAL_SECONDS = 1 const REGULAR_POLL_INTERVAL_SECONDS = 5 const INITIAL_POLL_COUNT = 10 -const API_VERSION = '2026-01' export const QUICK_WATCH_TIMEOUT_MS = 3000 export const QUICK_WATCH_POLL_INTERVAL_MS = 300 @@ -146,6 +146,6 @@ async function fetchBulkOperation(adminSession: AdminSession, operationId: strin query: GetBulkOperationById, session: adminSession, variables: {id: operationId}, - version: API_VERSION, + version: BULK_OPERATIONS_MIN_API_VERSION, }) } diff --git a/packages/app/src/cli/services/graphql/common.test.ts b/packages/app/src/cli/services/graphql/common.test.ts index 4e78c80c32..b76a2e5fc8 100644 --- a/packages/app/src/cli/services/graphql/common.test.ts +++ b/packages/app/src/cli/services/graphql/common.test.ts @@ -1,5 +1,6 @@ import {createAdminSessionAsApp, formatOperationInfo, resolveApiVersion, validateSingleOperation} from './common.js' import {OrganizationApp} from '../../models/organization.js' +import {BULK_OPERATIONS_MIN_API_VERSION} from '../bulk-operations/constants.js' import {ensureAuthenticatedAdminAsApp} from '@shopify/cli-kit/node/session' import {fetchApiVersions} from '@shopify/cli-kit/node/api/admin' import {describe, test, expect, vi, beforeEach} from 'vitest' @@ -126,7 +127,7 @@ describe('resolveApiVersion', () => { const result = await resolveApiVersion({ adminSession: mockAdminSession, userSpecifiedVersion: '2024-04', - minimumDefaultVersion: '2026-01', + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, }) expect(result).toBe('2024-04') @@ -181,21 +182,27 @@ describe('resolveApiVersion', () => { {handle: '2025-10', supported: true}, ]) - const result = await resolveApiVersion({adminSession: mockAdminSession, minimumDefaultVersion: '2026-01'}) + const result = await resolveApiVersion({ + adminSession: mockAdminSession, + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, + }) - expect(result).toBe('2026-01') + expect(result).toBe(BULK_OPERATIONS_MIN_API_VERSION) expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession) }) test('returns most recent supported version when newer than minimum version', async () => { vi.mocked(fetchApiVersions).mockResolvedValue([ - {handle: '2026-01', supported: true}, + {handle: BULK_OPERATIONS_MIN_API_VERSION, supported: true}, {handle: '2026-04', supported: true}, {handle: '2026-07', supported: true}, {handle: '2027-01', supported: false}, ]) - const result = await resolveApiVersion({adminSession: mockAdminSession, minimumDefaultVersion: '2026-01'}) + const result = await resolveApiVersion({ + adminSession: mockAdminSession, + minimumDefaultVersion: BULK_OPERATIONS_MIN_API_VERSION, + }) expect(result).toBe('2026-07') expect(fetchApiVersions).toHaveBeenCalledWith(mockAdminSession)