Skip to content

Commit

Permalink
Remove all conditional logic around App Management being disabled
Browse files Browse the repository at this point in the history
  • Loading branch information
amcaplan committed Jan 2, 2025
1 parent 7aa290c commit 9ecb7d4
Show file tree
Hide file tree
Showing 12 changed files with 30 additions and 211 deletions.
42 changes: 3 additions & 39 deletions packages/app/src/cli/services/deploy/bundle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'})
Expand Down Expand Up @@ -60,7 +59,7 @@ describe('bundleAndBuildExtensions', () => {
}

// When
await bundleAndBuildExtensions({app, identifiers, bundlePath}, envVars)
await bundleAndBuildExtensions({app, identifiers, bundlePath})

// Then
expect(extensionBundleMock).toHaveBeenCalledTimes(2)
Expand All @@ -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
Expand All @@ -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()
Expand Down
13 changes: 5 additions & 8 deletions packages/app/src/cli/services/deploy/bundle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
Expand Down
27 changes: 5 additions & 22 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 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]),
Expand All @@ -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()
Expand All @@ -100,7 +79,11 @@ 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()
Expand Down
11 changes: 3 additions & 8 deletions packages/app/src/cli/utilities/developer-platform-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ 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'

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

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

Expand Down Expand Up @@ -118,11 +116,8 @@ export function selectDeveloperPlatformClient({
configuration,
organization,
}: SelectDeveloperPlatformClientOptions = {}): DeveloperPlatformClient {
if (isAppManagementEnabled()) {
if (organization) return selectDeveloperPlatformClientByOrg(organization)
return selectDeveloperPlatformClientByConfig(configuration)
}
return new PartnersClient()
if (organization) return selectDeveloperPlatformClientByOrg(organization)
return selectDeveloperPlatformClientByConfig(configuration)
}

function selectDeveloperPlatformClientByOrg(organization: Organization): DeveloperPlatformClient {
Expand Down
2 changes: 1 addition & 1 deletion packages/cli-kit/src/private/node/api/headers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 0 additions & 1 deletion packages/cli-kit/src/private/node/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
83 changes: 3 additions & 80 deletions packages/cli-kit/src/private/node/session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
*
Expand Down
4 changes: 1 addition & 3 deletions packages/cli-kit/src/private/node/session/exchange.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ 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 * as jose from 'jose'
import {nonRandomUUID} from '@shopify/cli-kit/node/crypto'
Expand Down Expand Up @@ -34,14 +33,13 @@ export async function exchangeAccessForApplicationTokens(
store?: string,
): Promise<{[x: string]: ApplicationToken}> {
const token = identityToken.accessToken
const appManagementEnabled = isAppManagementEnabled()

const [partners, storefront, businessPlatform, admin, appManagement] = await Promise.all([
requestAppToken('partners', token, scopes.partners),
requestAppToken('storefront-renderer', token, scopes.storefront),
requestAppToken('business-platform', token, scopes.businessPlatform),
store ? requestAppToken('admin', token, scopes.admin, store) : {},
appManagementEnabled ? requestAppToken('app-management', token, scopes.appManagement) : {},
requestAppToken('app-management', token, scopes.appManagement),
])

return {
Expand Down
8 changes: 2 additions & 6 deletions packages/cli-kit/src/private/node/session/scopes.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import {allDefaultScopes, apiScopes} from './scopes.js'
import {environmentVariables} from '../constants.js'
import {describe, expect, test} from 'vitest'

describe('allDefaultScopes', () => {
Expand All @@ -24,12 +23,9 @@ describe('allDefaultScopes', () => {
])
})

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

test('includes App Management and Store Management', async () => {
// When
const got = allDefaultScopes([], envVars)
const got = allDefaultScopes([])

// Then
expect(got).toEqual([
Expand Down
Loading

0 comments on commit 9ecb7d4

Please sign in to comment.