Switch deploy to the active-app-version classification

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
 

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: {

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<Org
 }
 
 interface EnsureDeployContextResult {
-  identifiers: Identifiers
+  deployIdentifiers: DeployIdentifiers
   didMigrateExtensionsToDevDash: boolean
 }
 
@@ -131,7 +131,7 @@ export async function ensureDeployContext(options: DeployOptions): Promise<Ensur
     messages: [resetHelpMessage],
   })
 
-  const identifiers = await ensureDeploymentIdsPresence({
+  const deployIdentifiers = await ensureDeployIdentifiersFromAppVersion({
     app,
     appId: remoteApp.apiKey,
     appName: remoteApp.title,
@@ -144,15 +144,13 @@ export async function ensureDeployContext(options: DeployOptions): Promise<Ensur
     allowDeletes: options.allowDeletes,
   })
 
-  await updateAppIdentifiers({app, identifiers, command: 'deploy'})
-
   // if the current active app version is missing user_identifiers in some app module, then we are migrating to dev dash
   let didMigrateExtensionsToDevDash = false
   if (activeAppVersion) {
     didMigrateExtensionsToDevDash = activeAppVersion.appModuleVersions.some((version) => !version.registrationId)
   }
 
-  return {identifiers, didMigrateExtensionsToDevDash}
+  return {deployIdentifiers, didMigrateExtensionsToDevDash}
 }
 
 async function removeIncludeConfigOnDeployField(localApp: AppInterface) {

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,