Merge pull request #6105 from Shopify/07-14-send_uuid_on_app_version_create_to_do_app_matching

Send uuid on app version create to do app matching

Author: alfonso-noriega

packages/app/src/cli/models/app/app.test.ts

@@ -616,7 +616,12 @@ describe('manifest', () => {
     })
 
     // When
-    const manifest = await app.manifest()
+    const manifest = await app.manifest({
+      app: 'API_KEY',
+      extensions: {app_access: 'UUID_A'},
+      extensionIds: {},
+      extensionsNonUuidManaged: {},
+    })
 
     // Then
     expect(manifest).toEqual({
@@ -627,6 +632,7 @@ describe('manifest', () => {
           type: 'app_access_external',
           handle: 'app_access',
           uid: appAccessModule.uid,
+          uuid: 'UUID_A',
           assets: appAccessModule.uid,
           target: appAccessModule.contextValue,
           config: expect.objectContaining({
@@ -664,7 +670,7 @@ describe('manifest', () => {
     })
 
     // When
-    const manifest = await app.manifest()
+    const manifest = await app.manifest(undefined)
 
     // Then
     expect(manifest).toEqual({
@@ -722,7 +728,7 @@ describe('manifest', () => {
     })
 
     // When
-    const manifest = await app.manifest()
+    const manifest = await app.manifest(undefined)
 
     // Then
     expect(manifest).toEqual({

packages/app/src/cli/models/app/app.ts

@@ -1,6 +1,7 @@
 /* eslint-disable @typescript-eslint/no-non-null-assertion */
 import {AppErrors, isWebType} from './loader.js'
 import {ensurePathStartsWithSlash} from './validation/common.js'
+import {Identifiers} from './identifiers.js'
 import {ExtensionInstance} from '../extensions/extension-instance.js'
 import {isType} from '../../utilities/types.js'
 import {FunctionConfigType} from '../extensions/specifications/function.js'
@@ -313,7 +314,7 @@ export interface AppInterface<
    * If creating an app on the platform based on this app and its configuration, what default options should the app take?
    */
   creationDefaultOptions(): CreateAppOptions
-  manifest: () => Promise<AppManifest>
+  manifest: (identifiers: Identifiers | undefined) => Promise<AppManifest>
   removeExtension: (extensionUid: string) => void
   updateHiddenConfig: (values: Partial<AppHiddenConfig>) => Promise<void>
   setDevApplicationURLs: (devApplicationURLs: ApplicationURLs) => void
@@ -415,7 +416,7 @@ export class App<
     this.realExtensions.forEach((ext) => ext.patchWithAppDevURLs(devApplicationURLs))
   }
 
-  async manifest(): Promise<AppManifest> {
+  async manifest(identifiers: Identifiers | undefined): Promise<AppManifest> {
     const modules = await Promise.all(
       this.realExtensions.map(async (module) => {
         const config = await module.deployConfig({
@@ -426,6 +427,7 @@ export class App<
           type: module.externalType,
           handle: module.handle,
           uid: module.uid,
+          uuid: identifiers?.extensions[module.localIdentifier],
           assets: module.uid,
           target: module.contextValue,
           config: (config ?? {}) as JsonMapType,

packages/app/src/cli/models/app/identifiers.test.ts

@@ -208,12 +208,9 @@ describe('getAppIdentifiers', () => {
       })
 
       // When
-      const got = getAppIdentifiers(
-        {
-          app,
-        },
-        testDeveloperPlatformClient(),
-      )
+      const got = getAppIdentifiers({
+        app,
+      })
 
       // Then
       expect(got.app).toEqual('FOO')
@@ -238,7 +235,6 @@ describe('getAppIdentifiers', () => {
         {
           app,
         },
-        testDeveloperPlatformClient(),
         {SHOPIFY_API_KEY: 'FOO', SHOPIFY_TEST_UI_EXTENSION_ID: 'BAR'},
       )
 
@@ -247,34 +243,4 @@ describe('getAppIdentifiers', () => {
       expect((got.extensions ?? {})['test-ui-extension']).toEqual('BAR')
     })
   })
-
-  test('returns the UIDs when Atomic Deployments is enabled', async () => {
-    await inTemporaryDirectory(async (tmpDir: string) => {
-      // Given
-      const uiExtension = await testUIExtension({
-        directory: '/tmp/project/extensions/my-extension',
-        idEnvironmentVariableName: 'SHOPIFY_MY_EXTENSION_ID',
-      })
-      const app = testApp({
-        directory: tmpDir,
-        dotenv: {
-          path: joinPath(tmpDir, '.env'),
-          variables: {SHOPIFY_API_KEY: 'FOO', SHOPIFY_TEST_UI_EXTENSION_ID: 'BAR'},
-        },
-        allExtensions: [uiExtension],
-      })
-
-      // When
-      const got = getAppIdentifiers(
-        {
-          app,
-        },
-        testDeveloperPlatformClient({supportsAtomicDeployments: true}),
-      )
-
-      // Then
-      expect(got.app).toEqual('FOO')
-      expect((got.extensions ?? {})['test-ui-extension']).toEqual(uiExtension.uid)
-    })
-  })
 })

packages/app/src/cli/models/app/identifiers.ts

@@ -47,7 +47,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}: UpdateAppIdentifiersOptions,
+  {app, identifiers, command, developerPlatformClient}: UpdateAppIdentifiersOptions,
   systemEnvironment = process.env,
 ): Promise<AppInterface> {
   let dotenvFile = app.dotenv
@@ -71,7 +71,10 @@ export async function updateAppIdentifiers(
   })
 
   const contentIsEqual = deepCompare(dotenvFile.variables, updatedVariables)
-  const writeToFile = !contentIsEqual && (command === 'deploy' || command === 'release')
+  const writeToFile =
+    !contentIsEqual &&
+    (command === 'deploy' || command === 'release') &&
+    !developerPlatformClient.supportsAtomicDeployments
   dotenvFile.variables = updatedVariables
 
   if (writeToFile) {
@@ -95,7 +98,6 @@ interface GetAppIdentifiersOptions {
  */
 export function getAppIdentifiers(
   {app}: GetAppIdentifiersOptions,
-  developerPlatformClient: DeveloperPlatformClient,
   systemEnvironment = process.env,
 ): Partial<UuidOnlyIdentifiers> {
   const envVariables = {
@@ -108,9 +110,6 @@ export function getAppIdentifiers(
       // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
       extensionsIdentifiers[extension.localIdentifier] = envVariables[extension.idEnvironmentVariableName]!
     }
-    if (developerPlatformClient.supportsAtomicDeployments) {
-      extensionsIdentifiers[extension.localIdentifier] = extension.uid
-    }
   }
   app.allExtensions.forEach(processExtension)
 

packages/app/src/cli/services/bundle.test.ts

@@ -25,7 +25,7 @@ describe('writeManifestToBundle', () => {
       } as unknown as AppInterface
 
       // When
-      await writeManifestToBundle(mockApp, tmpDir)
+      await writeManifestToBundle(mockApp, tmpDir, undefined)
 
       // Then
       const manifestPath = joinPath(tmpDir, 'manifest.json')

packages/app/src/cli/services/bundle.ts

@@ -2,15 +2,20 @@
 import {AppInterface} from '../models/app/app.js'
 import {AssetUrlSchema, DeveloperPlatformClient} from '../utilities/developer-platform-client.js'
 import {MinimalAppIdentifiers} from '../models/organization.js'
+import {Identifiers} from '../models/app/identifiers.js'
 import {joinPath} from '@shopify/cli-kit/node/path'
 import {brotliCompress, zip} from '@shopify/cli-kit/node/archiver'
 import {formData, fetch} from '@shopify/cli-kit/node/http'
 import {readFileSync} from '@shopify/cli-kit/node/fs'
 import {AbortError} from '@shopify/cli-kit/node/error'
 import {writeFile} from 'fs/promises'
 
-export async function writeManifestToBundle(app: AppInterface, bundlePath: string) {
-  const appManifest = await app.manifest()
+export async function writeManifestToBundle(
+  app: AppInterface,
+  bundlePath: string,
+  identifiers: Identifiers | undefined,
+) {
+  const appManifest = await app.manifest(identifiers)
   const manifestPath = joinPath(bundlePath, 'manifest.json')
   await writeFile(manifestPath, JSON.stringify(appManifest, null, 2))
 }

packages/app/src/cli/services/context.ts

@@ -153,7 +153,7 @@ export async function ensureDeployContext(options: DeployOptions): Promise<Ident
     force,
     release: !noRelease,
     developerPlatformClient,
-    envIdentifiers: getAppIdentifiers({app}, developerPlatformClient),
+    envIdentifiers: getAppIdentifiers({app}),
     remoteApp,
     activeAppVersion,
   })

packages/app/src/cli/services/context/breakdown-extensions.ts

@@ -368,20 +368,24 @@ function loadExtensionsIdentifiersBreakdown(
     (ext) => extensionTypeStrategy(specs, ext.specification?.identifier) === 'uuid',
   )
 
+  function moduleHasUUIDorUID(module: AppModuleVersion, identifier: string) {
+    return module.registrationUuid === identifier || module.registrationId === identifier
+  }
+
   const extensionsToUpdate = Object.entries(localRegistration)
-    .filter(([_identifier, uuid]) => extensionModules.map((module) => module.registrationUuid!).includes(uuid))
+    .filter(([_identifier, uuid]) => extensionModules.some((module) => moduleHasUUIDorUID(module, uuid)))
     .map(([identifier, _uuid]) => identifier)
 
   let extensionsToCreate = Object.entries(localRegistration)
-    .filter(([_identifier, uuid]) => !extensionModules.map((module) => module.registrationUuid!).includes(uuid))
+    .filter(([_identifier, uuid]) => !extensionModules.some((module) => moduleHasUUIDorUID(module, uuid)))
     .map(([identifier, _uuid]) => identifier)
   extensionsToCreate = Array.from(new Set(extensionsToCreate.concat(toCreate.map((source) => source.localIdentifier))))
 
   const extensionsOnlyRemote = extensionModules
     .filter(
       (module) =>
-        !Object.values(localRegistration).includes(module.registrationUuid!) &&
-        !toCreate.map((source) => source.localIdentifier).includes(module.registrationUuid!),
+        !Object.values(localRegistration).some((uuid) => moduleHasUUIDorUID(module, uuid)) &&
+        !toCreate.map((source) => source.localIdentifier).some((identifier) => moduleHasUUIDorUID(module, identifier)),
     )
     .map((module) => module.registrationTitle)
 

packages/app/src/cli/services/context/id-matching.test.ts

@@ -781,7 +781,7 @@ describe('automaticMatchmaking: with Atomic Deployments enabled', () => {
     const got = await automaticMatchmaking(
       [EXTENSION_A, EXTENSION_A_2],
       [REGISTRATION_A],
-      {},
+      {'extension-a': 'UUID_A'},
       testDeveloperPlatformClient({supportsAtomicDeployments: true}),
     )
 
@@ -792,6 +792,7 @@ describe('automaticMatchmaking: with Atomic Deployments enabled', () => {
       toCreate: [EXTENSION_A_2],
       toManualMatch: {local: [], remote: []},
     }
+
     expect(got).toEqual(expected)
   })
 })

packages/app/src/cli/services/context/id-matching.ts

@@ -22,7 +22,9 @@ interface MatchResult {
  * Filter function to match a local and a remote source by type and handle
  */
 const sameTypeAndName = (local: LocalSource, remote: RemoteSource) => {
-  return remote.type === local.graphQLType && slugify(remote.title) === slugify(local.handle)
+  return (
+    remote.type.toLowerCase() === local.graphQLType.toLowerCase() && slugify(remote.title) === slugify(local.handle)
+  )
 }
 
 /**
@@ -67,26 +69,42 @@ function matchByNameAndType(
 /**
  * Automatically match local and remote sources if they have the same UID
  */
-function matchByUUID(
+function matchByUIDandUUID(
   local: LocalSource[],
   remote: RemoteSource[],
+  ids: IdentifiersExtensions,
 ): {
   matched: IdentifiersExtensions
   toCreate: LocalSource[]
   toConfirm: {local: LocalSource; remote: RemoteSource}[]
   toManualMatch: {local: LocalSource[]; remote: RemoteSource[]}
 } {
   const matched: IdentifiersExtensions = {}
+  const pendingLocal: LocalSource[] = []
 
+  // First, try to match by UID, then by UUID.
   local.forEach((localSource) => {
-    const possibleMatch = remote.find((remoteSource) => remoteSource.uuid === localSource.uid)
-    // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-    if (possibleMatch) matched[localSource.localIdentifier] = possibleMatch.uuid!
+    const matchByUID = remote.find((remoteSource) => remoteSource.id === localSource.uid)
+    const matchByUUID = remote.find((remoteSource) => remoteSource.uuid === ids[localSource.localIdentifier])
+
+    if (matchByUID) {
+      matched[localSource.localIdentifier] = matchByUID.id
+    } else if (matchByUUID) {
+      matched[localSource.localIdentifier] = matchByUUID.uuid
+    } else {
+      pendingLocal.push(localSource)
+    }
   })
 
-  const toCreate = local.filter((elem) => !matched[elem.localIdentifier])
+  const pendingRemote = remote.filter(
+    (remoteSource) =>
+      !Object.values(matched).includes(remoteSource.uuid) && !Object.values(matched).includes(remoteSource.id),
+  )
+
+  // Then, try to match by name and type as a last resort.
+  const {matched: matchedByName, toCreate, toConfirm, toManualMatch} = matchByNameAndType(pendingLocal, pendingRemote)
 
-  return {matched, toCreate, toConfirm: [], toManualMatch: {local: [], remote: []}}
+  return {matched: {...matched, ...matchedByName}, toCreate, toConfirm, toManualMatch}
 }
 
 function migrateLegacyFunctions(
@@ -225,7 +243,7 @@ export async function automaticMatchmaking(
   const {local, remote} = pendingAfterMigratingFunctions
 
   const {matched, toCreate, toConfirm, toManualMatch} = useUuidMatching
-    ? matchByUUID(local, remote)
+    ? matchByUIDandUUID(localSources, remoteSources, ids)
     : matchByNameAndType(local, remote)
 
   return {