feat: Switch Adaptation Project's generator/wizard to use Open Source writers #1775

IvoSG · 2024-03-25T11:32:04Z

Feat for #1611.

Adds writer functionality for adaptation projects.

…Projects

changeset-bot · 2024-03-25T11:32:07Z

🦋 Changeset detected

Latest commit: 35a6347

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 11 packages

Name	Type
@sap-ux/adp-tooling	Minor
@sap-ux/preview-middleware	Patch
@sap-ux/axios-extension	Patch
@sap-ux/create	Patch
@sap-ux/app-config-writer	Patch
@sap-ux/backend-proxy-middleware	Patch
@sap-ux/deploy-tooling	Patch
@sap-ux/environment-check	Patch
@sap-ux/system-access	Patch
@sap-ux/odata-cli	Patch
@sap-ux/generator-simple-fe	Patch

Not sure what this means? Click here to learn what changesets are.

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

…/1611/writersForAdpProjects

…SAP/open-ux-tools into feat/1611/writersForAdpProjects

sonarcloud · 2024-04-03T11:33:30Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

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

See analysis details on SonarCloud

nikmace

Coverage is currently at 88.7% and could be improved a little bit.

packages/adp-tooling/src/types.ts

packages/adp-tooling/src/writer/cf.ts

nikmace · 2024-04-04T05:54:48Z

packages/adp-tooling/src/writer/cf.ts

+    fs.copyTpl(join(tmplPath, '**/*.*'), join(basePath), config.app, undefined, {
+        globOptions: { dot: true, ignore: ['**/xs-security.json', '**/approuter/*.*', '**/manifest.appdescr_variant'] },
+        processDestinationPath: (filePath: string) =>
+            filePath
+                .replace(/gitignore.tmpl/g, '.gitignore')
+                .replace(/i18n\/i18n.properties/g, 'webapp/i18n/i18n.properties')
+    });


This can also be a new method that will handle the copying of the files. See the comment below for the full picture.

packages/adp-tooling/src/writer/s4hana.ts

packages/adp-tooling/templates/projects/abap/package.json

nikmace · 2024-04-04T07:05:34Z

packages/adp-tooling/test/unit/writer/__snapshots__/index.test.ts.snap

-        \\"@ui5/cli\\": \\"^3.9.1\\"
+        \\"@ui5/cli\\": \\"^3.6.0\\"


Just a question, why did this version change? It was ^3.9.1 before. ^3.6.0 will install the latest too.

.changeset/famous-panthers-occur.md

packages/adp-tooling/src/types.ts

packages/adp-tooling/templates/projects/abap/package.json

…ents and export only one generate method

nikmace

GitHub actions for some reason are not running and I cannot see the result of those voters. Also, there are some merge conflicts to be resolved.

Some comments from before have not been addressed yet. And could you please write what have you changed in my requested changes comments, so I can resolve those comments.

nikmace · 2024-04-16T10:00:33Z

packages/adp-tooling/templates/projects/onprem/package.json

@@ -14,7 +14,7 @@
        "@sap-ux/preview-middleware": "^0.11.1",
        "@sap-ux/ui5-proxy-middleware": "^1.3.0",
        "@sap-ux/deploy-tooling": "^0.11.7"<%}%>,
-        "@ui5/cli": "^3.9.1"
+        "@ui5/cli": "^3.6.0"


Same question as before, is there any reason for this version "downgrade".

packages/adp-tooling/src/base/project-utils.ts

packages/adp-tooling/src/types.ts

nikmace · 2024-04-16T13:16:25Z

packages/adp-tooling/src/types.ts

+export interface AdpCustomConfig {
+    adp: {
+        safeMode: boolean;
+        environment: ProjectType;


TBD: the environment is now 'OnPrem' but our change editors still have checks for 'ABAP'

@nikmace Why would the change editors need to know about the target environment?

packages/adp-tooling/templates/projects/s4/package.json

packages/adp-tooling/templates/projects/cf/xs-security.json

nikmace · 2024-04-16T13:24:17Z

packages/adp-tooling/src/base/project-utils.ts

+        const appdescrTplPath = join(templatePath, 'webapp/manifest.appdescr_variant');
+        const appdescrPath = join(projectPath, 'webapp/manifest.appdescr_variant');


Could you please use the enum from project access for different folders

packages/adp-tooling/src/base/project-utils.ts

packages/create/src/cli/generate/adaptation-project.ts

tobiasqueck

Sorry but that one is far away from being acceptable.

Please remove things like a pom.xml that only make sense for SAP internal projects and also remove all the duplicated files in the templates section.
Projects that are for an ABAP system are almost identical (no matter if on premise, private cloud our public cloud). The only difference is the additional build task and a minor change in the deploy script.
I am not an expert for CF but I am pretty sure that the generated structure is also not aligned with the latest guidance on CF projects.

My recommendation is actually to start from scratch again. Take the existing writer/templates (from main) and compare its results with the three different options. Then enhance the "core" that is the same for all option and add diffs if required for other targets. I'd recommend you have a look at the fiori-elements-writer and how it is designed.

…ml content generation

…cture, adapt commands, remove unused dependencie

packages/adp-tooling/test/unit/writer/index.test.ts

packages/adp-tooling/src/types.ts

tobiasqueck · 2024-05-08T07:04:18Z

packages/adp-tooling/templates/project/package.json

        "@sap-ux/preview-middleware": "^0.11.1",
        "@sap-ux/ui5-proxy-middleware": "^1.3.0",
        "@sap-ux/deploy-tooling": "^0.11.7"<%}%>,
-        "@ui5/cli": "^3.9.1"
+        "@ui5/task-adaptation": "^1.3.0",
+        "@ui5/cli": "^3.9.2"
    },
    "scripts": {
        "build": "ui5 build --exclude-task generateFlexChangesBundle generateComponentPreload --clean-dest",


Could you please add minify to the excluded tasks? Otherwise, .map files will be generated that are rejected by LREP on your ABAP versions

tobiasqueck · 2024-05-08T07:10:51Z

packages/adp-tooling/src/writer/index.ts

    }
+    await writeUI5DeployYaml(basePath, fullConfig, fs);
+    await writeUI5Yaml(basePath, fullConfig, fs);
+    await writeEnvFile(basePath, fullConfig, fs);


Does the user know that we are writing user/password to the filesystem? If not, we cannot do that.
Btw, as far as I understand, both user/password are only used for the additional tasks for S/4HANA Cloud Public Edition scenarios, however, such systems won't allow access with user/password, therefore, I'd strongly prefer to remove this user/password/env thing.

As discussed, please remove it.

tobiasqueck · 2024-05-08T07:13:03Z

packages/adp-tooling/src/writer/options.ts

+ * @returns list of required tasks.
+ */
+function getAdpCloudCustomTasks(config: AdpWriterConfig & { target: AbapTarget }): CustomTask[] {
+    const user = 'env:ABAP_USERNAME';


As mentioned in my other comment, this shouldn't be here. In the very worst case that there is a valid scenario for this, use at least FIORI_TOOLS_USER and FIORI_TOOLS_PASSWORD because they are available if any previous Fiori tools task or middleware relied on user/password.

As discussed on the phone, please remove it.

tobiasqueck · 2024-05-08T07:18:52Z

packages/adp-tooling/src/types.ts

    };
    package?: {
        name?: string;
        description?: string;
    };
+    flp?: {


That object is odd. bspName and credentials are used to access the original app while title, subtitle and inboundId are only being used in the i18n file. Did you miss adding a change to the appdescr if these properties are present or did I miss it in the code or is there some magic backend functionality that does something on deploy?

What is the difference between bspName and reference in the app section?

Currently, the title, subtitle and inbound id are only used in the i18n file because the appdescr information comes already filtered after the prompting.
bspName is the base app repoName and reference is the base app id

As discussed on the phone, please add the change for the inbound communication to the appdescr template file with the condtion that the flp object is provided

The change for the inbound it's more complex to be in the aprdescr template and because of that it's passed with other changes in the content

If it is too complex for ejs, implement it as a plain function and then add it to the list of changes. Most importantly move the logic here because you have a reference to it in your generated i18n.properties file

tobiasqueck · 2024-05-08T07:23:03Z

packages/adp-tooling/src/types.ts

+/**
+ * Enumerates the types of projects that can be made, each representing a specific kind of project structure.
+ */
+export const enum ProjectType {


Both are S/4 systems here, so please change it to something like Cloud and OnPremise. Even better would be if you reuse the type OperationsType (https://github.com/SAP/open-ux-tools/blob/main/packages/axios-extension/src/abap/types/adt-types.ts#L77) because that is exactly what you will get back from the backend as identify during the prompting phase.

packages/adp-tooling/src/types.ts

…meworkUrl, use operationTypes

tobiasqueck · 2024-05-17T11:27:17Z

packages/adp-tooling/templates/project/webapp/manifest.appdescr_variant

@@ -7,6 +7,9 @@
  "namespace": "apps/<%- app.reference %>/appVariants/<%- app.id %>/",
  "version": "0.1.0",
  "content": [
+<%if(app.content){-%><% for(let change of app.content){-%>
+    <%- JSON.stringify(change) %><%-","%>


It might be nice to align the spacing of your JSON with the rest of the document e.g. JSON.stringify(change, undefined, 4)

IvoSG added 2 commits March 24, 2024 13:20

feat: enhance writer for ADP Project to be able to create proper ADP …

d207584

…Projects

feat: enhance s4hana project generation

bb4a0a0

IvoSG requested review from GDamyanov and nikmace March 25, 2024 11:32

IvoSG and others added 7 commits April 2, 2024 17:03

feat: enhance cf project generation

363a702

Merge branch 'main' of https://github.com/SAP/open-ux-tools into feat…

2cffde0

…/1611/writersForAdpProjects

fix/lint errors and tests

774f0ac

fix: unit tests and code corrections

a9ae35a

Linting auto fix commit

ef58dd5

chore: add changeset

9aa7edc

Merge branch 'feat/1611/writersForAdpProjects' of https://github.com/…

0d0bef9

…SAP/open-ux-tools into feat/1611/writersForAdpProjects

IvoSG added adp-tooling feature New feature or request preview-middleware @sap-ux/preview-middleware labels Apr 3, 2024

IvoSG changed the title ~~Feat/1611/writers for adp projects~~ feat: Switch Adaptation Project's generator/wizard to use Open Source writers Apr 3, 2024

IvoSG marked this pull request as ready for review April 3, 2024 06:22

IvoSG requested a review from a team as a code owner April 3, 2024 06:22

IvoSG and others added 2 commits April 3, 2024 14:15

fix: sonar security issue

464b534

Merge branch 'main' into feat/1611/writersForAdpProjects

5ad6495

nikmace requested changes Apr 4, 2024

View reviewed changes

nikmace requested a review from testojs April 4, 2024 07:18

nikmace requested changes Apr 8, 2024

View reviewed changes

packages/adp-tooling/src/types.ts Outdated Show resolved Hide resolved

packages/adp-tooling/src/types.ts Outdated Show resolved Hide resolved

packages/adp-tooling/templates/projects/abap/package.json Outdated Show resolved Hide resolved

IvoSG added 4 commits April 16, 2024 05:26

chore: split the project template files for the different environments

82b0238

chore: use btp-utils to check the running environment

32a3214

chore: split the writers logic

5956a34

chore: add writer factory to instantiate class for different environm…

8a25655

…ents and export only one generate method

IvoSG requested a review from a team as a code owner April 16, 2024 05:37

nikmace requested changes Apr 16, 2024

View reviewed changes

tobiasqueck requested changes Apr 17, 2024

View reviewed changes

IvoSG and others added 9 commits April 24, 2024 11:10

fix: remove cf writer logic, move project utils and fix ui5-deploy.ya…

5d6d4cb

…ml content generation

chore: use one project template for onprem and s4 project types

004b5cb

fix: ui5-deploy.yaml writer and add appredload middleware

f28ca6e

chore: revert all writers factory logic, revert project template stru…

43ca69a

…cture, adapt commands, remove unused dependencie

Merge branch 'main' into feat/1611/writersForAdpProjects

4af85b2

chore: revert autoupdated import type

8ac89f4

fix: add whitespace

09d5cf4

fix: merge issue

d42387b

Merge branch 'main' into feat/1611/writersForAdpProjects

0e31e07

nikmace requested changes Apr 30, 2024

View reviewed changes

packages/adp-tooling/test/unit/writer/index.test.ts Outdated Show resolved Hide resolved

packages/adp-tooling/src/types.ts Outdated Show resolved Hide resolved

packages/adp-tooling/src/types.ts Outdated Show resolved Hide resolved

IvoSG added 2 commits April 30, 2024 08:50

chore: change types and remove unit tests for cf

2507b01

fix: default url if not provided

cb495b2

tobiasqueck reviewed May 8, 2024

View reviewed changes

tobiasqueck requested changes May 8, 2024

View reviewed changes

tobiasqueck reviewed May 8, 2024

View reviewed changes

packages/adp-tooling/src/types.ts Show resolved Hide resolved

tobiasqueck mentioned this pull request May 8, 2024

feat(adp-tooling): add migration adaptation template writer #1903

Merged

tobiasqueck reviewed May 13, 2024

View reviewed changes

packages/adp-tooling/src/types.ts Outdated Show resolved Hide resolved

IvoSG and others added 3 commits May 16, 2024 06:28

fix: remove .env file, write only changes in apdescr content, use fra…

0ca6004

…meworkUrl, use operationTypes

fix: typo

3dba791

Merge branch 'main' into feat/1611/writersForAdpProjects

6c21e2f

IvoSG requested a review from tobiasqueck May 16, 2024 12:15

IvoSG and others added 2 commits May 17, 2024 09:43

Merge branch 'main' into feat/1611/writersForAdpProjects

0516fc4

fix: migrator ui5 yaml writers

35a6347

tobiasqueck reviewed May 17, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Switch Adaptation Project's generator/wizard to use Open Source writers #1775

feat: Switch Adaptation Project's generator/wizard to use Open Source writers #1775

IvoSG commented Mar 25, 2024 •

edited

changeset-bot bot commented Mar 25, 2024 •

edited

sonarcloud bot commented Apr 3, 2024

nikmace left a comment

nikmace Apr 4, 2024

nikmace Apr 4, 2024

nikmace left a comment

nikmace Apr 16, 2024

nikmace Apr 16, 2024

tobiasqueck May 8, 2024

nikmace Apr 16, 2024

tobiasqueck left a comment

tobiasqueck May 8, 2024 •

edited

IvoSG May 16, 2024

tobiasqueck May 8, 2024

tobiasqueck May 13, 2024

IvoSG May 16, 2024

tobiasqueck May 8, 2024

tobiasqueck May 13, 2024

tobiasqueck May 8, 2024

IvoSG May 13, 2024 •

edited

tobiasqueck May 13, 2024

IvoSG May 16, 2024

tobiasqueck May 17, 2024

tobiasqueck May 8, 2024

IvoSG May 16, 2024

tobiasqueck May 17, 2024

		const appdescrTplPath = join(templatePath, 'webapp/manifest.appdescr_variant');
		const appdescrPath = join(projectPath, 'webapp/manifest.appdescr_variant');

feat: Switch Adaptation Project's generator/wizard to use Open Source writers #1775

Are you sure you want to change the base?

feat: Switch Adaptation Project's generator/wizard to use Open Source writers #1775

Conversation

IvoSG commented Mar 25, 2024 • edited

changeset-bot bot commented Mar 25, 2024 • edited

🦋 Changeset detected

sonarcloud bot commented Apr 3, 2024

Quality Gate passed

nikmace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikmace left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tobiasqueck left a comment

Choose a reason for hiding this comment

tobiasqueck May 8, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvoSG May 13, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IvoSG commented Mar 25, 2024 •

edited

changeset-bot bot commented Mar 25, 2024 •

edited

tobiasqueck May 8, 2024 •

edited

IvoSG May 13, 2024 •

edited