diff --git a/.changeset/two-lions-explode.md b/.changeset/two-lions-explode.md new file mode 100644 index 0000000000..21dff06e35 --- /dev/null +++ b/.changeset/two-lions-explode.md @@ -0,0 +1,5 @@ +--- +'@sap-ux/fiori-annotation-api': patch +--- + +fix: return null value for expression type null diff --git a/packages/fiori-annotation-api/src/change-converter.ts b/packages/fiori-annotation-api/src/change-converter.ts index 4d8de5efb6..9206e19ff9 100644 --- a/packages/fiori-annotation-api/src/change-converter.ts +++ b/packages/fiori-annotation-api/src/change-converter.ts @@ -580,14 +580,7 @@ export class ChangeConverter { }; this.annotationFileChanges.push(internalChange); } else if (onlyChangeValue && node.name === valueType) { - // element notation - const internalChange: ReplaceElementContent = { - type: REPLACE_ELEMENT_CONTENT, - uri: file.uri, - pointer: pointer, - newValue: [createTextNode(newValue)] - }; - this.annotationFileChanges.push(internalChange); + this.addReplacementContent(node, file.uri, pointer, newValue); } else if (node.attributes[type]) { // attribute notation this.annotationFileChanges.push({ @@ -618,6 +611,19 @@ export class ChangeConverter { this.convertUpdateExpressionForAttrributeType(file.uri, targetName, valueType, content, pointer, newValue); } } + private addReplacementContent(node: Element, fileUri: string, pointer: string, newValue: string): void { + // skip updating value for node name that are already null. + if (node.name !== Edm.Null) { + // element notation + const internalChange: ReplaceElementContent = { + type: REPLACE_ELEMENT_CONTENT, + uri: fileUri, + pointer: pointer, + newValue: [createTextNode(newValue)] + }; + this.annotationFileChanges.push(internalChange); + } + } private convertUpdateExpressionForAttrributeType( fileUri: string, diff --git a/packages/fiori-annotation-api/test/unit/fiori-service.test.ts b/packages/fiori-annotation-api/test/unit/fiori-service.test.ts index c54cc8b138..c719f1ac32 100644 --- a/packages/fiori-annotation-api/test/unit/fiori-service.test.ts +++ b/packages/fiori-annotation-api/test/unit/fiori-service.test.ts @@ -87,8 +87,8 @@ interface EditTestCase> { getChanges: (files: T) => Change[]; fsEditor?: Editor; log?: boolean; + expectUpdate?: boolean; } - interface CustomTest> { (testCase: EditTestCase, timeout?: number): void; only: CustomTest; @@ -96,32 +96,40 @@ interface CustomTest> { } const createEditTestCase = (>(): CustomTest => { - const run = (testCase: EditTestCase, timeout?: number) => () => { - for (const model of testCase.projectTestModels) { - const { info, root, files, serviceName } = model; - const name = `${info.type} ${info.name} ${info.version}`; - if (SKIP_TARGETS.has(model)) { - test.skip(name, () => {}); - } else { - test( - name, - async () => { - const text = await testEdit( - root, - testCase.getInitialChanges ? testCase.getInitialChanges(files) : [], - testCase.getChanges(files), - serviceName, - testCase.fsEditor, - testCase.log - ); - - expect(text).toMatchSnapshot(); - }, - timeout - ); + const run = + ({ expectUpdate = true, ...testCase }: EditTestCase, timeout?: number) => + () => { + for (const model of testCase.projectTestModels) { + const { info, root, files, serviceName } = model; + const name = `${info.type} ${info.name} ${info.version}`; + if (SKIP_TARGETS.has(model)) { + test.skip(name, () => {}); + } else { + test( + name, + async () => { + const { changedText, totalChangedFiles } = await testEdit( + root, + testCase.getInitialChanges ? testCase.getInitialChanges(files) : [], + testCase.getChanges(files), + serviceName, + testCase.fsEditor, + testCase.log + ); + if (totalChangedFiles > 0) { + expect(changedText).toMatchSnapshot(); + } + + if (!expectUpdate) { + expect(changedText).toBe(''); + expect(totalChangedFiles).toEqual(0); + } + }, + timeout + ); + } } - } - }; + }; // eslint-disable-next-line @typescript-eslint/no-explicit-any const runner: any = (testCase: EditTestCase, timeout?: number): void => { describe(testCase.name, run(testCase, timeout)); @@ -142,7 +150,7 @@ async function testEdit( serviceName: string, fsEditor?: Editor, log?: boolean -): Promise { +): Promise<{ changedText: string; totalChangedFiles: number }> { const editor = fsEditor ?? (await createFsEditorForProject(root)); const project = await getProject(root); const service = await FioriAnnotationService.createService(project, serviceName, '', editor, { @@ -163,23 +171,30 @@ async function testEdit( } const changedFileUris = applyChanges(service, changes); - await service.save(); - - for (const uri of changedFileUris.values()) { - const path = pathFromUri(uri); - const original = await promises.readFile(path, { encoding: 'utf-8' }); - const afterInitialChanges = initialChangeCache.get(uri); - const textAfterEdit = editor.read(path); - if (log) { - console.log('Original: \n', original); - if (afterInitialChanges !== undefined) { - console.log('After initial changes: \n', afterInitialChanges); + const changedFile = await service.save(); + + if (changedFile.files > 0) { + for (const uri of changedFileUris.values()) { + const path = pathFromUri(uri); + const original = await promises.readFile(path, { encoding: 'utf-8' }); + const afterInitialChanges = initialChangeCache.get(uri); + const textAfterEdit = editor.read(path); + if (log) { + console.log('Original: \n', original); + if (afterInitialChanges !== undefined) { + console.log('After initial changes: \n', afterInitialChanges); + } + console.log('After test changes: \n', textAfterEdit); } - console.log('After test changes: \n', textAfterEdit); + return ( + { changedText: textAfterEdit, totalChangedFiles: changedFile.files } ?? { + changedText: '', + totalChangedFiles: changedFile.files + } + ); } - return textAfterEdit ?? ''; } - return ''; + return { changedText: '', totalChangedFiles: changedFile.files }; } function applyChanges(service: FioriAnnotationService, changes: Change[]): Set { @@ -769,7 +784,7 @@ describe('fiori annotation service', () => { ` ); fsEditor.write(path, updatedSchemaFile); - const text = await testEdit( + const { changedText: text } = await testEdit( root, [], [ @@ -1357,6 +1372,51 @@ describe('fiori annotation service', () => { } ] }); + createEditTestCase({ + name: 'no update expected when value is already null', + projectTestModels: TEST_TARGETS, + getInitialChanges: (files) => [ + { + kind: ChangeType.InsertAnnotation, + uri: files.annotations, + content: { + type: 'annotation', + target: targetName, + value: { + term: DATA_POINT, + record: { + propertyValues: [ + { + name: 'Value', + value: { + type: 'Null' + } + } + ] + } + } + } + } + ], + getChanges: (files) => [ + { + kind: ChangeType.Update, + reference: { + target: targetName, + term: DATA_POINT + }, + uri: files.annotations, + pointer: 'record/propertyValues/0/value', + content: { + type: 'expression', + value: { + type: 'Null' + } + } + } + ], + expectUpdate: false + }); createEditTestCase({ name: "update propertyValue value 'string' value", projectTestModels: TEST_TARGETS, @@ -2000,7 +2060,7 @@ describe('fiori annotation service', () => { }; `; fsEditor.write(path, testData); - const text = await testEdit( + const { changedText: text } = await testEdit( root, [], [ @@ -3024,7 +3084,7 @@ describe('fiori annotation service', () => { } };`; fsEditor.write(mdPath, mdTestData); - const text = await testEdit( + const { changedText: text } = await testEdit( root, [], [ @@ -3213,7 +3273,7 @@ describe('fiori annotation service', () => { }, );`; fsEditor.write(path, testData); - const text = await testEdit( + const { changedText: text } = await testEdit( root, [], [ @@ -3262,7 +3322,7 @@ describe('fiori annotation service', () => { } );`; fsEditor.write(path, testData); - const text = await testEdit( + const { changedText } = await testEdit( root, [], [ @@ -3299,7 +3359,7 @@ describe('fiori annotation service', () => { false ); - expect(text).toMatchSnapshot(); + expect(changedText).toMatchSnapshot(); }); test('delete common text and textArrangement', async () => { @@ -3366,7 +3426,7 @@ describe('fiori annotation service', () => { // UI.KPI.DataPoint.Responsible.nickname: record/0/record/1/record/0 // UI.KPI.DataPoint.Responsible.fn: record/0/record/1/record/1 fsEditor.write(path, testData); - const text = await testEdit( + const { changedText } = await testEdit( root, [], [ @@ -3385,7 +3445,7 @@ describe('fiori annotation service', () => { false ); - expect(text).toMatchSnapshot(); + expect(changedText).toMatchSnapshot(); }); }); });