Merge pull request #5004 from Shopify/12-02-remove_all_conditional_logic_around_app_management_being_disabled

Remove all conditional logic around App Management being disabled

Author: amcaplan

.github/workflows/shopify-cli.yml

@@ -49,7 +49,6 @@ env:
   PNPM_VERSION: '8.15.7'
   BUNDLE_WITHOUT: 'test:development'
   SHOPIFY_FLAG_CLIENT_ID: ${{ secrets.SHOPIFY_FLAG_CLIENT_ID }}
-  SHOPIFY_CLI_PARTNERS_TOKEN: ${{ secrets.SHOPIFY_CLI_PARTNERS_TOKEN }}
   GH_TOKEN: ${{ secrets.SHOPIFY_GH_READ_CONTENT_TOKEN }}
 
 jobs:
@@ -87,6 +86,8 @@ jobs:
         run: pnpm nx run-many --all --skip-nx-cache --target=test --exclude=features --output-style=stream
       - name: Acceptance tests
         if: ${{ matrix.node == '18.20.3' }}
+        env:
+          SHOPIFY_CLI_PARTNERS_TOKEN: ${{ secrets.SHOPIFY_CLI_PARTNERS_TOKEN }}
         run: pnpm nx run features:test
       - name: Send Slack notification on failure
         uses: slackapi/slack-github-action@007b2c3c751a190b6f0f040e47ed024deaa72844 # [email protected]
@@ -305,6 +306,8 @@ jobs:
         with:
           node-version: ${{ matrix.node }}
       - name: Acceptance tests
+        env:
+          SHOPIFY_CLI_PARTNERS_TOKEN: ${{ secrets.SHOPIFY_CLI_PARTNERS_TOKEN }}
         run: pnpm test:features --output-style=stream
 
   pr-test-coverage:
@@ -355,6 +358,8 @@ jobs:
       - name: Unit tests
         run: pnpm test:unit --output-style=stream
       - name: Acceptance tests
+        env:
+          SHOPIFY_CLI_PARTNERS_TOKEN: ${{ secrets.SHOPIFY_CLI_PARTNERS_TOKEN }}
         run: pnpm test:features --output-style=stream
       - name: Setup tmate session
         if: ${{ failure() && inputs.debug-enabled }}

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

@@ -8,11 +8,10 @@ import {joinPath} from '@shopify/cli-kit/node/path'
 describe('bundleAndBuildExtensions', () => {
   let app: AppInterface
 
-  test('generates a manifest.json when App Management is enabled', async () => {
+  test('generates a manifest.json', async () => {
     await file.inTemporaryDirectory(async (tmpDir: string) => {
       // Given
       vi.spyOn(file, 'writeFileSync').mockResolvedValue(undefined)
-      const envVars = {USE_APP_MANAGEMENT_API: 'true'}
       const bundlePath = joinPath(tmpDir, 'bundle.zip')
 
       const uiExtension = await testUIExtension({type: 'web_pixel_extension'})
@@ -60,7 +59,7 @@ describe('bundleAndBuildExtensions', () => {
       }
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath}, envVars)
+      await bundleAndBuildExtensions({app, identifiers, bundlePath})
 
       // Then
       expect(extensionBundleMock).toHaveBeenCalledTimes(2)
@@ -73,41 +72,6 @@ describe('bundleAndBuildExtensions', () => {
     })
   })
 
-  test('does not generate the manifest.json when App Management is disabled', async () => {
-    await file.inTemporaryDirectory(async (tmpDir: string) => {
-      // Given
-      vi.spyOn(file, 'writeFileSync').mockResolvedValue(undefined)
-      const bundlePath = joinPath(tmpDir, 'bundle.zip')
-
-      const uiExtension = await testUIExtension({type: 'web_pixel_extension'})
-      const extensionBundleMock = vi.fn()
-      uiExtension.buildForBundle = extensionBundleMock
-      const themeExtension = await testThemeExtensions()
-      themeExtension.buildForBundle = extensionBundleMock
-      app = testApp({allExtensions: [uiExtension, themeExtension]})
-
-      const extensions: {[key: string]: string} = {}
-      for (const extension of app.allExtensions) {
-        extensions[extension.localIdentifier] = extension.localIdentifier
-      }
-      const identifiers = {
-        app: 'app-id',
-        extensions,
-        extensionIds: {},
-        extensionsNonUuidManaged: {},
-      }
-
-      // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath}, {})
-
-      // Then
-      expect(extensionBundleMock).toHaveBeenCalledTimes(2)
-      expect(file.writeFileSync).not.toHaveBeenCalled()
-
-      await expect(file.fileExists(bundlePath)).resolves.toBeTruthy()
-    })
-  })
-
   test('creates a zip file for a function extension', async () => {
     await file.inTemporaryDirectory(async (tmpDir: string) => {
       // Given
@@ -132,7 +96,7 @@ describe('bundleAndBuildExtensions', () => {
       }
 
       // When
-      await bundleAndBuildExtensions({app, identifiers, bundlePath}, {})
+      await bundleAndBuildExtensions({app, identifiers, bundlePath})
 
       // Then
       await expect(file.fileExists(bundlePath)).resolves.toBeTruthy()

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

@@ -6,7 +6,6 @@ import {AbortSignal} from '@shopify/cli-kit/node/abort'
 import {inTemporaryDirectory, mkdirSync, touchFile, writeFileSync} from '@shopify/cli-kit/node/fs'
 import {joinPath} from '@shopify/cli-kit/node/path'
 import {renderConcurrent} from '@shopify/cli-kit/node/ui'
-import {isAppManagementEnabled} from '@shopify/cli-kit/node/context/local'
 import {Writable} from 'stream'
 
 interface BundleOptions {
@@ -15,18 +14,16 @@ interface BundleOptions {
   identifiers?: Identifiers
 }
 
-export async function bundleAndBuildExtensions(options: BundleOptions, systemEnvironment = process.env) {
+export async function bundleAndBuildExtensions(options: BundleOptions) {
   await inTemporaryDirectory(async (tmpDir) => {
     const bundleDirectory = joinPath(tmpDir, 'bundle')
     mkdirSync(bundleDirectory)
     await touchFile(joinPath(bundleDirectory, '.shopify'))
 
-    if (isAppManagementEnabled(systemEnvironment)) {
-      // Include manifest in bundle
-      const appManifest = await options.app.manifest()
-      const manifestPath = joinPath(bundleDirectory, 'manifest.json')
-      writeFileSync(manifestPath, JSON.stringify(appManifest, null, 2))
-    }
+    // Include manifest in bundle
+    const appManifest = await options.app.manifest()
+    const manifestPath = joinPath(bundleDirectory, 'manifest.json')
+    writeFileSync(manifestPath, JSON.stringify(appManifest, null, 2))
 
     // Force the download of the javy binary in advance to avoid later problems,
     // as it might be done multiple times in parallel. https://github.com/Shopify/cli/issues/2877

packages/app/src/cli/services/dev/fetch.test.ts

@@ -54,7 +54,7 @@ afterEach(() => {
 })
 
 describe('fetchOrganizations', async () => {
-  test('returns fetched organizations from Partners when App Management is disabled', async () => {
+  test('returns fetched organizations from Partners and App Management', async () => {
     // Given
     const partnersClient: PartnersClient = testDeveloperPlatformClient({
       organizations: () => Promise.resolve([ORG1]),
@@ -68,27 +68,6 @@ describe('fetchOrganizations', async () => {
     // When
     const got = await fetchOrganizations()
 
-    // Then
-    expect(got).toEqual([ORG1])
-    expect(partnersClient.organizations).toHaveBeenCalled()
-    expect(appManagementClient.organizations).not.toHaveBeenCalled()
-  })
-
-  test('returns fetched organizations from Partners and App Management when App Management is enabled', async () => {
-    // Given
-    vi.stubEnv('USE_APP_MANAGEMENT_API', '1')
-    const partnersClient: PartnersClient = testDeveloperPlatformClient({
-      organizations: () => Promise.resolve([ORG1]),
-    }) as PartnersClient
-    const appManagementClient: AppManagementClient = testDeveloperPlatformClient({
-      organizations: () => Promise.resolve([ORG2]),
-    }) as AppManagementClient
-    vi.mocked(PartnersClient).mockReturnValue(partnersClient)
-    vi.mocked(AppManagementClient).mockReturnValue(appManagementClient)
-
-    // When
-    const got = await fetchOrganizations()
-
     // Then
     expect(got).toEqual([ORG1, ORG2])
     expect(partnersClient.organizations).toHaveBeenCalled()
@@ -100,14 +79,19 @@ describe('fetchOrganizations', async () => {
     const partnersClient: PartnersClient = testDeveloperPlatformClient({
       organizations: () => Promise.resolve([]),
     }) as PartnersClient
+    const appManagementClient: AppManagementClient = testDeveloperPlatformClient({
+      organizations: () => Promise.resolve([]),
+    }) as AppManagementClient
     vi.mocked(PartnersClient).mockReturnValue(partnersClient)
+    vi.mocked(AppManagementClient).mockReturnValue(appManagementClient)
 
     // When
     const got = fetchOrganizations()
 
     // Then
     await expect(got).rejects.toThrow(new NoOrgError(testPartnersUserSession.accountInfo))
     expect(partnersClient.organizations).toHaveBeenCalled()
+    expect(appManagementClient.organizations).toHaveBeenCalled()
   })
 })
 

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

@@ -55,7 +55,7 @@ import {
 import {DevSessionCreateMutation} from '../api/graphql/app-dev/generated/dev-session-create.js'
 import {DevSessionUpdateMutation} from '../api/graphql/app-dev/generated/dev-session-update.js'
 import {DevSessionDeleteMutation} from '../api/graphql/app-dev/generated/dev-session-delete.js'
-import {isAppManagementEnabled} from '@shopify/cli-kit/node/context/local'
+import {isAppManagementDisabled} from '@shopify/cli-kit/node/context/local'
 
 export enum ClientName {
   AppManagement = 'app-management',
@@ -77,9 +77,7 @@ export interface AppVersionIdentifiers {
 }
 
 export function allDeveloperPlatformClients(): DeveloperPlatformClient[] {
-  const clients: DeveloperPlatformClient[] = [new PartnersClient()]
-  if (isAppManagementEnabled()) clients.push(new AppManagementClient())
-  return clients
+  return isAppManagementDisabled() ? [new PartnersClient()] : [new PartnersClient(), new AppManagementClient()]
 }
 
 /**
@@ -118,11 +116,9 @@ export function selectDeveloperPlatformClient({
   configuration,
   organization,
 }: SelectDeveloperPlatformClientOptions = {}): DeveloperPlatformClient {
-  if (isAppManagementEnabled()) {
-    if (organization) return selectDeveloperPlatformClientByOrg(organization)
-    return selectDeveloperPlatformClientByConfig(configuration)
-  }
-  return new PartnersClient()
+  if (isAppManagementDisabled()) return new PartnersClient()
+  if (organization) return selectDeveloperPlatformClientByOrg(organization)
+  return selectDeveloperPlatformClientByConfig(configuration)
 }
 
 function selectDeveloperPlatformClientByOrg(organization: Organization): DeveloperPlatformClient {

packages/cli-kit/src/private/node/api/headers.ts

@@ -4,7 +4,7 @@ import {Environment, serviceEnvironment} from '../context/service.js'
 import {ExtendableError} from '../../../public/node/error.js'
 import https from 'https'
 
-export class RequestClientError extends ExtendableError {
+class RequestClientError extends ExtendableError {
   statusCode: number
   public constructor(message: string, statusCode: number) {
     super(message)

packages/cli-kit/src/private/node/constants.ts

@@ -45,7 +45,6 @@ export const environmentVariables = {
   otelURL: 'SHOPIFY_CLI_OTEL_EXPORTER_OTLP_ENDPOINT',
   themeKitAccessDomain: 'SHOPIFY_CLI_THEME_KIT_ACCESS_DOMAIN',
   json: 'SHOPIFY_FLAG_JSON',
-  useAppManagement: 'USE_APP_MANAGEMENT_API',
 }
 
 export const defaultThemeKitAccessDomain = 'theme-kit-access.shopifyapps.com'

packages/cli-kit/src/private/node/session.ts

@@ -12,20 +12,14 @@ import {
 import {IdentityToken, Session} from './session/schema.js'
 import * as secureStore from './session/store.js'
 import {pollForDeviceAuthorization, requestDeviceAuthorization} from './session/device-authorization.js'
-import {RequestClientError} from './api/headers.js'
-import {getCachedPartnerAccountStatus, setCachedPartnerAccountStatus} from './conf-store.js'
 import {isThemeAccessSession} from './api/rest.js'
 import {outputContent, outputToken, outputDebug} from '../../public/node/output.js'
-import {firstPartyDev, isAppManagementEnabled, themeToken} from '../../public/node/context/local.js'
+import {firstPartyDev, themeToken} from '../../public/node/context/local.js'
 import {AbortError, BugError} from '../../public/node/error.js'
-import {partnersRequest} from '../../public/node/api/partners.js'
-import {normalizeStoreFqdn, partnersFqdn, identityFqdn} from '../../public/node/context/fqdn.js'
-import {openURL} from '../../public/node/system.js'
-import {keypress} from '../../public/node/ui.js'
+import {normalizeStoreFqdn, identityFqdn} from '../../public/node/context/fqdn.js'
 import {getIdentityTokenInformation, getPartnersToken} from '../../public/node/environment.js'
-import {gql} from 'graphql-request'
 import {AdminSession} from '@shopify/cli-kit/node/session'
-import {outputCompleted, outputInfo, outputWarn} from '@shopify/cli-kit/node/output'
+import {outputCompleted} from '@shopify/cli-kit/node/output'
 import {isSpin} from '@shopify/cli-kit/node/context/spin'
 import {nonRandomUUID} from '@shopify/cli-kit/node/crypto'
 
@@ -247,9 +241,6 @@ The CLI is currently unable to prompt for reauthentication.`,
   if (envToken && applications.partnersApi) {
     tokens.partners = (await exchangeCustomPartnerToken(envToken)).accessToken
   }
-  if (!envToken && tokens.partners) {
-    await ensureUserHasPartnerAccount(tokens.partners, tokens.userId)
-  }
 
   setLastSeenAuthMethod(envToken ? 'partners_token' : 'device_auth')
   setLastSeenUserIdAfterAuth(tokens.userId)
@@ -301,74 +292,6 @@ async function executeCompleteFlow(applications: OAuthApplications, identityFqdn
   return session
 }
 
-/**
- * If the user creates an account from the Identity website, the created
- * account won't get a Partner organization created. We need to detect that
- * and take the user to create a partner organization.
- *
- * @param partnersToken - Partners token.
- */
-async function ensureUserHasPartnerAccount(partnersToken: string, userId: string | undefined) {
-  if (isAppManagementEnabled()) return
-
-  outputDebug(outputContent`Verifying that the user has a Partner organization`)
-  if (!(await hasPartnerAccount(partnersToken, userId))) {
-    outputInfo(`\nA Shopify Partners organization is needed to proceed.`)
-    outputInfo(`👉 Press any key to create one`)
-    await keypress()
-    await openURL(`https://${await partnersFqdn()}/signup`)
-    outputInfo(outputContent`👉 Press any key when you have ${outputToken.cyan('created the organization')}`)
-    outputWarn(outputContent`Make sure you've confirmed your Shopify and the Partner organization from the email`)
-    await keypress()
-    if (!(await hasPartnerAccount(partnersToken, userId))) {
-      throw new AbortError(
-        `Couldn't find your Shopify Partners organization`,
-        `Have you confirmed your accounts from the emails you received?`,
-      )
-    }
-  }
-}
-
-// eslint-disable-next-line @shopify/cli/no-inline-graphql
-const getFirstOrganization = gql`
-  {
-    organizations(first: 1) {
-      nodes {
-        id
-      }
-    }
-  }
-`
-
-/**
- * Validate if the current token is valid for partners API.
- *
- * @param partnersToken - Partners token.
- * @returns A promise that resolves to true if the token is valid for partners API.
- */
-async function hasPartnerAccount(partnersToken: string, userId?: string): Promise<boolean> {
-  const cacheKey = userId ?? partnersToken
-  const cachedStatus = getCachedPartnerAccountStatus(cacheKey)
-
-  if (cachedStatus) {
-    outputDebug(`Confirmed partner account exists from cache`)
-    return true
-  }
-
-  try {
-    await partnersRequest(getFirstOrganization, partnersToken)
-    setCachedPartnerAccountStatus(cacheKey)
-    return true
-    // eslint-disable-next-line no-catch-all/no-catch-all
-  } catch (error) {
-    if (error instanceof RequestClientError && error.statusCode === 404) {
-      return false
-    } else {
-      return true
-    }
-  }
-}
-
 /**
  * Refresh the tokens for a given session.
  *

packages/cli-kit/src/private/node/session/exchange.test.ts

@@ -55,20 +55,18 @@ describe('exchange identity token for application tokens', () => {
 
   test('returns tokens for all APIs if a store is passed', async () => {
     // Given
-    const response = new Response(JSON.stringify(data))
-
-    // Need to do it 3 times because a Response can only be used once
-    vi.mocked(shopifyFetch)
-      .mockResolvedValue(response)
-      .mockResolvedValueOnce(response.clone())
-      .mockResolvedValueOnce(response.clone())
-      .mockResolvedValueOnce(response.clone())
+    vi.mocked(shopifyFetch).mockImplementation(async () => Promise.resolve(new Response(JSON.stringify(data))))
 
     // When
     const got = await exchangeAccessForApplicationTokens(identityToken, scopes, 'storeFQDN')
 
     // Then
     const expected = {
+      'app-management': {
+        accessToken: 'access_token',
+        expiresAt: expiredDate,
+        scopes: ['scope', 'scope2'],
+      },
       partners: {
         accessToken: 'access_token',
         expiresAt: expiredDate,
@@ -109,6 +107,11 @@ describe('exchange identity token for application tokens', () => {
 
     // Then
     const expected = {
+      'app-management': {
+        accessToken: 'access_token',
+        expiresAt: expiredDate,
+        scopes: ['scope', 'scope2'],
+      },
       partners: {
         accessToken: 'access_token',
         expiresAt: expiredDate,