Merge pull request #6287 from Shopify/08-20-fix_issue_when_trying_to_import_and_already_imported_extension

Fix issue when trying to import an already imported extension

Author: gonzaloriestra

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

@@ -33,7 +33,7 @@ export interface Identifiers {
 }
 
 type UuidOnlyIdentifiers = Omit<Identifiers, 'extensionIds' | 'extensionsNonUuidManaged'>
-type UpdateAppIdentifiersCommand = 'dev' | 'deploy' | 'release'
+type UpdateAppIdentifiersCommand = 'dev' | 'deploy' | 'release' | 'import-extensions'
 interface UpdateAppIdentifiersOptions {
   app: AppInterface
   identifiers: UuidOnlyIdentifiers
@@ -72,9 +72,11 @@ export async function updateAppIdentifiers(
 
   const contentIsEqual = deepCompare(dotenvFile.variables, updatedVariables)
   const writeToFile =
-    !contentIsEqual &&
-    (command === 'deploy' || command === 'release') &&
-    !developerPlatformClient.supportsAtomicDeployments
+    (!contentIsEqual &&
+      (command === 'deploy' || command === 'release') &&
+      !developerPlatformClient.supportsAtomicDeployments) ||
+    command === 'import-extensions'
+
   dotenvFile.variables = updatedVariables
 
   if (writeToFile) {

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

@@ -2,7 +2,7 @@ import {ensureDeployContext} from './context.js'
 import {deploy, importExtensionsIfNeeded} from './deploy.js'
 import {uploadExtensionsBundle} from './deploy/upload.js'
 import {bundleAndBuildExtensions} from './deploy/bundle.js'
-import {importAllExtensions, allExtensionTypes} from './import-extensions.js'
+import {importAllExtensions, allExtensionTypes, filterOutImportedExtensions} from './import-extensions.js'
 import {getExtensions} from './fetch-extensions.js'
 import {reloadApp} from '../models/app/loader.js'
 import {
@@ -75,6 +75,9 @@ beforeEach(() => {
 
   // Mock getExtensions to return empty arrays by default
   vi.mocked(getExtensions).mockResolvedValue([])
+
+  // Mock filterOutImportedExtensions to return the extensions by default
+  vi.mocked(filterOutImportedExtensions).mockImplementation((_app, extensions) => extensions)
 })
 
 describe('deploy', () => {
@@ -840,6 +843,7 @@ describe('ImportExtensionsIfNeeded', () => {
 
     vi.mocked(isTTY).mockReturnValue(true)
     vi.mocked(getExtensions).mockResolvedValue([])
+    vi.mocked(filterOutImportedExtensions).mockReturnValue([])
 
     // When
     const result = await importExtensionsIfNeeded({

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

@@ -2,7 +2,7 @@ import {uploadExtensionsBundle, UploadExtensionsBundleOutput} from './deploy/upl
 
 import {ensureDeployContext} from './context.js'
 import {bundleAndBuildExtensions} from './deploy/bundle.js'
-import {allExtensionTypes, importAllExtensions} from './import-extensions.js'
+import {allExtensionTypes, filterOutImportedExtensions, importAllExtensions} from './import-extensions.js'
 import {getExtensions} from './fetch-extensions.js'
 import {AppLinkedInterface} from '../models/app/app.js'
 import {updateAppIdentifiers} from '../models/app/identifiers.js'
@@ -151,19 +151,21 @@ export async function importExtensionsIfNeeded(options: ImportExtensionsIfNeeded
     onlyDashboardManaged: true,
   })
 
-  if (extensions.length === 0) {
+  const extensionsNotImportedYet = filterOutImportedExtensions(options.app, extensions)
+
+  if (extensionsNotImportedYet.length === 0) {
     return app
   }
 
   if (developerPlatformClient.supportsDashboardManagedExtensions) {
     return handleSupportedDashboardExtensions({
       ...options,
-      extensions,
+      extensions: extensionsNotImportedYet,
     })
   } else {
     return handleUnsupportedDashboardExtensions({
       ...options,
-      extensions,
+      extensions: extensionsNotImportedYet,
     })
   }
 }

packages/app/src/cli/services/import-extensions.test.ts

@@ -1,6 +1,6 @@
-import {importExtensions} from './import-extensions.js'
+import {importExtensions, filterOutImportedExtensions} from './import-extensions.js'
 import {buildTomlObject} from './flow/extension-to-toml.js'
-import {testAppLinked, testDeveloperPlatformClient} from '../models/app/app.test-data.js'
+import {testAppLinked, testDeveloperPlatformClient, testUIExtension} from '../models/app/app.test-data.js'
 import {OrganizationApp} from '../models/organization.js'
 import {ExtensionRegistration} from '../api/graphql/all_app_extension_registrations.js'
 import {describe, expect, test, vi, beforeEach} from 'vitest'
@@ -237,3 +237,31 @@ describe('import-extensions', () => {
     })
   })
 })
+
+describe('filterOutImportedExtensions', () => {
+  test('filters out extensions that are already imported', async () => {
+    // Given
+    const extensionA = await testUIExtension({handle: 'my-extension-a'})
+    const extensionB = await testUIExtension({handle: 'my-extension-b'})
+
+    const app = testAppLinked({
+      dotenv: {
+        path: '.env',
+        variables: {
+          SHOPIFY_MY_EXTENSION_A_ID: 'uuidA',
+          SHOPIFY_MY_EXTENSION_B_ID: 'uuidB',
+          SHOPIFY_SOME_OTHER_ID: 'someOtherId',
+        },
+      },
+      allExtensions: [extensionA, extensionB],
+    })
+
+    const extensions = [flowExtensionA, flowExtensionB, marketingActivityExtension]
+
+    // When
+    const result = filterOutImportedExtensions(app, extensions)
+
+    // Then
+    expect(result).toEqual([marketingActivityExtension])
+  })
+})

packages/app/src/cli/services/import-extensions.ts

@@ -37,6 +37,7 @@ export async function importExtensions(options: ImportOptions) {
   const {app, remoteApp, developerPlatformClient, extensionTypes, extensions, buildTomlObject, all} = options
 
   let extensionsToMigrate = extensions.filter((ext) => extensionTypes.includes(ext.type.toLowerCase()))
+  extensionsToMigrate = filterOutImportedExtensions(app, extensionsToMigrate)
 
   if (extensionsToMigrate.length === 0) {
     throw new AbortError('No extensions to migrate')
@@ -79,11 +80,19 @@ export async function importExtensions(options: ImportOptions) {
       extensions: extensionUuids,
       app: remoteApp.apiKey,
     },
-    command: 'deploy',
+    command: 'import-extensions',
     developerPlatformClient,
   })
 }
 
+// import-extensions updates the .env file with the new UUIDs. we can use that to know if an extension was already imported.
+// From the app loaded extensions, get the UUID and compare with the pending extensions.
+export function filterOutImportedExtensions(app: AppLinkedInterface, extensions: ExtensionRegistration[]) {
+  const cachedUUIDs = app.dotenv?.variables ?? {}
+  const localExtensionUUIDs = app.allExtensions.map((ext) => cachedUUIDs[ext.idEnvironmentVariableName])
+  return extensions.filter((ext) => !localExtensionUUIDs.includes(ext.uuid))
+}
+
 export async function importAllExtensions(options: ImportAllOptions) {
   const migrationChoices = getMigrationChoices(options.extensions)
   await Promise.all(