diff --git a/.changeset/theme-mutations-missing-scope.md b/.changeset/theme-mutations-missing-scope.md new file mode 100644 index 00000000000..fa65debeb95 --- /dev/null +++ b/.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. diff --git a/packages/cli-kit/src/private/node/api/graphql.test.ts b/packages/cli-kit/src/private/node/api/graphql.test.ts index b886b08eba9..b4b692c8279 100644 --- a/packages/cli-kit/src/private/node/api/graphql.test.ts +++ b/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([]) + }) +}) diff --git a/packages/cli-kit/src/private/node/api/graphql.ts b/packages/cli-kit/src/private/node/api/graphql.ts index 9d99517f271..f01ebd9ea08 100644 --- a/packages/cli-kit/src/private/node/api/graphql.ts +++ b/packages/cli-kit/src/private/node/api/graphql.ts @@ -159,3 +159,9 @@ ${outputToken.json(error.response.errors)} } } } + +export function extractGraphQLErrors(error: unknown): NonNullable { + 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 [] +} diff --git a/packages/cli-kit/src/public/node/themes/api.test.ts b/packages/cli-kit/src/public/node/themes/api.test.ts index 2fd5d1a860a..abf10d832f0 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/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', () => { diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index efefab6be9d..ddbbdbc831f 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/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> @@ -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 { - 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 { 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 { 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( } 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