diff --git a/packages/app/src/cli/commands/app/deploy.ts b/packages/app/src/cli/commands/app/deploy.ts index 1ea49402070..19dfc282008 100644 --- a/packages/app/src/cli/commands/app/deploy.ts +++ b/packages/app/src/cli/commands/app/deploy.ts @@ -1,6 +1,5 @@ import {appFlags} from '../../flags.js' import {deploy} from '../../services/deploy.js' -import {getAppConfigurationState} from '../../models/app/loader.js' import {validateVersion} from '../../validations/version-name.js' import {validateMessage} from '../../validations/message.js' import metadata from '../../metadata.js' @@ -81,10 +80,6 @@ export default class Deploy extends AppLinkedCommand { })) const requiredNonTTYFlags = ['force'] - const configurationState = await getAppConfigurationState(flags.path, flags.config) - if (configurationState.state === 'template-only' && !clientId) { - requiredNonTTYFlags.push('client-id') - } this.failMissingNonTTYFlags(flags, requiredNonTTYFlags) const {app, remoteApp, developerPlatformClient, organization} = await linkedAppContext({ diff --git a/packages/app/src/cli/commands/app/release.ts b/packages/app/src/cli/commands/app/release.ts index 8fb6d419980..0c5785da5a2 100644 --- a/packages/app/src/cli/commands/app/release.ts +++ b/packages/app/src/cli/commands/app/release.ts @@ -2,7 +2,6 @@ import {appFlags} from '../../flags.js' import {release} from '../../services/release.js' import AppLinkedCommand, {AppLinkedCommandOutput} from '../../utilities/app-linked-command.js' import {linkedAppContext} from '../../services/app-context.js' -import {getAppConfigurationState} from '../../models/app/loader.js' import {Flags} from '@oclif/core' import {globalFlags} from '@shopify/cli-kit/node/cli' import {addPublicMetadata} from '@shopify/cli-kit/node/metadata' @@ -42,10 +41,6 @@ export default class Release extends AppLinkedCommand { })) const requiredNonTTYFlags = ['force'] - const configurationState = await getAppConfigurationState(flags.path, flags.config) - if (configurationState.state === 'template-only' && !clientId) { - requiredNonTTYFlags.push('client-id') - } this.failMissingNonTTYFlags(flags, requiredNonTTYFlags) const {app, remoteApp, developerPlatformClient} = await linkedAppContext({ 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 08883096545..8751bc4c755 100644 --- a/packages/app/src/cli/models/app/app.test-data.ts +++ b/packages/app/src/cli/models/app/app.test-data.ts @@ -1,12 +1,10 @@ import { App, - AppConfiguration, - AppConfigurationSchema, + AppSchema, AppConfigurationWithoutPath, AppInterface, AppLinkedInterface, CurrentAppConfiguration, - LegacyAppConfiguration, WebType, getAppVersionedSchema, } from './app.js' @@ -93,16 +91,13 @@ export const DEFAULT_CONFIG = { embedded: true, access_scopes: { scopes: 'read_products', + use_legacy_install_flow: true, }, } export function testApp(app: Partial = {}, schemaType: 'current' | 'legacy' = 'legacy'): AppInterface { const getConfig = () => { - if (schemaType === 'legacy') { - return {scopes: '', extension_directories: [], path: ''} - } else { - return DEFAULT_CONFIG as CurrentAppConfiguration - } + return DEFAULT_CONFIG as CurrentAppConfiguration } const newApp = new App({ @@ -125,7 +120,7 @@ export function testApp(app: Partial = {}, schemaType: 'current' | dotenv: app.dotenv, errors: app.errors, specifications: app.specifications ?? [], - configSchema: (app.configSchema ?? AppConfigurationSchema) as any, + configSchema: (app.configSchema ?? AppSchema) as any, remoteFlags: app.remoteFlags ?? [], hiddenConfig: app.hiddenConfig ?? {}, devApplicationURLs: app.devApplicationURLs, @@ -149,20 +144,6 @@ interface TestAppWithConfigOptions { config: object } -export function testAppWithLegacyConfig({ - app = {}, - config = {}, -}: TestAppWithConfigOptions): AppInterface { - const configuration: AppConfiguration = { - path: '', - scopes: '', - name: 'name', - extension_directories: [], - ...config, - } - return testApp({...app, configuration}) as AppInterface -} - export function testAppWithConfig(options?: TestAppWithConfigOptions): AppLinkedInterface { const app = testAppLinked(options?.app) app.configuration = { @@ -206,7 +187,9 @@ export function testOrganizationApp(app: Partial = {}): Organiz return {...defaultApp, ...app} } -export const placeholderAppConfiguration: AppConfigurationWithoutPath = {scopes: ''} +export const placeholderAppConfiguration: AppConfigurationWithoutPath = { + client_id: '', +} export async function testUIExtension( uiExtension: Omit, 'configuration'> & { diff --git a/packages/app/src/cli/models/app/app.test.ts b/packages/app/src/cli/models/app/app.test.ts index d1844e6f050..aac5bf37ff6 100644 --- a/packages/app/src/cli/models/app/app.test.ts +++ b/packages/app/src/cli/models/app/app.test.ts @@ -1,12 +1,9 @@ import { AppSchema, CurrentAppConfiguration, - LegacyAppConfiguration, getAppScopes, getAppScopesArray, getUIExtensionRendererVersion, - isCurrentAppSchema, - isLegacyAppSchema, validateExtensionsHandlesInCollection, validateFunctionExtensionsWithUiHandle, } from './app.js' @@ -61,43 +58,8 @@ const CORRECT_CURRENT_APP_SCHEMA: CurrentAppConfiguration = { }, } -const CORRECT_LEGACY_APP_SCHEMA: LegacyAppConfiguration = { - path: '', - extension_directories: [], - web_directories: [], - scopes: 'write_products', -} - describe('app schema validation', () => { - describe('legacy schema validator', () => { - test('checks whether legacy app schema is valid -- pass', () => { - expect(isLegacyAppSchema(CORRECT_LEGACY_APP_SCHEMA)).toBe(true) - }) - test('checks whether legacy app schema is valid -- fail', () => { - const config = { - ...CORRECT_LEGACY_APP_SCHEMA, - some_other_key: 'i am not valid, i will fail', - } - expect(isLegacyAppSchema(config)).toBe(false) - }) - }) - describe('current schema validator', () => { - test('checks whether current app schema is valid -- pass', () => { - expect(isCurrentAppSchema(CORRECT_CURRENT_APP_SCHEMA)).toBe(true) - }) - test('checks whether current app schema is valid -- fail', () => { - const config = { - ...CORRECT_CURRENT_APP_SCHEMA, - } - - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - delete config.client_id - - expect(isCurrentAppSchema(config)).toBe(false) - }) - test('extension_directories should be transformed to double asterisks', () => { const config = { ...CORRECT_CURRENT_APP_SCHEMA, @@ -210,24 +172,14 @@ describe('getUIExtensionRendererVersion', () => { }) describe('getAppScopes', () => { - test('returns the scopes key when schema is legacy', () => { - const config = {path: '', scopes: 'read_themes,read_products'} - expect(getAppScopes(config)).toEqual('read_themes,read_products') - }) - - test('returns the access_scopes.scopes key when schema is current', () => { + test('returns the access_scopes.scopes key', () => { const config = {...DEFAULT_CONFIG, access_scopes: {scopes: 'read_themes,read_themes'}} expect(getAppScopes(config)).toEqual('read_themes,read_themes') }) }) describe('getAppScopesArray', () => { - test('returns the scopes key when schema is legacy', () => { - const config = {path: '', scopes: 'read_themes, read_order ,write_products'} - expect(getAppScopesArray(config)).toEqual(['read_themes', 'read_order', 'write_products']) - }) - - test('returns the access_scopes.scopes key when schema is current', () => { + test('returns the access_scopes.scopes key', () => { const config = {...DEFAULT_CONFIG, access_scopes: {scopes: 'read_themes, read_order ,write_products'}} expect(getAppScopesArray(config)).toEqual(['read_themes', 'read_order', 'write_products']) }) @@ -328,18 +280,6 @@ Learn more: https://shopify.dev/docs/apps/build/authentication-authorization/app await expect(app.preDeployValidation()).resolves.not.toThrow() }) - test('does not throw an error for legacy schema apps', async () => { - // Given - const configuration: LegacyAppConfiguration = { - ...CORRECT_LEGACY_APP_SCHEMA, - scopes: 'read_orders', - } - const app = testApp(configuration, 'legacy') - - // When/Then - await expect(app.preDeployValidation()).resolves.not.toThrow() - }) - test('handles null/undefined subscriptions safely', async () => { // Given const configuration: CurrentAppConfiguration = { diff --git a/packages/app/src/cli/models/app/app.ts b/packages/app/src/cli/models/app/app.ts index d1a1e902e82..95130524223 100644 --- a/packages/app/src/cli/models/app/app.ts +++ b/packages/app/src/cli/models/app/app.ts @@ -3,7 +3,6 @@ import {AppErrors, isWebType} from './loader.js' import {ensurePathStartsWithSlash} from './validation/common.js' import {Identifiers} from './identifiers.js' import {ExtensionInstance} from '../extensions/extension-instance.js' -import {isType} from '../../utilities/types.js' import {FunctionConfigType} from '../extensions/specifications/function.js' import {ExtensionSpecification, RemoteAwareExtensionSpecification} from '../extensions/specification.js' import {AppConfigurationUsedByCli} from '../extensions/specifications/types/app_config.js' @@ -11,10 +10,10 @@ import {EditorExtensionCollectionType} from '../extensions/specifications/editor import {UIExtensionSchema} from '../extensions/specifications/ui_extension.js' import {CreateAppOptions, Flag} from '../../utilities/developer-platform-client.js' import {AppAccessSpecIdentifier} from '../extensions/specifications/app_config_app_access.js' -import {WebhookSubscriptionSchema} from '../extensions/specifications/app_config_webhook_schemas/webhook_subscription_schema.js' import {configurationFileNames} from '../../constants.js' import {ApplicationURLs} from '../../services/dev/urls.js' import {patchAppHiddenConfigFile} from '../../services/app/patch-app-configuration-file.js' +import {WebhookSubscription} from '../extensions/specifications/types/app_config_webhook.js' import {joinPath} from '@shopify/cli-kit/node/path' import {ZodObjectOf, zod} from '@shopify/cli-kit/node/schema' import {DotEnvFile} from '@shopify/cli-kit/node/dot-env' @@ -28,7 +27,6 @@ import { writeFileSync, } from '@shopify/cli-kit/node/fs' import {AbortError} from '@shopify/cli-kit/node/error' -import {normalizeDelimitedString} from '@shopify/cli-kit/common/string' import {JsonMapType} from '@shopify/cli-kit/node/toml' import {getArrayRejectingUndefined} from '@shopify/cli-kit/common/array' import {deepMergeObjects} from '@shopify/cli-kit/common/object' @@ -41,28 +39,6 @@ const ExtensionDirectoriesSchema = zod .transform(removeTrailingPathSeparator) .transform(fixSingleWildcards) -/** - * Schema for a freshly minted app template. - */ -export const LegacyAppSchema = zod - .object({ - client_id: zod.number().optional(), - name: zod.string().optional(), - scopes: zod - .string() - .transform((scopes) => normalizeDelimitedString(scopes) ?? '') - .default(''), - extension_directories: ExtensionDirectoriesSchema, - web_directories: zod.array(zod.string()).optional(), - webhooks: zod - .object({ - api_version: zod.string({required_error: 'String is required'}), - subscriptions: zod.array(WebhookSubscriptionSchema).optional(), - }) - .optional(), - }) - .strict() - function removeTrailingPathSeparator(value: string[] | undefined) { // eslint-disable-next-line no-useless-escape return value?.map((dir) => dir.replace(/[\/\\]+$/, '')) @@ -100,11 +76,6 @@ export interface AppHiddenConfig { dev_store_url?: string } -/** - * Utility schema that matches freshly minted or normal, linked, apps. - */ -export const AppConfigurationSchema = zod.union([LegacyAppSchema, AppSchema]) - // Types representing post-validated app configurations /** @@ -112,9 +83,8 @@ export const AppConfigurationSchema = zod.union([LegacyAppSchema, AppSchema]) * * Try to avoid using this: generally you should be working with a more specific type. */ -export type AppConfiguration = zod.infer & {path: string} - -export type AppConfigurationWithoutPath = zod.infer +export type AppConfiguration = zod.infer & {path: string} +export type AppConfigurationWithoutPath = zod.infer /** * App configuration for a normal, linked, app. Doesn't include properties that are module derived. @@ -131,11 +101,6 @@ export type CliBuildPreferences = BasicAppConfigurationWithoutModules['build'] */ export type CurrentAppConfiguration = BasicAppConfigurationWithoutModules & AppConfigurationUsedByCli -/** - * App configuration for a freshly minted app template. Very old apps *may* have a client_id provided. - */ -export type LegacyAppConfiguration = zod.infer & {path: string} - /** Validation schema that produces a provided app configuration type */ export type SchemaForConfig = ZodObjectOf> @@ -153,35 +118,12 @@ export function getAppVersionedSchema( } } -/** - * Check whether a shopify.app.toml schema is valid against the legacy schema definition. - * @param item - the item to validate - */ -export function isLegacyAppSchema(item: AppConfiguration): item is LegacyAppConfiguration { - const {path, ...rest} = item - return isType(LegacyAppSchema, rest) -} - -/** - * Check whether a shopify.app.toml schema is valid against the current schema definition. - * @param item - the item to validate - */ -export function isCurrentAppSchema(item: AppConfiguration): item is CurrentAppConfiguration { - const {path, ...rest} = item - return isType(AppSchema.nonstrict(), rest) -} - /** * Get scopes from a given app.toml config file. * @param config - a configuration file */ export function getAppScopes(config: AppConfiguration): string { - if (isLegacyAppSchema(config)) { - return config.scopes - } else if (isCurrentAppSchema(config)) { - return config.access_scopes?.scopes ?? '' - } - return '' + return (config as CurrentAppConfiguration).access_scopes?.scopes ?? '' } /** @@ -194,9 +136,7 @@ export function getAppScopesArray(config: AppConfiguration) { } export function usesLegacyScopesBehavior(config: AppConfiguration) { - if (isLegacyAppSchema(config)) return true - if (isCurrentAppSchema(config)) return config.access_scopes?.use_legacy_install_flow ?? false - return false + return (config as CurrentAppConfiguration).access_scopes?.use_legacy_install_flow === true } export function appHiddenConfigPath(appDirectory: string) { @@ -510,8 +450,7 @@ export class App< } get appIsEmbedded() { - if (isCurrentAppSchema(this.configuration)) return this.configuration.embedded - return this.appIsLaunchable() + return (this.configuration as CurrentAppConfiguration).embedded ?? this.appIsLaunchable() } creationDefaultOptions(): CreateAppOptions { @@ -555,25 +494,24 @@ export class App< } get includeConfigOnDeploy() { - if (isLegacyAppSchema(this.configuration)) return false return this.configuration.build?.include_config_on_deploy } private patchAppConfiguration(devApplicationURLs: ApplicationURLs) { - if (!isCurrentAppSchema(this.configuration)) return - + const config = this.configuration as CurrentAppConfiguration this.devApplicationURLs = devApplicationURLs - this.configuration.application_url = devApplicationURLs.applicationUrl + config.application_url = devApplicationURLs.applicationUrl if (devApplicationURLs.appProxy) { - this.configuration.app_proxy = { + config.app_proxy = { url: devApplicationURLs.appProxy.proxyUrl, subpath: devApplicationURLs.appProxy.proxySubPath, prefix: devApplicationURLs.appProxy.proxySubPathPrefix, } } - if (this.configuration.auth?.redirect_urls) { - this.configuration.auth.redirect_urls = devApplicationURLs.redirectUrlWhitelist + if (config.auth?.redirect_urls) { + config.auth.redirect_urls = devApplicationURLs.redirectUrlWhitelist } + this.configuration = config as TConfig } /** @@ -583,13 +521,14 @@ export class App< * @throws When app-specific webhooks are used with legacy install flow */ private validateWebhookLegacyFlowCompatibility(): void { - if (!isCurrentAppSchema(this.configuration)) return + // These fields might not exist on basic AppConfiguration but could be added by specifications + const config = this.configuration as CurrentAppConfiguration const hasAppSpecificWebhooks = - this.configuration.webhooks?.subscriptions?.some( - (subscription) => subscription.topics && subscription.topics.length > 0, + config.webhooks?.subscriptions?.some( + (subscription: WebhookSubscription) => subscription.topics && subscription.topics.length > 0, ) ?? false - const usesLegacyInstallFlow = this.configuration.access_scopes?.use_legacy_install_flow === true + const usesLegacyInstallFlow = config.access_scopes?.use_legacy_install_flow === true if (hasAppSpecificWebhooks && usesLegacyInstallFlow) { throw new AbortError( diff --git a/packages/app/src/cli/models/app/loader.test.ts b/packages/app/src/cli/models/app/loader.test.ts index a3bb741e08f..2c128f10327 100644 --- a/packages/app/src/cli/models/app/loader.test.ts +++ b/packages/app/src/cli/models/app/loader.test.ts @@ -12,8 +12,9 @@ import { loadHiddenConfig, } from './loader.js' import {parseHumanReadableError} from './error-parsing.js' -import {App, AppLinkedInterface, LegacyAppSchema, WebConfigurationSchema} from './app.js' +import {App, AppInterface, AppLinkedInterface, AppSchema, WebConfigurationSchema} from './app.js' import {DEFAULT_CONFIG, buildVersionedAppSchema, getWebhookConfig} from './app.test-data.js' +import {ExtensionInstance} from '../extensions/extension-instance.js' import {configurationFileNames, blocks} from '../../constants.js' import metadata from '../../metadata.js' import {loadLocalExtensionsSpecifications} from '../extensions/load-specifications.js' @@ -63,10 +64,44 @@ describe('load', () => { return loadApp({directory: tmpDir, specifications, userProvidedConfigName: undefined, ...extras}) } - const appConfiguration = ` + // Helper to get only real extensions (not configuration extensions) + function getRealExtensions(app: AppInterface) { + return app.allExtensions.filter((ext) => ext.specification.experience !== 'configuration') + } + + // Basic configuration without extensions - used for tests that expect no extensions + const basicAppConfiguration = ` name = "my_app" -scopes = "read_products" +client_id = "test-client-id" +application_url = "https://example.com" +embedded = true + +[auth] +redirect_urls = ["https://example.com/callback"] + +[webhooks] +api_version = "2024-01" +` + + // Minimal configuration without webhook subscriptions - used for tests that check exact extension counts + const minimalAppConfiguration = ` +name = "my_app" +client_id = "test-client-id" +application_url = "https://example.com" +embedded = true + +[auth] +redirect_urls = ["https://example.com/callback"] + +[webhooks] +api_version = "2024-01" ` + + // Configuration for tests that don't care about extension counts + const appConfigurationWithWebhooks = minimalAppConfiguration + + // Configuration for tests that check exact extension counts (no webhook subscriptions) + const appConfiguration = minimalAppConfiguration const linkedAppConfiguration = ` name = "for-testing" client_id = "1234567890" @@ -521,9 +556,10 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp({mode: 'local'}) // Then - expect(app.allExtensions).toHaveLength(1) - expect(app.allExtensions[0]!.configuration.name).toBe('my_extension') - expect(app.allExtensions[0]!.configuration.type).toBe('theme') + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + expect(realExtensions[0]!.configuration.name).toBe('my_extension') + expect(realExtensions[0]!.configuration.type).toBe('theme') expect(app.errors).toBeUndefined() }) @@ -726,8 +762,17 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app with custom located web blocks', async () => { // Given const {webDirectory} = await writeConfig(` - scopes = "" + name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true web_directories = ["must_be_here"] + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" `) await moveFile(webDirectory, joinPath(tmpDir, 'must_be_here')) @@ -741,8 +786,17 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app with custom located web blocks, only checks given directory', async () => { // Given const {webDirectory} = await writeConfig(` - scopes = "" + name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true web_directories = ["must_be_here"] + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" `) await moveFile(webDirectory, joinPath(tmpDir, 'cannot_be_here')) @@ -808,8 +862,17 @@ redirect_urls = [ "https://example.com/api/auth" ] test('loads the app when it has a extension with a valid configuration using a supported extension type and in a non-conventional directory configured in the app configuration file', async () => { // Given await writeConfig(` - scopes = "" + name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true extension_directories = ["custom_extension"] + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" `) const customExtensionDirectory = joinPath(tmpDir, 'custom_extension') await mkdir(customExtensionDirectory) @@ -884,8 +947,11 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(2) - const extensions = app.allExtensions.sort((extA, extB) => (extA.name < extB.name ? -1 : 1)) + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(2) + const extensions = realExtensions.sort((extA: ExtensionInstance, extB: ExtensionInstance) => + extA.name < extB.name ? -1 : 1, + ) expect(extensions[0]!.configuration.name).toBe('my_extension_1') expect(extensions[0]!.idEnvironmentVariableName).toBe('SHOPIFY_MY_EXTENSION_1_ID') expect(extensions[1]!.configuration.name).toBe('my_extension_2') @@ -930,8 +996,11 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(2) - const extensions = app.allExtensions.sort((extA, extB) => (extA.name < extB.name ? -1 : 1)) + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(2) + const extensions = realExtensions.sort((extA: ExtensionInstance, extB: ExtensionInstance) => + extA.name < extB.name ? -1 : 1, + ) expect(extensions[0]!.configuration.name).toBe('my_extension_1') expect(extensions[0]!.configuration.type).toBe('checkout_post_purchase') expect(extensions[0]!.configuration.api_version).toBe('2022-07') @@ -1108,8 +1177,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1176,8 +1246,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1254,8 +1325,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1338,8 +1410,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1414,8 +1487,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1528,8 +1602,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1641,8 +1716,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1696,8 +1772,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1743,8 +1820,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1807,8 +1885,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1883,8 +1962,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -1979,8 +2059,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -2046,8 +2127,9 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(1) - const extension = app.allExtensions[0] + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(1) + const extension = realExtensions[0] expect(extension).not.toBeUndefined() if (extension) { expect(extension.configuration).toMatchObject({ @@ -2216,8 +2298,11 @@ redirect_urls = [ "https://example.com/api/auth" ] const app = await loadTestingApp() // Then - expect(app.allExtensions).toHaveLength(2) - const functions = app.allExtensions.sort((extA, extB) => (extA.name < extB.name ? -1 : 1)) + const realExtensions = getRealExtensions(app) + expect(realExtensions).toHaveLength(2) + const functions = realExtensions.sort((extA: ExtensionInstance, extB: ExtensionInstance) => + extA.name < extB.name ? -1 : 1, + ) expect(functions[0]!.configuration.name).toBe('my-function-1') expect(functions[1]!.configuration.name).toBe('my-function-2') expect(functions[0]!.idEnvironmentVariableName).toBe('SHOPIFY_MY_FUNCTION_1_ID') @@ -2282,7 +2367,9 @@ redirect_urls = [ "https://example.com/api/auth" ] expect(metadata.getAllPublicMetadata()).toMatchObject({ project_type: 'node', env_package_manager_workspaces: false, - ...configAsCodeLegacyMetadata(), + cmd_app_all_configs_any: true, + cmd_app_all_configs_clients: JSON.stringify({'shopify.app.toml': 'test-client-id'}), + cmd_app_linked_config_used: true, }) }) @@ -2300,7 +2387,9 @@ redirect_urls = [ "https://example.com/api/auth" ] expect(metadata.getAllPublicMetadata()).toMatchObject({ project_type: 'node', env_package_manager_workspaces: true, - ...configAsCodeLegacyMetadata(), + cmd_app_all_configs_any: true, + cmd_app_all_configs_clients: JSON.stringify({'shopify.app.toml': 'test-client-id'}), + cmd_app_linked_config_used: true, }) }) @@ -2875,7 +2964,7 @@ describe('parseConfigurationObject', () => { expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp') }) - test('throws an error if fields are missing in a legacy schema TOML file', async () => { + test('throws an error if fields are missing in an app schema TOML file', async () => { const configurationObject = { scopes: [], } @@ -2884,16 +2973,16 @@ describe('parseConfigurationObject', () => { { code: 'invalid_type', expected: 'string', - received: 'array', - path: ['scopes'], - message: 'Expected string, received array', + received: 'undefined', + path: ['client_id'], + message: 'Required', }, ] const expectedFormatted = outputContent`\n${outputToken.errorText( 'Validation errors', )} in tmp:\n\n${parseHumanReadableError(errorObject)}` const abortOrReport = vi.fn() - await parseConfigurationObject(LegacyAppSchema, 'tmp', configurationObject, abortOrReport) + await parseConfigurationObject(AppSchema, 'tmp', configurationObject, abortOrReport) expect(abortOrReport).toHaveBeenCalledWith(expectedFormatted, {}, 'tmp') }) @@ -3442,32 +3531,12 @@ describe('WebhooksSchema', () => { describe('getAppConfigurationState', () => { test.each([ - [ - `scopes = " write_xyz, write_abc "`, - { - state: 'template-only', - configSource: 'cached', - configurationFileName: 'shopify.app.toml', - appDirectory: expect.any(String), - configurationPath: expect.stringMatching(/shopify.app.toml$/), - startingOptions: { - path: expect.stringMatching(/shopify.app.toml$/), - scopes: 'write_abc,write_xyz', - }, - }, - ], [ `client_id="abcdef"`, { state: 'connected-app', }, ], - [ - ``, - { - state: 'template-only', - }, - ], [ `client_id="abcdef" something_extra="keep"`, @@ -3495,7 +3564,7 @@ describe('getAppConfigurationState', () => { test('raises validation error when AppSchema is missing client_id', async () => { await inTemporaryDirectory(async (tmpDir) => { // We know this is a TOML file that follows the AppSchema because - // it can contain extra fields (something_extra). In LegacyAppSchema, all fields are optional. + // it can contain extra fields (something_extra). // This content is also missing a client_id field which should throw a validation error. const content = `something_extra = "some_value"` const appConfigPath = joinPath(tmpDir, 'shopify.app.toml') @@ -3513,8 +3582,19 @@ describe('loadConfigForAppCreation', () => { // Given await inTemporaryDirectory(async (tmpDir) => { const config = ` - scopes = "write_products,read_orders" name = "my-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [access_scopes] + scopes = "write_products,read_orders" + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) await writeFile(joinPath(tmpDir, 'package.json'), '{}') @@ -3525,7 +3605,7 @@ describe('loadConfigForAppCreation', () => { // Then expect(result).toEqual({ isLaunchable: false, - scopesArray: ['read_orders', 'write_products'], + scopesArray: ['write_products', 'read_orders'], name: 'my-app', directory: tmpDir, isEmbedded: false, @@ -3537,8 +3617,19 @@ describe('loadConfigForAppCreation', () => { // Given await inTemporaryDirectory(async (tmpDir) => { const config = ` - scopes = "write_products" + client_id = "test-client-id" name = "my-app" + application_url = "https://example.com" + embedded = true + + [access_scopes] + scopes = "write_products" + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) await writeFile(joinPath(tmpDir, 'package.json'), '{}') @@ -3571,8 +3662,19 @@ dev = "echo 'Hello, world!'" // Given await inTemporaryDirectory(async (tmpDir) => { const config = ` - scopes = "write_products" + client_id = "test-client-id" name = "my-app" + application_url = "https://example.com" + embedded = true + + [access_scopes] + scopes = "write_products" + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) await writeFile(joinPath(tmpDir, 'package.json'), '{}') @@ -3635,6 +3737,7 @@ dev = "echo 'Hello, world!'" await inTemporaryDirectory(async (tmpDir) => { const config = ` name = "my-app" + client_id = "test-client-id" scopes = "" ` await writeFile(joinPath(tmpDir, 'shopify.app.toml'), config) @@ -3656,22 +3759,6 @@ dev = "echo 'Hello, world!'" }) describe('loadHiddenConfig', () => { - test('returns empty object if no client_id in configuration', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const configuration = { - path: joinPath(tmpDir, 'shopify.app.toml'), - scopes: 'write_products', - } - - // When - const got = await loadHiddenConfig(tmpDir, configuration) - - // Then - expect(got).toEqual({}) - }) - }) - test('returns empty object if hidden config file does not exist', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given diff --git a/packages/app/src/cli/models/app/loader.ts b/packages/app/src/cli/models/app/loader.ts index da8531b1f94..6104b6e7a7f 100644 --- a/packages/app/src/cli/models/app/loader.ts +++ b/packages/app/src/cli/models/app/loader.ts @@ -6,18 +6,14 @@ import { WebType, getAppScopesArray, AppConfigurationInterface, - LegacyAppSchema, AppConfiguration, CurrentAppConfiguration, getAppVersionedSchema, - isCurrentAppSchema, AppSchema, - LegacyAppConfiguration, BasicAppConfigurationWithoutModules, SchemaForConfig, AppLinkedInterface, AppHiddenConfig, - isLegacyAppSchema, } from './app.js' import {parseHumanReadableError} from './error-parsing.js' import {configurationFileNames, dotEnvFileNames} from '../../constants.js' @@ -215,7 +211,7 @@ export async function checkFolderIsValidApp(directory: string) { export async function loadConfigForAppCreation(directory: string, name: string): Promise { const state = await getAppConfigurationState(directory) - const config: AppConfiguration = state.state === 'connected-app' ? state.basicConfiguration : state.startingOptions + const config: AppConfiguration = state.basicConfiguration const loadedConfiguration = await loadAppConfigurationFromState(state, [], []) const loader = new AppLoader({loadedConfiguration}) @@ -298,7 +294,7 @@ export async function loadAppUsingConfigurationState = TConfigState extends AppConfigurationStateLinked ? CurrentAppConfiguration - : LegacyAppConfiguration + : never export function getDotEnvFileName(configurationPath: string) { const configurationShorthand: string | undefined = getAppConfigurationShorthand(configurationPath) @@ -521,13 +517,9 @@ class AppLoader specification.uidStrategy === 'single') @@ -751,12 +743,13 @@ class AppLoader configuration.auth_callback_path).find((path) => path), - currentConfiguration.app_proxy, + // eslint-disable-next-line @typescript-eslint/no-explicit-any + (currentConfiguration as any).app_proxy, ) } } @@ -822,12 +815,7 @@ export type AppConfigurationStateLinked = AppConfigurationStateBasics & { basicConfiguration: BasicAppConfigurationWithoutModules } -type AppConfigurationStateTemplate = AppConfigurationStateBasics & { - state: 'template-only' - startingOptions: Omit -} - -type AppConfigurationState = AppConfigurationStateLinked | AppConfigurationStateTemplate +type AppConfigurationState = AppConfigurationStateLinked /** * Get the app configuration state from the file system. @@ -865,34 +853,17 @@ export async function getAppConfigurationState( const {configurationPath, configurationFileName} = await getConfigurationPath(appDirectory, configName) const file = await loadConfigurationFileContent(configurationPath) - const configFileHasNotBeenLinked = isLegacyAppSchema(file as AppConfiguration) - - if (configFileHasNotBeenLinked) { - const parsedConfig = await parseConfigurationFile(LegacyAppSchema, configurationPath) - return { - appDirectory, - configurationPath, - state: 'template-only', - startingOptions: { - ...file, - ...parsedConfig, - }, - configSource, - configurationFileName, - } - } else { - const parsedConfig = await parseConfigurationFile(AppSchema, configurationPath) - return { - state: 'connected-app', - appDirectory, - configurationPath, - basicConfiguration: { - ...file, - ...parsedConfig, - }, - configSource, - configurationFileName, - } + const parsedConfig = await parseConfigurationFile(AppSchema, configurationPath) + return { + state: 'connected-app', + appDirectory, + configurationPath, + basicConfiguration: { + ...file, + ...parsedConfig, + }, + configSource, + configurationFileName, } } @@ -909,32 +880,12 @@ async function loadAppConfigurationFromState< specifications: TModuleSpec[], remoteFlags: Flag[], ): Promise, TModuleSpec>> { - let file: JsonMapType - let schemaForConfigurationFile: SchemaForConfig> - { - let appSchema - switch (configState.state) { - case 'template-only': { - file = { - ...configState.startingOptions, - } - delete file.path - appSchema = LegacyAppSchema as unknown as SchemaForConfig> - break - } - case 'connected-app': { - file = { - ...configState.basicConfiguration, - } - delete file.path - const appVersionedSchema = getAppVersionedSchema(specifications) - appSchema = appVersionedSchema as SchemaForConfig> - break - } - } - - schemaForConfigurationFile = appSchema + const file: JsonMapType = { + ...configState.basicConfiguration, } + delete file.path + const appVersionedSchema = getAppVersionedSchema(specifications) + const schemaForConfigurationFile = appVersionedSchema as SchemaForConfig> const configuration = (await parseConfigurationFile( schemaForConfigurationFile, @@ -953,30 +904,22 @@ async function loadAppConfigurationFromState< } // enhance metadata based on the config type - switch (configState.state) { - case 'template-only': { - // nothing to add - break - } - case 'connected-app': { - let gitTracked = false - try { - gitTracked = await checkIfGitTracked(configState.appDirectory, configState.configurationPath) - // eslint-disable-next-line no-catch-all/no-catch-all - } catch { - // leave as false - } + let gitTracked = false + try { + gitTracked = await checkIfGitTracked(configState.appDirectory, configState.configurationPath) + // eslint-disable-next-line no-catch-all/no-catch-all + } catch { + // leave as false + } - configurationLoadResultMetadata = { - ...configurationLoadResultMetadata, - usesLinkedConfig: true, - name: configState.configurationFileName, - gitTracked, - source: configState.configSource, - usesCliManagedUrls: (configuration as LoadedAppConfigFromConfigState).build - ?.automatically_update_urls_on_dev, - } - } + configurationLoadResultMetadata = { + ...configurationLoadResultMetadata, + usesLinkedConfig: true, + name: configState.configurationFileName, + gitTracked, + source: configState.configSource, + usesCliManagedUrls: (configuration as LoadedAppConfigFromConfigState).build + ?.automatically_update_urls_on_dev, } return { diff --git a/packages/app/src/cli/models/extensions/specifications/app_config_privacy_compliance_webhooks.test.ts b/packages/app/src/cli/models/extensions/specifications/app_config_privacy_compliance_webhooks.test.ts index c7d99cb753c..4558a4c2c6d 100644 --- a/packages/app/src/cli/models/extensions/specifications/app_config_privacy_compliance_webhooks.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/app_config_privacy_compliance_webhooks.test.ts @@ -99,7 +99,7 @@ describe('privacy_compliance_webhooks', () => { }, } const privacyComplianceSpec = spec - const appConfiguration = {application_url: 'https://example.com/', scopes: ''} + const appConfiguration = {client_id: 'test-client-id', application_url: 'https://example.com/', scopes: ''} // When const result = privacyComplianceSpec.transformLocalToRemote!(object, appConfiguration) diff --git a/packages/app/src/cli/services/app-context.test.ts b/packages/app/src/cli/services/app-context.test.ts index 37ffaf956cc..a786c64a747 100644 --- a/packages/app/src/cli/services/app-context.test.ts +++ b/packages/app/src/cli/services/app-context.test.ts @@ -80,61 +80,6 @@ client_id="test-api-key"` }) }) - test('links app when it is not linked, and config file is cached', async () => { - await inTemporaryDirectory(async (tmp) => { - const content = '' - await writeAppConfig(tmp, content, 'shopify.app.stg.toml') - localStorage.setCachedAppInfo({ - appId: 'test-api-key', - title: 'Test App', - directory: tmp, - orgId: 'test-org-id', - configFile: 'shopify.app.stg.toml', - }) - - // Given - vi.mocked(link).mockResolvedValue({ - remoteApp: mockRemoteApp, - state: { - state: 'connected-app', - appDirectory: tmp, - configurationPath: `${tmp}/shopify.app.stg.toml`, - configSource: 'cached', - configurationFileName: 'shopify.app.stg.toml', - basicConfiguration: { - client_id: 'test-api-key', - path: normalizePath(joinPath(tmp, 'shopify.app.stg.toml')), - }, - }, - configuration: { - client_id: 'test-api-key', - name: 'test-app', - application_url: 'https://test-app.com', - path: normalizePath(joinPath(tmp, 'shopify.app.stg.toml')), - embedded: false, - }, - }) - - // When - const result = await linkedAppContext({ - directory: tmp, - forceRelink: false, - userProvidedConfigName: undefined, - clientId: undefined, - }) - - // Then - expect(result).toEqual({ - app: expect.any(Object), - remoteApp: mockRemoteApp, - developerPlatformClient: expect.any(Object), - specifications: [], - organization: mockOrganization, - }) - expect(link).toHaveBeenCalledWith({directory: tmp, apiKey: undefined, configName: 'shopify.app.stg.toml'}) - }) - }) - test('updates cached app info when remoteApp matches', async () => { await inTemporaryDirectory(async (tmp) => { // Given @@ -329,6 +274,15 @@ describe('localAppContext', () => { // Given const content = ` name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` await writeAppConfig(tmp, content) @@ -359,6 +313,15 @@ describe('localAppContext', () => { // Given const content = ` name = "test-app-custom" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` await writeAppConfig(tmp, content, 'shopify.app.custom.toml') @@ -384,6 +347,15 @@ describe('localAppContext', () => { // Given const appContent = ` name = "test-app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` const extensionContent = ` type = "ui_extension" @@ -402,6 +374,7 @@ describe('localAppContext', () => { await writeFile(joinPath(srcDir, 'index.js'), '// Extension code') // Mock local specifications to include ui_extension with proper validation + // Also include app_access and webhooks specs that contribute auth and webhooks to schema vi.mocked(loadLocalExtensionsSpecifications).mockResolvedValue([ { identifier: 'ui_extension', @@ -427,6 +400,36 @@ describe('localAppContext', () => { validate: async () => ({isErr: () => false, isOk: () => true} as any), contributeToAppConfigurationSchema: (schema: any) => schema, } as any, + { + identifier: 'app_access', + experience: 'configuration', + uidStrategy: 'single', + appModuleFeatures: () => [], + parseConfigurationObject: (obj: any) => ({ + state: 'ok', + data: obj, + errors: undefined, + }), + contributeToAppConfigurationSchema: (schema: any) => { + // Mock contribution of auth field + return schema + }, + } as any, + { + identifier: 'webhooks', + experience: 'configuration', + uidStrategy: 'single', + appModuleFeatures: () => [], + parseConfigurationObject: (obj: any) => ({ + state: 'ok', + data: obj, + errors: undefined, + }), + contributeToAppConfigurationSchema: (schema: any) => { + // Mock contribution of webhooks field + return schema + }, + } as any, ]) // When @@ -435,8 +438,9 @@ describe('localAppContext', () => { }) // Then - expect(result.allExtensions).toHaveLength(1) - expect(result.allExtensions[0]).toEqual( + const realExtensions = result.allExtensions.filter((ext) => ext.specification.experience !== 'configuration') + expect(realExtensions).toHaveLength(1) + expect(realExtensions[0]).toEqual( expect.objectContaining({ configuration: expect.objectContaining({ name: 'test-extension', diff --git a/packages/app/src/cli/services/app-context.ts b/packages/app/src/cli/services/app-context.ts index 61d4e9c923b..d480c8162d1 100644 --- a/packages/app/src/cli/services/app-context.ts +++ b/packages/app/src/cli/services/app-context.ts @@ -72,11 +72,10 @@ export async function linkedAppContext({ let configState = await getAppConfigurationState(directory, userProvidedConfigName) let remoteApp: OrganizationApp | undefined - // If the app is not linked, force a link. - if (configState.state === 'template-only' || forceRelink) { + // If forceRelink is true, force a link. + if (forceRelink) { // If forceRelink is true, we don't want to use the cached config name and instead prompt the user for a new one. - const configName = forceRelink ? undefined : configState.configurationFileName - const result = await link({directory, apiKey: clientId, configName}) + const result = await link({directory, apiKey: clientId, configName: undefined}) remoteApp = result.remoteApp configState = result.state } diff --git a/packages/app/src/cli/services/app/config/link-service.test.ts b/packages/app/src/cli/services/app/config/link-service.test.ts index b718461d2c5..e3d7b0366d7 100644 --- a/packages/app/src/cli/services/app/config/link-service.test.ts +++ b/packages/app/src/cli/services/app/config/link-service.test.ts @@ -3,12 +3,14 @@ import {testOrganizationApp, testDeveloperPlatformClient} from '../../../models/ import {DeveloperPlatformClient, selectDeveloperPlatformClient} from '../../../utilities/developer-platform-client.js' import {OrganizationApp, OrganizationSource} from '../../../models/organization.js' import {appNamePrompt, createAsNewAppPrompt, selectOrganizationPrompt} from '../../../prompts/dev.js' +import {selectConfigName} from '../../../prompts/config.js' import {beforeEach, describe, expect, test, vi} from 'vitest' import {inTemporaryDirectory, readFile, writeFileSync} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' vi.mock('./use.js') vi.mock('../../../prompts/dev.js') +vi.mock('../../../prompts/config.js') vi.mock('../../local-storage') vi.mock('@shopify/cli-kit/node/ui') vi.mock('../../dev/fetch.js') @@ -54,7 +56,19 @@ describe('link, with minimal mocking', () => { test('load from a fresh template, return a connected app', async () => { await inTemporaryDirectory(async (tmp) => { const initialContent = ` - scopes='write_something_unusual' +name = "test-app" +client_id = "test-client-id" +application_url = "https://example.com" +embedded = true + +[access_scopes] +scopes = "write_something_unusual" + +[auth] +redirect_urls = ["https://example.com/callback"] + +[webhooks] +api_version = "2024-01" ` const filePath = joinPath(tmp, 'shopify.app.toml') writeFileSync(filePath, initialContent) @@ -69,6 +83,7 @@ describe('link, with minimal mocking', () => { businessName: 'test', source: OrganizationSource.BusinessPlatform, }) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') const options = { directory: tmp, diff --git a/packages/app/src/cli/services/app/config/link.test.ts b/packages/app/src/cli/services/app/config/link.test.ts index 15735c34b2d..a5bd4e858ee 100644 --- a/packages/app/src/cli/services/app/config/link.test.ts +++ b/packages/app/src/cli/services/app/config/link.test.ts @@ -87,10 +87,10 @@ describe('link', () => { expect(configuration).toEqual({ client_id: '12345', name: 'app1', - extension_directories: [], application_url: 'https://example.com', embedded: true, access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, auth: { @@ -103,6 +103,7 @@ describe('link', () => { embedded: false, }, path: expect.stringMatching(/\/shopify.app.default-value.toml$/), + build: undefined, }) expect(state).toEqual({ @@ -144,13 +145,13 @@ describe('link', () => { const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -165,10 +166,10 @@ embedded = false expect(configuration).toEqual({ client_id: '12345', name: 'app1', - extension_directories: [], application_url: 'https://example.com', embedded: true, access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, auth: { @@ -181,6 +182,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.staging.toml$/), + build: undefined, }) expect(content).toEqual(expectedContent) }) @@ -195,6 +197,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockRejectedValue('App not found') + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') const apiClientConfiguration = { title: 'new-title', applicationUrl: 'https://api-client-config.com', @@ -313,6 +316,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockRejectedValue('App not found') + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') const apiClientConfiguration = { title: 'new-title', applicationUrl: 'https://api-client-config.com', @@ -661,6 +665,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockResolvedValue(await mockApp(tmp)) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) // When @@ -671,13 +676,13 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -707,11 +712,11 @@ embedded = false }) expect(configuration).toEqual({ client_id: '12345', - extension_directories: [], name: 'app1', application_url: 'https://example.com', embedded: true, access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, auth: { @@ -724,6 +729,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.toml$/), + build: undefined, }) expect(content).toEqual(expectedContent) }) @@ -742,6 +748,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockResolvedValue(await mockApp(tmp)) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) // When @@ -752,13 +759,13 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -776,8 +783,8 @@ embedded = false name: 'app1', application_url: 'https://example.com', embedded: true, - extension_directories: [], access_scopes: { + scopes: 'read_products', use_legacy_install_flow: true, }, auth: { @@ -790,6 +797,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.toml$/), + build: undefined, }) expect(content).toEqual(expectedContent) }) @@ -805,7 +813,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockResolvedValue(await mockApp(tmp)) - vi.mocked(selectConfigName).mockResolvedValue('shopify.app.staging.toml') + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(appFromIdentifiers).mockImplementation(async ({apiKey}: {apiKey: string}) => { return (await developerPlatformClient.appFromIdentifiers(apiKey))! }) @@ -818,7 +826,6 @@ embedded = false expect(content).toContain('name = "app1"') expect(configuration).toEqual({ client_id: 'api-key', - extension_directories: [], name: 'app1', application_url: 'https://example.com', embedded: true, @@ -835,6 +842,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.toml$/), + build: undefined, }) }) }) @@ -956,6 +964,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockRejectedValue(new Error('Shopify.app.toml not found')) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) // When @@ -1016,6 +1025,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockResolvedValue(await mockApp(tmp)) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) const remoteConfiguration = { ...DEFAULT_REMOTE_CONFIGURATION, @@ -1031,7 +1041,6 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true @@ -1039,6 +1048,7 @@ embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes scopes = "read_products,write_orders" +use_legacy_install_flow = true [auth] redirect_urls = [ "https://example.com/callback1" ] @@ -1052,12 +1062,12 @@ embedded = false expect(configuration).toEqual({ client_id: '12345', - extension_directories: [], name: 'app1', application_url: 'https://example.com', embedded: true, access_scopes: { scopes: 'read_products,write_orders', + use_legacy_install_flow: true, }, auth: { redirect_urls: ['https://example.com/callback1'], @@ -1069,6 +1079,7 @@ embedded = false embedded: false, }, path: expect.stringMatching(/\/shopify.app.toml$/), + build: undefined, }) expect(content).toEqual(expectedContent) }) @@ -1107,6 +1118,7 @@ embedded = false } vi.mocked(loadApp).mockRejectedValue('App not found') + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) const remoteConfiguration = { ...DEFAULT_REMOTE_CONFIGURATION, @@ -1246,6 +1258,8 @@ embedded = false developerPlatformClient, } + vi.mocked(loadApp).mockRejectedValue('App not found') + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) const remoteConfiguration = { ...DEFAULT_REMOTE_CONFIGURATION, @@ -1431,6 +1445,7 @@ embedded = true developerPlatformClient, } vi.mocked(loadApp).mockResolvedValue(await mockApp(tmp)) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue({ ...mockRemoteApp(), newApp: true, @@ -1445,7 +1460,6 @@ embedded = true const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true @@ -1455,6 +1469,7 @@ include_config_on_deploy = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1483,6 +1498,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockResolvedValue(await mockApp(tmp)) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue({ ...mockRemoteApp(), newApp: true, @@ -1497,7 +1513,6 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true @@ -1508,6 +1523,7 @@ include_config_on_deploy = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1532,6 +1548,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockResolvedValue(await mockApp(tmp)) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) const remoteConfiguration = { ...DEFAULT_REMOTE_CONFIGURATION, @@ -1549,13 +1566,13 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" application_url = "https://example.com" embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1580,6 +1597,7 @@ embedded = false developerPlatformClient, } vi.mocked(loadApp).mockResolvedValue(await mockApp(tmp)) + vi.mocked(selectConfigName).mockResolvedValue('shopify.app.toml') vi.mocked(fetchOrCreateOrganizationApp).mockResolvedValue(mockRemoteApp({developerPlatformClient})) const remoteConfiguration = { ...DEFAULT_REMOTE_CONFIGURATION, @@ -1595,7 +1613,6 @@ embedded = false const expectedContent = `# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration client_id = "12345" -extension_directories = [ ] name = "app1" handle = "handle" application_url = "https://example.com" @@ -1603,6 +1620,7 @@ embedded = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes +scopes = "read_products" use_legacy_install_flow = true [auth] @@ -1864,7 +1882,7 @@ async function mockApp( ) { const versionSchema = await buildVersionedAppSchema() const localApp = testApp(app) - localApp.configuration.client_id = schemaType === 'legacy' ? 12345 : '12345' + localApp.configuration.client_id = '12345' localApp.configSchema = versionSchema.schema localApp.specifications = versionSchema.configSpecifications localApp.directory = directory diff --git a/packages/app/src/cli/services/app/config/link.ts b/packages/app/src/cli/services/app/config/link.ts index 6782eed2900..1b68f79af17 100644 --- a/packages/app/src/cli/services/app/config/link.ts +++ b/packages/app/src/cli/services/app/config/link.ts @@ -3,10 +3,8 @@ import { AppConfiguration, CurrentAppConfiguration, getAppVersionedSchema, - isCurrentAppSchema, CliBuildPreferences, getAppScopes, - LegacyAppConfiguration, } from '../../../models/app/app.js' import {OrganizationApp} from '../../../models/organization.js' import {selectConfigName} from '../../../prompts/config.js' @@ -23,7 +21,6 @@ import { InvalidApiKeyErrorMessage, } from '../../context.js' import {Flag, DeveloperPlatformClient, CreateAppOptions} from '../../../utilities/developer-platform-client.js' -import {configurationFileNames} from '../../../constants.js' import {writeAppConfigurationFile} from '../write-app-configuration-file.js' import {getCachedCommandInfo} from '../../local-storage.js' import {RemoteAwareExtensionSpecification} from '../../../models/extensions/specification.js' @@ -190,16 +187,6 @@ async function getAppCreationDefaultsFromLocalApp(options: LinkOptions): Promise } type LocalAppOptions = - | { - state: 'legacy' - configFormat: 'legacy' - scopes: string - localAppIdMatchedRemote: false - existingBuildOptions: undefined - existingConfig: LegacyAppConfiguration - appDirectory: string - packageManager: PackageManager - } | { state: 'reusable-current-app' configFormat: 'current' @@ -224,7 +211,7 @@ type LocalAppOptions = } | { state: 'unable-to-load-config' - configFormat: 'legacy' + configFormat: 'current' localAppIdMatchedRemote: false } )) @@ -257,25 +244,14 @@ async function loadLocalAppOptions( }) const configuration = app.configuration - if (!isCurrentAppSchema(configuration)) { - return { - state: 'legacy', - configFormat: 'legacy', - scopes: getAppScopes(configuration), - localAppIdMatchedRemote: false, - existingBuildOptions: undefined, - existingConfig: configuration as LegacyAppConfiguration, - appDirectory: app.directory, - packageManager: app.packageManager, - } - } else if (app.configuration.client_id === remoteAppApiKey || options.isNewApp) { + if (app.configuration.client_id === remoteAppApiKey || options.isNewApp) { return { state: 'reusable-current-app', configFormat: 'current', scopes: getAppScopes(configuration), localAppIdMatchedRemote: true, existingBuildOptions: configuration.build, - existingConfig: configuration, + existingConfig: configuration as CurrentAppConfiguration, appDirectory: app.directory, packageManager: app.packageManager, } @@ -294,7 +270,7 @@ async function loadLocalAppOptions( } catch (error) { return { state: 'unable-to-load-config', - configFormat: 'legacy', + configFormat: 'current', scopes: '', localAppIdMatchedRemote: false, appDirectory: undefined, @@ -317,7 +293,7 @@ async function loadConfigurationFileName( options: LinkOptions, localAppInfo: { appDirectory?: string - format: 'legacy' | 'current' + format: 'current' }, ): Promise { // config name from the options takes precedence over everything else @@ -329,10 +305,6 @@ async function loadConfigurationFileName( const cache = getCachedCommandInfo() if (cache?.selectedToml) return cache.selectedToml as AppConfigurationFileName - if (localAppInfo.format === 'legacy') { - return configurationFileNames.app - } - const existingTomls = await getTomls(options.directory) const currentToml = existingTomls[remoteApp.apiKey] if (currentToml) return currentToml 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 6f8418c59b0..bf20c69aea6 100644 --- a/packages/app/src/cli/services/app/config/use.test.ts +++ b/packages/app/src/cli/services/app/config/use.test.ts @@ -79,9 +79,11 @@ describe('use', () => { const {schema: configSchema} = await buildVersionedAppSchema() const appWithoutClientID = testApp() + // Create a configuration without client_id to test the error case + const {client_id: clientId, ...configWithoutClientId} = appWithoutClientID.configuration vi.mocked(loadAppConfiguration).mockResolvedValue({ directory: tmp, - configuration: appWithoutClientID.configuration, + configuration: configWithoutClientId as any, configSchema, specifications: [], remoteFlags: [], diff --git a/packages/app/src/cli/services/app/config/use.ts b/packages/app/src/cli/services/app/config/use.ts index d99123cf58b..fa0a2ffee55 100644 --- a/packages/app/src/cli/services/app/config/use.ts +++ b/packages/app/src/cli/services/app/config/use.ts @@ -1,7 +1,7 @@ import {getAppConfigurationFileName, loadAppConfiguration} from '../../../models/app/loader.js' import {clearCurrentConfigFile, setCachedAppInfo} from '../../local-storage.js' import {selectConfigFile} from '../../../prompts/config.js' -import {AppConfiguration, CurrentAppConfiguration, isCurrentAppSchema} from '../../../models/app/app.js' +import {AppConfiguration, CurrentAppConfiguration} from '../../../models/app/app.js' import {DeveloperPlatformClient} from '../../../utilities/developer-platform-client.js' import {AbortError} from '@shopify/cli-kit/node/error' import {fileExists} from '@shopify/cli-kit/node/fs' @@ -76,7 +76,7 @@ export function setCurrentConfigPreference( }, ): asserts configuration is CurrentAppConfiguration { const {configFileName, directory} = options - if (isCurrentAppSchema(configuration) && configuration.client_id) { + if (configuration.client_id) { setCachedAppInfo({ directory, configFile: configFileName, 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 c16c008aa63..8877c2c2bfa 100644 --- a/packages/app/src/cli/services/app/env/pull.test.ts +++ b/packages/app/src/cli/services/app/env/pull.test.ts @@ -1,5 +1,5 @@ import {pullEnv} from './pull.js' -import {AppInterface, AppLinkedInterface} from '../../../models/app/app.js' +import {AppInterface, AppLinkedInterface, AppConfiguration} from '../../../models/app/app.js' import {testApp, testOrganizationApp} from '../../../models/app/app.test-data.js' import {Organization, OrganizationApp, OrganizationSource} from '../../../models/organization.js' import {describe, expect, vi, beforeEach, test} from 'vitest' @@ -108,9 +108,12 @@ function mockApp(currentVersion = '2.2.2'): AppInterface { directory: '/', configuration: { path: joinPath('/', 'shopify.app.toml'), - scopes: 'my-scope', + client_id: 'test-client-id', + access_scopes: { + scopes: 'my-scope', + }, extension_directories: ['extensions/*'], - }, + } as AppConfiguration, nodeDependencies, }) } 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 e434cda300d..444f7c2f8a1 100644 --- a/packages/app/src/cli/services/app/env/show.test.ts +++ b/packages/app/src/cli/services/app/env/show.test.ts @@ -1,6 +1,6 @@ import {showEnv} from './show.js' import {fetchOrganizations} from '../../dev/fetch.js' -import {AppInterface} from '../../../models/app/app.js' +import {AppInterface, AppConfiguration} from '../../../models/app/app.js' import {selectOrganizationPrompt} from '../../../prompts/dev.js' import {testApp, testOrganizationApp} from '../../../models/app/app.test-data.js' import {OrganizationSource} from '../../../models/organization.js' @@ -54,8 +54,11 @@ function mockApp(currentVersion = '2.2.2'): AppInterface { directory: '/', configuration: { path: joinPath('/', 'shopify.app.toml'), - scopes: 'my-scope', - }, + client_id: 'test-client-id', + access_scopes: { + scopes: 'my-scope', + }, + } as AppConfiguration, nodeDependencies, }) } diff --git a/packages/app/src/cli/services/app/write-app-configuration-file.test.ts b/packages/app/src/cli/services/app/write-app-configuration-file.test.ts index 2506dc3cbd8..22217b443dd 100644 --- a/packages/app/src/cli/services/app/write-app-configuration-file.test.ts +++ b/packages/app/src/cli/services/app/write-app-configuration-file.test.ts @@ -67,6 +67,7 @@ include_config_on_deploy = true [access_scopes] # Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes scopes = "read_products" +use_legacy_install_flow = true [auth] redirect_urls = [ diff --git a/packages/app/src/cli/services/context/identifiers-extensions.test.ts b/packages/app/src/cli/services/context/identifiers-extensions.test.ts index cc9a2adb30f..754ae0a7fbb 100644 --- a/packages/app/src/cli/services/context/identifiers-extensions.test.ts +++ b/packages/app/src/cli/services/context/identifiers-extensions.test.ts @@ -9,7 +9,7 @@ import { import {extensionMigrationPrompt, matchConfirmationPrompt} from './prompts.js' import {manualMatchIds} from './id-manual-matching.js' import {EnsureDeploymentIdsPresenceOptions, LocalSource, RemoteSource} from './identifiers.js' -import {AppInterface} from '../../models/app/app.js' +import {AppInterface, AppConfiguration} from '../../models/app/app.js' import { testApp, testAppConfigExtensions, @@ -108,10 +108,13 @@ const LOCAL_APP = ( directory: '/app', configuration: { path: '/shopify.app.toml', - scopes: 'read_products', + client_id: 'test-client-id', + access_scopes: { + scopes: 'read_products', + }, extension_directories: ['extensions/*'], ...(includeDeployConfig ? {build: {include_config_on_deploy: true}} : {}), - }, + } as AppConfiguration, allExtensions: [...uiExtensions, ...functionExtensions, ...configExtensions], }) } @@ -935,7 +938,8 @@ describe('deployConfirmed: handle non existent uuid managed extensions', () => { // When const CONFIG_A = await testAppConfigExtensions() - const ensureExtensionsIdsOptions = options([], [], {configExtensions: [CONFIG_A], developerPlatformClient}) + // Don't pass config extensions when includeConfigOnDeploy is false - they won't be in allExtensions + const ensureExtensionsIdsOptions = options([], [], {developerPlatformClient}) const got = await deployConfirmed(ensureExtensionsIdsOptions, [], [], { extensionsToCreate, validMatches, diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index af0af066266..d687d05301c 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -29,7 +29,7 @@ import {DevSessionStatusManager} from './dev/processes/dev-session/dev-session-s import {TunnelMode} from './dev/tunnel-mode.js' import {PortDetail, renderPortWarnings} from './dev/port-warnings.js' import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' -import {Web, isCurrentAppSchema, getAppScopesArray, AppLinkedInterface} from '../models/app/app.js' +import {Web, getAppScopesArray, AppLinkedInterface} from '../models/app/app.js' import {Organization, OrganizationApp, OrganizationStore} from '../models/organization.js' import {getAnalyticsTunnelType} from '../utilities/analytics.js' import metadata from '../metadata.js' @@ -199,42 +199,40 @@ export async function warnIfScopesDifferBeforeDev({ developerPlatformClient, }: Pick) { if (developerPlatformClient.supportsDevSessions) return - if (isCurrentAppSchema(localApp.configuration)) { - const localAccess = localApp.configuration.access_scopes - const remoteAccess = remoteApp.configuration?.access_scopes - - const rationaliseScopes = (scopeString: string | undefined) => { - if (!scopeString) return scopeString - return scopeString - .split(',') - .map((scope) => scope.trim()) - .sort() - .join(',') - } - const localScopes = rationaliseScopes(localAccess?.scopes) - const remoteScopes = rationaliseScopes(remoteAccess?.scopes) - - if (!localAccess?.use_legacy_install_flow && localScopes !== remoteScopes) { - const nextSteps = [ - [ - 'Run', - {command: formatPackageManagerCommand(localApp.packageManager, 'shopify app deploy')}, - 'to push your scopes to the Partner Dashboard', - ], - ] - - renderWarning({ - headline: [`The scopes in your TOML don't match the scopes in your Partner Dashboard`], - body: [ - `Scopes in ${basename(localApp.configuration.path)}:`, - scopesMessage(getAppScopesArray(localApp.configuration)), - '\n', - 'Scopes in Partner Dashboard:', - scopesMessage(remoteAccess?.scopes?.split(',') ?? []), - ], - nextSteps, - }) - } + const localAccess = localApp.configuration.access_scopes + const remoteAccess = remoteApp.configuration?.access_scopes + + const rationaliseScopes = (scopeString: string | undefined) => { + if (!scopeString) return scopeString + return scopeString + .split(',') + .map((scope) => scope.trim()) + .sort() + .join(',') + } + const localScopes = rationaliseScopes(localAccess?.scopes) + const remoteScopes = rationaliseScopes(remoteAccess?.scopes) + + if (!localAccess?.use_legacy_install_flow && localScopes !== remoteScopes) { + const nextSteps = [ + [ + 'Run', + {command: formatPackageManagerCommand(localApp.packageManager, 'shopify app deploy')}, + 'to push your scopes to the Partner Dashboard', + ], + ] + + renderWarning({ + headline: [`The scopes in your TOML don't match the scopes in your Partner Dashboard`], + body: [ + `Scopes in ${basename(localApp.configuration.path)}:`, + scopesMessage(getAppScopesArray(localApp.configuration)), + '\n', + 'Scopes in Partner Dashboard:', + scopesMessage(remoteAccess?.scopes?.split(',') ?? []), + ], + nextSteps, + }) } } 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 4ec5bc90d6e..0f48c84eeb1 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 @@ -11,7 +11,7 @@ import { } from '../../../models/app/app.test-data.js' import {ExtensionInstance} from '../../../models/extensions/extension-instance.js' import {loadApp, reloadApp} from '../../../models/app/loader.js' -import {AppLinkedInterface} from '../../../models/app/app.js' +import {AppLinkedInterface, AppConfiguration} from '../../../models/app/app.js' import {AppAccessSpecIdentifier} from '../../../models/extensions/specifications/app_config_app_access.js' import {PosSpecIdentifier} from '../../../models/extensions/specifications/app_config_point_of_sale.js' import {afterEach, beforeEach, describe, expect, test, vi} from 'vitest' @@ -275,7 +275,12 @@ describe('app-event-watcher', () => { // When const app = testAppLinked({ allExtensions: initialExtensions, - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: { + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + extension_directories: [], + path: 'shopify.app.custom.toml', + } as AppConfiguration, }) const mockManager = new MockESBuildContextManager() @@ -358,7 +363,12 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [extension1], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: { + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + extension_directories: [], + path: 'shopify.app.custom.toml', + } as AppConfiguration, }) const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent]) @@ -401,7 +411,12 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [flowExtension], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: { + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + extension_directories: [], + path: 'shopify.app.custom.toml', + } as AppConfiguration, }) // When @@ -435,7 +450,12 @@ describe('app-event-watcher', () => { const buildOutputPath = joinPath(tmpDir, '.shopify', 'bundle') const app = testAppLinked({ allExtensions: [extension1], - configuration: {scopes: '', extension_directories: [], path: 'shopify.app.custom.toml'}, + configuration: { + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + extension_directories: [], + path: 'shopify.app.custom.toml', + } as AppConfiguration, }) const mockFileWatcher = new MockFileWatcher(app, outputOptions, [fileWatchEvent]) diff --git a/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts b/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts index 5704a97ed6a..01d15deb10c 100644 --- a/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts +++ b/packages/app/src/cli/services/dev/app-events/file-watcher.test.ts @@ -7,6 +7,7 @@ import { testFunctionExtension, testUIExtension, } from '../../../models/app/app.test-data.js' +import {AppConfiguration} from '../../../models/app/app.js' import {flushPromises} from '@shopify/cli-kit/node/promises' import {describe, expect, test, vi} from 'vitest' import chokidar from 'chokidar' @@ -232,7 +233,11 @@ describe('file-watcher events', () => { const app = testAppLinked({ allExtensions: [ext1, ext2, posExtension], directory: dir, - configuration: {path: joinPath(dir, '/shopify.app.toml'), scopes: ''}, + configuration: { + path: joinPath(dir, '/shopify.app.toml'), + client_id: 'test-client-id', + access_scopes: {scopes: ''}, + } as AppConfiguration, }) // Add a custom gitignore file to the extension diff --git a/packages/app/src/cli/services/dev/select-app.test.ts b/packages/app/src/cli/services/dev/select-app.test.ts index d0ce4811d3b..0e72c297e9d 100644 --- a/packages/app/src/cli/services/dev/select-app.test.ts +++ b/packages/app/src/cli/services/dev/select-app.test.ts @@ -1,5 +1,5 @@ import {selectOrCreateApp} from './select-app.js' -import {AppInterface, WebType} from '../../models/app/app.js' +import {AppInterface, AppConfiguration, WebType} from '../../models/app/app.js' import {Organization, OrganizationSource} from '../../models/organization.js' import {appNamePrompt, createAsNewAppPrompt, selectAppPrompt} from '../../prompts/dev.js' import {testApp, testOrganizationApp, testDeveloperPlatformClient} from '../../models/app/app.test-data.js' @@ -9,7 +9,12 @@ vi.mock('../../prompts/dev') const LOCAL_APP: AppInterface = testApp({ directory: '', - configuration: {path: '/shopify.app.toml', scopes: 'read_products', extension_directories: ['extensions/*']}, + configuration: { + path: '/shopify.app.toml', + client_id: 'test-client-id', + access_scopes: {scopes: 'read_products'}, + extension_directories: ['extensions/*'], + } as AppConfiguration, webs: [ { directory: '', diff --git a/packages/app/src/cli/services/generate/extension.test.ts b/packages/app/src/cli/services/generate/extension.test.ts index 38becb99547..15cfa1e2e67 100644 --- a/packages/app/src/cli/services/generate/extension.test.ts +++ b/packages/app/src/cli/services/generate/extension.test.ts @@ -116,7 +116,8 @@ describe('initialize a extension', async () => { expect(vi.mocked(addNPMDependenciesIfNeeded)).toHaveBeenCalledTimes(2) const loadedApp = await loadApp({directory: tmpDir, specifications, userProvidedConfigName: undefined}) - expect(loadedApp.allExtensions.length).toEqual(2) + const realExtensions = loadedApp.allExtensions.filter((ext) => ext.specification.experience !== 'configuration') + expect(realExtensions.length).toEqual(2) }) }) @@ -580,7 +581,18 @@ async function withTemporaryApp( const webConfigurationPath = joinPath(tmpDir, blocks.web.directoryName, blocks.web.configurationName) const appConfiguration = ` name = "my_app" + client_id = "test-client-id" + application_url = "https://example.com" + embedded = true + + [access_scopes] scopes = "read_products" + + [auth] + redirect_urls = ["https://example.com/callback"] + + [webhooks] + api_version = "2024-01" ` const webConfiguration = ` type = "backend" diff --git a/packages/app/src/cli/services/info.test.ts b/packages/app/src/cli/services/info.test.ts index fee68b37094..223bd138e77 100644 --- a/packages/app/src/cli/services/info.test.ts +++ b/packages/app/src/cli/services/info.test.ts @@ -1,5 +1,5 @@ import {InfoOptions, info} from './info.js' -import {AppInterface, AppLinkedInterface} from '../models/app/app.js' +import {AppInterface, AppLinkedInterface, AppConfiguration} from '../models/app/app.js' import {OrganizationApp, OrganizationSource} from '../models/organization.js' import {selectOrganizationPrompt} from '../prompts/dev.js' import { @@ -274,9 +274,12 @@ function mockApp({ directory, configuration: { path: joinPath(directory, 'shopify.app.toml'), - scopes: 'my-scope', + client_id: 'test-client-id', + access_scopes: { + scopes: 'my-scope', + }, extension_directories: ['extensions/*'], - }, + } as AppConfiguration, nodeDependencies, ...(app ? app : {}), }) diff --git a/packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts b/packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts index b75607313a4..107932cc45e 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/partners-client.test.ts @@ -1,11 +1,11 @@ import {PartnersClient} from './partners-client.js' import {CreateAppQuery} from '../../api/graphql/create_app.js' -import {AppInterface, WebType} from '../../models/app/app.js' +import {AppInterface, AppConfiguration, WebType} from '../../models/app/app.js' import {Organization, OrganizationSource, OrganizationStore} from '../../models/organization.js' import { testPartnersUserSession, testApp, - testAppWithLegacyConfig, + testAppWithConfig, testOrganizationApp, } from '../../models/app/app.test-data.js' import {appNamePrompt} from '../../prompts/dev.js' @@ -23,7 +23,12 @@ beforeEach(() => { const LOCAL_APP: AppInterface = testApp({ directory: '', - configuration: {path: '/shopify.app.toml', scopes: 'read_products', extension_directories: ['extensions/*']}, + configuration: { + path: '/shopify.app.toml', + client_id: 'test-client-id', + access_scopes: {scopes: 'read_products'}, + extension_directories: ['extensions/*'], + } as AppConfiguration, webs: [ { directory: '', @@ -82,7 +87,7 @@ describe('createApp', () => { test('sends request to create app with launchable defaults and returns it', async () => { // Given const partnersClient = PartnersClient.getInstance(testPartnersUserSession) - const localApp = testAppWithLegacyConfig({config: {scopes: 'write_products'}}) + const localApp = testAppWithConfig({config: {access_scopes: {scopes: 'write_products'}}}) vi.mocked(appNamePrompt).mockResolvedValue('app-name') vi.mocked(partnersRequest).mockResolvedValueOnce({appCreate: {app: APP1, userErrors: []}}) const variables = { diff --git a/packages/app/src/cli/utilities/types.ts b/packages/app/src/cli/utilities/types.ts deleted file mode 100644 index 37fd9ac655f..00000000000 --- a/packages/app/src/cli/utilities/types.ts +++ /dev/null @@ -1,5 +0,0 @@ -import {zod} from '@shopify/cli-kit/node/schema' - -export function isType(schema: T, item: unknown): item is zod.infer { - return schema.safeParse(item).success -}