Skip to content

Commit

Permalink
Record business_platform_id not partner_id in remaining cases
Browse files Browse the repository at this point in the history
  • Loading branch information
amcaplan committed Dec 22, 2024
1 parent e746586 commit 4861084
Show file tree
Hide file tree
Showing 17 changed files with 87 additions and 46 deletions.
4 changes: 2 additions & 2 deletions packages/app/src/cli/commands/app/env/pull.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,14 +30,14 @@ export default class EnvPull extends AppCommand {
public async run(): Promise<AppCommandOutput> {
const {flags} = await this.parse(EnvPull)

const {app, remoteApp} = await linkedAppContext({
const {app, remoteApp, organization} = await linkedAppContext({
directory: flags.path,
clientId: flags['client-id'],
forceRelink: flags.reset,
userProvidedConfigName: flags.config,
})
const envFile = joinPath(app.directory, flags['env-file'] ?? getDotEnvFileName(app.configuration.path))
outputInfo(await pullEnv({app, remoteApp, envFile}))
outputInfo(await pullEnv({app, remoteApp, organization, envFile}))
return {app}
}
}
4 changes: 2 additions & 2 deletions packages/app/src/cli/commands/app/env/show.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ export default class EnvShow extends AppCommand {

public async run(): Promise<AppCommandOutput> {
const {flags} = await this.parse(EnvShow)
const {app, remoteApp} = await linkedAppContext({
const {app, remoteApp, organization} = await linkedAppContext({
directory: flags.path,
clientId: flags['client-id'],
forceRelink: flags.reset,
userProvidedConfigName: flags.config,
})
outputInfo(await showEnv(app, remoteApp))
outputInfo(await showEnv(app, remoteApp, organization))
return {app}
}
}
4 changes: 2 additions & 2 deletions packages/app/src/cli/commands/app/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@ export default class AppInfo extends AppCommand {
public async run(): Promise<AppCommandOutput> {
const {flags} = await this.parse(AppInfo)

const {app, remoteApp, developerPlatformClient} = await linkedAppContext({
const {app, remoteApp, organization, developerPlatformClient} = await linkedAppContext({
directory: flags.path,
clientId: flags['client-id'],
forceRelink: flags.reset,
userProvidedConfigName: flags.config,
unsafeReportMode: true,
})
outputInfo(
await info(app, remoteApp, {
await info(app, remoteApp, organization, {
format: (flags.json ? 'json' : 'text') as Format,
webEnv: flags['web-env'],
configName: flags.config,
Expand Down
8 changes: 7 additions & 1 deletion packages/app/src/cli/models/app/app.test-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1318,6 +1318,7 @@ export function testDeveloperPlatformClient(stubs: Partial<DeveloperPlatformClie
requiresOrganization: false,
supportsAtomicDeployments: false,
supportsDevSessions: stubs.supportsDevSessions ?? false,
organizationSource: OrganizationSource.BusinessPlatform,
session: () => Promise.resolve(testPartnersUserSession),
refreshToken: () => Promise.resolve(testPartnersUserSession.token),
accountInfo: () => Promise.resolve(testPartnersUserSession.accountInfo),
Expand Down Expand Up @@ -1378,7 +1379,12 @@ export function testDeveloperPlatformClient(stubs: Partial<DeveloperPlatformClie
retVal[
key as keyof Omit<
DeveloperPlatformClient,
'requiresOrganization' | 'supportsAtomicDeployments' | 'clientName' | 'webUiName' | 'supportsDevSessions'
| 'requiresOrganization'
| 'supportsAtomicDeployments'
| 'clientName'
| 'webUiName'
| 'supportsDevSessions'
| 'organizationSource'
>
] = vi.fn().mockImplementation(value)
}
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/app/config/link.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ export default async function link(options: LinkOptions, shouldRenderSuccess = t
format: localAppOptions.configFormat,
})

await logMetadataForLoadedContext(remoteApp)
await logMetadataForLoadedContext(remoteApp, developerPlatformClient.organizationSource)

// Finally, merge the remote app's configuration with the local app's configuration, and write it to the filesystem
const mergedAppConfiguration = await overwriteLocalConfigFileWithRemoteAppConfiguration({
Expand Down
7 changes: 6 additions & 1 deletion packages/app/src/cli/services/app/config/use.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {getAppConfigurationFileName, loadAppConfiguration} from '../../../models
import {clearCurrentConfigFile, setCachedAppInfo} from '../../local-storage.js'
import {selectConfigFile} from '../../../prompts/config.js'
import {appFromIdentifiers, logMetadataForLoadedContext} from '../../context.js'
import {OrganizationSource} from '../../../models/organization.js'
import {describe, expect, test, vi} from 'vitest'
import {inTemporaryDirectory, writeFileSync} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'
Expand Down Expand Up @@ -311,7 +312,11 @@ describe('use', () => {
await use(options)

// Then
expect(logMetadataForLoadedContext).toHaveBeenNthCalledWith(1, {apiKey: REMOTE_APP.apiKey, organizationId: '0'})
expect(logMetadataForLoadedContext).toHaveBeenNthCalledWith(
1,
{apiKey: REMOTE_APP.apiKey, organizationId: '0'},
OrganizationSource.Partners,
)
})
})
})
Expand Down
12 changes: 8 additions & 4 deletions packages/app/src/cli/services/app/config/use.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {selectConfigFile} from '../../../prompts/config.js'
import {AppConfiguration, CurrentAppConfiguration, isCurrentAppSchema} from '../../../models/app/app.js'
import {logMetadataForLoadedContext} from '../../context.js'
import {DeveloperPlatformClient} from '../../../utilities/developer-platform-client.js'
import {OrganizationSource} from '../../../models/organization.js'
import {AbortError} from '@shopify/cli-kit/node/error'
import {fileExists} from '@shopify/cli-kit/node/fs'
import {joinPath} from '@shopify/cli-kit/node/path'
Expand Down Expand Up @@ -102,8 +103,11 @@ async function getConfigFileName(directory: string, configName?: string): Promis
}

async function logMetadata(configuration: CurrentAppConfiguration) {
await logMetadataForLoadedContext({
organizationId: configuration.organization_id || '0',
apiKey: configuration.client_id,
})
await logMetadataForLoadedContext(
{
apiKey: configuration.client_id,
organizationId: configuration.organization_id || '0',

Check warning on line 109 in packages/app/src/cli/services/app/config/use.ts

View workflow job for this annotation

GitHub Actions / ESLint Report Analysis

packages/app/src/cli/services/app/config/use.ts#L109

[@typescript-eslint/prefer-nullish-coalescing] Prefer using nullish coalescing operator (`??`) instead of a logical or (`||`), as it is a safer operator.
},
configuration.organization_id ? OrganizationSource.BusinessPlatform : OrganizationSource.Partners,
)
}
14 changes: 10 additions & 4 deletions packages/app/src/cli/services/app/env/pull.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,18 @@
import {pullEnv} from './pull.js'
import {AppInterface, AppLinkedInterface} from '../../../models/app/app.js'
import {testApp, testOrganizationApp} from '../../../models/app/app.test-data.js'
import {OrganizationApp} from '../../../models/organization.js'
import {Organization, OrganizationApp, OrganizationSource} from '../../../models/organization.js'
import {describe, expect, vi, beforeEach, test} from 'vitest'
import * as file from '@shopify/cli-kit/node/fs'
import {resolvePath, joinPath} from '@shopify/cli-kit/node/path'
import {unstyled, stringifyMessage} from '@shopify/cli-kit/node/output'

const ORG1: Organization = {
id: '1',
businessName: 'My Org',
source: OrganizationSource.BusinessPlatform,
}

describe('env pull', () => {
let app: AppLinkedInterface
let remoteApp: OrganizationApp
Expand All @@ -23,7 +29,7 @@ describe('env pull', () => {

// When
const filePath = resolvePath(tmpDir, '.env')
const result = await pullEnv({app, remoteApp, envFile: filePath})
const result = await pullEnv({app, remoteApp, organization: ORG1, envFile: filePath})

// Then
expect(file.writeFile).toHaveBeenCalledWith(
Expand All @@ -49,7 +55,7 @@ describe('env pull', () => {
vi.spyOn(file, 'writeFile')

// When
const result = await pullEnv({app, remoteApp, envFile: filePath})
const result = await pullEnv({app, remoteApp, organization: ORG1, envFile: filePath})

// Then
expect(file.writeFile).toHaveBeenCalledWith(
Expand Down Expand Up @@ -83,7 +89,7 @@ describe('env pull', () => {
vi.spyOn(file, 'writeFile')

// When
const result = await pullEnv({app, remoteApp, envFile: filePath})
const result = await pullEnv({app, remoteApp, organization: ORG1, envFile: filePath})

// Then
expect(file.writeFile).not.toHaveBeenCalled()
Expand Down
7 changes: 4 additions & 3 deletions packages/app/src/cli/services/app/env/pull.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {AppLinkedInterface, getAppScopes} from '../../../models/app/app.js'

import {logMetadataForLoadedContext} from '../../context.js'

import {OrganizationApp} from '../../../models/organization.js'
import {Organization, OrganizationApp} from '../../../models/organization.js'
import {patchEnvFile} from '@shopify/cli-kit/node/dot-env'
import {diffLines} from 'diff'
import {fileExists, readFile, writeFile} from '@shopify/cli-kit/node/fs'
Expand All @@ -11,11 +11,12 @@ import {OutputMessage, outputContent, outputToken} from '@shopify/cli-kit/node/o
interface PullEnvOptions {
app: AppLinkedInterface
remoteApp: OrganizationApp
organization: Organization
envFile: string
}

export async function pullEnv({app, remoteApp, envFile}: PullEnvOptions): Promise<OutputMessage> {
await logMetadataForLoadedContext(remoteApp)
export async function pullEnv({app, remoteApp, organization, envFile}: PullEnvOptions): Promise<OutputMessage> {
await logMetadataForLoadedContext(remoteApp, organization.source)

const updatedValues = {
SHOPIFY_API_KEY: remoteApp.apiKey,
Expand Down
3 changes: 1 addition & 2 deletions packages/app/src/cli/services/app/env/show.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,13 +27,12 @@ describe('env show', () => {
source: OrganizationSource.BusinessPlatform,
apps: {nodes: []},
}
const organizationApp = testOrganizationApp()

vi.mocked(fetchOrganizations).mockResolvedValue([organization])
vi.mocked(selectOrganizationPrompt).mockResolvedValue(organization)

// When
const result = await showEnv(app, remoteApp)
const result = await showEnv(app, remoteApp, organization)

// Then
expect(file.writeFile).not.toHaveBeenCalled()
Expand Down
19 changes: 14 additions & 5 deletions packages/app/src/cli/services/app/env/show.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,25 @@
import {AppInterface, getAppScopes} from '../../../models/app/app.js'
import {OrganizationApp} from '../../../models/organization.js'
import {Organization, OrganizationApp} from '../../../models/organization.js'
import {logMetadataForLoadedContext} from '../../context.js'
import {OutputMessage, outputContent, outputToken} from '@shopify/cli-kit/node/output'

type Format = 'json' | 'text'

export async function showEnv(app: AppInterface, remoteApp: OrganizationApp): Promise<OutputMessage> {
return outputEnv(app, remoteApp, 'text')
export async function showEnv(
app: AppInterface,
remoteApp: OrganizationApp,
organization: Organization,
): Promise<OutputMessage> {
return outputEnv(app, remoteApp, organization, 'text')
}

export async function outputEnv(app: AppInterface, remoteApp: OrganizationApp, format: Format): Promise<OutputMessage> {
await logMetadataForLoadedContext(remoteApp)
export async function outputEnv(
app: AppInterface,
remoteApp: OrganizationApp,
organization: Organization,
format: Format,
): Promise<OutputMessage> {
await logMetadataForLoadedContext(remoteApp, organization.source)

if (format === 'json') {
return outputContent`${outputToken.json({
Expand Down
14 changes: 10 additions & 4 deletions packages/app/src/cli/services/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
AppLinkedInterface,
} from '../models/app/app.js'
import {Identifiers, updateAppIdentifiers, getAppIdentifiers} from '../models/app/identifiers.js'
import {Organization, OrganizationApp, OrganizationStore} from '../models/organization.js'
import {Organization, OrganizationApp, OrganizationSource, OrganizationStore} from '../models/organization.js'
import metadata from '../metadata.js'
import {getAppConfigurationFileName} from '../models/app/loader.js'
import {ExtensionInstance} from '../models/extensions/extension-instance.js'
Expand Down Expand Up @@ -247,7 +247,7 @@ export async function fetchOrCreateOrganizationApp(
})
remoteApp.developerPlatformClient = developerPlatformClient

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

return remoteApp
}
Expand Down Expand Up @@ -346,9 +346,15 @@ export function renderCurrentlyUsedConfigInfo({
})
}

export async function logMetadataForLoadedContext(app: {organizationId: string; apiKey: string}) {
export async function logMetadataForLoadedContext(
app: {apiKey: string; organizationId: string},
organizationSource: OrganizationSource,
) {
const orgIdKey = organizationSource === OrganizationSource.BusinessPlatform ? 'business_platform_id' : 'partner_id'
const organizationInfo = {[orgIdKey]: tryParseInt(app.organizationId)}

await metadata.addPublicMetadata(() => ({
partner_id: tryParseInt(app.organizationId),
...organizationInfo,
api_key: app.apiKey,
}))
}
Expand Down
14 changes: 7 additions & 7 deletions packages/app/src/cli/services/info.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ describe('info', () => {
vi.mocked(checkForNewVersion).mockResolvedValue(latestVersion)

// When
const result = stringifyMessage(await info(app, remoteApp, infoOptions()))
const result = stringifyMessage(await info(app, remoteApp, ORG1, infoOptions()))
// Then
expect(unstyled(result)).toMatch(`Shopify CLI ${CLI_KIT_VERSION}`)
})
Expand All @@ -101,7 +101,7 @@ describe('info', () => {
vi.mocked(checkForNewVersion).mockResolvedValue(undefined)

// When
const result = stringifyMessage(await info(app, remoteApp, infoOptions()))
const result = stringifyMessage(await info(app, remoteApp, ORG1, infoOptions()))
// Then
expect(unstyled(result)).toMatch(`Shopify CLI ${CLI_KIT_VERSION}`)
expect(unstyled(result)).not.toMatch('CLI reminder')
Expand All @@ -116,7 +116,7 @@ describe('info', () => {
vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1)

// When
const result = await info(app, remoteApp, {...infoOptions(), webEnv: true})
const result = await info(app, remoteApp, ORG1, {...infoOptions(), webEnv: true})

// Then
expect(unstyled(stringifyMessage(result))).toMatchInlineSnapshot(`
Expand All @@ -136,7 +136,7 @@ describe('info', () => {
vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1)

// When
const result = await info(app, remoteApp, {...infoOptions(), format: 'json', webEnv: true})
const result = await info(app, remoteApp, ORG1, {...infoOptions(), format: 'json', webEnv: true})

// Then
expect(unstyled(stringifyMessage(result))).toMatchInlineSnapshot(`
Expand Down Expand Up @@ -184,7 +184,7 @@ describe('info', () => {
vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1)

// When
const result = await info(app, remoteApp, infoOptions())
const result = await info(app, remoteApp, ORG1, infoOptions())

// Then
expect(result).toContain('Extensions with errors')
Expand Down Expand Up @@ -222,7 +222,7 @@ describe('info', () => {
vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1)

// When
const result = await info(app, remoteApp, infoOptions())
const result = await info(app, remoteApp, ORG1, infoOptions())

// Then
expect(result).toContain('📂 handle-for-extension-1')
Expand Down Expand Up @@ -253,7 +253,7 @@ describe('info', () => {
vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1)

// When
const result = await info(app, remoteApp, {format: 'json', webEnv: false, developerPlatformClient})
const result = await info(app, remoteApp, ORG1, {format: 'json', webEnv: false, developerPlatformClient})

// Then
expect(result).toBeInstanceOf(TokenizedString)
Expand Down
8 changes: 5 additions & 3 deletions packages/app/src/cli/services/info.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js
import {AppLinkedInterface, getAppScopes} from '../models/app/app.js'
import {configurationFileNames} from '../constants.js'
import {ExtensionInstance} from '../models/extensions/extension-instance.js'
import {OrganizationApp} from '../models/organization.js'
import {Organization, OrganizationApp} from '../models/organization.js'
import {platformAndArch} from '@shopify/cli-kit/node/os'
import {linesToColumns} from '@shopify/cli-kit/common/string'
import {basename, relativePath} from '@shopify/cli-kit/node/path'
Expand All @@ -27,10 +27,11 @@ interface Configurable {
export async function info(
app: AppLinkedInterface,
remoteApp: OrganizationApp,
organization: Organization,
options: InfoOptions,
): Promise<OutputMessage> {
if (options.webEnv) {
return infoWeb(app, remoteApp, options)
return infoWeb(app, remoteApp, organization, options)
} else {
return infoApp(app, remoteApp, options)
}
Expand All @@ -39,9 +40,10 @@ export async function info(
async function infoWeb(
app: AppLinkedInterface,
remoteApp: OrganizationApp,
organization: Organization,
{format}: InfoOptions,
): Promise<OutputMessage> {
return outputEnv(app, remoteApp, format)
return outputEnv(app, remoteApp, organization, format)
}

async function infoApp(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ export interface DeveloperPlatformClient {
readonly supportsAtomicDeployments: boolean
readonly requiresOrganization: boolean
readonly supportsDevSessions: boolean
readonly organizationSource: OrganizationSource
session: () => Promise<PartnersSession>
refreshToken: () => Promise<string>
accountInfo: () => Promise<PartnersSession['accountInfo']>
Expand Down
Loading

0 comments on commit 4861084

Please sign in to comment.