Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
- Updates the CLI banner message to include better next steps when an app could not be found

Co-authored-by: Kenneth Ye <kenneth.ye@shopify.com>
  • Loading branch information
alexanderMontague and Kenneth-Ye committed Jan 14, 2025
1 parent 6391f7b commit 5cf2d8a
Show file tree
Hide file tree
Showing 3 changed files with 130 additions and 29 deletions.
109 changes: 86 additions & 23 deletions packages/app/src/cli/services/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {fetchOrganizations, fetchOrgFromId} from './dev/fetch.js'
import {selectOrCreateApp} from './dev/select-app.js'
import {selectStore} from './dev/select-store.js'
import {ensureDeploymentIdsPresence} from './context/identifiers.js'
import {ensureDeployContext, ensureThemeExtensionDevContext} from './context.js'
import {appFromIdentifiers, ensureDeployContext, ensureThemeExtensionDevContext} from './context.js'
import {createExtension} from './dev/create-extension.js'
import {CachedAppInfo} from './local-storage.js'
import link from './app/config/link.js'
Expand Down Expand Up @@ -38,6 +38,8 @@ import {afterEach, beforeAll, beforeEach, describe, expect, test, vi} from 'vite
import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'
import {getPackageManager} from '@shopify/cli-kit/node/node-package-manager'
import {renderConfirmationPrompt, renderInfo, renderTasks, renderWarning, Task} from '@shopify/cli-kit/node/ui'
import {AbortError} from '@shopify/cli-kit/node/error'

Check failure on line 41 in packages/app/src/cli/services/context.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/context.test.ts#L41

[unused-imports/no-unused-imports] 'AbortError' is defined but never used.
import {isServiceAccount, isUserAccount} from './context/partner-account-info.js'

Check failure on line 42 in packages/app/src/cli/services/context.test.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/context.test.ts#L42

[import/order] `./context/partner-account-info.js` import should occur before import of `../models/organization.js`

const APP1: OrganizationApp = testOrganizationApp({
id: '1',
Expand Down Expand Up @@ -221,11 +223,11 @@ describe('ensureDeployContext', () => {
},
},
'\n',
'You can pass ',
'You can pass',
{
command: '--reset',
},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
],
headline: 'Using shopify.app.toml for default values:',
})
Expand Down Expand Up @@ -270,11 +272,11 @@ describe('ensureDeployContext', () => {
},
},
'\n',
'You can pass ',
'You can pass',
{
command: '--reset',
},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
],
headline: 'Using shopify.app.toml for default values:',
})
Expand Down Expand Up @@ -320,11 +322,11 @@ describe('ensureDeployContext', () => {
},
},
'\n',
'You can pass ',
'You can pass',
{
command: '--reset',
},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
],
headline: 'Using shopify.app.toml for default values:',
})
Expand Down Expand Up @@ -370,11 +372,11 @@ describe('ensureDeployContext', () => {
},
},
'\n',
'You can pass ',
'You can pass',
{
command: '--reset',
},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
],
headline: 'Using shopify.app.toml for default values:',
})
Expand Down Expand Up @@ -426,11 +428,11 @@ describe('ensureDeployContext', () => {
},
},
'\n',
'You can pass ',
'You can pass',
{
command: '--reset',
},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
],
headline: 'Using shopify.app.toml for default values:',
})
Expand Down Expand Up @@ -470,11 +472,11 @@ describe('ensureDeployContext', () => {
},
},
'\n',
'You can pass ',
'You can pass',
{
command: '--reset',
},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
],
headline: 'Using shopify.app.toml for default values:',
})
Expand Down Expand Up @@ -511,11 +513,11 @@ describe('ensureDeployContext', () => {
},
},
'\n',
'You can pass ',
'You can pass',
{
command: '--reset',
},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
],
headline: 'Using shopify.app.toml for default values:',
})
Expand Down Expand Up @@ -689,11 +691,72 @@ describe('ensureThemeExtensionDevContext', () => {
})
})

async function mockApp(directory: string, app?: Partial<AppInterface>) {
const versionSchema = await buildVersionedAppSchema()
const localApp = testApp(app)
localApp.configSchema = versionSchema.schema
localApp.specifications = versionSchema.configSpecifications
localApp.directory = directory
return localApp
}
describe('appFromIdentifiers', () => {
test('renders the org name when an app cannot be found and the account is a service account ', async () => {
vi.mocked(isServiceAccount).mockReturnValue(true)

await expect(
appFromIdentifiers({
apiKey: 'apiKey-12345',
developerPlatformClient: testDeveloperPlatformClient({
appFromIdentifiers: () => Promise.resolve(undefined),
accountInfo: () =>
Promise.resolve({
type: 'ServiceAccount',
orgName: 'My Test Org',
}),
}),
organizationId: 'orgId',
}),
).rejects.toThrowError(
expect.objectContaining({
message: 'No app with client ID apiKey-12345 found',
tryMessage: renderTryMessage(true, 'My Test Org'),
}),
)
})

test('renders the user email when an app cannot be found and the account is a user account ', async () => {
vi.mocked(isUserAccount).mockReturnValue(true)

await expect(
appFromIdentifiers({
apiKey: 'apiKey-12345',
developerPlatformClient: testDeveloperPlatformClient({
appFromIdentifiers: () => Promise.resolve(undefined),
accountInfo: () =>
Promise.resolve({
type: 'UserAccount',
email: 'user@example.com',
}),
}),
organizationId: 'orgId',
}),
).rejects.toThrowError(
expect.objectContaining({
message: 'No app with client ID apiKey-12345 found',
tryMessage: renderTryMessage(false, 'user@example.com'),
}),
)
})
})

const renderTryMessage = (isOrg: boolean, identifier: string) => [
{
list: {
title: 'Next steps:',
items: [
'Check that your account has permission to develop apps for this organization or contact the owner of the organization to grant you permission',
[
'Run',
{command: 'shopify auth logout'},
'to log into a different',
isOrg ? 'organization' : 'account',
'than',
{bold: identifier},
],
['Pass', {command: '--reset'}, 'to your command to create a new app'],
],
},
},
]
46 changes: 42 additions & 4 deletions packages/app/src/cli/services/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ import {Token, TokenItem, renderConfirmationPrompt, renderInfo, renderWarning} f
import {AbortError} from '@shopify/cli-kit/node/error'
import {outputContent} from '@shopify/cli-kit/node/output'
import {basename, sniffForJson} from '@shopify/cli-kit/node/path'
import {getCurrentAccountInfo} from '../api/graphql/current_account_info.js'

Check failure on line 33 in packages/app/src/cli/services/context.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/context.ts#L33

[import/order] `../api/graphql/current_account_info.js` import should occur before import of `@shopify/cli-kit/common/string`

Check failure on line 33 in packages/app/src/cli/services/context.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/context.ts#L33

[unused-imports/no-unused-imports] 'getCurrentAccountInfo' is defined but never used.

Check failure on line 33 in packages/app/src/cli/services/context.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/context.ts#L33

[@typescript-eslint/no-unused-vars] 'getCurrentAccountInfo' is defined but never used. Allowed unused vars must match /^_/u.
import {isServiceAccount, isUserAccount} from './context/partner-account-info.js'

Check failure on line 34 in packages/app/src/cli/services/context.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/context.ts#L34

[import/order] `./context/partner-account-info.js` import should occur before import of `../prompts/dev.js`

export const InvalidApiKeyErrorMessage = (apiKey: string) => {
return {
Expand All @@ -38,10 +40,30 @@ export const InvalidApiKeyErrorMessage = (apiKey: string) => {
}
}

export const resetHelpMessage: Token[] = [
'You can pass ',
export const resetHelpMessage = [
'You can pass',
{command: '--reset'},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
]

const appNotFoundHelpMessage = (accountIdentifier: string, isOrg: boolean = false) => [

Check failure on line 49 in packages/app/src/cli/services/context.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/context.ts#L49

[@typescript-eslint/no-inferrable-types] Type boolean trivially inferred from a boolean literal, remove type annotation.
{
list: {
title: 'Next steps:',
items: [
'Check that your account has permission to develop apps for this organization or contact the owner of the organization to grant you permission',
[
'Run',
{command: 'shopify auth logout'},
'to log into a different',
isOrg ? 'organization' : 'account',
'than',
{bold: accountIdentifier},
],
['Pass', {command: '--reset'}, 'to your command to create a new app'],
],
},
},
]

interface AppFromIdOptions {
Expand All @@ -65,7 +87,23 @@ export const appFromIdentifiers = async (options: AppFromIdOptions): Promise<Org
apiKey: options.apiKey,
organizationId,
})
if (!app) throw new AbortError([`Couldn't find the app with Client ID`, {command: options.apiKey}], resetHelpMessage)
if (!app) {
const accountInfo = await developerPlatformClient.accountInfo()
let identifier = 'Unknown account'
let isOrg = false

if (isServiceAccount(accountInfo)) {
identifier = accountInfo.orgName
isOrg = true
} else if (isUserAccount(accountInfo)) {
identifier = accountInfo.email
}

throw new AbortError(
[`No app with client ID`, {command: options.apiKey}, 'found'],
appNotFoundHelpMessage(identifier, isOrg),
)
}
return app
}

Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/logs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,11 @@ describe('logs', () => {
},
},
'\n',
'You can pass ',
'You can pass',
{
command: '--reset',
},
' to your command to reset your app configuration.',
'to your command to reset your app configuration.',
],
headline: 'Using shopify.app.toml for default values:',
})
Expand Down

0 comments on commit 5cf2d8a

Please sign in to comment.