Feat(app-config-writer): enhance prompt for convert preview config (#…

…2848)

* refactor user prompts

* add changeset

* undo package adjustment

* fix typo

* fix sonar errors

* fix unit tests

* fix sonar issue

* add unit tests

* minor adjustments

* refactoring

* fix logger

* move prompts from create module to app-config-writer

* adjust prompt texts

* adjust prompt texts II

* Update packages/app-config-writer/src/prompt/preview-config.ts

Co-authored-by: Louise Findlay <louise.findlay@sap.com>

---------

Co-authored-by: Louise Findlay <louise.findlay@sap.com>
Co-authored-by: Klaus Keller <66327622+Klaus-Keller@users.noreply.github.com>

.changeset/spicy-dolls-smoke.md

@@ -0,0 +1,6 @@
+---
+'@sap-ux/app-config-writer': patch
+'@sap-ux/create': patch
+---
+
+feat: enhance prompt for convert preview config

packages/app-config-writer/src/index.ts

@@ -1,4 +1,4 @@
-export { getSmartLinksTargetFromPrompt } from './prompt';
+export { getSmartLinksTargetFromPrompt, simulatePrompt, includeTestRunnersPrompt } from './prompt';
 export { generateSmartLinksConfig } from './smartlinks-config';
 export { generateInboundNavigationConfig, readManifest } from './navigation-config';
 export { generateVariantsConfig } from './variants-config';

packages/app-config-writer/src/preview-config/index.ts

@@ -1,4 +1,4 @@
-import { checkPrerequisites, getExplicitApprovalToAdjustFiles } from './prerequisites';
+import { checkPrerequisites } from './prerequisites';
 import { create as createStorage } from 'mem-fs';
 import { create, type Editor } from 'mem-fs-editor';
 import { deleteNoLongerUsedFiles, renameDefaultSandboxes, renameDefaultTestFiles } from './preview-files';
@@ -32,11 +32,6 @@ export async function convertToVirtualPreview(
         throw Error('The prerequisites are not met. For more information, see the log messages above.');
     }
 
-    if (!(await getExplicitApprovalToAdjustFiles())) {
-        logger?.error('You have not approved the conversion. The conversion has been aborted.');
-        return fs;
-    }
-
     await updatePreviewMiddlewareConfigs(fs, basePath, convertTests, logger);
     await renameDefaultSandboxes(fs, basePath, logger);
     if (convertTests) {

packages/app-config-writer/src/preview-config/prerequisites.ts

@@ -1,8 +1,6 @@
 import { join } from 'path';
-import { prompt } from 'prompts';
 import type { Editor } from 'mem-fs-editor';
 import type { Package } from '@sap-ux/project-access';
-import type { PromptObject } from 'prompts';
 import type { ToolsLogger } from '@sap-ux/logger';
 import { satisfies, valid } from 'semver';
 
@@ -107,19 +105,3 @@ export async function checkPrerequisites(
 
     return prerequisitesMet;
 }
-
-/**
- * Get the explicit approval form the user to do the conversion.
- *
- * @returns Explicit user approval to do the conversion.
- */
-export async function getExplicitApprovalToAdjustFiles(): Promise<boolean> {
-    const question: PromptObject = {
-        type: 'confirm',
-        name: 'approval',
-        initial: false,
-        message:
-            'The converter will rename the HTML files and delete the JS and TS files used for the existing preview functionality and configure virtual endpoints instead. Do you want to proceed with the conversion?'
-    };
-    return Boolean((await prompt([question])).approval);
-}

packages/app-config-writer/src/prompt/index.ts

@@ -1 +1,2 @@
 export { getSmartLinksTargetFromPrompt, promptUserPass } from './smartlinks-config';
+export * from './preview-config';

packages/app-config-writer/src/prompt/preview-config.ts

@@ -0,0 +1,41 @@
+import { prompt, type PromptObject, type Answers } from 'prompts';
+
+/**
+ * Prompt if the conversion should be done in simulation.
+ *
+ * @returns Indicator if the conversion should be simulated.
+ */
+export async function simulatePrompt(): Promise<boolean> {
+    const PROMPT_NAME = 'simulate';
+    const question: PromptObject = {
+        type: 'confirm',
+        name: PROMPT_NAME,
+        initial: true,
+        message: `The converter renames the local HTML files, deletes the JavaScript and TypeScript files used for the existing preview functionality, and configures virtual endpoints instead.
+Do you want to simulate the conversion?`
+    };
+    const answer = (await prompt([question])) as Answers<typeof PROMPT_NAME>;
+    return (
+        answer.simulate ?? (await Promise.reject(new Error('An error has occurred. The conversion has been canceled.')))
+    ); //in case of doubt, reject
+}
+
+/**
+ * Prompt if the conversion should include the test runners.
+ *
+ * @returns Indicator if the conversion should include test runners.
+ */
+export async function includeTestRunnersPrompt(): Promise<boolean> {
+    const PROMPT_NAME = 'includeTests';
+    const question: PromptObject = {
+        type: 'confirm',
+        name: PROMPT_NAME,
+        initial: false,
+        message: 'Do you want to convert the test runners?'
+    };
+    const answer = (await prompt([question])) as Answers<typeof PROMPT_NAME>;
+    return (
+        answer.includeTests ??
+        (await Promise.reject(new Error('An error has occurred. The conversion has been canceled.')))
+    ); //in case of doubt, reject
+}

packages/app-config-writer/test/unit/preview-config/index.test.ts

@@ -20,7 +20,6 @@ describe('index', () => {
     const updatePreviewMiddlewareConfigsSpy = jest.spyOn(ui5Yaml, 'updatePreviewMiddlewareConfigs');
     const renameDefaultSandboxesSpy = jest.spyOn(previewFiles, 'renameDefaultSandboxes');
     const checkPrerequisitesSpy = jest.spyOn(prerequisites, 'checkPrerequisites');
-    const getExplicitApprovalToAdjustFilesSpy = jest.spyOn(prerequisites, 'getExplicitApprovalToAdjustFiles');
     const deleteNoLongerUsedFilesSpy = jest.spyOn(previewFiles, 'deleteNoLongerUsedFiles');
 
     beforeEach(() => {
@@ -35,11 +34,8 @@ describe('index', () => {
     });
     describe('convertToVirtualPreview', () => {
         test('convert project to virtual preview', async () => {
-            getExplicitApprovalToAdjustFilesSpy.mockResolvedValue(true);
-
             await convertToVirtualPreview(basePath, { convertTests: false, logger, fs });
             expect(checkPrerequisitesSpy).toHaveBeenCalled();
-            expect(getExplicitApprovalToAdjustFilesSpy).toHaveBeenCalled();
             expect(updatePreviewMiddlewareConfigsSpy).toHaveBeenCalled();
             expect(renameDefaultSandboxesSpy).toHaveBeenCalled();
             expect(deleteNoLongerUsedFilesSpy).toHaveBeenCalled();
@@ -65,12 +61,9 @@ describe('index', () => {
             `
             );
 
-            getExplicitApprovalToAdjustFilesSpy.mockResolvedValue(true);
-
             await convertToVirtualPreview(basePath, { convertTests: true, logger, fs });
             expect(fs.read(join(basePath, 'ui5.yaml'))).toMatchSnapshot();
             expect(checkPrerequisitesSpy).toHaveBeenCalled();
-            expect(getExplicitApprovalToAdjustFilesSpy).toHaveBeenCalled();
             expect(updatePreviewMiddlewareConfigsSpy).toHaveBeenCalled();
             expect(renameDefaultSandboxesSpy).toHaveBeenCalled();
             expect(deleteNoLongerUsedFilesSpy).toHaveBeenCalled();
@@ -107,13 +100,11 @@ describe('index', () => {
                       - framework: "OPA5"
             `
             );
-            getExplicitApprovalToAdjustFilesSpy.mockResolvedValue(true);
 
             await convertToVirtualPreview(basePath, { convertTests: true, logger, fs });
             expect(fs.read(join(basePath, 'ui5.yaml'))).toMatchSnapshot();
             expect(fs.read(join(basePath, 'ui5-test.yaml'))).toMatchSnapshot();
             expect(checkPrerequisitesSpy).toHaveBeenCalled();
-            expect(getExplicitApprovalToAdjustFilesSpy).toHaveBeenCalled();
             expect(updatePreviewMiddlewareConfigsSpy).toHaveBeenCalled();
             expect(renameDefaultSandboxesSpy).toHaveBeenCalled();
             expect(deleteNoLongerUsedFilesSpy).toHaveBeenCalled();
@@ -131,22 +122,10 @@ describe('index', () => {
                 convertToVirtualPreview(missingPrerequisitesPath, { convertTests: false, logger, fs })
             ).rejects.toThrowError(`The prerequisites are not met. For more information, see the log messages above.`);
             expect(checkPrerequisitesSpy).toHaveBeenCalled();
-            expect(getExplicitApprovalToAdjustFilesSpy).not.toHaveBeenCalled();
             expect(updatePreviewMiddlewareConfigsSpy).not.toHaveBeenCalled();
             expect(renameDefaultSandboxesSpy).not.toHaveBeenCalled();
             expect(deleteNoLongerUsedFilesSpy).not.toHaveBeenCalled();
             expect(updateVariantsCreationScriptSpy).not.toHaveBeenCalled();
         });
-
-        test('do not convert project to virtual preview - missing approval', async () => {
-            getExplicitApprovalToAdjustFilesSpy.mockResolvedValue(false);
-
-            await convertToVirtualPreview(basePath, { convertTests: false, logger, fs });
-            expect(checkPrerequisitesSpy).toHaveBeenCalled();
-            expect(getExplicitApprovalToAdjustFilesSpy).toHaveBeenCalled();
-            expect(errorLogMock).toHaveBeenCalledWith(
-                'You have not approved the conversion. The conversion has been aborted.'
-            );
-        });
     });
 });

packages/app-config-writer/test/unit/prompt/preview-config.test.ts

@@ -0,0 +1,42 @@
+import { includeTestRunnersPrompt, simulatePrompt } from '../../../src';
+
+let promptReturnObject: object;
+jest.mock('prompts', () => {
+    return {
+        prompt: () => {
+            return promptReturnObject;
+        }
+    };
+});
+
+describe('Test prompts for convert preview', () => {
+    test('Test simulatePrompt - true', async () => {
+        promptReturnObject = { simulate: true };
+        expect(await simulatePrompt()).toBeTruthy();
+    });
+
+    test('Test simulatePrompt - false', async () => {
+        promptReturnObject = { simulate: false };
+        expect(await simulatePrompt()).toBeFalsy();
+    });
+
+    test('Test simulatePrompt - cancel', async () => {
+        promptReturnObject = { undefined };
+        await expect(simulatePrompt()).rejects.toThrowError();
+    });
+
+    test('Test includeTestRunnersPrompt - true', async () => {
+        promptReturnObject = { includeTests: true };
+        expect(await includeTestRunnersPrompt()).toBeTruthy();
+    });
+
+    test('Test includeTestRunnersPrompt - false', async () => {
+        promptReturnObject = { includeTests: false };
+        expect(await includeTestRunnersPrompt()).toBeFalsy();
+    });
+
+    test('Test includeTestRunnersPrompt - cancel', async () => {
+        promptReturnObject = { undefined };
+        await expect(includeTestRunnersPrompt()).rejects.toThrowError();
+    });
+});

packages/create/src/cli/convert/preview.ts

@@ -1,21 +1,19 @@
 import type { Command } from 'commander';
 import { getLogger, setLogLevelVerbose, traceChanges } from '../../tracing';
-import { convertToVirtualPreview } from '@sap-ux/app-config-writer';
+import { convertToVirtualPreview, simulatePrompt, includeTestRunnersPrompt } from '@sap-ux/app-config-writer';
+
 /**
  * Add a new sub-command to convert the preview of a project to virtual files.
  *
  * @param {Command} cmd - The command to add the convert sub-command to.
  */
 export function addConvertPreviewCommand(cmd: Command): void {
     cmd.command('preview-config [path]')
-        .option('-s, --simulate', 'simulate only do not write or install')
+        .option('-s, --simulate', 'simulate only do not write')
         .option('-v, --verbose', 'show verbose information')
         .option('-t, --tests', 'also convert test suite and test runners')
         .action(async (path, options) => {
-            if (options.verbose === true || options.simulate) {
-                setLogLevelVerbose();
-            }
-            await convertPreview(path, !!options.simulate, !!options.tests);
+            await convertPreview(path, options.simulate, options.tests, options.verbose);
         });
 }
 
@@ -25,15 +23,42 @@ export function addConvertPreviewCommand(cmd: Command): void {
  * @param {string} basePath - The path to the adaptation project.
  * @param {boolean} simulate - If set to true, then no files will be written to the filesystem.
  * @param {boolean} convertTests - If set to true, then test suite and test runners fill be included in the conversion.
+ * @param {boolean} verbose - If set to true, then verbose information will be logged.
  */
-async function convertPreview(basePath: string, simulate: boolean, convertTests: boolean): Promise<void> {
-    const logger = getLogger();
+async function convertPreview(
+    basePath: string,
+    simulate: boolean | undefined,
+    convertTests: boolean | undefined,
+    verbose = false
+): Promise<void> {
+    let logger = getLogger();
 
     if (!basePath) {
         basePath = process.cwd();
     }
 
-    logger.debug(`Called convert preview-config for path '${basePath}', simulate is '${simulate}'.`);
+    simulate =
+        simulate ??
+        (await simulatePrompt().catch((error: Error) => {
+            logger.error(error.message);
+            return process.exit(1);
+        }));
+    if (simulate || verbose) {
+        setLogLevelVerbose();
+    }
+    // Reinitialize logger with verbose log level
+    logger = getLogger();
+
+    convertTests =
+        convertTests ??
+        (await includeTestRunnersPrompt().catch((error: Error) => {
+            logger.error(error.message);
+            return process.exit(1);
+        }));
+
+    logger.debug(
+        `Called convert preview-config for path '${basePath}', simulate is '${simulate}', convert tests is '${convertTests}'.`
+    );
     try {
         const fs = await convertToVirtualPreview(basePath, { convertTests, logger });
 

packages/create/test/unit/cli/convert/preview.test.ts

@@ -6,7 +6,6 @@ import * as appConfigWriter from '@sap-ux/app-config-writer';
 import * as logger from '../../../../src/tracing/logger';
 import * as childProcess from 'child_process';
 import { join } from 'path';
-
 jest.mock('child_process');
 jest.mock('prompts');
 
@@ -16,6 +15,8 @@ describe('Test command convert preview', () => {
     let fsMock: Editor;
     let logLevelSpy: jest.SpyInstance;
     let spawnSpy: jest.SpyInstance;
+    const simulatePromptSpy = jest.spyOn(appConfigWriter, 'simulatePrompt');
+    const includeTestRunnersPromptSpy = jest.spyOn(appConfigWriter, 'includeTestRunnersPrompt');
 
     const getArgv = (arg: string[]) => ['', '', ...arg];
 
@@ -41,6 +42,8 @@ describe('Test command convert preview', () => {
     });
 
     test('Test create-fiori convert preview <appRoot>', async () => {
+        simulatePromptSpy.mockResolvedValue(false);
+        includeTestRunnersPromptSpy.mockResolvedValue(false);
         // Test execution
         const command = new Command('convert');
         addConvertPreviewCommand(command);
@@ -71,6 +74,8 @@ describe('Test command convert preview', () => {
     });
 
     test('Test create-fiori convert preview --verbose', async () => {
+        simulatePromptSpy.mockResolvedValue(false);
+        includeTestRunnersPromptSpy.mockResolvedValue(false);
         // Test execution
         const command = new Command('convert');
         addConvertPreviewCommand(command);
@@ -83,4 +88,71 @@ describe('Test command convert preview', () => {
         expect(fsMock.commit).toBeCalled();
         expect(spawnSpy).not.toBeCalled();
     });
+
+    test('Test create-fiori convert preview with simulate from prompt', async () => {
+        simulatePromptSpy.mockResolvedValue(true);
+        includeTestRunnersPromptSpy.mockResolvedValue(false);
+        // Test execution
+        const command = new Command('convert');
+        addConvertPreviewCommand(command);
+        await command.parseAsync(getArgv(['preview-config']));
+
+        // Result check
+        expect(logLevelSpy).toBeCalled();
+        expect(loggerMock.warn).not.toBeCalled();
+        expect(loggerMock.error).not.toBeCalled();
+        expect(spawnSpy).not.toBeCalled();
+        expect(fsMock.commit).not.toBeCalled();
+    });
+
+    test('Test create-fiori convert preview with simulate cancelled from prompt', async () => {
+        const mockExit = jest.spyOn(process, 'exit').mockImplementation();
+        simulatePromptSpy.mockResolvedValue(Promise.reject(new Error('test error')));
+        // Test execution
+        const command = new Command('convert');
+        addConvertPreviewCommand(command);
+        await command.parseAsync(getArgv(['preview-config']));
+
+        // Result check
+        expect(mockExit).toHaveBeenCalledWith(1);
+        expect(logLevelSpy).not.toBeCalled();
+        expect(loggerMock.warn).not.toBeCalled();
+        expect(loggerMock.error).toBeCalled();
+        expect(spawnSpy).not.toBeCalled();
+        //can't check for fs.commit here as we don't exit on process.exit(1)
+    });
+
+    test('Test create-fiori convert preview with simulate and test from prompt', async () => {
+        simulatePromptSpy.mockResolvedValue(true);
+        includeTestRunnersPromptSpy.mockResolvedValue(true);
+        // Test execution
+        const command = new Command('convert');
+        addConvertPreviewCommand(command);
+        await command.parseAsync(getArgv(['preview-config']));
+
+        // Result check
+        expect(logLevelSpy).toBeCalled();
+        expect(loggerMock.warn).not.toBeCalled();
+        expect(loggerMock.error).not.toBeCalled();
+        expect(spawnSpy).not.toBeCalled();
+        expect(fsMock.commit).not.toBeCalled();
+    });
+
+    test('Test create-fiori convert preview with simulate and test cancelled from prompt', async () => {
+        const mockExit = jest.spyOn(process, 'exit').mockImplementation();
+        simulatePromptSpy.mockResolvedValue(true);
+        includeTestRunnersPromptSpy.mockResolvedValue(Promise.reject(new Error('test error')));
+        // Test execution
+        const command = new Command('convert');
+        addConvertPreviewCommand(command);
+        await command.parseAsync(getArgv(['preview-config']));
+
+        // Result check
+        expect(mockExit).toHaveBeenCalledWith(1);
+        expect(logLevelSpy).toBeCalled();
+        expect(loggerMock.warn).not.toBeCalled();
+        expect(loggerMock.error).toBeCalled();
+        expect(spawnSpy).not.toBeCalled();
+        expect(fsMock.commit).not.toBeCalled();
+    });
 });