Skip to content

Commit

Permalink
Merge pull request #5003 from Shopify/centralize-app-management-gating
Browse files Browse the repository at this point in the history
Centralize gating of the App Management API
  • Loading branch information
amcaplan authored Dec 2, 2024
2 parents 2af2272 + b81ec36 commit 7fc88c3
Show file tree
Hide file tree
Showing 11 changed files with 54 additions and 20 deletions.
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/deploy/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {joinPath} from '@shopify/cli-kit/node/path'
describe('bundleAndBuildExtensions', () => {
let app: AppInterface

test('generates a manifest.json when USE_APP_MANAGEMENT_API is enabled', async () => {
test('generates a manifest.json when App Management is enabled', async () => {
await file.inTemporaryDirectory(async (tmpDir: string) => {
// Given
vi.spyOn(file, 'writeFileSync').mockResolvedValue(undefined)
Expand Down Expand Up @@ -73,7 +73,7 @@ describe('bundleAndBuildExtensions', () => {
})
})

test('does not generate the manifest.json when USE_APP_MANAGEMENT_API is disabled', async () => {
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)
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/deploy/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {zip} from '@shopify/cli-kit/node/archiver'
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 {isTruthy} from '@shopify/cli-kit/node/context/utilities'
import {renderConcurrent} from '@shopify/cli-kit/node/ui'
import {isAppManagementEnabled} from '@shopify/cli-kit/node/context/local'
import {Writable} from 'stream'

interface BundleOptions {
Expand All @@ -21,7 +21,7 @@ export async function bundleAndBuildExtensions(options: BundleOptions, systemEnv
mkdirSync(bundleDirectory)
await touchFile(joinPath(bundleDirectory, '.shopify'))

if (isTruthy(systemEnvironment.USE_APP_MANAGEMENT_API)) {
if (isAppManagementEnabled(systemEnvironment)) {
// Include manifest in bundle
const appManifest = await options.app.manifest()
const manifestPath = joinPath(bundleDirectory, 'manifest.json')
Expand Down
4 changes: 2 additions & 2 deletions packages/app/src/cli/services/dev/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ afterEach(() => {
})

describe('fetchOrganizations', async () => {
test('returns fetched organizations from Partners without USE_APP_MANAGEMENT_API', async () => {
test('returns fetched organizations from Partners when App Management is disabled', async () => {
// Given
const partnersClient: PartnersClient = testDeveloperPlatformClient({
organizations: () => Promise.resolve([ORG1]),
Expand All @@ -74,7 +74,7 @@ describe('fetchOrganizations', async () => {
expect(appManagementClient.organizations).not.toHaveBeenCalled()
})

test('returns fetched organizations from Partners and App Management with USE_APP_MANAGEMENT_API', async () => {
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({
Expand Down
6 changes: 3 additions & 3 deletions packages/app/src/cli/utilities/developer-platform-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,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 {isTruthy} from '@shopify/cli-kit/node/context/utilities'
import {isAppManagementEnabled} from '@shopify/cli-kit/node/context/local'

export enum ClientName {
AppManagement = 'app-management',
Expand All @@ -77,7 +77,7 @@ export interface AppVersionIdentifiers {

export function allDeveloperPlatformClients(): DeveloperPlatformClient[] {
const clients: DeveloperPlatformClient[] = [new PartnersClient()]
if (isTruthy(process.env.USE_APP_MANAGEMENT_API)) clients.push(new AppManagementClient())
if (isAppManagementEnabled()) clients.push(new AppManagementClient())
return clients
}

Expand Down Expand Up @@ -117,7 +117,7 @@ export function selectDeveloperPlatformClient({
configuration,
organization,
}: SelectDeveloperPlatformClientOptions = {}): DeveloperPlatformClient {
if (isTruthy(process.env.USE_APP_MANAGEMENT_API)) {
if (isAppManagementEnabled()) {
if (organization) return selectDeveloperPlatformClientByOrg(organization)
return selectDeveloperPlatformClientByConfig(configuration)
}
Expand Down
1 change: 1 addition & 0 deletions packages/cli-kit/src/private/node/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ 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'
Expand Down
5 changes: 2 additions & 3 deletions packages/cli-kit/src/private/node/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ 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, themeToken} from '../../public/node/context/local.js'
import {firstPartyDev, isAppManagementEnabled, 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'
Expand All @@ -26,7 +26,6 @@ import {getIdentityTokenInformation, getPartnersToken} from '../../public/node/e
import {gql} from 'graphql-request'
import {AdminSession} from '@shopify/cli-kit/node/session'
import {outputCompleted, outputInfo, outputWarn} from '@shopify/cli-kit/node/output'
import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
import {isSpin} from '@shopify/cli-kit/node/context/spin'
import {nonRandomUUID} from '@shopify/cli-kit/node/crypto'

Expand Down Expand Up @@ -310,7 +309,7 @@ async function executeCompleteFlow(applications: OAuthApplications, identityFqdn
* @param partnersToken - Partners token.
*/
async function ensureUserHasPartnerAccount(partnersToken: string, userId: string | undefined) {
if (isTruthy(process.env.USE_APP_MANAGEMENT_API)) return
if (isAppManagementEnabled()) return

outputDebug(outputContent`Verifying that the user has a Partner organization`)
if (!(await hasPartnerAccount(partnersToken, userId))) {
Expand Down
4 changes: 2 additions & 2 deletions packages/cli-kit/src/private/node/session/exchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import {identityFqdn} from '../../../public/node/context/fqdn.js'
import {shopifyFetch} from '../../../public/node/http.js'
import {err, ok, Result} from '../../../public/node/result.js'
import {AbortError, BugError, ExtendableError} from '../../../public/node/error.js'
import {isAppManagementEnabled} from '../../../public/node/context/local.js'
import {setLastSeenAuthMethod, setLastSeenUserIdAfterAuth} from '../session.js'
import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
import * as jose from 'jose'
import {nonRandomUUID} from '@shopify/cli-kit/node/crypto'

Expand Down Expand Up @@ -34,7 +34,7 @@ export async function exchangeAccessForApplicationTokens(
store?: string,
): Promise<{[x: string]: ApplicationToken}> {
const token = identityToken.accessToken
const appManagementEnabled = isTruthy(process.env.USE_APP_MANAGEMENT_API)
const appManagementEnabled = isAppManagementEnabled()

const [partners, storefront, businessPlatform, admin, appManagement] = await Promise.all([
requestAppToken('partners', token, scopes.partners),
Expand Down
3 changes: 2 additions & 1 deletion packages/cli-kit/src/private/node/session/scopes.test.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import {allDefaultScopes, apiScopes} from './scopes.js'
import {environmentVariables} from '../constants.js'
import {describe, expect, test} from 'vitest'

describe('allDefaultScopes', () => {
Expand All @@ -25,7 +26,7 @@ describe('allDefaultScopes', () => {

test('includes App Management and Store Management when the required env var is defined', async () => {
// Given
const envVars = {USE_APP_MANAGEMENT_API: 'true'}
const envVars = {[environmentVariables.useAppManagement]: 'true'}

// When
const got = allDefaultScopes([], envVars)
Expand Down
8 changes: 3 additions & 5 deletions packages/cli-kit/src/private/node/session/scopes.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import {allAPIs, API} from '../api.js'
import {BugError} from '../../../public/node/error.js'
import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
import {isAppManagementEnabled} from '../../../public/node/context/local.js'

/**
* Generate a flat array with all the default scopes for all the APIs plus
Expand Down Expand Up @@ -35,11 +35,9 @@ function defaultApiScopes(api: API, systemEnvironment = process.env): string[] {
case 'partners':
return ['cli']
case 'business-platform':
return isTruthy(systemEnvironment.USE_APP_MANAGEMENT_API)
? ['destinations', 'store-management']
: ['destinations']
return isAppManagementEnabled(systemEnvironment) ? ['destinations', 'store-management'] : ['destinations']
case 'app-management':
return isTruthy(systemEnvironment.USE_APP_MANAGEMENT_API) ? ['app-management'] : []
return isAppManagementEnabled(systemEnvironment) ? ['app-management'] : []
default:
throw new BugError(`Unknown API: ${api}`)
}
Expand Down
25 changes: 25 additions & 0 deletions packages/cli-kit/src/public/node/context/local.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
isDevelopment,
isShopify,
isUnitTest,
isAppManagementEnabled,
analyticsDisabled,
cloudEnvironment,
macAddress,
Expand Down Expand Up @@ -99,6 +100,30 @@ describe('hasGit', () => {
})
})

describe('isAppManagementEnabled', () => {
test('returns true when USE_APP_MANAGEMENT_API is truthy', () => {
// Given
const env = {USE_APP_MANAGEMENT_API: '1'}

// When
const got = isAppManagementEnabled(env)

// Then
expect(got).toBe(true)
})

test('returns false when USE_APP_MANAGEMENT_API is falsy', () => {
// Given
const env = {USE_APP_MANAGEMENT_API: '0'}

// When
const got = isAppManagementEnabled(env)

// Then
expect(got).toBe(false)
})
})

describe('analitycsDisabled', () => {
test('returns true when SHOPIFY_CLI_NO_ANALYTICS is truthy', () => {
// Given
Expand Down
10 changes: 10 additions & 0 deletions packages/cli-kit/src/public/node/context/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,16 @@ export function isVerbose(env = process.env): boolean {
return isTruthy(env[environmentVariables.verbose]) || process.argv.includes('--verbose')
}

/**
* It returns true if the App Management API is available.
*
* @param env - The environment variables from the environment of the current process.
* @returns True if the App Management API is available.
*/
export function isAppManagementEnabled(env = process.env): boolean {
return isTruthy(env[environmentVariables.useAppManagement])
}

/**
* Returns true if the environment in which the CLI is running is either
* a local environment (where dev is present) or a cloud environment (spin).
Expand Down

0 comments on commit 7fc88c3

Please sign in to comment.