Skip to content

Commit

Permalink
fix(ui5-app-inq): Checks if folderPath contains existing Fiori project (
Browse files Browse the repository at this point in the history
#2317)

* fix(ui5-app-inq): check fiori app, return error message

* fix(ui5-app-inq): unit test

* fix(ui5-app-inq): changeset

* fix(ui5-app-inq): fiori app validator

* Update index.ts

* fix(ui5-app-inq): fix for async

* fix(ui5-app-inq): code complexity

* fix(ui5-app-inq): changeset, jsdoc

* fix(ui5-app-inq): remove unnecessary code

* fix(ui5-app-inq): reorder function call

* fix(ui5-app-inq): optional fiori app check

* fix(ui5-app-inq): cleanup logic

* fix(ui5-app-inq): type description update

* fix(ui5-app-inq): update description
  • Loading branch information
martinjanus-sap authored Sep 12, 2024
1 parent 52a9d28 commit 09522df
Show file tree
Hide file tree
Showing 12 changed files with 150 additions and 21 deletions.
6 changes: 6 additions & 0 deletions .changeset/eleven-ducks-whisper.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sap-ux/ui5-application-inquirer': patch
'@sap-ux/project-access': patch
---

Validates provided path does not contain a Fiori project, appropriate validation message displayed.
3 changes: 2 additions & 1 deletion packages/project-access/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ export {
filterDataSourcesByType,
updatePackageScript,
findCapProjectRoot,
hasUI5CliV3
hasUI5CliV3,
findRootsForPath
} from './project';
export * from './types';
export * from './library';
3 changes: 2 additions & 1 deletion packages/project-access/src/project/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ export {
findFioriArtifacts,
findProjectRoot,
getAppRootFromWebappPath,
findCapProjectRoot
findCapProjectRoot,
findRootsForPath
} from './search';
export { getWebappPath, readUi5Yaml } from './ui5-config';
export { getMtaPath } from './mta';
Expand Down
2 changes: 1 addition & 1 deletion packages/project-access/src/project/search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ export async function getAppRootFromWebappPath(webappPath: string): Promise<stri
* @param path - path to check, e.g. to the manifest.json
* @returns - in case a supported app is found this function returns the appRoot and projectRoot path
*/
async function findRootsForPath(path: string): Promise<{ appRoot: string; projectRoot: string } | null> {
export async function findRootsForPath(path: string): Promise<{ appRoot: string; projectRoot: string } | null> {
try {
// Get the root of the app, that is where the package.json is, otherwise not supported
const appRoot = await findProjectRoot(path, false);
Expand Down
26 changes: 26 additions & 0 deletions packages/ui5-application-inquirer/src/prompts/prompt-helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { join } from 'path';
import { coerce, gte } from 'semver';
import { defaultProjectNumber, t } from '../i18n';
import { promptNames, type UI5ApplicationPromptOptions, type UI5ApplicationQuestion } from '../types';
import { validateProjectFolder } from '@sap-ux/project-input-validator';
import { validateFioriAppProjectFolder } from './validators';

/**
* Tests if a directory with the specified `appName` exists at the path specified by `targetPath`.
Expand Down Expand Up @@ -83,3 +85,27 @@ export function hidePrompts(
}
return questions;
}

/**
* @param targetPath the target directory path.
* @param appName the application directory name.
* @param validateFioriAppFolder if true, validates the target path as a Fiori App project.
* @returns true if validated for Fiori App Project and Project Folder, false if appName length is less than 2. Otherwise appropriate validation message.
*/
export async function validateTargetFolder(
targetPath: string,
appName: string,
validateFioriAppFolder?: boolean
): Promise<string | boolean> {
if (validateFioriAppFolder === true) {
const isFioriValid = await validateFioriAppProjectFolder(targetPath);
if (isFioriValid !== true) {
return isFioriValid;
}
}
const isProjectValid = validateProjectFolder(targetPath, appName);
if (isProjectValid !== true) {
return isProjectValid;
}
return true;
}
16 changes: 10 additions & 6 deletions packages/ui5-application-inquirer/src/prompts/prompts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {
extendWithOptions
} from '@sap-ux/inquirer-common';
import { getMtaPath } from '@sap-ux/project-access';
import { validateModuleName, validateNamespace, validateProjectFolder } from '@sap-ux/project-input-validator';
import { validateModuleName, validateNamespace } from '@sap-ux/project-input-validator';
import {
defaultVersion,
getDefaultUI5Theme,
Expand All @@ -25,7 +25,7 @@ import type { ListChoiceOptions } from 'inquirer';
import { t } from '../i18n';
import type { UI5ApplicationAnswers, UI5ApplicationPromptOptions, UI5ApplicationQuestion } from '../types';
import { promptNames } from '../types';
import { defaultAppName, hidePrompts, isVersionIncluded } from './prompt-helpers';
import { defaultAppName, hidePrompts, isVersionIncluded, validateTargetFolder } from './prompt-helpers';
import { validateAppName } from './validators';

/**
Expand Down Expand Up @@ -59,7 +59,10 @@ export function getQuestions(
[promptNames.title]: getTitlePrompt(),
[promptNames.namespace]: getNamespacePrompt(appName),
[promptNames.description]: getDescriptionPrompt(),
[promptNames.targetFolder]: getTargetFolderPrompt(targetDir),
[promptNames.targetFolder]: getTargetFolderPrompt(
targetDir,
promptOptions?.[promptNames.targetFolder]?.validateFioriAppFolder
),
[promptNames.ui5Version]: getUI5VersionPrompt(ui5Versions, promptOptions?.ui5Version),
[promptNames.addDeployConfig]: getAddDeployConfigPrompt(
targetDir,
Expand Down Expand Up @@ -341,9 +344,10 @@ function getUI5VersionPrompt(
* Gets the `targetFolder` prompt.
*
* @param targetDir provides a default value for the target folder path
* @param validateFioriAppFolder validates the target folder path as a Fiori app project
* @returns the `targetFolder` prompt
*/
function getTargetFolderPrompt(targetDir: string): UI5ApplicationQuestion {
function getTargetFolderPrompt(targetDir: string, validateFioriAppFolder?: boolean): UI5ApplicationQuestion {
return {
type: 'input',
name: promptNames.targetFolder,
Expand All @@ -355,9 +359,9 @@ function getTargetFolderPrompt(targetDir: string): UI5ApplicationQuestion {
breadcrumb: t('prompts.appFolderPathBreadcrumb')
},
default: (answers: UI5ApplicationAnswers) => answers.targetFolder || targetDir,
validate: (target, { name = '' }: UI5ApplicationAnswers): boolean | string => {
validate: async (target, { name = '' }: UI5ApplicationAnswers): Promise<boolean | string> => {
if (name.length > 2) {
return validateProjectFolder(target, name);
return await validateTargetFolder(target, name, validateFioriAppFolder);
}
return false;
}
Expand Down
17 changes: 16 additions & 1 deletion packages/ui5-application-inquirer/src/prompts/validators.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { validateModuleName } from '@sap-ux/project-input-validator';
import { appPathExists } from './prompt-helpers';
import { t } from '../i18n';

import { findRootsForPath } from '@sap-ux/project-access';
/**
* Returns true (valid) if the specified projectName is a valid module name
* and if an application folder (directory) at the specified path does not exist.
Expand All @@ -22,3 +22,18 @@ export function validateAppName(appName: string, targetDir: string): boolean | s
}
return true;
}

/**
* Returns true if the specified target path does not contain a Fiori project.
*
* @param targetDir the target directory path.
* @returns true, if not Fiori Project, or string message indicating that the path contains a Fiori project.
*/
export async function validateFioriAppProjectFolder(targetDir: string): Promise<string | boolean> {
const appRoot = await findRootsForPath(targetDir);
if (appRoot) {
return t('validators.folderContainsFioriApp', { path: appRoot.appRoot });
} else {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,15 +33,15 @@
"appEnableTypeScriptMessage": "Enable TypeScript",
"appShowAdvancedOptionsMessage": "Configure advanced options",
"appShowAdvancedOptionsHint": "Choosing 'No' will apply defaults"

},
"validators": {
"appFolderExistsAtPath": "A module with this name already exists in the folder: {{-path}}"
"appFolderExistsAtPath": "A module with this name already exists in the folder: {{-path}}",
"folderContainsFioriApp": "The project folder path already contains an SAP Fiori application in the folder: {{-path}}. Please choose a different folder and try again."
},
"ui5VersionLabels": {
"maintained": "Maintained",
"outOfMaintenance": "Out of maintenance",
"version_one": "version",
"version_other": "versions"
}
}
}
13 changes: 13 additions & 0 deletions packages/ui5-application-inquirer/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,19 @@ type TargetFolderPromptOptions = {
* Note that if a `default` option is also provided then this will be used instead of the `defaultValue` option.
*/
defaultValue?: string;
/**
* If set to `true`, the target folder prompt's validator will perform additional validation to
* determine if the specified target path is contained in an existing Fiori application project path, which is invalid.
*
* **Behavior**:
* - **CAP Projects**: Validates if the target folder is part of a CAP project with a supported Fiori app.
* - **Non-CAP Projects**: Checks for recognised SAP Fiori apps, such as Fiori elements or SAPUI5
* freestyle apps that have the correct structure and required dependencies.
* - **Validation Outcome**: Returns a validation message if the target folder meets the Fiori app criteria.
*
* If `false` or not provided, only ui5 project validation is performed without specific Fiori app checks.
*/
validateFioriAppFolder?: boolean;
};

type NamePromptOptions = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,21 @@ import { latestVersionString } from '@sap-ux/ui5-info';
import { join } from 'path';
import { initI18nUi5AppInquirer } from '../../../src/i18n';
import * as promptHelpers from '../../../src/prompts/prompt-helpers';
import { appPathExists, defaultAppName, hidePrompts, isVersionIncluded } from '../../../src/prompts/prompt-helpers';
import {
appPathExists,
defaultAppName,
hidePrompts,
isVersionIncluded,
validateTargetFolder
} from '../../../src/prompts/prompt-helpers';
import type { UI5ApplicationAnswers, UI5ApplicationPromptOptions, UI5ApplicationQuestion } from '../../../src/types';
import { promptNames } from '../../../src/types';
import * as projectValidators from '@sap-ux/project-input-validator';
import * as validators from '../../../src/prompts/validators';

jest.mock('@sap-ux/project-input-validator', () => ({
validateProjectFolder: jest.fn()
}));

describe('prompt-helpers', () => {
const testTempDir = join(__dirname, './test-tmp');
Expand Down Expand Up @@ -132,4 +144,25 @@ describe('prompt-helpers', () => {
expect(filteredPrompts).toEqual(expect.not.arrayContaining([{ name: promptNames.skipAnnotations }]));
expect(filteredPrompts).toEqual(expect.not.arrayContaining([{ name: promptNames.ui5Version }]));
});
test('validateTargetFolder', async () => {
// Test when name length > 2 and both validations pass
jest.spyOn(projectValidators, 'validateProjectFolder').mockReturnValue(true);
jest.spyOn(validators, 'validateFioriAppProjectFolder').mockResolvedValue(true);
const resultForValidCase = await validateTargetFolder('/some/target/path', 'validName', true);
expect(resultForValidCase).toBe(true);
});
test('validateTargetFolder - project folder validation error', async () => {
// Test when Project validation fails
const projectErrorMessage = 'Project validation error';
jest.spyOn(projectValidators, 'validateProjectFolder').mockReturnValue(projectErrorMessage);
const resultForProjectError = await validateTargetFolder('/some/target/path', 'validName');
expect(resultForProjectError).toBe(projectErrorMessage);
});
test('validateTargetFolder - fiori app project validation error', async () => {
// Test when Fiori validation fails
const fioriErrorMessage = 'Fiori validation error';
jest.spyOn(validators, 'validateFioriAppProjectFolder').mockResolvedValue(fioriErrorMessage);
const resultForFioriError = await validateTargetFolder('/some/target/path', 'validName', true);
expect(resultForFioriError).toBe(fioriErrorMessage);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,7 @@ describe('getQuestions', () => {
).toMatchInlineSnapshot(`"abc 123"`);
});

test('getQuestions, prompt: `targetFolder`', () => {
test('getQuestions, prompt: `targetFolder`', async () => {
const mockCwd = '/any/current/working/directory';
jest.spyOn(process, 'cwd').mockReturnValueOnce(mockCwd);
let questions = getQuestions([]);
Expand All @@ -213,12 +213,13 @@ describe('getQuestions', () => {
// validators
questions = getQuestions([]);
targetFolderPrompt = questions.find((question) => question.name === promptNames.targetFolder);
expect(targetFolderPrompt?.validate!(undefined, {})).toEqual(false);

const projectValidatorSpy = jest.spyOn(projectValidators, 'validateProjectFolder').mockReturnValueOnce(true);
await expect(targetFolderPrompt?.validate!(undefined, {})).resolves.toEqual(false);

const validateTargetFolderSpy = jest.spyOn(promptHelpers, 'validateTargetFolder').mockResolvedValueOnce(true);
const args = ['/some/target/path', { name: 'project1' }] as const;
expect(targetFolderPrompt?.validate!(...args)).toEqual(true);
expect(projectValidatorSpy).toHaveBeenCalledWith(...[args[0], args[1].name]);
await expect(targetFolderPrompt?.validate!(...args)).resolves.toEqual(true);
expect(validateTargetFolderSpy).toHaveBeenCalledWith(...[args[0]], args[1].name, undefined);

// Test `defaultValue` prompt option - should not replace existing default function
const promptOptionsDefaultValue = {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import * as projectValidators from '@sap-ux/project-input-validator';
import * as promptHelpers from '../../../src/prompts/prompt-helpers';
import { join } from 'path';
import { initI18nUi5AppInquirer } from '../../../src/i18n';
import { validateAppName } from '../../../src/prompts/validators';
import { initI18nUi5AppInquirer, t } from '../../../src/i18n';
import { validateAppName, validateFioriAppProjectFolder } from '../../../src/prompts/validators';
import { findRootsForPath } from '@sap-ux/project-access';

/**
* Workaround to allow spyOn
Expand All @@ -14,6 +15,10 @@ jest.mock('@sap-ux/project-input-validator', () => {
};
});

jest.mock('@sap-ux/project-access', () => ({
findRootsForPath: jest.fn()
}));

describe('validators', () => {
beforeAll(async () => {
await initI18nUi5AppInquirer();
Expand Down Expand Up @@ -42,4 +47,28 @@ describe('validators', () => {
`A module with this name already exists in the folder: ${targetPath}`
);
});
describe('validateFioriAppProjectFolder', () => {
const mockFindRootsForPath = jest.fn();

beforeEach(() => {
(findRootsForPath as jest.Mock) = mockFindRootsForPath;
jest.clearAllMocks();
});

test('should return true if no Fiori project is found in the target directory', async () => {
mockFindRootsForPath.mockResolvedValue(null);
const result = await validateFioriAppProjectFolder('/path/to/dir');
expect(result).toBe(true);
expect(mockFindRootsForPath).toHaveBeenCalledWith('/path/to/dir');
});

test('should return an error message if a Fiori project is found in the target directory', async () => {
const appRootPath = '/path/to/fiori/project';
const projectRootPath = 'test/path';
mockFindRootsForPath.mockResolvedValue({ appRoot: appRootPath, projectRoot: projectRootPath });
const result = await validateFioriAppProjectFolder('some/path');
expect(result).toEqual(t('validators.folderContainsFioriApp', { path: appRootPath }));
expect(mockFindRootsForPath).toHaveBeenCalledWith('some/path');
});
});
});

0 comments on commit 09522df

Please sign in to comment.