diff --git a/.vscode/launch.json b/.vscode/launch.json index 3d37bd0373..85b80a1e21 100644 --- a/.vscode/launch.json +++ b/.vscode/launch.json @@ -143,7 +143,7 @@ "fulfillment_constraints", "local_pickup_delivery_option_generator", "pickup_point_delivery_option_generator", - "marketing_activity_extension_cli" + "marketing_activity" ], "default": "checkout_ui_extension", }, diff --git a/packages/app/src/cli/models/extensions/load-specifications.ts b/packages/app/src/cli/models/extensions/load-specifications.ts index 755f26653e..8d662eb66e 100644 --- a/packages/app/src/cli/models/extensions/load-specifications.ts +++ b/packages/app/src/cli/models/extensions/load-specifications.ts @@ -21,7 +21,7 @@ import paymentExtensionSpec from './specifications/payments_app_extension.js' import posUISpec from './specifications/pos_ui_extension.js' import productSubscriptionSpec from './specifications/product_subscription.js' import taxCalculationSpec from './specifications/tax_calculation.js' -import marketingActivityExtensionSpec from './specifications/marketing_activity_extension.js' +import marketingActivitySpec from './specifications/marketing_activity.js' import themeSpec from './specifications/theme.js' import uiExtensionSpec from './specifications/ui_extension.js' import webPixelSpec from './specifications/web_pixel_extension.js' @@ -73,7 +73,7 @@ function loadSpecifications() { posUISpec, productSubscriptionSpec, taxCalculationSpec, - marketingActivityExtensionSpec, + marketingActivitySpec, themeSpec, uiExtensionSpec, webPixelSpec, diff --git a/packages/app/src/cli/models/extensions/specifications/marketing_activity_extension.ts b/packages/app/src/cli/models/extensions/specifications/marketing_activity.ts similarity index 59% rename from packages/app/src/cli/models/extensions/specifications/marketing_activity_extension.ts rename to packages/app/src/cli/models/extensions/specifications/marketing_activity.ts index 368d50c99b..7e0becd9e9 100644 --- a/packages/app/src/cli/models/extensions/specifications/marketing_activity_extension.ts +++ b/packages/app/src/cli/models/extensions/specifications/marketing_activity.ts @@ -1,22 +1,28 @@ -import {MarketingActivityExtensionSchema} from './marketing_activity_extension_schemas/marketing_activity_extension_schema.js' +import {MarketingActivityExtensionSchema} from './marketing_activity_schemas/marketing_activity_schema.js' import {createExtensionSpecification} from '../specification.js' +import {randomUUID} from '@shopify/cli-kit/node/crypto' const spec = createExtensionSpecification({ - identifier: 'marketing_activity_extension_cli', + identifier: 'marketing_activity', schema: MarketingActivityExtensionSchema, appModuleFeatures: (_) => ['bundling'], deployConfig: async (config, _) => { return { title: config.title, description: config.description, - app_api_url: config.app_api_url, + api_path: config.api_path, tactic: config.tactic, marketing_channel: config.marketing_channel, referring_domain: config.referring_domain, is_automation: config.is_automation, use_external_editor: config.use_external_editor, preview_data: config.preview_data, - fields: config.fields, + fields: config.fields.map((field) => ({ + ...field, + // NOTE: we're not using this id anywhere, generating it to satisfy the schema + // decided not to remove it from the schema for now to minimize the risk of breaking changes + id: randomUUID(), + })), } }, }) diff --git a/packages/app/src/cli/models/extensions/specifications/marketing_activity_extension_schemas/marketing_activity_extension_schema.test.ts b/packages/app/src/cli/models/extensions/specifications/marketing_activity_schemas/marketing_activity_schema.test.ts similarity index 98% rename from packages/app/src/cli/models/extensions/specifications/marketing_activity_extension_schemas/marketing_activity_extension_schema.test.ts rename to packages/app/src/cli/models/extensions/specifications/marketing_activity_schemas/marketing_activity_schema.test.ts index fde649a8ad..cddd66aaf3 100644 --- a/packages/app/src/cli/models/extensions/specifications/marketing_activity_extension_schemas/marketing_activity_extension_schema.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/marketing_activity_schemas/marketing_activity_schema.test.ts @@ -1,4 +1,4 @@ -import {MarketingActivityExtensionSchema} from './marketing_activity_extension_schema.js' +import {MarketingActivityExtensionSchema} from './marketing_activity_schema.js' import {describe, expect, test} from 'vitest' import {zod} from '@shopify/cli-kit/node/schema' @@ -8,7 +8,7 @@ describe('MarketingActivityExtensionSchema', () => { type: 'marketing_activity_extension', title: 'test extension 123', description: 'test description 123', - app_api_url: 'http://foo.bar/api', + api_path: '/api', tactic: 'ad', marketing_channel: 'social', referring_domain: 'http://foo.bar', diff --git a/packages/app/src/cli/models/extensions/specifications/marketing_activity_extension_schemas/marketing_activity_extension_schema.ts b/packages/app/src/cli/models/extensions/specifications/marketing_activity_schemas/marketing_activity_schema.ts similarity index 99% rename from packages/app/src/cli/models/extensions/specifications/marketing_activity_extension_schemas/marketing_activity_extension_schema.ts rename to packages/app/src/cli/models/extensions/specifications/marketing_activity_schemas/marketing_activity_schema.ts index 3eea7e9543..338d911630 100644 --- a/packages/app/src/cli/models/extensions/specifications/marketing_activity_extension_schemas/marketing_activity_extension_schema.ts +++ b/packages/app/src/cli/models/extensions/specifications/marketing_activity_schemas/marketing_activity_schema.ts @@ -2,7 +2,6 @@ import {BaseSchema} from '../../schemas.js' import {zod} from '@shopify/cli-kit/node/schema' const BaseFieldSchema = zod.object({ - id: zod.string(), ui_type: zod.string(), }) @@ -122,7 +121,7 @@ const UISchemaMapping: {[key: string]: zod.Schema} = { export const MarketingActivityExtensionSchema = BaseSchema.extend({ title: zod.string().min(1), description: zod.string().min(1), - app_api_url: zod.string(), + api_path: zod.string(), tactic: zod.enum([ 'ad', 'retargeting', diff --git a/packages/app/src/cli/services/context/identifiers-extensions.ts b/packages/app/src/cli/services/context/identifiers-extensions.ts index 244df44b2e..b4f8fe1e54 100644 --- a/packages/app/src/cli/services/context/identifiers-extensions.ts +++ b/packages/app/src/cli/services/context/identifiers-extensions.ts @@ -6,6 +6,7 @@ import {createExtension} from '../dev/create-extension.js' import {IdentifiersExtensions} from '../../models/app/identifiers.js' import {getUIExtensionsToMigrate, migrateExtensionsToUIExtension} from '../dev/migrate-to-ui-extension.js' import {getFlowExtensionsToMigrate, migrateFlowExtensions} from '../dev/migrate-flow-extension.js' +import {getMarketingActivtyExtensionsToMigrate} from '../dev/migrate-marketing-activity-extension.js' import {AppInterface} from '../../models/app/app.js' import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {getPaymentsExtensionsToMigrate, migrateAppModules} from '../dev/migrate-app-module.js' @@ -15,6 +16,7 @@ import {SingleWebhookSubscriptionType} from '../../models/extensions/specificati import {outputCompleted} from '@shopify/cli-kit/node/output' import {AbortSilentError} from '@shopify/cli-kit/node/error' import {groupBy} from '@shopify/cli-kit/common/collection' +import {isShopify} from '@shopify/cli-kit/node/context/local' interface AppWithExtensions { extensionRegistrations: RemoteSource[] @@ -28,12 +30,16 @@ export async function ensureExtensionsIds( dashboardManagedExtensionRegistrations: dashboardOnlyExtensions, }: AppWithExtensions, ) { + const isShopifolk = await isShopify() let remoteExtensions = initialRemoteExtensions const validIdentifiers = options.envIdentifiers.extensions ?? {} const localExtensions = options.app.allExtensions.filter((ext) => ext.isUUIDStrategyExtension) const uiExtensionsToMigrate = getUIExtensionsToMigrate(localExtensions, remoteExtensions, validIdentifiers) const flowExtensionsToMigrate = getFlowExtensionsToMigrate(localExtensions, dashboardOnlyExtensions, validIdentifiers) + const marketingActivityExtensionsToMigrate = isShopifolk + ? getMarketingActivtyExtensionsToMigrate(localExtensions, dashboardOnlyExtensions, validIdentifiers) + : [] const paymentsExtensionsToMigrate = getPaymentsExtensionsToMigrate( localExtensions, dashboardOnlyExtensions, @@ -63,6 +69,19 @@ export async function ensureExtensionsIds( remoteExtensions = remoteExtensions.concat(newRemoteExtensions) } + if (marketingActivityExtensionsToMigrate.length > 0) { + const confirmedMigration = await extensionMigrationPrompt(marketingActivityExtensionsToMigrate, false) + if (!confirmedMigration) throw new AbortSilentError() + const newRemoteExtensions = await migrateAppModules( + marketingActivityExtensionsToMigrate, + options.appId, + 'marketing_activity', + dashboardOnlyExtensions, + options.developerPlatformClient, + ) + remoteExtensions = remoteExtensions.concat(newRemoteExtensions) + } + if (paymentsExtensionsToMigrate.length > 0) { const confirmedMigration = await extensionMigrationPrompt(paymentsExtensionsToMigrate, false) if (!confirmedMigration) throw new AbortSilentError() diff --git a/packages/app/src/cli/services/dev/migrate-marketing-activity-extension.test.ts b/packages/app/src/cli/services/dev/migrate-marketing-activity-extension.test.ts new file mode 100644 index 0000000000..61f18ea231 --- /dev/null +++ b/packages/app/src/cli/services/dev/migrate-marketing-activity-extension.test.ts @@ -0,0 +1,103 @@ +import {getMarketingActivtyExtensionsToMigrate} from './migrate-marketing-activity-extension.js' +import {LocalSource, RemoteSource} from '../context/identifiers.js' +import {describe, expect, test} from 'vitest' + +function getLocalExtension(attributes: Partial = {}) { + return { + type: 'marketing_activity', + localIdentifier: 'test-marketing', + configuration: { + name: 'test-marketing', + }, + ...attributes, + } as unknown as LocalSource +} + +function getRemoteExtension(attributes: Partial = {}) { + return { + uuid: '1234', + type: 'marketing_activity_extension', + title: 'test-marketing-2', + ...attributes, + } as unknown as RemoteSource +} + +describe('getExtensionsToMigrate()', () => { + const defaultIds = { + 'test-marketing': '1234', + } + + test('matching my remote title and localIdentifier', () => { + // Given + const title = 'test123' + const localExtension = getLocalExtension({ + type: 'marketing_activity', + localIdentifier: title, + }) + const remoteExtension = getRemoteExtension({ + type: 'marketing_activity_extension', + title, + uuid: 'yy', + }) + + // When + const toMigrate = getMarketingActivtyExtensionsToMigrate([localExtension], [remoteExtension], defaultIds) + + // Then + expect(toMigrate).toStrictEqual([{local: localExtension, remote: remoteExtension}]) + }) + + test('matching my local and remote IDs', () => { + // Given + const localExtension = getLocalExtension({ + type: 'marketing_activity', + localIdentifier: 'test-marketing', + }) + const remoteExtension = getRemoteExtension({type: 'marketing_activity_extension', title: 'remote', uuid: '1234'}) + + // When + const toMigrate = getMarketingActivtyExtensionsToMigrate([localExtension], [remoteExtension], defaultIds) + + // Then + expect(toMigrate).toStrictEqual([{local: localExtension, remote: remoteExtension}]) + }) + + test('does not return extensions where local.type is not marketing_activity', () => { + // Given + const localExtension = getLocalExtension({type: 'checkout_ui_extension'}) + const remoteExtension = getRemoteExtension({type: 'flow_action_definition'}) + + // When + const toMigrate = getMarketingActivtyExtensionsToMigrate([localExtension], [remoteExtension], defaultIds) + + // Then + expect(toMigrate).toStrictEqual([]) + }) + + test('does not return extensions where remote.type is not marketing_activity_extension', () => { + // Given + const localExtension = getLocalExtension({type: 'PRODUCT_SUBSCRIPTION_UI_EXTENSION'}) + const remoteExtension = getRemoteExtension({type: 'PRODUCT_SUBSCRIPTION_UI_EXTENSION'}) + + // When + const toMigrate = getMarketingActivtyExtensionsToMigrate([localExtension], [remoteExtension], defaultIds) + + // Then + expect(toMigrate).toStrictEqual([]) + }) + + test('if neither title/name or ids match, does not return any extensions', () => { + // Given + const localExtension = getLocalExtension({type: 'flow_action'}) + const remoteExtension = getRemoteExtension({ + type: 'flow_action_definition', + uuid: '5678', + }) + + // When + const toMigrate = getMarketingActivtyExtensionsToMigrate([localExtension], [remoteExtension], defaultIds) + + // Then + expect(toMigrate).toStrictEqual([]) + }) +}) diff --git a/packages/app/src/cli/services/dev/migrate-marketing-activity-extension.ts b/packages/app/src/cli/services/dev/migrate-marketing-activity-extension.ts new file mode 100644 index 0000000000..2c5a3569ea --- /dev/null +++ b/packages/app/src/cli/services/dev/migrate-marketing-activity-extension.ts @@ -0,0 +1,36 @@ +import {LocalSource, RemoteSource} from '../context/identifiers.js' +import {IdentifiersExtensions} from '../../models/app/identifiers.js' +import {getExtensionIds, LocalRemoteSource} from '../context/id-matching.js' +import {slugify} from '@shopify/cli-kit/common/string' + +export function getMarketingActivtyExtensionsToMigrate( + localSources: LocalSource[], + remoteSources: RemoteSource[], + identifiers: IdentifiersExtensions, +) { + const ids = getExtensionIds(localSources, identifiers) + const localExtensionTypesToMigrate = ['marketing_activity'] + const remoteExtensionTypesToMigrate = ['marketing_activity_extension'] + const typesMap = new Map([['marketing_activity', 'marketing_activity_extension']]) + + const local = localSources.filter((source) => localExtensionTypesToMigrate.includes(source.type)) + const remote = remoteSources.filter((source) => remoteExtensionTypesToMigrate.includes(source.type)) + + // Map remote sources by uuid and slugified title (the slugified title is used for matching with local folder) + const remoteSourcesMap = new Map() + remote.forEach((remoteSource) => { + remoteSourcesMap.set(remoteSource.uuid, remoteSource) + remoteSourcesMap.set(slugify(remoteSource.title), remoteSource) + }) + + return local.reduce((accumulator, localSource) => { + const localSourceId = ids[localSource.localIdentifier] ?? 'unknown' + const remoteSource = remoteSourcesMap.get(localSourceId) || remoteSourcesMap.get(localSource.localIdentifier) + const typeMatch = typesMap.get(localSource.type) === remoteSource?.type + + if (remoteSource && typeMatch) { + accumulator.push({local: localSource, remote: remoteSource}) + } + return accumulator + }, []) +} diff --git a/packages/app/src/cli/services/marketing_activity/extension-to-toml.test.ts b/packages/app/src/cli/services/marketing_activity/extension-to-toml.test.ts index 7a434a3820..928f11d17c 100644 --- a/packages/app/src/cli/services/marketing_activity/extension-to-toml.test.ts +++ b/packages/app/src/cli/services/marketing_activity/extension-to-toml.test.ts @@ -5,7 +5,7 @@ import {describe, expect, test} from 'vitest' const defaultDashboardConfig: MarketingActivityDashboardConfig = { title: 'test mae', description: 'test mae description', - app_api_url: 'https://google.es', + app_api_url: 'https://google.es/api/v1', tactic: 'ad', platform: 'facebook', is_automation: false, @@ -25,7 +25,7 @@ const defaultDashboardConfig: MarketingActivityDashboardConfig = { ], } describe('extension-to-toml', () => { - test('correctly builds a toml string for a marketing_activity_extension', () => { + test('converts the dashboard config to the new cli config', () => { // Given const extension: ExtensionRegistration = { id: '26237698049', @@ -42,12 +42,12 @@ describe('extension-to-toml', () => { // Then expect(got).toEqual(`[[extensions]] -type = "marketing_activity_extension_cli" +type = "marketing_activity" name = "test mae" handle = "mae-test-123" title = "test mae" description = "test mae description" -app_api_url = "https://google.es" +api_path = "/api/v1" tactic = "ad" marketing_channel = "social" referring_domain = "facebook.com" @@ -58,7 +58,6 @@ is_automation = false value = "test value" [[extensions.fields]] - id = "123" ui_type = "text-single-line" name = "test_field" label = "test field" @@ -86,33 +85,7 @@ is_automation = false const got = buildTomlObject(extension) // Then - expect(got).toEqual(`[[extensions]] -type = "marketing_activity_extension_cli" -name = "test mae" -handle = "mae-test-123455555555544444" -title = "test mae" -description = "test mae description" -app_api_url = "https://google.es" -tactic = "ad" -marketing_channel = "social" -referring_domain = "facebook.com" -is_automation = false - - [[extensions.preview_data]] - label = "test label" - value = "test value" - - [[extensions.fields]] - id = "123" - ui_type = "text-single-line" - name = "test_field" - label = "test field" - help_text = "help text" - required = false - min_length = 1 - max_length = 50 - placeholder = "placeholder" -`) + expect(got).toContain('handle = "mae-test-123455555555544444"') }) test('sets the channel and referring domain to empty string if no platform mapping is found', () => { @@ -131,32 +104,7 @@ is_automation = false const got = buildTomlObject(extension) // Then - expect(got).toEqual(`[[extensions]] -type = "marketing_activity_extension_cli" -name = "test mae" -handle = "mae-test-123" -title = "test mae" -description = "test mae description" -app_api_url = "https://google.es" -tactic = "ad" -marketing_channel = "" -referring_domain = "" -is_automation = false - - [[extensions.preview_data]] - label = "test label" - value = "test value" - - [[extensions.fields]] - id = "123" - ui_type = "text-single-line" - name = "test_field" - label = "test field" - help_text = "help text" - required = false - min_length = 1 - max_length = 50 - placeholder = "placeholder" -`) + expect(got).toContain('marketing_channel = ""') + expect(got).toContain('referring_domain = ""') }) }) diff --git a/packages/app/src/cli/services/marketing_activity/extension-to-toml.ts b/packages/app/src/cli/services/marketing_activity/extension-to-toml.ts index 51a3f05ae1..19cc8cf382 100644 --- a/packages/app/src/cli/services/marketing_activity/extension-to-toml.ts +++ b/packages/app/src/cli/services/marketing_activity/extension-to-toml.ts @@ -155,6 +155,11 @@ const PLATFORM_DOMAIN_MAP: {[key: string]: string | null} = { flow: null, } +function getUrlPath(url: string) { + const urlObject = new URL(url) + return urlObject.pathname + urlObject.search + urlObject.hash +} + /** * Given a dashboard-built marketing activity extension config file, convert it to toml for the CLI extension */ @@ -166,19 +171,19 @@ export function buildTomlObject(extension: ExtensionRegistration): string { const localExtensionRepresentation = { extensions: [ { - type: 'marketing_activity_extension_cli', + type: 'marketing_activity', name: config.title, handle: slugify(extension.title.substring(0, MAX_EXTENSION_HANDLE_LENGTH)), title: config.title, description: config.description, - app_api_url: config.app_api_url, + api_path: getUrlPath(config.app_api_url), tactic: config.tactic, marketing_channel: PLATFORM_CHANNEL_MAP[config.platform] ?? '', referring_domain: PLATFORM_DOMAIN_MAP[config.platform] ?? '', is_automation: config.is_automation, use_external_editor: config.use_external_editor, preview_data: config.preview_data, - fields: config.fields, + fields: config.fields.map(({id, ...field}) => field), }, ], }