Merge pull request #6239 from Shopify/08-11-handle_uid_mis-matches_post-migration

Handle UID mis-matches post-migration

Author: isaacroldan

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

@@ -362,7 +362,13 @@ async function resolveRemoteExtensionIdentifiersBreakdown(
   const version = activeAppVersion || (await developerPlatformClient.activeAppVersion(remoteApp))
   if (!version) return
 
-  const extensionIdentifiersBreakdown = loadExtensionsIdentifiersBreakdown(version, localRegistration, toCreate, specs)
+  const extensionIdentifiersBreakdown = loadExtensionsIdentifiersBreakdown(
+    version,
+    localRegistration,
+    toCreate,
+    specs,
+    developerPlatformClient,
+  )
 
   const dashboardOnlyFinal = dashboardOnly.filter(
     (dashboardOnly) =>
@@ -384,13 +390,24 @@ function loadExtensionsIdentifiersBreakdown(
   localRegistration: IdentifiersExtensions,
   toCreate: LocalSource[],
   specs: ExtensionSpecification[],
+  developerPlatformClient: DeveloperPlatformClient,
 ) {
   const extensionModules = activeAppVersion?.appModuleVersions.filter(
     (ext) => extensionTypeStrategy(specs, ext.specification?.identifier) === 'uuid',
   )
 
+  // In AppManagement, matching has to be via UID, but we acccept UUID matches if the UID is empty (migration pending)
+  // In Partners, we keep the legacy match of only UUID.
   function moduleHasUUIDorUID(module: AppModuleVersion, identifier: string) {
-    return module.registrationUuid === identifier || module.registrationId === identifier
+    const UuidMatch = module.registrationUuid === identifier
+    const UidMatch = module.registrationId === identifier
+    const pendingMigration = module.registrationId.length === 0
+
+    if (developerPlatformClient.supportsAtomicDeployments) {
+      return UidMatch || (pendingMigration && UuidMatch)
+    } else {
+      return UuidMatch
+    }
   }
 
   const allExistingExtensions = Object.entries(localRegistration)

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

@@ -780,9 +780,10 @@ describe('automaticMatchmaking: with Atomic Deployments enabled', () => {
 
   test('creates the missing extension when there is a remote one', async () => {
     // When
+    const registrationA = {...REGISTRATION_A, id: ''}
     const got = await automaticMatchmaking(
       [EXTENSION_A, EXTENSION_A_2],
-      [REGISTRATION_A],
+      [registrationA],
       {'extension-a': 'UUID_A'},
       testDeveloperPlatformClient({supportsAtomicDeployments: true}),
     )
@@ -805,16 +806,21 @@ describe('outputAddedIDs', () => {
     vi.mocked(outputInfo).mockClear()
 
     // Extension B has a valid UID
-    REGISTRATION_B.id = EXTENSION_B.uid
+    // Extension C is marked as toCreate because we only try to match by UID (because it has a real one)
+    const registrationA = {...REGISTRATION_A, id: ''}
+    const registrationB = {...REGISTRATION_B, id: EXTENSION_B.uid}
+    const registrationC = {...REGISTRATION_C}
+    const registrationD = {...REGISTRATION_D, id: ''}
 
     // When: Extensions are matched by UUID (not by UID)
-    await automaticMatchmaking(
-      [EXTENSION_A, EXTENSION_B, EXTENSION_C],
-      [REGISTRATION_A, REGISTRATION_B, REGISTRATION_C],
+    const result = await automaticMatchmaking(
+      [EXTENSION_A, EXTENSION_B, EXTENSION_C, EXTENSION_D],
+      [registrationA, registrationB, registrationC, registrationD],
       {
         'extension-a': 'UUID_A',
         'extension-b': 'UUID_B',
         'extension-c': 'UUID_C',
+        'extension-d': 'UUID_D',
       },
       testDeveloperPlatformClient({supportsAtomicDeployments: true}),
     )
@@ -823,10 +829,13 @@ describe('outputAddedIDs', () => {
     expect(outputInfo).toHaveBeenCalledWith('Generating extension IDs\n')
     expect(outputInfo).toHaveBeenCalledWith(expect.stringContaining('\x1B[36mextension-a\x1B[39m | Added ID: UUID_A'))
     expect(outputInfo).not.toHaveBeenCalledWith(expect.stringContaining('Added ID: UUID_B'))
-    expect(outputInfo).toHaveBeenCalledWith(expect.stringContaining('\x1B[35mextension-c\x1B[39m | Added ID: UUID_C'))
+    expect(outputInfo).not.toHaveBeenCalledWith(expect.stringContaining('Added ID: UUID_C'))
+    expect(outputInfo).toHaveBeenCalledWith(expect.stringContaining('\x1B[35mextension-d\x1B[39m | Added ID: UUID_D'))
     expect(outputInfo).toHaveBeenCalledWith('\n')
 
     // Verify it was called 4 times total (header + 2 extensions + footer)
     expect(outputInfo).toHaveBeenCalledTimes(4)
+
+    expect(result.toCreate).toEqual([EXTENSION_C])
   })
 })

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

@@ -86,23 +86,26 @@ function matchByUIDandUUID(
   const matchedByUUID: IdentifiersExtensions = {}
 
   // First, try to match by UID, then by UUID.
+  // But only accept a UUID match if the ID for the remote source is empty, meaning is still pending migration.
   local.forEach((localSource) => {
     const matchByUID = remote.find((remoteSource) => remoteSource.id === localSource.uid)
     const matchByUUID = remote.find((remoteSource) => remoteSource.uuid === ids[localSource.localIdentifier])
 
     if (matchByUID) {
       matchedByUID[localSource.localIdentifier] = matchByUID.id
-    } else if (matchByUUID) {
+    } else if (matchByUUID && matchByUUID.id.length === 0) {
       matchedByUUID[localSource.localIdentifier] = matchByUUID.uuid
     } else {
       pendingLocal.push(localSource)
     }
   })
 
+  // Remote source with a valid UID is not pending, shouldn't be tried to be matched by name/type.
   const pendingRemote = remote.filter(
     (remoteSource) =>
       !Object.values(matchedByUUID).includes(remoteSource.uuid) &&
-      !Object.values(matchedByUID).includes(remoteSource.id),
+      !Object.values(matchedByUID).includes(remoteSource.id) &&
+      remoteSource.id.length === 0,
   )
 
   // Then, try to match by name and type as a last resort.