From 53ee5e38ed873ee91196123826760d3c0c98b57b Mon Sep 17 00:00:00 2001
From: Nina Doschek <ndoschek@eclipsesource.com>
Date: Wed, 28 Feb 2024 15:23:40 +0100
Subject: [PATCH] Allow updates of CompositeMenuNode properties (#12948)
 (#13425)

- refactor updateOptions function to allow updates
- add unit test cases for updateOptions

Fixes #12948
---
 .../common/menu/composite-menu-node.spec.ts   | 67 +++++++++++++++++++
 .../src/common/menu/composite-menu-node.ts    | 10 +--
 2 files changed, 72 insertions(+), 5 deletions(-)
 create mode 100644 packages/core/src/common/menu/composite-menu-node.spec.ts

diff --git a/packages/core/src/common/menu/composite-menu-node.spec.ts b/packages/core/src/common/menu/composite-menu-node.spec.ts
new file mode 100644
index 0000000000000..24a002af1a526
--- /dev/null
+++ b/packages/core/src/common/menu/composite-menu-node.spec.ts
@@ -0,0 +1,67 @@
+// *****************************************************************************
+// Copyright (C) 2024 EclipseSource and others.
+//
+// This program and the accompanying materials are made available under the
+// terms of the Eclipse Public License v. 2.0 which is available at
+// http://www.eclipse.org/legal/epl-2.0.
+//
+// This Source Code may also be made available under the following Secondary
+// Licenses when the conditions for such availability set forth in the Eclipse
+// Public License v. 2.0 are satisfied: GNU General Public License, version 2
+// with the GNU Classpath Exception which is available at
+// https://www.gnu.org/software/classpath/license.html.
+//
+// SPDX-License-Identifier: EPL-2.0 OR GPL-2.0-only WITH Classpath-exception-2.0
+// *****************************************************************************
+import { expect } from 'chai';
+import { CompositeMenuNode } from './composite-menu-node';
+import { CompoundMenuNodeRole } from './menu-types';
+
+describe('composite-menu-node', () => {
+    describe('updateOptions', () => {
+        it('should update undefined node properties', () => {
+            const node = new CompositeMenuNode('test-id');
+            node.updateOptions({ label: 'node-label', icon: 'icon', order: 'a', role: CompoundMenuNodeRole.Flat, when: 'node-condition' });
+            expect(node.label).to.equal('node-label');
+            expect(node.icon).to.equal('icon');
+            expect(node.order).to.equal('a');
+            expect(node.role).to.equal(CompoundMenuNodeRole.Flat);
+            expect(node.when).to.equal('node-condition');
+        });
+        it('should update existing node properties', () => {
+            const node = new CompositeMenuNode('test-id', 'test-label', { icon: 'test-icon', order: 'a1', role: CompoundMenuNodeRole.Submenu, when: 'test-condition' });
+            node.updateOptions({ label: 'NEW-label', icon: 'NEW-icon', order: 'a2', role: CompoundMenuNodeRole.Flat, when: 'NEW-condition' });
+            expect(node.label).to.equal('NEW-label');
+            expect(node.icon).to.equal('NEW-icon');
+            expect(node.order).to.equal('a2');
+            expect(node.role).to.equal(CompoundMenuNodeRole.Flat);
+            expect(node.when).to.equal('NEW-condition');
+        });
+        it('should update only the icon without affecting other properties', () => {
+            const node = new CompositeMenuNode('test-id', 'test-label', { icon: 'test-icon', order: 'a' });
+            node.updateOptions({ icon: 'NEW-icon' });
+            expect(node.label).to.equal('test-label');
+            expect(node.icon).to.equal('NEW-icon');
+            expect(node.order).to.equal('a');
+        });
+        it('should not allow to unset properties', () => {
+            const node = new CompositeMenuNode('test-id', 'test-label', { icon: 'test-icon', order: 'a' });
+            node.updateOptions({ icon: undefined });
+            expect(node.label).to.equal('test-label');
+            expect(node.icon).to.equal('test-icon');
+            expect(node.order).to.equal('a');
+        });
+        it('should allow to set empty strings in properties', () => {
+            const node = new CompositeMenuNode('test-id', 'test-label');
+            node.updateOptions({ label: '' });
+            expect(node.label).to.equal('');
+        });
+        it('should not cause side effects when updating a property to its existing value', () => {
+            const node = new CompositeMenuNode('test-id', 'test-label', { icon: 'test-icon', order: 'a' });
+            node.updateOptions({ icon: 'test-icon' });
+            expect(node.label).to.equal('test-label');
+            expect(node.icon).to.equal('test-icon');
+            expect(node.order).to.equal('a');
+        });
+    });
+});
diff --git a/packages/core/src/common/menu/composite-menu-node.ts b/packages/core/src/common/menu/composite-menu-node.ts
index aad7208cf9c55..afc1819a3b058 100644
--- a/packages/core/src/common/menu/composite-menu-node.ts
+++ b/packages/core/src/common/menu/composite-menu-node.ts
@@ -63,11 +63,11 @@ export class CompositeMenuNode implements MutableCompoundMenuNode {
 
     updateOptions(options?: SubMenuOptions): void {
         if (options) {
-            this.iconClass ??= options.icon ?? options.iconClass;
-            this.label ??= options.label;
-            this.order ??= options.order;
-            this._role ??= options.role;
-            this._when ??= options.when;
+            this.iconClass = options.icon ?? options.iconClass ?? this.iconClass;
+            this.label = options.label ?? this.label;
+            this.order = options.order ?? this.order;
+            this._role = options.role ?? this._role;
+            this._when = options.when ?? this._when;
         }
     }