From 4861084b408f17fbef5bb8fcc7f9361a7c33accc Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Sun, 22 Dec 2024 21:02:24 +0200 Subject: [PATCH] Record business_platform_id not partner_id in remaining cases --- packages/app/src/cli/commands/app/env/pull.ts | 4 ++-- packages/app/src/cli/commands/app/env/show.ts | 4 ++-- packages/app/src/cli/commands/app/info.ts | 4 ++-- .../app/src/cli/models/app/app.test-data.ts | 8 +++++++- .../app/src/cli/services/app/config/link.ts | 2 +- .../src/cli/services/app/config/use.test.ts | 7 ++++++- .../app/src/cli/services/app/config/use.ts | 12 ++++++++---- .../app/src/cli/services/app/env/pull.test.ts | 14 ++++++++++---- packages/app/src/cli/services/app/env/pull.ts | 7 ++++--- .../app/src/cli/services/app/env/show.test.ts | 3 +-- packages/app/src/cli/services/app/env/show.ts | 19 ++++++++++++++----- packages/app/src/cli/services/context.ts | 14 ++++++++++---- packages/app/src/cli/services/info.test.ts | 14 +++++++------- packages/app/src/cli/services/info.ts | 8 +++++--- .../utilities/developer-platform-client.ts | 1 + .../app-management-client.ts | 5 +++-- .../partners-client.ts | 7 ++++--- 17 files changed, 87 insertions(+), 46 deletions(-) diff --git a/packages/app/src/cli/commands/app/env/pull.ts b/packages/app/src/cli/commands/app/env/pull.ts index 60fb0defa35..6d2e9f9a5be 100644 --- a/packages/app/src/cli/commands/app/env/pull.ts +++ b/packages/app/src/cli/commands/app/env/pull.ts @@ -30,14 +30,14 @@ export default class EnvPull extends AppCommand { public async run(): Promise { 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} } } diff --git a/packages/app/src/cli/commands/app/env/show.ts b/packages/app/src/cli/commands/app/env/show.ts index 67ec0d3e2a1..f637aca0373 100644 --- a/packages/app/src/cli/commands/app/env/show.ts +++ b/packages/app/src/cli/commands/app/env/show.ts @@ -19,13 +19,13 @@ export default class EnvShow extends AppCommand { public async run(): Promise { 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} } } diff --git a/packages/app/src/cli/commands/app/info.ts b/packages/app/src/cli/commands/app/info.ts index 5de3daa4b58..c94727d0abc 100644 --- a/packages/app/src/cli/commands/app/info.ts +++ b/packages/app/src/cli/commands/app/info.ts @@ -33,7 +33,7 @@ export default class AppInfo extends AppCommand { public async run(): Promise { 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, @@ -41,7 +41,7 @@ export default class AppInfo extends AppCommand { 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, diff --git a/packages/app/src/cli/models/app/app.test-data.ts b/packages/app/src/cli/models/app/app.test-data.ts index 7f46adf12f4..8da468be1b0 100644 --- a/packages/app/src/cli/models/app/app.test-data.ts +++ b/packages/app/src/cli/models/app/app.test-data.ts @@ -1318,6 +1318,7 @@ export function testDeveloperPlatformClient(stubs: Partial Promise.resolve(testPartnersUserSession), refreshToken: () => Promise.resolve(testPartnersUserSession.token), accountInfo: () => Promise.resolve(testPartnersUserSession.accountInfo), @@ -1378,7 +1379,12 @@ export function testDeveloperPlatformClient(stubs: Partial ] = vi.fn().mockImplementation(value) } diff --git a/packages/app/src/cli/services/app/config/link.ts b/packages/app/src/cli/services/app/config/link.ts index 2d147f594b7..cb3308f3518 100644 --- a/packages/app/src/cli/services/app/config/link.ts +++ b/packages/app/src/cli/services/app/config/link.ts @@ -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({ diff --git a/packages/app/src/cli/services/app/config/use.test.ts b/packages/app/src/cli/services/app/config/use.test.ts index d8069bc188c..0bcf989a2d7 100644 --- a/packages/app/src/cli/services/app/config/use.test.ts +++ b/packages/app/src/cli/services/app/config/use.test.ts @@ -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' @@ -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, + ) }) }) }) diff --git a/packages/app/src/cli/services/app/config/use.ts b/packages/app/src/cli/services/app/config/use.ts index 4c644731bfc..0bd5262bef5 100644 --- a/packages/app/src/cli/services/app/config/use.ts +++ b/packages/app/src/cli/services/app/config/use.ts @@ -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' @@ -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', + }, + configuration.organization_id ? OrganizationSource.BusinessPlatform : OrganizationSource.Partners, + ) } diff --git a/packages/app/src/cli/services/app/env/pull.test.ts b/packages/app/src/cli/services/app/env/pull.test.ts index 6bfdeacf921..c16c008aa63 100644 --- a/packages/app/src/cli/services/app/env/pull.test.ts +++ b/packages/app/src/cli/services/app/env/pull.test.ts @@ -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 @@ -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( @@ -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( @@ -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() diff --git a/packages/app/src/cli/services/app/env/pull.ts b/packages/app/src/cli/services/app/env/pull.ts index aba36faad31..422749dd02e 100644 --- a/packages/app/src/cli/services/app/env/pull.ts +++ b/packages/app/src/cli/services/app/env/pull.ts @@ -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' @@ -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 { - await logMetadataForLoadedContext(remoteApp) +export async function pullEnv({app, remoteApp, organization, envFile}: PullEnvOptions): Promise { + await logMetadataForLoadedContext(remoteApp, organization.source) const updatedValues = { SHOPIFY_API_KEY: remoteApp.apiKey, diff --git a/packages/app/src/cli/services/app/env/show.test.ts b/packages/app/src/cli/services/app/env/show.test.ts index 3f79caff203..e434cda300d 100644 --- a/packages/app/src/cli/services/app/env/show.test.ts +++ b/packages/app/src/cli/services/app/env/show.test.ts @@ -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() diff --git a/packages/app/src/cli/services/app/env/show.ts b/packages/app/src/cli/services/app/env/show.ts index b455e59d963..a2dbab01a07 100644 --- a/packages/app/src/cli/services/app/env/show.ts +++ b/packages/app/src/cli/services/app/env/show.ts @@ -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 { - return outputEnv(app, remoteApp, 'text') +export async function showEnv( + app: AppInterface, + remoteApp: OrganizationApp, + organization: Organization, +): Promise { + return outputEnv(app, remoteApp, organization, 'text') } -export async function outputEnv(app: AppInterface, remoteApp: OrganizationApp, format: Format): Promise { - await logMetadataForLoadedContext(remoteApp) +export async function outputEnv( + app: AppInterface, + remoteApp: OrganizationApp, + organization: Organization, + format: Format, +): Promise { + await logMetadataForLoadedContext(remoteApp, organization.source) if (format === 'json') { return outputContent`${outputToken.json({ diff --git a/packages/app/src/cli/services/context.ts b/packages/app/src/cli/services/context.ts index 09be435baf3..0556c385995 100644 --- a/packages/app/src/cli/services/context.ts +++ b/packages/app/src/cli/services/context.ts @@ -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' @@ -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 } @@ -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, })) } diff --git a/packages/app/src/cli/services/info.test.ts b/packages/app/src/cli/services/info.test.ts index 4ccea66b5a2..15c4161cf1d 100644 --- a/packages/app/src/cli/services/info.test.ts +++ b/packages/app/src/cli/services/info.test.ts @@ -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}`) }) @@ -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') @@ -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(` @@ -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(` @@ -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') @@ -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') @@ -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) diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index 9ec1f0057c9..3adf9ea510e 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -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' @@ -27,10 +27,11 @@ interface Configurable { export async function info( app: AppLinkedInterface, remoteApp: OrganizationApp, + organization: Organization, options: InfoOptions, ): Promise { if (options.webEnv) { - return infoWeb(app, remoteApp, options) + return infoWeb(app, remoteApp, organization, options) } else { return infoApp(app, remoteApp, options) } @@ -39,9 +40,10 @@ export async function info( async function infoWeb( app: AppLinkedInterface, remoteApp: OrganizationApp, + organization: Organization, {format}: InfoOptions, ): Promise { - return outputEnv(app, remoteApp, format) + return outputEnv(app, remoteApp, organization, format) } async function infoApp( diff --git a/packages/app/src/cli/utilities/developer-platform-client.ts b/packages/app/src/cli/utilities/developer-platform-client.ts index 7a8673be6c4..2ba275b1c47 100644 --- a/packages/app/src/cli/utilities/developer-platform-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client.ts @@ -211,6 +211,7 @@ export interface DeveloperPlatformClient { readonly supportsAtomicDeployments: boolean readonly requiresOrganization: boolean readonly supportsDevSessions: boolean + readonly organizationSource: OrganizationSource session: () => Promise refreshToken: () => Promise accountInfo: () => Promise diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index a4c0d52fa54..e97a90700a5 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -151,6 +151,7 @@ export class AppManagementClient implements DeveloperPlatformClient { public readonly requiresOrganization = true public readonly supportsAtomicDeployments = true public readonly supportsDevSessions = true + public readonly organizationSource = OrganizationSource.BusinessPlatform private _session: PartnersSession | undefined private _businessPlatformToken: string | undefined @@ -243,7 +244,7 @@ export class AppManagementClient implements DeveloperPlatformClient { return organizationsResult.currentUserAccount.organizations.nodes.map((org) => ({ id: idFromEncodedGid(org.id), businessName: org.name, - source: OrganizationSource.BusinessPlatform, + source: this.organizationSource, })) } @@ -262,7 +263,7 @@ export class AppManagementClient implements DeveloperPlatformClient { return { id: orgId, businessName: org.name, - source: OrganizationSource.BusinessPlatform, + source: this.organizationSource, } } diff --git a/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts b/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts index 9de7511b990..1e857fe5629 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts @@ -213,6 +213,7 @@ export class PartnersClient implements DeveloperPlatformClient { public readonly supportsAtomicDeployments = false public readonly requiresOrganization = false public readonly supportsDevSessions = false + public readonly organizationSource = OrganizationSource.Partners private _session: PartnersSession | undefined constructor(session?: PartnersSession) { @@ -284,7 +285,7 @@ export class PartnersClient implements DeveloperPlatformClient { return result.organizations.nodes!.map((org) => ({ id: org!.id, businessName: org!.businessName, - source: OrganizationSource.Partners, + source: this.organizationSource, })) } catch (error: unknown) { if ((error as {statusCode?: number}).statusCode === 404) { @@ -299,7 +300,7 @@ export class PartnersClient implements DeveloperPlatformClient { const variables: FindOrganizationBasicVariables = {id: orgId} const result: FindOrganizationBasicQuerySchema = await this.request(FindOrganizationBasicQuery, variables) const org: Omit | undefined = result.organizations.nodes[0] - return org ? {...org, source: OrganizationSource.Partners} : undefined + return org ? {...org, source: this.organizationSource} : undefined } async orgAndApps(orgId: string): Promise> { @@ -587,7 +588,7 @@ export class PartnersClient implements DeveloperPlatformClient { const partnersSession = await this.session() throw new NoOrgError(partnersSession.accountInfo, orgId) } - const parsedOrg = {id: org.id, businessName: org.businessName, source: OrganizationSource.Partners} + const parsedOrg = {id: org.id, businessName: org.businessName, source: this.organizationSource} const appsWithOrg = org.apps.nodes.map((app) => ({...app, organizationId: org.id})) return {organization: parsedOrg, apps: {...org.apps, nodes: appsWithOrg}, stores: []} }