PR feedback: Added an options type for resolveApiVersion

Author: nickwesselman

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 () => {

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) => ({

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 () => {

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)
 

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,

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<void> {
-  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.',

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)

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<string> {
+export async function resolveApiVersion(options: ResolveApiVersionOptions): Promise<string> {
+  const {adminSession, userSpecifiedVersion, minimumDefaultVersion} = options
+
   if (userSpecifiedVersion === 'unstable') return userSpecifiedVersion
 
   const availableVersions = await fetchApiVersions(adminSession)