diff --git a/packages/app/src/cli/models/app/identifiers.ts b/packages/app/src/cli/models/app/identifiers.ts index 44c8334385e..82411a9a3cd 100644 --- a/packages/app/src/cli/models/app/identifiers.ts +++ b/packages/app/src/cli/models/app/identifiers.ts @@ -10,7 +10,7 @@ export interface IdentifiersExtensions { [localIdentifier: string]: string } -export interface Identifiers { +interface Identifiers { /** Application's API Key */ app: string diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts index c75371302b0..d3cde5781c9 100644 --- a/packages/app/src/cli/services/context.test.ts +++ b/packages/app/src/cli/services/context.test.ts @@ -1,7 +1,7 @@ import {fetchOrganizations, fetchOrgFromId} from './dev/fetch.js' import {selectOrCreateApp} from './dev/select-app.js' import {selectStore} from './dev/select-store.js' -import {ensureDeploymentIdsPresence} from './context/identifiers.js' +import {ensureDeployIdentifiersFromAppVersion} from './context/deploy-app-version.js' import {appFromIdentifiers, ensureDeployContext} from './context.js' import {CachedAppInfo} from './local-storage.js' import link from './app/config/link.js' @@ -110,7 +110,7 @@ vi.mock('./dev/select-store') vi.mock('../prompts/dev') vi.mock('@shopify/organizations') vi.mock('../models/app/identifiers') -vi.mock('./context/identifiers') +vi.mock('./context/deploy-app-version') vi.mock('../models/app/loader.js') vi.mock('@shopify/cli-kit/node/node-package-manager.js') vi.mock('@shopify/cli-kit/node/ui') @@ -174,13 +174,7 @@ describe('ensureDeployContext', () => { 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 = { - app: APP2.apiKey, - extensions: {}, - extensionIds: {}, - extensionsNonUuidManaged: {}, - } - vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) + vi.mocked(ensureDeployIdentifiersFromAppVersion).mockResolvedValue(emptyDeployIdentifiers()) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') const options = { @@ -196,13 +190,7 @@ describe('ensureDeployContext', () => { 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}}}) - const identifiers = { - app: APP2.apiKey, - extensions: {}, - extensionIds: {}, - extensionsNonUuidManaged: {}, - } - vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) + vi.mocked(ensureDeployIdentifiersFromAppVersion).mockResolvedValue(emptyDeployIdentifiers()) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') mockTomlFileRemove.mockResolvedValue(undefined) @@ -238,13 +226,7 @@ describe('ensureDeployContext', () => { test('removes the include_config_on_deploy field when using app management API and the value 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(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) + vi.mocked(ensureDeployIdentifiersFromAppVersion).mockResolvedValue(emptyDeployIdentifiers()) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') mockTomlFileRemove.mockResolvedValue(undefined) @@ -280,13 +262,7 @@ describe('ensureDeployContext', () => { test('sets didMigrateExtensionsToDevDash to true when app modules are missing registration IDs', 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(ensureDeployIdentifiersFromAppVersion).mockResolvedValue(emptyDeployIdentifiers()) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') const activeAppVersion = { @@ -325,13 +301,7 @@ describe('ensureDeployContext', () => { test('sets didMigrateExtensionsToDevDash to false when all app modules have registration IDs', 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(ensureDeployIdentifiersFromAppVersion).mockResolvedValue(emptyDeployIdentifiers()) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') const activeAppVersion = { @@ -422,6 +392,10 @@ describe('appFromIdentifiers', () => { }) }) +function emptyDeployIdentifiers() { + return {appModuleUuids: {}, appModuleRegistrationIds: {}} +} + const renderTryMessage = (isOrg: boolean, identifier: string) => [ { list: { diff --git a/packages/app/src/cli/services/context.ts b/packages/app/src/cli/services/context.ts index d2d81730080..8f7373f83c5 100644 --- a/packages/app/src/cli/services/context.ts +++ b/packages/app/src/cli/services/context.ts @@ -1,11 +1,11 @@ import {selectOrCreateApp} from './dev/select-app.js' import {fetchOrganizations, fetchOrgFromId} from './dev/fetch.js' -import {ensureDeploymentIdsPresence} from './context/identifiers.js' +import {ensureDeployIdentifiersFromAppVersion} from './context/deploy-app-version.js' import {CachedAppInfo} from './local-storage.js' import {DeployOptions} from './deploy.js' import {formatConfigInfoBody} from './format-config-info-body.js' import {AppInterface, AppLinkedInterface} from '../models/app/app.js' -import {Identifiers, updateAppIdentifiers, getAppIdentifiers} from '../models/app/identifiers.js' +import {DeployIdentifiers, getAppIdentifiers} from '../models/app/identifiers.js' import {Organization, OrganizationApp, OrganizationSource, OrganizationStore} from '../models/organization.js' import metadata from '../metadata.js' import {getAppConfigurationFileName} from '../models/app/loader.js' @@ -101,7 +101,7 @@ export const appFromIdentifiers = async (options: AppFromIdOptions): Promise !version.registrationId) } - return {identifiers, didMigrateExtensionsToDevDash} + return {deployIdentifiers, didMigrateExtensionsToDevDash} } async function removeIncludeConfigOnDeployField(localApp: AppInterface) { diff --git a/packages/app/src/cli/services/deploy.test.ts b/packages/app/src/cli/services/deploy.test.ts index 0af16d422c3..2a06642566e 100644 --- a/packages/app/src/cli/services/deploy.test.ts +++ b/packages/app/src/cli/services/deploy.test.ts @@ -17,7 +17,6 @@ import { testOrganization, testProject, } from '../models/app/app.test-data.js' -import {updateAppIdentifiers} from '../models/app/identifiers.js' import {AppInterface, AppLinkedInterface} from '../models/app/app.js' import {OrganizationApp} from '../models/organization.js' import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' @@ -52,7 +51,6 @@ vi.mock('./context.js') vi.mock('./deploy/upload.js') vi.mock('./deploy/bundle.js') vi.mock('./dev/fetch.js') -vi.mock('../models/app/identifiers.js') vi.mock('@shopify/cli-kit/node/context/local') vi.mock('@shopify/cli-kit/node/ui') vi.mock('@shopify/cli-kit/node/crypto') @@ -116,7 +114,7 @@ describe('deploy', () => { organizationId: 'org-id', appModules: [], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, }) }) @@ -186,11 +184,10 @@ describe('deploy', () => { organizationId: 'org-id', appModules: [], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, }) expect(bundleAndBuildExtensions).toHaveBeenCalledOnce() - expect(updateAppIdentifiers).toHaveBeenCalledOnce() }) test('uploads the extension bundle with 1 UI extension', async () => { @@ -234,11 +231,10 @@ describe('deploy', () => { }, ], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, }) expect(bundleAndBuildExtensions).toHaveBeenCalledOnce() - expect(updateAppIdentifiers).toHaveBeenCalledOnce() }) test('uploads the extension bundle with 1 theme extension', async () => { @@ -282,11 +278,10 @@ describe('deploy', () => { }, ], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, }) expect(bundleAndBuildExtensions).toHaveBeenCalledOnce() - expect(updateAppIdentifiers).toHaveBeenCalledOnce() }) test('uploads the extension bundle with 1 function extension', async () => { @@ -347,12 +342,11 @@ describe('deploy', () => { }, ], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, bundlePath: expect.stringMatching(/bundle.zip$/), release: true, }) expect(bundleAndBuildExtensions).toHaveBeenCalledOnce() - expect(updateAppIdentifiers).toHaveBeenCalledOnce() }) test('uploads the extension bundle with 1 UI and 1 theme extension', async () => { @@ -415,12 +409,11 @@ describe('deploy', () => { }, ], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, commitReference, }) expect(bundleAndBuildExtensions).toHaveBeenCalledOnce() - expect(updateAppIdentifiers).toHaveBeenCalledOnce() }) test('pushes the configuration extension if include config on deploy ', async () => { @@ -468,12 +461,11 @@ describe('deploy', () => { }, ], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, commitReference, }) expect(bundleAndBuildExtensions).toHaveBeenCalledOnce() - expect(updateAppIdentifiers).toHaveBeenCalledOnce() }) test('doesnt push the configuration extension if include config on deploy is disabled', async () => { @@ -512,12 +504,11 @@ describe('deploy', () => { organizationId: 'org-id', appModules: [], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, commitReference, }) expect(bundleAndBuildExtensions).toHaveBeenCalledOnce() - expect(updateAppIdentifiers).toHaveBeenCalledOnce() }) test('shows a success message', async () => { @@ -712,22 +703,13 @@ async function testDeployBundle({ didMigrateExtensionsToDevDash = false, }: TestDeployBundleInput) { // Given - const extensionsPayload: {[key: string]: string} = {} - for (const extension of app.allExtensions.filter((ext) => ext.isUUIDStrategyExtension)) { - extensionsPayload[extension.localIdentifier] = extension.localIdentifier - } - const extensionsNonUuidPayload: {[key: string]: string} = {} - for (const extension of app.allExtensions.filter((ext) => !ext.isUUIDStrategyExtension)) { - extensionsNonUuidPayload[extension.localIdentifier] = extension.localIdentifier - } - const identifiers = { - app: 'app-id', - extensions: extensionsPayload, - extensionIds: {}, - extensionsNonUuidManaged: extensionsNonUuidPayload, + const appModuleUuids: {[key: string]: string} = {} + for (const extension of app.allExtensions) { + appModuleUuids[extension.localIdentifier] = extension.localIdentifier } + const deployIdentifiers = {appModuleUuids, appModuleRegistrationIds: {}} - vi.mocked(ensureDeployContext).mockResolvedValue({identifiers, didMigrateExtensionsToDevDash}) + vi.mocked(ensureDeployContext).mockResolvedValue({deployIdentifiers, didMigrateExtensionsToDevDash}) vi.mocked(uploadExtensionsBundle).mockResolvedValue({ validationErrors: [], @@ -736,7 +718,6 @@ async function testDeployBundle({ ...(!released && {deployError: 'no release error'}), location: 'https://partners.shopify.com/0/apps/0/versions/1', }) - vi.mocked(updateAppIdentifiers).mockResolvedValue(app) await deploy({ app, diff --git a/packages/app/src/cli/services/deploy.ts b/packages/app/src/cli/services/deploy.ts index 758fdb83a8f..02bc9fb7d35 100644 --- a/packages/app/src/cli/services/deploy.ts +++ b/packages/app/src/cli/services/deploy.ts @@ -6,7 +6,6 @@ import {allExtensionTypes, filterOutImportedExtensions, importAllExtensions} fro import {getExtensions} from './fetch-extensions.js' import {AppLinkedInterface} from '../models/app/app.js' import {Project} from '../models/project/project.js' -import {updateAppIdentifiers} from '../models/app/identifiers.js' import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' import {Organization, OrganizationApp} from '../models/organization.js' import {reloadApp} from '../models/app/loader.js' @@ -148,7 +147,7 @@ export async function deploy(options: DeployOptions) { allowDeletes, }) - const {identifiers, didMigrateExtensionsToDevDash} = await ensureDeployContext({ + const {deployIdentifiers, didMigrateExtensionsToDevDash} = await ensureDeployContext({ ...options, app, developerPlatformClient, @@ -156,8 +155,6 @@ export async function deploy(options: DeployOptions) { allowDeletes, }) - const appModuleUuids = {...identifiers.extensions, ...identifiers.extensionsNonUuidManaged} - const release = !noRelease const apiKey = remoteApp.apiKey @@ -172,87 +169,79 @@ export async function deploy(options: DeployOptions) { let uploadExtensionsBundleResult!: UploadExtensionsBundleOutput - try { - const candidateBundlePath = joinPath( - options.app.directory, - '.shopify', - `deploy-bundle.${developerPlatformClient.bundleFormat}`, - ) - await mkdir(dirname(candidateBundlePath)) + const candidateBundlePath = joinPath( + options.app.directory, + '.shopify', + `deploy-bundle.${developerPlatformClient.bundleFormat}`, + ) + await mkdir(dirname(candidateBundlePath)) - const appManifest = await app.manifest(appManifestUuids(app, appModuleUuids)) + const appManifest = await app.manifest(appManifestUuids(app, deployIdentifiers.appModuleUuids)) - const bundlePath = await bundleAndBuildExtensions({ - app, - appManifest, - bundlePath: candidateBundlePath, - skipBuild: options.skipBuild, - }) + const bundlePath = await bundleAndBuildExtensions({ + app, + appManifest, + bundlePath: candidateBundlePath, + skipBuild: options.skipBuild, + }) - let uploadTaskTitle + let uploadTaskTitle - if (release) { - uploadTaskTitle = 'Releasing an app version' - } else { - uploadTaskTitle = 'Creating an app version' - } + if (release) { + uploadTaskTitle = 'Releasing an app version' + } else { + uploadTaskTitle = 'Creating an app version' + } - const tasks: Task[] = [ - { - title: 'Running validation', - task: async () => { - await app.preDeployValidation() - }, + const tasks: Task[] = [ + { + title: 'Running validation', + task: async () => { + await app.preDeployValidation() }, - { - title: uploadTaskTitle, - task: async () => { - const appModules = await Promise.all( - app.allExtensions.flatMap((ext) => - ext.bundleConfig({appModuleUuids, developerPlatformClient, apiKey, appConfiguration: app.configuration}), - ), - ) - - uploadExtensionsBundleResult = await uploadExtensionsBundle({ - appManifest, - appId: remoteApp.id, - apiKey, - name: app.name, - organizationId: remoteApp.organizationId, - bundlePath, - appModules: getArrayRejectingUndefined(appModules), - release, - developerPlatformClient, - extensionIds: identifiers.extensionIds, - message: options.message, - version: options.version, - commitReference: options.commitReference, - }) - - await updateAppIdentifiers({app, identifiers, command: 'deploy'}) - }, + }, + { + title: uploadTaskTitle, + task: async () => { + const appModules = await Promise.all( + app.allExtensions.flatMap((ext) => + ext.bundleConfig({ + appModuleUuids: deployIdentifiers.appModuleUuids, + developerPlatformClient, + apiKey, + appConfiguration: app.configuration, + }), + ), + ) + + uploadExtensionsBundleResult = await uploadExtensionsBundle({ + appManifest, + appId: remoteApp.id, + apiKey, + name: app.name, + organizationId: remoteApp.organizationId, + bundlePath, + appModules: getArrayRejectingUndefined(appModules), + release, + developerPlatformClient, + appModuleRegistrationIds: deployIdentifiers.appModuleRegistrationIds, + message: options.message, + version: options.version, + commitReference: options.commitReference, + }) }, - ] - - await renderTasks(tasks) + }, + ] - await outputCompletionMessage({ - app, - project: options.project, - release, - uploadExtensionsBundleResult, - didMigrateExtensionsToDevDash, - }) + await renderTasks(tasks) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - } catch (error: any) { - /** - * If deployment fails when uploading we want the identifiers to be persisted - * for the next run. - */ - await updateAppIdentifiers({app, identifiers, command: 'deploy'}) - throw error - } + await outputCompletionMessage({ + app, + project: options.project, + release, + uploadExtensionsBundleResult, + didMigrateExtensionsToDevDash, + }) return {app} } diff --git a/packages/app/src/cli/services/deploy/upload.test.ts b/packages/app/src/cli/services/deploy/upload.test.ts index fab9ca04a84..c01e3154c8f 100644 --- a/packages/app/src/cli/services/deploy/upload.test.ts +++ b/packages/app/src/cli/services/deploy/upload.test.ts @@ -39,7 +39,7 @@ describe('uploadExtensionsBundle', () => { {uuid: '123', config: '{}', context: '', handle: 'handle', specificationIdentifier: 'ui_extension'}, ], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, }) @@ -85,7 +85,7 @@ describe('uploadExtensionsBundle', () => { {uuid: '123', config: '{}', context: '', handle: 'handle', specificationIdentifier: 'ui_extension'}, ], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, message: 'test', version: '1.0.0', @@ -129,7 +129,7 @@ describe('uploadExtensionsBundle', () => { bundlePath: undefined, appModules: [], developerPlatformClient, - extensionIds: {}, + appModuleRegistrationIds: {}, release: true, }) @@ -247,7 +247,7 @@ describe('uploadExtensionsBundle', () => { {uuid: '456', config: '{}', context: '', handle: 'handle', specificationIdentifier: 'ui_extension'}, ], developerPlatformClient, - extensionIds: { + appModuleRegistrationIds: { 'amortizable-marketplace-ext': '123', 'amortizable-marketplace-ext-2': '456', }, @@ -350,7 +350,7 @@ describe('uploadExtensionsBundle', () => { {uuid: '456', config: '{}', context: '', handle: 'handle', specificationIdentifier: 'ui_extension'}, ], developerPlatformClient, - extensionIds: { + appModuleRegistrationIds: { 'amortizable-marketplace-ext': '123', 'amortizable-marketplace-ext-2': '456', }, diff --git a/packages/app/src/cli/services/deploy/upload.ts b/packages/app/src/cli/services/deploy/upload.ts index 807d353c733..b0c22decfc2 100644 --- a/packages/app/src/cli/services/deploy/upload.ts +++ b/packages/app/src/cli/services/deploy/upload.ts @@ -1,4 +1,4 @@ -import {IdentifiersExtensions} from '../../models/app/identifiers.js' +import {ExtensionUuidsByLocalIdentifier} from '../../models/app/identifiers.js' import {AppDeploySchema, AppModuleSettings} from '../../api/graphql/app_deploy.js' import {AppDeployOptions, DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' @@ -32,8 +32,7 @@ interface UploadExtensionsBundleOptions { /** App Modules extra data */ appModules: AppModuleSettings[] - /** The extensions' numeric identifiers (expressed as a string). */ - extensionIds: IdentifiersExtensions + appModuleRegistrationIds: ExtensionUuidsByLocalIdentifier /** Wether or not to release the version */ release: boolean @@ -114,7 +113,7 @@ export async function uploadExtensionsBundle( if (!result.appDeploy.appVersion) { const customSections: AlertCustomSection[] = deploymentErrorsToCustomSections( result.appDeploy.userErrors ?? [], - options.extensionIds, + options.appModuleRegistrationIds, options.appModules, { version: options.version, @@ -147,7 +146,7 @@ const GENERIC_ERRORS_TITLE = '\n' export function deploymentErrorsToCustomSections( errors: AppDeploySchema['appDeploy']['userErrors'], - extensionIds: IdentifiersExtensions, + appModuleRegistrationIds: ExtensionUuidsByLocalIdentifier, appModules: AppModuleSettings[], flags: { version?: string @@ -157,11 +156,11 @@ export function deploymentErrorsToCustomSections( return error.details?.some((detail) => detail.extension_id) ?? false } - const isCliError = (error: (typeof errors)[0], extensionIds: IdentifiersExtensions) => { + const isCliError = (error: (typeof errors)[0], appModuleRegistrationIds: ExtensionUuidsByLocalIdentifier) => { const errorExtensionId = error.details?.find((detail) => typeof detail.extension_id !== 'undefined')?.extension_id ?? '' - return Object.values(extensionIds).includes(errorExtensionId.toString()) + return Object.values(appModuleRegistrationIds).includes(errorExtensionId.toString()) } const isAppManagementValidationError = (error: (typeof errors)[0]) => { @@ -173,11 +172,11 @@ export function deploymentErrorsToCustomSections( const [extensionErrors, nonExtensionErrors] = partition(nonAppManagementErrors, (error) => isExtensionError(error)) - const [cliErrors, partnersErrors] = partition(extensionErrors, (error) => isCliError(error, extensionIds)) + const [cliErrors, partnersErrors] = partition(extensionErrors, (error) => isCliError(error, appModuleRegistrationIds)) const customSections = [ ...generalErrorsSection(nonExtensionErrors, {version: flags.version}), - ...cliErrorsSections(cliErrors, extensionIds), + ...cliErrorsSections(cliErrors, appModuleRegistrationIds), ...partnersErrorsSections(partnersErrors), ...appManagementErrorsSection(appManagementErrors, appModules), ] @@ -228,7 +227,10 @@ function generalErrorsSection( } } -function cliErrorsSections(errors: AppDeploySchema['appDeploy']['userErrors'], identifiers: IdentifiersExtensions) { +function cliErrorsSections( + errors: AppDeploySchema['appDeploy']['userErrors'], + identifiers: ExtensionUuidsByLocalIdentifier, +) { return errors.reduce((sections, error) => { const field = (error.field ?? ['unknown']).join('.').replace('extension_points', 'extensions.targeting') const errorMessage = field === 'base' ? error.message : `${field}: ${error.message}`