Route theme mutations through requestThemeAdminDoc

.changeset/theme-mutations-missing-scope.md

@@ -0,0 +1,5 @@
+---
+'@shopify/cli-kit': patch
+---
+
+Show the missing theme access scope message for theme changes, as theme reads already do.

packages/cli-kit/src/private/node/api/graphql.test.ts

@@ -1,4 +1,4 @@
-import {debugLogRequestInfo, extractGraphQLErrorMessages, errorHandler} from './graphql.js'
+import {debugLogRequestInfo, extractGraphQLErrorMessages, errorHandler, extractGraphQLErrors} from './graphql.js'
 import {GraphQLClientError} from './headers.js'
 import {AbortError} from '../../../public/node/error.js'
 import {runWithCommandEventsForCommand} from '../../../public/node/command-events.js'
@@ -218,3 +218,23 @@ describe('errorHandler', () => {
     expect(handler(error)).toBe(error)
   })
 })
+
+describe('extractGraphQLErrors', () => {
+  const graphQLErrors = [{message: 'Challenge Required', extensions: {code: 'CHALLENGE_REQUIRED'}}]
+  const clientError = new ClientError(
+    {status: 200, headers: new Map(), errors: graphQLErrors} as unknown as ClientError['response'],
+    {query: 'query {}'},
+  )
+
+  test('returns the same GraphQL errors before and after errorHandler maps the error', () => {
+    const mappedError = errorHandler('Admin')(clientError, 'req-123')
+
+    expect(mappedError).toBeInstanceOf(GraphQLClientError)
+    expect(extractGraphQLErrors(clientError)).toEqual(graphQLErrors)
+    expect(extractGraphQLErrors(mappedError)).toEqual(graphQLErrors)
+  })
+
+  test('returns no GraphQL errors for an error that is not a GraphQL error', () => {
+    expect(extractGraphQLErrors(new AbortError('Network error'))).toEqual([])
+  })
+})

packages/cli-kit/src/private/node/api/graphql.ts

@@ -159,3 +159,9 @@ ${outputToken.json(error.response.errors)}
     }
   }
 }
+
+export function extractGraphQLErrors(error: unknown): NonNullable<ClientError['response']['errors']> {
+  if (error instanceof ClientError) return Array.isArray(error.response.errors) ? error.response.errors : []
+  if (error instanceof GraphQLClientError) return Array.isArray(error.errors) ? error.errors : []
+  return []
+}

packages/cli-kit/src/public/node/themes/api.test.ts

@@ -28,6 +28,7 @@ import {GetThemes} from '../../../cli/api/graphql/admin/generated/get_themes.js'
 import {GetTheme} from '../../../cli/api/graphql/admin/generated/get_theme.js'
 import {FindDevelopmentThemeByName} from '../../../cli/api/graphql/admin/generated/find_development_theme_by_name.js'
 import {MetafieldDefinitionsByOwnerType} from '../../../cli/api/graphql/admin/generated/metafield_definitions_by_owner_type.js'
+import {errorHandler} from '../../../private/node/api/graphql.js'
 import {adminRequestDoc, supportedApiVersions} from '../api/admin.js'
 import {AbortError} from '../error.js'
 
@@ -502,6 +503,16 @@ describe('themeUpdate', () => {
     expect(theme!.name).toEqual(name)
     expect(theme!.role).toEqual(role)
   })
+
+  test('throws a friendly error when access is denied by a missing theme access scope', async () => {
+    vi.mocked(adminRequestDoc).mockRejectedValue(
+      errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'),
+    )
+
+    await expect(themeUpdate(123, {name: 'renamed theme'}, session)).rejects.toThrow(
+      'The authenticated account or access token is missing `write_themes` access scope.',
+    )
+  })
 })
 
 describe('themePublish', () => {
@@ -539,6 +550,16 @@ describe('themePublish', () => {
       expect(theme!.role).toEqual(role)
     })
   }
+
+  test('throws a friendly error when access is denied by a missing theme access scope', async () => {
+    vi.mocked(adminRequestDoc).mockRejectedValue(
+      errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'),
+    )
+
+    await expect(themePublish(123, session)).rejects.toThrow(
+      'The authenticated account or access token is missing `write_themes` access scope.',
+    )
+  })
 })
 
 describe('deleteThemeAssets', () => {
@@ -597,6 +618,16 @@ describe('deleteThemeAssets', () => {
     })
     expect(output).toEqual([{key, success: true, operation: 'DELETE'}])
   })
+
+  test('throws a friendly error when access is denied by a missing theme access scope', async () => {
+    vi.mocked(adminRequestDoc).mockRejectedValue(
+      errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'),
+    )
+
+    await expect(deleteThemeAssets(123, ['snippets/product-variant-picker.liquid'], session)).rejects.toThrow(
+      'The authenticated account or access token is missing `write_themes` access scope.',
+    )
+  })
 })
 
 describe('themeDelete', () => {
@@ -625,6 +656,16 @@ describe('themeDelete', () => {
       expect(response).toBe(true)
     })
   }
+
+  test('throws a friendly error when access is denied by a missing theme access scope', async () => {
+    vi.mocked(adminRequestDoc).mockRejectedValue(
+      errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'),
+    )
+
+    await expect(themeDelete(123, session)).rejects.toThrow(
+      'The authenticated account or access token is missing `write_themes` access scope.',
+    )
+  })
 })
 
 describe('themeDuplicate', () => {
@@ -660,6 +701,16 @@ describe('themeDuplicate', () => {
       })
     })
   }
+
+  test('throws a friendly error when access is denied by a missing theme access scope', async () => {
+    vi.mocked(adminRequestDoc).mockRejectedValue(
+      errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'),
+    )
+
+    await expect(themeDuplicate(123, 'duplicate theme', session)).rejects.toThrow(
+      'The authenticated account or access token is missing `write_themes` access scope.',
+    )
+  })
 })
 
 describe('request errors', () => {
@@ -825,6 +876,16 @@ describe('bulkUploadThemeAssets', async () => {
       'Error uploading theme files: Something went wrong',
     )
   })
+
+  test('throws a friendly error when access is denied by a missing theme access scope', async () => {
+    vi.mocked(adminRequestDoc).mockRejectedValue(
+      errorHandler('Admin')(themeAccessDeniedError('`write_themes` access scope.'), 'request-id'),
+    )
+
+    await expect(
+      bulkUploadThemeAssets(123, [{key: 'snippets/product-variant-picker.liquid', value: 'content'}], session),
+    ).rejects.toThrow('The authenticated account or access token is missing `write_themes` access scope.')
+  })
 })
 
 describe('parseThemeFileContent', () => {

packages/cli-kit/src/public/node/themes/api.ts

@@ -27,13 +27,14 @@ import {GetThemes} from '../../../cli/api/graphql/admin/generated/get_themes.js'
 import {GetTheme} from '../../../cli/api/graphql/admin/generated/get_theme.js'
 import {FindDevelopmentThemeByName} from '../../../cli/api/graphql/admin/generated/find_development_theme_by_name.js'
 import {OnlineStorePasswordProtection} from '../../../cli/api/graphql/admin/generated/online_store_password_protection.js'
+import {extractGraphQLErrors} from '../../../private/node/api/graphql.js'
 import {RequestModeInput} from '../http.js'
 import {adminRequestDoc, type AdminRequestOptions} from '../api/admin.js'
 import {AdminSession} from '../session.js'
 import {AbortError} from '../error.js'
 import {outputDebug} from '../output.js'
 import {recordTiming, recordEvent, recordError} from '../analytics.js'
-import {ClientError, type Variables} from 'graphql-request'
+import {type Variables} from 'graphql-request'
 import type {InlineToken, TokenItem} from '../ui.js'
 
 export type ThemeParams = Partial<Pick<Theme, 'name' | 'role' | 'processing' | 'src'>>
@@ -242,7 +243,7 @@ export async function deleteThemeAssets(id: number, filenames: Key[], session: A
   for (let i = 0; i < filenames.length; i += batchSize) {
     const batch = filenames.slice(i, i + batchSize)
     // eslint-disable-next-line no-await-in-loop
-    const {themeFilesDelete} = await adminRequestDoc({
+    const {themeFilesDelete} = await requestThemeAdminDoc({
       query: ThemeFilesDelete,
       session,
       variables: {
@@ -332,7 +333,7 @@ async function uploadFiles(
   files: {filename: string; body: {type: OnlineStoreThemeFileBodyInputType; value: string}}[],
   session: AdminSession,
 ): Promise<ThemeFilesUpsertMutation> {
-  return adminRequestDoc({
+  return requestThemeAdminDoc({
     query: ThemeFilesUpsert,
     session,
     variables: {themeId: themeGid(themeId), files},
@@ -420,7 +421,7 @@ export async function themeUpdate(id: number, params: ThemeParams, session: Admi
   }
   recordEvent('theme-api:update-theme')
 
-  const {themeUpdate} = await adminRequestDoc({
+  const {themeUpdate} = await requestThemeAdminDoc({
     query: ThemeUpdate,
     session,
     variables: {id: composeThemeGid(id), input},
@@ -451,7 +452,7 @@ export async function themeUpdate(id: number, params: ThemeParams, session: Admi
 
 export async function themePublish(id: number, session: AdminSession): Promise<Theme | undefined> {
   recordEvent('theme-api:publish-theme')
-  const {themePublish} = await adminRequestDoc({
+  const {themePublish} = await requestThemeAdminDoc({
     query: ThemePublish,
     session,
     variables: {id: composeThemeGid(id)},
@@ -482,7 +483,7 @@ export async function themePublish(id: number, session: AdminSession): Promise<T
 
 export async function themeDelete(id: number, session: AdminSession): Promise<boolean | undefined> {
   recordEvent('theme-api:delete-theme')
-  const {themeDelete} = await adminRequestDoc({
+  const {themeDelete} = await requestThemeAdminDoc({
     query: ThemeDelete,
     session,
     variables: {id: composeThemeGid(id)},
@@ -521,7 +522,7 @@ export async function themeDuplicate(
   let requestId: string | undefined
   recordEvent('theme-api:duplicate-theme')
 
-  const {themeDuplicate} = await adminRequestDoc({
+  const {themeDuplicate} = await requestThemeAdminDoc({
     query: ThemeDuplicate,
     session,
     variables: {id: composeThemeGid(id), name},
@@ -637,8 +638,6 @@ async function requestThemeAdminDoc<TResult, TVariables extends Variables>(
 }
 
 function abortIfMissingThemeAccessScope(error: unknown): void {
-  if (!(error instanceof ClientError)) return
-
   const requiredAccess = getThemeAccessRequirementForAccessDeniedError(error)
   if (!requiredAccess) return
 
@@ -663,9 +662,18 @@ function abortIfMissingThemeAccessScope(error: unknown): void {
     [
       'For',
       {command: 'theme push'},
-      'and',
+      {char: ','},
       {command: 'theme dev'},
       {char: ','},
+      {command: 'theme publish'},
+      {char: ','},
+      {command: 'theme delete'},
+      {char: ','},
+      {command: 'theme rename'},
+      {char: ','},
+      'and',
+      {command: 'theme duplicate'},
+      {char: ','},
       'add both the',
       {command: 'read_themes'},
       'and',
@@ -687,11 +695,10 @@ function abortIfMissingThemeAccessScope(error: unknown): void {
   )
 }
 
-function getThemeAccessRequirementForAccessDeniedError(error: ClientError): string | undefined {
-  const graphQLErrors = error.response.errors
-  if (!Array.isArray(graphQLErrors)) return undefined
-
-  const accessDeniedError = graphQLErrors.find((graphQLError) => graphQLError.extensions?.code === 'ACCESS_DENIED')
+function getThemeAccessRequirementForAccessDeniedError(error: unknown): string | undefined {
+  const accessDeniedError = extractGraphQLErrors(error).find(
+    (graphQLError) => graphQLError.extensions?.code === 'ACCESS_DENIED',
+  )
   if (!accessDeniedError) return undefined
 
   const requiredAccess = accessDeniedError.extensions?.requiredAccess