diff --git a/packages/app/src/cli/models/extensions/specification.ts b/packages/app/src/cli/models/extensions/specification.ts index d9e9256d642..c6dd0a61ada 100644 --- a/packages/app/src/cli/models/extensions/specification.ts +++ b/packages/app/src/cli/models/extensions/specification.ts @@ -26,9 +26,19 @@ export type ExtensionFeature = export type TransformationConfig = Record +/** + * Options for converting platform content to the local format. + * + * `handle` is the module handle. Some modules keep it outside their config, so the config alone can't restore it. + */ +export interface RemoteToLocalTransformOptions { + flags?: Flag[] + handle?: string +} + export interface CustomTransformationConfig { forward?: (obj: object, appConfiguration: AppConfiguration, options?: {flags?: Flag[]}) => object - reverse?: (obj: object, options?: {flags?: Flag[]}) => object + reverse?: (obj: object, options?: RemoteToLocalTransformOptions) => object } type ExtensionExperience = 'extension' | 'configuration' @@ -119,7 +129,7 @@ export interface ExtensionSpecification object + transformRemoteToLocal?: (remoteContent: object, options?: RemoteToLocalTransformOptions) => object uidStrategy: UidStrategy diff --git a/packages/app/src/cli/models/extensions/specifications/app_config_events.ts b/packages/app/src/cli/models/extensions/specifications/app_config_events.ts index 227c2327511..2ab248ae658 100644 --- a/packages/app/src/cli/models/extensions/specifications/app_config_events.ts +++ b/packages/app/src/cli/models/extensions/specifications/app_config_events.ts @@ -7,7 +7,7 @@ export const EventsSpecIdentifier = 'events' const EventsTransformConfig: CustomTransformationConfig = { forward: transformFromEventsConfig, - reverse: (content: object) => transformToEventsConfig(content), + reverse: transformToEventsConfig, } const EventsSchema = BaseSchemaWithoutHandle.extend({ diff --git a/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts b/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts index a744bcd8be5..579eecb72ed 100644 --- a/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.test.ts @@ -1,4 +1,5 @@ import {transformToEventsConfig, transformFromEventsConfig} from './app_config_events.js' +import {deepMergeObjects} from '@shopify/cli-kit/common/object' import {describe, expect, test} from 'vitest' describe('transformFromEventsConfig', () => { @@ -115,6 +116,25 @@ describe('transformFromEventsConfig', () => { expect(result).toEqual(content) }) + test('prepends application_url to a relative URI in a single subscription object', () => { + const content = { + events: { + api_version: '2024-01', + subscription: {topic: 'orders', uri: '/webhooks/orders', actions: ['create']}, + }, + } + const appConfiguration = {application_url: 'https://tunnel.example.com'} + + const result = transformFromEventsConfig(content, appConfiguration) + + expect(result).toEqual({ + events: { + api_version: '2024-01', + subscription: {topic: 'orders', uri: 'https://tunnel.example.com/webhooks/orders', actions: ['create']}, + }, + }) + }) + test('returns content as-is when events is undefined', () => { const content = {} const appConfiguration = {application_url: 'https://tunnel.example.com'} @@ -123,6 +143,35 @@ describe('transformFromEventsConfig', () => { expect(result).toEqual(content) }) + + test('leaves subscriptions without a string uri untouched while resolving the others', () => { + const content = { + events: { + api_version: '2024-01', + subscription: [ + {topic: 'orders', actions: ['create']}, + {topic: 'orders', uri: null, actions: ['paid']}, + {topic: 'products', uri: 123, actions: ['create']}, + {topic: 'products', uri: '/webhooks/products', actions: ['update']}, + ], + }, + } + const appConfiguration = {application_url: 'https://tunnel.example.com'} + + const result = transformFromEventsConfig(content, appConfiguration) + + expect(result).toEqual({ + events: { + api_version: '2024-01', + subscription: [ + {topic: 'orders', actions: ['create']}, + {topic: 'orders', uri: null, actions: ['paid']}, + {topic: 'products', uri: 123, actions: ['create']}, + {topic: 'products', uri: 'https://tunnel.example.com/webhooks/products', actions: ['update']}, + ], + }, + }) + }) }) describe('transformToEventsConfig', () => { @@ -209,10 +258,293 @@ describe('transformToEventsConfig', () => { const result = transformToEventsConfig(remoteContent) + expect(result).toStrictEqual({ + events: { + api_version: '2024-01', + }, + }) + }) + + test('handles a null subscription field', () => { + const remoteContent = { + events: { + api_version: '2024-01', + subscription: null, + }, + } + + const result = transformToEventsConfig(remoteContent) + + expect(result).toStrictEqual({ + events: { + api_version: '2024-01', + }, + }) + }) + test('normalizes a single subscription, removes derived fields, and preserves its handle', () => { + const remoteContent = { + events: { + api_version: '2024-01', + subscription: { + topic: 'orders', + uri: 'https://example.com/webhook', + actions: ['create'], + handle: 'order-notifier', + api_version: '2024-01', + identifier: 'id-1', + }, + }, + } + + const result = transformToEventsConfig(remoteContent) + + expect(result).toEqual({ + events: { + api_version: '2024-01', + subscription: [ + { + topic: 'orders', + uri: 'https://example.com/webhook', + actions: ['create'], + handle: 'order-notifier', + }, + ], + }, + }) + }) + + test('strips a subscription api_version that matches the events default in the list shape', () => { + const remoteContent = { + events: { + api_version: '2024-01', + subscription: [ + { + topic: 'orders', + uri: 'https://example.com/a', + actions: ['create'], + api_version: '2024-01', + identifier: 'id-a', + }, + ], + }, + } + + const result = transformToEventsConfig(remoteContent) + expect(result).toEqual({ events: { api_version: '2024-01', - subscription: undefined, + subscription: [{topic: 'orders', uri: 'https://example.com/a', actions: ['create']}], + }, + }) + }) + + test('keeps a subscription api_version that overrides the events default', () => { + const remoteContent = { + events: { + api_version: '2024-01', + subscription: { + topic: 'orders', + uri: 'https://example.com/a', + actions: ['create'], + api_version: '2025-07', + identifier: 'id-a', + }, + }, + } + + const result = transformToEventsConfig(remoteContent) + + expect(result).toEqual({ + events: { + api_version: '2024-01', + subscription: [ + { + topic: 'orders', + uri: 'https://example.com/a', + actions: ['create'], + api_version: '2025-07', + handle: 'orders-create', + }, + ], + }, + }) + }) + + test('keeps a subscription api_version when the events default is absent', () => { + const remoteContent = { + events: { + subscription: [ + { + topic: 'orders', + uri: 'https://example.com/a', + actions: ['create'], + api_version: '2024-01', + identifier: 'id-a', + }, + ], + }, + } + + const result = transformToEventsConfig(remoteContent) + + expect(result).toStrictEqual({ + events: { + subscription: [{topic: 'orders', uri: 'https://example.com/a', actions: ['create'], api_version: '2024-01'}], + }, + }) + }) + + test('derives a handle from the topic and actions of a single subscription', () => { + const remoteContent = { + events: { + api_version: '2024-01', + subscription: { + topic: 'orders', + uri: 'https://example.com/webhook', + actions: ['create'], + identifier: 'id-1', + }, + }, + } + + const result = transformToEventsConfig(remoteContent) + + expect(result).toEqual({ + events: { + api_version: '2024-01', + subscription: [ + { + topic: 'orders', + uri: 'https://example.com/webhook', + actions: ['create'], + handle: 'orders-create', + }, + ], + }, + }) + }) + + test('uses the module handle for a single subscription without one', () => { + const remoteContent = {events: {subscription: {topic: 'orders', actions: ['create']}}} + + const result = transformToEventsConfig(remoteContent, {handle: 'order-notifier'}) + + expect(result).toEqual({ + events: {subscription: [{topic: 'orders', actions: ['create'], handle: 'order-notifier'}]}, + }) + }) + + test('prefers the module handle over a legacy subscription handle', () => { + const remoteContent = {events: {subscription: {topic: 'orders', actions: ['create'], handle: 'from-config'}}} + + const result = transformToEventsConfig(remoteContent, {handle: 'from-module'}) + + expect(result).toEqual({ + events: {subscription: [{topic: 'orders', actions: ['create'], handle: 'from-module'}]}, + }) + }) + + test('falls back to the subscription handle when no module handle is given', () => { + const remoteContent = {events: {subscription: {topic: 'orders', actions: ['create'], handle: 'from-config'}}} + + const result = transformToEventsConfig(remoteContent) + + expect(result).toEqual({ + events: {subscription: [{topic: 'orders', actions: ['create'], handle: 'from-config'}]}, + }) + }) + + test('ignores the module handle for subscriptions in an array', () => { + const remoteContent = {events: {subscription: [{topic: 'orders', actions: ['create'], handle: 'order-notifier'}]}} + + const result = transformToEventsConfig(remoteContent, {handle: 'events'}) + + expect(result).toEqual({ + events: {subscription: [{topic: 'orders', actions: ['create'], handle: 'order-notifier'}]}, + }) + }) + + test.each([49, 50])('limits the generated handle for a topic with %i characters', (topicLength) => { + const topic = 'a'.repeat(topicLength) + + const result = transformToEventsConfig({events: {subscription: {topic, actions: ['create']}}}) + + expect(result).toEqual({events: {subscription: [{topic, actions: ['create'], handle: topic}]}}) + }) + + test('includes multiple actions in the generated handle', () => { + const result = transformToEventsConfig({events: {subscription: {topic: 'orders', actions: ['create', 'paid']}}}) + + expect(result).toEqual({ + events: {subscription: [{topic: 'orders', actions: ['create', 'paid'], handle: 'orders-create-paid'}]}, + }) + }) + + test('merging single-subscription modules keeps the shared events default and only the overriding api_version', () => { + // Modules deployed from one configuration share events.api_version, and the platform + // materializes it onto every subscription that does not override it. + const productChanges = { + handle: 'rocky-product-77', + config: { + events: { + api_version: '2026-10', + subscription: { + identifier: 'id-77', + topic: 'Product', + actions: ['create', 'update', 'delete'], + uri: 'https://example.com/a', + api_version: '2026-10', + }, + }, + }, + } + const productUpdates = { + handle: 'rocky-product-8', + config: { + events: { + api_version: '2026-10', + subscription: { + identifier: 'id-8', + topic: 'Product', + actions: ['create', 'update'], + uri: 'https://example.com/b', + api_version: 'unstable', + }, + }, + }, + } + const link = (modules: {handle: string; config: object}[]) => + modules + .map(({handle, config}) => transformToEventsConfig(config, {handle})) + .reduce((merged, local) => deepMergeObjects(merged, local), {}) + + const expectedSubscriptions = { + 'rocky-product-77': { + topic: 'Product', + actions: ['create', 'update', 'delete'], + uri: 'https://example.com/a', + handle: 'rocky-product-77', + }, + 'rocky-product-8': { + topic: 'Product', + actions: ['create', 'update'], + uri: 'https://example.com/b', + handle: 'rocky-product-8', + api_version: 'unstable', + }, + } + + expect(link([productChanges, productUpdates])).toEqual({ + events: { + api_version: '2026-10', + subscription: [expectedSubscriptions['rocky-product-77'], expectedSubscriptions['rocky-product-8']], + }, + }) + expect(link([productUpdates, productChanges])).toEqual({ + events: { + api_version: '2026-10', + subscription: [expectedSubscriptions['rocky-product-8'], expectedSubscriptions['rocky-product-77']], }, }) }) diff --git a/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.ts b/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.ts index b811be56baa..3db6f96b414 100644 --- a/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.ts +++ b/packages/app/src/cli/models/extensions/specifications/transform/app_config_events.ts @@ -1,11 +1,21 @@ import {prependApplicationUrl} from '../validation/url_prepender.js' +import {MAX_EXTENSION_HANDLE_LENGTH} from '../../schemas.js' import {CurrentAppConfiguration} from '../../../app/app.js' +import {RemoteToLocalTransformOptions} from '../../specification.js' import {getPathValue} from '@shopify/cli-kit/common/object' +import {slugify} from '@shopify/cli-kit/common/string' + +interface EventSubscription { + // The events schema is untyped locally, so a subscription may be missing its uri + // or carry a non-string value. Such subscriptions are left for the server to reject. + uri?: unknown + [key: string]: unknown +} interface EventsConfig { events?: { api_version?: string - subscription?: {uri: string; [key: string]: unknown}[] + subscription?: EventSubscription | EventSubscription[] } } @@ -27,37 +37,78 @@ export function transformFromEventsConfig(content: object, appConfiguration?: ob appUrl = (appConfiguration as CurrentAppConfiguration)?.application_url } + const subscription = eventsConfig.events.subscription + const resolved = wrapSubscriptions(subscription).map((sub) => + typeof sub.uri === 'string' ? {...sub, uri: prependApplicationUrl(sub.uri, appUrl)} : sub, + ) + return { ...eventsConfig, events: { ...eventsConfig.events, - subscription: eventsConfig.events.subscription.map((sub) => ({ - ...sub, - uri: prependApplicationUrl(sub.uri, appUrl), - })), + subscription: Array.isArray(subscription) ? resolved : resolved[0], }, } } +interface RemoteEventsModule { + api_version?: string + subscription?: RemoteEventSubscription | RemoteEventSubscription[] | null +} + +interface RemoteEventSubscription { + identifier?: string + handle?: string + api_version?: string + topic: string + actions: string[] + [key: string]: unknown +} + /** - * Transforms the events config from remote to local format. - * Strips the server-managed 'identifier' field from subscriptions, and the - * subscription 'api_version' when it matches the events default. + * Transforms one events module from remote to local format. + * Strips the server-managed 'identifier' field, and the per-subscription + * 'api_version' when it matches the module default. Single-subscription + * objects are normalized to a one-element array. The platform keeps their handle + * on the module, not in the subscription, so the module handle is restored when given. + * Without one, the handle is derived from the topic and actions. */ -export function transformToEventsConfig(content: object) { - const eventsConfig = getPathValue(content, 'events') as {api_version: string; subscription: object[]} - const apiVersion = getPathValue(eventsConfig, 'api_version') as string - const subscription = getPathValue(eventsConfig, 'subscription') as {identifier: string; api_version?: string}[] +export function transformToEventsConfig(content: object, options?: RemoteToLocalTransformOptions) { + const {api_version: apiVersion, subscription} = getPathValue(content, 'events') ?? {} + + const clean = (sub: RemoteEventSubscription) => { + const {identifier: _, api_version: subApiVersion, ...rest} = sub + const overridesDefault = subApiVersion !== undefined && subApiVersion !== apiVersion + return overridesDefault ? {...rest, api_version: subApiVersion} : rest + } - // Server adds identifier and fills [events].api_version into subscriptions that omit it - const cleanedSubscriptions = subscription?.map((sub) => { - const {identifier, api_version: subscriptionApiVersion, ...rest} = sub - const overridesDefault = subscriptionApiVersion !== undefined && subscriptionApiVersion !== apiVersion - return overridesDefault ? {...rest, api_version: subscriptionApiVersion} : rest - }) + let cleanedSubscriptions: object[] | undefined + if (Array.isArray(subscription)) { + cleanedSubscriptions = subscription.map(clean) + } else if (subscription) { + // The platform treats the module handle as the subscription's identity: the runtime, the uid + // and the identifier are all derived from it, and a nested handle is rejected on write. A + // nested handle only survives on older versions, so it must not win over the module handle. + const handle = options?.handle ?? subscription.handle ?? handleFromSubscriptionData(subscription) + cleanedSubscriptions = [clean({...subscription, handle})] + } - const events = - (apiVersion ?? cleanedSubscriptions) ? {api_version: apiVersion, subscription: cleanedSubscriptions} : {} + const events: {api_version?: string; subscription?: object[]} = {} + if (apiVersion !== undefined) { + events.api_version = apiVersion + } + if (cleanedSubscriptions !== undefined) { + events.subscription = cleanedSubscriptions + } return {events} } + +function handleFromSubscriptionData(subscription: RemoteEventSubscription): string { + const handle = slugify([subscription.topic, ...subscription.actions].join('-')) + return handle.slice(0, MAX_EXTENSION_HANDLE_LENGTH).replace(/-$/, '') +} + +function wrapSubscriptions(subscription: T | T[]): T[] { + return Array.isArray(subscription) ? subscription : [subscription] +} diff --git a/packages/app/src/cli/services/app/select-app.test.ts b/packages/app/src/cli/services/app/select-app.test.ts index 5e2805d8a1a..5657c0d128d 100644 --- a/packages/app/src/cli/services/app/select-app.test.ts +++ b/packages/app/src/cli/services/app/select-app.test.ts @@ -1,4 +1,4 @@ -import {fetchAppRemoteConfiguration} from './select-app.js' +import {fetchAppRemoteConfiguration, remoteAppConfigurationExtensionContent} from './select-app.js' import {configurationSpecifications, testDeveloperPlatformClient} from '../../models/app/app.test-data.js' import {AppModuleVersion, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {MinimalAppIdentifiers, MinimalOrganizationApp} from '../../models/organization.js' @@ -164,3 +164,40 @@ describe('fetchAppRemoteConfiguration', () => { expect(result).toBeUndefined() }) }) + +describe('remoteAppConfigurationExtensionContent', () => { + test('restores single-subscription events handles from their modules', async () => { + // Given + const eventsModule = (handle: string, topic: string): AppModuleVersion => ({ + registrationId: handle, + registrationUuid: `UUID_${handle}`, + registrationTitle: handle, + type: 'events', + config: {events: {api_version: '2024-01', subscription: {topic, actions: ['create'], uri: 'https://myapp.com'}}}, + specification: { + identifier: 'events', + name: 'Events', + experience: 'configuration', + options: {managementExperience: 'cli'}, + }, + }) + + // When + const result = remoteAppConfigurationExtensionContent( + [eventsModule('order-notifier', 'orders/create'), eventsModule('product-sync', 'products/create')], + await configurationSpecifications(), + [], + ) + + // Then + expect(result).toEqual({ + events: { + api_version: '2024-01', + subscription: [ + {topic: 'orders/create', actions: ['create'], uri: 'https://myapp.com', handle: 'order-notifier'}, + {topic: 'products/create', actions: ['create'], uri: 'https://myapp.com', handle: 'product-sync'}, + ], + }, + }) + }) +}) diff --git a/packages/app/src/cli/services/app/select-app.ts b/packages/app/src/cli/services/app/select-app.ts index 915e88933b3..d3918664892 100644 --- a/packages/app/src/cli/services/app/select-app.ts +++ b/packages/app/src/cli/services/app/select-app.ts @@ -68,7 +68,8 @@ export function remoteAppConfigurationExtensionContent( const config = module.config if (!config) return - remoteAppConfig = deepMergeObjects(remoteAppConfig, configSpec.transformRemoteToLocal?.(config, {flags}) ?? config) + const localConfig = configSpec.transformRemoteToLocal?.(config, {flags, handle: module.registrationTitle}) ?? config + remoteAppConfig = deepMergeObjects(remoteAppConfig, localConfig) }) return {...remoteAppConfig} diff --git a/packages/app/src/cli/services/context/deploy-identifier-matching.test.ts b/packages/app/src/cli/services/context/deploy-identifier-matching.test.ts index 64b951d285e..85f358605d2 100644 --- a/packages/app/src/cli/services/context/deploy-identifier-matching.test.ts +++ b/packages/app/src/cli/services/context/deploy-identifier-matching.test.ts @@ -504,6 +504,58 @@ describe('classifyDeployExtensionChanges', () => { ) }) + test('does not mark events as updated when single-subscription modules match the remote ones', async () => { + const localEvents = ['order-notifier', 'product-sync'].map( + (handle) => + new ExtensionInstance({ + specification: appEventsSpec, + configuration: { + events: { + api_version: '2024-01', + subscription: {handle, topic: `${handle}/create`, actions: ['create'], uri: 'https://example.com'}, + }, + } as BaseConfigType, + configurationPath: '/app/shopify.app.toml', + directory: '/app', + }), + ) + const remoteEvents = await Promise.all( + localEvents.map( + async (extension): Promise => ({ + registrationId: extension.uid, + registrationUuid: `${extension.uid}-uuid`, + registrationTitle: extension.handle, + type: 'events', + config: await extension.deployConfig({apiKey: REMOTE_APP.apiKey, appConfiguration: APP.configuration}), + specification: { + identifier: 'events', + name: 'Events', + experience: 'configuration', + options: {managementExperience: 'cli'}, + }, + }), + ), + ) + + await ensureDeployIdentifiersFromAppVersion( + deployOptions({ + app: testApp({...APP, allExtensions: localEvents, specifications: [appEventsSpec]}), + activeAppVersion: {appModuleVersions: remoteEvents}, + }), + ) + + expect(deployOrReleaseConfirmationPrompt).toHaveBeenCalledWith( + expect.objectContaining({ + configExtensionIdentifiersBreakdown: { + existingFieldNames: ['events'], + existingUpdatedFieldNames: [], + newFieldNames: [], + deletedFieldNames: [], + }, + }), + ) + }) + test('relinks a created local to an un-migrated remote that shares its handle and type', async () => { const pendingRemote = { registrationId: '', diff --git a/packages/app/src/cli/services/context/deploy-identifier-matching.ts b/packages/app/src/cli/services/context/deploy-identifier-matching.ts index 04236f1173d..f0323a68f35 100644 --- a/packages/app/src/cli/services/context/deploy-identifier-matching.ts +++ b/packages/app/src/cli/services/context/deploy-identifier-matching.ts @@ -189,8 +189,10 @@ async function localAppConfigurationExtensionContent(app: AppInterface, apiKey: // eslint-disable-next-line no-await-in-loop const deployConfig = await extension.deployConfig({apiKey, appConfiguration: app.configuration}) const localConfig = - extension.specification.transformRemoteToLocal?.(deployConfig ?? {}, {flags: app.remoteFlags}) ?? - extension.configuration + extension.specification.transformRemoteToLocal?.(deployConfig ?? {}, { + flags: app.remoteFlags, + handle: extension.handle, + }) ?? extension.configuration appConfig = deepMergeObjects(appConfig, localConfig) }