Skip to content

Commit

Permalink
Merge pull request #4233 from Shopify/mitch/deploy-error-fixes
Browse files Browse the repository at this point in the history 
Run link before deploy and dev and a small refactor
  • Loading branch information
@MitchDickinson
MitchDickinson committed Jul 31, 2024
2 parents 204542c + 64750af commit 54f07a3
Show file tree
Hide file tree
Showing 3 changed files with 86 additions and 40 deletions.
70 changes: 51 additions & 19 deletions packages/app/src/cli/services/context.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -958,7 +958,7 @@ api_version = "2023-04"
describe('ensureDeployContext', () => {
test("fetches the app from the partners' API and returns it alongside the id when identifiers are available locally and the app has no extensions", async () => {
// Given
const app = testApp()
const app = testAppWithConfig({config: {client_id: APP2.apiKey}})
const identifiers = {
app: APP2.apiKey,
extensions: {},
Expand All @@ -969,6 +969,11 @@ describe('ensureDeployContext', () => {
vi.mocked(fetchAppDetailsFromApiKey).mockResolvedValueOnce(APP2)
vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers)
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(link).mockResolvedValue(app.configuration)
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(buildDeveloperPlatformClient())
const writeAppConfigurationFileSpy = vi
.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
.mockResolvedValue()

// When
const got = await ensureDeployContext(deployOptions(app))
Expand Down Expand Up @@ -1024,11 +1029,15 @@ describe('ensureDeployContext', () => {
vi.mocked(getAppIdentifiers).mockReturnValue({app: undefined})
vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers)
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(link).mockResolvedValue(app.configuration)

const writeAppConfigurationFileSpy = vi
.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
.mockResolvedValue()
const opts = deployOptions(app)

vi.mocked(selectDeveloperPlatformClient).mockReturnValue(opts.developerPlatformClient)

// When
const got = await ensureDeployContext(opts)

Expand All @@ -1050,17 +1059,22 @@ 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 app = testApp()
const app = testAppWithConfig({config: {client_id: APP1.apiKey}})
const identifiers = {
app: APP1.apiKey,
extensions: {},
extensionIds: {},
extensionsNonUuidManaged: {},
}
vi.mocked(getAppIdentifiers).mockReturnValue({app: undefined})
vi.mocked(fetchAppDetailsFromApiKey).mockResolvedValueOnce(APP2)
vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers)
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(link).mockResolvedValue({...app.configuration, organization_id: ORG1.id})

const writeAppConfigurationFileSpy = vi
.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
.mockResolvedValue()

const developerPlatformClient = buildDeveloperPlatformClient({
async orgAndApps(_orgId: string) {
return {
Expand All @@ -1077,15 +1091,8 @@ describe('ensureDeployContext', () => {
const got = await ensureDeployContext(opts)

// Then
expect(fetchOrganizations).toHaveBeenCalledOnce()
expect(selectOrCreateApp).toHaveBeenCalledWith(
app.name,
[APP1, APP2],
false,
ORG1,
opts.developerPlatformClient,
DEFAULT_SELECT_APP_OPTIONS,
)
expect(link).toBeCalledWith({directory: app.directory}, false)

expect(updateAppIdentifiers).toBeCalledWith({
app,
identifiers,
Expand All @@ -1101,14 +1108,19 @@ describe('ensureDeployContext', () => {

test("throws an app not found error if the app with the Client ID doesn't exist", async () => {
// Given
const app = testApp()
const app = testAppWithConfig()
vi.mocked(getAppIdentifiers).mockReturnValue({app: APP1.apiKey})
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(link).mockResolvedValue(app.configuration)

const developerPlatformClient = testDeveloperPlatformClient({
appFromId: vi.fn().mockRejectedValue(new AbortError("Couldn't find the app with Client ID key1")),
})
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(developerPlatformClient)

const opts = {
...deployOptions(app),
developerPlatformClient: testDeveloperPlatformClient({
appFromId: vi.fn().mockRejectedValue(new AbortError("Couldn't find the app with Client ID key1")),
}),
developerPlatformClient,
}

// When
Expand Down Expand Up @@ -1174,7 +1186,7 @@ describe('ensureDeployContext', () => {
})
test('load the app extension using the remote extensions specifications', async () => {
// Given
const app = testApp()
const app = testAppWithConfig({config: {client_id: APP2.apiKey}})
const identifiers = {
app: APP2.apiKey,
extensions: {},
Expand All @@ -1190,6 +1202,8 @@ describe('ensureDeployContext', () => {
vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers)
vi.mocked(loadApp).mockResolvedValue(appWithExtensions)
vi.mocked(updateAppIdentifiers).mockResolvedValue(appWithExtensions)
vi.mocked(link).mockResolvedValue(app.configuration)
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(buildDeveloperPlatformClient())

// When
const got = await ensureDeployContext(deployOptions(app))
Expand Down Expand Up @@ -1220,6 +1234,9 @@ describe('ensureDeployContext', () => {
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)
vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml')
vi.mocked(link).mockResolvedValue(app.configuration)
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(buildDeveloperPlatformClient())

const writeAppConfigurationFileSpy = vi
.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
.mockResolvedValue()
Expand Down Expand Up @@ -1270,6 +1287,8 @@ describe('ensureDeployContext', () => {
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(renderConfirmationPrompt).mockResolvedValue(false)
vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml')
vi.mocked(link).mockResolvedValue(app.configuration)
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(buildDeveloperPlatformClient())
const writeAppConfigurationFileSpy = vi
.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
.mockResolvedValue()
Expand Down Expand Up @@ -1314,14 +1333,19 @@ describe('ensureDeployContext', () => {
vi.mocked(fetchAppDetailsFromApiKey).mockResolvedValueOnce(APP2)
vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers)
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(link).mockResolvedValue((app as any).configuration)
// vi.mocked(selectDeveloperPlatformClient).mockReturnValue(testDeveloperPlatformClient)
vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml')
const writeAppConfigurationFileSpy = vi
.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
.mockResolvedValue()
const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {})

const options = deployOptions(app)
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient)

// When
await ensureDeployContext(deployOptions(app))
await ensureDeployContext(options)

// Then
expect(metadataSpyOn).toHaveBeenNthCalledWith(2, expect.any(Function))
Expand Down Expand Up @@ -1370,6 +1394,8 @@ describe('ensureDeployContext', () => {
.mockResolvedValue()
const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {})

const options = deployOptions(app, true)
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient)
// When
await ensureDeployContext(deployOptions(app, true))

Expand Down Expand Up @@ -1413,14 +1439,18 @@ describe('ensureDeployContext', () => {
vi.mocked(fetchAppDetailsFromApiKey).mockResolvedValueOnce(APP2)
vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers)
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(link).mockResolvedValue(app.configuration)
vi.mocked(renderConfirmationPrompt).mockResolvedValue(false)
vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml')
const writeAppConfigurationFileSpy = vi
.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
.mockResolvedValue()

const options = deployOptions(app, false, true)
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient)

// When
await ensureDeployContext(deployOptions(app, false, true))
await ensureDeployContext(options)

// Then
expect(renderConfirmationPrompt).not.toHaveBeenCalled()
Expand Down Expand Up @@ -1458,6 +1488,8 @@ describe('ensureDeployContext', () => {
vi.mocked(loadApp).mockResolvedValue(app)
vi.mocked(renderConfirmationPrompt).mockResolvedValue(false)
vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml')
vi.mocked(link).mockResolvedValue(app.configuration)
vi.mocked(selectDeveloperPlatformClient).mockReturnValue(buildDeveloperPlatformClient())
const writeAppConfigurationFileSpy = vi
.spyOn(writeAppConfigurationFile, 'writeAppConfigurationFile')
.mockResolvedValue()
Expand Down
54 changes: 34 additions & 20 deletions packages/app/src/cli/services/context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ export async function ensureDevContext(options: DevContextOptions): Promise<DevC
let developerPlatformClient = options.developerPlatformClient
const {configuration, cachedInfo, remoteApp} = await getAppContext({
...options,
promptLinkingApp: !options.apiKey,
enableLinkingPrompt: !options.apiKey,
})
developerPlatformClient = remoteApp?.developerPlatformClient ?? developerPlatformClient

Expand Down Expand Up @@ -429,7 +429,9 @@ export interface DeployContextOptions {
export async function ensureDeployContext(options: DeployContextOptions): Promise<DeployContextOutput> {
const {reset, force, noRelease} = options
let developerPlatformClient = options.developerPlatformClient
const [remoteApp] = await fetchAppAndIdentifiers(options, developerPlatformClient)
// do the link here
const [remoteApp] = await fetchAppAndIdentifiers(options, developerPlatformClient, true, true)

developerPlatformClient = remoteApp.developerPlatformClient ?? developerPlatformClient

const specifications = await fetchSpecifications({developerPlatformClient, app: remoteApp})
Expand Down Expand Up @@ -556,7 +558,7 @@ function includeConfigOnDeployPrompt(configPath: string): Promise<boolean> {
export async function ensureReleaseContext(options: ReleaseContextOptions): Promise<ReleaseContextOutput> {
let developerPlatformClient =
options.developerPlatformClient ?? selectDeveloperPlatformClient({configuration: options.app.configuration})
const [remoteApp, envIdentifiers] = await fetchAppAndIdentifiers(options, developerPlatformClient)
const [remoteApp, envIdentifiers] = await fetchAppAndIdentifiers(options, developerPlatformClient, true, true)
developerPlatformClient = remoteApp.developerPlatformClient ?? developerPlatformClient
const identifiers: Identifiers = envIdentifiers as Identifiers

Expand Down Expand Up @@ -638,19 +640,22 @@ export async function fetchAppAndIdentifiers(
reset: boolean
apiKey?: string
},
developerPlatformClient: DeveloperPlatformClient,
initialDeveloperPlatformClient: DeveloperPlatformClient,
reuseFromDev = true,
enableLinkingPrompt = false,
): Promise<[OrganizationApp, Partial<UuidOnlyIdentifiers>]> {
let developerPlatformClient = initialDeveloperPlatformClient
const app = options.app
let reuseDevCache = reuseFromDev
let envIdentifiers = getAppIdentifiers({app}, developerPlatformClient)
let remoteApp: OrganizationApp | undefined

if (options.reset) {
const configuration = await linkIfNecessary(app.directory, options.reset, enableLinkingPrompt)
if (configuration !== undefined) {
envIdentifiers = {app: undefined, extensions: {}}
reuseDevCache = false
const configuration = await link({directory: app.directory, developerPlatformClient})
app.configuration = configuration
developerPlatformClient = selectDeveloperPlatformClient({configuration})
}

if (isCurrentAppSchema(app.configuration)) {
Expand Down Expand Up @@ -750,26 +755,15 @@ export async function getAppContext({
directory,
developerPlatformClient,
configName,
promptLinkingApp = true,
enableLinkingPrompt = true,
}: {
reset: boolean
directory: string
developerPlatformClient: DeveloperPlatformClient
configName?: string
promptLinkingApp?: boolean
enableLinkingPrompt?: boolean
}): Promise<AppContext> {
const previousCachedInfo = getCachedAppInfo(directory)

if (reset) clearCachedAppInfo(directory)

const firstTimeSetup = previousCachedInfo === undefined
const usingConfigAndResetting = previousCachedInfo?.configFile && reset
const usingConfigWithNoTomls =
previousCachedInfo?.configFile && (await glob(joinPath(directory, 'shopify.app*.toml'))).length === 0

if (promptLinkingApp && (firstTimeSetup || usingConfigAndResetting || usingConfigWithNoTomls)) {
await link({directory, baseConfigName: previousCachedInfo?.configFile}, false)
}
await linkIfNecessary(directory, reset, enableLinkingPrompt)

let cachedInfo = getCachedAppInfo(directory)

Expand Down Expand Up @@ -807,6 +801,26 @@ export async function getAppContext({
}
}

async function linkIfNecessary(
directory: string,
reset: boolean,
enableLinkingPrompt: boolean,
): Promise<CurrentAppConfiguration | undefined> {
const previousCachedInfo = getCachedAppInfo(directory)

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 performAppLink = enableLinkingPrompt && (firstTimeSetup || usingConfigAndResetting || usingConfigWithNoTomls)
if (performAppLink) {
return link({directory, baseConfigName: previousCachedInfo?.configFile}, false)
}
}

/**
* Fetch all orgs the user belongs to and show a prompt to select one of them
* @param developerPlatformClient - The client to access the platform API
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/info.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ class AppInfo {
directory: this.app.directory,
reset: false,
configName: this.options.configName,
promptLinkingApp: false,
enableLinkingPrompt: false,
})
developerPlatformClient = remoteApp?.developerPlatformClient ?? developerPlatformClient

Expand Down

0 comments on commit 54f07a3

Please sign in to comment.