Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use app context in app versions list #4650

Merged
merged 3 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 7 additions & 8 deletions packages/app/src/cli/commands/app/versions/list.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import {appFlags} from '../../../flags.js'
import versionList from '../../../services/versions-list.js'
import {loadLocalExtensionsSpecifications} from '../../../models/extensions/load-specifications.js'
import {AppInterface} from '../../../models/app/app.js'
import {loadApp} from '../../../models/app/loader.js'
import {showApiKeyDeprecationWarning} from '../../../prompts/deprecation-warnings.js'
import AppCommand, {AppCommandOutput} from '../../../utilities/app-command.js'
import {linkedAppContext} from '../../../services/app-context.js'
import {globalFlags} from '@shopify/cli-kit/node/cli'
import {Args, Flags} from '@oclif/core'

Expand Down Expand Up @@ -49,17 +47,18 @@ export default class VersionsList extends AppCommand {
await showApiKeyDeprecationWarning()
}
const apiKey = flags['client-id'] || flags['api-key']
const specifications = await loadLocalExtensionsSpecifications()
const app: AppInterface = await loadApp({
specifications,

const {app, remoteApp, developerPlatformClient} = await linkedAppContext({
directory: flags.path,
clientId: apiKey,
forceRelink: false,
userProvidedConfigName: flags.config,
})

await versionList({
app,
apiKey,
reset: false,
remoteApp,
developerPlatformClient,
json: flags.json,
})

Expand Down
23 changes: 0 additions & 23 deletions packages/app/src/cli/services/context.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
ensureDeployContext,
ensureThemeExtensionDevContext,
DeployContextOptions,
ensureVersionsListContext,
} from './context.js'
import {createExtension} from './dev/create-extension.js'
import {CachedAppInfo, clearCachedAppInfo, getCachedAppInfo, setCachedAppInfo} from './local-storage.js'
Expand Down Expand Up @@ -1514,28 +1513,6 @@ describe('ensureThemeExtensionDevContext', () => {
})
})

describe('ensureVersionsListContext', () => {
test('returns the developer platform client and the app', async () => {
// Given
const app = testApp()
const developerPlatformClient = buildDeveloperPlatformClient()

// When
const got = await ensureVersionsListContext({
app,
apiKey: APP2.apiKey,
reset: false,
developerPlatformClient,
})

// Then
expect(got).toEqual({
remoteApp: APP2,
developerPlatformClient,
})
})
})

async function mockApp(directory: string, app?: Partial<AppInterface>) {
const versionSchema = await buildVersionedAppSchema()
const localApp = testApp(app)
Expand Down
36 changes: 0 additions & 36 deletions packages/app/src/cli/services/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,42 +463,6 @@ function includeConfigOnDeployPrompt(configPath: string): Promise<boolean> {
})
}

interface VersionListContextOptions {
app: AppInterface
apiKey?: string
reset: false
developerPlatformClient?: DeveloperPlatformClient
}

interface VersionsListContextOutput {
developerPlatformClient: DeveloperPlatformClient
remoteApp: OrganizationApp
}

/**
* Make sure there is a valid context to execute `versions list`
*
* If there is an API key via flag, configuration or env file, we check if it is valid. Otherwise, throw an error.
* If there is no API key (or is invalid), show prompts to select an org and app.
*
* @param options - Current dev context options
* @returns The Developer Platform client and the app
*/
export async function ensureVersionsListContext(
options: VersionListContextOptions,
): Promise<VersionsListContextOutput> {
let developerPlatformClient =
options.developerPlatformClient ?? selectDeveloperPlatformClient({configuration: options.app.configuration})
const [remoteApp] = await fetchAppAndIdentifiers(options, developerPlatformClient)
developerPlatformClient = remoteApp.developerPlatformClient ?? developerPlatformClient

await logMetadataForLoadedContext({organizationId: remoteApp.organizationId, apiKey: remoteApp.apiKey})
return {
developerPlatformClient,
remoteApp,
}
}

export async function fetchOrCreateOrganizationApp(
options: AppCreationDefaultOptions,
directory?: string,
Expand Down
71 changes: 19 additions & 52 deletions packages/app/src/cli/services/versions-list.test.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import versionList from './versions-list.js'
import {ensureVersionsListContext, renderCurrentlyUsedConfigInfo} from './context.js'
import {testApp, testDeveloperPlatformClient} from '../models/app/app.test-data.js'
import {renderCurrentlyUsedConfigInfo} from './context.js'
import {testAppLinked, testDeveloperPlatformClient, testOrganizationApp} from '../models/app/app.test-data.js'
import {Organization} from '../models/organization.js'
import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js'
import {AppVersionsQuerySchema} from '../api/graphql/get_versions_list.js'
import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest'
import {afterEach, describe, expect, test, vi} from 'vitest'
import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output'

vi.mock('../models/app/identifiers.js')
Expand All @@ -19,57 +19,25 @@ const ORG1: Organization = {
businessName: 'name of org 1',
}

const remoteApp = testOrganizationApp({organizationId: ORG1.id, apiKey: 'api-key', title: 'app-title', id: 'app-id'})

function buildDeveloperPlatformClient(): DeveloperPlatformClient {
return testDeveloperPlatformClient({
orgFromId: (_orgId: string) => Promise.resolve(ORG1),
})
}

describe('versions-list', () => {
beforeEach(() => {
vi.mocked(ensureVersionsListContext).mockResolvedValue({
developerPlatformClient: buildDeveloperPlatformClient(),
remoteApp: {
id: 'app-id',
apiKey: 'app-api-key',
title: 'app-title',
organizationId: ORG1.id,
apiSecretKeys: [],
grantedScopes: [],
flags: [],
},
})
// vi.mocked(fetchOrgFromId).mockResolvedValue(ORG1)
})

test('ensures there is a valid context to execute `versions list`', async () => {
// Given
const app = testApp({})

// When
await versionList({
app,
reset: false,
json: false,
})

// Then
expect(ensureVersionsListContext).toHaveBeenCalledWith({
app,
reset: false,
json: false,
})
})

test('show a message when there are no app versions', async () => {
// Given
const app = testApp({})
const app = testAppLinked({})
const outputMock = mockAndCaptureOutput()

// When
await versionList({
app,
reset: false,
remoteApp,
developerPlatformClient: buildDeveloperPlatformClient(),
json: false,
})

Expand All @@ -79,48 +47,49 @@ describe('versions-list', () => {

test('show currently used config info', async () => {
// Given
const app = testApp({})
const app = testAppLinked({})

// When
await versionList({
app,
reset: false,
remoteApp,
developerPlatformClient: buildDeveloperPlatformClient(),
json: false,
})

// Then
expect(renderCurrentlyUsedConfigInfo).toHaveBeenCalledWith({
org: 'name of org 1',
appName: 'app-title',
configFile: undefined,
configFile: 'shopify.app.toml',
})
})

test('throw error when there is no app', async () => {
// Given
const app = testApp({})
const app = testAppLinked({})
const developerPlatformClient: DeveloperPlatformClient = testDeveloperPlatformClient({
appVersions: (_appId) => Promise.resolve({app: null}),
})

// When
const output = versionList({
app,
reset: false,
remoteApp,
json: false,
developerPlatformClient,
})

// Then
await expect(output).rejects.toThrow('Invalid API Key: app-api-key')
await expect(output).rejects.toThrow('Invalid API Key: api-key')
})

// asserting the exact format of the table is hard to do consistently across different environments
const terminalWidth = process.stdout.columns

test.skipIf(terminalWidth !== undefined)('render table when there are app versions', async () => {
// Given
const app = testApp({})
const app = testAppLinked({})
const mockOutput = mockAndCaptureOutput()
const appVersionsResult: AppVersionsQuerySchema = {
app: {
Expand Down Expand Up @@ -162,8 +131,7 @@ describe('versions-list', () => {
// When
await versionList({
app,
apiKey: 'apiKey',
reset: false,
remoteApp,
json: false,
developerPlatformClient,
})
Expand All @@ -181,7 +149,7 @@ View all 31 app versions in the Test Dashboard ( https://test.shopify.com/org-id

test('render json when there are app versions', async () => {
// Given
const app = testApp({})
const app = testAppLinked({})

const mockOutput = mockAndCaptureOutput()
const appVersionsResult: AppVersionsQuerySchema = {
Expand Down Expand Up @@ -217,8 +185,7 @@ View all 31 app versions in the Test Dashboard ( https://test.shopify.com/org-id
// When
await versionList({
app,
apiKey: 'apiKey',
reset: false,
remoteApp,
json: true,
developerPlatformClient,
})
Expand Down
27 changes: 11 additions & 16 deletions packages/app/src/cli/services/versions-list.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {ensureVersionsListContext, renderCurrentlyUsedConfigInfo} from './context.js'
import {renderCurrentlyUsedConfigInfo} from './context.js'
import {fetchOrgFromId} from './dev/fetch.js'
import {AppVersionsQuerySchema} from '../api/graphql/get_versions_list.js'
import {AppInterface, isCurrentAppSchema} from '../models/app/app.js'
import {AppLinkedInterface} from '../models/app/app.js'
import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js'
import {OrganizationApp} from '../models/organization.js'
import colors from '@shopify/cli-kit/node/colors'
Expand Down Expand Up @@ -82,31 +82,26 @@ async function fetchAppVersions(
}

interface VersionListOptions {
app: AppInterface
apiKey?: string
reset: false
app: AppLinkedInterface
remoteApp: OrganizationApp
developerPlatformClient: DeveloperPlatformClient
json: boolean
developerPlatformClient?: DeveloperPlatformClient
}

export default async function versionList(options: VersionListOptions) {
const result = await ensureVersionsListContext(options)
const developerPlatformClient = options.developerPlatformClient ?? result.developerPlatformClient
const {remoteApp, developerPlatformClient} = options

const {organizationId, title} = result.remoteApp

const {appVersions, totalResults} = await fetchAppVersions(developerPlatformClient, result.remoteApp, options.json)

const {businessName: org} = await fetchOrgFromId(organizationId, developerPlatformClient)
const {appVersions, totalResults} = await fetchAppVersions(developerPlatformClient, remoteApp, options.json)

const {businessName: org} = await fetchOrgFromId(remoteApp.organizationId, developerPlatformClient)
if (options.json) {
return outputInfo(JSON.stringify(appVersions, null, 2))
}

renderCurrentlyUsedConfigInfo({
org,
appName: title,
configFile: isCurrentAppSchema(options.app.configuration) ? basename(options.app.configuration.path) : undefined,
appName: remoteApp.title,
configFile: basename(options.app.configuration.path),
gonzaloriestra marked this conversation as resolved.
Show resolved Hide resolved
})

if (appVersions.length === 0) {
Expand All @@ -127,7 +122,7 @@ export default async function versionList(options: VersionListOptions) {

const link = outputToken.link(
developerPlatformClient.webUiName,
[await developerPlatformClient.appDeepLink(result.remoteApp), 'versions'].join('/'),
[await developerPlatformClient.appDeepLink(remoteApp), 'versions'].join('/'),
)

outputInfo(outputContent`\nView all ${String(totalResults)} app versions in the ${link}`)
Expand Down