Merge pull request #4777 from Shopify/uid-matching-fixes

Fix UID matching issues

packages/app/src/cli/api/graphql/app-management/generated/active-app-release.ts

@@ -19,6 +19,7 @@ export type ActiveAppReleaseQuery = {
         name: string
         appModules: {
           uuid: string
+          userIdentifier: string
           handle: string
           config: JsonMapType
           specification: {identifier: string; externalIdentifier: string; name: string}
@@ -30,6 +31,7 @@ export type ActiveAppReleaseQuery = {
 
 export type ReleasedAppModuleFragment = {
   uuid: string
+  userIdentifier: string
   handle: string
   config: JsonMapType
   specification: {identifier: string; externalIdentifier: string; name: string}
@@ -46,6 +48,7 @@ export const ReleasedAppModuleFragmentDoc = {
         kind: 'SelectionSet',
         selections: [
           {kind: 'Field', name: {kind: 'Name', value: 'uuid'}},
+          {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}},
           {kind: 'Field', name: {kind: 'Name', value: 'handle'}},
           {kind: 'Field', name: {kind: 'Name', value: 'config'}},
           {
@@ -176,6 +179,7 @@ export const ActiveAppRelease = {
         kind: 'SelectionSet',
         selections: [
           {kind: 'Field', name: {kind: 'Name', value: 'uuid'}},
+          {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}},
           {kind: 'Field', name: {kind: 'Name', value: 'handle'}},
           {kind: 'Field', name: {kind: 'Name', value: 'config'}},
           {

packages/app/src/cli/api/graphql/app-management/generated/app-version-by-id.ts

@@ -14,6 +14,7 @@ export type AppVersionByIdQuery = {
     metadata: {versionTag?: string | null}
     appModules: {
       uuid: string
+      userIdentifier: string
       handle: string
       config: JsonMapType
       specification: {identifier: string; externalIdentifier: string; name: string}
@@ -89,6 +90,7 @@ export const AppVersionById = {
         kind: 'SelectionSet',
         selections: [
           {kind: 'Field', name: {kind: 'Name', value: 'uuid'}},
+          {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}},
           {kind: 'Field', name: {kind: 'Name', value: 'handle'}},
           {kind: 'Field', name: {kind: 'Name', value: 'config'}},
           {

packages/app/src/cli/api/graphql/app-management/generated/apps.ts

@@ -20,6 +20,7 @@ export type ListAppsQuery = {
             name: string
             appModules: {
               uuid: string
+              userIdentifier: string
               handle: string
               config: JsonMapType
               specification: {identifier: string; externalIdentifier: string; name: string}
@@ -147,6 +148,7 @@ export const ListApps = {
         kind: 'SelectionSet',
         selections: [
           {kind: 'Field', name: {kind: 'Name', value: 'uuid'}},
+          {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}},
           {kind: 'Field', name: {kind: 'Name', value: 'handle'}},
           {kind: 'Field', name: {kind: 'Name', value: 'config'}},
           {

packages/app/src/cli/api/graphql/app-management/generated/create-app-version.ts

@@ -17,6 +17,7 @@ export type CreateAppVersionMutation = {
       id: string
       appModules: {
         uuid: string
+        userIdentifier: string
         handle: string
         config: JsonMapType
         specification: {identifier: string; externalIdentifier: string; name: string}
@@ -157,6 +158,7 @@ export const CreateAppVersion = {
         kind: 'SelectionSet',
         selections: [
           {kind: 'Field', name: {kind: 'Name', value: 'uuid'}},
+          {kind: 'Field', name: {kind: 'Name', value: 'userIdentifier'}},
           {kind: 'Field', name: {kind: 'Name', value: 'handle'}},
           {kind: 'Field', name: {kind: 'Name', value: 'config'}},
           {

packages/app/src/cli/api/graphql/app-management/queries/active-app-release.graphql

@@ -23,6 +23,7 @@ query activeAppRelease($appId: ID!) {
 
 fragment ReleasedAppModule on AppModule {
   uuid
+  userIdentifier
   handle
   config
   specification {

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

@@ -10,23 +10,20 @@ vi.mock('../dev/create-extension')
 
 const REGISTRATION_A: RemoteSource = {
   uuid: 'UUID_A',
-  uid: 'UID_A',
   id: 'A',
   title: 'EXTENSION_A',
   type: 'CHECKOUT_POST_PURCHASE',
 }
 
 const REGISTRATION_A_2: RemoteSource = {
   uuid: 'UUID_A_2',
-  uid: 'UID_A_2',
   id: 'A_2',
   title: 'EXTENSION_A_2',
   type: 'CHECKOUT_POST_PURCHASE',
 }
 
 const REGISTRATION_A_3: RemoteSource = {
   uuid: 'UUID_A_3',
-  uid: 'UID_A_3',
   id: 'A_3',
   title: 'EXTENSION_A_3',
   type: 'CHECKOUT_POST_PURCHASE',
@@ -41,31 +38,27 @@ const REGISTRATION_A_4: RemoteSource = {
 
 const REGISTRATION_B: RemoteSource = {
   uuid: 'UUID_B',
-  uid: 'UID_B',
   id: 'B',
   title: 'EXTENSION_B',
   type: 'SUBSCRIPTION_MANAGEMENT',
 }
 
 const REGISTRATION_C: RemoteSource = {
   uuid: 'UUID_C',
-  uid: 'UID_C',
   id: 'C',
   title: 'EXTENSION_C',
   type: 'THEME_APP_EXTENSION',
 }
 
 const REGISTRATION_D: RemoteSource = {
   uuid: 'UUID_D',
-  uid: 'UID_D',
   id: 'D',
   title: 'EXTENSION_D',
   type: 'WEB_PIXEL_EXTENSION',
 }
 
 const REGISTRATION_FUNCTION_A: RemoteSource = {
   uuid: 'FUNCTION_UUID_A',
-  uid: 'FUNCTION_UID_A',
   id: 'FUNCTION_A',
   title: 'FUNCTION A',
   type: 'FUNCTION',
@@ -107,7 +100,7 @@ beforeAll(async () => {
     },
     entrySourceFilePath: '',
     devUUID: 'devUUID',
-    uid: 'UID_A',
+    uid: 'UUID_A',
   })
 
   EXTENSION_A_2 = await testUIExtension({
@@ -131,7 +124,7 @@ beforeAll(async () => {
     },
     entrySourceFilePath: '',
     devUUID: 'devUUID',
-    uid: 'UID_A_2',
+    uid: 'UIUD_A_2',
   })
 
   EXTENSION_B = await testUIExtension({
@@ -155,7 +148,7 @@ beforeAll(async () => {
     },
     entrySourceFilePath: '',
     devUUID: 'devUUID',
-    uid: 'UID_B',
+    uid: 'UUID_B',
   })
 
   EXTENSION_B_2 = await testUIExtension({
@@ -179,7 +172,7 @@ beforeAll(async () => {
     },
     entrySourceFilePath: '',
     devUUID: 'devUUID',
-    uid: 'UID_B_2',
+    uid: 'UUID_B_2',
   })
 
   EXTENSION_C = await testUIExtension({
@@ -203,7 +196,7 @@ beforeAll(async () => {
     },
     entrySourceFilePath: '',
     devUUID: 'devUUID',
-    uid: 'UID_C',
+    uid: 'UUID_C',
   })
 
   EXTENSION_D = await testUIExtension({
@@ -228,7 +221,7 @@ beforeAll(async () => {
     outputPath: '',
     entrySourceFilePath: '',
     devUUID: 'devUUID',
-    uid: 'UID_D',
+    uid: 'UUID_D',
   })
 
   FUNCTION_A = await testFunctionExtension({
@@ -794,7 +787,7 @@ describe('automaticMatchmaking: with Atomic Deployments enabled', () => {
 
     // Then
     const expected = {
-      identifiers: {'extension-a': 'UID_A'},
+      identifiers: {'extension-a': 'UUID_A'},
       toConfirm: [],
       toCreate: [EXTENSION_A_2],
       toManualMatch: {local: [], remote: []},

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

@@ -67,7 +67,7 @@ function matchByNameAndType(
 /**
  * Automatically match local and remote sources if they have the same UID
  */
-function matchByUID(
+function matchByUUID(
   local: LocalSource[],
   remote: RemoteSource[],
 ): {
@@ -79,9 +79,9 @@ function matchByUID(
   const matched: IdentifiersExtensions = {}
 
   local.forEach((localSource) => {
-    const possibleMatch = remote.find((remoteSource) => remoteSource.uid === localSource.uid)
+    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.uid!
+    if (possibleMatch) matched[localSource.localIdentifier] = possibleMatch.uuid!
   })
 
   const toCreate = local.filter((elem) => !matched[elem.localIdentifier])
@@ -207,15 +207,14 @@ export async function automaticMatchmaking(
   identifiers: IdentifiersExtensions,
   developerPlatformClient: DeveloperPlatformClient,
 ): Promise<MatchResult> {
-  const useUidMatching = developerPlatformClient.supportsAtomicDeployments
+  const useUuidMatching = developerPlatformClient.supportsAtomicDeployments
   const ids = getExtensionIds(localSources, identifiers)
   const localIds = Object.values(ids)
 
   const existsRemotely = (local: LocalSource) =>
     remoteSources.some((remote) => {
       if (remote.type !== developerPlatformClient.toExtensionGraphQLType(local.graphQLType)) return false
-      const remoteIdField = useUidMatching ? 'uid' : 'uuid'
-      return ids[local.localIdentifier] === remote[remoteIdField]
+      return ids[local.localIdentifier] === remote.uuid
     })
 
   const {migrated: migratedFunctions, pending: pendingAfterMigratingFunctions} = migrateLegacyFunctions(
@@ -225,8 +224,8 @@ export async function automaticMatchmaking(
   )
   const {local, remote} = pendingAfterMigratingFunctions
 
-  const {matched, toCreate, toConfirm, toManualMatch} = useUidMatching
-    ? matchByUID(local, remote)
+  const {matched, toCreate, toConfirm, toManualMatch} = useUuidMatching
+    ? matchByUUID(local, remote)
     : matchByNameAndType(local, remote)
 
   return {

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

@@ -311,7 +311,6 @@ async function createExtensions(
       result[extension.localIdentifier] = {
         // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
         id: extension.uid!,
-        uid: extension.uid,
         // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
         uuid: extension.uid!,
         type: extension.type,

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

@@ -24,7 +24,6 @@ export interface EnsureDeploymentIdsPresenceOptions {
 
 export interface RemoteSource {
   uuid: string
-  uid?: string
   type: string
   id: string
   title: string

packages/app/src/cli/utilities/developer-platform-client/app-management-client.test.ts

@@ -16,6 +16,7 @@ import {fetch} from '@shopify/cli-kit/node/http'
 import {businessPlatformOrganizationsRequest} from '@shopify/cli-kit/node/api/business-platform'
 import {appManagementRequestDoc} from '@shopify/cli-kit/node/api/app-management'
 import {BugError} from '@shopify/cli-kit/node/error'
+import {randomUUID} from '@shopify/cli-kit/node/crypto'
 
 vi.mock('@shopify/cli-kit/node/http')
 vi.mock('@shopify/cli-kit/node/api/business-platform')
@@ -47,6 +48,7 @@ const templateDisallowedByBetaFlag: GatedExtensionTemplate = {
 function moduleFromExtension(extension: ExtensionInstance) {
   return {
     uuid: extension.uid,
+    userIdentifier: extension.uid,
     handle: extension.handle,
     config: extension.configuration,
     specification: {
@@ -76,6 +78,36 @@ describe('diffAppModules', () => {
     expect(removed).toEqual([moduleA])
     expect(updated).toEqual([moduleB])
   })
+
+  // This test considers the case where there are local and remote modules, which may have slightly different properties
+  test('extracts the added, removed and updated modules before deployment', async () => {
+    // Given
+    const [remoteModuleA, remoteModuleB] = [moduleFromExtension(extensionA), moduleFromExtension(extensionB)]
+    // Under some circumstances, local UUID may differ from remote.
+    // So we are testing that diffing happens based on the shared userIdentifier
+    // property, not the UUID.
+    const localModuleB = {
+      ...remoteModuleB,
+      uuid: randomUUID(),
+    }
+    const localModuleC = {
+      ...moduleFromExtension(extensionC),
+      uuid: randomUUID(),
+    }
+
+    const before = [remoteModuleA, remoteModuleB]
+    const after = [localModuleB, localModuleC]
+
+    // When
+    const {added, removed, updated} = diffAppModules({currentModules: before, selectedVersionModules: after})
+
+    // Then
+    expect(added).toEqual([localModuleC])
+    expect(removed).toEqual([remoteModuleA])
+    // Updated returns the remote module, not the local one. This shouldn't matter
+    // as the module identifiers are the same.
+    expect(updated).toEqual([remoteModuleB])
+  })
 })
 
 describe('templateSpecifications', () => {

packages/app/src/cli/utilities/developer-platform-client/app-management-client.ts

@@ -902,12 +902,12 @@ interface DiffAppModulesOutput {
 }
 
 export function diffAppModules({currentModules, selectedVersionModules}: DiffAppModulesInput): DiffAppModulesOutput {
-  const currentModuleUids = currentModules.map((mod) => mod.uuid)
-  const selectedVersionModuleUids = selectedVersionModules.map((mod) => mod.uuid)
-  const removed = currentModules.filter((mod) => !selectedVersionModuleUids.includes(mod.uuid))
-  const added = selectedVersionModules.filter((mod) => !currentModuleUids.includes(mod.uuid))
-  const addedUids = added.map((mod) => mod.uuid)
-  const updated = selectedVersionModules.filter((mod) => !addedUids.includes(mod.uuid))
+  const currentModuleUids = currentModules.map((mod) => mod.userIdentifier)
+  const selectedVersionModuleUids = selectedVersionModules.map((mod) => mod.userIdentifier)
+  const added = selectedVersionModules.filter((mod) => !currentModuleUids.includes(mod.userIdentifier))
+  const removed = currentModules.filter((mod) => !selectedVersionModuleUids.includes(mod.userIdentifier))
+  const removedUids = removed.map((mod) => mod.userIdentifier)
+  const updated = currentModules.filter((mod) => !removedUids.includes(mod.userIdentifier))
   return {added, removed, updated}
 }
 
@@ -946,8 +946,8 @@ function mapBusinessPlatformStoresToOrganizationStores(storesArray: ShopNode[]):
 
 function appModuleVersion(mod: ReleasedAppModuleFragment): Required<AppModuleVersion> {
   return {
-    registrationId: mod.uuid,
-    registrationUuid: mod.uuid,
+    registrationId: mod.userIdentifier,
+    registrationUuid: mod.userIdentifier,
     registrationTitle: mod.handle,
     type: mod.specification.externalIdentifier,
     config: mod.config,