From b60dff9dd29cd1757b4292e2391fd3cf4ce0169e Mon Sep 17 00:00:00 2001 From: Andrew Hassan Date: Fri, 3 Jan 2025 16:08:55 -0500 Subject: [PATCH 01/50] Validate shopify_function version on build --- .../src/cli/services/function/build.test.ts | 25 ++++++++ .../app/src/cli/services/function/build.ts | 58 ++++++++++++++----- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/packages/app/src/cli/services/function/build.test.ts b/packages/app/src/cli/services/function/build.test.ts index af07517f4b2..463f926927c 100644 --- a/packages/app/src/cli/services/function/build.test.ts +++ b/packages/app/src/cli/services/function/build.test.ts @@ -67,6 +67,9 @@ async function installShopifyLibrary(tmpDir: string) { const runModule = joinPath(shopifyFunctionDir, 'run.ts') await writeFile(runModule, '') + const packageJson = joinPath(shopifyFunctionDir, 'package.json') + await writeFile(packageJson, JSON.stringify({version: '1.0.0'})) + return shopifyFunction } @@ -136,6 +139,28 @@ describe('bundleExtension', () => { }) }) + test('errors if shopify library is not on a compatible version', async () => { + await inTemporaryDirectory(async (tmpDir) => { + // Given + const incompatibleVersion = '0.0.1' + const ourFunction = await testFunctionExtension({dir: tmpDir}) + ourFunction.entrySourceFilePath = joinPath(tmpDir, 'src/index.ts') + await installShopifyLibrary(tmpDir) + await writeFile( + joinPath(tmpDir, 'node_modules/@shopify/shopify_function/package.json'), + JSON.stringify({version: incompatibleVersion}), + ) + + // When + const got = bundleExtension(ourFunction, {stdout, stderr, signal, app}) + + // Then + await expect(got).rejects.toThrow( + /The installed version of the Shopify Functions JavaScript library is not compatible with this version of Shopify CLI./, + ) + }) + }) + test('errors if user function not found', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index 8d86c1ba643..f16b9724d11 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -9,7 +9,7 @@ import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/out import {exec} from '@shopify/cli-kit/node/system' import {dirname, joinPath} from '@shopify/cli-kit/node/path' import {build as esBuild, BuildResult} from 'esbuild' -import {findPathUp, inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' +import {findPathUp, inTemporaryDirectory, readFile, writeFile} from '@shopify/cli-kit/node/fs' import {AbortSignal} from '@shopify/cli-kit/node/abort' import {renderTasks} from '@shopify/cli-kit/node/ui' import {pickBy} from '@shopify/cli-kit/common/object' @@ -17,6 +17,26 @@ import {runWithTimer} from '@shopify/cli-kit/node/metadata' import {AbortError} from '@shopify/cli-kit/node/error' import {Writable} from 'stream' +const SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION = '1' + +class InvalidShopifyFunctionPackageError extends AbortError { + constructor(message: string) { + super( + message, + outputContent`Make sure you have a compatible version of the ${outputToken.yellow( + '@shopify/shopify_function', + )} library installed.`, + [ + outputContent`Add ${outputToken.green( + `"@shopify/shopify_function": "~${SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION}.0.0"`, + )} to the dependencies section of the package.json file in your function's directory, if not already present.` + .value, + `Run your package manager's install command to update dependencies.`, + ], + ) + } +} + interface JSFunctionBuildOptions { stdout: Writable stderr: Writable @@ -117,19 +137,7 @@ async function checkForShopifyFunctionRuntimeEntrypoint(fun: ExtensionInstance) { + const packageJsonPath = await findPathUp('node_modules/@shopify/shopify_function/package.json', { + type: 'file', + cwd: fun.directory, + }) + + if (!packageJsonPath) { + throw new InvalidShopifyFunctionPackageError('Could not find the Shopify Functions JavaScript library.') + } + + const packageJson = JSON.parse(await readFile(packageJsonPath)) + const majorVersion = packageJson.version.split('.')[0] + + if (majorVersion !== SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION) { + throw new InvalidShopifyFunctionPackageError( + 'The installed version of the Shopify Functions JavaScript library is not compatible with this version of Shopify CLI.', + ) + } +} + export async function bundleExtension( fun: ExtensionInstance, options: JSFunctionBuildOptions, processEnv = process.env, ) { + await validateShopifyFunctionPackageVersion(fun) const entryPoint = await checkForShopifyFunctionRuntimeEntrypoint(fun) const esbuildOptions = { @@ -276,6 +305,7 @@ export class ExportJavyBuilder implements JavyBuilder { } async bundle(fun: ExtensionInstance, options: JSFunctionBuildOptions, processEnv = process.env) { + await validateShopifyFunctionPackageVersion(fun) await checkForShopifyFunctionRuntimeEntrypoint(fun) const contents = this.entrypointContents From f64195eaa6e9f79033224d332fca688aff21c99a Mon Sep 17 00:00:00 2001 From: Andrew Hassan Date: Fri, 3 Jan 2025 17:09:49 -0500 Subject: [PATCH 02/50] Use the latest patch version when generating function --- packages/app/src/cli/services/function/build.ts | 2 +- packages/app/src/cli/services/generate/extension.ts | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index f16b9724d11..09975277432 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -17,7 +17,7 @@ import {runWithTimer} from '@shopify/cli-kit/node/metadata' import {AbortError} from '@shopify/cli-kit/node/error' import {Writable} from 'stream' -const SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION = '1' +export const SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION = '1' class InvalidShopifyFunctionPackageError extends AbortError { constructor(message: string) { diff --git a/packages/app/src/cli/services/generate/extension.ts b/packages/app/src/cli/services/generate/extension.ts index 5afacabf85f..de4c156bb43 100644 --- a/packages/app/src/cli/services/generate/extension.ts +++ b/packages/app/src/cli/services/generate/extension.ts @@ -1,6 +1,6 @@ import {configurationFileNames, versions} from '../../constants.js' import {AppInterface} from '../../models/app/app.js' -import {buildGraphqlTypes} from '../function/build.js' +import {buildGraphqlTypes, SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION} from '../function/build.js' import {GenerateExtensionContentOutput} from '../../prompts/generate/extension.js' import {ExtensionFlavor, ExtensionTemplate} from '../../models/app/template.js' import {ensureDownloadedExtensionFlavorExists, ensureExtensionDirectoryExists} from '../extensions/common.js' @@ -299,7 +299,10 @@ function getSrcFileExtension(extensionFlavor: ExtensionFlavorValue): SrcFileExte export function getFunctionRuntimeDependencies(templateLanguage: string): DependencyVersion[] { const dependencies: DependencyVersion[] = [] if (templateLanguage === 'javascript') { - dependencies.push({name: '@shopify/shopify_function', version: '1.0.0'}) + dependencies.push({ + name: '@shopify/shopify_function', + version: `~${SHOPIFY_FUNCTION_NPM_PACKAGE_MAJOR_VERSION}.0.0`, + }) } return dependencies } From f4c75771513a1e7d9633ad78db266508dbca54c0 Mon Sep 17 00:00:00 2001 From: Andrew Hassan Date: Mon, 6 Jan 2025 10:55:26 -0500 Subject: [PATCH 03/50] Add changeset --- .changeset/tough-guests-behave.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/tough-guests-behave.md diff --git a/.changeset/tough-guests-behave.md b/.changeset/tough-guests-behave.md new file mode 100644 index 00000000000..70bfc102ca1 --- /dev/null +++ b/.changeset/tough-guests-behave.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Validate the @shopify/shopify_function NPM package version is compatible with the Javy version From 2269a79ecc288478e41959810ce974388b7aa49f Mon Sep 17 00:00:00 2001 From: Fatai Salami Date: Thu, 5 Dec 2024 19:18:47 +0000 Subject: [PATCH 04/50] Add support for MOTO in credit card payments extension schema --- .../credit_card_payments_app_extension_schema.test.ts | 2 ++ .../credit_card_payments_app_extension_schema.ts | 4 ++++ 2 files changed, 6 insertions(+) diff --git a/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.test.ts b/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.test.ts index b5f5e5867b3..6ca0bf4061f 100644 --- a/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.test.ts @@ -21,6 +21,7 @@ const config: CreditCardPaymentsAppExtensionConfigType = { supported_countries: ['CA'], supported_payment_methods: ['PAYMENT_METHOD'], supported_buyer_contexts: [{currency: 'USD'}, {currency: 'CAD'}], + supports_moto: true, supports_3ds: false, test_mode_available: true, supports_deferred_payments: false, @@ -194,6 +195,7 @@ describe('creditCardPaymentsAppExtensionDeployConfig', () => { supported_payment_methods: config.supported_payment_methods, supported_buyer_contexts: config.supported_buyer_contexts, test_mode_available: config.test_mode_available, + supports_moto: config.supports_moto, supports_3ds: config.supports_3ds, supports_deferred_payments: config.supports_deferred_payments, supports_installments: config.supports_installments, diff --git a/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.ts b/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.ts index e4f65957dc0..22d004d586b 100644 --- a/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.ts +++ b/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.ts @@ -29,6 +29,7 @@ export const CreditCardPaymentsAppExtensionSchema = BasePaymentsAppExtensionSche targeting: zod.array(zod.object({target: zod.literal(CREDIT_CARD_TARGET)})).length(1), verification_session_url: zod.string().url().optional(), ui_extension_handle: zod.string().optional(), + supports_moto: zod.boolean(), encryption_certificate_fingerprint: zod .string() .min(1, {message: "Encryption certificate fingerprint can't be blank"}), @@ -72,6 +73,7 @@ export interface CreditCardPaymentsAppExtensionDeployConfigType extends BasePaym supports_3ds: boolean // CreditCard-specific fields + supports_moto: boolean start_verification_session_url?: string ui_extension_registration_uuid?: string ui_extension_handle?: string @@ -106,6 +108,7 @@ export function creditCardDeployConfigToCLIConfig( supported_buyer_contexts: config.supported_buyer_contexts, test_mode_available: config.test_mode_available, supports_3ds: config.supports_3ds, + supports_moto: config.supports_moto, supports_deferred_payments: config.supports_deferred_payments, supports_installments: config.supports_installments, verification_session_url: config.start_verification_session_url, @@ -132,6 +135,7 @@ export async function creditCardPaymentsAppExtensionDeployConfig( supported_buyer_contexts: config.supported_buyer_contexts, test_mode_available: config.test_mode_available, supports_3ds: config.supports_3ds, + supports_moto: config.supports_moto, supports_deferred_payments: config.supports_deferred_payments, encryption_certificate_fingerprint: config.encryption_certificate_fingerprint, supports_installments: config.supports_installments, From 55c6e45a79a0e8479ef37bdf7a9f0f69ffc46a37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 7 Jan 2025 16:18:53 +0100 Subject: [PATCH 05/50] Fix extension_directories wildcards to always be recursive --- packages/app/src/cli/models/app/app.test.ts | 1 + packages/app/src/cli/models/app/app.ts | 17 +++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/app/src/cli/models/app/app.test.ts b/packages/app/src/cli/models/app/app.test.ts index da46653b30c..20f98d61383 100644 --- a/packages/app/src/cli/models/app/app.test.ts +++ b/packages/app/src/cli/models/app/app.test.ts @@ -30,6 +30,7 @@ const CORRECT_CURRENT_APP_SCHEMA: CurrentAppConfiguration = { path: '', name: 'app 1', client_id: '12345', + extension_directories: ['extensions/*'], webhooks: { api_version: '2023-04', privacy_compliance: { diff --git a/packages/app/src/cli/models/app/app.ts b/packages/app/src/cli/models/app/app.ts index 395dde13113..150c74504c5 100644 --- a/packages/app/src/cli/models/app/app.ts +++ b/packages/app/src/cli/models/app/app.ts @@ -23,6 +23,12 @@ import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array' // Schemas for loading app configuration +const ExtensionDirectoriesSchema = zod + .array(zod.string()) + .optional() + .transform(removeTrailingPathSeparator) + .transform(fixSingleWildcards) + /** * Schema for a freshly minted app template. */ @@ -34,7 +40,7 @@ export const LegacyAppSchema = zod .string() .transform((scopes) => normalizeDelimitedString(scopes) ?? '') .default(''), - extension_directories: zod.array(zod.string()).optional().transform(removeTrailingPathSeparator), + extension_directories: ExtensionDirectoriesSchema, web_directories: zod.array(zod.string()).optional(), webhooks: zod .object({ @@ -49,6 +55,13 @@ function removeTrailingPathSeparator(value: string[] | undefined) { // eslint-disable-next-line no-useless-escape return value?.map((dir) => dir.replace(/[\/\\]+$/, '')) } + +// If a path ends with a single asterisk, modify it to end with a double asterisk. +// This is to support the glob pattern used by chokidar and watch for changes in subfolders. +function fixSingleWildcards(value: string[] | undefined) { + return value?.map((dir) => dir.replace(/\*$/, '**')) +} + /** * Schema for a normal, linked app. Properties from modules are not validated. */ @@ -62,7 +75,7 @@ export const AppSchema = zod.object({ include_config_on_deploy: zod.boolean().optional(), }) .optional(), - extension_directories: zod.array(zod.string()).optional().transform(removeTrailingPathSeparator), + extension_directories: ExtensionDirectoriesSchema, web_directories: zod.array(zod.string()).optional(), }) From b874496f6b7fda5306a75214dd38c8036f0813ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 8 Jan 2025 10:55:31 +0100 Subject: [PATCH 06/50] Improve regex and tests --- packages/app/src/cli/models/app/app.test.ts | 28 +++++++++++++++++++++ packages/app/src/cli/models/app/app.ts | 3 ++- 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/models/app/app.test.ts b/packages/app/src/cli/models/app/app.test.ts index 20f98d61383..7152a6ac190 100644 --- a/packages/app/src/cli/models/app/app.test.ts +++ b/packages/app/src/cli/models/app/app.test.ts @@ -1,4 +1,5 @@ import { + AppSchema, CurrentAppConfiguration, LegacyAppConfiguration, getAppScopes, @@ -92,6 +93,33 @@ describe('app schema validation', () => { expect(isCurrentAppSchema(config)).toBe(false) }) + + test('extension_directories should be transformed to double asterisks', () => { + const config = { + ...CORRECT_CURRENT_APP_SCHEMA, + extension_directories: ['extensions/*'], + } + const parsed = AppSchema.parse(config) + expect(parsed.extension_directories).toEqual(['extensions/**']) + }) + + test('extension_directories is not transformed if it ends with double asterisks', () => { + const config = { + ...CORRECT_CURRENT_APP_SCHEMA, + extension_directories: ['extensions/**'], + } + const parsed = AppSchema.parse(config) + expect(parsed.extension_directories).toEqual(['extensions/**']) + }) + + test('extension_directories is not transformed if it doesnt end with a wildcard', () => { + const config = { + ...CORRECT_CURRENT_APP_SCHEMA, + extension_directories: ['extensions'], + } + const parsed = AppSchema.parse(config) + expect(parsed.extension_directories).toEqual(['extensions']) + }) }) }) diff --git a/packages/app/src/cli/models/app/app.ts b/packages/app/src/cli/models/app/app.ts index 150c74504c5..c59e110b23e 100644 --- a/packages/app/src/cli/models/app/app.ts +++ b/packages/app/src/cli/models/app/app.ts @@ -59,7 +59,8 @@ function removeTrailingPathSeparator(value: string[] | undefined) { // If a path ends with a single asterisk, modify it to end with a double asterisk. // This is to support the glob pattern used by chokidar and watch for changes in subfolders. function fixSingleWildcards(value: string[] | undefined) { - return value?.map((dir) => dir.replace(/\*$/, '**')) + // eslint-disable-next-line no-useless-escape + return value?.map((dir) => dir.replace(/([^\*])\*$/, '$1**')) } /** From f1ab6c75f7ccd583c1bfa4f437373ec2a5c373a0 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Sun, 22 Dec 2024 21:02:24 +0200 Subject: [PATCH 07/50] 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 082522d78a6..d77dcdc3715 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 34a9164c81b..26e3e96057a 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 16b5f9e5eb0..29ce599e74d 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' @@ -246,7 +246,7 @@ export async function fetchOrCreateOrganizationApp( }) remoteApp.developerPlatformClient = developerPlatformClient - await logMetadataForLoadedContext({organizationId: remoteApp.organizationId, apiKey: remoteApp.apiKey}) + await logMetadataForLoadedContext(remoteApp, developerPlatformClient.organizationSource) return remoteApp } @@ -345,9 +345,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 c81a49547f8..d2ccbf07549 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 98e77e8493c..1eebe449b25 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 @@ -153,6 +153,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 @@ -245,7 +246,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, })) } @@ -264,7 +265,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 a819392e444..955f1d122be 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> { @@ -590,7 +591,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: []} } From 0aedc845b6f02ca26b9330b618913c289454cb0b Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 17:29:24 +0200 Subject: [PATCH 08/50] Remove metadata reporting from config use It's already being done downstream when we load the remote app. --- .../src/cli/services/app/config/use.test.ts | 34 ------------------- .../app/src/cli/services/app/config/use.ts | 14 -------- 2 files changed, 48 deletions(-) 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 0bcf989a2d7..6f8418c59b0 100644 --- a/packages/app/src/cli/services/app/config/use.test.ts +++ b/packages/app/src/cli/services/app/config/use.test.ts @@ -4,13 +4,10 @@ import { testApp, testAppWithConfig, testDeveloperPlatformClient, - testOrganizationApp, } from '../../../models/app/app.test-data.js' import {getAppConfigurationFileName, loadAppConfiguration} from '../../../models/app/loader.js' 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' @@ -23,8 +20,6 @@ vi.mock('../../../models/app/loader.js') vi.mock('@shopify/cli-kit/node/ui') vi.mock('../../context.js') -const REMOTE_APP = testOrganizationApp() - describe('use', () => { test('clears currentConfiguration when reset is true', async () => { await inTemporaryDirectory(async (tmp) => { @@ -290,35 +285,6 @@ describe('use', () => { expect(setCachedAppInfo).toHaveBeenCalledWith({directory, configFile: 'shopify.app.something.toml'}) }) }) - - test('logs metadata', async () => { - await inTemporaryDirectory(async (directory) => { - // Given - const {configuration} = testApp({}, 'current') - const {schema: configSchema} = await buildVersionedAppSchema() - vi.mocked(loadAppConfiguration).mockResolvedValue({ - directory, - configuration, - configSchema, - specifications: [], - remoteFlags: [], - }) - vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.something.toml') - vi.mocked(appFromIdentifiers).mockResolvedValue(REMOTE_APP) - createConfigFile(directory, 'shopify.app.something.toml') - const options = {directory, configName: 'something', developerPlatformClient: testDeveloperPlatformClient()} - - // When - await use(options) - - // Then - expect(logMetadataForLoadedContext).toHaveBeenNthCalledWith( - 1, - {apiKey: REMOTE_APP.apiKey, organizationId: '0'}, - OrganizationSource.Partners, - ) - }) - }) }) function createConfigFile(tmp: string, fileName: string) { diff --git a/packages/app/src/cli/services/app/config/use.ts b/packages/app/src/cli/services/app/config/use.ts index 0bd5262bef5..d99123cf58b 100644 --- a/packages/app/src/cli/services/app/config/use.ts +++ b/packages/app/src/cli/services/app/config/use.ts @@ -2,9 +2,7 @@ import {getAppConfigurationFileName, loadAppConfiguration} from '../../../models import {clearCurrentConfigFile, setCachedAppInfo} from '../../local-storage.js' 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' @@ -61,8 +59,6 @@ export default async function use({ }) } - await logMetadata(configuration) - return configFileName } @@ -101,13 +97,3 @@ async function getConfigFileName(directory: string, configName?: string): Promis } return selectConfigFile(directory) } - -async function logMetadata(configuration: CurrentAppConfiguration) { - await logMetadataForLoadedContext( - { - apiKey: configuration.client_id, - organizationId: configuration.organization_id || '0', - }, - configuration.organization_id ? OrganizationSource.BusinessPlatform : OrganizationSource.Partners, - ) -} From cf2bc94b0fc73abe04512019eff8bc1234088e1c Mon Sep 17 00:00:00 2001 From: Paul Buzuloiu Date: Thu, 9 Jan 2025 21:21:52 +0000 Subject: [PATCH 09/50] fixup! Add support for MOTO in credit card payments extension schema --- ...card_payments_app_extension_schema.test.ts | 40 +++++++++++++++++++ ...edit_card_payments_app_extension_schema.ts | 5 ++- 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.test.ts b/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.test.ts index 6ca0bf4061f..c8d4fd47f9b 100644 --- a/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.test.ts @@ -173,6 +173,46 @@ describe('CreditCardPaymentsAppExtensionSchema', () => { ]), ) }) + + test('returns an error if supports_moto is not a boolean', async () => { + // When/Then + expect(() => + CreditCardPaymentsAppExtensionSchema.parse({ + ...config, + supports_moto: 'true', + }), + ).toThrowError( + new zod.ZodError([ + { + code: 'invalid_type', + expected: 'boolean', + received: 'string', + path: ['supports_moto'], + message: 'Value must be Boolean', + }, + ]), + ) + }) + + test('returns an error if supports_moto is not present', async () => { + // When/Then + expect(() => + CreditCardPaymentsAppExtensionSchema.parse({ + ...config, + supports_moto: undefined, + }), + ).toThrowError( + new zod.ZodError([ + { + code: 'invalid_type', + expected: 'boolean', + received: 'undefined', + path: ['supports_moto'], + message: 'supports_moto is required', + }, + ]), + ) + }) }) describe('creditCardPaymentsAppExtensionDeployConfig', () => { diff --git a/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.ts b/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.ts index 22d004d586b..b6515040426 100644 --- a/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.ts +++ b/packages/app/src/cli/models/extensions/specifications/payments_app_extension_schemas/credit_card_payments_app_extension_schema.ts @@ -29,7 +29,10 @@ export const CreditCardPaymentsAppExtensionSchema = BasePaymentsAppExtensionSche targeting: zod.array(zod.object({target: zod.literal(CREDIT_CARD_TARGET)})).length(1), verification_session_url: zod.string().url().optional(), ui_extension_handle: zod.string().optional(), - supports_moto: zod.boolean(), + supports_moto: zod.boolean({ + required_error: 'supports_moto is required', + invalid_type_error: 'Value must be Boolean', + }), encryption_certificate_fingerprint: zod .string() .min(1, {message: "Encryption certificate fingerprint can't be blank"}), From 15477047d0f1ce25c4c2b02abf1fef9d3cd03913 Mon Sep 17 00:00:00 2001 From: Jamie Guerrero Date: Wed, 8 Jan 2025 14:40:54 -0500 Subject: [PATCH 10/50] Hide banner that tells you to change app URL in Partners dashboard when using App Management API --- packages/app/src/cli/services/dev.ts | 2 +- packages/app/src/cli/services/dev/ui.tsx | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index 3b88ec56922..a3e788a3cfb 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -277,7 +277,7 @@ async function handleUpdatingOfPartnerUrls( // When running dev app urls are pushed directly to API Client config instead of creating a new app version // so current app version and API Client config will have diferent url values. if (shouldUpdateURLs) await updateURLs(newURLs, apiKey, developerPlatformClient, localApp) - await outputUpdateURLsResult(shouldUpdateURLs, newURLs, remoteApp, localApp) + await outputUpdateURLsResult(shouldUpdateURLs, newURLs, remoteApp, localApp, developerPlatformClient) } } return shouldUpdateURLs diff --git a/packages/app/src/cli/services/dev/ui.tsx b/packages/app/src/cli/services/dev/ui.tsx index 1a3e5ac4256..bf3199170a6 100644 --- a/packages/app/src/cli/services/dev/ui.tsx +++ b/packages/app/src/cli/services/dev/ui.tsx @@ -3,6 +3,7 @@ import {Dev, DevProps} from './ui/components/Dev.js' import {AppInterface, isCurrentAppSchema} from '../../models/app/app.js' import {OrganizationApp} from '../../models/organization.js' import {getAppConfigurationShorthand} from '../../models/app/loader.js' +import {ClientName, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import React from 'react' import {partnersFqdn} from '@shopify/cli-kit/node/context/fqdn' import {render, renderInfo} from '@shopify/cli-kit/node/ui' @@ -17,9 +18,11 @@ export async function outputUpdateURLsResult( urls: PartnersURLs, remoteApp: OrganizationApp, localApp: AppInterface, + developerPlatformClient?: DeveloperPlatformClient, ) { + const usingAppManagementClient = developerPlatformClient?.clientName === ClientName.AppManagement const dashboardURL = await partnersURL(remoteApp.organizationId, remoteApp.id) - if (remoteApp.newApp) { + if (remoteApp.newApp && !usingAppManagementClient) { renderInfo({ headline: `For your convenience, we've given your app a default URL: ${urls.applicationUrl}.`, body: [ From 10333fca5ca7bfed4a5c4de928ef38cf4c701c31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 3 Jan 2025 11:14:31 +0100 Subject: [PATCH 11/50] Use UID instead of handle to identify extensions in app-watcher --- packages/app/src/cli/models/app/app.ts | 6 +++--- .../cli/models/extensions/extension-instance.ts | 7 ++++--- .../cli/services/dev/app-events/app-diffing.ts | 17 +++++++++-------- .../dev/app-events/app-event-watcher-handler.ts | 7 ++++--- .../dev/app-events/app-event-watcher.ts | 8 ++++---- .../dev/app-events/app-watcher-esbuild.ts | 8 ++++---- .../cli/services/dev/app-events/file-watcher.ts | 9 ++++++--- .../cli/services/dev/processes/dev-session.ts | 1 + 8 files changed, 35 insertions(+), 28 deletions(-) diff --git a/packages/app/src/cli/models/app/app.ts b/packages/app/src/cli/models/app/app.ts index c59e110b23e..106210c3625 100644 --- a/packages/app/src/cli/models/app/app.ts +++ b/packages/app/src/cli/models/app/app.ts @@ -273,7 +273,7 @@ export interface AppInterface< */ creationDefaultOptions(): AppCreationDefaultOptions manifest: () => Promise - removeExtension: (extensionHandle: string) => void + removeExtension: (extensionUid: string) => void } type AppConstructor< @@ -441,8 +441,8 @@ export class App< } } - removeExtension(extensionHandle: string) { - this.realExtensions = this.realExtensions.filter((ext) => ext.handle !== extensionHandle) + removeExtension(extensionUid: string) { + this.realExtensions = this.realExtensions.filter((ext) => ext.uid !== extensionUid) } get includeConfigOnDeploy() { diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index dcdfa4d66fc..481352a7e2c 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -25,7 +25,7 @@ import {DeveloperPlatformClient} from '../../utilities/developer-platform-client import {AppConfigurationWithoutPath, CurrentAppConfiguration} from '../app/app.js' import {ok} from '@shopify/cli-kit/node/result' import {constantize, slugify} from '@shopify/cli-kit/common/string' -import {hashString, nonRandomUUID, randomUUID} from '@shopify/cli-kit/node/crypto' +import {hashString, nonRandomUUID} from '@shopify/cli-kit/node/crypto' import {partnersFqdn} from '@shopify/cli-kit/node/context/fqdn' import {joinPath, basename} from '@shopify/cli-kit/node/path' import {fileExists, touchFile, moveFile, writeFile, glob} from '@shopify/cli-kit/node/fs' @@ -150,11 +150,12 @@ export class ExtensionInstance ext.handle) + const oldExtensionsUids = oldExtensions.map((ext) => ext.uid) const newExtensions = newApp.realExtensions - const newExtensionsHandles = newExtensions.map((ext) => ext.handle) + const newExtensionsUids = newExtensions.map((ext) => ext.uid) - const createdExtensions = newExtensions.filter((ext) => !oldExtensionsHandles.includes(ext.handle)) - const deletedExtensions = oldExtensions.filter((ext) => !newExtensionsHandles.includes(ext.handle)) + const createdExtensions = newExtensions.filter((ext) => !oldExtensionsUids.includes(ext.uid)) + const deletedExtensions = oldExtensions.filter((ext) => !newExtensionsUids.includes(ext.uid)) let updatedExtensions if (includeUpdated) { updatedExtensions = newExtensions.filter((ext) => { - const oldConfig = oldExtensions.find((oldExt) => oldExt.handle === ext.handle)?.configuration - const newConfig = ext.configuration - if (oldConfig === undefined) return false - return JSON.stringify(oldConfig) !== JSON.stringify(newConfig) + const oldExtension = oldExtensions.find((oldExt) => oldExt.uid === ext.uid) + if (!oldExtension) return false + const configChanged = JSON.stringify(oldExtension.configuration) !== JSON.stringify(ext.configuration) + const extensionPathChanged = oldExtension.configurationPath !== ext.configurationPath + return configChanged || extensionPathChanged }) } diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts index c0ed7dc56a8..492eef70bc3 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher-handler.ts @@ -24,8 +24,9 @@ export async function handleWatcherEvents( const appReloadNeeded = events.some((event) => eventsThatRequireReload.includes(event.type)) const otherEvents = events.filter((event) => !eventsThatRequireReload.includes(event.type)) - let appEvent: AppEvent = {app, extensionEvents: [], path: events[0].path, startTime: events[0].startTime} - if (appReloadNeeded) appEvent = await ReloadAppHandler({event: events[0], app, options, extensions: []}) + if (appReloadNeeded) return ReloadAppHandler({event: events[0], app, options, extensions: []}) + + const appEvent: AppEvent = {app, extensionEvents: [], path: events[0].path, startTime: events[0].startTime} for (const event of otherEvents) { const affectedExtensions = app.realExtensions.filter((ext) => ext.directory === event.extensionPath) @@ -73,7 +74,7 @@ const handlers: {[key in WatcherEvent['type']]: Handler} = { */ function ExtensionFolderDeletedHandler({event, app, extensions}: HandlerInput): AppEvent { const events = extensions.map((ext) => { - app.removeExtension(ext.handle) + app.removeExtension(ext.uid) return {type: EventType.Deleted, extension: ext} }) return {app, extensionEvents: events, startTime: event.startTime, path: event.path} diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts index c281fa350e3..ad2d569282d 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.ts @@ -83,7 +83,7 @@ export interface AppEvent { appWasReloaded?: boolean } -type ExtensionBuildResult = {status: 'ok'; handle: string} | {status: 'error'; error: string; handle: string} +type ExtensionBuildResult = {status: 'ok'; uid: string} | {status: 'error'; error: string; uid: string} /** * App event watcher will emit events when changes are detected in the file system. @@ -230,12 +230,12 @@ export class AppEventWatcher extends EventEmitter { const ext = extEvent.extension return useConcurrentOutputContext({outputPrefix: ext.handle, stripAnsi: false}, async () => { try { - if (this.esbuildManager.contexts?.[ext.handle]?.length) { + if (this.esbuildManager.contexts?.[ext.uid]?.length) { await this.esbuildManager.rebuildContext(ext) } else { await this.buildExtension(ext) } - extEvent.buildResult = {status: 'ok', handle: ext.handle} + extEvent.buildResult = {status: 'ok', uid: ext.uid} // eslint-disable-next-line no-catch-all/no-catch-all, @typescript-eslint/no-explicit-any } catch (error: any) { // If there is an `errors` array, it's an esbuild error, format it and log it @@ -249,7 +249,7 @@ export class AppEventWatcher extends EventEmitter { } else { this.options.stderr.write(error.message) } - extEvent.buildResult = {status: 'error', error: error.message, handle: ext.handle} + extEvent.buildResult = {status: 'error', error: error.message, uid: ext.uid} } }) }) diff --git a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts index c456e4ba83a..b0e4386a6d7 100644 --- a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts +++ b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.ts @@ -66,14 +66,14 @@ export class ESBuildContextManager { return esContext(esbuildOptions) }) - this.contexts[extension.handle] = await Promise.all(assetContextPromises.concat(mainContextPromise)) + this.contexts[extension.uid] = await Promise.all(assetContextPromises.concat(mainContextPromise)) }) await Promise.all(promises) } async rebuildContext(extension: ExtensionInstance) { - const context = this.contexts[extension.handle] + const context = this.contexts[extension.uid] if (!context) return await Promise.all(context.map((ctxt) => ctxt.rebuild())) @@ -102,10 +102,10 @@ export class ESBuildContextManager { } async deleteContexts(extensions: ExtensionInstance[]) { - const promises = extensions.map((ext) => this.contexts[ext.handle]?.map((context) => context.dispose())).flat() + const promises = extensions.map((ext) => this.contexts[ext.uid]?.map((context) => context.dispose())).flat() await Promise.all(promises) extensions.forEach((ext) => { - const {[ext.handle]: _, ...rest} = this.contexts + const {[ext.uid]: _, ...rest} = this.contexts this.contexts = rest }) } diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.ts index b4bec20c58d..975915f0ab0 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.ts @@ -15,7 +15,7 @@ const DEFAULT_DEBOUNCE_TIME_IN_MS = 200 const EXTENSION_CREATION_TIMEOUT_IN_MS = 60000 const EXTENSION_CREATION_CHECK_INTERVAL_IN_MS = 200 const FILE_DELETE_TIMEOUT_IN_MS = 500 - +const EXTENSION_MOVE_CHECK_INTERVAL_IN_MS = 100 /** * Event emitted by the file watcher * @@ -221,8 +221,11 @@ export class FileWatcher { this.pushEvent({type: 'app_config_deleted', path, extensionPath, startTime}) } else if (isExtensionToml) { // When a toml is deleted, we can consider every extension in that folder was deleted. - this.extensionPaths = this.extensionPaths.filter((extPath) => extPath !== extensionPath) - this.pushEvent({type: 'extension_folder_deleted', path: extensionPath, extensionPath, startTime}) + // We need to wait in case this is actually just moving folders around, not deleting them. + setTimeout(() => { + this.extensionPaths = this.extensionPaths.filter((extPath) => extPath !== extensionPath) + this.pushEvent({type: 'extension_folder_deleted', path: extensionPath, extensionPath, startTime}) + }, EXTENSION_MOVE_CHECK_INTERVAL_IN_MS) } else { setTimeout(() => { // If the extensionPath is not longer in the list, the extension was deleted while the timeout was running. diff --git a/packages/app/src/cli/services/dev/processes/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session.ts index 83dd54392d7..3f437892cec 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session.ts @@ -172,6 +172,7 @@ async function handleDevSessionResult( async function printActionRequiredMessages(processOptions: DevSessionProcessOptions, event?: AppEvent) { if (!event) return const extensionEvents = event.extensionEvents ?? [] + const warningMessages = getArrayRejectingUndefined( await Promise.all( extensionEvents.map((eve) => From 0424583f059585f952bfa48cca75c98382caeb29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 3 Jan 2025 12:26:41 +0100 Subject: [PATCH 12/50] Update tests --- .../dev/app-events/app-diffing.test.ts | 9 ++- .../dev/app-events/app-event-watcher.test.ts | 68 +++++++++++++------ .../app-events/app-watcher-esbuild.test.ts | 32 ++++++--- 3 files changed, 74 insertions(+), 35 deletions(-) diff --git a/packages/app/src/cli/services/dev/app-events/app-diffing.test.ts b/packages/app/src/cli/services/dev/app-events/app-diffing.test.ts index d49ff204b4a..0ae2bc17027 100644 --- a/packages/app/src/cli/services/dev/app-events/app-diffing.test.ts +++ b/packages/app/src/cli/services/dev/app-events/app-diffing.test.ts @@ -2,8 +2,13 @@ import {appDiff} from './app-diffing.js' import {testApp, testAppConfigExtensions, testUIExtension} from '../../../models/app/app.test-data.js' import {describe, expect, test} from 'vitest' -const extension1 = await testUIExtension({type: 'ui_extension', handle: 'h1', directory: '/extensions/ui_extension_1'}) -const extension2 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_2'}) +const extension1 = await testUIExtension({ + type: 'ui_extension', + handle: 'h1', + directory: '/extensions/ui_extension_1', + uid: 'uid1', +}) +const extension2 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_2', uid: 'uid2'}) const posExtension = await testAppConfigExtensions() const posExtensionWithDifferentConfig = await testAppConfigExtensions(true) diff --git a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts index 0423ea1d963..1cfceafad94 100644 --- a/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/app-event-watcher.test.ts @@ -17,15 +17,30 @@ import {AbortSignal, AbortController} from '@shopify/cli-kit/node/abort' import {flushPromises} from '@shopify/cli-kit/node/promises' import {inTemporaryDirectory} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' +import {nonRandomUUID} from '@shopify/cli-kit/node/crypto' import {Writable} from 'stream' vi.mock('../../../models/app/loader.js') vi.mock('./app-watcher-esbuild.js') // Extensions 1 and 1B simulate extensions defined in the same directory (same toml) -const extension1 = await testUIExtension({type: 'ui_extension', handle: 'h1', directory: '/extensions/ui_extension_1'}) -const extension1B = await testUIExtension({type: 'ui_extension', handle: 'h2', directory: '/extensions/ui_extension_1'}) -const extension2 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_2'}) +const extension1 = await testUIExtension({ + type: 'ui_extension', + handle: 'h1', + directory: '/extensions/ui_extension_1', + uid: 'uid1', +}) +const extension1B = await testUIExtension({ + type: 'ui_extension', + handle: 'h2', + directory: '/extensions/ui_extension_1', + uid: 'uid1B', +}) +const extension2 = await testUIExtension({ + type: 'ui_extension', + directory: '/extensions/ui_extension_2', + uid: 'uid2', +}) const flowExtension = await testFlowActionExtension('/extensions/flow_action') const posExtension = await testAppConfigExtensions() const appAccessExtension = await testAppAccessConfigExtension() @@ -37,12 +52,14 @@ const extension1Updated = await testUIExtension({ name: 'updated_name1', handle: 'h1', directory: '/extensions/ui_extension_1', + uid: 'uid1', }) const extension1BUpdated = await testUIExtension({ type: 'ui_extension', name: 'updated_name1B', handle: 'h2', directory: '/extensions/ui_extension_1', + uid: 'uid1B', }) const posExtensionUpdated = await testAppConfigExtensions(true) @@ -105,9 +122,7 @@ const testCases: TestCase[] = [ }, initialExtensions: [extension1, posExtension], finalExtensions: [extension1, extension2, posExtension], - extensionEvents: [ - {type: EventType.Created, extension: extension2, buildResult: {status: 'ok', handle: 'test-ui-extension'}}, - ], + extensionEvents: [{type: EventType.Created, extension: extension2, buildResult: {status: 'ok', uid: 'uid2'}}], needsAppReload: true, }, { @@ -120,7 +135,7 @@ const testCases: TestCase[] = [ }, initialExtensions: [extension1, extension2, posExtension], finalExtensions: [extension1, extension2, posExtension], - extensionEvents: [{type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', handle: 'h1'}}], + extensionEvents: [{type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', uid: 'uid1'}}], }, { name: 'file_updated affecting a single extension', @@ -132,7 +147,7 @@ const testCases: TestCase[] = [ }, initialExtensions: [extension1, extension2, posExtension], finalExtensions: [extension1, extension2, posExtension], - extensionEvents: [{type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', handle: 'h1'}}], + extensionEvents: [{type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', uid: 'uid1'}}], }, { name: 'file_deleted affecting a single extension', @@ -144,7 +159,7 @@ const testCases: TestCase[] = [ }, initialExtensions: [extension1, extension2, posExtension], finalExtensions: [extension1, extension2, posExtension], - extensionEvents: [{type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', handle: 'h1'}}], + extensionEvents: [{type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', uid: 'uid1'}}], }, { name: 'file_created affecting a multiple extensions', @@ -157,8 +172,8 @@ const testCases: TestCase[] = [ initialExtensions: [extension1, extension1B, extension2, posExtension], finalExtensions: [extension1, extension1B, extension2, posExtension], extensionEvents: [ - {type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', handle: 'h1'}}, - {type: EventType.Updated, extension: extension1B, buildResult: {status: 'ok', handle: 'h2'}}, + {type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', uid: 'uid1'}}, + {type: EventType.Updated, extension: extension1B, buildResult: {status: 'ok', uid: 'uid1B'}}, ], }, { @@ -172,8 +187,8 @@ const testCases: TestCase[] = [ initialExtensions: [extension1, extension1B, extension2, posExtension], finalExtensions: [extension1, extension1B, extension2, posExtension], extensionEvents: [ - {type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', handle: 'h1'}}, - {type: EventType.Updated, extension: extension1B, buildResult: {status: 'ok', handle: 'h2'}}, + {type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', uid: 'uid1'}}, + {type: EventType.Updated, extension: extension1B, buildResult: {status: 'ok', uid: 'uid1B'}}, ], }, { @@ -187,8 +202,8 @@ const testCases: TestCase[] = [ initialExtensions: [extension1, extension1B, extension2, posExtension], finalExtensions: [extension1, extension1B, extension2, posExtension], extensionEvents: [ - {type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', handle: 'h1'}}, - {type: EventType.Updated, extension: extension1B, buildResult: {status: 'ok', handle: 'h2'}}, + {type: EventType.Updated, extension: extension1, buildResult: {status: 'ok', uid: 'uid1'}}, + {type: EventType.Updated, extension: extension1B, buildResult: {status: 'ok', uid: 'uid1B'}}, ], }, { @@ -202,9 +217,17 @@ const testCases: TestCase[] = [ initialExtensions: [extension1, extension2, posExtension, webhookExtension], finalExtensions: [extension1, extension2, posExtensionUpdated, appAccessExtension], extensionEvents: [ - {type: EventType.Updated, extension: posExtensionUpdated, buildResult: {status: 'ok', handle: 'point-of-sale'}}, + { + type: EventType.Updated, + extension: posExtensionUpdated, + buildResult: {status: 'ok', uid: nonRandomUUID('point-of-sale')}, + }, {type: EventType.Deleted, extension: webhookExtension}, - {type: EventType.Created, extension: appAccessExtension, buildResult: {status: 'ok', handle: 'app-access'}}, + { + type: EventType.Created, + extension: appAccessExtension, + buildResult: {status: 'ok', uid: nonRandomUUID('app-access')}, + }, ], needsAppReload: true, }, @@ -219,8 +242,8 @@ const testCases: TestCase[] = [ initialExtensions: [extension1, extension1B, extension2], finalExtensions: [extension1Updated, extension1BUpdated, extension2], extensionEvents: [ - {type: EventType.Updated, extension: extension1Updated, buildResult: {status: 'ok', handle: 'h1'}}, - {type: EventType.Updated, extension: extension1BUpdated, buildResult: {status: 'ok', handle: 'h2'}}, + {type: EventType.Updated, extension: extension1Updated, buildResult: {status: 'ok', uid: 'uid1'}}, + {type: EventType.Updated, extension: extension1BUpdated, buildResult: {status: 'ok', uid: 'uid1B'}}, ], needsAppReload: true, }, @@ -295,7 +318,7 @@ describe('app-event-watcher', () => { const initialEvents = app.realExtensions.map((eve) => ({ type: EventType.Updated, extension: eve, - buildResult: {status: 'ok', handle: eve.handle}, + buildResult: {status: 'ok', uid: eve.uid}, })) expect(emitSpy).toHaveBeenCalledWith('ready', { app, @@ -406,8 +429,9 @@ describe('app-event-watcher', () => { class MockESBuildContextManager extends ESBuildContextManager { contexts = { // The keys are the extension handles, the values are the ESBuild contexts mocked - h1: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}], - h2: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}], + uid1: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}], + uid1B: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}], + uid2: [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}], 'test-ui-extension': [{rebuild: vi.fn(), watch: vi.fn(), serve: vi.fn(), cancel: vi.fn(), dispose: vi.fn()}], } diff --git a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.test.ts b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.test.ts index 4c1867eb43d..749ffd5492e 100644 --- a/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.test.ts +++ b/packages/app/src/cli/services/dev/app-events/app-watcher-esbuild.test.ts @@ -12,8 +12,17 @@ vi.mock('@luckycatfactory/esbuild-graphql-loader', () => ({ }, })) -const extension1 = await testUIExtension({type: 'ui_extension', handle: 'h1', directory: '/extensions/ui_extension_1'}) -const extension2 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_2'}) +const extension1 = await testUIExtension({ + type: 'ui_extension', + handle: 'h1', + directory: '/extensions/ui_extension_1', + uid: 'uid1', +}) +const extension2 = await testUIExtension({ + type: 'ui_extension', + directory: '/extensions/ui_extension_2', + uid: 'uid2', +}) describe('app-watcher-esbuild', () => { const options: DevAppWatcherOptions = { @@ -31,8 +40,8 @@ describe('app-watcher-esbuild', () => { await manager.createContexts(extensions) // Then - expect(manager.contexts).toHaveProperty('h1') - expect(manager.contexts).toHaveProperty('test-ui-extension') + expect(manager.contexts).toHaveProperty('uid1') + expect(manager.contexts).toHaveProperty('uid2') }) test('creating multiple contexts for the same extension', async () => { @@ -44,6 +53,7 @@ describe('app-watcher-esbuild', () => { } const manager = new ESBuildContextManager(options) const extension = await testUIExtension({ + uid: 'conditional-extension-uid', configuration: { ...extension2.configuration, handle: 'conditional-extension', @@ -75,8 +85,8 @@ describe('app-watcher-esbuild', () => { await manager.createContexts([extension]) // Then - expect(manager.contexts).toHaveProperty('conditional-extension') - expect(manager.contexts['conditional-extension']).toHaveLength(2) + expect(manager.contexts).toHaveProperty('conditional-extension-uid') + expect(manager.contexts['conditional-extension-uid']).toHaveLength(2) }) test('deleting contexts', async () => { @@ -89,8 +99,8 @@ describe('app-watcher-esbuild', () => { await manager.deleteContexts([extension1]) // Then - expect(manager.contexts).not.toHaveProperty('h1') - expect(manager.contexts).toHaveProperty('test-ui-extension') + expect(manager.contexts).not.toHaveProperty('uid1') + expect(manager.contexts).toHaveProperty('uid2') }) test('updating contexts with an app event', async () => { @@ -112,15 +122,15 @@ describe('app-watcher-esbuild', () => { await manager.updateContexts(appEvent) // Then - expect(manager.contexts).toHaveProperty('h1') - expect(manager.contexts).not.toHaveProperty('test-ui-extension') + expect(manager.contexts).toHaveProperty('uid1') + expect(manager.contexts).not.toHaveProperty('uid2') }) test('rebuilding contexts', async () => { // Given const manager = new ESBuildContextManager(options) await manager.createContexts([extension1]) - const spyContext = vi.spyOn(manager.contexts.h1![0]!, 'rebuild').mockResolvedValue({} as any) + const spyContext = vi.spyOn(manager.contexts.uid1![0]!, 'rebuild').mockResolvedValue({} as any) const spyCopy = vi.spyOn(fs, 'copyFile').mockResolvedValue() // When From f6526a9dda88ba89f9de805033200db525576d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 3 Jan 2025 12:31:48 +0100 Subject: [PATCH 13/50] Some cleanup in tests --- .../models/extensions/extension-instance.ts | 2 +- .../dev/app-events/app-diffing.test.ts | 7 +----- .../dev/app-events/app-event-watcher.test.ts | 22 +++---------------- .../app-events/app-watcher-esbuild.test.ts | 15 +++---------- 4 files changed, 8 insertions(+), 38 deletions(-) diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 481352a7e2c..9bd3a563f81 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -370,7 +370,7 @@ export class ExtensionInstance ({ }, })) -const extension1 = await testUIExtension({ - type: 'ui_extension', - handle: 'h1', - directory: '/extensions/ui_extension_1', - uid: 'uid1', -}) -const extension2 = await testUIExtension({ - type: 'ui_extension', - directory: '/extensions/ui_extension_2', - uid: 'uid2', -}) +const extension1 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_1', uid: 'uid1'}) +const extension2 = await testUIExtension({type: 'ui_extension', directory: '/extensions/ui_extension_2', uid: 'uid2'}) describe('app-watcher-esbuild', () => { const options: DevAppWatcherOptions = { @@ -138,6 +129,6 @@ describe('app-watcher-esbuild', () => { // Then expect(spyContext).toHaveBeenCalled() - expect(spyCopy).toHaveBeenCalledWith('/path/to/output/h1/dist', '/extensions/ui_extension_1/dist') + expect(spyCopy).toHaveBeenCalledWith('/path/to/output/uid1/dist', '/extensions/ui_extension_1/dist') }) }) From d28d0309cf43ee42fc9b8c603603d26c621a407d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 3 Jan 2025 12:50:18 +0100 Subject: [PATCH 14/50] Update more tests to use uid --- .../app/src/cli/services/dev/update-extension.test.ts | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/app/src/cli/services/dev/update-extension.test.ts b/packages/app/src/cli/services/dev/update-extension.test.ts index 6a2f0ac6fd2..c14117342dc 100644 --- a/packages/app/src/cli/services/dev/update-extension.test.ts +++ b/packages/app/src/cli/services/dev/update-extension.test.ts @@ -30,7 +30,6 @@ vi.mock('../../models/app/loader.js', async () => { const apiKey = 'mock-api-key' const registrationId = 'mock-registration-id' -const handle = 'mock-handle' const stdout = {write: vi.fn()} as any const stderr = {write: vi.fn()} as any @@ -42,16 +41,17 @@ describe('updateExtensionDraft()', () => { runtime_context: 'strict', settings: {type: 'object'}, type: 'web_pixel_extension', - handle, + handle: 'mock-handle', } as any const mockExtension = await testUIExtension({ devUUID: '1', configuration, directory: tmpDir, + uid: 'uid1', }) - await mkdir(joinPath(tmpDir, 'mock-handle', 'dist')) + await mkdir(joinPath(tmpDir, 'uid1', 'dist')) const outputPath = mockExtension.getOutputPathForDirectory(tmpDir) await writeFile(outputPath, 'test content') @@ -69,7 +69,7 @@ describe('updateExtensionDraft()', () => { expect(developerPlatformClient.updateExtension).toHaveBeenCalledWith({ apiKey, context: '', - handle, + handle: 'mock-handle', registrationId, config: '{"runtime_context":"strict","runtime_configuration_definition":{"type":"object"},"serialized_script":"dGVzdCBjb250ZW50"}', @@ -259,9 +259,10 @@ describe('updateExtensionDraft()', () => { devUUID: '1', directory: tmpDir, type: 'web_pixel_extension', + uid: 'uid1', }) - await mkdir(joinPath(tmpDir, mockExtension.handle, 'dist')) + await mkdir(joinPath(tmpDir, mockExtension.uid, 'dist')) const outputPath = mockExtension.getOutputPathForDirectory(tmpDir) await writeFile(outputPath, 'test content') From 62ac685b8de0ce8bffe15018f6da110aff291496 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 3 Jan 2025 12:57:04 +0100 Subject: [PATCH 15/50] Update more tests to use uid --- packages/app/src/cli/services/dev/update-extension.test.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/app/src/cli/services/dev/update-extension.test.ts b/packages/app/src/cli/services/dev/update-extension.test.ts index c14117342dc..d79a6326eba 100644 --- a/packages/app/src/cli/services/dev/update-extension.test.ts +++ b/packages/app/src/cli/services/dev/update-extension.test.ts @@ -30,6 +30,7 @@ vi.mock('../../models/app/loader.js', async () => { const apiKey = 'mock-api-key' const registrationId = 'mock-registration-id' +const handle = 'mock-handle' const stdout = {write: vi.fn()} as any const stderr = {write: vi.fn()} as any @@ -41,7 +42,7 @@ describe('updateExtensionDraft()', () => { runtime_context: 'strict', settings: {type: 'object'}, type: 'web_pixel_extension', - handle: 'mock-handle', + handle, } as any const mockExtension = await testUIExtension({ @@ -69,7 +70,7 @@ describe('updateExtensionDraft()', () => { expect(developerPlatformClient.updateExtension).toHaveBeenCalledWith({ apiKey, context: '', - handle: 'mock-handle', + handle, registrationId, config: '{"runtime_context":"strict","runtime_configuration_definition":{"type":"object"},"serialized_script":"dGVzdCBjb250ZW50"}', From e433cf61e29ff1c3a3356bf53826053c9510eea4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 3 Jan 2025 15:28:53 +0100 Subject: [PATCH 16/50] Clean up --- .../app/src/cli/services/dev/app-events/file-watcher.ts | 9 +++------ .../app/src/cli/services/dev/processes/dev-session.ts | 1 - 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.ts index 975915f0ab0..b4bec20c58d 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.ts @@ -15,7 +15,7 @@ const DEFAULT_DEBOUNCE_TIME_IN_MS = 200 const EXTENSION_CREATION_TIMEOUT_IN_MS = 60000 const EXTENSION_CREATION_CHECK_INTERVAL_IN_MS = 200 const FILE_DELETE_TIMEOUT_IN_MS = 500 -const EXTENSION_MOVE_CHECK_INTERVAL_IN_MS = 100 + /** * Event emitted by the file watcher * @@ -221,11 +221,8 @@ export class FileWatcher { this.pushEvent({type: 'app_config_deleted', path, extensionPath, startTime}) } else if (isExtensionToml) { // When a toml is deleted, we can consider every extension in that folder was deleted. - // We need to wait in case this is actually just moving folders around, not deleting them. - setTimeout(() => { - this.extensionPaths = this.extensionPaths.filter((extPath) => extPath !== extensionPath) - this.pushEvent({type: 'extension_folder_deleted', path: extensionPath, extensionPath, startTime}) - }, EXTENSION_MOVE_CHECK_INTERVAL_IN_MS) + this.extensionPaths = this.extensionPaths.filter((extPath) => extPath !== extensionPath) + this.pushEvent({type: 'extension_folder_deleted', path: extensionPath, extensionPath, startTime}) } else { setTimeout(() => { // If the extensionPath is not longer in the list, the extension was deleted while the timeout was running. diff --git a/packages/app/src/cli/services/dev/processes/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session.ts index 3f437892cec..83dd54392d7 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session.ts @@ -172,7 +172,6 @@ async function handleDevSessionResult( async function printActionRequiredMessages(processOptions: DevSessionProcessOptions, event?: AppEvent) { if (!event) return const extensionEvents = event.extensionEvents ?? [] - const warningMessages = getArrayRejectingUndefined( await Promise.all( extensionEvents.map((eve) => From 564c391c59dac9e3e224bcff93c885f9b2eab661 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Mon, 13 Jan 2025 15:52:16 +0100 Subject: [PATCH 17/50] Rename bundle to bundle-for-release: --- package.json | 2 +- shipit.nightly.yml | 2 +- shipit.production.yml | 2 +- shipit.stable.yml.sample | 2 +- shipit.stable_3_71.yml | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index 949485bfc03..5bb5a2cf985 100644 --- a/package.json +++ b/package.json @@ -22,7 +22,7 @@ "type-check": "nx run-many --target=type-check --all --skip-nx-cache", "type-check:affected": "nx affected --target=type-check", "build": "nx run-many --target=build --all --skip-nx-cache", - "bundle": "nx run-many --target=bundle --all --skip-nx-cache", + "bundle-for-release": "nx run-many --target=bundle --all --skip-nx-cache", "build:affected": "nx affected --target=build", "refresh-templates": "nx run-many --target=refresh-templates --all --skip-nx-cache", "refresh-manifests": "nx run-many --target=refresh-manifests --all --skip-nx-cache && bin/prettify-manifests.js && pnpm refresh-readme", diff --git a/shipit.nightly.yml b/shipit.nightly.yml index 5831ebb8c12..3a8e682d816 100644 --- a/shipit.nightly.yml +++ b/shipit.nightly.yml @@ -22,7 +22,7 @@ deploy: - bash -i -c "if [ -f '.changeset/pre.json' ]; then npm_config_loglevel=verbose pnpm changeset pre exit; fi" - bash -i -c "npm_config_loglevel=verbose pnpm changeset version --snapshot nightly" - bash -i -c "npm_config_loglevel=verbose pnpm changeset-manifests" - - bash -i -c "npm_config_loglevel=verbose NODE_ENV=production pnpm bundle" + - bash -i -c "npm_config_loglevel=verbose NODE_ENV=production pnpm bundle-for-release" - bash -i -c "npm_config_loglevel=verbose node bin/create-cli-duplicate-package.js" - bash -i -c "npm_config_loglevel=verbose pnpm changeset publish --tag nightly" - bash -i -c "./bin/package.js" diff --git a/shipit.production.yml b/shipit.production.yml index 1cd22385aa2..8f73d6cf4a8 100644 --- a/shipit.production.yml +++ b/shipit.production.yml @@ -12,7 +12,7 @@ dependencies: deploy: override: - bash -i -c "npm_config_loglevel=verbose pnpm clean" - - bash -i -c "npm_config_loglevel=verbose NODE_ENV=production pnpm bundle" + - bash -i -c "npm_config_loglevel=verbose NODE_ENV=production pnpm bundle-for-release" - bash -i -c "npm_config_loglevel=verbose node bin/create-cli-duplicate-package.js" - bash -i -c "npm_config_loglevel=verbose pnpm changeset publish" - bash -i -c "./bin/package.js" diff --git a/shipit.stable.yml.sample b/shipit.stable.yml.sample index 478d2dbd540..90a3ca6bff4 100644 --- a/shipit.stable.yml.sample +++ b/shipit.stable.yml.sample @@ -12,7 +12,7 @@ dependencies: deploy: override: - bash -i -c "npm_config_loglevel=verbose pnpm clean" - - bash -i -c "npm_config_loglevel=verbose NODE_ENV=production pnpm bundle" + - bash -i -c "npm_config_loglevel=verbose NODE_ENV=production pnpm bundle-for-release" - bash -i -c "npm_config_loglevel=verbose node bin/create-cli-duplicate-package.js" - bash -i -c "npm_config_loglevel=verbose pnpm changeset publish" # When this is no longer the latest stable version, do 2 things: diff --git a/shipit.stable_3_71.yml b/shipit.stable_3_71.yml index 478d2dbd540..90a3ca6bff4 100644 --- a/shipit.stable_3_71.yml +++ b/shipit.stable_3_71.yml @@ -12,7 +12,7 @@ dependencies: deploy: override: - bash -i -c "npm_config_loglevel=verbose pnpm clean" - - bash -i -c "npm_config_loglevel=verbose NODE_ENV=production pnpm bundle" + - bash -i -c "npm_config_loglevel=verbose NODE_ENV=production pnpm bundle-for-release" - bash -i -c "npm_config_loglevel=verbose node bin/create-cli-duplicate-package.js" - bash -i -c "npm_config_loglevel=verbose pnpm changeset publish" # When this is no longer the latest stable version, do 2 things: From b02d1f975f6ab90c64865027736c627b715adeb4 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Wed, 8 Jan 2025 13:27:55 +0200 Subject: [PATCH 18/50] Rework app info to use components --- packages/app/src/cli/commands/app/info.ts | 20 +- packages/app/src/cli/services/info.ts | 207 ++++++++++-------- .../src/private/node/ui/components/Alert.tsx | 9 +- .../private/node/ui/components/FatalError.tsx | 7 +- .../node/ui/components/TabularData.tsx | 36 +++ 5 files changed, 180 insertions(+), 99 deletions(-) create mode 100644 packages/cli-kit/src/private/node/ui/components/TabularData.tsx diff --git a/packages/app/src/cli/commands/app/info.ts b/packages/app/src/cli/commands/app/info.ts index c94727d0abc..be003403689 100644 --- a/packages/app/src/cli/commands/app/info.ts +++ b/packages/app/src/cli/commands/app/info.ts @@ -5,6 +5,7 @@ import {linkedAppContext} from '../../services/app-context.js' import {Flags} from '@oclif/core' import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' import {outputInfo} from '@shopify/cli-kit/node/output' +import {renderInfo} from '@shopify/cli-kit/node/ui' export default class AppInfo extends AppCommand { static summary = 'Print basic information about your app and extensions.' @@ -40,14 +41,17 @@ export default class AppInfo extends AppCommand { userProvidedConfigName: flags.config, unsafeReportMode: true, }) - outputInfo( - await info(app, remoteApp, organization, { - format: (flags.json ? 'json' : 'text') as Format, - webEnv: flags['web-env'], - configName: flags.config, - developerPlatformClient, - }), - ) + const results = await info(app, remoteApp, organization, { + format: (flags.json ? 'json' : 'text') as Format, + webEnv: flags['web-env'], + configName: flags.config, + developerPlatformClient, + }) + if (typeof results === 'string' || 'value' in results) { + outputInfo(results) + } else { + renderInfo({customSections: results}) + } if (app.errors) process.exit(2) return {app} diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index 3adf9ea510e..7d2835354bd 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -6,11 +6,19 @@ import {configurationFileNames} from '../constants.js' import {ExtensionInstance} from '../models/extensions/extension-instance.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' -import {OutputMessage, outputContent, outputToken, formatSection, stringifyMessage} from '@shopify/cli-kit/node/output' +import { + OutputMessage, + formatPackageManagerCommand, + outputContent, + outputToken, + stringifyMessage, +} from '@shopify/cli-kit/node/output' +import {InlineToken, renderInfo} from '@shopify/cli-kit/node/ui' import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' +type CustomSection = Exclude[0]['customSections'], undefined>[number] + export type Format = 'json' | 'text' export interface InfoOptions { format: Format @@ -19,17 +27,13 @@ export interface InfoOptions { webEnv: boolean developerPlatformClient: DeveloperPlatformClient } -interface Configurable { - type: string - externalType: string -} export async function info( app: AppLinkedInterface, remoteApp: OrganizationApp, organization: Organization, options: InfoOptions, -): Promise { +): Promise { if (options.webEnv) { return infoWeb(app, remoteApp, organization, options) } else { @@ -50,7 +54,7 @@ async function infoApp( app: AppLinkedInterface, remoteApp: OrganizationApp, options: InfoOptions, -): Promise { +): Promise { if (options.format === 'json') { const extensionsInfo = withPurgedSchemas(app.allExtensions.filter((ext) => ext.isReturnedAsInfo())) let appWithSupportedExtensions = { @@ -119,30 +123,22 @@ class AppInfo { this.options = options } - async output(): Promise { - const sections: [string, string][] = [ - await this.devConfigsSection(), + async output(): Promise { + return [ + ...(await this.devConfigsSection()), this.projectSettingsSection(), - await this.appComponentsSection(), + ...(await this.appComponentsSection()), await this.systemInfoSection(), ] - return sections.map((sectionContents: [string, string]) => formatSection(...sectionContents)).join('\n\n') } - async devConfigsSection(): Promise<[string, string]> { - const title = `Current app configuration` - const postscript = outputContent`💡 To change these, run ${outputToken.packagejsonScript( - this.app.packageManager, - 'dev', - '--reset', - )}`.value - + async devConfigsSection(): Promise { let updateUrls = NOT_CONFIGURED_TEXT if (this.app.configuration.build?.automatically_update_urls_on_dev !== undefined) { updateUrls = this.app.configuration.build.automatically_update_urls_on_dev ? 'Yes' : 'No' } - let partnersAccountInfo = ['Partners account', 'unknown'] + let partnersAccountInfo: [string, string] = ['Partners account', 'unknown'] const retrievedAccountInfo = await this.options.developerPlatformClient.accountInfo() if (isServiceAccount(retrievedAccountInfo)) { partnersAccountInfo = ['Service account', retrievedAccountInfo.orgName] @@ -150,108 +146,131 @@ class AppInfo { partnersAccountInfo = ['Partners account', retrievedAccountInfo.email] } - const lines = [ - ['Configuration file', basename(this.app.configuration.path) || configurationFileNames.app], - ['App name', this.remoteApp.title || NOT_CONFIGURED_TEXT], - ['Client ID', this.remoteApp.apiKey || NOT_CONFIGURED_TEXT], - ['Access scopes', getAppScopes(this.app.configuration)], - ['Dev store', this.app.configuration.build?.dev_store_url || NOT_CONFIGURED_TEXT], - ['Update URLs', updateUrls], - partnersAccountInfo, + return [ + this.tableSection( + 'Current app configuration', + [ + ['Configuration file', {filePath: basename(this.app.configuration.path) || configurationFileNames.app}], + ['App name', this.remoteApp.title || NOT_CONFIGURED_TEXT], + ['Client ID', this.remoteApp.apiKey || NOT_CONFIGURED_TEXT], + ['Access scopes', getAppScopes(this.app.configuration)], + ['Dev store', this.app.configuration.build?.dev_store_url || NOT_CONFIGURED_TEXT], + ['Update URLs', updateUrls], + partnersAccountInfo, + ], + {isFirstItem: true}, + ), + { + body: [ + '💡 To change these, run', + {command: formatPackageManagerCommand(this.app.packageManager, 'dev', '--reset')}, + ], + }, ] - return [title, `${linesToColumns(lines)}\n\n${postscript}`] } - projectSettingsSection(): [string, string] { - const title = 'Your Project' - const lines = [['Root location', this.app.directory]] - return [title, linesToColumns(lines)] + projectSettingsSection(): CustomSection { + return this.tableSection('Your Project', [['Root location', {filePath: this.app.directory}]]) } - async appComponentsSection(): Promise<[string, string]> { - const title = 'Directory Components' - - let body = this.webComponentsSection() - - function augmentWithExtensions( - extensions: TExtension[], - outputFormatter: (extension: TExtension) => string, - ) { - const types = new Set(extensions.map((ext) => ext.type)) - types.forEach((extensionType: string) => { - const relevantExtensions = extensions.filter((extension: TExtension) => extension.type === extensionType) - if (relevantExtensions[0]) { - body += `\n\n${outputContent`${outputToken.subheading(relevantExtensions[0].externalType)}`.value}` - relevantExtensions.forEach((extension: TExtension) => { - body += outputFormatter(extension) - }) - } - }) - } + async appComponentsSection(): Promise { + const webComponentsSection = this.webComponentsSection() const supportedExtensions = this.app.allExtensions.filter((ext) => ext.isReturnedAsInfo()) - augmentWithExtensions(supportedExtensions, this.extensionSubSection.bind(this)) + const extensionsSections = this.extensionsSections(supportedExtensions) + let errorsSection: CustomSection | undefined if (this.app.errors?.isEmpty() === false) { - body += `\n\n${outputContent`${outputToken.subheading('Extensions with errors')}`.value}` - supportedExtensions.forEach((extension) => { - body += this.invalidExtensionSubSection(extension) - }) + errorsSection = this.tableSection( + 'Extensions with errors', + ( + supportedExtensions + .map((extension) => this.invalidExtensionSubSection(extension)) + .filter((data) => typeof data !== 'undefined') as [string, InlineToken][][] + ).flat(), + ) } - return [title, body] + + return [ + { + title: '\nDirectory components'.toUpperCase(), + body: '', + }, + ...(webComponentsSection ? [webComponentsSection] : []), + ...extensionsSections, + ...(errorsSection ? [errorsSection] : []), + ] } - webComponentsSection(): string { + webComponentsSection(): CustomSection | undefined { const errors: OutputMessage[] = [] - const subtitle = outputContent`${outputToken.subheading('web')}`.value - const toplevel = ['📂 web', ''] - const sublevels: [string, string][] = [] + const sublevels: [string, InlineToken][] = [] + if (!this.app.webs[0]) return this.app.webs.forEach((web) => { if (web.configuration) { if (web.configuration.name) { const {name, roles} = web.configuration - sublevels.push([` 📂 ${name} (${roles.join(',')})`, relativePath(this.app.directory, web.directory)]) + sublevels.push([ + ` 📂 ${name} (${roles.join(',')})`, + {filePath: relativePath(this.app.directory, web.directory)}, + ]) } else { web.configuration.roles.forEach((role) => { - sublevels.push([` 📂 ${role}`, relativePath(this.app.directory, web.directory)]) + sublevels.push([` 📂 ${role}`, {filePath: relativePath(this.app.directory, web.directory)}]) }) } } else { - sublevels.push([` 📂 ${UNKNOWN_TEXT}`, relativePath(this.app.directory, web.directory)]) + sublevels.push([` 📂 ${UNKNOWN_TEXT}`, {filePath: relativePath(this.app.directory, web.directory)}]) } if (this.app.errors) { const error = this.app.errors.getError(`${web.directory}/${configurationFileNames.web}`) if (error) errors.push(error) } }) - let errorContent = `\n${errors.map((error) => this.formattedError(error)).join('\n')}` - if (errorContent.trim() === '') errorContent = '' - return `${subtitle}\n${linesToColumns([toplevel, ...sublevels])}${errorContent}` + return this.subtableSection('web', [ + ['📂 web', ''], + ...sublevels, + ...errors.map((error): [string, InlineToken] => ['', {error: this.formattedError(error)}]), + ]) } - extensionSubSection(extension: ExtensionInstance): string { + extensionsSections(extensions: ExtensionInstance[]): CustomSection[] { + const types = Array.from(new Set(extensions.map((ext) => ext.type))) + return types + .map((extensionType: string): CustomSection | undefined => { + const relevantExtensions = extensions.filter((extension: ExtensionInstance) => extension.type === extensionType) + if (relevantExtensions[0]) { + return this.subtableSection( + relevantExtensions[0].externalType, + relevantExtensions.map((ext) => this.extensionSubSection(ext)).flat(), + ) + } + }) + .filter((section: CustomSection | undefined) => section !== undefined) as CustomSection[] + } + + extensionSubSection(extension: ExtensionInstance): [string, InlineToken][] { const config = extension.configuration - const details = [ - [`📂 ${extension.handle}`, relativePath(this.app.directory, extension.directory)], - [' config file', relativePath(extension.directory, extension.configurationPath)], + const details: [string, InlineToken][] = [ + [`📂 ${extension.handle}`, {filePath: relativePath(this.app.directory, extension.directory)}], + [' config file', {filePath: relativePath(extension.directory, extension.configurationPath)}], ] if (config && config.metafields?.length) { details.push([' metafields', `${config.metafields.length}`]) } - return `\n${linesToColumns(details)}` + return details } - invalidExtensionSubSection(extension: ExtensionInstance): string { + invalidExtensionSubSection(extension: ExtensionInstance): [string, InlineToken][] | undefined { const error = this.app.errors?.getError(extension.configurationPath) - if (!error) return '' - const details = [ - [`📂 ${extension.handle}`, relativePath(this.app.directory, extension.directory)], - [' config file', relativePath(extension.directory, extension.configurationPath)], + if (!error) return + return [ + [`📂 ${extension.handle}`, {filePath: relativePath(this.app.directory, extension.directory)}], + [' config file', {filePath: relativePath(extension.directory, extension.configurationPath)}], + [' message', {error: this.formattedError(error)}], ] - const formattedError = this.formattedError(error) - return `\n${linesToColumns(details)}\n${formattedError}` } formattedError(str: OutputMessage): string { @@ -260,16 +279,28 @@ class AppInfo { return outputContent`${outputToken.errorText(errorLines.join('\n'))}`.value } - async systemInfoSection(): Promise<[string, string]> { - const title = 'Tooling and System' + async systemInfoSection(): Promise { const {platform, arch} = platformAndArch() - const lines: string[][] = [ + return this.tableSection('Tooling and System', [ ['Shopify CLI', CLI_KIT_VERSION], ['Package manager', this.app.packageManager], ['OS', `${platform}-${arch}`], ['Shell', process.env.SHELL || 'unknown'], ['Node version', process.version], - ] - return [title, linesToColumns(lines)] + ]) + } + + tableSection(title: string, rows: [string, InlineToken][], {isFirstItem = false} = {}): CustomSection { + return { + title: `${isFirstItem ? '' : '\n'}${title.toUpperCase()}\n`, + body: {tabularData: rows, firstColumnSubdued: true}, + } + } + + subtableSection(title: string, rows: [string, InlineToken][]): CustomSection { + return { + title, + body: {tabularData: rows, firstColumnSubdued: true}, + } } } diff --git a/packages/cli-kit/src/private/node/ui/components/Alert.tsx b/packages/cli-kit/src/private/node/ui/components/Alert.tsx index f732f33b833..fc573cd55dc 100644 --- a/packages/cli-kit/src/private/node/ui/components/Alert.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Alert.tsx @@ -2,12 +2,13 @@ import {Banner, BannerType} from './Banner.js' import {Link} from './Link.js' import {List} from './List.js' import {BoldToken, InlineToken, LinkToken, TokenItem, TokenizedText} from './TokenizedText.js' +import {TabularData, TabularDataProps} from './TabularData.js' import {Box, Text} from 'ink' import React, {FunctionComponent} from 'react' export interface CustomSection { title?: string - body: TokenItem + body: TabularDataProps | TokenItem } export interface AlertProps { @@ -57,7 +58,11 @@ const Alert: FunctionComponent = ({ {customSections.map((section, index) => ( {section.title ? {section.title} : null} - + {typeof section.body === 'object' && 'tabularData' in section.body ? ( + + ) : ( + + )} ))} diff --git a/packages/cli-kit/src/private/node/ui/components/FatalError.tsx b/packages/cli-kit/src/private/node/ui/components/FatalError.tsx index c5e65dafcbe..b632ac0d177 100644 --- a/packages/cli-kit/src/private/node/ui/components/FatalError.tsx +++ b/packages/cli-kit/src/private/node/ui/components/FatalError.tsx @@ -2,6 +2,7 @@ import {Banner} from './Banner.js' import {TokenizedText} from './TokenizedText.js' import {Command} from './Command.js' import {List} from './List.js' +import {TabularData} from './TabularData.js' import {BugError, cleanSingleStackTracePath, ExternalError, FatalError as Fatal} from '../../../../public/node/error.js' import {Box, Text} from 'ink' import React, {FunctionComponent} from 'react' @@ -58,7 +59,11 @@ const FatalError: FunctionComponent = ({error}) => { {error.customSections.map((section, index) => ( {section.title ? {section.title} : null} - + {typeof section.body === 'object' && 'tabularData' in section.body ? ( + + ) : ( + + )} ))} diff --git a/packages/cli-kit/src/private/node/ui/components/TabularData.tsx b/packages/cli-kit/src/private/node/ui/components/TabularData.tsx new file mode 100644 index 00000000000..d97d238c219 --- /dev/null +++ b/packages/cli-kit/src/private/node/ui/components/TabularData.tsx @@ -0,0 +1,36 @@ +import {InlineToken, TokenizedText, tokenItemToString} from './TokenizedText.js' +import {unstyled} from '../../../../public/node/output.js' +import {Box} from 'ink' +import React, {FunctionComponent} from 'react' + +export interface TabularDataProps { + tabularData: InlineToken[][] + firstColumnSubdued?: boolean +} + +const TabularData: FunctionComponent = ({tabularData: data, firstColumnSubdued}) => { + const columnWidths: number[] = data.reduce((acc, row) => { + row.forEach((cell, index) => { + acc[index] = Math.max(acc[index] ?? 0, unstyled((tokenItemToString(cell))).length) + }) + return acc + }, []) + + return ( + + {data.map((row, index) => ( + + {row.map((cell, index) => ( + + + + ))} + + ))} + + ) +} + +export {TabularData} From 17d42a2b1dea6583de107d5fcd62dacafffcf49e Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Wed, 8 Jan 2025 14:54:45 +0200 Subject: [PATCH 19/50] Improve handling of error messages --- packages/app/src/cli/services/info.ts | 77 +++++++------------ .../node/ui/components/TabularData.tsx | 2 +- 2 files changed, 30 insertions(+), 49 deletions(-) diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index 7d2835354bd..8a533c100ad 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -11,7 +11,7 @@ import { OutputMessage, formatPackageManagerCommand, outputContent, - outputToken, + shouldDisplayColors, stringifyMessage, } from '@shopify/cli-kit/node/output' import {InlineToken, renderInfo} from '@shopify/cli-kit/node/ui' @@ -109,8 +109,9 @@ function withPurgedSchemas(extensions: object[]): object[] { }) } -const UNKNOWN_TEXT = outputContent`${outputToken.italic('unknown')}`.value -const NOT_CONFIGURED_TEXT = outputContent`${outputToken.italic('Not yet configured')}`.value +const UNKNOWN_TEXT = 'unknown' +const NOT_CONFIGURED_TOKEN: InlineToken = {subdued: 'Not yet configured'} +const NOT_LOADED_TEXT = 'NOT LOADED' class AppInfo { private readonly app: AppLinkedInterface @@ -133,7 +134,7 @@ class AppInfo { } async devConfigsSection(): Promise { - let updateUrls = NOT_CONFIGURED_TEXT + let updateUrls = NOT_CONFIGURED_TOKEN if (this.app.configuration.build?.automatically_update_urls_on_dev !== undefined) { updateUrls = this.app.configuration.build.automatically_update_urls_on_dev ? 'Yes' : 'No' } @@ -151,10 +152,10 @@ class AppInfo { 'Current app configuration', [ ['Configuration file', {filePath: basename(this.app.configuration.path) || configurationFileNames.app}], - ['App name', this.remoteApp.title || NOT_CONFIGURED_TEXT], - ['Client ID', this.remoteApp.apiKey || NOT_CONFIGURED_TEXT], + ['App name', this.remoteApp.title || NOT_CONFIGURED_TOKEN], + ['Client ID', this.remoteApp.apiKey || NOT_CONFIGURED_TOKEN], ['Access scopes', getAppScopes(this.app.configuration)], - ['Dev store', this.app.configuration.build?.dev_store_url || NOT_CONFIGURED_TEXT], + ['Dev store', this.app.configuration.build?.dev_store_url ?? NOT_CONFIGURED_TOKEN], ['Update URLs', updateUrls], partnersAccountInfo, ], @@ -175,36 +176,19 @@ class AppInfo { async appComponentsSection(): Promise { const webComponentsSection = this.webComponentsSection() - - const supportedExtensions = this.app.allExtensions.filter((ext) => ext.isReturnedAsInfo()) - const extensionsSections = this.extensionsSections(supportedExtensions) - - let errorsSection: CustomSection | undefined - if (this.app.errors?.isEmpty() === false) { - errorsSection = this.tableSection( - 'Extensions with errors', - ( - supportedExtensions - .map((extension) => this.invalidExtensionSubSection(extension)) - .filter((data) => typeof data !== 'undefined') as [string, InlineToken][][] - ).flat(), - ) - } - return [ { title: '\nDirectory components'.toUpperCase(), body: '', }, ...(webComponentsSection ? [webComponentsSection] : []), - ...extensionsSections, - ...(errorsSection ? [errorsSection] : []), + ...this.extensionsSections(), ] } webComponentsSection(): CustomSection | undefined { const errors: OutputMessage[] = [] - const sublevels: [string, InlineToken][] = [] + const sublevels: InlineToken[][] = [] if (!this.app.webs[0]) return this.app.webs.forEach((web) => { if (web.configuration) { @@ -220,7 +204,7 @@ class AppInfo { }) } } else { - sublevels.push([` 📂 ${UNKNOWN_TEXT}`, {filePath: relativePath(this.app.directory, web.directory)}]) + sublevels.push([{subdued: ` 📂 ${UNKNOWN_TEXT}`}, {filePath: relativePath(this.app.directory, web.directory)}]) } if (this.app.errors) { const error = this.app.errors.getError(`${web.directory}/${configurationFileNames.web}`) @@ -231,11 +215,12 @@ class AppInfo { return this.subtableSection('web', [ ['📂 web', ''], ...sublevels, - ...errors.map((error): [string, InlineToken] => ['', {error: this.formattedError(error)}]), + ...errors.map((error): InlineToken[] => [{error: 'error'}, {error: this.formattedError(error)}]), ]) } - extensionsSections(extensions: ExtensionInstance[]): CustomSection[] { + extensionsSections(): CustomSection[] { + const extensions = this.app.allExtensions.filter((ext) => ext.isReturnedAsInfo()) const types = Array.from(new Set(extensions.map((ext) => ext.type))) return types .map((extensionType: string): CustomSection | undefined => { @@ -250,33 +235,29 @@ class AppInfo { .filter((section: CustomSection | undefined) => section !== undefined) as CustomSection[] } - extensionSubSection(extension: ExtensionInstance): [string, InlineToken][] { + extensionSubSection(extension: ExtensionInstance): InlineToken[][] { const config = extension.configuration - const details: [string, InlineToken][] = [ - [`📂 ${extension.handle}`, {filePath: relativePath(this.app.directory, extension.directory)}], + const details: InlineToken[][] = [ + [`📂 ${extension.handle || NOT_LOADED_TEXT}`, {filePath: relativePath(this.app.directory, extension.directory)}], [' config file', {filePath: relativePath(extension.directory, extension.configurationPath)}], ] if (config && config.metafields?.length) { details.push([' metafields', `${config.metafields.length}`]) } + const error = this.app.errors?.getError(extension.configurationPath) + if (error) { + details.push([{error: ' error'}, {error: this.formattedError(error)}]) + } return details } - invalidExtensionSubSection(extension: ExtensionInstance): [string, InlineToken][] | undefined { - const error = this.app.errors?.getError(extension.configurationPath) - if (!error) return - return [ - [`📂 ${extension.handle}`, {filePath: relativePath(this.app.directory, extension.directory)}], - [' config file', {filePath: relativePath(extension.directory, extension.configurationPath)}], - [' message', {error: this.formattedError(error)}], - ] - } - formattedError(str: OutputMessage): string { - const [errorFirstLine, ...errorRemainingLines] = stringifyMessage(str).split('\n') - const errorLines = [`! ${errorFirstLine}`, ...errorRemainingLines.map((line) => ` ${line}`)] - return outputContent`${outputToken.errorText(errorLines.join('\n'))}`.value + // Some errors have newlines at the beginning for no apparent reason + const rawErrorMessage = stringifyMessage(str).trim() + if (shouldDisplayColors()) return rawErrorMessage + const [errorFirstLine, ...errorRemainingLines] = stringifyMessage(str).trim().split('\n') + return [`! ${errorFirstLine}`, ...errorRemainingLines.map((line) => ` ${line}`)].join('\n') } async systemInfoSection(): Promise { @@ -285,19 +266,19 @@ class AppInfo { ['Shopify CLI', CLI_KIT_VERSION], ['Package manager', this.app.packageManager], ['OS', `${platform}-${arch}`], - ['Shell', process.env.SHELL || 'unknown'], + ['Shell', process.env.SHELL ?? 'unknown'], ['Node version', process.version], ]) } - tableSection(title: string, rows: [string, InlineToken][], {isFirstItem = false} = {}): CustomSection { + tableSection(title: string, rows: InlineToken[][], {isFirstItem = false} = {}): CustomSection { return { title: `${isFirstItem ? '' : '\n'}${title.toUpperCase()}\n`, body: {tabularData: rows, firstColumnSubdued: true}, } } - subtableSection(title: string, rows: [string, InlineToken][]): CustomSection { + subtableSection(title: string, rows: InlineToken[][]): CustomSection { return { title, body: {tabularData: rows, firstColumnSubdued: true}, diff --git a/packages/cli-kit/src/private/node/ui/components/TabularData.tsx b/packages/cli-kit/src/private/node/ui/components/TabularData.tsx index d97d238c219..b865da1dbfe 100644 --- a/packages/cli-kit/src/private/node/ui/components/TabularData.tsx +++ b/packages/cli-kit/src/private/node/ui/components/TabularData.tsx @@ -21,7 +21,7 @@ const TabularData: FunctionComponent = ({tabularData: data, fi {data.map((row, index) => ( {row.map((cell, index) => ( - + From 0db0229112a4f0b352bed788bcc6afb4215ef2d4 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 13:07:12 +0200 Subject: [PATCH 20/50] Improve display of web section 1. Add a default relative path (otherwise relativePath returns a blank string) 2. Move roles to a separate line item --- packages/app/src/cli/services/info.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index 8a533c100ad..6c2dcea9ccf 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -194,10 +194,11 @@ class AppInfo { if (web.configuration) { if (web.configuration.name) { const {name, roles} = web.configuration - sublevels.push([ - ` 📂 ${name} (${roles.join(',')})`, - {filePath: relativePath(this.app.directory, web.directory)}, - ]) + const pathToWeb = relativePath(this.app.directory, web.directory) + sublevels.push([` 📂 ${name}`, {filePath: pathToWeb || '/'}]) + if (roles.length > 0) { + sublevels.push([' roles', roles.join(', ')]) + } } else { web.configuration.roles.forEach((role) => { sublevels.push([` 📂 ${role}`, {filePath: relativePath(this.app.directory, web.directory)}]) From 2c643e3df1fe51bac7ed3778917d30f8d846c9ba Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 14:08:08 +0200 Subject: [PATCH 21/50] Update tests to match new rendering system --- packages/app/src/cli/services/info.test.ts | 63 +++++++++++++++++----- 1 file changed, 49 insertions(+), 14 deletions(-) diff --git a/packages/app/src/cli/services/info.test.ts b/packages/app/src/cli/services/info.test.ts index 15c4161cf1d..4b86c0bb70a 100644 --- a/packages/app/src/cli/services/info.test.ts +++ b/packages/app/src/cli/services/info.test.ts @@ -14,10 +14,13 @@ import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js import {describe, expect, vi, test} from 'vitest' import {checkForNewVersion} from '@shopify/cli-kit/node/node-package-manager' import {joinPath} from '@shopify/cli-kit/node/path' -import {TokenizedString, stringifyMessage, unstyled} from '@shopify/cli-kit/node/output' +import {OutputMessage, TokenizedString, stringifyMessage, unstyled} from '@shopify/cli-kit/node/output' import {inTemporaryDirectory, writeFileSync} from '@shopify/cli-kit/node/fs' +import {InlineToken, renderInfo} from '@shopify/cli-kit/node/ui' import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' +type CustomSection = Exclude[0]['customSections'], undefined>[number] + vi.mock('../prompts/dev.js') vi.mock('@shopify/cli-kit/node/node-package-manager') vi.mock('../utilities/developer-platform-client.js') @@ -116,7 +119,7 @@ describe('info', () => { vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1) // When - const result = await info(app, remoteApp, ORG1, {...infoOptions(), webEnv: true}) + const result = (await info(app, remoteApp, ORG1, {...infoOptions(), webEnv: true})) as OutputMessage // Then expect(unstyled(stringifyMessage(result))).toMatchInlineSnapshot(` @@ -136,7 +139,7 @@ describe('info', () => { vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1) // When - const result = await info(app, remoteApp, ORG1, {...infoOptions(), format: 'json', webEnv: true}) + const result = (await info(app, remoteApp, ORG1, {...infoOptions(), format: 'json', webEnv: true})) as OutputMessage // Then expect(unstyled(stringifyMessage(result))).toMatchInlineSnapshot(` @@ -184,18 +187,28 @@ describe('info', () => { vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1) // When - const result = await info(app, remoteApp, ORG1, infoOptions()) + const result = await info(app, remoteApp, ORG1, infoOptions()) as CustomSection[] + const uiData = tabularDataSectionFromInfo(result, 'ui_extension_external') + const checkoutData = tabularDataSectionFromInfo(result, 'checkout_ui_extension_external') // Then - expect(result).toContain('Extensions with errors') + // Doesn't use the type as part of the title - expect(result).not.toContain('📂 ui_extension') - // Shows handle in title - expect(result).toContain('📂 handle-for-extension-1') + expect(JSON.stringify(uiData)).not.toContain('📂 ui_extension') + + // Shows handle as title + const uiExtensionTitle = uiData[0]![0] + expect(uiExtensionTitle).toBe('📂 handle-for-extension-1') + // Displays errors + const uiExtensionErrorsRow = errorRow(uiData) + expect(uiExtensionErrorsRow[1]).toStrictEqual({error: 'Mock error with ui_extension'}) + // Shows default handle derived from name when no handle is present - expect(result).toContain('📂 extension-2') - expect(result).toContain('! Mock error with ui_extension') - expect(result).toContain('! Mock error with checkout_ui_extension') + const checkoutExtensionTitle = checkoutData[0]![0] + expect(checkoutExtensionTitle).toBe('📂 extension-2') + // Displays errors + const checkoutExtensionErrorsRow = errorRow(checkoutData) + expect(checkoutExtensionErrorsRow[1]).toStrictEqual({error: 'Mock error with checkout_ui_extension'}) }) }) @@ -222,11 +235,14 @@ describe('info', () => { vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1) // When - const result = await info(app, remoteApp, ORG1, infoOptions()) + const result = (await info(app, remoteApp, ORG1, infoOptions())) as CustomSection[] + const uiExtensionsData = tabularDataSectionFromInfo(result, 'ui_extension_external') + const relevantExtension = extensionTitleRow(uiExtensionsData, 'handle-for-extension-1') + const irrelevantExtension = extensionTitleRow(uiExtensionsData, 'point_of_sale') // Then - expect(result).toContain('📂 handle-for-extension-1') - expect(result).not.toContain('📂 point_of_sale') + expect(relevantExtension).toBeDefined() + expect(irrelevantExtension).not.toBeDefined() }) }) @@ -293,3 +309,22 @@ function mockApp({ ...(app ? app : {}), }) } + +function tabularDataSectionFromInfo(info: CustomSection[], title: string): InlineToken[][] { + const section = info.find((section) => section.title === title) + if (!section) throw new Error(`Section ${title} not found`) + if (!(typeof section.body === 'object' && 'tabularData' in section.body)) { + throw new Error(`Expected to be a table: ${JSON.stringify(section.body)}`) + } + return section.body.tabularData +} + +function errorRow(data: InlineToken[][]): InlineToken[] { + const row = data.find((row: InlineToken[]) => typeof row[0] === 'object' && 'error' in row[0])! + if (!row) throw new Error('Error row not found') + return row +} + +function extensionTitleRow(data: InlineToken[][], title: string): InlineToken[] | undefined { + return data.find((row) => typeof row[0] === 'string' && row[0].match(new RegExp(title))) +} From 6ce820ad83fc4977f0fc973bbefcd749b7fec8b0 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 14:08:40 +0200 Subject: [PATCH 22/50] Remove no-longer-relevant tests We removed the upgrade reminders in https://github.com/Shopify/cli/pull/4310 but we missed deleting the tests! --- packages/app/src/cli/services/info.test.ts | 30 ---------------------- 1 file changed, 30 deletions(-) diff --git a/packages/app/src/cli/services/info.test.ts b/packages/app/src/cli/services/info.test.ts index 4b86c0bb70a..dc1ac758df2 100644 --- a/packages/app/src/cli/services/info.test.ts +++ b/packages/app/src/cli/services/info.test.ts @@ -12,12 +12,10 @@ import { import {AppErrors} from '../models/app/loader.js' import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' import {describe, expect, vi, test} from 'vitest' -import {checkForNewVersion} from '@shopify/cli-kit/node/node-package-manager' import {joinPath} from '@shopify/cli-kit/node/path' import {OutputMessage, TokenizedString, stringifyMessage, unstyled} from '@shopify/cli-kit/node/output' import {inTemporaryDirectory, writeFileSync} from '@shopify/cli-kit/node/fs' import {InlineToken, renderInfo} from '@shopify/cli-kit/node/ui' -import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' type CustomSection = Exclude[0]['customSections'], undefined>[number] @@ -83,34 +81,6 @@ function infoOptions(): InfoOptions { describe('info', () => { const remoteApp = testOrganizationApp() - test('returns update shopify cli reminder when last version is greater than current version', async () => { - await inTemporaryDirectory(async (tmp) => { - // Given - const latestVersion = '2.2.3' - const app = mockApp({directory: tmp}) - vi.mocked(checkForNewVersion).mockResolvedValue(latestVersion) - - // When - const result = stringifyMessage(await info(app, remoteApp, ORG1, infoOptions())) - // Then - expect(unstyled(result)).toMatch(`Shopify CLI ${CLI_KIT_VERSION}`) - }) - }) - - test('returns update shopify cli reminder when last version lower or equals to current version', async () => { - await inTemporaryDirectory(async (tmp) => { - // Given - const app = mockApp({directory: tmp}) - vi.mocked(checkForNewVersion).mockResolvedValue(undefined) - - // When - 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') - }) - }) - test('returns the web environment as a text when webEnv is true', async () => { await inTemporaryDirectory(async (tmp) => { // Given From 3ce2ba2f84ea36cc9dee2db7f43a6e84f12a785a Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 16:33:38 +0200 Subject: [PATCH 23/50] Lint fixes --- packages/app/src/cli/services/info.ts | 2 +- packages/cli-kit/src/private/node/ui/components/TabularData.tsx | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index 6c2dcea9ccf..494c7d3ff64 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -233,7 +233,7 @@ class AppInfo { ) } }) - .filter((section: CustomSection | undefined) => section !== undefined) as CustomSection[] + .filter((section: CustomSection | undefined) => section !== undefined) } extensionSubSection(extension: ExtensionInstance): InlineToken[][] { diff --git a/packages/cli-kit/src/private/node/ui/components/TabularData.tsx b/packages/cli-kit/src/private/node/ui/components/TabularData.tsx index b865da1dbfe..a228308af54 100644 --- a/packages/cli-kit/src/private/node/ui/components/TabularData.tsx +++ b/packages/cli-kit/src/private/node/ui/components/TabularData.tsx @@ -11,7 +11,7 @@ export interface TabularDataProps { const TabularData: FunctionComponent = ({tabularData: data, firstColumnSubdued}) => { const columnWidths: number[] = data.reduce((acc, row) => { row.forEach((cell, index) => { - acc[index] = Math.max(acc[index] ?? 0, unstyled((tokenItemToString(cell))).length) + acc[index] = Math.max(acc[index] ?? 0, unstyled(tokenItemToString(cell)).length) }) return acc }, []) From e8ee7548dad4e34f079145ac4eefdfa5456fa3bf Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 17:33:51 +0200 Subject: [PATCH 24/50] Refer to User instead of Partners account because we may not be on Partners --- packages/app/src/cli/services/info.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index 494c7d3ff64..42fcd83269b 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -139,12 +139,12 @@ class AppInfo { updateUrls = this.app.configuration.build.automatically_update_urls_on_dev ? 'Yes' : 'No' } - let partnersAccountInfo: [string, string] = ['Partners account', 'unknown'] + let userAccountInfo: [string, string] = ['User', 'unknown'] const retrievedAccountInfo = await this.options.developerPlatformClient.accountInfo() if (isServiceAccount(retrievedAccountInfo)) { - partnersAccountInfo = ['Service account', retrievedAccountInfo.orgName] + userAccountInfo = ['Service account', retrievedAccountInfo.orgName] } else if (isUserAccount(retrievedAccountInfo)) { - partnersAccountInfo = ['Partners account', retrievedAccountInfo.email] + userAccountInfo[1] = retrievedAccountInfo.email } return [ @@ -157,7 +157,7 @@ class AppInfo { ['Access scopes', getAppScopes(this.app.configuration)], ['Dev store', this.app.configuration.build?.dev_store_url ?? NOT_CONFIGURED_TOKEN], ['Update URLs', updateUrls], - partnersAccountInfo, + userAccountInfo, ], {isFirstItem: true}, ), From bb3b100731eb49c982ab60cd9b00890ffe33660f Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 17:51:12 +0200 Subject: [PATCH 25/50] Add changesets --- .changeset/light-windows-sit.md | 5 +++++ .changeset/red-brooms-lick.md | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 .changeset/light-windows-sit.md create mode 100644 .changeset/red-brooms-lick.md diff --git a/.changeset/light-windows-sit.md b/.changeset/light-windows-sit.md new file mode 100644 index 00000000000..07598bb80a8 --- /dev/null +++ b/.changeset/light-windows-sit.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': minor +--- + +Give `app info` a facelift and correct a few display bugs diff --git a/.changeset/red-brooms-lick.md b/.changeset/red-brooms-lick.md new file mode 100644 index 00000000000..d4fdc36ea7a --- /dev/null +++ b/.changeset/red-brooms-lick.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': minor +--- + +Add tabular data display component to UI kit From b19fce5e725c955dc7904a38194631a371eaeb4f Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 21:11:13 +0200 Subject: [PATCH 26/50] Render app name as userInput token --- packages/app/src/cli/services/info.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index 42fcd83269b..87856124279 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -152,7 +152,7 @@ class AppInfo { 'Current app configuration', [ ['Configuration file', {filePath: basename(this.app.configuration.path) || configurationFileNames.app}], - ['App name', this.remoteApp.title || NOT_CONFIGURED_TOKEN], + ['App name', this.remoteApp.title ? {userInput: this.remoteApp.title} : NOT_CONFIGURED_TOKEN], ['Client ID', this.remoteApp.apiKey || NOT_CONFIGURED_TOKEN], ['Access scopes', getAppScopes(this.app.configuration)], ['Dev store', this.app.configuration.build?.dev_store_url ?? NOT_CONFIGURED_TOKEN], From 261ee48f7598dea409b79b21a60f5691ce32a86a Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 9 Jan 2025 21:11:26 +0200 Subject: [PATCH 27/50] Add example of tabularData to kitchen sink --- .../cli/src/cli/services/kitchen-sink/static.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/packages/cli/src/cli/services/kitchen-sink/static.ts b/packages/cli/src/cli/services/kitchen-sink/static.ts index c6b342f8207..32a01e8c1e6 100644 --- a/packages/cli/src/cli/services/kitchen-sink/static.ts +++ b/packages/cli/src/cli/services/kitchen-sink/static.ts @@ -34,6 +34,22 @@ export async function staticService() { ], }) + renderInfo({ + headline: 'About your app', + customSections: [ + { + body: { + tabularData: [ + ['Configuration file', {filePath: 'shopify.app.scalable-transaction-app.toml'}], + ['App name', {userInput: 'scalable-transaction-app'}], + ['Access scopes', 'read_products,write_products'], + ], + firstColumnSubdued: true, + }, + }, + ], + }) + renderInfo({ headline: [{userInput: 'my-app'}, 'initialized and ready to build.'], nextSteps: [ From 67b19b75131897426c7cc0203bf22837714eb1f5 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Mon, 13 Jan 2025 15:04:19 +0200 Subject: [PATCH 28/50] Use exported AlertCustomSection type --- packages/app/src/cli/services/info.test.ts | 10 +++----- packages/app/src/cli/services/info.ts | 30 ++++++++++------------ 2 files changed, 18 insertions(+), 22 deletions(-) diff --git a/packages/app/src/cli/services/info.test.ts b/packages/app/src/cli/services/info.test.ts index dc1ac758df2..6ca0aebc5b9 100644 --- a/packages/app/src/cli/services/info.test.ts +++ b/packages/app/src/cli/services/info.test.ts @@ -15,9 +15,7 @@ import {describe, expect, vi, test} from 'vitest' import {joinPath} from '@shopify/cli-kit/node/path' import {OutputMessage, TokenizedString, stringifyMessage, unstyled} from '@shopify/cli-kit/node/output' import {inTemporaryDirectory, writeFileSync} from '@shopify/cli-kit/node/fs' -import {InlineToken, renderInfo} from '@shopify/cli-kit/node/ui' - -type CustomSection = Exclude[0]['customSections'], undefined>[number] +import {AlertCustomSection, InlineToken} from '@shopify/cli-kit/node/ui' vi.mock('../prompts/dev.js') vi.mock('@shopify/cli-kit/node/node-package-manager') @@ -157,7 +155,7 @@ describe('info', () => { vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1) // When - const result = await info(app, remoteApp, ORG1, infoOptions()) as CustomSection[] + const result = (await info(app, remoteApp, ORG1, infoOptions())) as AlertCustomSection[] const uiData = tabularDataSectionFromInfo(result, 'ui_extension_external') const checkoutData = tabularDataSectionFromInfo(result, 'checkout_ui_extension_external') @@ -205,7 +203,7 @@ describe('info', () => { vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1) // When - const result = (await info(app, remoteApp, ORG1, infoOptions())) as CustomSection[] + const result = (await info(app, remoteApp, ORG1, infoOptions())) as AlertCustomSection[] const uiExtensionsData = tabularDataSectionFromInfo(result, 'ui_extension_external') const relevantExtension = extensionTitleRow(uiExtensionsData, 'handle-for-extension-1') const irrelevantExtension = extensionTitleRow(uiExtensionsData, 'point_of_sale') @@ -280,7 +278,7 @@ function mockApp({ }) } -function tabularDataSectionFromInfo(info: CustomSection[], title: string): InlineToken[][] { +function tabularDataSectionFromInfo(info: AlertCustomSection[], title: string): InlineToken[][] { const section = info.find((section) => section.title === title) if (!section) throw new Error(`Section ${title} not found`) if (!(typeof section.body === 'object' && 'tabularData' in section.body)) { diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index 87856124279..cd8ff5f6643 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -14,11 +14,9 @@ import { shouldDisplayColors, stringifyMessage, } from '@shopify/cli-kit/node/output' -import {InlineToken, renderInfo} from '@shopify/cli-kit/node/ui' +import {AlertCustomSection, InlineToken} from '@shopify/cli-kit/node/ui' import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' -type CustomSection = Exclude[0]['customSections'], undefined>[number] - export type Format = 'json' | 'text' export interface InfoOptions { format: Format @@ -33,7 +31,7 @@ export async function info( remoteApp: OrganizationApp, organization: Organization, options: InfoOptions, -): Promise { +): Promise { if (options.webEnv) { return infoWeb(app, remoteApp, organization, options) } else { @@ -54,7 +52,7 @@ async function infoApp( app: AppLinkedInterface, remoteApp: OrganizationApp, options: InfoOptions, -): Promise { +): Promise { if (options.format === 'json') { const extensionsInfo = withPurgedSchemas(app.allExtensions.filter((ext) => ext.isReturnedAsInfo())) let appWithSupportedExtensions = { @@ -124,7 +122,7 @@ class AppInfo { this.options = options } - async output(): Promise { + async output(): Promise { return [ ...(await this.devConfigsSection()), this.projectSettingsSection(), @@ -133,7 +131,7 @@ class AppInfo { ] } - async devConfigsSection(): Promise { + async devConfigsSection(): Promise { let updateUrls = NOT_CONFIGURED_TOKEN if (this.app.configuration.build?.automatically_update_urls_on_dev !== undefined) { updateUrls = this.app.configuration.build.automatically_update_urls_on_dev ? 'Yes' : 'No' @@ -170,11 +168,11 @@ class AppInfo { ] } - projectSettingsSection(): CustomSection { + projectSettingsSection(): AlertCustomSection { return this.tableSection('Your Project', [['Root location', {filePath: this.app.directory}]]) } - async appComponentsSection(): Promise { + async appComponentsSection(): Promise { const webComponentsSection = this.webComponentsSection() return [ { @@ -186,7 +184,7 @@ class AppInfo { ] } - webComponentsSection(): CustomSection | undefined { + webComponentsSection(): AlertCustomSection | undefined { const errors: OutputMessage[] = [] const sublevels: InlineToken[][] = [] if (!this.app.webs[0]) return @@ -220,11 +218,11 @@ class AppInfo { ]) } - extensionsSections(): CustomSection[] { + extensionsSections(): AlertCustomSection[] { const extensions = this.app.allExtensions.filter((ext) => ext.isReturnedAsInfo()) const types = Array.from(new Set(extensions.map((ext) => ext.type))) return types - .map((extensionType: string): CustomSection | undefined => { + .map((extensionType: string): AlertCustomSection | undefined => { const relevantExtensions = extensions.filter((extension: ExtensionInstance) => extension.type === extensionType) if (relevantExtensions[0]) { return this.subtableSection( @@ -233,7 +231,7 @@ class AppInfo { ) } }) - .filter((section: CustomSection | undefined) => section !== undefined) + .filter((section: AlertCustomSection | undefined) => section !== undefined) } extensionSubSection(extension: ExtensionInstance): InlineToken[][] { @@ -261,7 +259,7 @@ class AppInfo { return [`! ${errorFirstLine}`, ...errorRemainingLines.map((line) => ` ${line}`)].join('\n') } - async systemInfoSection(): Promise { + async systemInfoSection(): Promise { const {platform, arch} = platformAndArch() return this.tableSection('Tooling and System', [ ['Shopify CLI', CLI_KIT_VERSION], @@ -272,14 +270,14 @@ class AppInfo { ]) } - tableSection(title: string, rows: InlineToken[][], {isFirstItem = false} = {}): CustomSection { + tableSection(title: string, rows: InlineToken[][], {isFirstItem = false} = {}): AlertCustomSection { return { title: `${isFirstItem ? '' : '\n'}${title.toUpperCase()}\n`, body: {tabularData: rows, firstColumnSubdued: true}, } } - subtableSection(title: string, rows: InlineToken[][]): CustomSection { + subtableSection(title: string, rows: InlineToken[][]): AlertCustomSection { return { title, body: {tabularData: rows, firstColumnSubdued: true}, From 74298784759df70b0e82a94477fa816ec7d645f6 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Mon, 13 Jan 2025 15:07:48 +0200 Subject: [PATCH 29/50] Suggest more updated command --- packages/app/src/cli/services/info.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/info.ts b/packages/app/src/cli/services/info.ts index cd8ff5f6643..1b820227a9d 100644 --- a/packages/app/src/cli/services/info.ts +++ b/packages/app/src/cli/services/info.ts @@ -162,7 +162,7 @@ class AppInfo { { body: [ '💡 To change these, run', - {command: formatPackageManagerCommand(this.app.packageManager, 'dev', '--reset')}, + {command: formatPackageManagerCommand(this.app.packageManager, 'shopify app config link')}, ], }, ] From 7df0f33a77f1055e86c5a9edead3aa621e2c5dfd Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Mon, 13 Jan 2025 18:24:51 +0200 Subject: [PATCH 30/50] Lint fix --- packages/app/src/cli/services/info.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/info.test.ts b/packages/app/src/cli/services/info.test.ts index 6ca0aebc5b9..21cbb059c06 100644 --- a/packages/app/src/cli/services/info.test.ts +++ b/packages/app/src/cli/services/info.test.ts @@ -107,7 +107,11 @@ describe('info', () => { vi.mocked(selectOrganizationPrompt).mockResolvedValue(ORG1) // When - const result = (await info(app, remoteApp, ORG1, {...infoOptions(), format: 'json', webEnv: true})) as OutputMessage + const result = (await info(app, remoteApp, ORG1, { + ...infoOptions(), + format: 'json', + webEnv: true, + })) as OutputMessage // Then expect(unstyled(stringifyMessage(result))).toMatchInlineSnapshot(` From 49ef62445885a1a4b6542b87c79693bf8970e63d Mon Sep 17 00:00:00 2001 From: Jamie Guerrero Date: Mon, 13 Jan 2025 11:08:36 -0500 Subject: [PATCH 31/50] Create app with latest api version --- .../app-management-client.test.ts | 85 ++++++++++++++++++- .../app-management-client.ts | 21 ++++- 2 files changed, 101 insertions(+), 5 deletions(-) diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts index 84f17cb74b5..ef01580534a 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts @@ -7,13 +7,19 @@ import { versionDeepLink, } from './app-management-client.js' import {OrganizationBetaFlagsQuerySchema} from './app-management-client/graphql/organization_beta_flags.js' -import {testUIExtension, testRemoteExtensionTemplates, testOrganizationApp} from '../../models/app/app.test-data.js' +import { + testUIExtension, + testRemoteExtensionTemplates, + testOrganizationApp, + testOrganization, +} from '../../models/app/app.test-data.js' import {ExtensionInstance} from '../../models/extensions/extension-instance.js' import {ListApps} from '../../api/graphql/app-management/generated/apps.js' import {PublicApiVersionsQuery} from '../../api/graphql/webhooks/generated/public-api-versions.js' import {AvailableTopicsQuery} from '../../api/graphql/webhooks/generated/available-topics.js' import {CliTesting, CliTestingMutation} from '../../api/graphql/webhooks/generated/cli-testing.js' import {SendSampleWebhookVariables} from '../../services/webhook/request-sample.js' +import {CreateApp} from '../../api/graphql/app-management/generated/create-app.js' import {describe, expect, test, vi} from 'vitest' import {CLI_KIT_VERSION} from '@shopify/cli-kit/common/version' import {fetch} from '@shopify/cli-kit/node/http' @@ -295,6 +301,83 @@ describe('searching for apps', () => { }) }) +describe('createApp', () => { + test('fetches latest stable API version for webhooks module', async () => { + // Given + const client = new AppManagementClient() + const org = testOrganization() + const mockedApiVersionResult: PublicApiVersionsQuery = { + publicApiVersions: [{handle: '2024-07'}, {handle: '2024-10'}, {handle: '2025-01'}, {handle: 'unstable'}], + } + vi.mocked(webhooksRequest).mockResolvedValueOnce(mockedApiVersionResult) + vi.mocked(appManagementRequestDoc).mockResolvedValueOnce({appCreate: {app: {id: '1', key: 'key'}, userErrors: []}}) + + // When + client.token = () => Promise.resolve('token') + await client.createApp(org, 'app-name') + + // Then + expect(webhooksRequest).toHaveBeenCalledWith(org.id, expect.anything(), 'token', expect.any(Object)) + expect(appManagementRequestDoc).toHaveBeenCalledWith( + org.id, + CreateApp, + 'token', + expect.objectContaining({ + appSource: { + appModules: expect.arrayContaining([ + { + config: { + api_version: '2025-01', + }, + specificationIdentifier: 'webhooks', + uid: 'webhooks', + }, + ]), + }, + }), + ) + }) + + test('creates app successfully and returns expected app structure', async () => { + // Given + const appName = 'app-name' + const client = new AppManagementClient() + const org = testOrganization() + const expectedApp = { + id: '1', + key: 'api-key', + apiKey: 'api-key', + apiSecretKeys: [], + flags: [], + grantedScopes: [], + organizationId: '1', + title: appName, + newApp: true, + developerPlatformClient: expect.any(AppManagementClient), + } + + vi.mocked(webhooksRequest).mockResolvedValueOnce({ + publicApiVersions: [{handle: '2024-07'}, {handle: '2024-10'}, {handle: '2025-01'}, {handle: 'unstable'}], + }) + vi.mocked(appManagementRequestDoc).mockResolvedValueOnce({ + appCreate: { + app: { + id: expectedApp.id, + key: expectedApp.key, + }, + userErrors: [], + }, + }) + + // When + client.token = () => Promise.resolve('token') + const result = await client.createApp(org, appName) + + // Then + expect(result).toMatchObject(expectedApp) + }) +}) + describe('apiVersions', () => { test('fetches available public API versions', async () => { // Given 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 98e77e8493c..d6b3f5fd7e4 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 @@ -75,7 +75,6 @@ import { import {ListOrganizations} from '../../api/graphql/business-platform-destinations/generated/organizations.js' import {AppHomeSpecIdentifier} from '../../models/extensions/specifications/app_config_app_home.js' import {BrandingSpecIdentifier} from '../../models/extensions/specifications/app_config_branding.js' -import {WebhooksSpecIdentifier} from '../../models/extensions/specifications/app_config_webhook.js' import {AppAccessSpecIdentifier} from '../../models/extensions/specifications/app_config_app_access.js' import {CONFIG_EXTENSION_IDS} from '../../models/extensions/extension-instance.js' import {DevSessionCreate, DevSessionCreateMutation} from '../../api/graphql/app-dev/generated/dev-session-create.js' @@ -117,6 +116,7 @@ import { SchemaDefinitionByApiTypeQuery, SchemaDefinitionByApiTypeQueryVariables, } from '../../api/graphql/functions/generated/schema-definition-by-api-type.js' +import {WebhooksSpecIdentifier} from '../../models/extensions/specifications/app_config_webhook.js' import {ensureAuthenticatedAppManagement, ensureAuthenticatedBusinessPlatform} from '@shopify/cli-kit/node/session' import {isUnitTest} from '@shopify/cli-kit/node/context/local' import {AbortError, BugError} from '@shopify/cli-kit/node/error' @@ -369,7 +369,15 @@ export class AppManagementClient implements DeveloperPlatformClient { directory?: string }, ): Promise { - const variables = createAppVars(name, options?.isLaunchable, options?.scopesArray) + // Query for latest api version + const apiVersions = await this.apiVersions(org.id) + const apiVersion = + apiVersions.publicApiVersions + .filter((version) => version !== 'unstable') + .sort() + .at(-1) ?? 'unstable' + + const variables = createAppVars(name, options?.isLaunchable, options?.scopesArray, apiVersion) const mutation = CreateApp const result = await appManagementRequestDoc(org.id, mutation, await this.token(), variables) @@ -909,7 +917,12 @@ export class AppManagementClient implements DeveloperPlatformClient { const MAGIC_URL = 'https://shopify.dev/apps/default-app-home' const MAGIC_REDIRECT_URL = 'https://shopify.dev/apps/default-app-home/api/auth' -function createAppVars(name: string, isLaunchable = true, scopesArray?: string[]): CreateAppMutationVariables { +function createAppVars( + name: string, + isLaunchable = true, + scopesArray?: string[], + apiVersion?: string, +): CreateAppMutationVariables { return { appSource: { appModules: [ @@ -932,7 +945,7 @@ function createAppVars(name: string, isLaunchable = true, scopesArray?: string[] // Change the uid to WebhooksSpecIdentifier uid: 'webhooks', specificationIdentifier: WebhooksSpecIdentifier, - config: {api_version: '2024-01'}, + config: {api_version: apiVersion}, }, { // Change the uid to AppAccessSpecIdentifier From 6c74a9e0f140e8f475ac044efdcec0f8cd19c2b6 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Thu, 5 Dec 2024 13:32:27 +0200 Subject: [PATCH 32/50] Filter orgs to only those with develop_apps permission --- .../generated/organizations.ts | 7 +++++++ .../queries/organizations.graphql | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/api/graphql/business-platform-destinations/generated/organizations.ts b/packages/app/src/cli/api/graphql/business-platform-destinations/generated/organizations.ts index bb19e6628bb..f4f74d84b49 100644 --- a/packages/app/src/cli/api/graphql/business-platform-destinations/generated/organizations.ts +++ b/packages/app/src/cli/api/graphql/business-platform-destinations/generated/organizations.ts @@ -29,6 +29,13 @@ export const ListOrganizations = { { kind: 'Field', name: {kind: 'Name', value: 'organizations'}, + arguments: [ + { + kind: 'Argument', + name: {kind: 'Name', value: 'hasAccessToDestination'}, + value: {kind: 'EnumValue', value: 'DEVELOPER_DASHBOARD'}, + }, + ], selectionSet: { kind: 'SelectionSet', selections: [ diff --git a/packages/app/src/cli/api/graphql/business-platform-destinations/queries/organizations.graphql b/packages/app/src/cli/api/graphql/business-platform-destinations/queries/organizations.graphql index c6d9194ff3d..e1c8a46d508 100644 --- a/packages/app/src/cli/api/graphql/business-platform-destinations/queries/organizations.graphql +++ b/packages/app/src/cli/api/graphql/business-platform-destinations/queries/organizations.graphql @@ -1,7 +1,7 @@ query ListOrganizations { currentUserAccount { uuid - organizations { + organizations(hasAccessToDestination: DEVELOPER_DASHBOARD) { nodes { id name From a622479f77702b31dd1171d2ba3febeaffd31f77 Mon Sep 17 00:00:00 2001 From: Jamie Guerrero Date: Fri, 10 Jan 2025 12:34:29 -0500 Subject: [PATCH 33/50] Remove app prompt for both Partners and App Management apps --- packages/app/src/cli/services/dev.ts | 5 +- packages/app/src/cli/services/dev/ui.test.tsx | 104 +----------------- packages/app/src/cli/services/dev/ui.tsx | 71 +----------- .../app/src/cli/services/dev/urls.test.ts | 5 +- packages/app/src/cli/services/dev/urls.ts | 13 +-- 5 files changed, 11 insertions(+), 187 deletions(-) diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index a3e788a3cfb..382e0eee799 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -18,7 +18,7 @@ import {fetchAppPreviewMode} from './dev/fetch.js' import {installAppDependencies} from './dependencies.js' import {DevConfig, DevProcesses, setupDevProcesses} from './dev/processes/setup-dev-processes.js' import {frontAndBackendConfig} from './dev/processes/utils.js' -import {outputUpdateURLsResult, renderDev} from './dev/ui.js' +import {renderDev} from './dev/ui.js' import {DeveloperPreviewController} from './dev/ui/components/Dev.js' import {DevProcessFunction} from './dev/processes/types.js' import {getCachedAppInfo, setCachedAppInfo} from './local-storage.js' @@ -264,7 +264,7 @@ async function handleUpdatingOfPartnerUrls( const newURLs = generatePartnersURLs( network.proxyUrl, webs.map(({configuration}) => configuration.auth_callback_path).find((path) => path), - isCurrentAppSchema(localApp.configuration) ? localApp.configuration.app_proxy : undefined, + localApp.configuration.app_proxy, ) shouldUpdateURLs = await shouldOrPromptUpdateURLs({ currentURLs: network.currentUrls, @@ -277,7 +277,6 @@ async function handleUpdatingOfPartnerUrls( // When running dev app urls are pushed directly to API Client config instead of creating a new app version // so current app version and API Client config will have diferent url values. if (shouldUpdateURLs) await updateURLs(newURLs, apiKey, developerPlatformClient, localApp) - await outputUpdateURLsResult(shouldUpdateURLs, newURLs, remoteApp, localApp, developerPlatformClient) } } return shouldUpdateURLs diff --git a/packages/app/src/cli/services/dev/ui.test.tsx b/packages/app/src/cli/services/dev/ui.test.tsx index 478ee53bde0..e3aa1cd6274 100644 --- a/packages/app/src/cli/services/dev/ui.test.tsx +++ b/packages/app/src/cli/services/dev/ui.test.tsx @@ -1,10 +1,9 @@ -import {outputUpdateURLsResult, renderDev} from './ui.js' +import {renderDev} from './ui.js' import {Dev} from './ui/components/Dev.js' import { testApp, testDeveloperPlatformClient, testFunctionExtension, - testOrganizationApp, testThemeExtensions, testUIExtension, } from '../../models/app/app.test-data.js' @@ -32,107 +31,6 @@ afterEach(() => { mockAndCaptureOutput().clear() }) -describe('output', () => { - describe('outputUpdateURLsResult', () => { - const urls = { - applicationUrl: 'https://lala.cloudflare.io/', - redirectUrlWhitelist: ['https://lala.cloudflare.io/auth/callback'], - } - - test('shows info about tunnel URL and links to Partners Dashboard when app is brand new', async () => { - // Given - const outputMock = mockAndCaptureOutput() - const localApp = await mockApp() - - const remoteApp = testOrganizationApp({newApp: true}) - - // When - await outputUpdateURLsResult(true, urls, remoteApp, localApp) - - // Then - expect(outputMock.output()).toMatchInlineSnapshot(` - "╭─ info ───────────────────────────────────────────────────────────────────────╮ - │ │ - │ For your convenience, we've given your app a default URL: │ - │ https://lala.cloudflare.io/. │ - │ │ - │ You can update your app's URL anytime in the Partners Dashboard [1] But │ - │ once your app is live, updating its URL will disrupt user access. │ - │ │ - ╰──────────────────────────────────────────────────────────────────────────────╯ - [1] https://partners.shopify.com/1/apps/1/edit - " - `) - }) - - test('shows nothing when urls were updated', async () => { - // Given - const outputMock = mockAndCaptureOutput() - const localApp = await mockApp() - - const remoteApp = testOrganizationApp({newApp: false}) - - // When - await outputUpdateURLsResult(true, urls, remoteApp, localApp) - - // Then - expect(outputMock.output()).toEqual('') - }) - - test('shows how to update app urls on partners when app is not brand new, urls were not updated and app uses legacy config', async () => { - // Given - const outputMock = mockAndCaptureOutput() - const localApp = await mockApp() - - const remoteApp = testOrganizationApp({newApp: false}) - - // When - await outputUpdateURLsResult(false, urls, remoteApp, localApp) - - // Then - expect(outputMock.output()).toMatchInlineSnapshot(` - "╭─ info ───────────────────────────────────────────────────────────────────────╮ - │ │ - │ To make URL updates manually, you can add the following URLs as redirects │ - │ in your Partners Dashboard [1]: │ - │ │ - │ │ - │ • https://lala.cloudflare.io/auth/callback │ - │ │ - ╰──────────────────────────────────────────────────────────────────────────────╯ - [1] https://partners.shopify.com/1/apps/1/edit - " - `) - }) - - test('shows how to update app urls with config push when app is not brand new, urls were updated and app uses new config', async () => { - // Given - const outputMock = mockAndCaptureOutput() - const localApp = await mockApp(true) - - const remoteApp = testOrganizationApp({newApp: false}) - - // When - await outputUpdateURLsResult(false, urls, remoteApp, localApp) - - // Then - expect(outputMock.output()).toMatchInlineSnapshot(` - "╭─ info ───────────────────────────────────────────────────────────────────────╮ - │ │ - │ To update URLs manually, add the following URLs to │ - │ shopify.app.staging.toml under auth > redirect_urls and run │ - │ \`yarn shopify app config push --config=staging\` │ - │ │ - │ │ - │ • https://lala.cloudflare.io/auth/callback │ - │ │ - ╰──────────────────────────────────────────────────────────────────────────────╯ - " - `) - }) - }) -}) - describe('ui', () => { describe('renderDev', () => { test("doesn't use ink when terminal doesn't support TTY", async () => { diff --git a/packages/app/src/cli/services/dev/ui.tsx b/packages/app/src/cli/services/dev/ui.tsx index bf3199170a6..359bd68a44a 100644 --- a/packages/app/src/cli/services/dev/ui.tsx +++ b/packages/app/src/cli/services/dev/ui.tsx @@ -1,70 +1,10 @@ -import {PartnersURLs} from './urls.js' import {Dev, DevProps} from './ui/components/Dev.js' -import {AppInterface, isCurrentAppSchema} from '../../models/app/app.js' -import {OrganizationApp} from '../../models/organization.js' -import {getAppConfigurationShorthand} from '../../models/app/loader.js' -import {ClientName, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import React from 'react' -import {partnersFqdn} from '@shopify/cli-kit/node/context/fqdn' -import {render, renderInfo} from '@shopify/cli-kit/node/ui' -import {basename} from '@shopify/cli-kit/node/path' -import {formatPackageManagerCommand} from '@shopify/cli-kit/node/output' +import {render} from '@shopify/cli-kit/node/ui' import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' import {isTruthy} from '@shopify/cli-kit/node/context/utilities' import {isUnitTest} from '@shopify/cli-kit/node/context/local' -export async function outputUpdateURLsResult( - updated: boolean, - urls: PartnersURLs, - remoteApp: OrganizationApp, - localApp: AppInterface, - developerPlatformClient?: DeveloperPlatformClient, -) { - const usingAppManagementClient = developerPlatformClient?.clientName === ClientName.AppManagement - const dashboardURL = await partnersURL(remoteApp.organizationId, remoteApp.id) - if (remoteApp.newApp && !usingAppManagementClient) { - renderInfo({ - headline: `For your convenience, we've given your app a default URL: ${urls.applicationUrl}.`, - body: [ - "You can update your app's URL anytime in the", - dashboardURL, - 'But once your app is live, updating its URL will disrupt user access.', - ], - }) - } else if (!updated) { - if (isCurrentAppSchema(localApp.configuration)) { - const fileName = basename(localApp.configuration.path) - const configName = getAppConfigurationShorthand(fileName) - const pushCommandArgs = configName ? [`--config=${configName}`] : [] - - renderInfo({ - body: [ - `To update URLs manually, add the following URLs to ${fileName} under auth > redirect_urls and run\n`, - { - command: formatPackageManagerCommand( - localApp.packageManager, - `shopify app config push`, - ...pushCommandArgs, - ), - }, - '\n\n', - {list: {items: urls.redirectUrlWhitelist}}, - ], - }) - } else { - renderInfo({ - body: [ - 'To make URL updates manually, you can add the following URLs as redirects in your', - dashboardURL, - {char: ':'}, - '\n\n', - {list: {items: urls.redirectUrlWhitelist}}, - ], - }) - } - } -} - export async function renderDev({ processes, previewUrl, @@ -97,15 +37,6 @@ export async function renderDev({ } } -async function partnersURL(organizationId: string, appId: string) { - return { - link: { - label: 'Partners Dashboard', - url: `https://${await partnersFqdn()}/${organizationId}/apps/${appId}/edit`, - }, - } -} - async function renderDevNonInteractive({ processes, app: {canEnablePreviewMode}, diff --git a/packages/app/src/cli/services/dev/urls.test.ts b/packages/app/src/cli/services/dev/urls.test.ts index b0874807f27..a16185ea64e 100644 --- a/packages/app/src/cli/services/dev/urls.test.ts +++ b/packages/app/src/cli/services/dev/urls.test.ts @@ -16,6 +16,7 @@ import { import {UpdateURLsVariables} from '../../api/graphql/update_urls.js' import {setCachedAppInfo} from '../local-storage.js' import {patchAppConfigurationFile} from '../app/patch-app-configuration-file.js' +import {AppLinkedInterface} from '../../models/app/app.js' import {beforeEach, describe, expect, vi, test} from 'vitest' import {AbortError} from '@shopify/cli-kit/node/error' import {checkPortAvailability, getAvailableTCPPort} from '@shopify/cli-kit/node/tcp' @@ -314,7 +315,7 @@ describe('shouldOrPromptUpdateURLs', () => { currentURLs, appDirectory: '/path', apiKey: 'api-key', - localApp: testApp({configuration: {...DEFAULT_CONFIG, client_id: 'different'}}, 'current'), + localApp: testApp({configuration: {...DEFAULT_CONFIG, client_id: 'different'}}, 'current') as AppLinkedInterface, } vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) @@ -334,7 +335,7 @@ describe('shouldOrPromptUpdateURLs', () => { currentURLs, appDirectory: '/path', apiKey: 'api-key', - localApp, + localApp: localApp as AppLinkedInterface, } vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) diff --git a/packages/app/src/cli/services/dev/urls.ts b/packages/app/src/cli/services/dev/urls.ts index 42d8564b776..dca7def9c8e 100644 --- a/packages/app/src/cli/services/dev/urls.ts +++ b/packages/app/src/cli/services/dev/urls.ts @@ -1,10 +1,5 @@ import {updateURLsPrompt} from '../../prompts/dev.js' -import { - AppConfigurationInterface, - AppInterface, - CurrentAppConfiguration, - isCurrentAppSchema, -} from '../../models/app/app.js' +import {AppConfigurationInterface, AppLinkedInterface, CurrentAppConfiguration} from '../../models/app/app.js' import {UpdateURLsSchema, UpdateURLsVariables} from '../../api/graphql/update_urls.js' import {setCachedAppInfo} from '../local-storage.js' import {AppConfigurationUsedByCli} from '../../models/extensions/specifications/types/app_config.js' @@ -202,7 +197,7 @@ export async function updateURLs( throw new AbortError(errors) } - if (localApp && isCurrentAppSchema(localApp.configuration) && localApp.configuration.client_id === apiKey) { + if (localApp && localApp.configuration.client_id === apiKey) { const patch = { application_url: urls.applicationUrl, auth: { @@ -243,7 +238,7 @@ interface ShouldOrPromptUpdateURLsOptions { appDirectory: string cachedUpdateURLs?: boolean newApp?: boolean - localApp?: AppInterface + localApp?: AppLinkedInterface apiKey: string } @@ -258,7 +253,7 @@ export async function shouldOrPromptUpdateURLs(options: ShouldOrPromptUpdateURLs options.currentURLs.redirectUrlWhitelist, ) - if (options.localApp && isCurrentAppSchema(options.localApp.configuration)) { + if (options.localApp) { const localConfiguration = options.localApp.configuration localConfiguration.build = { ...localConfiguration.build, From 5156a87dfb3f2334cf8ad95815696e899b93392c Mon Sep 17 00:00:00 2001 From: Alex Montague Date: Tue, 14 Jan 2025 14:27:11 -0500 Subject: [PATCH 34/50] closes: https://github.com/Shopify/develop-app-inner-loop/issues/2460 - Updates the CLI banner message to include better next steps when an app could not be found Co-authored-by: Kenneth Ye --- packages/app/src/cli/services/context.test.ts | 112 ++++++++++++++---- packages/app/src/cli/services/context.ts | 45 ++++++- packages/app/src/cli/services/logs.test.ts | 4 +- 3 files changed, 129 insertions(+), 32 deletions(-) diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts index 9cd9a9be202..84fb72cf80b 100644 --- a/packages/app/src/cli/services/context.test.ts +++ b/packages/app/src/cli/services/context.test.ts @@ -2,13 +2,14 @@ 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' import {fetchSpecifications} from './generate/fetch-extension-specifications.js' import * as patchAppConfigurationFile from './app/patch-app-configuration-file.js' import {DeployOptions} from './deploy.js' +import {isServiceAccount, isUserAccount} from './context/partner-account-info.js' import { MinimalAppIdentifiers, AppApiKeyAndOrgId, @@ -22,15 +23,13 @@ import {selectOrganizationPrompt} from '../prompts/dev.js' import { DEFAULT_CONFIG, testDeveloperPlatformClient, - testApp, testAppWithConfig, testOrganizationApp, testThemeExtensions, - buildVersionedAppSchema, } from '../models/app/app.test-data.js' import metadata from '../metadata.js' import {AppConfigurationStateLinked, getAppConfigurationFileName, isWebType, loadApp} from '../models/app/loader.js' -import {AppInterface, AppLinkedInterface} from '../models/app/app.js' +import {AppLinkedInterface} from '../models/app/app.js' import * as loadSpecifications from '../models/extensions/load-specifications.js' import {DeveloperPlatformClient, selectDeveloperPlatformClient} from '../utilities/developer-platform-client.js' import {RemoteAwareExtensionSpecification} from '../models/extensions/specification.js' @@ -221,11 +220,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:', }) @@ -270,11 +269,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:', }) @@ -320,11 +319,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:', }) @@ -370,11 +369,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:', }) @@ -426,11 +425,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:', }) @@ -470,11 +469,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:', }) @@ -511,11 +510,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:', }) @@ -689,11 +688,72 @@ describe('ensureThemeExtensionDevContext', () => { }) }) -async function mockApp(directory: string, app?: Partial) { - 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'], + ], + }, + }, +] diff --git a/packages/app/src/cli/services/context.ts b/packages/app/src/cli/services/context.ts index 29ce599e74d..e3296723840 100644 --- a/packages/app/src/cli/services/context.ts +++ b/packages/app/src/cli/services/context.ts @@ -5,6 +5,7 @@ import {createExtension} from './dev/create-extension.js' import {CachedAppInfo} from './local-storage.js' import {patchAppConfigurationFile} from './app/patch-app-configuration-file.js' import {DeployOptions} from './deploy.js' +import {isServiceAccount, isUserAccount} from './context/partner-account-info.js' import {selectOrganizationPrompt} from '../prompts/dev.js' import { AppInterface, @@ -38,10 +39,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 = false) => [ + { + 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 { @@ -65,7 +86,23 @@ export const appFromIdentifiers = async (options: AppFromIdOptions): Promise { }, }, '\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:', }) From 4969c1f0a874f3b9d262d866f3c176b110f6ebc8 Mon Sep 17 00:00:00 2001 From: Shaun Stanworth Date: Wed, 15 Jan 2025 10:54:34 +0000 Subject: [PATCH 35/50] Fix error handler for invalid TOML files --- .changeset/small-guests-brake.md | 5 +++++ packages/app/src/cli/models/app/loader.ts | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) create mode 100644 .changeset/small-guests-brake.md diff --git a/.changeset/small-guests-brake.md b/.changeset/small-guests-brake.md new file mode 100644 index 00000000000..10fec700f94 --- /dev/null +++ b/.changeset/small-guests-brake.md @@ -0,0 +1,5 @@ +--- +'@shopify/app': patch +--- + +Better error message for certain types of invalid app TOML files diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index eb340a4fde2..58d80a73359 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -82,7 +82,7 @@ export async function loadConfigurationFileContent( // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (err: any) { // TOML errors have line, pos and col properties - if (err.line && err.pos && err.col) { + if (err.line !== undefined && err.pos !== undefined && err.col !== undefined) { return abortOrReport( outputContent`Fix the following error in ${outputToken.path(filepath)}:\n${err.message}`, {}, From 5c186d4d906b599bc6f53d69bcc66631b2e535c0 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Wed, 15 Jan 2025 13:07:44 +0200 Subject: [PATCH 36/50] Unify look of theme info with the rest of the CLI --- packages/theme/src/cli/commands/theme/info.ts | 3 +- packages/theme/src/cli/services/info.ts | 52 ++++++++++--------- 2 files changed, 30 insertions(+), 25 deletions(-) diff --git a/packages/theme/src/cli/commands/theme/info.ts b/packages/theme/src/cli/commands/theme/info.ts index 009715f7342..5cea2b107df 100644 --- a/packages/theme/src/cli/commands/theme/info.ts +++ b/packages/theme/src/cli/commands/theme/info.ts @@ -7,6 +7,7 @@ import {ensureAuthenticatedThemes} from '@shopify/cli-kit/node/session' import {AbortError} from '@shopify/cli-kit/node/error' import {globalFlags, jsonFlag} from '@shopify/cli-kit/node/cli' import {formatSection, outputInfo} from '@shopify/cli-kit/node/output' +import {renderInfo} from '@shopify/cli-kit/node/ui' export default class Info extends ThemeCommand { static description = @@ -52,7 +53,7 @@ export default class Info extends ThemeCommand { outputInfo(infoMessage) } else { const infoMessage = await fetchDevInfo({cliVersion: this.config.version}) - outputInfo(infoMessage) + renderInfo({customSections: infoMessage}) } } } diff --git a/packages/theme/src/cli/services/info.ts b/packages/theme/src/cli/services/info.ts index d71390a5baf..9228b6edf5b 100644 --- a/packages/theme/src/cli/services/info.ts +++ b/packages/theme/src/cli/services/info.ts @@ -6,9 +6,8 @@ import {checkForNewVersion} from '@shopify/cli-kit/node/node-package-manager' import {themeEditorUrl, themePreviewUrl} from '@shopify/cli-kit/node/themes/urls' import {Theme} from '@shopify/cli-kit/node/themes/types' import {AdminSession} from '@shopify/cli-kit/node/session' -import {linesToColumns} from '@shopify/cli-kit/common/string' -import {OutputMessage, formatSection} from '@shopify/cli-kit/node/output' import {getOutputUpdateCLIReminder} from '@shopify/cli-kit/node/upgrade' +import {AlertCustomSection} from '@shopify/cli-kit/node/ui' interface ThemeInfo { theme: { @@ -58,34 +57,39 @@ export async function fetchThemeInfo( return theme ? themeInfoJSON(theme, adminSession) : undefined } -export async function fetchDevInfo(config: {cliVersion: string}): Promise { - const sections: [string, string][] = [devConfigSection(), await systemInfoSection(config)] - const message = sections.map((sectionContents) => formatSection(...sectionContents)).join('\n\n') - return message +export async function fetchDevInfo(config: {cliVersion: string}): Promise { + return [devConfigSection(), await systemInfoSection(config)] } -function devConfigSection(): [string, string] { - const title = 'Theme Configuration' +function devConfigSection(): AlertCustomSection { const store = getThemeStore() || 'Not configured' - let developmentTheme = getDevelopmentTheme() - developmentTheme = developmentTheme ? `#${developmentTheme}` : 'Not set' - const lines: string[][] = [ - ['Store', store], - ['Development Theme ID', developmentTheme], - ] - return [title, linesToColumns(lines)] + const developmentTheme = getDevelopmentTheme() + return { + title: 'Theme Configuration', + body: { + tabularData: [ + ['Store', store], + ['Development Theme ID', developmentTheme ? `#${developmentTheme}` : {subdued: 'Not set'}], + ], + firstColumnSubdued: true, + }, + } } -async function systemInfoSection(config: {cliVersion: string}): Promise<[string, string]> { - const title = 'Tooling and System' +async function systemInfoSection(config: {cliVersion: string}): Promise { const {platform, arch} = platformAndArch() - const lines: string[][] = [ - ['Shopify CLI', await cliVersionInfo(config)], - ['OS', `${platform}-${arch}`], - ['Shell', process.env.SHELL || 'unknown'], - ['Node version', process.version], - ] - return [title, linesToColumns(lines)] + return { + title: 'Tooling and System', + body: { + tabularData: [ + ['Shopify CLI', await cliVersionInfo(config)], + ['OS', `${platform}-${arch}`], + ['Shell', process.env.SHELL || 'unknown'], + ['Node version', process.version], + ], + firstColumnSubdued: true, + }, + } } async function cliVersionInfo(config: {cliVersion: string}): Promise { From 6d3a4c7e2c9ab421426f64d90f078d7c969b1503 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Wed, 15 Jan 2025 13:09:25 +0200 Subject: [PATCH 37/50] Remove CLI upgrade reminder We have other reminders now, so this doesn't add much anymore. --- packages/theme/src/cli/services/info.ts | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/packages/theme/src/cli/services/info.ts b/packages/theme/src/cli/services/info.ts index 9228b6edf5b..f0a7053233d 100644 --- a/packages/theme/src/cli/services/info.ts +++ b/packages/theme/src/cli/services/info.ts @@ -2,11 +2,9 @@ import {getDevelopmentTheme, getThemeStore} from './local-storage.js' import {findOrSelectTheme} from '../utilities/theme-selector.js' import {DevelopmentThemeManager} from '../utilities/development-theme-manager.js' import {platformAndArch} from '@shopify/cli-kit/node/os' -import {checkForNewVersion} from '@shopify/cli-kit/node/node-package-manager' import {themeEditorUrl, themePreviewUrl} from '@shopify/cli-kit/node/themes/urls' import {Theme} from '@shopify/cli-kit/node/themes/types' import {AdminSession} from '@shopify/cli-kit/node/session' -import {getOutputUpdateCLIReminder} from '@shopify/cli-kit/node/upgrade' import {AlertCustomSection} from '@shopify/cli-kit/node/ui' interface ThemeInfo { @@ -82,7 +80,7 @@ async function systemInfoSection(config: {cliVersion: string}): Promise { - const dependency = '@shopify/cli' - const newestVersion = await checkForNewVersion(dependency, config.cliVersion) - if (!newestVersion) return config.cliVersion - const upgradeMessage = getOutputUpdateCLIReminder(newestVersion) - return [config.cliVersion, upgradeMessage].join(' ').trim() -} From 0db0b0629761da788f2345c3e0bc08692ba5a879 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Wed, 15 Jan 2025 13:13:18 +0200 Subject: [PATCH 38/50] Refactor for consistency and brevity --- packages/theme/src/cli/services/info.ts | 37 +++++++++++-------------- 1 file changed, 16 insertions(+), 21 deletions(-) diff --git a/packages/theme/src/cli/services/info.ts b/packages/theme/src/cli/services/info.ts index f0a7053233d..70cc54dbc61 100644 --- a/packages/theme/src/cli/services/info.ts +++ b/packages/theme/src/cli/services/info.ts @@ -5,7 +5,7 @@ import {platformAndArch} from '@shopify/cli-kit/node/os' import {themeEditorUrl, themePreviewUrl} from '@shopify/cli-kit/node/themes/urls' import {Theme} from '@shopify/cli-kit/node/themes/types' import {AdminSession} from '@shopify/cli-kit/node/session' -import {AlertCustomSection} from '@shopify/cli-kit/node/ui' +import {AlertCustomSection, InlineToken} from '@shopify/cli-kit/node/ui' interface ThemeInfo { theme: { @@ -62,30 +62,25 @@ export async function fetchDevInfo(config: {cliVersion: string}): Promise { const {platform, arch} = platformAndArch() + return tabularSection('Tooling and System', [ + ['Shopify CLI', config.cliVersion], + ['OS', `${platform}-${arch}`], + ['Shell', process.env.SHELL || 'unknown'], + ['Node version', process.version], + ]) +} + +function tabularSection(title: string, data: InlineToken[][]): AlertCustomSection { return { - title: 'Tooling and System', - body: { - tabularData: [ - ['Shopify CLI', config.cliVersion], - ['OS', `${platform}-${arch}`], - ['Shell', process.env.SHELL || 'unknown'], - ['Node version', process.version], - ], - firstColumnSubdued: true, - }, + title, + body: {tabularData: data, firstColumnSubdued: true}, } } From 1b8ad07153e96d4f8a1e72874f13af2f9a4eb9e9 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Wed, 15 Jan 2025 13:26:49 +0200 Subject: [PATCH 39/50] Add changeset --- .changeset/healthy-tips-raise.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/healthy-tips-raise.md diff --git a/.changeset/healthy-tips-raise.md b/.changeset/healthy-tips-raise.md new file mode 100644 index 00000000000..9e3b621305a --- /dev/null +++ b/.changeset/healthy-tips-raise.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': minor +--- + +Give theme info a facelift using standard UI components From 1a5aec270dab0a9f73af50e6d83a610eb80f575f Mon Sep 17 00:00:00 2001 From: Guilherme Carreiro Date: Wed, 15 Jan 2025 13:27:58 +0100 Subject: [PATCH 40/50] Fix `shopify theme dev` to no longer fail when development themes expire in internationalized stores --- .changeset/lemon-pants-add.md | 6 +++ .../src/public/node/themes/api.test.ts | 2 +- .../cli-kit/src/public/node/themes/api.ts | 39 ++++++++++--------- 3 files changed, 28 insertions(+), 19 deletions(-) create mode 100644 .changeset/lemon-pants-add.md diff --git a/.changeset/lemon-pants-add.md b/.changeset/lemon-pants-add.md new file mode 100644 index 00000000000..5b4b91e14fe --- /dev/null +++ b/.changeset/lemon-pants-add.md @@ -0,0 +1,6 @@ +--- +'@shopify/cli-kit': patch +'@shopify/theme': patch +--- + +Fix `shopify theme dev` to no longer fail when development themes expire in internationalized stores 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 66e2397786d..c32c2687cb0 100644 --- a/packages/cli-kit/src/public/node/themes/api.test.ts +++ b/packages/cli-kit/src/public/node/themes/api.test.ts @@ -59,7 +59,7 @@ describe('fetchTheme', () => { test('returns undefined when a theme is not found', async () => { const errorResponse = { status: 200, - errors: [{message: 'Theme does not exist'} as any], + errors: [{message: 'Tema não existe'} as any], } vi.mocked(adminRequestDoc).mockRejectedValue(new ClientError(errorResponse, {query: ''})) diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index 0423f433e00..b5665e44f6b 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -25,33 +25,36 @@ import {buildTheme} from '@shopify/cli-kit/node/themes/factories' import {Result, Checksum, Key, Theme, ThemeAsset, Operation} from '@shopify/cli-kit/node/themes/types' import {outputDebug} from '@shopify/cli-kit/node/output' import {sleep} from '@shopify/cli-kit/node/system' -import {ClientError} from 'graphql-request' export type ThemeParams = Partial> export type AssetParams = Pick & Partial> export async function fetchTheme(id: number, session: AdminSession): Promise { + const gid = composeThemeGid(id) + try { - const response = await adminRequestDoc(GetTheme, session, {id: composeThemeGid(id)}, undefined, { + const {theme} = await adminRequestDoc(GetTheme, session, {id: gid}, undefined, { handleErrors: false, }) - const {theme} = response - if (!theme) { - return undefined - } - return buildTheme({ - id: parseGid(theme.id), - processing: theme.processing, - role: theme.role.toLowerCase(), - name: theme.name, - }) - } catch (error) { - if (error instanceof ClientError) { - if (error.response?.errors?.[0]?.message === 'Theme does not exist') { - return undefined - } + + if (theme) { + return buildTheme({ + id: parseGid(theme.id), + processing: theme.processing, + role: theme.role.toLowerCase(), + name: theme.name, + }) } - throw new AbortError(`Failed to fetch theme: ${id}`) + + // eslint-disable-next-line no-catch-all/no-catch-all + } catch (_error) { + /** + * Consumers of this and other theme APIs in this file expect either a theme + * or `undefined`. + * + * Error handlers should not inspect GraphQL error messages directly, as + * they are internationalized. + */ } } From 25f98610465f5b8ec81370d533e67938e21de5b2 Mon Sep 17 00:00:00 2001 From: James Meng Date: Wed, 15 Jan 2025 09:03:32 -0800 Subject: [PATCH 41/50] Add debug log to fetchTheme errorHandler --- packages/cli-kit/src/public/node/themes/api.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index b5665e44f6b..1efb70abfec 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -55,6 +55,7 @@ export async function fetchTheme(id: number, session: AdminSession): Promise Date: Wed, 15 Jan 2025 09:37:47 -0800 Subject: [PATCH 42/50] Replace isStorefrontPasswordProtected with API call We were previously determining whether or not a storefront was password protected by making a HTTP request to the storefront and checking the response status. This worked for us in the past but is a known fragile piece. We've now shipped the ability to query whether the storefront is password protected directly in the Admin API so we no longer need to guess. This replaces all instances of `isStorefrontPasswordProtected` with the updated API call. --- .changeset/young-windows-deliver.md | 7 ++ .../dev/processes/theme-app-extension.ts | 2 +- .../online_store_password_protection.ts | 45 ++++++++++ .../online_store_password_protection.graphql | 7 ++ .../cli-kit/src/public/node/themes/api.ts | 12 +++ packages/theme/src/cli/services/console.ts | 2 +- packages/theme/src/cli/services/dev.ts | 5 +- .../storefront-session.test.ts | 89 +++---------------- .../theme-environment/storefront-session.ts | 11 +-- 9 files changed, 90 insertions(+), 90 deletions(-) create mode 100644 .changeset/young-windows-deliver.md create mode 100644 packages/cli-kit/src/cli/api/graphql/admin/generated/online_store_password_protection.ts create mode 100644 packages/cli-kit/src/cli/api/graphql/admin/queries/online_store_password_protection.graphql diff --git a/.changeset/young-windows-deliver.md b/.changeset/young-windows-deliver.md new file mode 100644 index 00000000000..352812888ef --- /dev/null +++ b/.changeset/young-windows-deliver.md @@ -0,0 +1,7 @@ +--- +'@shopify/cli-kit': patch +'@shopify/theme': patch +'@shopify/app': patch +--- + +Utilize Admin API to determine if a storefront is password protected diff --git a/packages/app/src/cli/services/dev/processes/theme-app-extension.ts b/packages/app/src/cli/services/dev/processes/theme-app-extension.ts index b7d826d1ed2..25c427c93e0 100644 --- a/packages/app/src/cli/services/dev/processes/theme-app-extension.ts +++ b/packages/app/src/cli/services/dev/processes/theme-app-extension.ts @@ -52,7 +52,7 @@ export async function setupPreviewThemeAppExtensionsProcess( ]) const storeFqdn = adminSession.storeFqdn - const storefrontPassword = (await isStorefrontPasswordProtected(storeFqdn)) + const storefrontPassword = (await isStorefrontPasswordProtected(adminSession)) ? await ensureValidPassword('', storeFqdn) : undefined diff --git a/packages/cli-kit/src/cli/api/graphql/admin/generated/online_store_password_protection.ts b/packages/cli-kit/src/cli/api/graphql/admin/generated/online_store_password_protection.ts new file mode 100644 index 00000000000..981e88f6f24 --- /dev/null +++ b/packages/cli-kit/src/cli/api/graphql/admin/generated/online_store_password_protection.ts @@ -0,0 +1,45 @@ +/* eslint-disable @typescript-eslint/consistent-type-definitions */ +import * as Types from './types.js' + +import {TypedDocumentNode as DocumentNode} from '@graphql-typed-document-node/core' + +export type OnlineStorePasswordProtectionQueryVariables = Types.Exact<{[key: string]: never}> + +export type OnlineStorePasswordProtectionQuery = {onlineStore: {passwordProtection: {enabled: boolean}}} + +export const OnlineStorePasswordProtection = { + kind: 'Document', + definitions: [ + { + kind: 'OperationDefinition', + operation: 'query', + name: {kind: 'Name', value: 'OnlineStorePasswordProtection'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + { + kind: 'Field', + name: {kind: 'Name', value: 'onlineStore'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + { + kind: 'Field', + name: {kind: 'Name', value: 'passwordProtection'}, + selectionSet: { + kind: 'SelectionSet', + selections: [ + {kind: 'Field', name: {kind: 'Name', value: 'enabled'}}, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + {kind: 'Field', name: {kind: 'Name', value: '__typename'}}, + ], + }, + }, + ], + }, + }, + ], +} as unknown as DocumentNode diff --git a/packages/cli-kit/src/cli/api/graphql/admin/queries/online_store_password_protection.graphql b/packages/cli-kit/src/cli/api/graphql/admin/queries/online_store_password_protection.graphql new file mode 100644 index 00000000000..e1b58c0d502 --- /dev/null +++ b/packages/cli-kit/src/cli/api/graphql/admin/queries/online_store_password_protection.graphql @@ -0,0 +1,7 @@ +query OnlineStorePasswordProtection { + onlineStore { + passwordProtection { + enabled + } + } +} diff --git a/packages/cli-kit/src/public/node/themes/api.ts b/packages/cli-kit/src/public/node/themes/api.ts index b5665e44f6b..3612bc93789 100644 --- a/packages/cli-kit/src/public/node/themes/api.ts +++ b/packages/cli-kit/src/public/node/themes/api.ts @@ -18,6 +18,7 @@ import { import {MetafieldDefinitionsByOwnerType} from '../../../cli/api/graphql/admin/generated/metafield_definitions_by_owner_type.js' import {GetThemes} from '../../../cli/api/graphql/admin/generated/get_themes.js' import {GetTheme} from '../../../cli/api/graphql/admin/generated/get_theme.js' +import {OnlineStorePasswordProtection} from '../../../cli/api/graphql/admin/generated/online_store_password_protection.js' import {restRequest, RestResponse, adminRequestDoc} from '@shopify/cli-kit/node/api/admin' import {AdminSession} from '@shopify/cli-kit/node/session' import {AbortError} from '@shopify/cli-kit/node/error' @@ -356,6 +357,17 @@ export async function metafieldDefinitionsByOwnerType(type: MetafieldOwnerType, })) } +export async function passwordProtected(session: AdminSession): Promise { + const {onlineStore} = await adminRequestDoc(OnlineStorePasswordProtection, session) + if (!onlineStore) { + unexpectedGraphQLError("Unable to get details about the storefront's password protection") + } + + const {passwordProtection} = onlineStore + + return passwordProtection.enabled +} + async function request( method: string, path: string, diff --git a/packages/theme/src/cli/services/console.ts b/packages/theme/src/cli/services/console.ts index f35d98bcbd6..a0bb806d552 100644 --- a/packages/theme/src/cli/services/console.ts +++ b/packages/theme/src/cli/services/console.ts @@ -9,7 +9,7 @@ import {consoleLog} from '@shopify/cli-kit/node/output' export async function ensureReplEnv(adminSession: AdminSession, storePasswordFlag?: string) { const themeId = await findOrCreateReplTheme(adminSession) - const storePassword = (await isStorefrontPasswordProtected(adminSession.storeFqdn)) + const storePassword = (await isStorefrontPasswordProtected(adminSession)) ? await ensureValidPassword(storePasswordFlag, adminSession.storeFqdn) : undefined diff --git a/packages/theme/src/cli/services/dev.ts b/packages/theme/src/cli/services/dev.ts index aec35d21711..93579799975 100644 --- a/packages/theme/src/cli/services/dev.ts +++ b/packages/theme/src/cli/services/dev.ts @@ -42,9 +42,8 @@ export async function dev(options: DevOptions) { return } - const storefrontPasswordPromise = isStorefrontPasswordProtected(options.adminSession.storeFqdn).then( - (needsPassword) => - needsPassword ? ensureValidPassword(options.storePassword, options.adminSession.storeFqdn) : undefined, + const storefrontPasswordPromise = await isStorefrontPasswordProtected(options.adminSession).then((needsPassword) => + needsPassword ? ensureValidPassword(options.storePassword, options.adminSession.storeFqdn) : undefined, ) const localThemeExtensionFileSystem = emptyThemeExtFileSystem() diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts index 6727cdfd592..2081259d8b6 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-session.test.ts @@ -7,96 +7,29 @@ import { import {describe, expect, test, vi} from 'vitest' import {fetch} from '@shopify/cli-kit/node/http' import {AbortError} from '@shopify/cli-kit/node/error' +import {passwordProtected} from '@shopify/cli-kit/node/themes/api' +import {type AdminSession} from '@shopify/cli-kit/node/session' vi.mock('@shopify/cli-kit/node/http') +vi.mock('@shopify/cli-kit/node/themes/api') describe('Storefront API', () => { describe('isStorefrontPasswordProtected', () => { - test('returns true when the request is redirected to the password page', async () => { - // Given - vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/password'})) - - // When - const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') - - // Then - expect(isProtected).toBe(true) - expect(fetch).toBeCalledWith('https://store.myshopify.com', { - method: 'GET', - }) - }) - - test('returns false when request is not redirected', async () => { - // Given - vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com'})) - - // When - const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') - - // Then - expect(isProtected).toBe(false) - expect(fetch).toBeCalledWith('https://store.myshopify.com', { - method: 'GET', - }) - }) - - test('returns false when store redirects to a different domain', async () => { - // Given - vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.se'})) - - // When - const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') - - // Then - expect(isProtected).toBe(false) - }) + const adminSession: AdminSession = { + storeFqdn: 'example-store.myshopify.com', + token: '123456', + } - test('returns false when store redirects to a different URI', async () => { + test('makes an API call to check if the storefront is password protected', async () => { // Given - vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/random'})) + vi.mocked(passwordProtected).mockResolvedValueOnce(true) // When - const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') - - // Then - expect(isProtected).toBe(false) - }) - - test('return true when store redirects to //password', async () => { - // Given - vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/fr-CA/password'})) - - // When - const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') + const isProtected = await isStorefrontPasswordProtected(adminSession) // Then expect(isProtected).toBe(true) - }) - - test('returns false if response is not a 302', async () => { - // Given - vi.mocked(fetch).mockResolvedValue(response({status: 200, url: 'https://store.myshopify.com/random'})) - - // When - const isProtected = await isStorefrontPasswordProtected('store.myshopify.com') - - // Then - expect(isProtected).toBe(false) - }) - - test('ignores query params', async () => { - // Given - vi.mocked(fetch) - .mockResolvedValueOnce(response({status: 200, url: 'https://store.myshopify.com/random?a=b'})) - .mockResolvedValueOnce(response({status: 200, url: 'https://store.myshopify.com/password?a=b'})) - - // When - const redirectToRandomPath = await isStorefrontPasswordProtected('store.myshopify.com') - const redirectToPasswordPath = await isStorefrontPasswordProtected('store.myshopify.com') - - // Then - expect(redirectToRandomPath).toBe(false) - expect(redirectToPasswordPath).toBe(true) + expect(passwordProtected).toHaveBeenCalledWith(adminSession) }) }) diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts index b40fd26d605..aee71ee4305 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-session.ts @@ -3,16 +3,13 @@ import {defaultHeaders} from './storefront-utils.js' import {fetch} from '@shopify/cli-kit/node/http' import {AbortError} from '@shopify/cli-kit/node/error' import {outputDebug} from '@shopify/cli-kit/node/output' +import {type AdminSession} from '@shopify/cli-kit/node/session' +import {passwordProtected} from '@shopify/cli-kit/node/themes/api' export class ShopifyEssentialError extends Error {} -export async function isStorefrontPasswordProtected(storeURL: string): Promise { - const response = await fetch(prependHttps(storeURL), { - method: 'GET', - }) - - const redirectLocation = new URL(response.url) - return redirectLocation.pathname.endsWith('/password') +export async function isStorefrontPasswordProtected(session: AdminSession): Promise { + return passwordProtected(session) } /** From 451099291c6a5235c28084c1b488d618993863e8 Mon Sep 17 00:00:00 2001 From: Gray Gilmore Date: Wed, 15 Jan 2025 09:41:06 -0800 Subject: [PATCH 43/50] Run pnpm graphql-codegen --- packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts b/packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts index ab109b34c12..6a831d59181 100644 --- a/packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts +++ b/packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts @@ -250,7 +250,7 @@ export type ThemeRole = * publishing are restricted until the merchant resolves the licensing issue. */ | 'LOCKED' - /** TThe currently published theme. There can only be one main theme at any time. */ + /** The currently published theme. There can only be one main theme at any time. */ | 'MAIN' /** The currently published theme that is only accessible to a mobile client. */ | 'MOBILE' From 5be14a5b08ed172352b02c1a0bd21e07a87f3788 Mon Sep 17 00:00:00 2001 From: Nic Day Date: Wed, 15 Jan 2025 20:47:08 +0000 Subject: [PATCH 44/50] Fix documentation. Download -> Push --- .changeset/short-tigers-trade.md | 5 +++++ docs-shopify.dev/commands/interfaces/theme-push.interface.ts | 2 +- docs-shopify.dev/generated/generated_docs_data.json | 4 ++-- packages/cli/README.md | 2 +- packages/cli/oclif.manifest.json | 2 +- packages/theme/src/cli/commands/theme/push.ts | 2 +- packages/theme/src/cli/services/push.ts | 2 +- 7 files changed, 12 insertions(+), 7 deletions(-) create mode 100644 .changeset/short-tigers-trade.md diff --git a/.changeset/short-tigers-trade.md b/.changeset/short-tigers-trade.md new file mode 100644 index 00000000000..2e6315e7fb6 --- /dev/null +++ b/.changeset/short-tigers-trade.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Update documentation for `theme push --only` diff --git a/docs-shopify.dev/commands/interfaces/theme-push.interface.ts b/docs-shopify.dev/commands/interfaces/theme-push.interface.ts index 24b51d648a3..91511cea64a 100644 --- a/docs-shopify.dev/commands/interfaces/theme-push.interface.ts +++ b/docs-shopify.dev/commands/interfaces/theme-push.interface.ts @@ -49,7 +49,7 @@ export interface themepush { '-n, --nodelete'?: '' /** - * Download only the specified files (Multiple flags allowed). + * Push only the specified files (Multiple flags allowed). * @environment SHOPIFY_FLAG_ONLY */ '-o, --only '?: string diff --git a/docs-shopify.dev/generated/generated_docs_data.json b/docs-shopify.dev/generated/generated_docs_data.json index a63ead60ab6..d4032cee3f7 100644 --- a/docs-shopify.dev/generated/generated_docs_data.json +++ b/docs-shopify.dev/generated/generated_docs_data.json @@ -6186,7 +6186,7 @@ "syntaxKind": "PropertySignature", "name": "-o, --only ", "value": "string", - "description": "Download only the specified files (Multiple flags allowed).", + "description": "Push only the specified files (Multiple flags allowed).", "isOptional": true, "environmentValue": "SHOPIFY_FLAG_ONLY" }, @@ -6236,7 +6236,7 @@ "environmentValue": "SHOPIFY_FLAG_IGNORE" } ], - "value": "export interface themepush {\n /**\n * Allow push to a live theme.\n * @environment SHOPIFY_FLAG_ALLOW_LIVE\n */\n '-a, --allow-live'?: ''\n\n /**\n * Push theme files from your remote development theme.\n * @environment SHOPIFY_FLAG_DEVELOPMENT\n */\n '-d, --development'?: ''\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment '?: string\n\n /**\n * Skip downloading the specified files (Multiple flags allowed).\n * @environment SHOPIFY_FLAG_IGNORE\n */\n '-x, --ignore '?: string\n\n /**\n * Output the result as JSON.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Push theme files from your remote live theme.\n * @environment SHOPIFY_FLAG_LIVE\n */\n '-l, --live'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Prevent deleting remote files that don't exist locally.\n * @environment SHOPIFY_FLAG_NODELETE\n */\n '-n, --nodelete'?: ''\n\n /**\n * Download only the specified files (Multiple flags allowed).\n * @environment SHOPIFY_FLAG_ONLY\n */\n '-o, --only '?: string\n\n /**\n * Password generated from the Theme Access app.\n * @environment SHOPIFY_CLI_THEME_TOKEN\n */\n '--password '?: string\n\n /**\n * The path to your theme directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Publish as the live theme after uploading.\n * @environment SHOPIFY_FLAG_PUBLISH\n */\n '-p, --publish'?: ''\n\n /**\n * Store URL. It can be the store prefix (example) or the full myshopify.com URL (example.myshopify.com, https://example.myshopify.com).\n * @environment SHOPIFY_FLAG_STORE\n */\n '-s, --store '?: string\n\n /**\n * Require theme check to pass without errors before pushing. Warnings are allowed.\n * @environment SHOPIFY_FLAG_STRICT_PUSH\n */\n '--strict'?: ''\n\n /**\n * Theme ID or name of the remote theme.\n * @environment SHOPIFY_FLAG_THEME_ID\n */\n '-t, --theme '?: string\n\n /**\n * Create a new unpublished theme and push to it.\n * @environment SHOPIFY_FLAG_UNPUBLISHED\n */\n '-u, --unpublished'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" + "value": "export interface themepush {\n /**\n * Allow push to a live theme.\n * @environment SHOPIFY_FLAG_ALLOW_LIVE\n */\n '-a, --allow-live'?: ''\n\n /**\n * Push theme files from your remote development theme.\n * @environment SHOPIFY_FLAG_DEVELOPMENT\n */\n '-d, --development'?: ''\n\n /**\n * The environment to apply to the current command.\n * @environment SHOPIFY_FLAG_ENVIRONMENT\n */\n '-e, --environment '?: string\n\n /**\n * Skip downloading the specified files (Multiple flags allowed).\n * @environment SHOPIFY_FLAG_IGNORE\n */\n '-x, --ignore '?: string\n\n /**\n * Output the result as JSON.\n * @environment SHOPIFY_FLAG_JSON\n */\n '-j, --json'?: ''\n\n /**\n * Push theme files from your remote live theme.\n * @environment SHOPIFY_FLAG_LIVE\n */\n '-l, --live'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * Prevent deleting remote files that don't exist locally.\n * @environment SHOPIFY_FLAG_NODELETE\n */\n '-n, --nodelete'?: ''\n\n /**\n * Push only the specified files (Multiple flags allowed).\n * @environment SHOPIFY_FLAG_ONLY\n */\n '-o, --only '?: string\n\n /**\n * Password generated from the Theme Access app.\n * @environment SHOPIFY_CLI_THEME_TOKEN\n */\n '--password '?: string\n\n /**\n * The path to your theme directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Publish as the live theme after uploading.\n * @environment SHOPIFY_FLAG_PUBLISH\n */\n '-p, --publish'?: ''\n\n /**\n * Store URL. It can be the store prefix (example) or the full myshopify.com URL (example.myshopify.com, https://example.myshopify.com).\n * @environment SHOPIFY_FLAG_STORE\n */\n '-s, --store '?: string\n\n /**\n * Require theme check to pass without errors before pushing. Warnings are allowed.\n * @environment SHOPIFY_FLAG_STRICT_PUSH\n */\n '--strict'?: ''\n\n /**\n * Theme ID or name of the remote theme.\n * @environment SHOPIFY_FLAG_THEME_ID\n */\n '-t, --theme '?: string\n\n /**\n * Create a new unpublished theme and push to it.\n * @environment SHOPIFY_FLAG_UNPUBLISHED\n */\n '-u, --unpublished'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" } } } diff --git a/packages/cli/README.md b/packages/cli/README.md index 587e511fb83..d2d2b28efbf 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -2166,7 +2166,7 @@ FLAGS -j, --json Output the result as JSON. -l, --live Push theme files from your remote live theme. -n, --nodelete Prevent deleting remote files that don't exist locally. - -o, --only=... Download only the specified files (Multiple flags allowed). + -o, --only=... Push only the specified files (Multiple flags allowed). -p, --publish Publish as the live theme after uploading. -s, --store= Store URL. It can be the store prefix (example) or the full myshopify.com URL (example.myshopify.com, https://example.myshopify.com). diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 901847d77e9..3d419b578bc 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -6123,7 +6123,7 @@ }, "only": { "char": "o", - "description": "Download only the specified files (Multiple flags allowed).", + "description": "Push only the specified files (Multiple flags allowed).", "env": "SHOPIFY_FLAG_ONLY", "hasDynamicHelp": false, "multiple": true, diff --git a/packages/theme/src/cli/commands/theme/push.ts b/packages/theme/src/cli/commands/theme/push.ts index 0d4cbdec097..1303f33563c 100644 --- a/packages/theme/src/cli/commands/theme/push.ts +++ b/packages/theme/src/cli/commands/theme/push.ts @@ -71,7 +71,7 @@ export default class Push extends ThemeCommand { }), only: Flags.string({ char: 'o', - description: 'Download only the specified files (Multiple flags allowed).', + description: 'Push only the specified files (Multiple flags allowed).', multiple: true, env: 'SHOPIFY_FLAG_ONLY', }), diff --git a/packages/theme/src/cli/services/push.ts b/packages/theme/src/cli/services/push.ts index 078b9f74825..5a44dde6f47 100644 --- a/packages/theme/src/cli/services/push.ts +++ b/packages/theme/src/cli/services/push.ts @@ -71,7 +71,7 @@ export interface PushFlags { /** Runs the push command without deleting local files. */ nodelete?: boolean - /** Download only the specified files (Multiple flags allowed). */ + /** Push only the specified files (Multiple flags allowed). */ only?: string[] /** Skip downloading the specified files (Multiple flags allowed). */ From c68928d0e1e83ec397ff0a2f5b943f588b78bbb3 Mon Sep 17 00:00:00 2001 From: Alex Montague Date: Wed, 15 Jan 2025 17:02:32 -0500 Subject: [PATCH 45/50] run graphql codegen Co-authored-by: Kenneth Ye --- packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts b/packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts index ab109b34c12..6a831d59181 100644 --- a/packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts +++ b/packages/cli-kit/src/cli/api/graphql/admin/generated/types.d.ts @@ -250,7 +250,7 @@ export type ThemeRole = * publishing are restricted until the merchant resolves the licensing issue. */ | 'LOCKED' - /** TThe currently published theme. There can only be one main theme at any time. */ + /** The currently published theme. There can only be one main theme at any time. */ | 'MAIN' /** The currently published theme that is only accessible to a mobile client. */ | 'MOBILE' From 7b5b5a1e0d2b43e2eef51f09db70ee459156dd6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Wed, 15 Jan 2025 13:11:46 +0100 Subject: [PATCH 46/50] Don't show dev shorcuts until dev session is ready --- .../cli/services/dev/processes/dev-session.ts | 6 ++ .../services/dev/ui/components/Dev.test.tsx | 68 ++++++++++++++++++- .../cli/services/dev/ui/components/Dev.tsx | 44 ++++++++++-- 3 files changed, 111 insertions(+), 7 deletions(-) diff --git a/packages/app/src/cli/services/dev/processes/dev-session.ts b/packages/app/src/cli/services/dev/processes/dev-session.ts index 83dd54392d7..d85ac2e09e0 100644 --- a/packages/app/src/cli/services/dev/processes/dev-session.ts +++ b/packages/app/src/cli/services/dev/processes/dev-session.ts @@ -59,6 +59,12 @@ let bundleControllers: AbortController[] = [] // Since the watcher can emit events before the dev session is ready, we need to keep track of the status let isDevSessionReady = false +export function devSessionStatus() { + return { + isDevSessionReady, + } +} + export async function setupDevSessionProcess({ app, apiKey, diff --git a/packages/app/src/cli/services/dev/ui/components/Dev.test.tsx b/packages/app/src/cli/services/dev/ui/components/Dev.test.tsx index 684d6136f70..134eac3dff5 100644 --- a/packages/app/src/cli/services/dev/ui/components/Dev.test.tsx +++ b/packages/app/src/cli/services/dev/ui/components/Dev.test.tsx @@ -1,6 +1,7 @@ import {calculatePrefixColumnSize, Dev} from './Dev.js' import {fetchAppPreviewMode} from '../../fetch.js' import {testDeveloperPlatformClient, testUIExtension} from '../../../../models/app/app.test-data.js' +import {devSessionStatus} from '../../processes/dev-session.js' import { getLastFrameAfterUnmount, render, @@ -12,7 +13,7 @@ import { } from '@shopify/cli-kit/node/testing/ui' import {AbortController, AbortSignal} from '@shopify/cli-kit/node/abort' import React from 'react' -import {describe, expect, test, vi} from 'vitest' +import {describe, expect, test, vi, beforeEach} from 'vitest' import {unstyled} from '@shopify/cli-kit/node/output' import {openURL} from '@shopify/cli-kit/node/system' import {Writable} from 'stream' @@ -20,6 +21,7 @@ import {Writable} from 'stream' vi.mock('@shopify/cli-kit/node/system') vi.mock('../../../context.js') vi.mock('../../fetch.js') +vi.mock('../../processes/dev-session.js') const developerPlatformClient = testDeveloperPlatformClient() @@ -40,6 +42,10 @@ const developerPreview = { } describe('Dev', () => { + beforeEach(() => { + vi.mocked(devSessionStatus).mockReturnValue({isDevSessionReady: true}) + }) + test('renders a stream of concurrent outputs from sub-processes, shortcuts and a preview url', async () => { // Given let backendPromiseResolve: () => void @@ -968,6 +974,66 @@ describe('Dev', () => { // unmount so that polling is cleared after every test renderInstance.unmount() }) + + test('updates UI when devSessionEnabled changes from false to true', async () => { + // Given + vi.mocked(devSessionStatus).mockReturnValue({isDevSessionReady: false}) + + const renderInstance = render( + , + ) + + // Initial state - dev session not ready + expect(unstyled(renderInstance.lastFrame()!).replace(/\d/g, '0')).toMatchInlineSnapshot(` + " + ──────────────────────────────────────────────────────────────────────────────────────────────────── + + › Press q │ quit + + Preview URL: https://shopify.com + GraphiQL URL: http://localhost:0000/graphiql + " + `) + + // When dev session becomes ready + vi.mocked(devSessionStatus).mockReturnValue({isDevSessionReady: true}) + + // Wait for the polling interval to update the UI + await waitForContent(renderInstance, 'preview in your browser') + + // Then - preview shortcut should be visible + expect(unstyled(renderInstance.lastFrame()!).replace(/\d/g, '0')).toMatchInlineSnapshot(` + " + ──────────────────────────────────────────────────────────────────────────────────────────────────── + + › Press g │ open GraphiQL (Admin API) in your browser + › Press p │ preview in your browser + › Press q │ quit + + Preview URL: https://shopify.com + GraphiQL URL: http://localhost:0000/graphiql + " + `) + + // unmount so that polling is cleared after every test + renderInstance.unmount() + }) }) describe('calculatePrefixColumnSize', () => { diff --git a/packages/app/src/cli/services/dev/ui/components/Dev.tsx b/packages/app/src/cli/services/dev/ui/components/Dev.tsx index 32c38675d2a..ef634f0eee9 100644 --- a/packages/app/src/cli/services/dev/ui/components/Dev.tsx +++ b/packages/app/src/cli/services/dev/ui/components/Dev.tsx @@ -1,6 +1,7 @@ import metadata from '../../../../metadata.js' import {DeveloperPlatformClient} from '../../../../utilities/developer-platform-client.js' import {ExtensionInstance} from '../../../../models/extensions/extension-instance.js' +import {devSessionStatus} from '../../processes/dev-session.js' import {OutputProcess} from '@shopify/cli-kit/node/output' import {ConcurrentOutput} from '@shopify/cli-kit/node/ui/components' import {useAbortSignal} from '@shopify/cli-kit/node/ui/hooks' @@ -64,6 +65,7 @@ const Dev: FunctionComponent = ({ const {isRawModeSupported: canUseShortcuts} = useStdin() const pollingInterval = useRef() + const devSessionPollingInterval = useRef() const localhostGraphiqlUrl = `http://localhost:${graphiqlPort}/graphiql` const defaultStatusMessage = `Preview URL: ${previewUrl}${ graphiqlUrl ? `\nGraphiQL URL: ${localhostGraphiqlUrl}` : '' @@ -83,11 +85,13 @@ const Dev: FunctionComponent = ({ }, 2000) } clearInterval(pollingInterval.current) + clearInterval(devSessionPollingInterval.current) await app.developerPlatformClient.devSessionDelete({appId: app.id, shopFqdn}) await developerPreview.disable() }) const [devPreviewEnabled, setDevPreviewEnabled] = useState(true) + const [devSessionEnabled, setDevSessionEnabled] = useState(devSessionStatus().isDevSessionReady) const [error, setError] = useState(undefined) const errorHandledProcesses = useMemo(() => { @@ -106,6 +110,32 @@ const Dev: FunctionComponent = ({ }) }, [processes, abortController]) + /* + * Poll Dev Session status + * + * Polling mechanism to check if the dev session is ready. + * When the session is ready, the polling stops and the shortcuts are shown. + * Reason is that shortcuts won't work properly until the session is ready and the app is installed. + * + * This only applies for App Management dev-sessions. + */ + useEffect(() => { + const pollDevSession = async () => { + const {isDevSessionReady} = devSessionStatus() + setDevSessionEnabled(isDevSessionReady) + if (isDevSessionReady) clearInterval(devSessionPollingInterval.current) + } + + if (app.developerPlatformClient.supportsDevSessions) { + // eslint-disable-next-line @typescript-eslint/no-misused-promises + devSessionPollingInterval.current = setInterval(pollDevSession, 200) + } else { + setDevSessionEnabled(true) + } + + return () => clearInterval(devSessionPollingInterval.current) + }, [devSessionStatus]) + useEffect(() => { const pollDevPreviewMode = async () => { try { @@ -160,12 +190,12 @@ const Dev: FunctionComponent = ({ try { setError('') - if (input === 'p' && previewUrl) { + if (input === 'p' && previewUrl && devSessionEnabled) { await metadata.addPublicMetadata(() => ({ cmd_dev_preview_url_opened: true, })) await openURL(previewUrl) - } else if (input === 'g' && graphiqlUrl) { + } else if (input === 'g' && graphiqlUrl && devSessionEnabled) { await metadata.addPublicMetadata(() => ({ cmd_dev_graphiql_opened: true, })) @@ -244,15 +274,17 @@ const Dev: FunctionComponent = ({ {devPreviewEnabled ? ✔ on : ✖ off} ) : null} - {graphiqlUrl ? ( + {graphiqlUrl && devSessionEnabled ? ( {figures.pointerSmall} Press g {figures.lineVertical} open GraphiQL (Admin API) in your browser ) : null} - - {figures.pointerSmall} Press p {figures.lineVertical} preview in your browser - + {devSessionEnabled ? ( + + {figures.pointerSmall} Press p {figures.lineVertical} preview in your browser + + ) : null} {figures.pointerSmall} Press q {figures.lineVertical} quit From d509fb0fa5ebc8beccb5c2d0d7977f3189328ae8 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Mon, 2 Dec 2024 00:46:14 +0200 Subject: [PATCH 47/50] Remove all conditional logic around App Management being disabled --- .../src/cli/services/deploy/bundle.test.ts | 42 +--------- .../app/src/cli/services/deploy/bundle.ts | 13 ++- .../app/src/cli/services/dev/fetch.test.ts | 28 ++----- .../utilities/developer-platform-client.ts | 11 +-- .../cli-kit/src/private/node/api/headers.ts | 2 +- .../cli-kit/src/private/node/constants.ts | 1 - packages/cli-kit/src/private/node/session.ts | 83 +------------------ .../src/private/node/session/exchange.ts | 4 +- .../src/private/node/session/scopes.test.ts | 8 +- .../src/private/node/session/scopes.ts | 15 ++-- .../src/public/node/context/local.test.ts | 25 ------ .../cli-kit/src/public/node/context/local.ts | 10 --- 12 files changed, 31 insertions(+), 211 deletions(-) diff --git a/packages/app/src/cli/services/deploy/bundle.test.ts b/packages/app/src/cli/services/deploy/bundle.test.ts index c35c5db44f9..10079ac2539 100644 --- a/packages/app/src/cli/services/deploy/bundle.test.ts +++ b/packages/app/src/cli/services/deploy/bundle.test.ts @@ -8,11 +8,10 @@ import {joinPath} from '@shopify/cli-kit/node/path' describe('bundleAndBuildExtensions', () => { let app: AppInterface - test('generates a manifest.json when App Management is enabled', async () => { + test('generates a manifest.json', async () => { await file.inTemporaryDirectory(async (tmpDir: string) => { // Given vi.spyOn(file, 'writeFileSync').mockResolvedValue(undefined) - const envVars = {USE_APP_MANAGEMENT_API: 'true'} const bundlePath = joinPath(tmpDir, 'bundle.zip') const uiExtension = await testUIExtension({type: 'web_pixel_extension'}) @@ -60,7 +59,7 @@ describe('bundleAndBuildExtensions', () => { } // When - await bundleAndBuildExtensions({app, identifiers, bundlePath}, envVars) + await bundleAndBuildExtensions({app, identifiers, bundlePath}) // Then expect(extensionBundleMock).toHaveBeenCalledTimes(2) @@ -73,41 +72,6 @@ describe('bundleAndBuildExtensions', () => { }) }) - test('does not generate the manifest.json when App Management is disabled', async () => { - await file.inTemporaryDirectory(async (tmpDir: string) => { - // Given - vi.spyOn(file, 'writeFileSync').mockResolvedValue(undefined) - const bundlePath = joinPath(tmpDir, 'bundle.zip') - - const uiExtension = await testUIExtension({type: 'web_pixel_extension'}) - const extensionBundleMock = vi.fn() - uiExtension.buildForBundle = extensionBundleMock - const themeExtension = await testThemeExtensions() - themeExtension.buildForBundle = extensionBundleMock - app = testApp({allExtensions: [uiExtension, themeExtension]}) - - const extensions: {[key: string]: string} = {} - for (const extension of app.allExtensions) { - extensions[extension.localIdentifier] = extension.localIdentifier - } - const identifiers = { - app: 'app-id', - extensions, - extensionIds: {}, - extensionsNonUuidManaged: {}, - } - - // When - await bundleAndBuildExtensions({app, identifiers, bundlePath}, {}) - - // Then - expect(extensionBundleMock).toHaveBeenCalledTimes(2) - expect(file.writeFileSync).not.toHaveBeenCalled() - - await expect(file.fileExists(bundlePath)).resolves.toBeTruthy() - }) - }) - test('creates a zip file for a function extension', async () => { await file.inTemporaryDirectory(async (tmpDir: string) => { // Given @@ -132,7 +96,7 @@ describe('bundleAndBuildExtensions', () => { } // When - await bundleAndBuildExtensions({app, identifiers, bundlePath}, {}) + await bundleAndBuildExtensions({app, identifiers, bundlePath}) // Then await expect(file.fileExists(bundlePath)).resolves.toBeTruthy() diff --git a/packages/app/src/cli/services/deploy/bundle.ts b/packages/app/src/cli/services/deploy/bundle.ts index e550bf91372..f11186640b8 100644 --- a/packages/app/src/cli/services/deploy/bundle.ts +++ b/packages/app/src/cli/services/deploy/bundle.ts @@ -6,7 +6,6 @@ import {AbortSignal} from '@shopify/cli-kit/node/abort' import {inTemporaryDirectory, mkdirSync, touchFile, writeFileSync} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' import {renderConcurrent} from '@shopify/cli-kit/node/ui' -import {isAppManagementEnabled} from '@shopify/cli-kit/node/context/local' import {Writable} from 'stream' interface BundleOptions { @@ -15,18 +14,16 @@ interface BundleOptions { identifiers?: Identifiers } -export async function bundleAndBuildExtensions(options: BundleOptions, systemEnvironment = process.env) { +export async function bundleAndBuildExtensions(options: BundleOptions) { await inTemporaryDirectory(async (tmpDir) => { const bundleDirectory = joinPath(tmpDir, 'bundle') mkdirSync(bundleDirectory) await touchFile(joinPath(bundleDirectory, '.shopify')) - if (isAppManagementEnabled(systemEnvironment)) { - // Include manifest in bundle - const appManifest = await options.app.manifest() - const manifestPath = joinPath(bundleDirectory, 'manifest.json') - writeFileSync(manifestPath, JSON.stringify(appManifest, null, 2)) - } + // Include manifest in bundle + const appManifest = await options.app.manifest() + const manifestPath = joinPath(bundleDirectory, 'manifest.json') + writeFileSync(manifestPath, JSON.stringify(appManifest, null, 2)) // Force the download of the javy binary in advance to avoid later problems, // as it might be done multiple times in parallel. https://github.com/Shopify/cli/issues/2877 diff --git a/packages/app/src/cli/services/dev/fetch.test.ts b/packages/app/src/cli/services/dev/fetch.test.ts index 12665ef2650..04d70b2611b 100644 --- a/packages/app/src/cli/services/dev/fetch.test.ts +++ b/packages/app/src/cli/services/dev/fetch.test.ts @@ -54,7 +54,7 @@ afterEach(() => { }) describe('fetchOrganizations', async () => { - test('returns fetched organizations from Partners when App Management is disabled', async () => { + test('returns fetched organizations from Partners and App Management', async () => { // Given const partnersClient: PartnersClient = testDeveloperPlatformClient({ organizations: () => Promise.resolve([ORG1]), @@ -68,27 +68,6 @@ describe('fetchOrganizations', async () => { // When const got = await fetchOrganizations() - // Then - expect(got).toEqual([ORG1]) - expect(partnersClient.organizations).toHaveBeenCalled() - expect(appManagementClient.organizations).not.toHaveBeenCalled() - }) - - test('returns fetched organizations from Partners and App Management when App Management is enabled', async () => { - // Given - vi.stubEnv('USE_APP_MANAGEMENT_API', '1') - const partnersClient: PartnersClient = testDeveloperPlatformClient({ - organizations: () => Promise.resolve([ORG1]), - }) as PartnersClient - const appManagementClient: AppManagementClient = testDeveloperPlatformClient({ - organizations: () => Promise.resolve([ORG2]), - }) as AppManagementClient - vi.mocked(PartnersClient).mockReturnValue(partnersClient) - vi.mocked(AppManagementClient).mockReturnValue(appManagementClient) - - // When - const got = await fetchOrganizations() - // Then expect(got).toEqual([ORG1, ORG2]) expect(partnersClient.organizations).toHaveBeenCalled() @@ -100,7 +79,11 @@ describe('fetchOrganizations', async () => { const partnersClient: PartnersClient = testDeveloperPlatformClient({ organizations: () => Promise.resolve([]), }) as PartnersClient + const appManagementClient: AppManagementClient = testDeveloperPlatformClient({ + organizations: () => Promise.resolve([]), + }) as AppManagementClient vi.mocked(PartnersClient).mockReturnValue(partnersClient) + vi.mocked(AppManagementClient).mockReturnValue(appManagementClient) // When const got = fetchOrganizations() @@ -108,6 +91,7 @@ describe('fetchOrganizations', async () => { // Then await expect(got).rejects.toThrow(new NoOrgError(testPartnersUserSession.accountInfo)) expect(partnersClient.organizations).toHaveBeenCalled() + expect(appManagementClient.organizations).toHaveBeenCalled() }) }) diff --git a/packages/app/src/cli/utilities/developer-platform-client.ts b/packages/app/src/cli/utilities/developer-platform-client.ts index d2ccbf07549..24e7aacad11 100644 --- a/packages/app/src/cli/utilities/developer-platform-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client.ts @@ -55,7 +55,6 @@ import { import {DevSessionCreateMutation} from '../api/graphql/app-dev/generated/dev-session-create.js' import {DevSessionUpdateMutation} from '../api/graphql/app-dev/generated/dev-session-update.js' import {DevSessionDeleteMutation} from '../api/graphql/app-dev/generated/dev-session-delete.js' -import {isAppManagementEnabled} from '@shopify/cli-kit/node/context/local' export enum ClientName { AppManagement = 'app-management', @@ -77,8 +76,7 @@ export interface AppVersionIdentifiers { } export function allDeveloperPlatformClients(): DeveloperPlatformClient[] { - const clients: DeveloperPlatformClient[] = [new PartnersClient()] - if (isAppManagementEnabled()) clients.push(new AppManagementClient()) + const clients: DeveloperPlatformClient[] = [new PartnersClient(), new AppManagementClient()] return clients } @@ -118,11 +116,8 @@ export function selectDeveloperPlatformClient({ configuration, organization, }: SelectDeveloperPlatformClientOptions = {}): DeveloperPlatformClient { - if (isAppManagementEnabled()) { - if (organization) return selectDeveloperPlatformClientByOrg(organization) - return selectDeveloperPlatformClientByConfig(configuration) - } - return new PartnersClient() + if (organization) return selectDeveloperPlatformClientByOrg(organization) + return selectDeveloperPlatformClientByConfig(configuration) } function selectDeveloperPlatformClientByOrg(organization: Organization): DeveloperPlatformClient { diff --git a/packages/cli-kit/src/private/node/api/headers.ts b/packages/cli-kit/src/private/node/api/headers.ts index 7d5ac55ee9a..b00905a43e5 100644 --- a/packages/cli-kit/src/private/node/api/headers.ts +++ b/packages/cli-kit/src/private/node/api/headers.ts @@ -4,7 +4,7 @@ import {Environment, serviceEnvironment} from '../context/service.js' import {ExtendableError} from '../../../public/node/error.js' import https from 'https' -export class RequestClientError extends ExtendableError { +class RequestClientError extends ExtendableError { statusCode: number public constructor(message: string, statusCode: number) { super(message) diff --git a/packages/cli-kit/src/private/node/constants.ts b/packages/cli-kit/src/private/node/constants.ts index c9a4ab91517..faee18c688e 100644 --- a/packages/cli-kit/src/private/node/constants.ts +++ b/packages/cli-kit/src/private/node/constants.ts @@ -45,7 +45,6 @@ export const environmentVariables = { otelURL: 'SHOPIFY_CLI_OTEL_EXPORTER_OTLP_ENDPOINT', themeKitAccessDomain: 'SHOPIFY_CLI_THEME_KIT_ACCESS_DOMAIN', json: 'SHOPIFY_FLAG_JSON', - useAppManagement: 'USE_APP_MANAGEMENT_API', } export const defaultThemeKitAccessDomain = 'theme-kit-access.shopifyapps.com' diff --git a/packages/cli-kit/src/private/node/session.ts b/packages/cli-kit/src/private/node/session.ts index 041df38d971..a6e620ad8a8 100644 --- a/packages/cli-kit/src/private/node/session.ts +++ b/packages/cli-kit/src/private/node/session.ts @@ -12,20 +12,14 @@ import { import {IdentityToken, Session} from './session/schema.js' import * as secureStore from './session/store.js' import {pollForDeviceAuthorization, requestDeviceAuthorization} from './session/device-authorization.js' -import {RequestClientError} from './api/headers.js' -import {getCachedPartnerAccountStatus, setCachedPartnerAccountStatus} from './conf-store.js' import {isThemeAccessSession} from './api/rest.js' import {outputContent, outputToken, outputDebug} from '../../public/node/output.js' -import {firstPartyDev, isAppManagementEnabled, themeToken} from '../../public/node/context/local.js' +import {firstPartyDev, themeToken} from '../../public/node/context/local.js' import {AbortError, BugError} from '../../public/node/error.js' -import {partnersRequest} from '../../public/node/api/partners.js' -import {normalizeStoreFqdn, partnersFqdn, identityFqdn} from '../../public/node/context/fqdn.js' -import {openURL} from '../../public/node/system.js' -import {keypress} from '../../public/node/ui.js' +import {normalizeStoreFqdn, identityFqdn} from '../../public/node/context/fqdn.js' import {getIdentityTokenInformation, getPartnersToken} from '../../public/node/environment.js' -import {gql} from 'graphql-request' import {AdminSession} from '@shopify/cli-kit/node/session' -import {outputCompleted, outputInfo, outputWarn} from '@shopify/cli-kit/node/output' +import {outputCompleted} from '@shopify/cli-kit/node/output' import {isSpin} from '@shopify/cli-kit/node/context/spin' import {nonRandomUUID} from '@shopify/cli-kit/node/crypto' @@ -247,9 +241,6 @@ The CLI is currently unable to prompt for reauthentication.`, if (envToken && applications.partnersApi) { tokens.partners = (await exchangeCustomPartnerToken(envToken)).accessToken } - if (!envToken && tokens.partners) { - await ensureUserHasPartnerAccount(tokens.partners, tokens.userId) - } setLastSeenAuthMethod(envToken ? 'partners_token' : 'device_auth') setLastSeenUserIdAfterAuth(tokens.userId) @@ -301,74 +292,6 @@ async function executeCompleteFlow(applications: OAuthApplications, identityFqdn return session } -/** - * If the user creates an account from the Identity website, the created - * account won't get a Partner organization created. We need to detect that - * and take the user to create a partner organization. - * - * @param partnersToken - Partners token. - */ -async function ensureUserHasPartnerAccount(partnersToken: string, userId: string | undefined) { - if (isAppManagementEnabled()) return - - outputDebug(outputContent`Verifying that the user has a Partner organization`) - if (!(await hasPartnerAccount(partnersToken, userId))) { - outputInfo(`\nA Shopify Partners organization is needed to proceed.`) - outputInfo(`👉 Press any key to create one`) - await keypress() - await openURL(`https://${await partnersFqdn()}/signup`) - outputInfo(outputContent`👉 Press any key when you have ${outputToken.cyan('created the organization')}`) - outputWarn(outputContent`Make sure you've confirmed your Shopify and the Partner organization from the email`) - await keypress() - if (!(await hasPartnerAccount(partnersToken, userId))) { - throw new AbortError( - `Couldn't find your Shopify Partners organization`, - `Have you confirmed your accounts from the emails you received?`, - ) - } - } -} - -// eslint-disable-next-line @shopify/cli/no-inline-graphql -const getFirstOrganization = gql` - { - organizations(first: 1) { - nodes { - id - } - } - } -` - -/** - * Validate if the current token is valid for partners API. - * - * @param partnersToken - Partners token. - * @returns A promise that resolves to true if the token is valid for partners API. - */ -async function hasPartnerAccount(partnersToken: string, userId?: string): Promise { - const cacheKey = userId ?? partnersToken - const cachedStatus = getCachedPartnerAccountStatus(cacheKey) - - if (cachedStatus) { - outputDebug(`Confirmed partner account exists from cache`) - return true - } - - try { - await partnersRequest(getFirstOrganization, partnersToken) - setCachedPartnerAccountStatus(cacheKey) - return true - // eslint-disable-next-line no-catch-all/no-catch-all - } catch (error) { - if (error instanceof RequestClientError && error.statusCode === 404) { - return false - } else { - return true - } - } -} - /** * Refresh the tokens for a given session. * diff --git a/packages/cli-kit/src/private/node/session/exchange.ts b/packages/cli-kit/src/private/node/session/exchange.ts index 2e3b8a540a4..e234ee4f3a6 100644 --- a/packages/cli-kit/src/private/node/session/exchange.ts +++ b/packages/cli-kit/src/private/node/session/exchange.ts @@ -5,7 +5,6 @@ import {identityFqdn} from '../../../public/node/context/fqdn.js' import {shopifyFetch} from '../../../public/node/http.js' import {err, ok, Result} from '../../../public/node/result.js' import {AbortError, BugError, ExtendableError} from '../../../public/node/error.js' -import {isAppManagementEnabled} from '../../../public/node/context/local.js' import {setLastSeenAuthMethod, setLastSeenUserIdAfterAuth} from '../session.js' import * as jose from 'jose' import {nonRandomUUID} from '@shopify/cli-kit/node/crypto' @@ -34,14 +33,13 @@ export async function exchangeAccessForApplicationTokens( store?: string, ): Promise<{[x: string]: ApplicationToken}> { const token = identityToken.accessToken - const appManagementEnabled = isAppManagementEnabled() const [partners, storefront, businessPlatform, admin, appManagement] = await Promise.all([ requestAppToken('partners', token, scopes.partners), requestAppToken('storefront-renderer', token, scopes.storefront), requestAppToken('business-platform', token, scopes.businessPlatform), store ? requestAppToken('admin', token, scopes.admin, store) : {}, - appManagementEnabled ? requestAppToken('app-management', token, scopes.appManagement) : {}, + requestAppToken('app-management', token, scopes.appManagement), ]) return { diff --git a/packages/cli-kit/src/private/node/session/scopes.test.ts b/packages/cli-kit/src/private/node/session/scopes.test.ts index d7f88fd9b53..4b28edfc995 100644 --- a/packages/cli-kit/src/private/node/session/scopes.test.ts +++ b/packages/cli-kit/src/private/node/session/scopes.test.ts @@ -1,5 +1,4 @@ import {allDefaultScopes, apiScopes} from './scopes.js' -import {environmentVariables} from '../constants.js' import {describe, expect, test} from 'vitest' describe('allDefaultScopes', () => { @@ -24,12 +23,9 @@ describe('allDefaultScopes', () => { ]) }) - test('includes App Management and Store Management when the required env var is defined', async () => { - // Given - const envVars = {[environmentVariables.useAppManagement]: 'true'} - + test('includes App Management and Store Management', async () => { // When - const got = allDefaultScopes([], envVars) + const got = allDefaultScopes([]) // Then expect(got).toEqual([ diff --git a/packages/cli-kit/src/private/node/session/scopes.ts b/packages/cli-kit/src/private/node/session/scopes.ts index 072fb29d2fc..91803f77709 100644 --- a/packages/cli-kit/src/private/node/session/scopes.ts +++ b/packages/cli-kit/src/private/node/session/scopes.ts @@ -1,6 +1,5 @@ import {allAPIs, API} from '../api.js' import {BugError} from '../../../public/node/error.js' -import {isAppManagementEnabled} from '../../../public/node/context/local.js' /** * Generate a flat array with all the default scopes for all the APIs plus @@ -8,8 +7,8 @@ import {isAppManagementEnabled} from '../../../public/node/context/local.js' * @param extraScopes - custom user-defined scopes * @returns Array of scopes */ -export function allDefaultScopes(extraScopes: string[] = [], systemEnvironment = process.env): string[] { - let scopes = allAPIs.map((api) => defaultApiScopes(api, systemEnvironment)).flat() +export function allDefaultScopes(extraScopes: string[] = []): string[] { + let scopes = allAPIs.map((api) => defaultApiScopes(api)).flat() scopes = ['openid', ...scopes, ...extraScopes].map(scopeTransform) return Array.from(new Set(scopes)) } @@ -21,12 +20,12 @@ export function allDefaultScopes(extraScopes: string[] = [], systemEnvironment = * @param extraScopes - custom user-defined scopes * @returns Array of scopes */ -export function apiScopes(api: API, extraScopes: string[] = [], systemEnvironment = process.env): string[] { - const scopes = [...defaultApiScopes(api, systemEnvironment), ...extraScopes.map(scopeTransform)].map(scopeTransform) +export function apiScopes(api: API, extraScopes: string[] = []): string[] { + const scopes = [...defaultApiScopes(api), ...extraScopes.map(scopeTransform)].map(scopeTransform) return Array.from(new Set(scopes)) } -function defaultApiScopes(api: API, systemEnvironment = process.env): string[] { +function defaultApiScopes(api: API): string[] { switch (api) { case 'admin': return ['graphql', 'themes', 'collaborator'] @@ -35,9 +34,9 @@ function defaultApiScopes(api: API, systemEnvironment = process.env): string[] { case 'partners': return ['cli'] case 'business-platform': - return isAppManagementEnabled(systemEnvironment) ? ['destinations', 'store-management'] : ['destinations'] + return ['destinations', 'store-management'] case 'app-management': - return isAppManagementEnabled(systemEnvironment) ? ['app-management'] : [] + return ['app-management'] default: throw new BugError(`Unknown API: ${api}`) } diff --git a/packages/cli-kit/src/public/node/context/local.test.ts b/packages/cli-kit/src/public/node/context/local.test.ts index 14c69b10fbe..2c687428e11 100644 --- a/packages/cli-kit/src/public/node/context/local.test.ts +++ b/packages/cli-kit/src/public/node/context/local.test.ts @@ -4,7 +4,6 @@ import { isDevelopment, isShopify, isUnitTest, - isAppManagementEnabled, analyticsDisabled, cloudEnvironment, macAddress, @@ -100,30 +99,6 @@ describe('hasGit', () => { }) }) -describe('isAppManagementEnabled', () => { - test('returns true when USE_APP_MANAGEMENT_API is truthy', () => { - // Given - const env = {USE_APP_MANAGEMENT_API: '1'} - - // When - const got = isAppManagementEnabled(env) - - // Then - expect(got).toBe(true) - }) - - test('returns false when USE_APP_MANAGEMENT_API is falsy', () => { - // Given - const env = {USE_APP_MANAGEMENT_API: '0'} - - // When - const got = isAppManagementEnabled(env) - - // Then - expect(got).toBe(false) - }) -}) - describe('analitycsDisabled', () => { test('returns true when SHOPIFY_CLI_NO_ANALYTICS is truthy', () => { // Given diff --git a/packages/cli-kit/src/public/node/context/local.ts b/packages/cli-kit/src/public/node/context/local.ts index d4c1de54eee..cc8a73b2db2 100644 --- a/packages/cli-kit/src/public/node/context/local.ts +++ b/packages/cli-kit/src/public/node/context/local.ts @@ -46,16 +46,6 @@ export function isVerbose(env = process.env): boolean { return isTruthy(env[environmentVariables.verbose]) || process.argv.includes('--verbose') } -/** - * It returns true if the App Management API is available. - * - * @param env - The environment variables from the environment of the current process. - * @returns True if the App Management API is available. - */ -export function isAppManagementEnabled(env = process.env): boolean { - return isTruthy(env[environmentVariables.useAppManagement]) -} - /** * Returns true if the environment in which the CLI is running is either * a local environment (where dev is present) or a cloud environment (spin). From dfb94ce77dbd46d023854e2f67e4cc7911701403 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Tue, 14 Jan 2025 17:24:25 +0200 Subject: [PATCH 48/50] Update failing tests --- .../src/private/node/session/exchange.test.ts | 25 +++++++++++++------ .../src/private/node/session/scopes.test.ts | 2 ++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/packages/cli-kit/src/private/node/session/exchange.test.ts b/packages/cli-kit/src/private/node/session/exchange.test.ts index c4869815c5e..b8e9feab9ef 100644 --- a/packages/cli-kit/src/private/node/session/exchange.test.ts +++ b/packages/cli-kit/src/private/node/session/exchange.test.ts @@ -55,20 +55,21 @@ describe('exchange identity token for application tokens', () => { test('returns tokens for all APIs if a store is passed', async () => { // Given - const response = new Response(JSON.stringify(data)) - - // Need to do it 3 times because a Response can only be used once - vi.mocked(shopifyFetch) - .mockResolvedValue(response) - .mockResolvedValueOnce(response.clone()) - .mockResolvedValueOnce(response.clone()) - .mockResolvedValueOnce(response.clone()) + vi.mocked(shopifyFetch).mockImplementation(async () => Promise.resolve(new Response(JSON.stringify(data)))) // When const got = await exchangeAccessForApplicationTokens(identityToken, scopes, 'storeFQDN') // Then const expected = { + 'app-management': { + accessToken: "access_token", + expiresAt: expiredDate, + scopes: [ + "scope", + "scope2", + ], + }, partners: { accessToken: 'access_token', expiresAt: expiredDate, @@ -109,6 +110,14 @@ describe('exchange identity token for application tokens', () => { // Then const expected = { + 'app-management': { + accessToken: "access_token", + expiresAt: expiredDate, + scopes: [ + "scope", + "scope2", + ], + }, partners: { accessToken: 'access_token', expiresAt: expiredDate, diff --git a/packages/cli-kit/src/private/node/session/scopes.test.ts b/packages/cli-kit/src/private/node/session/scopes.test.ts index 4b28edfc995..74626d8a410 100644 --- a/packages/cli-kit/src/private/node/session/scopes.test.ts +++ b/packages/cli-kit/src/private/node/session/scopes.test.ts @@ -19,6 +19,8 @@ describe('allDefaultScopes', () => { 'https://api.shopify.com/auth/shop.storefront-renderer.devtools', 'https://api.shopify.com/auth/partners.app.cli.access', 'https://api.shopify.com/auth/destinations.readonly', + 'https://api.shopify.com/auth/organization.store-management', + 'https://api.shopify.com/auth/organization.apps.manage', ...customScopes, ]) }) From 893ee2e1ce8387e55ac7bd85cc54261dba6928a3 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Tue, 14 Jan 2025 17:48:05 +0200 Subject: [PATCH 49/50] Disable app management when using a Partners CLI token Otherwise we hit permissions issues. --- .../utilities/developer-platform-client.ts | 5 ++-- .../src/private/node/session/exchange.test.ts | 14 +++------- .../src/private/node/session/exchange.ts | 3 ++- .../src/public/node/context/local.test.ts | 27 +++++++++++++++++++ .../cli-kit/src/public/node/context/local.ts | 11 ++++++++ 5 files changed, 47 insertions(+), 13 deletions(-) diff --git a/packages/app/src/cli/utilities/developer-platform-client.ts b/packages/app/src/cli/utilities/developer-platform-client.ts index 24e7aacad11..8fff7486de4 100644 --- a/packages/app/src/cli/utilities/developer-platform-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client.ts @@ -55,6 +55,7 @@ import { import {DevSessionCreateMutation} from '../api/graphql/app-dev/generated/dev-session-create.js' import {DevSessionUpdateMutation} from '../api/graphql/app-dev/generated/dev-session-update.js' import {DevSessionDeleteMutation} from '../api/graphql/app-dev/generated/dev-session-delete.js' +import {isAppManagementDisabled} from '@shopify/cli-kit/node/context/local' export enum ClientName { AppManagement = 'app-management', @@ -76,8 +77,7 @@ export interface AppVersionIdentifiers { } export function allDeveloperPlatformClients(): DeveloperPlatformClient[] { - const clients: DeveloperPlatformClient[] = [new PartnersClient(), new AppManagementClient()] - return clients + return isAppManagementDisabled() ? [new PartnersClient()] : [new PartnersClient(), new AppManagementClient()] } /** @@ -116,6 +116,7 @@ export function selectDeveloperPlatformClient({ configuration, organization, }: SelectDeveloperPlatformClientOptions = {}): DeveloperPlatformClient { + if (isAppManagementDisabled()) return new PartnersClient() if (organization) return selectDeveloperPlatformClientByOrg(organization) return selectDeveloperPlatformClientByConfig(configuration) } diff --git a/packages/cli-kit/src/private/node/session/exchange.test.ts b/packages/cli-kit/src/private/node/session/exchange.test.ts index b8e9feab9ef..1d2e2fe6b1d 100644 --- a/packages/cli-kit/src/private/node/session/exchange.test.ts +++ b/packages/cli-kit/src/private/node/session/exchange.test.ts @@ -63,12 +63,9 @@ describe('exchange identity token for application tokens', () => { // Then const expected = { 'app-management': { - accessToken: "access_token", + accessToken: 'access_token', expiresAt: expiredDate, - scopes: [ - "scope", - "scope2", - ], + scopes: ['scope', 'scope2'], }, partners: { accessToken: 'access_token', @@ -111,12 +108,9 @@ describe('exchange identity token for application tokens', () => { // Then const expected = { 'app-management': { - accessToken: "access_token", + accessToken: 'access_token', expiresAt: expiredDate, - scopes: [ - "scope", - "scope2", - ], + scopes: ['scope', 'scope2'], }, partners: { accessToken: 'access_token', diff --git a/packages/cli-kit/src/private/node/session/exchange.ts b/packages/cli-kit/src/private/node/session/exchange.ts index e234ee4f3a6..87dccc92702 100644 --- a/packages/cli-kit/src/private/node/session/exchange.ts +++ b/packages/cli-kit/src/private/node/session/exchange.ts @@ -5,6 +5,7 @@ import {identityFqdn} from '../../../public/node/context/fqdn.js' import {shopifyFetch} from '../../../public/node/http.js' import {err, ok, Result} from '../../../public/node/result.js' import {AbortError, BugError, ExtendableError} from '../../../public/node/error.js' +import {isAppManagementDisabled} from '../../../public/node/context/local.js' import {setLastSeenAuthMethod, setLastSeenUserIdAfterAuth} from '../session.js' import * as jose from 'jose' import {nonRandomUUID} from '@shopify/cli-kit/node/crypto' @@ -39,7 +40,7 @@ export async function exchangeAccessForApplicationTokens( requestAppToken('storefront-renderer', token, scopes.storefront), requestAppToken('business-platform', token, scopes.businessPlatform), store ? requestAppToken('admin', token, scopes.admin, store) : {}, - requestAppToken('app-management', token, scopes.appManagement), + isAppManagementDisabled() ? {} : requestAppToken('app-management', token, scopes.appManagement), ]) return { diff --git a/packages/cli-kit/src/public/node/context/local.test.ts b/packages/cli-kit/src/public/node/context/local.test.ts index 2c687428e11..6ca48208a22 100644 --- a/packages/cli-kit/src/public/node/context/local.test.ts +++ b/packages/cli-kit/src/public/node/context/local.test.ts @@ -7,13 +7,16 @@ import { analyticsDisabled, cloudEnvironment, macAddress, + isAppManagementDisabled, } from './local.js' +import {getPartnersToken} from '../environment.js' import {fileExists} from '../fs.js' import {exec} from '../system.js' import {expect, describe, vi, test} from 'vitest' vi.mock('../fs.js') vi.mock('../system.js') +vi.mock('../environment.js') describe('isUnitTest', () => { test('returns true when SHOPIFY_UNIT_TEST is truthy', () => { @@ -99,6 +102,30 @@ describe('hasGit', () => { }) }) +describe('isAppManagementDisabled', () => { + test('returns true when a Partners token is present', () => { + // Given + vi.mocked(getPartnersToken).mockReturnValue('token') + + // When + const got = isAppManagementDisabled() + + // Then + expect(got).toBe(true) + }) + + test('returns false when a Partners token is not present', () => { + // Given + vi.mocked(getPartnersToken).mockReturnValue(undefined) + + // When + const got = isAppManagementDisabled() + + // Then + expect(got).toBe(false) + }) +}) + describe('analitycsDisabled', () => { test('returns true when SHOPIFY_CLI_NO_ANALYTICS is truthy', () => { // Given diff --git a/packages/cli-kit/src/public/node/context/local.ts b/packages/cli-kit/src/public/node/context/local.ts index cc8a73b2db2..e24d874e277 100644 --- a/packages/cli-kit/src/public/node/context/local.ts +++ b/packages/cli-kit/src/public/node/context/local.ts @@ -4,6 +4,7 @@ import {getCIMetadata, isSet, Metadata} from '../../../private/node/context/util import {environmentVariables, pathConstants} from '../../../private/node/constants.js' import {fileExists} from '../fs.js' import {exec} from '../system.js' +import {getPartnersToken} from '../environment.js' import isInteractive from 'is-interactive' import macaddress from 'macaddress' import {homedir} from 'os' @@ -46,6 +47,16 @@ export function isVerbose(env = process.env): boolean { return isTruthy(env[environmentVariables.verbose]) || process.argv.includes('--verbose') } +/** + * It returns true if the App Management API is disabled. + * This should only be relevant when using a Partners token. + * + * @returns True if the App Management API is disabled. + */ +export function isAppManagementDisabled(): boolean { + return Boolean(getPartnersToken()) +} + /** * Returns true if the environment in which the CLI is running is either * a local environment (where dev is present) or a cloud environment (spin). From de4d15e59c3958a12b6569d76da767c961e29e82 Mon Sep 17 00:00:00 2001 From: Ariel Caplan Date: Tue, 14 Jan 2025 18:08:25 +0200 Subject: [PATCH 50/50] Only use CLI token for acceptance tests --- .github/workflows/shopify-cli.yml | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/.github/workflows/shopify-cli.yml b/.github/workflows/shopify-cli.yml index 0d6cafe99b8..e5396300f0d 100644 --- a/.github/workflows/shopify-cli.yml +++ b/.github/workflows/shopify-cli.yml @@ -49,7 +49,6 @@ env: PNPM_VERSION: '8.15.7' BUNDLE_WITHOUT: 'test:development' SHOPIFY_FLAG_CLIENT_ID: ${{ secrets.SHOPIFY_FLAG_CLIENT_ID }} - SHOPIFY_CLI_PARTNERS_TOKEN: ${{ secrets.SHOPIFY_CLI_PARTNERS_TOKEN }} GH_TOKEN: ${{ secrets.SHOPIFY_GH_READ_CONTENT_TOKEN }} jobs: @@ -87,6 +86,8 @@ jobs: run: pnpm nx run-many --all --skip-nx-cache --target=test --exclude=features --output-style=stream - name: Acceptance tests if: ${{ matrix.node == '18.20.3' }} + env: + SHOPIFY_CLI_PARTNERS_TOKEN: ${{ secrets.SHOPIFY_CLI_PARTNERS_TOKEN }} run: pnpm nx run features:test - name: Send Slack notification on failure uses: slackapi/slack-github-action@007b2c3c751a190b6f0f040e47ed024deaa72844 # pin@v1.23.0 @@ -305,6 +306,8 @@ jobs: with: node-version: ${{ matrix.node }} - name: Acceptance tests + env: + SHOPIFY_CLI_PARTNERS_TOKEN: ${{ secrets.SHOPIFY_CLI_PARTNERS_TOKEN }} run: pnpm test:features --output-style=stream pr-test-coverage: @@ -355,6 +358,8 @@ jobs: - name: Unit tests run: pnpm test:unit --output-style=stream - name: Acceptance tests + env: + SHOPIFY_CLI_PARTNERS_TOKEN: ${{ secrets.SHOPIFY_CLI_PARTNERS_TOKEN }} run: pnpm test:features --output-style=stream - name: Setup tmate session if: ${{ failure() && inputs.debug-enabled }}