Skip to content

Commit

Permalink
Merge pull request #4197 from Shopify/bc/add-migrate-on-deploy
Browse files Browse the repository at this point in the history
add deploy marketing activity extension logic
  • Loading branch information
billycai committed Sep 12, 2024
2 parents b86bc88 + b85dc54 commit 9541bc0
Show file tree
Hide file tree
Showing 10 changed files with 189 additions and 73 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/models/extensions/load-specifications.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -73,7 +73,7 @@ function loadSpecifications() {
posUISpec,
productSubscriptionSpec,
taxCalculationSpec,
marketingActivityExtensionSpec,
marketingActivitySpec,
themeSpec,
uiExtensionSpec,
webPixelSpec,
Expand Down
Original file line number Diff line number Diff line change
@@ -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(),
})),
}
},
})
Expand Down
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
})

Expand Down Expand Up @@ -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',
Expand Down
19 changes: 19 additions & 0 deletions packages/app/src/cli/services/context/identifiers-extensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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[]
Expand All @@ -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,
Expand Down Expand Up @@ -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()
Expand Down
Original file line number Diff line number Diff line change
@@ -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<LocalSource> = {}) {
return {
type: 'marketing_activity',
localIdentifier: 'test-marketing',
configuration: {
name: 'test-marketing',
},
...attributes,
} as unknown as LocalSource
}

function getRemoteExtension(attributes: Partial<RemoteSource> = {}) {
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([])
})
})
Original file line number Diff line number Diff line change
@@ -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<string, string>([['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<string, RemoteSource>()
remote.forEach((remoteSource) => {
remoteSourcesMap.set(remoteSource.uuid, remoteSource)
remoteSourcesMap.set(slugify(remoteSource.title), remoteSource)
})

return local.reduce<LocalRemoteSource[]>((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
}, [])
}
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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',
Expand All @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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 = ""')
})
})
Loading

0 comments on commit 9541bc0

Please sign in to comment.