Merge pull request #4388 from Shopify/fix-app-deploy-stable

[stable] Fix app deploy
Shopify · Aug 28, 2024 · a1e03fb · a1e03fb
2 parents 45e9e8a + 5adfab2
commit a1e03fb
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 14 deletions.
diff --git a/.changeset/seven-starfishes-jump.md b/.changeset/seven-starfishes-jump.md
@@ -0,0 +1,5 @@
+---
+'@shopify/app': patch
+---
+
+Do not link on deploy when there is a current config
diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts
@@ -37,6 +37,7 @@ import {
   testThemeExtensions,
   testAppConfigExtensions,
   buildVersionedAppSchema,
+  testAppWithLegacyConfig,
 } from '../models/app/app.test-data.js'
 import metadata from '../metadata.js'
 import {
@@ -844,7 +845,7 @@ api_version = "2023-04"
     // Then
     expect(clearCachedAppInfo).toHaveBeenCalledWith(options.directory)
     expect(options.developerPlatformClient.appsForOrg).toBeCalled()
-    expect(link).not.toBeCalled()
+    expect(link).toBeCalled()
   })
 
   test('reset triggers link if opted into config in code', async () => {
@@ -1065,6 +1066,7 @@ describe('ensureDeployContext', () => {
 
   test('prompts the user to create or select an app and returns it with its id when the app has no extensions', async () => {
     // Given
+    const legacyApp = testAppWithLegacyConfig({config: {}})
     const app = testAppWithConfig({config: {client_id: APP1.apiKey}})
     const identifiers = {
       app: APP1.apiKey,
@@ -1074,12 +1076,9 @@ describe('ensureDeployContext', () => {
     }
     vi.mocked(getAppIdentifiers).mockReturnValue({app: undefined})
     vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers)
-    vi.mocked(loadApp).mockResolvedValue(app)
+    vi.mocked(loadApp).mockResolvedValue(legacyApp)
     vi.mocked(link).mockResolvedValue({...app.configuration, organization_id: ORG1.id})
-
-    const writeAppConfigurationFileSpy = vi
-      .spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
-      .mockResolvedValue()
+    vi.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile').mockResolvedValue()
 
     const developerPlatformClient = buildDeveloperPlatformClient({
       async orgAndApps(_orgId: string) {
@@ -1091,17 +1090,17 @@ describe('ensureDeployContext', () => {
       },
       appFromId: () => Promise.resolve(APP2),
     })
-    const opts: DeployContextOptions = {...deployOptions(app), developerPlatformClient}
+    const opts: DeployContextOptions = {...deployOptions(legacyApp), developerPlatformClient}
     vi.mocked(selectDeveloperPlatformClient).mockReturnValue(developerPlatformClient)
 
     // When
     const got = await ensureDeployContext(opts)
 
     // Then
-    expect(link).toBeCalledWith({directory: app.directory}, false)
+    expect(link).toBeCalled()
 
     expect(updateAppIdentifiers).toBeCalledWith({
-      app,
+      app: legacyApp,
       identifiers,
       command: 'deploy',
       developerPlatformClient,

diff --git a/packages/app/src/cli/services/context.ts b/packages/app/src/cli/services/context.ts
@@ -423,7 +423,7 @@ export interface DeployContextOptions {
  * That means we have a valid session, organization and app.
  *
  * If there is an API key via flag, configuration or env file, we check if it is valid. Otherwise, throw an error.
- * If there is no API key (or is invalid), show prompts to select an org and app.
+ * If there is no app (or is invalid), show prompts to select an org and app.
  * Finally, the info is updated in the env file.
  *
  * @param options - Current dev context options
@@ -433,8 +433,8 @@ export interface DeployContextOptions {
 export async function ensureDeployContext(options: DeployContextOptions): Promise<DeployContextOutput> {
   const {reset, force, noRelease} = options
   let developerPlatformClient = options.developerPlatformClient
-  // do the link here
-  const [remoteApp] = await fetchAppAndIdentifiers(options, developerPlatformClient, true, !options.apiKey)
+  const enableLinkingPrompt = !options.apiKey && !isCurrentAppSchema(options.app.configuration)
+  const [remoteApp] = await fetchAppAndIdentifiers(options, developerPlatformClient, true, enableLinkingPrompt)
 
   developerPlatformClient = remoteApp.developerPlatformClient ?? developerPlatformClient
 
@@ -815,11 +815,11 @@ async function linkIfNecessary(
   if (reset) clearCachedAppInfo(directory)
 
   const firstTimeSetup = previousCachedInfo === undefined
-  const usingConfigAndResetting = previousCachedInfo?.configFile && reset
   const usingConfigWithNoTomls: boolean =
     previousCachedInfo?.configFile !== undefined && (await glob(joinPath(directory, 'shopify.app*.toml'))).length === 0
+  const unlinked = firstTimeSetup || usingConfigWithNoTomls
+  const performAppLink = reset || (enableLinkingPrompt && unlinked)
 
-  const performAppLink = enableLinkingPrompt && (firstTimeSetup || usingConfigAndResetting || usingConfigWithNoTomls)
   if (performAppLink) {
     return link({directory, baseConfigName: previousCachedInfo?.configFile}, false)
   }