From 3e980d95aa95678e7d6619de06eb7740ffd5b119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Fri, 18 Oct 2024 12:12:44 +0200 Subject: [PATCH] revert changes to urls.ts --- .../app/src/cli/services/dev/urls.test.ts | 69 +++++++++++++++++-- packages/app/src/cli/services/dev/urls.ts | 23 ++++--- 2 files changed, 76 insertions(+), 16 deletions(-) diff --git a/packages/app/src/cli/services/dev/urls.test.ts b/packages/app/src/cli/services/dev/urls.test.ts index 07cf37c7da7..b0874807f27 100644 --- a/packages/app/src/cli/services/dev/urls.test.ts +++ b/packages/app/src/cli/services/dev/urls.test.ts @@ -10,11 +10,11 @@ import { import { DEFAULT_CONFIG, testApp, - testAppLinked, testAppWithConfig, testDeveloperPlatformClient, } from '../../models/app/app.test-data.js' import {UpdateURLsVariables} from '../../api/graphql/update_urls.js' +import {setCachedAppInfo} from '../local-storage.js' import {patchAppConfigurationFile} from '../app/patch-app-configuration-file.js' import {beforeEach, describe, expect, vi, test} from 'vitest' import {AbortError} from '@shopify/cli-kit/node/error' @@ -216,7 +216,6 @@ describe('shouldOrPromptUpdateURLs', () => { appDirectory: '/path', newApp: true, apiKey: 'api-key', - localApp: testAppLinked({configuration: {...DEFAULT_CONFIG, client_id: 'api-key'}}), } // When @@ -226,13 +225,44 @@ describe('shouldOrPromptUpdateURLs', () => { expect(got).toEqual(true) }) + test('returns true if the cached value is true (always)', async () => { + // Given + const options = { + currentURLs, + appDirectory: '/path', + cachedUpdateURLs: true, + apiKey: 'api-key', + } + + // When + const got = await shouldOrPromptUpdateURLs(options) + + // Then + expect(got).toEqual(true) + }) + + test('returns false if the cached value is false (never)', async () => { + // Given + const options = { + currentURLs, + appDirectory: '/path', + cachedUpdateURLs: false, + apiKey: 'api-key', + } + + // When + const got = await shouldOrPromptUpdateURLs(options) + + // Then + expect(got).toEqual(false) + }) + test('returns true when the user selects yes', async () => { // Given const options = { currentURLs, appDirectory: '/path', apiKey: 'api-key', - localApp: testAppLinked({configuration: {...DEFAULT_CONFIG, client_id: 'api-key'}}), } vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) @@ -249,7 +279,6 @@ describe('shouldOrPromptUpdateURLs', () => { currentURLs, appDirectory: '/path', apiKey: 'api-key', - localApp: testAppLinked({configuration: {...DEFAULT_CONFIG, client_id: 'api-key'}}), } vi.mocked(renderConfirmationPrompt).mockResolvedValue(false) @@ -260,13 +289,32 @@ describe('shouldOrPromptUpdateURLs', () => { expect(got).toEqual(false) }) - test('does not update config file if current config client does not match remote', async () => { + test('saves the response for the next time', async () => { + // Given + const options = { + currentURLs, + appDirectory: '/path', + apiKey: 'api-key', + } + vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) + + // When + await shouldOrPromptUpdateURLs(options) + + // Then + expect(setCachedAppInfo).toHaveBeenNthCalledWith(1, { + directory: '/path', + updateURLs: true, + }) + }) + + test('does not update config file or cache if current config client does not match remote', async () => { // Given const options = { currentURLs, appDirectory: '/path', apiKey: 'api-key', - localApp: testAppLinked({configuration: {...DEFAULT_CONFIG, client_id: 'different'}}), + localApp: testApp({configuration: {...DEFAULT_CONFIG, client_id: 'different'}}, 'current'), } vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) @@ -275,12 +323,13 @@ describe('shouldOrPromptUpdateURLs', () => { // Then expect(result).toBe(true) + expect(setCachedAppInfo).not.toHaveBeenCalled() expect(patchAppConfigurationFile).not.toHaveBeenCalled() }) test('updates the config file if current config client matches remote', async () => { // Given - const localApp = testAppLinked({configuration: {...DEFAULT_CONFIG, client_id: 'api-key'}}) + const localApp = testApp({configuration: {...DEFAULT_CONFIG, client_id: 'api-key'}}, 'current') const options = { currentURLs, appDirectory: '/path', @@ -294,6 +343,7 @@ describe('shouldOrPromptUpdateURLs', () => { // Then expect(result).toBe(true) + expect(setCachedAppInfo).not.toHaveBeenCalled() expect(patchAppConfigurationFile).toHaveBeenCalledWith( expect.objectContaining({ path: localApp.configuration.path, @@ -389,6 +439,7 @@ describe('generateFrontendURL', () => { // Then expect(got).toEqual({frontendUrl: 'https://4040-gitpod.url.fqdn.com', frontendPort: 4040, usingLocalhost: false}) + expect(setCachedAppInfo).not.toBeCalled() expect(renderSelectPrompt).not.toBeCalled() }) @@ -406,6 +457,7 @@ describe('generateFrontendURL', () => { frontendPort: 4040, usingLocalhost: false, }) + expect(setCachedAppInfo).not.toBeCalled() expect(renderSelectPrompt).not.toBeCalled() }) @@ -425,6 +477,7 @@ describe('generateFrontendURL', () => { frontendPort: 4040, usingLocalhost: false, }) + expect(setCachedAppInfo).not.toBeCalled() expect(renderSelectPrompt).not.toBeCalled() }) @@ -444,6 +497,7 @@ describe('generateFrontendURL', () => { frontendPort: 1234, usingLocalhost: false, }) + expect(setCachedAppInfo).not.toBeCalled() expect(renderSelectPrompt).not.toBeCalled() }) @@ -464,6 +518,7 @@ describe('generateFrontendURL', () => { frontendPort: 4040, usingLocalhost: false, }) + expect(setCachedAppInfo).not.toBeCalled() expect(renderSelectPrompt).not.toBeCalled() }) diff --git a/packages/app/src/cli/services/dev/urls.ts b/packages/app/src/cli/services/dev/urls.ts index bdd571af2c0..42d8564b776 100644 --- a/packages/app/src/cli/services/dev/urls.ts +++ b/packages/app/src/cli/services/dev/urls.ts @@ -1,11 +1,12 @@ import {updateURLsPrompt} from '../../prompts/dev.js' import { AppConfigurationInterface, - AppLinkedInterface, + AppInterface, CurrentAppConfiguration, isCurrentAppSchema, } 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' import {DeveloperPlatformClient} from '../../utilities/developer-platform-client.js' import {patchAppConfigurationFile} from '../app/patch-app-configuration-file.js' @@ -242,7 +243,7 @@ interface ShouldOrPromptUpdateURLsOptions { appDirectory: string cachedUpdateURLs?: boolean newApp?: boolean - localApp: AppLinkedInterface + localApp?: AppInterface apiKey: string } @@ -257,14 +258,18 @@ export async function shouldOrPromptUpdateURLs(options: ShouldOrPromptUpdateURLs options.currentURLs.redirectUrlWhitelist, ) - const localConfiguration = options.localApp.configuration - localConfiguration.build = { - ...localConfiguration.build, - automatically_update_urls_on_dev: shouldUpdateURLs, + if (options.localApp && isCurrentAppSchema(options.localApp.configuration)) { + const localConfiguration = options.localApp.configuration + localConfiguration.build = { + ...localConfiguration.build, + automatically_update_urls_on_dev: shouldUpdateURLs, + } + const patch = {build: {automatically_update_urls_on_dev: shouldUpdateURLs}} + const path = options.localApp.configuration.path + await patchAppConfigurationFile({path, patch, schema: options.localApp.configSchema}) + } else { + setCachedAppInfo({directory: options.appDirectory, updateURLs: shouldUpdateURLs}) } - const patch = {build: {automatically_update_urls_on_dev: shouldUpdateURLs}} - const path = options.localApp.configuration.path - await patchAppConfigurationFile({path, patch, schema: options.localApp.configSchema}) } return shouldUpdateURLs }