fix: fileName property in the change file's content of the manifest…

… change editors writers differs from the name of the change file (#2898) * feat: change the fileName for every manifest editor change * test: adjust tests * chore: add cset * fix: change the range to be lower * refactor: remove three digits from being generated * fix: add check if change name is missing * refactor: add a change name map * feat: throw an error in case map returns undefined --------- Co-authored-by: GDamyanov <[email protected]>
SAP · Feb 19, 2025 · bb38bef · bb38bef
1 parent ef120e7
commit bb38bef
Show file tree

Hide file tree

Showing 9 changed files with 61 additions and 51 deletions.
diff --git a/.changeset/silly-beds-live.md b/.changeset/silly-beds-live.md
@@ -0,0 +1,5 @@
+---
+'@sap-ux/adp-tooling': patch
+---
+
+fix: Change fileName for the manifest change editors writers differs from the actual real file name
diff --git a/packages/adp-tooling/src/base/change-utils.ts b/packages/adp-tooling/src/base/change-utils.ts
@@ -11,7 +11,8 @@ import {
     type DescriptorVariant,
     type InboundContent,
     type ManifestChangeProperties,
-    type PropertyValueType
+    type PropertyValueType,
+    ChangeTypeMap
 } from '../types';
 import { renderFile } from 'ejs';
 
@@ -42,10 +43,10 @@ export function writeAnnotationChange(
     try {
         const changesFolderPath = path.join(projectPath, DirName.Webapp, DirName.Changes);
         const annotationsFolderPath = path.join(changesFolderPath, DirName.Annotations);
+
         if (change) {
-            const changeFileName = `id_${timestamp}_addAnnotationsToOData.change`;
+            const changeFileName = `${change.fileName}.change`;
             const changeFilePath = path.join(changesFolderPath, changeFileName);
-            change.fileName = `${change.fileName}_addAnnotationsToOData`;
             writeChangeToFile(changeFilePath, change, fs);
         }
 
@@ -83,25 +84,19 @@ export function writeAnnotationChange(
  *
  * @param {string} projectPath - The root path of the project.
  * @param {ManifestChangeProperties} change - The change data to be written to the file.
- * @param {string} fileName - The name of the file to write the change data to.
  * @param {Editor} fs - The `mem-fs-editor` instance used for file operations.
  * @param {string} [dir] - An optional subdirectory within the 'changes' directory where the file will be written.
  * @returns {void}
  */
-export function writeChangeToFolder(
-    projectPath: string,
-    change: ManifestChangeProperties,
-    fileName: string,
-    fs: Editor,
-    dir = ''
-): void {
+export function writeChangeToFolder(projectPath: string, change: ManifestChangeProperties, fs: Editor, dir = ''): void {
     try {
         let targetFolderPath = path.join(projectPath, DirName.Webapp, DirName.Changes);
 
         if (dir) {
             targetFolderPath = path.join(targetFolderPath, dir);
         }
 
+        const fileName = `${change.fileName}.change`;
         const filePath = path.join(targetFolderPath, fileName);
         writeChangeToFile(filePath, change, fs);
     } catch (e) {
@@ -271,8 +266,16 @@ export function getChange(
     content: object,
     changeType: ChangeType
 ): ManifestChangeProperties {
+    const changeName = ChangeTypeMap[changeType];
+
+    if (!changeName) {
+        throw new Error(`Could not extract the change name from the change type: ${changeType}`);
+    }
+
+    const fileName = `id_${timestamp}_${changeName}`;
+
     return {
-        fileName: `id_${timestamp}`,
+        fileName,
         namespace: path.posix.join(namespace, DirName.Changes),
         layer,
         fileType: 'change',

diff --git a/packages/adp-tooling/src/types.ts b/packages/adp-tooling/src/types.ts
@@ -351,6 +351,18 @@ export const enum ChangeType {
     CHANGE_INBOUND = 'appdescr_app_changeInbound'
 }
 
+/**
+ * A mapping of ChangeType values to their respective change names.
+ */
+export const ChangeTypeMap: Record<ChangeType, string> = {
+    [ChangeType.ADD_NEW_MODEL]: 'addNewModel',
+    [ChangeType.ADD_ANNOTATIONS_TO_ODATA]: 'addAnnotationsToOData',
+    [ChangeType.CHANGE_DATA_SOURCE]: 'changeDataSource',
+    [ChangeType.ADD_COMPONENT_USAGES]: 'addComponentUsages',
+    [ChangeType.ADD_LIBRARY_REFERENCE]: 'addLibraries',
+    [ChangeType.CHANGE_INBOUND]: 'changeInbound'
+} as const;
+
 /**
  * Maps a ChangeType to the corresponding data structure needed for that type of change.
  * This conditional type ensures type safety by linking each change type with its relevant data model.

diff --git a/packages/adp-tooling/src/writer/changes/writers/component-usages-writer.ts b/packages/adp-tooling/src/writer/changes/writers/component-usages-writer.ts
@@ -68,7 +68,7 @@ export class ComponentUsagesWriter implements IWriter<ComponentUsagesData> {
             ChangeType.ADD_COMPONENT_USAGES
         );
 
-        writeChangeToFolder(this.projectPath, compUsagesChange, `id_${timestamp}_addComponentUsages.change`, this.fs);
+        writeChangeToFolder(this.projectPath, compUsagesChange, this.fs);
 
         if (!('library' in data)) {
             return;
@@ -78,6 +78,6 @@ export class ComponentUsagesWriter implements IWriter<ComponentUsagesData> {
         const libTimestamp = timestamp + 1;
         const refLibChange = getChange(data.variant, libTimestamp, libRefContent, ChangeType.ADD_LIBRARY_REFERENCE);
 
-        writeChangeToFolder(this.projectPath, refLibChange, `id_${libTimestamp}_addLibraries.change`, this.fs);
+        writeChangeToFolder(this.projectPath, refLibChange, this.fs);
     }
 }
diff --git a/packages/adp-tooling/src/writer/changes/writers/data-source-writer.ts b/packages/adp-tooling/src/writer/changes/writers/data-source-writer.ts
@@ -63,19 +63,14 @@ export class DataSourceWriter implements IWriter<DataSourceData> {
         const content = this.constructContent(id, uri, maxAge);
         const change = getChange(variant, timestamp, content, ChangeType.CHANGE_DATA_SOURCE);
 
-        writeChangeToFolder(this.projectPath, change, `id_${timestamp}_changeDataSource.change`, this.fs);
+        writeChangeToFolder(this.projectPath, change, this.fs);
 
         if (annotationId && annotationUri) {
             const annotationContent = this.constructContent(annotationId, annotationUri);
             const annotationTs = timestamp + 1;
             const annotationChange = getChange(variant, annotationTs, annotationContent, ChangeType.CHANGE_DATA_SOURCE);
 
-            writeChangeToFolder(
-                this.projectPath,
-                annotationChange,
-                `id_${annotationTs}_changeDataSource.change`,
-                this.fs
-            );
+            writeChangeToFolder(this.projectPath, annotationChange, this.fs);
         }
     }
 }
diff --git a/packages/adp-tooling/src/writer/changes/writers/inbound-writer.ts b/packages/adp-tooling/src/writer/changes/writers/inbound-writer.ts
@@ -84,7 +84,7 @@ export class InboundWriter implements IWriter<InboundData> {
             const content = this.constructContent(data);
             const change = getChange(data.variant, timestamp, content, ChangeType.CHANGE_INBOUND);
 
-            writeChangeToFolder(this.projectPath, change, `id_${timestamp}_changeInbound.change`, this.fs);
+            writeChangeToFolder(this.projectPath, change, this.fs);
         } else {
             if (changeWithInboundId.content) {
                 this.getEnhancedContent(data, changeWithInboundId.content);

diff --git a/packages/adp-tooling/src/writer/changes/writers/new-model-writer.ts b/packages/adp-tooling/src/writer/changes/writers/new-model-writer.ts
@@ -82,6 +82,6 @@ export class NewModelWriter implements IWriter<NewModelData> {
         const content = this.constructContent(data);
         const change = getChange(data.variant, timestamp, content, ChangeType.ADD_NEW_MODEL);
 
-        writeChangeToFolder(this.projectPath, change, `id_${timestamp}_addNewModel.change`, this.fs);
+        writeChangeToFolder(this.projectPath, change, this.fs);
     }
 }
diff --git a/packages/adp-tooling/test/unit/base/change-utils.test.ts b/packages/adp-tooling/test/unit/base/change-utils.test.ts
@@ -47,33 +47,30 @@ describe('Change Utils', () => {
         });
 
         const projectPath = 'project';
-        const change = { key: 'value' };
-        const fileName = 'something.change';
+        const change = { key: 'value', fileName: 'something' };
         const writeJsonSpy = jest.fn();
         const mockFs = { writeJSON: writeJsonSpy };
 
         it('should write change to the specified folder without subdirectory', () => {
             writeChangeToFolder(
                 projectPath,
                 change as unknown as ManifestChangeProperties,
-                fileName,
                 mockFs as unknown as Editor
             );
 
-            expect(writeJsonSpy).toHaveBeenCalledWith(expect.stringContaining(fileName), change);
+            expect(writeJsonSpy).toHaveBeenCalledWith(expect.stringContaining(change.fileName), change);
         });
 
         it('should write change to the specified folder with subdirectory', () => {
             const dir = 'subdir';
             writeChangeToFolder(
                 projectPath,
                 change as unknown as ManifestChangeProperties,
-                fileName,
                 mockFs as unknown as Editor,
                 dir
             );
 
-            expect(writeJsonSpy).toHaveBeenCalledWith(expect.stringContaining(path.join(dir, fileName)), change);
+            expect(writeJsonSpy).toHaveBeenCalledWith(expect.stringContaining(path.join(dir, change.fileName)), change);
         });
 
         it('should throw error when writing json fails', () => {
@@ -82,13 +79,12 @@ describe('Change Utils', () => {
                 throw new Error(errMsg);
             });
 
-            const expectedPath = path.join('project', 'webapp', 'changes', fileName);
+            const expectedPath = path.join('project', 'webapp', 'changes', `${change.fileName}.change`);
 
             expect(() => {
                 writeChangeToFolder(
                     projectPath,
                     change as unknown as ManifestChangeProperties,
-                    fileName,
                     mockFs as unknown as Editor
                 );
             }).toThrow(
@@ -147,6 +143,20 @@ describe('Change Utils', () => {
         };
         const mockContent = { key: 'value' };
 
+        it('should throw error when changeType is an empty string', () => {
+            const invalidChangeType = '' as unknown as ChangeType;
+            expect(() =>
+                getChange(mockData.projectData, mockData.timestamp, mockContent, invalidChangeType)
+            ).toThrowError(`Could not extract the change name from the change type: ${invalidChangeType}`);
+        });
+
+        it('should throw error when changeType is undefined', () => {
+            const invalidChangeType = undefined as unknown as ChangeType;
+            expect(() =>
+                getChange(mockData.projectData, mockData.timestamp, mockContent, invalidChangeType)
+            ).toThrowError(`Could not extract the change name from the change type: ${invalidChangeType}`);
+        });
+
         it('should return the correct change object structure', () => {
             const result = getChange(
                 mockData.projectData,
@@ -156,7 +166,7 @@ describe('Change Utils', () => {
             );
 
             expect(result).toEqual({
-                fileName: `id_${mockData.timestamp}`,
+                fileName: expect.stringContaining('_addAnnotationsToOData'),
                 namespace: `${mockData.projectData.namespace}/changes`,
                 layer: mockData.projectData.layer,
                 fileType: 'change',
@@ -328,7 +338,7 @@ describe('Change Utils', () => {
                 dataSource: '/sap/opu/odata/source'
             }
         } as AnnotationsData;
-        const mockChange = { key: 'value' };
+        const mockChange = { key: 'value', fileName: 'id_123456789_addAnnotationsToOData' };
         const writeJsonSpy = jest.fn();
         const writeSpy = jest.fn();
         const copySpy = jest.fn();

diff --git a/packages/adp-tooling/test/unit/writer/changes/writers/index.test.ts b/packages/adp-tooling/test/unit/writer/changes/writers/index.test.ts
@@ -206,12 +206,7 @@ describe('ComponentUsagesWriter', () => {
         jest.useRealTimers();
 
         expect(writeChangeToFolderMock).toHaveBeenCalledTimes(1);
-        expect(writeChangeToFolderMock).toHaveBeenCalledWith(
-            mockProjectPath,
-            expect.any(Object),
-            `id_${systemTime.getTime()}_addComponentUsages.change`,
-            {}
-        );
+        expect(writeChangeToFolderMock).toHaveBeenCalledWith(mockProjectPath, expect.any(Object), expect.any(Object));
     });
 });
 
@@ -275,12 +270,7 @@ describe('NewModelWriter', () => {
             ChangeType.ADD_NEW_MODEL
         );
 
-        expect(writeChangeToFolderMock).toHaveBeenCalledWith(
-            mockProjectPath,
-            expect.any(Object),
-            expect.stringContaining('_addNewModel.change'),
-            {}
-        );
+        expect(writeChangeToFolderMock).toHaveBeenCalledWith(mockProjectPath, expect.any(Object), expect.any(Object));
     });
 });
 
@@ -342,12 +332,7 @@ describe('DataSourceWriter', () => {
             expect.anything()
         );
 
-        expect(writeChangeToFolder).toHaveBeenCalledWith(
-            mockProjectPath,
-            expect.any(Object),
-            `id_${systemTime.getTime()}_changeDataSource.change`,
-            {}
-        );
+        expect(writeChangeToFolder).toHaveBeenCalledWith(mockProjectPath, expect.any(Object), expect.any(Object));
     });
 
     it('should add annotation change if annotationUri is provided', async () => {