From 05865515255b430be379a91c86e916d37e4cc61d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Tue, 7 Jul 2026 12:59:18 +0200 Subject: [PATCH] Remove atomic deployment support flag Assisted-By: devx/782ef931-ac0a-4cc4-bff1-a2511c379664 --- .../app/src/cli/models/app/app.test-data.ts | 8 +- .../src/cli/models/app/identifiers.test.ts | 31 +- .../app/src/cli/models/app/identifiers.ts | 12 +- .../app/add-uid-to-extension-toml.test.ts | 44 +- .../services/app/add-uid-to-extension-toml.ts | 4 +- packages/app/src/cli/services/context.test.ts | 214 +------ packages/app/src/cli/services/context.ts | 85 +-- .../context/breakdown-extensions.test.ts | 205 ------- .../services/context/breakdown-extensions.ts | 8 +- .../cli/services/context/id-matching.test.ts | 550 +----------------- .../src/cli/services/context/id-matching.ts | 2 +- .../context/identifiers-extensions.test.ts | 62 +- .../context/identifiers-extensions.ts | 41 +- packages/app/src/cli/services/deploy.ts | 6 +- .../app/src/cli/services/import-extensions.ts | 3 +- .../utilities/developer-platform-client.ts | 1 - .../app-management-client.ts | 1 - .../partners-client.ts | 1 - 18 files changed, 79 insertions(+), 1199 deletions(-) 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 48c5beb2120..1adb031c62f 100644 --- a/packages/app/src/cli/models/app/app.test-data.ts +++ b/packages/app/src/cli/models/app/app.test-data.ts @@ -1331,7 +1331,6 @@ export function testDeveloperPlatformClient(stubs: Partial ] = vi.fn().mockImplementation(value) } diff --git a/packages/app/src/cli/models/app/identifiers.test.ts b/packages/app/src/cli/models/app/identifiers.test.ts index 5ad091c0ac9..e0615febd4c 100644 --- a/packages/app/src/cli/models/app/identifiers.test.ts +++ b/packages/app/src/cli/models/app/identifiers.test.ts @@ -1,12 +1,12 @@ import {updateAppIdentifiers, getAppIdentifiers} from './identifiers.js' -import {testApp, testAppWithConfig, testDeveloperPlatformClient, testUIExtension} from './app.test-data.js' +import {testApp, testAppWithConfig, testUIExtension} from './app.test-data.js' import {describe, expect, test} from 'vitest' import {readAndParseDotEnv} from '@shopify/cli-kit/node/dot-env' import {fileExists, inTemporaryDirectory, readFile, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' describe('updateAppIdentifiers', () => { - test('persists the ids that are not env variables when deploying, creating a new file', async () => { + test('updates ids in memory when deploying without creating a new env file', async () => { await inTemporaryDirectory(async (tmpDir: string) => { // Given const uiExtension = await testUIExtension() @@ -25,19 +25,15 @@ describe('updateAppIdentifiers', () => { }, }, command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient(), }) // Then - const dotEnvFile = await readAndParseDotEnv(joinPath(tmpDir, '.env')) - expect(dotEnvFile.variables.SHOPIFY_API_KEY).toEqual('FOO') - expect(dotEnvFile.variables.SHOPIFY_MY_EXTENSION_ID).toEqual('BAR') - expect(gotApp.dotenv?.variables.SHOPIFY_API_KEY).toEqual('FOO') - expect(gotApp.dotenv?.variables.SHOPIFY_MY_EXTENSION_ID).toEqual('BAR') + await expect(fileExists(joinPath(tmpDir, '.env'))).resolves.toBe(false) + expect(gotApp.dotenv).toBeUndefined() }) }) - test('persists the ids in the config-specific env file when deploying, updating the existing file', async () => { + test('does not write ids to the config-specific env file when deploying', async () => { await inTemporaryDirectory(async (tmpDir: string) => { // Given const dotEnvFilePath = joinPath(tmpDir, '.env.staging') @@ -62,20 +58,17 @@ describe('updateAppIdentifiers', () => { }, }, command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient(), }) // Then const dotEnvFileContent = await readFile(dotEnvFilePath) const dotEnvFile = await readAndParseDotEnv(dotEnvFilePath) - expect(dotEnvFileContent).toEqual( - '#comment\nEXISTING_VAR=value\nSHOPIFY_MY_EXTENSION_ID=BAR\n#anothercomment\nSHOPIFY_API_KEY=FOO', - ) + expect(dotEnvFileContent).toEqual('#comment\nEXISTING_VAR=value\nSHOPIFY_MY_EXTENSION_ID=OLDID\n#anothercomment') expect(dotEnvFile.variables.EXISTING_VAR).toEqual('value') - expect(dotEnvFile.variables.SHOPIFY_API_KEY).toEqual('FOO') - expect(dotEnvFile.variables.SHOPIFY_MY_EXTENSION_ID).toEqual('BAR') - expect(gotApp.dotenv?.variables.SHOPIFY_API_KEY).toEqual('FOO') - expect(gotApp.dotenv?.variables.SHOPIFY_MY_EXTENSION_ID).toEqual('BAR') + expect(dotEnvFile.variables.SHOPIFY_API_KEY).toBeUndefined() + expect(dotEnvFile.variables.SHOPIFY_MY_EXTENSION_ID).toEqual('OLDID') + expect(gotApp.dotenv?.variables.SHOPIFY_API_KEY).toBeUndefined() + expect(gotApp.dotenv?.variables.SHOPIFY_MY_EXTENSION_ID).toBeUndefined() }) }) @@ -99,7 +92,6 @@ describe('updateAppIdentifiers', () => { }, }, command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient(), }, {SHOPIFY_API_KEY: 'FOO', SHOPIFY_MY_EXTENSION_ID: 'BAR'}, ) @@ -115,7 +107,7 @@ describe('updateAppIdentifiers', () => { }) }) -test('does not change a unified config TOML with multiple when the uid is already present for atomic deployments', async () => { +test('does not change a unified config TOML with multiple when the uid is already present', async () => { await inTemporaryDirectory(async (tmpDir: string) => { // Given const uiExtension1 = await testUIExtension({ @@ -167,7 +159,6 @@ type = "ui_extension"`, }, }, command: 'deploy', - developerPlatformClient: testDeveloperPlatformClient({supportsAtomicDeployments: true}), }, {SHOPIFY_API_KEY: 'FOO', SHOPIFY_MY_EXTENSION_ID: 'BAR'}, ) diff --git a/packages/app/src/cli/models/app/identifiers.ts b/packages/app/src/cli/models/app/identifiers.ts index 773bb3759cd..ed38e1ecdb0 100644 --- a/packages/app/src/cli/models/app/identifiers.ts +++ b/packages/app/src/cli/models/app/identifiers.ts @@ -1,11 +1,9 @@ import {getDotEnvFileName} from './loader.js' import {ExtensionInstance} from '../extensions/extension-instance.js' -import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {patchEnvFile} from '@shopify/cli-kit/node/dot-env' import {constantize} from '@shopify/cli-kit/common/string' import {joinPath} from '@shopify/cli-kit/node/path' import {fileExists, readFile, writeFile} from '@shopify/cli-kit/node/fs' -import {deepCompare} from '@shopify/cli-kit/common/object' import type {AppInterface} from './app.js' export interface IdentifiersExtensions { @@ -38,7 +36,6 @@ interface UpdateAppIdentifiersOptions { app: AppInterface identifiers: UuidOnlyIdentifiers command: UpdateAppIdentifiersCommand - developerPlatformClient: DeveloperPlatformClient } /** @@ -47,7 +44,7 @@ interface UpdateAppIdentifiersOptions { * @returns An copy of the app with the environment updated to reflect the updated identifiers. */ export async function updateAppIdentifiers( - {app, identifiers, command, developerPlatformClient}: UpdateAppIdentifiersOptions, + {app, identifiers, command}: UpdateAppIdentifiersOptions, systemEnvironment = process.env, ): Promise { let dotenvFile = app.dotenv @@ -67,12 +64,7 @@ export async function updateAppIdentifiers( } }) - const contentIsEqual = deepCompare(dotenvFile.variables, updatedVariables) - const writeToFile = - (!contentIsEqual && - (command === 'deploy' || command === 'release') && - !developerPlatformClient.supportsAtomicDeployments) || - command === 'import-extensions' + const writeToFile = command === 'import-extensions' dotenvFile.variables = updatedVariables diff --git a/packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts b/packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts index e3d91805dfd..0b482bbeaf3 100644 --- a/packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts +++ b/packages/app/src/cli/services/app/add-uid-to-extension-toml.test.ts @@ -1,43 +1,11 @@ import {addUidToTomlsIfNecessary} from './add-uid-to-extension-toml.js' import {ExtensionInstance} from '../../models/extensions/extension-instance.js' -import {testDeveloperPlatformClient, testUIExtension} from '../../models/app/app.test-data.js' +import {testDeveloperPlatformClient} from '../../models/app/app.test-data.js' import {describe, test, expect} from 'vitest' import {writeFile, readFile, inTemporaryDirectory} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' describe('addUidToTomlsIfNecessary', () => { - test('skips if platform does not support atomic deployments', async () => { - await inTemporaryDirectory(async (tmpDir) => { - // Given - const tomlPath = joinPath(tmpDir, 'extension.toml') - const tomlContent = ` - type = "checkout_ui_extension" - handle = "my-extension" - ` - await writeFile(tomlPath, tomlContent) - - const extension = await testUIExtension({ - directory: tmpDir, - configuration: { - handle: 'my-extension', - type: 'checkout_ui_extension', - name: 'test', - }, - configurationPath: tomlPath, - uid: '123', - }) - - const client = testDeveloperPlatformClient({supportsAtomicDeployments: false}) - - // When - await addUidToTomlsIfNecessary([extension], client) - - // Then - const updatedContent = await readFile(tomlPath) - expect(updatedContent).toBe(tomlContent) - }) - }) - test('adds uid to single extension TOML', async () => { await inTemporaryDirectory(async (tmpDir) => { // Given @@ -56,7 +24,7 @@ describe('addUidToTomlsIfNecessary', () => { configuration: {}, } as ExtensionInstance - const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + const client = testDeveloperPlatformClient({}) // When await addUidToTomlsIfNecessary([extension], client) @@ -97,7 +65,7 @@ scopes = "write_metaobject_definitions,write_metaobjects,write_products" configuration: {}, } as ExtensionInstance - const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + const client = testDeveloperPlatformClient({}) // When await addUidToTomlsIfNecessary([extension], client) @@ -155,7 +123,7 @@ scopes = "write_metaobject_definitions,write_metaobjects,write_products" configuration: {}, } as ExtensionInstance - const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + const client = testDeveloperPlatformClient({}) // When await addUidToTomlsIfNecessary([extension, extension1], client) @@ -197,7 +165,7 @@ scopes = "write_metaobject_definitions,write_metaobjects,write_products" }, } as ExtensionInstance - const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + const client = testDeveloperPlatformClient({}) // When await addUidToTomlsIfNecessary([extension], client) @@ -226,7 +194,7 @@ scopes = "write_metaobject_definitions,write_metaobjects,write_products" configuration: {}, } as ExtensionInstance - const client = testDeveloperPlatformClient({supportsAtomicDeployments: true}) + const client = testDeveloperPlatformClient({}) // When await addUidToTomlsIfNecessary([extension], client) diff --git a/packages/app/src/cli/services/app/add-uid-to-extension-toml.ts b/packages/app/src/cli/services/app/add-uid-to-extension-toml.ts index 34b4aa17b2f..676dbe76900 100644 --- a/packages/app/src/cli/services/app/add-uid-to-extension-toml.ts +++ b/packages/app/src/cli/services/app/add-uid-to-extension-toml.ts @@ -5,10 +5,8 @@ import {getPathValue} from '@shopify/cli-kit/common/object' export async function addUidToTomlsIfNecessary( extensions: ExtensionInstance[], - developerPlatformClient: DeveloperPlatformClient, + _developerPlatformClient: DeveloperPlatformClient, ) { - if (!developerPlatformClient.supportsAtomicDeployments) return - // We can't update the TOML files in parallel because some extensions might share the same file for (const extension of extensions) { // eslint-disable-next-line no-await-in-loop diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts index 9e5b1589e4b..ff236891e6f 100644 --- a/packages/app/src/cli/services/context.test.ts +++ b/packages/app/src/cli/services/context.test.ts @@ -23,8 +23,7 @@ import { testThemeExtensions, testProject, } from '../models/app/app.test-data.js' -import metadata from '../metadata.js' -import {getAppConfigurationFileName, isWebType, loadApp} from '../models/app/loader.js' +import {getAppConfigurationFileName, isWebType} from '../models/app/loader.js' import {AppLinkedInterface} from '../models/app/app.js' import * as loadSpecifications from '../models/extensions/load-specifications.js' import { @@ -37,7 +36,6 @@ import {selectOrganizationPrompt} from '@shopify/organizations' import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file' import {isServiceAccount, isUserAccount} from '@shopify/cli-kit/node/session' import {afterEach, beforeAll, beforeEach, describe, expect, test, vi} from 'vitest' -import {AbortError} from '@shopify/cli-kit/node/error' import {mockAndCaptureOutput} from '@shopify/cli-kit/node/testing/output' import {getPackageManager} from '@shopify/cli-kit/node/node-package-manager' import {renderConfirmationPrompt, renderInfo, renderTasks, renderWarning, Task} from '@shopify/cli-kit/node/ui' @@ -181,168 +179,7 @@ afterEach(() => { }) describe('ensureDeployContext', () => { - test('prompts the user to include the configuration if the flag is false and current config is false', async () => { - // Given - const app = testAppWithConfig({config: {client_id: APP2.apiKey, build: {include_config_on_deploy: false}}}) - const identifiers = { - app: APP2.apiKey, - extensions: {}, - extensionIds: {}, - extensionsNonUuidManaged: {}, - } - vi.mocked(getAppIdentifiers).mockReturnValue({app: undefined}) - vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) - vi.mocked(loadApp).mockResolvedValue(app) - vi.mocked(link).mockResolvedValue((app as any).configuration) - vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - mockTomlFilePatch.mockResolvedValue(undefined) - const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {}) - - const options = deployOptions(app) - vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient) - - // When - await ensureDeployContext(options) - - // Then - expect(metadataSpyOn).toHaveBeenCalled() - - expect(renderConfirmationPrompt).toHaveBeenCalled() - expect(mockTomlFilePatch).toHaveBeenCalled() - expect(renderInfo).toHaveBeenCalledWith({ - body: [ - { - list: { - items: ['Org: org1', 'App: app2', 'Include config: No'], - }, - }, - '\n', - 'You can pass', - { - command: '--reset', - }, - 'to your command to reset your app configuration.', - ], - headline: 'Using shopify.app.toml for default values:', - }) - mockTomlFilePatch.mockClear() - }) - - test('doesnt prompt the user to include the configuration and display the current value if the config is true', async () => { - // Given - const app = testAppWithConfig({config: {client_id: APP2.apiKey, build: {include_config_on_deploy: true}}}) - const identifiers = { - app: APP2.apiKey, - extensions: {}, - extensionIds: {}, - extensionsNonUuidManaged: {}, - } - vi.mocked(getAppIdentifiers).mockReturnValue({app: undefined}) - vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) - vi.mocked(loadApp).mockResolvedValue(app) - vi.mocked(link).mockResolvedValue((app as any).configuration) - // vi.mocked(selectDeveloperPlatformClient).mockReturnValue(testDeveloperPlatformClient) - vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - mockTomlFilePatch.mockResolvedValue(undefined) - const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {}) - - const options = deployOptions(app) - vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient) - - // When - await ensureDeployContext(options) - - // Then - expect(metadataSpyOn).not.toHaveBeenCalled() - - expect(renderConfirmationPrompt).not.toHaveBeenCalled() - expect(mockTomlFilePatch).not.toHaveBeenCalled() - expect(renderInfo).toHaveBeenCalledWith({ - body: [ - { - list: { - items: ['Org: org1', 'App: app2', 'Include config: Yes'], - }, - }, - '\n', - 'You can pass', - { - command: '--reset', - }, - 'to your command to reset your app configuration.', - ], - headline: 'Using shopify.app.toml for default values:', - }) - mockTomlFilePatch.mockClear() - }) - - test('prompts the user to include the configuration when reset is used and the flag is present', async () => { - // Given - const app = testAppWithConfig({config: {client_id: APP2.apiKey, build: {include_config_on_deploy: true}}}) - const identifiers = { - app: APP2.apiKey, - extensions: {}, - extensionIds: {}, - extensionsNonUuidManaged: {}, - } - vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) - vi.mocked(renderConfirmationPrompt).mockResolvedValue(false) - vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - mockTomlFilePatch.mockResolvedValue(undefined) - const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {}) - - const options = deployOptions(app, true) - vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient) - // When - await ensureDeployContext(deployOptions(app, true)) - - // Then - expect(metadataSpyOn).toHaveBeenNthCalledWith(1, expect.any(Function)) - expect(metadataSpyOn.mock.calls[0]![0]()).toEqual({cmd_deploy_confirm_include_config_used: false}) - - expect(renderConfirmationPrompt).toHaveBeenCalled() - expect(mockTomlFilePatch).toHaveBeenCalledWith({build: {include_config_on_deploy: false}}) - - expect(renderInfo).toHaveBeenCalledWith({ - body: [ - { - list: { - items: ['Org: org1', 'App: app2', 'Include config: No'], - }, - }, - '\n', - 'You can pass', - { - command: '--reset', - }, - 'to your command to reset your app configuration.', - ], - headline: 'Using shopify.app.toml for default values:', - }) - mockTomlFilePatch.mockClear() - }) - - test('aborts when force is true and include_config_on_deploy is not set on Partners', async () => { - // Given - const app = testAppWithConfig({config: {client_id: APP2.apiKey}}) - const identifiers = { - app: APP2.apiKey, - extensions: {}, - extensionIds: {}, - extensionsNonUuidManaged: {}, - } - vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) - vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - - const options = deployOptions(app, false, true) - vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient) - - // When/Then - await expect(ensureDeployContext(options)).rejects.toThrowError(AbortError) - await expect(ensureDeployContext(options)).rejects.toThrow('You must specify a value for') - }) - - test('does not abort when force is true and include_config_on_deploy is not set for App Management', async () => { + test('does not abort when force is true and include_config_on_deploy is not set', async () => { // Given const app = testAppWithConfig({config: {client_id: APP2.apiKey}}) const identifiers = { @@ -356,7 +193,7 @@ describe('ensureDeployContext', () => { const options = { ...deployOptions(app, false, true), - developerPlatformClient: buildDeveloperPlatformClient({supportsAtomicDeployments: true}), + developerPlatformClient: buildDeveloperPlatformClient({}), } vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient) @@ -364,45 +201,6 @@ describe('ensureDeployContext', () => { await expect(ensureDeployContext(options)).resolves.toBeDefined() }) - test('prompts the user to include the configuration when force is used and the flag is present', async () => { - // Given - const app = testAppWithConfig({config: {client_id: APP2.apiKey, build: {include_config_on_deploy: true}}}) - const identifiers = { - app: APP2.apiKey, - extensions: {}, - extensionIds: {}, - extensionsNonUuidManaged: {}, - } - vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) - vi.mocked(renderConfirmationPrompt).mockResolvedValue(false) - vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - mockTomlFilePatch.mockResolvedValue(undefined) - - // When - await ensureDeployContext(deployOptions(app, false, true)) - - // Then - expect(renderConfirmationPrompt).not.toHaveBeenCalled() - expect(mockTomlFilePatch).not.toHaveBeenCalled() - expect(renderInfo).toHaveBeenCalledWith({ - body: [ - { - list: { - items: ['Org: org1', 'App: app2', 'Include config: Yes'], - }, - }, - '\n', - 'You can pass', - { - command: '--reset', - }, - 'to your command to reset your app configuration.', - ], - headline: 'Using shopify.app.toml for default values:', - }) - mockTomlFilePatch.mockClear() - }) - test('removes the include_config_on_deploy field when using app management API and the value is true', async () => { // Given const app = testAppWithConfig({config: {client_id: APP2.apiKey, build: {include_config_on_deploy: true}}}) @@ -425,7 +223,7 @@ describe('ensureDeployContext', () => { reset: false, force: false, noRelease: false, - developerPlatformClient: buildDeveloperPlatformClient({supportsAtomicDeployments: true}), + developerPlatformClient: buildDeveloperPlatformClient({}), skipBuild: false, } await ensureDeployContext(options) @@ -468,7 +266,7 @@ describe('ensureDeployContext', () => { reset: false, force: false, noRelease: false, - developerPlatformClient: buildDeveloperPlatformClient({supportsAtomicDeployments: true}), + developerPlatformClient: buildDeveloperPlatformClient({}), skipBuild: false, } await ensureDeployContext(options) @@ -515,7 +313,6 @@ describe('ensureDeployContext', () => { } const developerPlatformClient = buildDeveloperPlatformClient({ - supportsAtomicDeployments: true, activeAppVersion: () => Promise.resolve(activeAppVersion), }) @@ -562,7 +359,6 @@ describe('ensureDeployContext', () => { } const developerPlatformClient = buildDeveloperPlatformClient({ - supportsAtomicDeployments: true, activeAppVersion: () => Promise.resolve(activeAppVersion), }) diff --git a/packages/app/src/cli/services/context.ts b/packages/app/src/cli/services/context.ts index 5dabb0beb08..2032bce875f 100644 --- a/packages/app/src/cli/services/context.ts +++ b/packages/app/src/cli/services/context.ts @@ -23,7 +23,7 @@ import {selectOrganizationPrompt} from '@shopify/organizations' import {TomlFile} from '@shopify/cli-kit/node/toml/toml-file' import {isServiceAccount, isUserAccount} from '@shopify/cli-kit/node/session' import {tryParseInt} from '@shopify/cli-kit/common/string' -import {Token, renderConfirmationPrompt, renderInfo, renderWarning} from '@shopify/cli-kit/node/ui' +import {Token, renderInfo, renderWarning} from '@shopify/cli-kit/node/ui' import {AbortError} from '@shopify/cli-kit/node/error' import {outputContent} from '@shopify/cli-kit/node/output' import {basename, sniffForJson} from '@shopify/cli-kit/node/path' @@ -145,10 +145,10 @@ interface EnsureDeployContextResult { * @returns The selected org, app and dev store */ export async function ensureDeployContext(options: DeployOptions): Promise { - const {reset, force, noRelease, app, remoteApp, developerPlatformClient, organization} = options + const {force, noRelease, app, remoteApp, developerPlatformClient, organization} = options const activeAppVersion = await developerPlatformClient.activeAppVersion(remoteApp) - const includeConfigOnDeploy = await checkIncludeConfigOnDeploy({app, reset, force, developerPlatformClient}) + const includeConfigOnDeploy = await checkIncludeConfigOnDeploy({app}) renderCurrentlyUsedConfigInfo({ org: organization.businessName, @@ -173,67 +173,20 @@ export async function ensureDeployContext(options: DeployOptions): Promise !version.registrationId) } return {identifiers, didMigrateExtensionsToDevDash} } -interface ShouldOrPromptIncludeConfigDeployOptions { - appDirectory: string - localApp: AppInterface -} - -async function checkIncludeConfigOnDeploy({ - app, - reset, - force, - developerPlatformClient, -}: { - app: AppInterface - reset: boolean - force: boolean - developerPlatformClient: DeveloperPlatformClient -}): Promise { - if (developerPlatformClient.supportsAtomicDeployments) { - await removeIncludeConfigOnDeployField(app) - return undefined - } - - let includeConfigOnDeploy = app.includeConfigOnDeploy - if (reset) includeConfigOnDeploy = undefined - - if (force && includeConfigOnDeploy === undefined) { - // If a partner is deploying on CI/CD with include_config_on_deploy not set, - // we need to abort the deploy, because the default value changed to true. - const message = [ - 'You must specify a value for', - {command: 'include_config_on_deploy'}, - 'in your TOML file. Including configuration will be required very soon.', - ] - const nextSteps = [ - 'Run', - {command: 'shopify app deploy'}, - 'interactively, without', - {command: '--allow-updates'}, - 'or', - {command: '--allow-deletes'}, - '.', - ] - throw new AbortError(message, nextSteps) - } - - if (force || includeConfigOnDeploy === true) return includeConfigOnDeploy - - return promptAndSaveIncludeConfigOnDeploy({ - appDirectory: app.directory, - localApp: app, - }) +async function checkIncludeConfigOnDeploy({app}: {app: AppInterface}): Promise { + await removeIncludeConfigOnDeployField(app) + return undefined } async function removeIncludeConfigOnDeployField(localApp: AppInterface) { @@ -272,28 +225,6 @@ function renderWarningAboutIncludeConfigOnDeploy() { }) } -async function promptAndSaveIncludeConfigOnDeploy(options: ShouldOrPromptIncludeConfigDeployOptions): Promise { - const shouldIncludeConfigDeploy = await includeConfigOnDeployPrompt(options.localApp.configPath) - options.localApp.configuration.build = { - ...options.localApp.configuration.build, - include_config_on_deploy: shouldIncludeConfigDeploy, - } - const configFile = await TomlFile.read(options.localApp.configPath) - await configFile.patch({build: {include_config_on_deploy: shouldIncludeConfigDeploy}}) - await metadata.addPublicMetadata(() => ({cmd_deploy_confirm_include_config_used: shouldIncludeConfigDeploy})) - return shouldIncludeConfigDeploy -} - -function includeConfigOnDeployPrompt(configPath: string): Promise { - return renderConfirmationPrompt({ - message: `Include \`${basename( - configPath, - )}\` configuration on \`deploy\`? Soon, this will no longer be optional and configuration will be included on every deploy.`, - confirmationMessage: 'Yes, always (Recommended)', - cancellationMessage: 'No, not now', - }) -} - export async function fetchOrCreateOrganizationApp( options: CreateAppOptions & {organizationId?: string}, ): Promise { diff --git a/packages/app/src/cli/services/context/breakdown-extensions.test.ts b/packages/app/src/cli/services/context/breakdown-extensions.test.ts index 7ac6882d58c..3e67d706db5 100644 --- a/packages/app/src/cli/services/context/breakdown-extensions.test.ts +++ b/packages/app/src/cli/services/context/breakdown-extensions.test.ts @@ -533,211 +533,6 @@ describe('extensionsIdentifiersDeployBreakdown', () => { remoteExtensionsRegistrations: remoteExtensionRegistrations.app, }) }) - test('and there is an active version with matching cli app modules then cli extension should be updated as "unchanged", and no dashboard extension should be migrated', async () => { - // Given - const extensionsToConfirm = { - validMatches: {EXTENSION_A: 'UUID_A'}, - dashboardOnlyExtensions: [REGISTRATION_DASHBOARD_A], - extensionsToCreate: [EXTENSION_A_2], - didMigrateDashboardExtensions: false, - } - vi.mocked(ensureExtensionsIds).mockResolvedValue(extensionsToConfirm) - const remoteExtensionRegistrations = { - app: { - extensionRegistrations: [REGISTRATION_A], - configurationRegistrations: [], - dashboardManagedExtensionRegistrations: [REGISTRATION_DASHBOARD_A], - }, - } - const activeAppVersion = { - appModuleVersions: [MODULE_CONFIG_A, MODULE_DASHBOARD_A, MODULE_CLI_A], - } - let fetchActiveAppVersionCalled = false - const developerPlatformClient: DeveloperPlatformClient = testDeveloperPlatformClient({ - appExtensionRegistrations: (_app: MinimalAppIdentifiers) => Promise.resolve(remoteExtensionRegistrations), - activeAppVersion: (_app: MinimalAppIdentifiers) => { - fetchActiveAppVersionCalled = true - return Promise.resolve(activeAppVersion) - }, - }) - - // When - const result = await extensionsIdentifiersDeployBreakdown( - await options({uiExtensions, developerPlatformClient, activeAppVersion}), - ) - - // Then - expect(fetchActiveAppVersionCalled).toBe(false) - expect(result).toEqual({ - extensionIdentifiersBreakdown: { - onlyRemote: [], - toCreate: [buildExtensionBreakdownInfo('extension-a-2', 'test-ui-extension-uid')], - toUpdate: [], - unchanged: [ - buildExtensionBreakdownInfo('EXTENSION_A', undefined), - buildDashboardBreakdownInfo('Dashboard A'), - ], - }, - extensionsToConfirm, - remoteExtensionsRegistrations: remoteExtensionRegistrations.app, - }) - }) - - test('and there is an active version with a module matching a local spec external identifier then cli extension should be unchanged', async () => { - // Given - const extensionsToConfirm = { - validMatches: {EXTENSION_A: 'UUID_A'}, - dashboardOnlyExtensions: [], - extensionsToCreate: [], - didMigrateDashboardExtensions: false, - } - vi.mocked(ensureExtensionsIds).mockResolvedValue(extensionsToConfirm) - const remoteExtensionRegistrations = { - app: { - extensionRegistrations: [REGISTRATION_A], - configurationRegistrations: [], - dashboardManagedExtensionRegistrations: [], - }, - } - const activeAppVersion = { - appModuleVersions: [MODULE_CLI_A_EXTERNAL_IDENTIFIER], - } - - const developerPlatformClient: DeveloperPlatformClient = testDeveloperPlatformClient({ - appExtensionRegistrations: (_app: MinimalAppIdentifiers) => Promise.resolve(remoteExtensionRegistrations), - }) - - // When - const result = await extensionsIdentifiersDeployBreakdown( - await options({uiExtensions: [EXTENSION_A], developerPlatformClient, activeAppVersion}), - ) - - // Then - expect(result).toEqual({ - extensionIdentifiersBreakdown: { - onlyRemote: [], - toCreate: [], - toUpdate: [], - unchanged: [buildExtensionBreakdownInfo('EXTENSION_A', undefined)], - }, - extensionsToConfirm, - remoteExtensionsRegistrations: remoteExtensionRegistrations.app, - }) - }) - - test('and there is an active version with matching dashboard migrated cli app modules then migrated extension should be returned in the to create', async () => { - // Given - const remoteExtensionRegistrations = { - app: { - extensionRegistrations: [REGISTRATION_A, REGISTRATION_DASH_MIGRATED_A], - configurationRegistrations: [], - dashboardManagedExtensionRegistrations: [REGISTRATION_DASHBOARD_A, REGISTRATION_DASH_MIGRATED_A], - }, - } - const extensionsToConfirm = { - validMatches: {EXTENSION_A: 'UUID_A', DASH_MIGRATED_EXTENSION_A: 'UUID_DM_A'}, - dashboardOnlyExtensions: [REGISTRATION_DASHBOARD_A, REGISTRATION_DASH_MIGRATED_A], - extensionsToCreate: [EXTENSION_A_2], - didMigrateDashboardExtensions: true, - } - vi.mocked(ensureExtensionsIds).mockResolvedValue(extensionsToConfirm) - const activeAppVersion = { - appModuleVersions: [MODULE_CONFIG_A, MODULE_DASHBOARD_A, MODULE_CLI_A, MODULE_DASHBOARD_MIGRATED_CLI_A], - } - let fetchActiveAppVersionCalled = false - const developerPlatformClient: DeveloperPlatformClient = testDeveloperPlatformClient({ - appExtensionRegistrations: (_app: MinimalAppIdentifiers) => Promise.resolve(remoteExtensionRegistrations), - activeAppVersion: (_app: MinimalAppIdentifiers) => { - fetchActiveAppVersionCalled = true - return Promise.resolve(activeAppVersion) - }, - }) - - // When - const result = await extensionsIdentifiersDeployBreakdown( - await options({uiExtensions, developerPlatformClient, activeAppVersion}), - ) - - // Then - expect(fetchActiveAppVersionCalled).toBe(true) - expect(result).toEqual({ - extensionIdentifiersBreakdown: { - onlyRemote: [], - toCreate: [ - buildExtensionBreakdownInfo('DASH_MIGRATED_EXTENSION_A', 'UUID_DM_A'), - buildExtensionBreakdownInfo('extension-a-2', 'test-ui-extension-uid'), - ], - toUpdate: [], - unchanged: [ - buildExtensionBreakdownInfo('EXTENSION_A', undefined), - buildDashboardBreakdownInfo('Dashboard A'), - ], - }, - extensionsToConfirm, - remoteExtensionsRegistrations: remoteExtensionRegistrations.app, - }) - }) - test('and there is an active version with no matching local app modules then they should be returned in the extensions list to delete', async () => { - // Given - const remoteExtensionRegistrations = { - app: { - extensionRegistrations: [REGISTRATION_A, REGISTRATION_DASH_MIGRATED_A], - configurationRegistrations: [], - dashboardManagedExtensionRegistrations: [ - REGISTRATION_DASHBOARD_A, - REGISTRATION_DASH_MIGRATED_A, - REGISTRATION_DASHBOARD_NEW, - ], - }, - } - const extensionsToConfirm = { - validMatches: {EXTENSION_A: 'UUID_A', DASH_MIGRATED_EXTENSION_A: 'UUID_DM_A'}, - dashboardOnlyExtensions: [REGISTRATION_DASHBOARD_A, REGISTRATION_DASH_MIGRATED_A, REGISTRATION_DASHBOARD_NEW], - extensionsToCreate: [EXTENSION_A_2], - didMigrateDashboardExtensions: false, - } - vi.mocked(ensureExtensionsIds).mockResolvedValue(extensionsToConfirm) - const activeVersion = { - appModuleVersions: [ - MODULE_CONFIG_A, - MODULE_DASHBOARD_A, - MODULE_CLI_A, - MODULE_DASHBOARD_MIGRATED_CLI_A, - MODULE_DELETED_DASHBOARD_B, - MODULE_DELETED_CLI_B, - ], - } - const developerPlatformClient: DeveloperPlatformClient = testDeveloperPlatformClient({ - appExtensionRegistrations: (_app: MinimalAppIdentifiers) => Promise.resolve(remoteExtensionRegistrations), - activeAppVersion: (_app: MinimalAppIdentifiers) => Promise.resolve(activeVersion), - }) - - // When - const result = await extensionsIdentifiersDeployBreakdown(await options({uiExtensions, developerPlatformClient})) - - // Then - expect(result).toEqual({ - extensionIdentifiersBreakdown: { - onlyRemote: [ - buildExtensionBreakdownInfo('Checkout post purchase Deleted B', 'B'), - buildDashboardBreakdownInfo('Dashboard Deleted B'), - ], - toCreate: [ - buildExtensionBreakdownInfo('DASH_MIGRATED_EXTENSION_A', 'UUID_DM_A'), - buildExtensionBreakdownInfo('extension-a-2', 'test-ui-extension-uid'), - buildDashboardBreakdownInfo('Dashboard New'), - ], - toUpdate: [], - unchanged: [ - buildExtensionBreakdownInfo('EXTENSION_A', undefined), - buildDashboardBreakdownInfo('Dashboard A'), - ], - }, - extensionsToConfirm, - remoteExtensionsRegistrations: remoteExtensionRegistrations.app, - }) - }) - test('and there is an active version with modules without UID, those should be returned as toUpdate, the rest, unchanged', async () => { // Given const extensionsToConfirm = { diff --git a/packages/app/src/cli/services/context/breakdown-extensions.ts b/packages/app/src/cli/services/context/breakdown-extensions.ts index aad339444f5..e8400fa2fb3 100644 --- a/packages/app/src/cli/services/context/breakdown-extensions.ts +++ b/packages/app/src/cli/services/context/breakdown-extensions.ts @@ -345,7 +345,7 @@ function loadExtensionsIdentifiersBreakdown( validMatches: IdentifiersExtensions, toCreate: LocalSource[], specs: ExtensionSpecification[], - developerPlatformClient: DeveloperPlatformClient, + _developerPlatformClient: DeveloperPlatformClient, ) { const extensionModules = activeAppVersion?.appModuleVersions.filter((ext) => { const spec = specs.find( @@ -362,11 +362,7 @@ function loadExtensionsIdentifiersBreakdown( const UidMatch = module.registrationId === identifier const pendingMigration = module.registrationId.length === 0 - if (developerPlatformClient.supportsAtomicDeployments) { - return UidMatch || (pendingMigration && UuidMatch) - } else { - return UuidMatch - } + return UidMatch || (pendingMigration && UuidMatch) } const allExistingExtensions = Object.entries(validMatches) diff --git a/packages/app/src/cli/services/context/id-matching.test.ts b/packages/app/src/cli/services/context/id-matching.test.ts index a8313fe5765..8c73e8d2d5e 100644 --- a/packages/app/src/cli/services/context/id-matching.test.ts +++ b/packages/app/src/cli/services/context/id-matching.test.ts @@ -251,552 +251,10 @@ beforeAll(async () => { }) }) -describe('automaticMatchmaking: some local, no remote ones', () => { - test('creates all local extensions', async () => { - // When - const got = await automaticMatchmaking([EXTENSION_A, EXTENSION_B], [], {}, testDeveloperPlatformClient()) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [EXTENSION_A, EXTENSION_B], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: some local of the same type, no remote ones', () => { - test('creates all local extensions', async () => { - // When - const got = await automaticMatchmaking([EXTENSION_A, EXTENSION_A_2], [], {}, testDeveloperPlatformClient()) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [EXTENSION_A, EXTENSION_A_2], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: some local of the same type, only one remote', () => { - test('creates all local extensions', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_A_2], - [REGISTRATION_A], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A'}, - toConfirm: [], - toCreate: [EXTENSION_A_2], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: some local of the same type, a remote with same type but a remote name that doesnt match a local handle', () => { - test('prompts for manual matching', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_A_2], - [REGISTRATION_A_3], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [EXTENSION_A, EXTENSION_A_2], remote: [REGISTRATION_A_3]}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: some local of the same type, one matching remote and one not matching', () => { - test('prompts for manual matching', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_A_2], - [REGISTRATION_A, REGISTRATION_A_3], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A'}, - toConfirm: [{local: EXTENSION_A_2, remote: REGISTRATION_A_3}], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: some local of the same type, two remotes that do not match', () => { - test('prompts for manual matching', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_A_2], - [REGISTRATION_A_3, REGISTRATION_A_4], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [EXTENSION_A, EXTENSION_A_2], remote: [REGISTRATION_A_3, REGISTRATION_A_4]}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: two pairs of local and only one pair of remote but with diff names', () => { - test('creates one pair, adds the other to manual match', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_A_2, EXTENSION_B, EXTENSION_B_2], - [REGISTRATION_A_3, REGISTRATION_A_4], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [EXTENSION_B, EXTENSION_B_2], - toManualMatch: {local: [EXTENSION_A, EXTENSION_A_2], remote: [REGISTRATION_A_3, REGISTRATION_A_4]}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: same number of local and remote with matching types', () => { - test('matches them automatically', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_B], - [REGISTRATION_A, REGISTRATION_B], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A', 'extension-b': 'UUID_B'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: more local than remote, all remote match some local', () => { - test('matches some and will create the rest', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_B, EXTENSION_C, EXTENSION_D], - [REGISTRATION_A, REGISTRATION_B], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A', 'extension-b': 'UUID_B'}, - toConfirm: [], - toCreate: [EXTENSION_C, EXTENSION_D], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: remote have types not present locally', () => { - test('create local ones but remind we have unmatched remote', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_B], - [REGISTRATION_C, REGISTRATION_D], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [EXTENSION_A, EXTENSION_B], - toManualMatch: {local: [], remote: [REGISTRATION_C, REGISTRATION_D]}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: some sources match, but other are missing', () => { - test('matches when possible and leave rest to manual matching', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_B], - [REGISTRATION_A, REGISTRATION_C], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A'}, - toConfirm: [], - toCreate: [EXTENSION_B], - toManualMatch: {local: [], remote: [REGISTRATION_C]}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: multiple sources of the same type locally and remotely', () => { - test('matches automatically', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_A_2], - [REGISTRATION_A, REGISTRATION_A_2], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: { - 'extension-a': 'UUID_A', - 'extension-a-2': 'UUID_A_2', - }, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: multiple sources of the same type locally and remotely, others can be matched', () => { - test('matches automatically and creates new one', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_A_2, EXTENSION_B], - [REGISTRATION_A, REGISTRATION_A_2], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: { - 'extension-a': 'UUID_A', - 'extension-a-2': 'UUID_A_2', - }, - toConfirm: [], - toCreate: [EXTENSION_B], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: more remote of the same type than local', () => { - test('matches one and leaves to manual matching', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A], - [REGISTRATION_A, REGISTRATION_A_2], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: [REGISTRATION_A_2]}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: more remote of the same type than local, but none matching', () => { - test('leaves to manual matching', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A], - [REGISTRATION_A_2, REGISTRATION_A_3], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [EXTENSION_A], - toManualMatch: {local: [], remote: [REGISTRATION_A_2, REGISTRATION_A_3]}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: more remote of different types than local', () => { - test('matches one and leaves to manual matching', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A], - [REGISTRATION_A, REGISTRATION_B], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: [REGISTRATION_B]}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: same name but different remote type (but still matches)', () => { - test('matches automatically', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_D], - [REGISTRATION_D_WITH_EXTERNAL_ID], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-d': 'UUID_D'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: some sources have uuid, others can be matched', () => { - test('matches automatically', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_B], - [REGISTRATION_A, REGISTRATION_B], - { - EXTENSION_A: 'UUID_A', - }, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A', 'extension-b': 'UUID_B'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe("automaticMatchmaking: some sources have uuid, but doesn't match a remote one", () => { - test('matches to the correct UUID', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_B], - [REGISTRATION_A, REGISTRATION_B], - { - EXTENSION_A: 'UUID_WRONG', - }, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A', 'extension-b': 'UUID_B'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: duplicated sources types but some of them already matched', () => { - test('matches the other extensions', async () => { - // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_A_2, EXTENSION_B], - [REGISTRATION_A, REGISTRATION_A_2, REGISTRATION_B], - { - EXTENSION_A: 'UUID_A', - }, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'extension-a': 'UUID_A', 'extension-a-2': 'UUID_A_2', 'extension-b': 'UUID_B'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: automatic matches with different names', () => { - test('matches pending confirmation', async () => { - // When - const registrationNewA = {...REGISTRATION_A, title: 'A_NEW'} - const registrationNewB = {...REGISTRATION_B, title: 'B_NEW'} - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_B], - [registrationNewA, registrationNewB], - {}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {}, - toConfirm: [ - {local: EXTENSION_A, remote: registrationNewA}, - {local: EXTENSION_B, remote: registrationNewB}, - ], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: if identifiers contains something else', () => { - test('is ignored', async () => { - // When - const got = await automaticMatchmaking([], [], {FUNCTION_A: 'FUNCTION_A'}, testDeveloperPlatformClient()) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: functions', () => { - test('creates all local functions', async () => { - // When - const got = await automaticMatchmaking([FUNCTION_A], [], {}, testDeveloperPlatformClient()) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [FUNCTION_A], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: migrates functions with legacy IDs to extension IDs', () => { - test('updates function when using legacy ID and value exists on remote', async () => { - // When - const got = await automaticMatchmaking( - [FUNCTION_A], - [REGISTRATION_FUNCTION_A], - {'function-a': 'LEGACY_FUNCTION_ULID_A'}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'function-a': 'FUNCTION_UUID_A'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - - expect(got).toEqual(expected) - }) - - test('updates function when using legacy UUID and value exists on remote', async () => { - // When - const got = await automaticMatchmaking( - [FUNCTION_A], - [REGISTRATION_FUNCTION_A], - {'function-a': 'LEGACY_FUNCTION_UUID_A'}, - testDeveloperPlatformClient(), - ) - - // Then - const expected = { - identifiers: {'function-a': 'FUNCTION_UUID_A'}, - toConfirm: [], - toCreate: [], - toManualMatch: {local: [], remote: []}, - } - - expect(got).toEqual(expected) - }) - - test('creates local function when it does not exist on remote', async () => { - // When - const got = await automaticMatchmaking([FUNCTION_A], [], {}, testDeveloperPlatformClient()) - - // Then - const expected = { - identifiers: {}, - toConfirm: [], - toCreate: [FUNCTION_A], - toManualMatch: {local: [], remote: []}, - } - expect(got).toEqual(expected) - }) -}) - -describe('automaticMatchmaking: with Atomic Deployments enabled', () => { +describe('automaticMatchmaking', () => { test('creates all local extensions when there are no remote ones', async () => { // When - const got = await automaticMatchmaking( - [EXTENSION_A, EXTENSION_B], - [], - {}, - testDeveloperPlatformClient({supportsAtomicDeployments: true}), - ) + const got = await automaticMatchmaking([EXTENSION_A, EXTENSION_B], [], {}, testDeveloperPlatformClient({})) // Then const expected = { @@ -815,7 +273,7 @@ describe('automaticMatchmaking: with Atomic Deployments enabled', () => { [EXTENSION_A, EXTENSION_A_2], [registrationA], {'extension-a': 'UUID_A'}, - testDeveloperPlatformClient({supportsAtomicDeployments: true}), + testDeveloperPlatformClient({}), ) // Then @@ -852,7 +310,7 @@ describe('outputAddedIDs', () => { 'extension-c': 'UUID_C', 'extension-d': 'UUID_D', }, - testDeveloperPlatformClient({supportsAtomicDeployments: true}), + testDeveloperPlatformClient({}), ) // Then: outputInfo should be called with the expected messages diff --git a/packages/app/src/cli/services/context/id-matching.ts b/packages/app/src/cli/services/context/id-matching.ts index f6546355e10..04660d65571 100644 --- a/packages/app/src/cli/services/context/id-matching.ts +++ b/packages/app/src/cli/services/context/id-matching.ts @@ -212,7 +212,7 @@ export async function automaticMatchmaking( identifiers: IdentifiersExtensions, developerPlatformClient: DeveloperPlatformClient, ): Promise { - const useUuidMatching = developerPlatformClient.supportsAtomicDeployments + const useUuidMatching = true const ids = getExtensionIds(localSources, identifiers) const localIds = Object.values(ids) 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 8686cc8c5df..3b38367b31e 100644 --- a/packages/app/src/cli/services/context/identifiers-extensions.test.ts +++ b/packages/app/src/cli/services/context/identifiers-extensions.test.ts @@ -422,9 +422,9 @@ describe('matchmaking returns ok with pending manual matches', () => { extensions: { EXTENSION_A: 'UUID_A', EXTENSION_A_2: 'UUID_A_2', - 'extension-b': 'UUID_B', + 'extension-b': EXTENSION_B.uid, }, - extensionIds: {EXTENSION_A: 'A', EXTENSION_A_2: 'A_2', 'extension-b': 'B'}, + extensionIds: {EXTENSION_A: 'A', EXTENSION_A_2: 'A_2', 'extension-b': EXTENSION_B.uid}, extensionsNonUuidManaged: {}, }) }) @@ -495,9 +495,9 @@ describe('matchmaking returns ok with pending manual matches and manual match fa expect(got).toEqual({ extensions: { EXTENSION_A: 'UUID_A', - 'extension-a-2': 'UUID_A_3', + 'extension-a-2': EXTENSION_A_2.uid, }, - extensionIds: {EXTENSION_A: 'A', 'extension-a-2': 'A_3'}, + extensionIds: {EXTENSION_A: 'A', 'extension-a-2': EXTENSION_A_2.uid}, extensionsNonUuidManaged: {}, }) }) @@ -561,10 +561,10 @@ describe('matchmaking returns ok with pending some pending to create', () => { ) // Then - expect(developerPlatformClient.createExtension).toBeCalledTimes(2) + expect(developerPlatformClient.createExtension).not.toHaveBeenCalled() expect(got).toEqual({ - extensions: {'extension-a': 'UUID_A', 'extension-a-2': 'UUID_A_2'}, - extensionIds: {'extension-a': 'A', 'extension-a-2': 'A_2'}, + extensions: {'extension-a': EXTENSION_A.uid, 'extension-a-2': EXTENSION_A_2.uid}, + extensionIds: {'extension-a': EXTENSION_A.uid, 'extension-a-2': EXTENSION_A_2.uid}, extensionsNonUuidManaged: {}, }) }) @@ -667,10 +667,10 @@ describe('matchmaking returns ok with some pending confirmation', () => { }) // Then - expect(developerPlatformClient.createExtension).toBeCalledTimes(1) + expect(developerPlatformClient.createExtension).not.toHaveBeenCalled() expect(got).toEqual({ - extensions: {'extension-b': 'UUID_B'}, - extensionIds: {'extension-b': 'B'}, + extensions: {'extension-b': EXTENSION_B.uid}, + extensionIds: {'extension-b': EXTENSION_B.uid}, extensionsNonUuidManaged: {}, }) }) @@ -980,11 +980,11 @@ describe('deployConfirmed: handle non existent uuid managed extensions', () => { }) // Then - expect(developerPlatformClient.createExtension).toBeCalledTimes(1) + expect(developerPlatformClient.createExtension).not.toHaveBeenCalled() expect(got).toEqual({ extensions: {}, - extensionIds: {app_access: 'C_A'}, - extensionsNonUuidManaged: {app_access: 'UUID_C_A'}, + extensionIds: {app_access: CONFIG_A.uid}, + extensionsNonUuidManaged: {app_access: CONFIG_A.uid}, }) }) test('when the include config on deploy flag is disabled but draft extensions should be used configuration extensions are created with context', async () => { @@ -1004,17 +1004,10 @@ describe('deployConfirmed: handle non existent uuid managed extensions', () => { }) // Then - expect(developerPlatformClient.createExtension).toHaveBeenCalledWith({ - apiKey: 'appId', - type: PAYMENTS_A.graphQLType, - config: '{}', - handle: PAYMENTS_A.handle, - title: PAYMENTS_A.handle, - context: 'payments.offsite.render', - }) + expect(developerPlatformClient.createExtension).not.toHaveBeenCalled() expect(got).toEqual({ - extensions: {'payments-extension': 'PAYMENTS_A_UUID'}, - extensionIds: {'payments-extension': 'PAYMENTS_A'}, + extensions: {'payments-extension': PAYMENTS_A.uid}, + extensionIds: {'payments-extension': PAYMENTS_A.uid}, extensionsNonUuidManaged: {}, }) }) @@ -1173,14 +1166,9 @@ describe('ensureNonUuidManagedExtensionsIds: for extensions managed in the TOML' ) // Then - expect(developerPlatformClient.createExtension).toBeCalledTimes(4) - expect(Object.values(extensionsNonUuidManaged)).toEqual([ - 'webhook-subscription-1', - 'webhook-subscription-2', - 'webhook-subscription-3', - 'webhook-subscription-4', - ]) - expect(Object.values(extensionsIdsNonUuidManaged)).toEqual(['1', '2', '3', '4']) + expect(developerPlatformClient.createExtension).not.toHaveBeenCalled() + expect(Object.values(extensionsNonUuidManaged)).toEqual(localSources.map((source) => source.uid)) + expect(Object.values(extensionsIdsNonUuidManaged)).toEqual(localSources.map((source) => source.uid)) }) test('if there are possible matches, creates the extension for those that dont match', async () => { @@ -1250,14 +1238,14 @@ describe('ensureNonUuidManagedExtensionsIds: for extensions managed in the TOML' ) // Then - expect(developerPlatformClient.createExtension).toBeCalledTimes(4) + expect(developerPlatformClient.createExtension).not.toHaveBeenCalled() expect(Object.values(extensionsNonUuidManaged)).toEqual([ 'webhook-subscription-uuid', - 'webhook-subscription-1', - 'webhook-subscription-2', - 'webhook-subscription-3', - 'webhook-subscription-4', + ...localSources.slice(1).map((source) => source.uid), + ]) + expect(Object.values(extensionsIdsNonUuidManaged)).toEqual([ + 'webhook-subscription-id', + ...localSources.slice(1).map((source) => source.uid), ]) - expect(Object.values(extensionsIdsNonUuidManaged)).toEqual(['webhook-subscription-id', '1', '2', '3', '4']) }) }) diff --git a/packages/app/src/cli/services/context/identifiers-extensions.ts b/packages/app/src/cli/services/context/identifiers-extensions.ts index 8a8887f93d4..b657c666659 100644 --- a/packages/app/src/cli/services/context/identifiers-extensions.ts +++ b/packages/app/src/cli/services/context/identifiers-extensions.ts @@ -2,7 +2,6 @@ import {manualMatchIds} from './id-manual-matching.js' import {automaticMatchmaking} from './id-matching.js' import {EnsureDeploymentIdsPresenceOptions, LocalSource, RemoteSource} from './identifiers.js' import {extensionMigrationPrompt, matchConfirmationPrompt} from './prompts.js' -import {createExtension} from '../dev/create-extension.js' import {IdentifiersExtensions} from '../../models/app/identifiers.js' import {migrateExtensionsToUIExtension} from '../dev/migrate-to-ui-extension.js' import {migrateFlowExtensions} from '../dev/migrate-flow-extension.js' @@ -22,7 +21,6 @@ import {ExtensionSpecification} from '../../models/extensions/specification.js' import {ExtensionInstance} from '../../models/extensions/extension-instance.js' import {SingleWebhookSubscriptionType} from '../../models/extensions/specifications/app_config_webhook_schemas/webhooks_schema.js' import {PartnersClient} from '../../utilities/developer-platform-client/partners-client.js' -import {outputCompleted} from '@shopify/cli-kit/node/output' import {AbortSilentError} from '@shopify/cli-kit/node/error' import {groupBy} from '@shopify/cli-kit/common/collection' @@ -209,7 +207,7 @@ export async function deployConfirmed( const validMatchesById: {[key: string]: string} = {} if (extensionsToCreate.length > 0) { - const newIdentifiers = await createExtensions(extensionsToCreate, options.appId, options.developerPlatformClient) + const newIdentifiers = await createExtensions(extensionsToCreate) for (const [localIdentifier, registration] of Object.entries(newIdentifiers)) { validMatches[localIdentifier] = registration.uuid validMatchesById[localIdentifier] = registration.id @@ -273,7 +271,7 @@ function loadExtensionIds( export async function ensureNonUuidManagedExtensionsIds( remoteConfigurationRegistrations: RemoteSource[], app: AppInterface, - appId: string, + _appId: string, includeDraftExtensions = false, developerPlatformClient: DeveloperPlatformClient, ) { @@ -295,7 +293,7 @@ export async function ensureNonUuidManagedExtensionsIds( ) if (extensionsToCreate.length > 0) { - const newIdentifiers = await createExtensions(extensionsToCreate, appId, developerPlatformClient, false) + const newIdentifiers = await createExtensions(extensionsToCreate) for (const [localIdentifier, registration] of Object.entries(newIdentifiers)) { validMatches[localIdentifier] = registration.uuid validMatchesById[localIdentifier] = registration.id @@ -305,35 +303,14 @@ export async function ensureNonUuidManagedExtensionsIds( return {extensionsNonUuidManaged: validMatches, extensionsIdsNonUuidManaged: validMatchesById} } -async function createExtensions( - extensions: LocalSource[], - appId: string, - developerPlatformClient: DeveloperPlatformClient, - output = true, -) { +async function createExtensions(extensions: LocalSource[]) { const result: {[localIdentifier: string]: RemoteSource} = {} for (const extension of extensions) { - if (developerPlatformClient.supportsAtomicDeployments) { - // Just pretend to create the extension, as it's not necessary to do anything - // in this case. - result[extension.localIdentifier] = { - id: extension.uid, - uuid: extension.uid, - type: extension.type, - title: extension.handle, - } - } else { - // Create one at a time to avoid API rate limiting issues. - // eslint-disable-next-line no-await-in-loop - const registration = await createExtension( - appId, - extension.graphQLType, - extension.handle, - developerPlatformClient, - extension.contextValue, - ) - if (output) outputCompleted(`Created extension ${extension.handle}.`) - result[extension.localIdentifier] = registration + result[extension.localIdentifier] = { + id: extension.uid, + uuid: extension.uid, + type: extension.type, + title: extension.handle, } } return result diff --git a/packages/app/src/cli/services/deploy.ts b/packages/app/src/cli/services/deploy.ts index 2f9d31a27ba..33dffd2b727 100644 --- a/packages/app/src/cli/services/deploy.ts +++ b/packages/app/src/cli/services/deploy.ts @@ -228,7 +228,7 @@ export async function deploy(options: DeployOptions) { bundlePath: candidateBundlePath, identifiers, skipBuild: options.skipBuild, - isDevDashboardApp: developerPlatformClient.supportsAtomicDeployments, + isDevDashboardApp: true, }) let uploadTaskTitle @@ -271,7 +271,7 @@ export async function deploy(options: DeployOptions) { commitReference: options.commitReference, }) - await updateAppIdentifiers({app, identifiers, command: 'deploy', developerPlatformClient}) + await updateAppIdentifiers({app, identifiers, command: 'deploy'}) }, }, ] @@ -292,7 +292,7 @@ export async function deploy(options: DeployOptions) { * If deployment fails when uploading we want the identifiers to be persisted * for the next run. */ - await updateAppIdentifiers({app, identifiers, command: 'deploy', developerPlatformClient}) + await updateAppIdentifiers({app, identifiers, command: 'deploy'}) throw error } diff --git a/packages/app/src/cli/services/import-extensions.ts b/packages/app/src/cli/services/import-extensions.ts index 9c72206eaf1..a1429f81ec7 100644 --- a/packages/app/src/cli/services/import-extensions.ts +++ b/packages/app/src/cli/services/import-extensions.ts @@ -79,7 +79,7 @@ async function handleExtensionDirectory({ } export async function importExtensions(options: ImportOptions) { - const {app, remoteApp, developerPlatformClient, extensionTypes, extensions, buildExtensionConfig, all} = options + const {app, remoteApp, extensionTypes, extensions, buildExtensionConfig, all} = options let extensionsToMigrate = extensions.filter((ext) => extensionTypes.includes(ext.type.toLowerCase())) extensionsToMigrate = filterOutImportedExtensions(app, extensionsToMigrate) @@ -131,7 +131,6 @@ export async function importExtensions(options: ImportOptions) { app: remoteApp.apiKey, }, command: 'import-extensions', - developerPlatformClient, }) } diff --git a/packages/app/src/cli/utilities/developer-platform-client.ts b/packages/app/src/cli/utilities/developer-platform-client.ts index 3262fa14c36..05e8edefbb4 100644 --- a/packages/app/src/cli/utilities/developer-platform-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client.ts @@ -222,7 +222,6 @@ export interface TemplateSpecificationsOptions { export interface DeveloperPlatformClient { readonly clientName: ClientName readonly webUiName: string - readonly supportsAtomicDeployments: boolean readonly organizationSource: OrganizationSource readonly bundleFormat: 'zip' | 'br' readonly supportsDashboardManagedExtensions: boolean diff --git a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts index 3cf7f04814b..e3e9c353b67 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts @@ -192,7 +192,6 @@ export class AppManagementClient implements DeveloperPlatformClient { public readonly clientName = ClientName.AppManagement public readonly webUiName = 'Developer Dashboard' - public readonly supportsAtomicDeployments = true public readonly organizationSource = OrganizationSource.BusinessPlatform public readonly bundleFormat = 'br' public readonly supportsDashboardManagedExtensions = false diff --git a/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts b/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts index b247216bbd6..cfef1c6e64e 100644 --- a/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts +++ b/packages/app/src/cli/utilities/developer-platform-client/partners-client.ts @@ -198,7 +198,6 @@ export class PartnersClient implements DeveloperPlatformClient { public readonly clientName = ClientName.Partners public readonly webUiName = 'Partner Dashboard' - public readonly supportsAtomicDeployments = false public readonly organizationSource = OrganizationSource.Partners public readonly bundleFormat = 'zip' public readonly supportsDashboardManagedExtensions = true