Skip to content

Commit

Permalink
Merge pull request #5172 from Shopify/jg/hide-partners-banner-app-url…
Browse files Browse the repository at this point in the history 
…-when-using-appmgmtapi

Hide all banners that tell you to change app URL
  • Loading branch information
@jamieguerrero
jamieguerrero authored Jan 16, 2025
2 parents 43059d7 + a622479 commit e2b829d
Show file tree
Hide file tree
Showing 5 changed files with 11 additions and 184 deletions.
5 changes: 2 additions & 3 deletions packages/app/src/cli/services/dev.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import {fetchAppPreviewMode} from './dev/fetch.js'
import {installAppDependencies} from './dependencies.js'
import {DevConfig, DevProcesses, setupDevProcesses} from './dev/processes/setup-dev-processes.js'
import {frontAndBackendConfig} from './dev/processes/utils.js'
import {outputUpdateURLsResult, renderDev} from './dev/ui.js'
import {renderDev} from './dev/ui.js'
import {DeveloperPreviewController} from './dev/ui/components/Dev.js'
import {DevProcessFunction} from './dev/processes/types.js'
import {getCachedAppInfo, setCachedAppInfo} from './local-storage.js'
Expand Down Expand Up @@ -264,7 +264,7 @@ async function handleUpdatingOfPartnerUrls(
const newURLs = generatePartnersURLs(
network.proxyUrl,
webs.map(({configuration}) => configuration.auth_callback_path).find((path) => path),
isCurrentAppSchema(localApp.configuration) ? localApp.configuration.app_proxy : undefined,
localApp.configuration.app_proxy,
)
shouldUpdateURLs = await shouldOrPromptUpdateURLs({
currentURLs: network.currentUrls,
Expand All @@ -277,7 +277,6 @@ async function handleUpdatingOfPartnerUrls(
// When running dev app urls are pushed directly to API Client config instead of creating a new app version
// so current app version and API Client config will have diferent url values.
if (shouldUpdateURLs) await updateURLs(newURLs, apiKey, developerPlatformClient, localApp)
await outputUpdateURLsResult(shouldUpdateURLs, newURLs, remoteApp, localApp)
}
}
return shouldUpdateURLs
Expand Down
104 changes: 1 addition & 103 deletions packages/app/src/cli/services/dev/ui.test.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import {outputUpdateURLsResult, renderDev} from './ui.js'
import {renderDev} from './ui.js'
import {Dev} from './ui/components/Dev.js'
import {
testApp,
testDeveloperPlatformClient,
testFunctionExtension,
testOrganizationApp,
testThemeExtensions,
testUIExtension,
} from '../../models/app/app.test-data.js'
Expand Down Expand Up @@ -32,107 +31,6 @@ afterEach(() => {
mockAndCaptureOutput().clear()
})

describe('output', () => {
describe('outputUpdateURLsResult', () => {
const urls = {
applicationUrl: 'https://lala.cloudflare.io/',
redirectUrlWhitelist: ['https://lala.cloudflare.io/auth/callback'],
}

test('shows info about tunnel URL and links to Partners Dashboard when app is brand new', async () => {
// Given
const outputMock = mockAndCaptureOutput()
const localApp = await mockApp()

const remoteApp = testOrganizationApp({newApp: true})

// When
await outputUpdateURLsResult(true, urls, remoteApp, localApp)

// Then
expect(outputMock.output()).toMatchInlineSnapshot(`
"╭─ info ───────────────────────────────────────────────────────────────────────╮
│ │
│ For your convenience, we've given your app a default URL: │
│ https://lala.cloudflare.io/. │
│ │
│ You can update your app's URL anytime in the Partners Dashboard [1] But │
│ once your app is live, updating its URL will disrupt user access. │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
[1] https://partners.shopify.com/1/apps/1/edit
"
`)
})

test('shows nothing when urls were updated', async () => {
// Given
const outputMock = mockAndCaptureOutput()
const localApp = await mockApp()

const remoteApp = testOrganizationApp({newApp: false})

// When
await outputUpdateURLsResult(true, urls, remoteApp, localApp)

// Then
expect(outputMock.output()).toEqual('')
})

test('shows how to update app urls on partners when app is not brand new, urls were not updated and app uses legacy config', async () => {
// Given
const outputMock = mockAndCaptureOutput()
const localApp = await mockApp()

const remoteApp = testOrganizationApp({newApp: false})

// When
await outputUpdateURLsResult(false, urls, remoteApp, localApp)

// Then
expect(outputMock.output()).toMatchInlineSnapshot(`
"╭─ info ───────────────────────────────────────────────────────────────────────╮
│ │
│ To make URL updates manually, you can add the following URLs as redirects │
│ in your Partners Dashboard [1]: │
│ │
│ │
│ • https://lala.cloudflare.io/auth/callback │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
[1] https://partners.shopify.com/1/apps/1/edit
"
`)
})

test('shows how to update app urls with config push when app is not brand new, urls were updated and app uses new config', async () => {
// Given
const outputMock = mockAndCaptureOutput()
const localApp = await mockApp(true)

const remoteApp = testOrganizationApp({newApp: false})

// When
await outputUpdateURLsResult(false, urls, remoteApp, localApp)

// Then
expect(outputMock.output()).toMatchInlineSnapshot(`
"╭─ info ───────────────────────────────────────────────────────────────────────╮
│ │
│ To update URLs manually, add the following URLs to │
│ shopify.app.staging.toml under auth > redirect_urls and run │
│ \`yarn shopify app config push --config=staging\` │
│ │
│ │
│ • https://lala.cloudflare.io/auth/callback │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
"
`)
})
})
})

describe('ui', () => {
describe('renderDev', () => {
test("doesn't use ink when terminal doesn't support TTY", async () => {
Expand Down
68 changes: 1 addition & 67 deletions packages/app/src/cli/services/dev/ui.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,67 +1,10 @@
import {PartnersURLs} from './urls.js'
import {Dev, DevProps} from './ui/components/Dev.js'
import {AppInterface, isCurrentAppSchema} from '../../models/app/app.js'
import {OrganizationApp} from '../../models/organization.js'
import {getAppConfigurationShorthand} from '../../models/app/loader.js'
import React from 'react'
import {partnersFqdn} from '@shopify/cli-kit/node/context/fqdn'
import {render, renderInfo} from '@shopify/cli-kit/node/ui'
import {basename} from '@shopify/cli-kit/node/path'
import {formatPackageManagerCommand} from '@shopify/cli-kit/node/output'
import {render} from '@shopify/cli-kit/node/ui'
import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system'
import {isTruthy} from '@shopify/cli-kit/node/context/utilities'
import {isUnitTest} from '@shopify/cli-kit/node/context/local'

export async function outputUpdateURLsResult(
updated: boolean,
urls: PartnersURLs,
remoteApp: OrganizationApp,
localApp: AppInterface,
) {
const dashboardURL = await partnersURL(remoteApp.organizationId, remoteApp.id)
if (remoteApp.newApp) {
renderInfo({
headline: `For your convenience, we've given your app a default URL: ${urls.applicationUrl}.`,
body: [
"You can update your app's URL anytime in the",
dashboardURL,
'But once your app is live, updating its URL will disrupt user access.',
],
})
} else if (!updated) {
if (isCurrentAppSchema(localApp.configuration)) {
const fileName = basename(localApp.configuration.path)
const configName = getAppConfigurationShorthand(fileName)
const pushCommandArgs = configName ? [`--config=${configName}`] : []

renderInfo({
body: [
`To update URLs manually, add the following URLs to ${fileName} under auth > redirect_urls and run\n`,
{
command: formatPackageManagerCommand(
localApp.packageManager,
`shopify app config push`,
...pushCommandArgs,
),
},
'\n\n',
{list: {items: urls.redirectUrlWhitelist}},
],
})
} else {
renderInfo({
body: [
'To make URL updates manually, you can add the following URLs as redirects in your',
dashboardURL,
{char: ':'},
'\n\n',
{list: {items: urls.redirectUrlWhitelist}},
],
})
}
}
}

export async function renderDev({
processes,
previewUrl,
Expand Down Expand Up @@ -94,15 +37,6 @@ export async function renderDev({
}
}

async function partnersURL(organizationId: string, appId: string) {
return {
link: {
label: 'Partners Dashboard',
url: `https://${await partnersFqdn()}/${organizationId}/apps/${appId}/edit`,
},
}
}

async function renderDevNonInteractive({
processes,
app: {canEnablePreviewMode},
Expand Down
5 changes: 3 additions & 2 deletions packages/app/src/cli/services/dev/urls.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import {UpdateURLsVariables} from '../../api/graphql/update_urls.js'
import {setCachedAppInfo} from '../local-storage.js'
import {patchAppConfigurationFile} from '../app/patch-app-configuration-file.js'
import {AppLinkedInterface} from '../../models/app/app.js'
import {beforeEach, describe, expect, vi, test} from 'vitest'
import {AbortError} from '@shopify/cli-kit/node/error'
import {checkPortAvailability, getAvailableTCPPort} from '@shopify/cli-kit/node/tcp'
Expand Down Expand Up @@ -314,7 +315,7 @@ describe('shouldOrPromptUpdateURLs', () => {
currentURLs,
appDirectory: '/path',
apiKey: 'api-key',
localApp: testApp({configuration: {...DEFAULT_CONFIG, client_id: 'different'}}, 'current'),
localApp: testApp({configuration: {...DEFAULT_CONFIG, client_id: 'different'}}, 'current') as AppLinkedInterface,
}
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)

Expand All @@ -334,7 +335,7 @@ describe('shouldOrPromptUpdateURLs', () => {
currentURLs,
appDirectory: '/path',
apiKey: 'api-key',
localApp,
localApp: localApp as AppLinkedInterface,
}
vi.mocked(renderConfirmationPrompt).mockResolvedValue(true)

Expand Down
13 changes: 4 additions & 9 deletions packages/app/src/cli/services/dev/urls.ts
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,5 @@
import {updateURLsPrompt} from '../../prompts/dev.js'
import {
AppConfigurationInterface,
AppInterface,
CurrentAppConfiguration,
isCurrentAppSchema,
} from '../../models/app/app.js'
import {AppConfigurationInterface, AppLinkedInterface, CurrentAppConfiguration} from '../../models/app/app.js'
import {UpdateURLsSchema, UpdateURLsVariables} from '../../api/graphql/update_urls.js'
import {setCachedAppInfo} from '../local-storage.js'
import {AppConfigurationUsedByCli} from '../../models/extensions/specifications/types/app_config.js'
Expand Down Expand Up @@ -202,7 +197,7 @@ export async function updateURLs(
throw new AbortError(errors)
}

if (localApp && isCurrentAppSchema(localApp.configuration) && localApp.configuration.client_id === apiKey) {
if (localApp && localApp.configuration.client_id === apiKey) {
const patch = {
application_url: urls.applicationUrl,
auth: {
Expand Down Expand Up @@ -243,7 +238,7 @@ interface ShouldOrPromptUpdateURLsOptions {
appDirectory: string
cachedUpdateURLs?: boolean
newApp?: boolean
localApp?: AppInterface
localApp?: AppLinkedInterface
apiKey: string
}

Expand All @@ -258,7 +253,7 @@ export async function shouldOrPromptUpdateURLs(options: ShouldOrPromptUpdateURLs
options.currentURLs.redirectUrlWhitelist,
)

if (options.localApp && isCurrentAppSchema(options.localApp.configuration)) {
if (options.localApp) {
const localConfiguration = options.localApp.configuration
localConfiguration.build = {
...localConfiguration.build,
Expand Down

0 comments on commit e2b829d

Please sign in to comment.