diff --git a/packages/app/src/cli/services/app/patch-app-configuration-file.test.ts b/packages/app/src/cli/services/app/patch-app-configuration-file.test.ts index 264c7a9d981..658b1f564f1 100644 --- a/packages/app/src/cli/services/app/patch-app-configuration-file.test.ts +++ b/packages/app/src/cli/services/app/patch-app-configuration-file.test.ts @@ -1,4 +1,9 @@ -import {patchAppConfigurationFile, patchAppHiddenConfigFile} from './patch-app-configuration-file.js' +import { + patchAppHiddenConfigFile, + setAppConfigValue, + unsetAppConfigValue, + setManyAppConfigValues, +} from './patch-app-configuration-file.js' import {getAppVersionedSchema} from '../../models/app/app.js' import {loadLocalExtensionsSpecifications} from '../../models/extensions/load-specifications.js' import {readFile, writeFileSync, inTemporaryDirectory} from '@shopify/cli-kit/node/fs' @@ -33,150 +38,6 @@ function writeDefaulToml(tmpDir: string) { return configPath } -describe('patchAppConfigurationFile', () => { - test('updates existing configuration with new values and adds new top-levelfields, replaces arrays', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const configPath = writeDefaulToml(tmpDir) - const patch = { - name: 'Updated App Name', - application_url: 'https://example.com', - access_scopes: { - use_legacy_install_flow: false, - }, - auth: { - redirect_urls: ['https://example.com/redirect3', 'https://example.com/redirect4'], - }, - } - - await patchAppConfigurationFile({path: configPath, patch, schema}) - - const updatedTomlFile = await readFile(configPath) - expect(updatedTomlFile) - .toEqual(`# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration - -client_id = "12345" -name = "Updated App Name" -application_url = "https://example.com" -embedded = true - -[access_scopes] -# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes -use_legacy_install_flow = false - -[auth] -redirect_urls = [ - "https://example.com/redirect3", - "https://example.com/redirect4" -] - -[webhooks] -api_version = "2023-04" -`) - }) - }) - - test('Adds new table to the toml file', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const configPath = writeDefaulToml(tmpDir) - const patch = { - application_url: 'https://example.com', - build: { - dev_store_url: 'example.myshopify.com', - }, - } - - await patchAppConfigurationFile({path: configPath, patch, schema}) - - const updatedTomlFile = await readFile(configPath) - expect(updatedTomlFile) - .toEqual(`# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration - -client_id = "12345" -name = "app1" -application_url = "https://example.com" -embedded = true - -[access_scopes] -# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes -use_legacy_install_flow = true - -[auth] -redirect_urls = [ - "https://example.com/redirect", - "https://example.com/redirect2" -] - -[webhooks] -api_version = "2023-04" - -[build] -dev_store_url = "example.myshopify.com" -`) - }) - }) - - test('Adds a new field to a toml table, merging with exsisting values', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const configPath = writeDefaulToml(tmpDir) - const patch = { - application_url: 'https://example.com', - access_scopes: { - scopes: 'read_products', - }, - } - - await patchAppConfigurationFile({path: configPath, patch, schema}) - - const updatedTomlFile = await readFile(configPath) - expect(updatedTomlFile) - .toEqual(`# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration - -client_id = "12345" -name = "app1" -application_url = "https://example.com" -embedded = true - -[access_scopes] -# Learn more at https://shopify.dev/docs/apps/tools/cli/configuration#access_scopes -use_legacy_install_flow = true -scopes = "read_products" - -[auth] -redirect_urls = [ - "https://example.com/redirect", - "https://example.com/redirect2" -] - -[webhooks] -api_version = "2023-04" -`) - }) - }) - - test('does not validate the toml if no schema is provided', async () => { - await inTemporaryDirectory(async (tmpDir) => { - const configPath = joinPath(tmpDir, 'shopify.app.toml') - writeFileSync( - configPath, - ` -random_toml_field = "random_value" -`, - ) - const patch = {name: 123} - - await patchAppConfigurationFile({path: configPath, patch, schema: undefined}) - - const updatedTomlFile = await readFile(configPath) - expect(updatedTomlFile) - .toEqual(`# Learn more about configuring your app at https://shopify.dev/docs/apps/tools/cli/configuration - -random_toml_field = "random_value" -name = 123 -`) - }) - }) -}) - describe('patchAppHiddenConfigFile', () => { test('creates a new hidden config file when it does not exist', async () => { await inTemporaryDirectory(async (tmpDir) => { @@ -257,3 +118,176 @@ describe('patchAppHiddenConfigFile', () => { }) }) }) + +describe('setAppConfigValue', () => { + test('sets a top-level value in the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await setAppConfigValue(configPath, 'name', 'Updated App Name', schema) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('name = "Updated App Name"') + }) + }) + + test('sets a nested value in the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await setAppConfigValue(configPath, 'build.dev_store_url', 'example.myshopify.com', schema) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('[build]') + expect(updatedTomlFile).toContain('dev_store_url = "example.myshopify.com"') + }) + }) + + test('sets a deeply nested value in the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await setAppConfigValue(configPath, 'build.auth.settings', true, schema) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('[build.auth]') + expect(updatedTomlFile).toContain('settings = true') + }) + }) +}) + +describe('unsetAppConfigValue', () => { + test('unsets a top-level value in the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await unsetAppConfigValue(configPath, 'name', schema) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).not.toContain('name = "app1"') + }) + }) + + test('unsets a nested value in existing table in the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + // Add a value first + await setAppConfigValue(configPath, 'build.dev_store_url', 'example.myshopify.com', schema) + + // Now unset it + await unsetAppConfigValue(configPath, 'build.dev_store_url', schema) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('[build]') + expect(updatedTomlFile).not.toContain('dev_store_url = "example.myshopify.com"') + }) + }) +}) + +describe('setManyAppConfigValues', () => { + test('sets multiple top-level values in the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await setManyAppConfigValues( + configPath, + [ + {keyPath: 'name', value: 'Updated App Name'}, + {keyPath: 'client_id', value: '67890'}, + ], + schema, + ) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('name = "Updated App Name"') + expect(updatedTomlFile).toContain('client_id = "67890"') + }) + }) + + test('sets a mix of top-level and nested values in the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await setManyAppConfigValues( + configPath, + [ + {keyPath: 'name', value: 'Updated App Name'}, + {keyPath: 'build.dev_store_url', value: 'example.myshopify.com'}, + ], + schema, + ) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('name = "Updated App Name"') + expect(updatedTomlFile).toContain('[build]') + expect(updatedTomlFile).toContain('dev_store_url = "example.myshopify.com"') + }) + }) + + test('properly handles array values in the configuration', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await setManyAppConfigValues( + configPath, + [{keyPath: 'auth.redirect_urls', value: ['https://example.com/redirect3', 'https://example.com/redirect4']}], + schema, + ) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('[auth]') + expect(updatedTomlFile).toContain('redirect_urls = [') + expect(updatedTomlFile).toContain('"https://example.com/redirect3"') + expect(updatedTomlFile).toContain('"https://example.com/redirect4"') + expect(updatedTomlFile).not.toContain('"https://example.com/redirect"') + expect(updatedTomlFile).not.toContain('"https://example.com/redirect2"') + }) + }) + + test('combines multiple nested keys into a single object structure', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await setManyAppConfigValues( + configPath, + [ + {keyPath: 'build.dev_store_url', value: 'example.myshopify.com'}, + {keyPath: 'build.automatically_update_urls_on_dev', value: true}, + ], + schema, + ) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('[build]') + expect(updatedTomlFile).toContain('dev_store_url = "example.myshopify.com"') + expect(updatedTomlFile).toContain('automatically_update_urls_on_dev = true') + }) + }) + + test('updates existing configuration with new values and replaces arrays', async () => { + await inTemporaryDirectory(async (tmpDir) => { + const configPath = writeDefaulToml(tmpDir) + + await setManyAppConfigValues( + configPath, + [ + {keyPath: 'name', value: 'Updated App Name'}, + {keyPath: 'application_url', value: 'https://example.com'}, + {keyPath: 'access_scopes.use_legacy_install_flow', value: false}, + {keyPath: 'auth.redirect_urls', value: ['https://example.com/redirect3', 'https://example.com/redirect4']}, + ], + schema, + ) + + const updatedTomlFile = await readFile(configPath) + expect(updatedTomlFile).toContain('name = "Updated App Name"') + expect(updatedTomlFile).toContain('application_url = "https://example.com"') + expect(updatedTomlFile).toContain('use_legacy_install_flow = false') + expect(updatedTomlFile).toContain('redirect_urls = [') + expect(updatedTomlFile).toContain('"https://example.com/redirect3"') + expect(updatedTomlFile).toContain('"https://example.com/redirect4"') + expect(updatedTomlFile).not.toContain('"https://example.com/redirect"') + }) + }) +}) diff --git a/packages/app/src/cli/services/app/patch-app-configuration-file.ts b/packages/app/src/cli/services/app/patch-app-configuration-file.ts index 46cddbf22dc..8a44b8a42bf 100644 --- a/packages/app/src/cli/services/app/patch-app-configuration-file.ts +++ b/packages/app/src/cli/services/app/patch-app-configuration-file.ts @@ -22,8 +22,9 @@ export interface PatchTomlOptions { * @param path - The path to the app configuration file. * @param patch - The patch to apply to the app configuration file. * @param schema - The schema to validate the patch against. If not provided, the toml will not be validated. + * @internal Internal function, use setAppConfigValue, unsetAppConfigValue, or setManyAppConfigValues instead */ -export async function patchAppConfigurationFile({path, patch, schema}: PatchTomlOptions) { +async function patchAppConfigurationFile({path, patch, schema}: PatchTomlOptions) { const tomlContents = await readFile(path) const configuration = decodeToml(tomlContents) @@ -41,6 +42,89 @@ export async function patchAppConfigurationFile({path, patch, schema}: PatchToml await writeFile(path, encodedString) } +/** + * Sets a single value in the app configuration file based on a dotted key path. + * + * @param path - The path to the app configuration file. + * @param keyPath - The dotted key path to set the value at (e.g. 'build.dev_store_url') + * @param value - The value to set + * @param schema - The schema to validate the patch against. If not provided, the toml will not be validated. + */ +export async function setAppConfigValue(path: string, keyPath: string, value: unknown, schema?: zod.AnyZodObject) { + const patch = createPatchFromDottedPath(keyPath, value) + await patchAppConfigurationFile({path, patch, schema}) +} + +/** + * Sets multiple values in the app configuration file. + * + * @param path - The path to the app configuration file + * @param configValues - Array of keyPath and value pairs to set + * @param schema - The schema to validate the patch against. If not provided, the toml will not be validated. + * + * @example + * ```ts + * await setManyAppConfigValues('shopify.app.toml', [ + * { keyPath: 'application_url', value: 'https://example.com' }, + * { keyPath: 'auth.redirect_urls', value: ['https://example.com/callback'] } + * ], schema) + * ``` + */ +export async function setManyAppConfigValues( + path: string, + configValues: {keyPath: string; value: unknown}[], + schema?: zod.AnyZodObject, +) { + const patch = configValues.reduce((acc, {keyPath, value}) => { + const valuePatch = createPatchFromDottedPath(keyPath, value) + return deepMergeObjects(acc, valuePatch, replaceArrayStrategy) + }, {}) + + await patchAppConfigurationFile({path, patch, schema}) +} + +/** + * Unsets a value in the app configuration file based on a dotted key path. + * + * @param path - The path to the app configuration file. + * @param keyPath - The dotted key path to unset (e.g. 'build.include_config_on_deploy') + * @param schema - The schema to validate the patch against. If not provided, the toml will not be validated. + */ +export async function unsetAppConfigValue(path: string, keyPath: string, schema?: zod.AnyZodObject) { + const patch = createPatchFromDottedPath(keyPath, undefined) + await patchAppConfigurationFile({path, patch, schema}) +} + +/** + * Creates a patch object from a dotted key path and a value + * For example, 'build.dev_store_url' with value 'example.myshopify.com' + * will create \{ build: \{ dev_store_url: 'example.myshopify.com' \} \} + */ +function createPatchFromDottedPath(keyPath: string, value: unknown): {[key: string]: unknown} { + const keys = keyPath.split('.') + if (keys.length === 1) { + return {[keyPath]: value} + } + + const obj: {[key: string]: unknown} = {} + let currentObj = obj + + for (let i = 0; i < keys.length - 1; i++) { + const key = keys[i] + if (key) { + currentObj[key] = {} + currentObj = currentObj[key] as {[key: string]: unknown} + } + } + + const lastKey = keys[keys.length - 1] + if (lastKey) { + currentObj[lastKey] = value + } + + return obj +} + export function replaceArrayStrategy(_: unknown[], newArray: unknown[]): unknown[] { return newArray } diff --git a/packages/app/src/cli/services/context.test.ts b/packages/app/src/cli/services/context.test.ts index 84fb72cf80b..447dd691424 100644 --- a/packages/app/src/cli/services/context.test.ts +++ b/packages/app/src/cli/services/context.test.ts @@ -7,7 +7,7 @@ import {createExtension} from './dev/create-extension.js' import {CachedAppInfo} from './local-storage.js' import link from './app/config/link.js' import {fetchSpecifications} from './generate/fetch-extension-specifications.js' -import * as patchAppConfigurationFile from './app/patch-app-configuration-file.js' +import * as patchAppConfigurationFileModule from './app/patch-app-configuration-file.js' import {DeployOptions} from './deploy.js' import {isServiceAccount, isUserAccount} from './context/partner-account-info.js' import { @@ -135,6 +135,13 @@ vi.mock('./context/partner-account-info.js') vi.mock('./generate/fetch-extension-specifications.js') vi.mock('./app/select-app.js') vi.mock('../utilities/developer-platform-client.js') +vi.mock('./app/patch-app-configuration-file.js', () => { + return { + patchAppConfigurationFile: vi.fn(), + setAppConfigValue: vi.fn(), + unsetAppConfigValue: vi.fn(), + } +}) beforeAll(async () => { const localSpecs = await loadSpecifications.loadLocalExtensionsSpecifications() @@ -190,9 +197,7 @@ describe('ensureDeployContext', () => { vi.mocked(renderConfirmationPrompt).mockResolvedValue(true) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const setAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'setAppConfigValue').mockResolvedValue() const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {}) // When @@ -203,14 +208,11 @@ describe('ensureDeployContext', () => { expect(metadataSpyOn.mock.calls[0]![0]()).toEqual({cmd_deploy_confirm_include_config_used: true}) expect(renderConfirmationPrompt).toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).toHaveBeenCalledWith( - expect.objectContaining({ - path: app.configuration.path, - patch: { - build: {include_config_on_deploy: true}, - }, - schema: expect.any(Object), - }), + expect(setAppConfigValueSpy).toHaveBeenCalledWith( + app.configuration.path, + 'build.include_config_on_deploy', + true, + expect.any(Object), ) expect(renderInfo).toHaveBeenCalledWith({ body: [ @@ -228,7 +230,7 @@ describe('ensureDeployContext', () => { ], headline: 'Using shopify.app.toml for default values:', }) - patchAppConfigurationFileSpy.mockRestore() + setAppConfigValueSpy.mockRestore() }) test('prompts the user to include the configuration and set it to false when not confirmed if the flag is not present', async () => { @@ -243,23 +245,18 @@ describe('ensureDeployContext', () => { vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) vi.mocked(renderConfirmationPrompt).mockResolvedValue(false) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const setAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'setAppConfigValue').mockResolvedValue() // When await ensureDeployContext(deployOptions(app)) // Then expect(renderConfirmationPrompt).toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).toHaveBeenCalledWith( - expect.objectContaining({ - path: app.configuration.path, - patch: { - build: {include_config_on_deploy: false}, - }, - schema: expect.any(Object), - }), + expect(setAppConfigValueSpy).toHaveBeenCalledWith( + app.configuration.path, + 'build.include_config_on_deploy', + false, + expect.any(Object), ) expect(renderInfo).toHaveBeenCalledWith({ body: [ @@ -277,7 +274,7 @@ describe('ensureDeployContext', () => { ], headline: 'Using shopify.app.toml for default values:', }) - patchAppConfigurationFileSpy.mockRestore() + setAppConfigValueSpy.mockRestore() }) test('prompts the user to include the configuration if the flag is false', async () => { @@ -295,9 +292,7 @@ describe('ensureDeployContext', () => { vi.mocked(link).mockResolvedValue((app as any).configuration) // vi.mocked(selectDeveloperPlatformClient).mockReturnValue(testDeveloperPlatformClient) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const setAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'setAppConfigValue').mockResolvedValue() const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {}) const options = deployOptions(app) @@ -310,7 +305,7 @@ describe('ensureDeployContext', () => { expect(metadataSpyOn).toHaveBeenCalled() expect(renderConfirmationPrompt).toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).toHaveBeenCalled() + expect(setAppConfigValueSpy).toHaveBeenCalled() expect(renderInfo).toHaveBeenCalledWith({ body: [ { @@ -327,7 +322,7 @@ describe('ensureDeployContext', () => { ], headline: 'Using shopify.app.toml for default values:', }) - patchAppConfigurationFileSpy.mockRestore() + setAppConfigValueSpy.mockRestore() }) test('doesnt prompt the user to include the configuration and display the current value if the flag is true', async () => { @@ -345,9 +340,7 @@ describe('ensureDeployContext', () => { vi.mocked(link).mockResolvedValue((app as any).configuration) // vi.mocked(selectDeveloperPlatformClient).mockReturnValue(testDeveloperPlatformClient) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const setAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'setAppConfigValue').mockResolvedValue() const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {}) const options = deployOptions(app) @@ -360,7 +353,7 @@ describe('ensureDeployContext', () => { expect(metadataSpyOn).not.toHaveBeenCalled() expect(renderConfirmationPrompt).not.toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).not.toHaveBeenCalled() + expect(setAppConfigValueSpy).not.toHaveBeenCalled() expect(renderInfo).toHaveBeenCalledWith({ body: [ { @@ -377,7 +370,7 @@ describe('ensureDeployContext', () => { ], headline: 'Using shopify.app.toml for default values:', }) - patchAppConfigurationFileSpy.mockRestore() + setAppConfigValueSpy.mockRestore() }) test('prompts the user to include the configuration when reset is used and the flag is present', async () => { @@ -392,9 +385,7 @@ describe('ensureDeployContext', () => { vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) vi.mocked(renderConfirmationPrompt).mockResolvedValue(false) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const setAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'setAppConfigValue').mockResolvedValue() const metadataSpyOn = vi.spyOn(metadata, 'addPublicMetadata').mockImplementation(async () => {}) const options = deployOptions(app, true) @@ -407,14 +398,11 @@ describe('ensureDeployContext', () => { expect(metadataSpyOn.mock.calls[0]![0]()).toEqual({cmd_deploy_confirm_include_config_used: false}) expect(renderConfirmationPrompt).toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).toHaveBeenCalledWith( - expect.objectContaining({ - path: app.configuration.path, - patch: { - build: {include_config_on_deploy: false}, - }, - schema: expect.any(Object), - }), + expect(setAppConfigValueSpy).toHaveBeenCalledWith( + app.configuration.path, + 'build.include_config_on_deploy', + false, + expect.any(Object), ) expect(renderInfo).toHaveBeenCalledWith({ @@ -433,7 +421,7 @@ describe('ensureDeployContext', () => { ], headline: 'Using shopify.app.toml for default values:', }) - patchAppConfigurationFileSpy.mockRestore() + setAppConfigValueSpy.mockRestore() }) test('doesnt prompt the user to include the configuration when force is used if the flag is not present', async () => { @@ -448,9 +436,7 @@ describe('ensureDeployContext', () => { vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) vi.mocked(renderConfirmationPrompt).mockResolvedValue(false) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const setAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'setAppConfigValue').mockResolvedValue() const options = deployOptions(app, false, true) vi.mocked(selectDeveloperPlatformClient).mockReturnValue(options.developerPlatformClient) @@ -460,7 +446,7 @@ describe('ensureDeployContext', () => { // Then expect(renderConfirmationPrompt).not.toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).not.toHaveBeenCalled() + expect(setAppConfigValueSpy).not.toHaveBeenCalled() expect(renderInfo).toHaveBeenCalledWith({ body: [ { @@ -477,7 +463,7 @@ describe('ensureDeployContext', () => { ], headline: 'Using shopify.app.toml for default values:', }) - patchAppConfigurationFileSpy.mockRestore() + setAppConfigValueSpy.mockRestore() }) test('prompts the user to include the configuration when force is used and the flag is present', async () => { @@ -492,16 +478,14 @@ describe('ensureDeployContext', () => { vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) vi.mocked(renderConfirmationPrompt).mockResolvedValue(false) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const setAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'setAppConfigValue').mockResolvedValue() // When await ensureDeployContext(deployOptions(app, false, true)) // Then expect(renderConfirmationPrompt).not.toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).not.toHaveBeenCalled() + expect(setAppConfigValueSpy).not.toHaveBeenCalled() expect(renderInfo).toHaveBeenCalledWith({ body: [ { @@ -518,7 +502,7 @@ describe('ensureDeployContext', () => { ], headline: 'Using shopify.app.toml for default values:', }) - patchAppConfigurationFileSpy.mockRestore() + setAppConfigValueSpy.mockRestore() }) test('removes the include_config_on_deploy field when using app management API and the value is true', async () => { @@ -532,9 +516,7 @@ describe('ensureDeployContext', () => { } vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const unsetAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'unsetAppConfigValue').mockResolvedValue() // When const options = { @@ -550,14 +532,10 @@ describe('ensureDeployContext', () => { // Then expect(renderConfirmationPrompt).not.toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).toHaveBeenCalledWith( - expect.objectContaining({ - path: app.configuration.path, - patch: { - build: {include_config_on_deploy: undefined}, - }, - schema: expect.any(Object), - }), + expect(unsetAppConfigValueSpy).toHaveBeenCalledWith( + app.configuration.path, + 'build.include_config_on_deploy', + expect.any(Object), ) expect(renderInfo).toHaveBeenCalledWith({ body: [ @@ -569,7 +547,7 @@ describe('ensureDeployContext', () => { url: 'https://shopify.dev/docs/apps/build/cli-for-apps/app-configuration#build', }, }) - patchAppConfigurationFileSpy.mockRestore() + unsetAppConfigValueSpy.mockRestore() }) test('removes the include_config_on_deploy field when using app management API and the value is false', async () => { @@ -583,9 +561,7 @@ describe('ensureDeployContext', () => { } vi.mocked(ensureDeploymentIdsPresence).mockResolvedValue(identifiers) vi.mocked(getAppConfigurationFileName).mockReturnValue('shopify.app.toml') - const patchAppConfigurationFileSpy = vi - .spyOn(patchAppConfigurationFile, 'patchAppConfigurationFile') - .mockResolvedValue() + const unsetAppConfigValueSpy = vi.spyOn(patchAppConfigurationFileModule, 'unsetAppConfigValue').mockResolvedValue() // When const options = { @@ -601,14 +577,10 @@ describe('ensureDeployContext', () => { // Then expect(renderConfirmationPrompt).not.toHaveBeenCalled() - expect(patchAppConfigurationFileSpy).toHaveBeenCalledWith( - expect.objectContaining({ - path: app.configuration.path, - patch: { - build: {include_config_on_deploy: undefined}, - }, - schema: expect.any(Object), - }), + expect(unsetAppConfigValueSpy).toHaveBeenCalledWith( + app.configuration.path, + 'build.include_config_on_deploy', + expect.any(Object), ) expect(renderWarning).toHaveBeenCalledWith({ body: [ @@ -620,7 +592,7 @@ describe('ensureDeployContext', () => { url: 'https://shopify.dev/docs/apps/build/cli-for-apps/app-configuration#build', }, }) - patchAppConfigurationFileSpy.mockRestore() + unsetAppConfigValueSpy.mockRestore() }) }) diff --git a/packages/app/src/cli/services/context.ts b/packages/app/src/cli/services/context.ts index a42770117fc..3b11d05fd81 100644 --- a/packages/app/src/cli/services/context.ts +++ b/packages/app/src/cli/services/context.ts @@ -3,7 +3,7 @@ import {fetchOrganizations} from './dev/fetch.js' import {ensureDeploymentIdsPresence} from './context/identifiers.js' import {createExtension} from './dev/create-extension.js' import {CachedAppInfo} from './local-storage.js' -import {patchAppConfigurationFile} from './app/patch-app-configuration-file.js' +import {setAppConfigValue, unsetAppConfigValue} from './app/patch-app-configuration-file.js' import {DeployOptions} from './deploy.js' import {isServiceAccount, isUserAccount} from './context/partner-account-info.js' import {selectOrganizationPrompt} from '../prompts/dev.js' @@ -212,8 +212,7 @@ async function removeIncludeConfigOnDeployField(localApp: AppInterface) { const includeConfigOnDeploy = configuration.build?.include_config_on_deploy if (includeConfigOnDeploy === undefined) return - const patch = {build: {include_config_on_deploy: undefined}} - await patchAppConfigurationFile({path: localApp.configuration.path, patch, schema: localApp.configSchema}) + await unsetAppConfigValue(localApp.configuration.path, 'build.include_config_on_deploy', localApp.configSchema) includeConfigOnDeploy ? renderInfoAboutIncludeConfigOnDeploy() : renderWarningAboutIncludeConfigOnDeploy() } @@ -251,8 +250,12 @@ async function promptIncludeConfigOnDeploy(options: ShouldOrPromptIncludeConfigD ...localConfiguration.build, include_config_on_deploy: shouldIncludeConfigDeploy, } - const patch = {build: {include_config_on_deploy: shouldIncludeConfigDeploy}} - await patchAppConfigurationFile({path: localConfiguration.path, patch, schema: options.localApp.configSchema}) + await setAppConfigValue( + localConfiguration.path, + 'build.include_config_on_deploy', + shouldIncludeConfigDeploy, + options.localApp.configSchema, + ) await metadata.addPublicMetadata(() => ({cmd_deploy_confirm_include_config_used: shouldIncludeConfigDeploy})) } diff --git a/packages/app/src/cli/services/dev.ts b/packages/app/src/cli/services/dev.ts index 0da66072b93..4569fae6f24 100644 --- a/packages/app/src/cli/services/dev.ts +++ b/packages/app/src/cli/services/dev.ts @@ -23,7 +23,7 @@ import {DevProcessFunction} from './dev/processes/types.js' import {getCachedAppInfo, setCachedAppInfo} from './local-storage.js' import {canEnablePreviewMode} from './extensions/common.js' import {fetchAppRemoteConfiguration} from './app/select-app.js' -import {patchAppConfigurationFile} from './app/patch-app-configuration-file.js' +import {setManyAppConfigValues} from './app/patch-app-configuration-file.js' import {DevSessionStatusManager} from './dev/processes/dev-session/dev-session-status-manager.js' import {DeveloperPlatformClient} from '../utilities/developer-platform-client.js' import {Web, isCurrentAppSchema, getAppScopesArray, AppLinkedInterface} from '../models/app/app.js' @@ -124,8 +124,11 @@ async function prepareForDev(commandOptions: DevOptions): Promise { ...app.configuration.build, dev_store_url: store.shopDomain, } - const patch = {build: {dev_store_url: store.shopDomain}} - await patchAppConfigurationFile({path: app.configuration.path, patch, schema: app.configSchema}) + await setManyAppConfigValues( + app.configuration.path, + [{keyPath: 'build.dev_store_url', value: store.shopDomain}], + app.configSchema, + ) } if (!commandOptions.skipDependenciesInstallation && !app.usesWorkspaces) { @@ -246,7 +249,7 @@ export async function warnIfScopesDifferBeforeDev({ } async function actionsBeforeLaunchingDevProcesses(config: DevConfig) { - setPreviousAppId(config.commandOptions.directory, config.remoteApp.apiKey) + setCachedAppInfo({directory: config.commandOptions.directory, previousAppId: config.remoteApp.apiKey}) await logMetadataForDev({ devOptions: config.commandOptions, @@ -520,7 +523,3 @@ async function validateCustomPorts(webConfigs: Web[], graphiqlPort: number) { })(), ]) } - -function setPreviousAppId(directory: string, apiKey: string) { - setCachedAppInfo({directory, previousAppId: apiKey}) -} diff --git a/packages/app/src/cli/services/dev/urls.test.ts b/packages/app/src/cli/services/dev/urls.test.ts index fb7505ffd23..e9b65bcef15 100644 --- a/packages/app/src/cli/services/dev/urls.test.ts +++ b/packages/app/src/cli/services/dev/urls.test.ts @@ -15,7 +15,7 @@ import { } 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 {setManyAppConfigValues} 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' @@ -26,7 +26,11 @@ import {renderConfirmationPrompt, renderSelectPrompt} from '@shopify/cli-kit/nod import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' vi.mock('../local-storage.js') -vi.mock('../app/patch-app-configuration-file.js') +vi.mock('../app/patch-app-configuration-file.js', () => { + return { + setManyAppConfigValues: vi.fn(), + } +}) vi.mock('@shopify/cli-kit/node/tcp') vi.mock('@shopify/cli-kit/node/context/spin') vi.mock('@shopify/cli-kit/node/context/local') @@ -92,21 +96,20 @@ describe('updateURLs', () => { await updateURLs(urls, apiKey, testDeveloperPlatformClient(), appWithConfig) // Then - expect(patchAppConfigurationFile).toHaveBeenCalledWith( - expect.objectContaining({ - path: appWithConfig.configuration.path, - patch: { - application_url: 'https://example.com', - auth: { - redirect_urls: [ - 'https://example.com/auth/callback', - 'https://example.com/auth/shopify/callback', - 'https://example.com/api/auth/callback', - ], - }, + expect(setManyAppConfigValues).toHaveBeenCalledWith( + appWithConfig.configuration.path, + [ + {keyPath: 'application_url', value: 'https://example.com'}, + { + keyPath: 'auth.redirect_urls', + value: [ + 'https://example.com/auth/callback', + 'https://example.com/auth/shopify/callback', + 'https://example.com/api/auth/callback', + ], }, - schema: expect.any(Object), - }), + ], + expect.any(Object), ) }) @@ -180,26 +183,23 @@ describe('updateURLs', () => { await updateURLs(urls, apiKey, testDeveloperPlatformClient(), appWithConfig) // Then - expect(patchAppConfigurationFile).toHaveBeenCalledWith( - expect.objectContaining({ - path: appWithConfig.configuration.path, - patch: { - application_url: 'https://example.com', - auth: { - redirect_urls: [ - 'https://example.com/auth/callback', - 'https://example.com/auth/shopify/callback', - 'https://example.com/api/auth/callback', - ], - }, - app_proxy: { - url: 'https://example.com', - subpath: 'subpath', - prefix: 'prefix', - }, + expect(setManyAppConfigValues).toHaveBeenCalledWith( + appWithConfig.configuration.path, + [ + {keyPath: 'application_url', value: 'https://example.com'}, + { + keyPath: 'auth.redirect_urls', + value: [ + 'https://example.com/auth/callback', + 'https://example.com/auth/shopify/callback', + 'https://example.com/api/auth/callback', + ], }, - schema: expect.any(Object), - }), + {keyPath: 'app_proxy.url', value: 'https://example.com'}, + {keyPath: 'app_proxy.subpath', value: 'subpath'}, + {keyPath: 'app_proxy.prefix', value: 'prefix'}, + ], + expect.any(Object), ) }) }) @@ -360,7 +360,7 @@ describe('shouldOrPromptUpdateURLs', () => { // Then expect(result).toBe(true) expect(setCachedAppInfo).not.toHaveBeenCalled() - expect(patchAppConfigurationFile).not.toHaveBeenCalled() + expect(setManyAppConfigValues).not.toHaveBeenCalled() }) test('updates the config file if current config client matches remote', async () => { @@ -385,14 +385,10 @@ describe('shouldOrPromptUpdateURLs', () => { // Then expect(result).toBe(true) expect(setCachedAppInfo).not.toHaveBeenCalled() - expect(patchAppConfigurationFile).toHaveBeenCalledWith( - expect.objectContaining({ - path: localApp.configuration.path, - patch: { - build: {automatically_update_urls_on_dev: true}, - }, - schema: expect.any(Object), - }), + expect(setManyAppConfigValues).toHaveBeenCalledWith( + localApp.configuration.path, + [{keyPath: 'build.automatically_update_urls_on_dev', value: true}], + localApp.configSchema, ) }) }) diff --git a/packages/app/src/cli/services/dev/urls.ts b/packages/app/src/cli/services/dev/urls.ts index 8567869ed2b..53c313f117f 100644 --- a/packages/app/src/cli/services/dev/urls.ts +++ b/packages/app/src/cli/services/dev/urls.ts @@ -4,7 +4,7 @@ import {UpdateURLsSchema, UpdateURLsVariables} from '../../api/graphql/update_ur 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' +import {setManyAppConfigValues} from '../app/patch-app-configuration-file.js' import {AbortError, BugError} from '@shopify/cli-kit/node/error' import {Config} from '@oclif/core' import {checkPortAvailability, getAvailableTCPPort} from '@shopify/cli-kit/node/tcp' @@ -197,23 +197,20 @@ export async function updateURLs( } if (localApp && localApp.configuration.client_id === apiKey) { - const patch = { - application_url: urls.applicationUrl, - auth: { - redirect_urls: urls.redirectUrlWhitelist, - }, - ...(urls.appProxy - ? { - app_proxy: { - url: urls.appProxy.proxyUrl, - subpath: urls.appProxy.proxySubPath, - prefix: urls.appProxy.proxySubPathPrefix, - }, - } - : {}), + const configValues = [ + {keyPath: 'application_url', value: urls.applicationUrl}, + {keyPath: 'auth.redirect_urls', value: urls.redirectUrlWhitelist}, + ] + + if (urls.appProxy) { + configValues.push( + {keyPath: 'app_proxy.url', value: urls.appProxy.proxyUrl}, + {keyPath: 'app_proxy.subpath', value: urls.appProxy.proxySubPath}, + {keyPath: 'app_proxy.prefix', value: urls.appProxy.proxySubPathPrefix}, + ) } - await patchAppConfigurationFile({path: localApp.configuration.path, patch, schema: localApp.configSchema}) + await setManyAppConfigValues(localApp.configuration.path, configValues, localApp.configSchema) } } @@ -262,9 +259,12 @@ export async function shouldOrPromptUpdateURLs(options: ShouldOrPromptUpdateURLs ...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}) + await setManyAppConfigValues( + path, + [{keyPath: 'build.automatically_update_urls_on_dev', value: shouldUpdateURLs}], + options.localApp.configSchema, + ) } else { setCachedAppInfo({directory: options.appDirectory, updateURLs: shouldUpdateURLs}) }