feat(generator-adp): Make possible to configure the Adaptation project generator from a json. #3188

avasilev-sap · 2025-04-25T09:46:33Z

Feat for #3015 .

Introduce the json workflow - we can start the yeoman generator from the CLI passing a json string as an argument to the command. The workflow bypasses all prompts, and the project is generated without any additional configuration.
Add unit + integration tests.

Testing done:

Manually verified after installing the generator locally and creating a project with the following configuration:
yo @sap-ux/adp '{ "targetFolder": "/Users/I762682/Workspace/adaptations", "application": "sap.ui.demoapps.rta.fe", "system": "U1Y_099", "client": "099", "username": "VASILEVATA", "password": "...", "projectName": "test.json.flow", "applicationTitle": "Test json flow", "namespace": "customer.test.json.flow" }'. After the project creation the adaptation editor is started successfully.

changeset-bot · 2025-04-25T09:46:38Z

⚠️ No Changeset found

Latest commit: 0131b29

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

cla-assistant · 2025-04-25T09:46:48Z

All committers have signed the CLA.

packages/generator-adp/src/translations/generator-adp.i18n.json

lfindlaysap

@avasilev-sap, I've added a few suggestions.

sonarqubecloud · 2025-04-25T10:21:45Z

Quality Gate passed

Issues
4 New issues
0 Accepted issues

Measures
0 Security Hotspots
92.7% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

nikmace

Overall the code looks good. There are 4 issues in the Sonar that need to be fixed (1 of them should be accepted though). Also, the text on which Louise requested changes.

nikmace · 2025-04-25T11:25:12Z

packages/generator-adp/src/app/index.ts

+    private async _validateJsonInput({
+        projectName,
+        targetFolder,
+        namespace,
+        system
+    }: {
+        projectName: string;
+        targetFolder: string;
+        namespace: string;
+        system: string;
+    }): Promise<void> {
+        let validationResult = validateProjectName(projectName, targetFolder, this.isCustomerBase);
+        if (isString(validationResult)) {
+            throw new Error(validationResult);
+        }
+
+        validationResult = validateNamespaceAdp(namespace, projectName, this.isCustomerBase);
+        if (isString(validationResult)) {
+            throw new Error(validationResult);
+        }
+
+        const systemEndpoint = await this.systemLookup.getSystemByName(system);
+        if (!systemEndpoint) {
+            throw new Error(t('error.systemNotFound', { systemName: system }));
+        }
+    }


I think this part of the code could be extracted into the existing validators.ts file to not clutter the generator code here. This will be better for tests as I will explain below in the in my other comment.

I think it would be fine instead of an object to pass all of the properties inline, since they are 4-5. We also need to pass the systemLookup instance and fix the JSDoc afterwards.

/** * Validates input fields for project name, namespace, and system. * * @param {string} projectName - Name of the project. * @param {string} targetFolder - Path to the target folder. * @param {string} namespace - Namespace of the application. * @param {string} system - Selected system name. * @param {SystemLookup} systemLookup - Instance of system lookup service. */ export function validateJsonInput(projectName: string, targetFolder: string, namespace: string, system: string, systemLookup: SystemLookup) {}

Also, suggesting to modify the translation parameter systemName to just system, that way you will just do this:

throw new Error(t('error.systemNotFound', { system }));

nikmace · 2025-04-25T11:42:06Z

packages/generator-adp/src/app/index.ts

+            // Application config
+            application: baseApplicationName,
+            applicationTitle,
+            applicationComponentHierarchy = '',


This variable is never used in this code, so it could be removed for the time being, at least until this PR goes in: #3146. It should also be removed from ConfigAnswers interface.

nikmace · 2025-04-25T11:44:57Z

packages/generator-adp/src/app/index.ts

+        }
+
+        const {
+            // System config


These comments are not needed.

nikmace · 2025-04-25T12:06:02Z

packages/generator-adp/src/utils/parse-json-arg.ts

+export function parseJsonInput(jsonString: string): Record<string, string> | undefined {
+    try {
+        const parsed = JSON.parse(jsonString);
+
+        if (!isRecordOfStrings(parsed)) {
+            return;
+        }
+
+        return parsed;
+    } catch (error) {
+        // Do nothing.
+    }
+}


I think it would be good to have an interface for this instead of using a Record<string, string> so we know exactly what could come in this JSON? Also, we should explicitly return undefined.

nikmace · 2025-04-25T12:16:11Z

packages/generator-adp/src/utils/parse-json-arg.ts

+/**
+ * Returns the first argument from a list of CLI arguments. If the first argument
+ * is not a string returns empty string.
+ *
+ * @param args The list of CLI command arguments.
+ * @returns The first parameter in the argument's list as string.
+ */
+export function getFirstArgAsString(args: string | string[]): string {


All of the JSDocs should look the same or be similar in terms of structure.

Although this is optional and the benefits are not so much, I try to put the types for the properties in the JSDoc (see args).
Also, after the @param {type} property usually comes a dash ("-").

All of the above make the comments more sophisticated, helping both humans and machines understand your code.

Suggested change

/**

* Returns the first argument from a list of CLI arguments. If the first argument

* is not a string returns empty string.

*

* @param args The list of CLI command arguments.

* @returns The first parameter in the argument's list as string.

*/

export function getFirstArgAsString(args: string | string[]): string {

/**

* Returns the first argument from a list of CLI arguments. If the first argument

* is not a string returns empty string.

*

* @param {string | string[]} args - The list of CLI command arguments.

* @returns {string} The first parameter in the argument's list as string.

*/

export function getFirstArgAsString(args: string | string[]): string {

nikmace · 2025-04-25T12:21:57Z

packages/generator-adp/test/app.test.ts

+    it('should throw an error when the project name in the json input is not valid', async () => {
+        const jsonInput = {
+            system: 'urlA',
+            username: 'user1',
+            password: 'pass1',
+            client: '010',
+            application: 'sap.ui.demoapps.f1',
+            projectName: 'Invalid project name',
+            targetFolder: testOutputDir
+        };
+        const jsonInputString = JSON.stringify(jsonInput);
+
+        const runContext = yeomanTest
+            .create(adpGenerator, { resolved: generatorPath }, { cwd: testOutputDir })
+            .withArguments([jsonInputString]);
+
+        await expect(runContext.run()).rejects.toThrow(
+            t('adp.projectNameUppercaseError', { ns: PROJECT_INPUT_VALIDATOR_NS })
+        );
+    });
+
+    it('should throw an error when the namespace in the json input is not valid', async () => {
+        const jsonInput = {
+            system: 'urlA',
+            username: 'user1',
+            password: 'pass1',
+            client: '010',
+            application: 'sap.ui.demoapps.f1',
+            projectName: 'failure',
+            namespace: 'customer.invalid namespace',
+            targetFolder: testOutputDir
+        };
+        const jsonInputString = JSON.stringify(jsonInput);
+
+        const runContext = yeomanTest
+            .create(adpGenerator, { resolved: generatorPath }, { cwd: testOutputDir })
+            .withArguments([jsonInputString]);
+
+        await expect(runContext.run()).rejects.toThrow(
+            t('adp.namespaceValidationError', { ns: PROJECT_INPUT_VALIDATOR_NS })
+        );
+    });
+
+    it('should throw an error when the system is not found in the json workflow', async () => {
+        const jsonInput = {
+            system: 'nonexistent system',
+            username: 'user1',
+            password: 'pass1',
+            client: '010',
+            application: 'sap.ui.demoapps.f1',
+            projectName: 'failure',
+            namespace: 'customer.my.app',
+            applicationTitle: 'My app title',
+            targetFolder: testOutputDir
+        };
+        const jsonInputString = JSON.stringify(jsonInput);
+
+        const runContext = yeomanTest
+            .create(adpGenerator, { resolved: generatorPath }, { cwd: testOutputDir })
+            .withArguments([jsonInputString]);
+
+        await expect(runContext.run()).rejects.toThrow(t('error.systemNotFound', { systemName: jsonInput.system }));
+    });


When you extract the validations in a separate file, you will no longer need these tests. Just mock the validateJsonInput function. There should not be so many integration tests just to check if something breaks in the writing phase. We already have one test above which checks if there is an error thrown during the writing phase, no need for more in my opinion. To make sure the test works, you have added a successful integration test that generates a project from JSON inpu,t and that is enough.

nikmace · 2025-04-25T12:25:51Z

packages/generator-adp/src/app/index.ts

+import { getFirstArgAsString, parseJsonInput } from '../utils/parse-json-arg';
+import type { AbapServiceProvider } from '@sap-ux/axios-extension';
+import { getDefaultNamespace, getDefaultProjectName } from './questions/helper/default-values';
+import { validateNamespaceAdp, validateProjectName } from '@sap-ux/project-input-validator';
+import { isString } from '../utils/type-guards';


Could you please reorder the imports so they are like this:
First npm packages -> then monorepo packages -> local imports

I like to order them in a certain way but this is optional:

feat: make possible to create adaptation project from json input

cabacee

avasilev-sap added the generator-adp @sap-ux/generator-adp label Apr 25, 2025

avasilev-sap requested a review from nikmace April 25, 2025 09:46

avasilev-sap self-assigned this Apr 25, 2025

avasilev-sap requested review from a team as code owners April 25, 2025 09:46

lfindlaysap reviewed Apr 25, 2025

View reviewed changes

packages/generator-adp/src/translations/generator-adp.i18n.json Show resolved Hide resolved

lfindlaysap reviewed Apr 25, 2025

View reviewed changes

packages/generator-adp/src/translations/generator-adp.i18n.json Show resolved Hide resolved

lfindlaysap requested changes Apr 25, 2025

View reviewed changes

test: update snapshot for adaptation project json workflow

0131b29

nikmace requested changes Apr 25, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(generator-adp): Make possible to configure the Adaptation project generator from a json. #3188

feat(generator-adp): Make possible to configure the Adaptation project generator from a json. #3188

avasilev-sap commented Apr 25, 2025

changeset-bot bot commented Apr 25, 2025 •

edited

Loading

cla-assistant bot commented Apr 25, 2025 •

edited

Loading

lfindlaysap left a comment

sonarqubecloud bot commented Apr 25, 2025

nikmace left a comment •

edited

Loading

nikmace Apr 25, 2025

nikmace Apr 25, 2025

nikmace Apr 25, 2025

nikmace Apr 25, 2025

nikmace Apr 25, 2025

nikmace Apr 25, 2025

nikmace Apr 25, 2025

feat(generator-adp): Make possible to configure the Adaptation project generator from a json. #3188

Are you sure you want to change the base?

feat(generator-adp): Make possible to configure the Adaptation project generator from a json. #3188

Conversation

avasilev-sap commented Apr 25, 2025

changeset-bot bot commented Apr 25, 2025 • edited Loading

⚠️ No Changeset found

cla-assistant bot commented Apr 25, 2025 • edited Loading

lfindlaysap left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Apr 25, 2025

Quality Gate passed

nikmace left a comment • edited Loading

Choose a reason for hiding this comment

nikmace Apr 25, 2025

Choose a reason for hiding this comment

nikmace Apr 25, 2025

Choose a reason for hiding this comment

nikmace Apr 25, 2025

Choose a reason for hiding this comment

nikmace Apr 25, 2025

Choose a reason for hiding this comment

nikmace Apr 25, 2025

Choose a reason for hiding this comment

nikmace Apr 25, 2025

Choose a reason for hiding this comment

nikmace Apr 25, 2025

Choose a reason for hiding this comment

changeset-bot bot commented Apr 25, 2025 •

edited

Loading

cla-assistant bot commented Apr 25, 2025 •

edited

Loading

nikmace left a comment •

edited

Loading