From 9ba5ada22c89075527ed52a2997d5db9abdafb38 Mon Sep 17 00:00:00 2001 From: Will Conrad Date: Mon, 21 Oct 2024 20:45:46 -0500 Subject: [PATCH] fix: another pass of refactoring env and blob commands Co-authored-by: Thomas Lane <163203257+tlane25@users.noreply.github.com> Co-authored-by: Thomas Lane --- src/commands/blobs/blobs-delete.ts | 9 ++++-- src/commands/blobs/blobs-set.ts | 3 +- src/commands/env/env-clone.ts | 3 +- src/commands/env/env-set.ts | 3 +- src/commands/env/env-unset.ts | 5 ++-- src/utils/prompts/blob-delete-prompts.ts | 24 +++++++-------- src/utils/prompts/confirm-prompt.ts | 6 ++-- src/utils/prompts/env-clone-prompt.ts | 4 ++- ...et-set-prompts.ts => env-unset-prompts.ts} | 0 src/utils/prompts/prompt-messages.ts | 9 +++++- .../commands/blobs/blobs-delete.test.ts | 30 +++++++++---------- .../commands/blobs/blobs-set.test.ts | 8 ++--- .../commands/env/env-clone.test.ts | 6 ++-- .../integration/commands/env/env-set.test.ts | 28 +++++++++-------- .../commands/env/env-unset.test.ts | 6 ++-- tests/integration/utils/mock-api.js | 4 +++ 16 files changed, 80 insertions(+), 68 deletions(-) rename src/utils/prompts/{unset-set-prompts.ts => env-unset-prompts.ts} (100%) diff --git a/src/commands/blobs/blobs-delete.ts b/src/commands/blobs/blobs-delete.ts index c430a947a26..c15d1b13965 100644 --- a/src/commands/blobs/blobs-delete.ts +++ b/src/commands/blobs/blobs-delete.ts @@ -1,12 +1,17 @@ import { getStore } from '@netlify/blobs' import { chalk, error as printError, log } from '../../utils/command-helpers.js' -import { blobDeletePrompts } from '../../utils/prompts/blob-delete-prompts.js' +import { promptBlobDelete } from '../../utils/prompts/blob-delete-prompts.js' /** * The blobs:delete command */ export const blobsDelete = async (storeName: string, key: string, _options: Record, command: any) => { + // Prevents prompts from blocking scripted commands + if (command.scriptedCommand) { + _options.force = true + } + const { api, siteInfo } = command.netlify const { force } = _options @@ -18,7 +23,7 @@ export const blobsDelete = async (storeName: string, key: string, _options: Reco }) if (force === undefined) { - await blobDeletePrompts(key, storeName) + await promptBlobDelete(key, storeName) } try { diff --git a/src/commands/blobs/blobs-set.ts b/src/commands/blobs/blobs-set.ts index df096000d76..326b512238f 100644 --- a/src/commands/blobs/blobs-set.ts +++ b/src/commands/blobs/blobs-set.ts @@ -20,9 +20,8 @@ export const blobsSet = async ( options: Options, command: BaseCommand, ) => { - // Prevents prompts from blocking scripted commands - if (command.scriptedCommand){ + if (command.scriptedCommand) { options.force = true } diff --git a/src/commands/env/env-clone.ts b/src/commands/env/env-clone.ts index c8a450e78d9..9a7184c2861 100644 --- a/src/commands/env/env-clone.ts +++ b/src/commands/env/env-clone.ts @@ -56,9 +56,8 @@ const cloneEnvVars = async ({ api, force, siteFrom, siteTo }): Promise } export const envClone = async (options: OptionValues, command: BaseCommand) => { - // Prevents prompts from blocking scripted commands - if (command.scriptedCommand){ + if (command.scriptedCommand) { options.force = true } diff --git a/src/commands/env/env-set.ts b/src/commands/env/env-set.ts index 01a1460179a..1d43d400946 100644 --- a/src/commands/env/env-set.ts +++ b/src/commands/env/env-set.ts @@ -116,9 +116,8 @@ const setInEnvelope = async ({ api, context, force, key, scope, secret, siteInfo } export const envSet = async (key: string, value: string, options: OptionValues, command: BaseCommand) => { - // Prevents prompts from blocking scripted commands - if (command.scriptedCommand){ + if (command.scriptedCommand) { options.force = true } diff --git a/src/commands/env/env-unset.ts b/src/commands/env/env-unset.ts index 30131287d55..3a7c756bdf7 100644 --- a/src/commands/env/env-unset.ts +++ b/src/commands/env/env-unset.ts @@ -2,7 +2,7 @@ import { OptionValues } from 'commander' import { chalk, log, logJson, exit } from '../../utils/command-helpers.js' import { AVAILABLE_CONTEXTS, translateFromEnvelopeToMongo } from '../../utils/env/index.js' -import { promptOverwriteEnvVariable } from '../../utils/prompts/unset-set-prompts.js' +import { promptOverwriteEnvVariable } from '../../utils/prompts/env-unset-prompts.js' import BaseCommand from '../base-command.js' /** * Deletes a given key from the env of a site configured with Envelope @@ -68,9 +68,8 @@ const unsetInEnvelope = async ({ api, context, force, key, siteInfo }) => { } export const envUnset = async (key: string, options: OptionValues, command: BaseCommand) => { - // Prevents prompts from blocking scripted commands - if (command.scriptedCommand){ + if (command.scriptedCommand) { options.force = true } diff --git a/src/utils/prompts/blob-delete-prompts.ts b/src/utils/prompts/blob-delete-prompts.ts index 301bf71cfb9..42e37b8428a 100644 --- a/src/utils/prompts/blob-delete-prompts.ts +++ b/src/utils/prompts/blob-delete-prompts.ts @@ -1,19 +1,17 @@ -import { chalk, log } from '../command-helpers.js' +import { log } from '../command-helpers.js' import { confirmPrompt } from './confirm-prompt.js' +import { destructiveCommandMessages } from './prompt-messages.js' + +export const promptBlobDelete = async (key: string, storeName: string): Promise => { + const { overwriteNoticeMessage } = destructiveCommandMessages + const { generateWarningMessage, overwriteConfirmationMessage } = destructiveCommandMessages.blobDelete + + const warningMessage = generateWarningMessage(key, storeName) -const generateBlobWarningMessage = (key: string, storeName: string): void => { log() - log( - `${chalk.redBright('Warning')}: The following blob key ${chalk.cyan(key)} will be deleted from store ${chalk.cyan( - storeName, - )}:`, - ) + log(warningMessage) log() - log(`${chalk.yellowBright('Notice')}: To overwrite without this warning, you can use the --force flag.`) -} - -export const blobDeletePrompts = async (key: string, storeName: string): Promise => { - generateBlobWarningMessage(key, storeName) - await confirmPrompt('Do you want to proceed with deleting the value at this key?') + log(overwriteNoticeMessage) + await confirmPrompt(overwriteConfirmationMessage) } diff --git a/src/utils/prompts/confirm-prompt.ts b/src/utils/prompts/confirm-prompt.ts index dc3c36426c6..fa6ea6dc89f 100644 --- a/src/utils/prompts/confirm-prompt.ts +++ b/src/utils/prompts/confirm-prompt.ts @@ -4,14 +4,14 @@ import { log, exit } from '../command-helpers.js' export const confirmPrompt = async (message: string): Promise => { try { - const { wantsToSet } = await inquirer.prompt({ + const { confirm } = await inquirer.prompt({ type: 'confirm', - name: 'wantsToSet', + name: 'confirm', message, default: false, }) log() - if (!wantsToSet) { + if (!confirm) { exit() } } catch (error) { diff --git a/src/utils/prompts/env-clone-prompt.ts b/src/utils/prompts/env-clone-prompt.ts index 66cb2c3c804..b72d2c73d26 100644 --- a/src/utils/prompts/env-clone-prompt.ts +++ b/src/utils/prompts/env-clone-prompt.ts @@ -26,7 +26,9 @@ export async function promptEnvCloneOverwrite(siteId: string, existingEnvVars: E log() log(noticeEnvVarsMessage) log() - existingEnvVarKeys.forEach(log) + existingEnvVarKeys.forEach((envVar) => { + log(envVar) + }) log() log(overwriteNoticeMessage) diff --git a/src/utils/prompts/unset-set-prompts.ts b/src/utils/prompts/env-unset-prompts.ts similarity index 100% rename from src/utils/prompts/unset-set-prompts.ts rename to src/utils/prompts/env-unset-prompts.ts diff --git a/src/utils/prompts/prompt-messages.ts b/src/utils/prompts/prompt-messages.ts index 663706a6190..5b6479a70ce 100644 --- a/src/utils/prompts/prompt-messages.ts +++ b/src/utils/prompts/prompt-messages.ts @@ -1,5 +1,4 @@ import { chalk } from '../command-helpers.js' -import { EnvVar } from '../types.js' export const destructiveCommandMessages = { overwriteNoticeMessage: `${chalk.yellowBright( @@ -12,6 +11,14 @@ export const destructiveCommandMessages = { overwriteConfirmationMessage: 'Do you want to proceed with overwriting this blob key existing value?', }, + blobDelete: { + generateWarningMessage: (key: string, storeName: string) => + `${chalk.redBright('Warning')}: The following blob key ${chalk.cyan(key)} will be deleted from store ${chalk.cyan( + storeName, + )}:`, + overwriteConfirmationMessage: 'Do you want to proceed with deleting the value at this key?', + }, + envSet: { generateWarningMessage: (variableName: string) => `${chalk.redBright('Warning')}: The environment variable ${chalk.bgBlueBright(variableName)} already exists.`, diff --git a/tests/integration/commands/blobs/blobs-delete.test.ts b/tests/integration/commands/blobs/blobs-delete.test.ts index dc387c0bd7f..fbe3766ebf2 100644 --- a/tests/integration/commands/blobs/blobs-delete.test.ts +++ b/tests/integration/commands/blobs/blobs-delete.test.ts @@ -8,6 +8,7 @@ import { describe, expect, test, vi, beforeEach } from 'vitest' import BaseCommand from '../../../../src/commands/base-command.js' import { createBlobsCommand } from '../../../../src/commands/blobs/blobs.js' import { log } from '../../../../src/utils/command-helpers.js' +import { destructiveCommandMessages } from '../../../../src/utils/prompts/prompt-messages.js' import { Route } from '../../utils/mock-api-vitest.js' import { getEnvironmentVariables, withMockApi } from '../../utils/mock-api.js' @@ -44,13 +45,10 @@ describe('blob:delete command', () => { const storeName = 'my-store' const key = 'my-key' - const warningMessage = `${chalk.redBright('Warning')}: The following blob key ${chalk.cyan( - key, - )} will be deleted from store ${chalk.cyan(storeName)}:` + const { overwriteNoticeMessage } = destructiveCommandMessages + const { generateWarningMessage, overwriteConfirmationMessage } = destructiveCommandMessages.blobDelete - const noticeMessage = `${chalk.yellowBright( - 'Notice', - )}: To overwrite without this warning, you can use the --force flag.` + const warningMessage = generateWarningMessage(key, storeName) const successMessage = `${chalk.greenBright('Success')}: Blob ${chalk.yellow(key)} deleted from store ${chalk.yellow( storeName, @@ -73,19 +71,19 @@ describe('blob:delete command', () => { const program = new BaseCommand('netlify') createBlobsCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: true }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: true }) await program.parseAsync(['', '', 'blob:delete', storeName, key]) expect(promptSpy).toHaveBeenCalledWith({ type: 'confirm', - name: 'wantsToSet', - message: expect.stringContaining('Do you want to proceed with deleting the value at this key?'), + name: 'confirm', + message: expect.stringContaining(overwriteConfirmationMessage), default: false, }) expect(log).toHaveBeenCalledWith(warningMessage) - expect(log).toHaveBeenCalledWith(noticeMessage) + expect(log).toHaveBeenCalledWith(overwriteNoticeMessage) expect(log).toHaveBeenCalledWith(successMessage) }) }) @@ -103,7 +101,7 @@ describe('blob:delete command', () => { const program = new BaseCommand('netlify') createBlobsCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: false }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: false }) try { await program.parseAsync(['', '', 'blob:delete', storeName, key]) @@ -114,13 +112,13 @@ describe('blob:delete command', () => { expect(promptSpy).toHaveBeenCalledWith({ type: 'confirm', - name: 'wantsToSet', - message: expect.stringContaining('Do you want to proceed with deleting the value at this key?'), + name: 'confirm', + message: expect.stringContaining(overwriteConfirmationMessage), default: false, }) expect(log).toHaveBeenCalledWith(warningMessage) - expect(log).toHaveBeenCalledWith(noticeMessage) + expect(log).toHaveBeenCalledWith(overwriteNoticeMessage) expect(log).not.toHaveBeenCalledWith(successMessage) }) }) @@ -145,7 +143,7 @@ describe('blob:delete command', () => { expect(promptSpy).not.toHaveBeenCalled() expect(log).not.toHaveBeenCalledWith(warningMessage) - expect(log).not.toHaveBeenCalledWith(noticeMessage) + expect(log).not.toHaveBeenCalledWith(overwriteNoticeMessage) expect(log).toHaveBeenCalledWith(successMessage) }) }) @@ -176,7 +174,7 @@ describe('blob:delete command', () => { expect(promptSpy).not.toHaveBeenCalled() expect(log).not.toHaveBeenCalledWith(warningMessage) - expect(log).not.toHaveBeenCalledWith(noticeMessage) + expect(log).not.toHaveBeenCalledWith(overwriteNoticeMessage) expect(log).not.toHaveBeenCalledWith(successMessage) }) }) diff --git a/tests/integration/commands/blobs/blobs-set.test.ts b/tests/integration/commands/blobs/blobs-set.test.ts index 2a74228ef59..17650b122a1 100644 --- a/tests/integration/commands/blobs/blobs-set.test.ts +++ b/tests/integration/commands/blobs/blobs-set.test.ts @@ -105,13 +105,13 @@ describe('blob:set command', () => { const program = new BaseCommand('netlify') createBlobsCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: true }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: true }) await program.parseAsync(['', '', 'blob:set', storeName, key, newValue]) expect(promptSpy).toHaveBeenCalledWith({ type: 'confirm', - name: 'wantsToSet', + name: 'confirm', message: expect.stringContaining(overwriteConfirmationMessage), default: false, }) @@ -139,7 +139,7 @@ describe('blob:set command', () => { const program = new BaseCommand('netlify') createBlobsCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: false }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: false }) try { await program.parseAsync(['', '', 'blob:set', storeName, key, newValue]) @@ -150,7 +150,7 @@ describe('blob:set command', () => { expect(promptSpy).toHaveBeenCalledWith({ type: 'confirm', - name: 'wantsToSet', + name: 'confirm', message: expect.stringContaining(overwriteConfirmationMessage), default: false, }) diff --git a/tests/integration/commands/env/env-clone.test.ts b/tests/integration/commands/env/env-clone.test.ts index 79890018cb6..984a74e2ef2 100644 --- a/tests/integration/commands/env/env-clone.test.ts +++ b/tests/integration/commands/env/env-clone.test.ts @@ -45,13 +45,13 @@ describe('env:clone command', () => { const program = new BaseCommand('netlify') createEnvCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: true }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: true }) await program.parseAsync(['', '', 'env:clone', '-t', siteIdTwo]) expect(promptSpy).toHaveBeenCalledWith({ type: 'confirm', - name: 'wantsToSet', + name: 'confirm', message: expect.stringContaining(overwriteConfirmationMessage), default: false, }) @@ -96,7 +96,7 @@ describe('env:clone command', () => { const program = new BaseCommand('netlify') createEnvCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: false }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: false }) try { await program.parseAsync(['', '', 'env:clone', '-t', siteIdTwo]) diff --git a/tests/integration/commands/env/env-set.test.ts b/tests/integration/commands/env/env-set.test.ts index 2938d250a43..b540b1c1152 100644 --- a/tests/integration/commands/env/env-set.test.ts +++ b/tests/integration/commands/env/env-set.test.ts @@ -7,11 +7,11 @@ import { describe, expect, test, vi, beforeEach } from 'vitest' import BaseCommand from '../../../../src/commands/base-command.js' import { createEnvCommand } from '../../../../src/commands/env/env.js' import { log } from '../../../../src/utils/command-helpers.js' -import { generateSetMessage } from '../../../../src/utils/prompts/env-set-prompts.js' +import { destructiveCommandMessages } from '../../../../src/utils/prompts/prompt-messages.js' import { FixtureTestContext, setupFixtureTests } from '../../utils/fixture.js' import { getEnvironmentVariables, withMockApi } from '../../utils/mock-api.js' -import routes from './api-routes.js' +import { routes } from './api-routes.js' vi.mock('../../../../src/utils/command-helpers.js', async () => ({ ...(await vi.importActual('../../../../src/utils/command-helpers.js')), @@ -269,12 +269,14 @@ describe('env:set command', () => { }) }) - describe('user is prompted to confirm when setting an env var that already exists', () => { + describe.only('user is prompted to confirmOverwrite when setting an env var that already exists', () => { // already exists as value in withMockApi const existingVar = 'EXISTING_VAR' const newEnvValue = 'value' + const { overwriteNoticeMessage } = destructiveCommandMessages + const { generateWarningMessage, overwriteConfirmationMessage } = destructiveCommandMessages.envSet - const { confirmMessage, noticeMessage, warningMessage } = generateSetMessage(existingVar) + const warningMessage = generateWarningMessage(existingVar) const successMessage = `Set environment variable ${chalk.yellow( `${existingVar}=${newEnvValue}`, @@ -287,10 +289,10 @@ describe('env:set command', () => { test('should log warnings and prompts if enviroment variable already exists', async () => { await withMockApi(routes, async ({ apiUrl }) => { Object.assign(process.env, getEnvironmentVariables({ apiUrl })) - + console.log(process.env.SHLVL) const program = new BaseCommand('netlify') - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: true }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: true }) createEnvCommand(program) @@ -298,13 +300,13 @@ describe('env:set command', () => { expect(promptSpy).toHaveBeenCalledWith({ type: 'confirm', - name: 'wantsToSet', - message: expect.stringContaining(confirmMessage), + name: 'confirm', + message: expect.stringContaining(overwriteConfirmationMessage), default: false, }) expect(log).toHaveBeenCalledWith(warningMessage) - expect(log).toHaveBeenCalledWith(noticeMessage) + expect(log).toHaveBeenCalledWith(overwriteNoticeMessage) expect(log).toHaveBeenCalledWith(successMessage) }) }) @@ -323,7 +325,7 @@ describe('env:set command', () => { expect(promptSpy).not.toHaveBeenCalled() expect(log).not.toHaveBeenCalledWith(warningMessage) - expect(log).not.toHaveBeenCalledWith(noticeMessage) + expect(log).not.toHaveBeenCalledWith(overwriteNoticeMessage) expect(log).toHaveBeenCalledWith( `Set environment variable ${chalk.yellow(`${'NEW_ENV_VAR'}=${'NEW_VALUE'}`)} in the ${chalk.magenta( 'all', @@ -346,7 +348,7 @@ describe('env:set command', () => { expect(promptSpy).not.toHaveBeenCalled() expect(log).not.toHaveBeenCalledWith(warningMessage) - expect(log).not.toHaveBeenCalledWith(noticeMessage) + expect(log).not.toHaveBeenCalledWith(overwriteNoticeMessage) expect(log).toHaveBeenCalledWith(successMessage) }) }) @@ -358,7 +360,7 @@ describe('env:set command', () => { const program = new BaseCommand('netlify') createEnvCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: false }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: false }) try { await program.parseAsync(['', '', 'env:set', existingVar, newEnvValue]) @@ -370,7 +372,7 @@ describe('env:set command', () => { expect(promptSpy).toHaveBeenCalled() expect(log).toHaveBeenCalledWith(warningMessage) - expect(log).toHaveBeenCalledWith(noticeMessage) + expect(log).toHaveBeenCalledWith(overwriteNoticeMessage) expect(log).not.toHaveBeenCalledWith(successMessage) }) }) diff --git a/tests/integration/commands/env/env-unset.test.ts b/tests/integration/commands/env/env-unset.test.ts index 35f2837f890..4af9adea62a 100644 --- a/tests/integration/commands/env/env-unset.test.ts +++ b/tests/integration/commands/env/env-unset.test.ts @@ -100,13 +100,13 @@ describe('env:unset command', () => { const program = new BaseCommand('netlify') createEnvCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: true }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: true }) await program.parseAsync(['', '', 'env:unset', existingVar]) expect(promptSpy).toHaveBeenCalledWith({ type: 'confirm', - name: 'wantsToSet', + name: 'confirm', message: expect.stringContaining(overwriteConfirmationMessage), default: false, }) @@ -143,7 +143,7 @@ describe('env:unset command', () => { const program = new BaseCommand('netlify') createEnvCommand(program) - const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ wantsToSet: false }) + const promptSpy = vi.spyOn(inquirer, 'prompt').mockResolvedValue({ confirm: false }) try { await program.parseAsync(['', '', 'env:unset', existingVar]) diff --git a/tests/integration/utils/mock-api.js b/tests/integration/utils/mock-api.js index d330902a80b..a03f84fc7cd 100644 --- a/tests/integration/utils/mock-api.js +++ b/tests/integration/utils/mock-api.js @@ -78,10 +78,14 @@ export const withMockApi = async (routes, testHandler, silent = false) => { } } +// `SHLVL` set to "1" to mock commands run from terminal command line +// `SHLVL` used to overwrite prompts for scripted commands in production/dev +// environments see `scriptedCommand` property of `BaseCommand` export const getEnvironmentVariables = ({ apiUrl }) => ({ NETLIFY_AUTH_TOKEN: 'fake-token', NETLIFY_SITE_ID: 'site_id', NETLIFY_API_URL: apiUrl, + SHLVL: '1', }) /**