Recent objects do not update when object names are changed (#6927)

* fix tree name issue

* add name to key, and name observers to recent objects

* no need to change key

* make more of app reactive to name changes

* fix browse bar and document title

* listen in properties for name changes

* add tests for renaming

* yeah spelling linter

* add semantic tags to forms and fixup tests

* change purpose

* actually delete the listener

* ensuring deletion

---------

Co-authored-by: Jesse Mazzella <ozyx@users.noreply.github.com>

e2e/appActions.js

@@ -188,7 +188,7 @@ async function createPlanFromJSON(page, { name, json, parent = 'mine' }) {
   await page.click(`li:text("Plan")`);
 
   // Modify the name input field of the domain object to accept 'name'
-  const nameInput = page.locator('form[name="mctForm"] .first input[type="text"]');
+  const nameInput = page.getByLabel('Title', { exact: true });
   await nameInput.fill('');
   await nameInput.fill(name);
 
@@ -640,6 +640,21 @@ async function getCanvasPixels(page, canvasSelector) {
   return getTelemValuePromise;
 }
 
+/**
+ * @param {import('@playwright/test').Page} page
+ * @param {string} myItemsFolderName
+ * @param {string} url
+ * @param {string} newName
+ */
+async function renameObjectFromContextMenu(page, url, newName) {
+  await openObjectTreeContextMenu(page, url);
+  await page.click('li:text("Edit Properties")');
+  const nameInput = page.getByLabel('Title', { exact: true });
+  await nameInput.fill('');
+  await nameInput.fill(newName);
+  await page.click('[aria-label="Save"]');
+}
+
 // eslint-disable-next-line no-undef
 module.exports = {
   createDomainObjectWithDefaults,
@@ -660,5 +675,6 @@ module.exports = {
   setTimeConductorBounds,
   setIndependentTimeConductorBounds,
   selectInspectorTab,
-  waitForPlotsToRender
+  waitForPlotsToRender,
+  renameObjectFromContextMenu
 };

e2e/tests/functional/recentObjects.e2e.spec.js

@@ -59,59 +59,57 @@ test.describe('Recent Objects', () => {
     await page.mouse.move(0, 100);
     await page.mouse.up();
   });
-  test.fixme(
+  test('Navigated objects show up in recents, object renames and deletions are reflected', async ({
-    'Navigated objects show up in recents, object renames and deletions are reflected',
+    page
-    async ({ page }) => {
+  }) => {
-      test.info().annotations.push({
+    test.info().annotations.push({
-        type: 'issue',
+      type: 'issue',
-        description: 'https://github.com/nasa/openmct/issues/6818'
+      description: 'https://github.com/nasa/openmct/issues/6818'
-      });
+    });
 
-      // Verify that both created objects appear in the list and are in the correct order
+    // Verify that both created objects appear in the list and are in the correct order
-      await assertInitialRecentObjectsListState();
+    await assertInitialRecentObjectsListState();
 
-      // Navigate to the folder by clicking on the main object name in the recent objects list item
+    // Navigate to the folder by clicking on the main object name in the recent objects list item
-      await page.getByRole('listitem', { name: folderA.name }).getByText(folderA.name).click();
+    await page.getByRole('listitem', { name: folderA.name }).getByText(folderA.name).click();
-      await page.waitForURL(`**/${folderA.uuid}?*`);
+    await page.waitForURL(`**/${folderA.uuid}?*`);
-      expect(recentObjectsList.getByRole('listitem').nth(0).getByText(folderA.name)).toBeTruthy();
+    expect(recentObjectsList.getByRole('listitem').nth(0).getByText(folderA.name)).toBeTruthy();
 
-      // Rename
+    // Rename
-      folderA.name = `${folderA.name}-NEW!`;
+    folderA.name = `${folderA.name}-NEW!`;
-      await page.locator('.l-browse-bar__object-name').fill('');
+    await page.locator('.l-browse-bar__object-name').fill('');
-      await page.locator('.l-browse-bar__object-name').fill(folderA.name);
+    await page.locator('.l-browse-bar__object-name').fill(folderA.name);
-      await page.keyboard.press('Enter');
+    await page.keyboard.press('Enter');
 
-      // Verify rename has been applied in recent objects list item and objects paths
+    // Verify rename has been applied in recent objects list item and objects paths
-      expect(
+    expect(
-        await page
-          .getByRole('navigation', {
-            name: clock.name
-          })
-          .locator('a')
-          .filter({
-            hasText: folderA.name
-          })
-          .count()
-      ).toBeGreaterThan(0);
-      expect(recentObjectsList.getByRole('listitem', { name: folderA.name })).toBeTruthy();
-
-      // Delete
-      await page.click('button[title="Show selected item in tree"]');
-      // Delete the folder via the left tree pane treeitem context menu
       await page
-        .getByRole('treeitem', { name: new RegExp(folderA.name) })
+        .getByRole('navigation', {
+          name: clock.name
+        })
         .locator('a')
-        .click({
+        .filter({
-          button: 'right'
+          hasText: folderA.name
-        });
+        })
-      await page.getByRole('menuitem', { name: /Remove/ }).click();
+        .count()
-      await page.getByRole('button', { name: 'OK' }).click();
+    ).toBeGreaterThan(0);
+    expect(recentObjectsList.getByRole('listitem', { name: folderA.name })).toBeTruthy();
 
-      // Verify that the folder and clock are no longer in the recent objects list
+    await page.click('button[title="Show selected item in tree"]');
-      await expect(recentObjectsList.getByRole('listitem', { name: folderA.name })).toBeHidden();
+    // Delete the folder via the left tree pane treeitem context menu
-      await expect(recentObjectsList.getByRole('listitem', { name: clock.name })).toBeHidden();
+    await page
-    }
+      .getByRole('treeitem', { name: new RegExp(folderA.name) })
-  );
+      .locator('a')
+      .click({
+        button: 'right'
+      });
+    await page.getByRole('menuitem', { name: /Remove/ }).click();
+    await page.getByRole('button', { name: 'OK' }).click();
+
+    // Verify that the folder and clock are no longer in the recent objects list
+    await expect(recentObjectsList.getByRole('listitem', { name: folderA.name })).toBeHidden();
+    await expect(recentObjectsList.getByRole('listitem', { name: clock.name })).toBeHidden();
+  });
 
   test('Clicking on an object in the path of a recent object navigates to the object', async ({
     page,

e2e/tests/functional/renaming.e2e.spec.js

@@ -0,0 +1,78 @@
+/*****************************************************************************
+ * Open MCT, Copyright (c) 2014-2023, United States Government
+ * as represented by the Administrator of the National Aeronautics and Space
+ * Administration. All rights reserved.
+ *
+ * Open MCT is licensed under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ * http://www.apache.org/licenses/LICENSE-2.0.
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ * Open MCT includes source code licensed under additional open source
+ * licenses. See the Open Source Licenses file (LICENSES.md) included with
+ * this source code distribution or the Licensing information page available
+ * at runtime from the About dialog for additional information.
+ *****************************************************************************/
+
+/*
+This test suite is dedicated to tests for renaming objects, and their global application effects.
+*/
+
+const { test, expect } = require('../../baseFixtures.js');
+const {
+  createDomainObjectWithDefaults,
+  renameObjectFromContextMenu
+} = require('../../appActions.js');
+
+test.describe('Renaming objects', () => {
+  test.beforeEach(async ({ page }) => {
+    // Go to baseURL
+    await page.goto('./', { waitUntil: 'networkidle' });
+  });
+
+  test('When renaming objects, the browse bar and various components all update', async ({
+    page
+  }) => {
+    const folder = await createDomainObjectWithDefaults(page, {
+      type: 'Folder'
+    });
+    // Create a new 'Clock' object with default settings
+    const clock = await createDomainObjectWithDefaults(page, {
+      type: 'Clock',
+      parent: folder.uuid
+    });
+
+    // Rename
+    clock.name = `${clock.name}-NEW!`;
+    await renameObjectFromContextMenu(page, clock.url, clock.name);
+    // check inspector for new name
+    const titleValue = await page
+      .getByLabel('Title inspector properties')
+      .getByLabel('inspector property value')
+      .textContent();
+    expect(titleValue).toBe(clock.name);
+    // check browse bar for new name
+    await expect(page.locator(`.l-browse-bar >> text=${clock.name}`)).toBeVisible();
+    // check tree item for new name
+    await expect(
+      page.getByRole('listitem', {
+        name: clock.name
+      })
+    ).toBeVisible();
+    // check recent objects for new name
+    await expect(
+      page.getByRole('navigation', {
+        name: clock.name
+      })
+    ).toBeVisible();
+    // check title for new name
+    const title = await page.title();
+    expect(title).toBe(clock.name);
+  });
+});

e2e/tests/functional/tree.e2e.spec.js

@@ -23,7 +23,7 @@
 const { test, expect } = require('../../pluginFixtures.js');
 const {
   createDomainObjectWithDefaults,
-  openObjectTreeContextMenu
+  renameObjectFromContextMenu
 } = require('../../appActions.js');
 
 test.describe('Main Tree', () => {
@@ -249,18 +249,3 @@ async function expandTreePaneItemByName(page, name) {
   });
   await treeItem.locator('.c-disclosure-triangle').click();
 }
-
-/**
- * @param {import('@playwright/test').Page} page
- * @param {string} myItemsFolderName
- * @param {string} url
- * @param {string} newName
- */
-async function renameObjectFromContextMenu(page, url, newName) {
-  await openObjectTreeContextMenu(page, url);
-  await page.click('li:text("Edit Properties")');
-  const nameInput = page.locator('form[name="mctForm"] .first input[type="text"]');
-  await nameInput.fill('');
-  await nameInput.fill(newName);
-  await page.click('[aria-label="Save"]');
-}

src/api/forms/components/FormRow.vue

@@ -22,9 +22,9 @@
 
 <template>
   <div class="form-row c-form__row" :class="[{ first: first }, cssClass]" @onChange="onChange">
-    <div class="c-form-row__label" :title="row.description">
+    <label class="c-form-row__label" :title="row.description" :for="`form-${row.key}`">
       {{ row.name }}
-    </div>
+    </label>
     <div class="c-form-row__state-indicator" :class="reqClass"></div>
     <div v-if="row.control" ref="rowElement" class="c-form-row__controls"></div>
   </div>

src/api/forms/components/controls/TextField.vue

@@ -23,7 +23,14 @@
 <template>
   <span class="form-control shell">
     <span class="field control" :class="model.cssClass">
-      <input v-model="field" type="text" :size="model.size" @input="updateText()" />
+      <input
+        :id="`form-${model.key}`"
+        v-model="field"
+        :name="model.key"
+        type="text"
+        :size="model.size"
+        @input="updateText()"
+      />
     </span>
   </span>
 </template>

src/plugins/inspectorViews/properties/DetailText.vue

@@ -21,11 +21,11 @@
 -->
 
 <template>
-  <li class="c-inspect-properties__row">
+  <li class="c-inspect-properties__row" :aria-label="`${detail.name} inspector properties`">
-    <div class="c-inspect-properties__label">
+    <div class="c-inspect-properties__label" aria-label="inspector property name">
       {{ detail.name }}
     </div>
-    <div class="c-inspect-properties__value">
+    <div class="c-inspect-properties__value" aria-label="inspector property value">
       {{ detail.value }}
     </div>
   </li>

src/plugins/inspectorViews/properties/Location.vue

@@ -73,9 +73,43 @@ export default {
     }
   },
   async mounted() {
+    this.nameChangeListeners = {};
     await this.createPathBreadCrumb();
   },
+  unmounted() {
+    Object.values(this.nameChangeListeners).forEach((unlisten) => {
+      unlisten();
+    });
+  },
   methods: {
+    updateObjectPathName(keyString, newName) {
+      this.pathBreadCrumb = this.pathBreadCrumb.map((pathObject) => {
+        if (this.openmct.objects.makeKeyString(pathObject.domainObject.identifier) === keyString) {
+          return {
+            ...pathObject,
+            domainObject: { ...pathObject.domainObject, name: newName }
+          };
+        }
+        return pathObject;
+      });
+    },
+    removeNameListenerFor(domainObject) {
+      const keyString = this.openmct.objects.makeKeyString(domainObject.identifier);
+      if (this.nameChangeListeners[keyString]) {
+        this.nameChangeListeners[keyString]();
+        delete this.nameChangeListeners[keyString];
+      }
+    },
+    addNameListenerFor(domainObject) {
+      const keyString = this.openmct.objects.makeKeyString(domainObject.identifier);
+      if (!this.nameChangeListeners[keyString]) {
+        this.nameChangeListeners[keyString] = this.openmct.objects.observe(
+          domainObject,
+          'name',
+          this.updateObjectPathName.bind(this, keyString)
+        );
+      }
+    },
     async createPathBreadCrumb() {
       if (!this.domainObject && this.parentDomainObject) {
         this.setPathBreadCrumb([this.parentDomainObject]);
@@ -98,7 +132,15 @@ export default {
         };
       });
 
+      this.pathBreadCrumb.forEach((pathObject) => {
+        this.removeNameListenerFor(pathObject.domainObject);
+      });
+
       this.pathBreadCrumb = pathBreadCrumb;
+
+      this.pathBreadCrumb.forEach((pathObject) => {
+        this.addNameListenerFor(pathObject.domainObject);
+      });
     }
   }
 };

src/plugins/inspectorViews/properties/Properties.vue

@@ -230,7 +230,22 @@ export default {
       return `detail-${component}`;
     },
     updateSelection(selection) {
+      this.removeListener();
       this.selection.splice(0, this.selection.length, ...selection);
+      if (this.domainObject) {
+        this.addListener();
+      }
+    },
+    removeListener() {
+      if (this.nameListener) {
+        this.nameListener();
+        this.nameListener = null;
+      }
+    },
+    addListener() {
+      this.nameListener = this.openmct.objects.observe(this.context?.item, 'name', (newValue) => {
+        this.context.item = { ...this.context?.item, name: newValue };
+      });
     }
   }
 };

src/ui/components/ObjectPath.vue

@@ -78,6 +78,7 @@ export default {
     };
   },
   async mounted() {
+    this.nameChangeListeners = {};
     const keyString = this.openmct.objects.makeKeyString(this.domainObject.identifier);
 
     if (keyString && this.keyString !== keyString) {
@@ -108,8 +109,16 @@ export default {
         // remove ROOT and object itself from path
         this.orderedPath = pathWithDomainObject.slice(1, pathWithDomainObject.length - 1).reverse();
       }
+      this.orderedPath.forEach((pathObject) => {
+        this.addNameListenerFor(pathObject.domainObject);
+      });
     }
   },
+  unmounted() {
+    Object.values(this.nameChangeListeners).forEach((unlisten) => {
+      unlisten();
+    });
+  },
   methods: {
     /**
      * Generate the hash url for the given object path, removing the '/ROOT' prefix if present.
@@ -120,6 +129,34 @@ export default {
       const path = `/browse/${this.openmct.objects.getRelativePath(objectPath)}`;
 
       return path.replace('ROOT/', '');
+    },
+    updateObjectPathName(keyString, newName) {
+      this.orderedPath = this.orderedPath.map((pathObject) => {
+        if (this.openmct.objects.makeKeyString(pathObject.domainObject.identifier) === keyString) {
+          return {
+            ...pathObject,
+            domainObject: { ...pathObject.domainObject, name: newName }
+          };
+        }
+        return pathObject;
+      });
+    },
+    removeNameListenerFor(domainObject) {
+      const keyString = this.openmct.objects.makeKeyString(domainObject.identifier);
+      if (this.nameChangeListeners[keyString]) {
+        this.nameChangeListeners[keyString]();
+        delete this.nameChangeListeners[keyString];
+      }
+    },
+    addNameListenerFor(domainObject) {
+      const keyString = this.openmct.objects.makeKeyString(domainObject.identifier);
+      if (!this.nameChangeListeners[keyString]) {
+        this.nameChangeListeners[keyString] = this.openmct.objects.observe(
+          domainObject,
+          'name',
+          this.updateObjectPathName.bind(this, keyString)
+        );
+      }
     }
   }
 };

src/ui/inspector/ObjectName.vue

@@ -104,12 +104,19 @@ export default {
     if (this.statusUnsubscribe) {
       this.statusUnsubscribe();
     }
+    if (this.nameUnsubscribe) {
+      this.nameUnsubscribe();
+    }
   },
   methods: {
     updateSelection(selection) {
       if (this.statusUnsubscribe) {
         this.statusUnsubscribe();
-        this.statusUnsubscribe = undefined;
+        this.statusUnsubscribe = null;
+      }
+      if (this.nameUnsubscribe) {
+        this.nameUnsubscribe();
+        this.nameUnsubscribe = null;
       }
 
       if (selection.length === 0 || selection[0].length === 0) {
@@ -132,6 +139,11 @@ export default {
           this.keyString = this.openmct.objects.makeKeyString(this.domainObject.identifier);
           this.status = this.openmct.status.get(this.keyString);
           this.statusUnsubscribe = this.openmct.status.observe(this.keyString, this.updateStatus);
+          this.nameUnsubscribe = this.openmct.objects.observe(
+            this.domainObject,
+            'name',
+            this.updateName
+          );
         } else if (selection[0][0].context.layoutItem) {
           this.layoutItem = selection[0][0].context.layoutItem;
         }
@@ -144,6 +156,9 @@ export default {
     },
     updateStatus(status) {
       this.status = status;
+    },
+    updateName(newName) {
+      this.domainObject = { ...this.domainObject, name: newName };
     }
   }
 };

src/ui/layout/RecentObjectsList.vue

@@ -59,13 +59,47 @@ export default {
   },
   mounted() {
     this.compositionCollections = {};
+    this.nameChangeListeners = {};
     this.openmct.router.on('change:path', this.onPathChange);
     this.getSavedRecentItems();
   },
   unmounted() {
     this.openmct.router.off('change:path', this.onPathChange);
+    Object.values(this.nameChangeListeners).forEach((unlisten) => {
+      unlisten();
+    });
   },
   methods: {
+    addNameListenerFor(domainObject) {
+      const keyString = this.openmct.objects.makeKeyString(domainObject.identifier);
+      if (!this.nameChangeListeners[keyString]) {
+        this.nameChangeListeners[keyString] = this.openmct.objects.observe(
+          domainObject,
+          'name',
+          this.updateRecentObjectName.bind(this, keyString)
+        );
+      }
+    },
+    updateRecentObjectName(keyString, newName) {
+      this.recents = this.recents.map((recentObject) => {
+        if (
+          this.openmct.objects.makeKeyString(recentObject.domainObject.identifier) === keyString
+        ) {
+          return {
+            ...recentObject,
+            domainObject: { ...recentObject.domainObject, name: newName }
+          };
+        }
+        return recentObject;
+      });
+    },
+    removeNameListenerFor(domainObject) {
+      const keyString = this.openmct.objects.makeKeyString(domainObject.identifier);
+      if (this.nameChangeListeners[keyString]) {
+        this.nameChangeListeners[keyString]();
+        delete this.nameChangeListeners[keyString];
+      }
+    },
     /**
      * Add a composition collection to the map and register its remove handler
      * @param {string} navigationPath
@@ -112,6 +146,7 @@ export default {
       // Get composition collections and add composition listeners for composable objects
       savedRecents.forEach((recentObject) => {
         const { domainObject, navigationPath } = recentObject;
+        this.addNameListenerFor(domainObject);
         if (this.shouldTrackCompositionFor(domainObject)) {
           this.compositionCollections[navigationPath] = {};
           this.compositionCollections[navigationPath].collection =
@@ -161,6 +196,8 @@ export default {
         return;
       }
 
+      this.addNameListenerFor(domainObject);
+
       // Move the object to the top if its already existing in the recents list
       const existingIndex = this.recents.findIndex((recentObject) => {
         return navigationPath === recentObject.navigationPath;
@@ -179,6 +216,7 @@ export default {
       while (this.recents.length > MAX_RECENT_ITEMS) {
         const poppedRecentItem = this.recents.pop();
         this.removeCompositionListenerFor(poppedRecentItem.navigationPath);
+        this.removeNameListenerFor(poppedRecentItem.domainObject);
       }
 
       this.setSavedRecentItems();
@@ -236,6 +274,9 @@ export default {
             label: 'OK',
             callback: () => {
               localStorage.removeItem(LOCAL_STORAGE_KEY__RECENT_OBJECTS);
+              Object.values(this.nameChangeListeners).forEach((unlisten) => {
+                unlisten();
+              });
               this.recents = [];
               dialog.dismiss();
               this.$emit('setClearButtonDisabled', true);

src/ui/layout/mct-tree.vue

@@ -83,7 +83,7 @@
         <div :style="childrenHeightStyles">
           <tree-item
             v-for="(treeItem, index) in visibleItems"
-            :key="`${treeItem.navigationPath}-${index}`"
+            :key="`${treeItem.navigationPath}-${index}-${treeItem.object.name}`"
             :node="treeItem"
             :is-selector-tree="isSelectorTree"
             :selected-item="selectedItem"

src/ui/router/Browse.js

@@ -37,9 +37,13 @@ define([], function () {
       openmct.layout.$refs.browseBar.viewKey = viewProvider.key;
     }
 
-    function updateDocumentTitleOnNameMutation(domainObject) {
+    function updateDocumentTitleOnNameMutation(newName) {
-      if (typeof domainObject.name === 'string' && domainObject.name !== document.title) {
+      if (typeof newName === 'string' && newName !== document.title) {
-        document.title = domainObject.name;
+        document.title = newName;
+        openmct.layout.$refs.browseBar.domainObject = {
+          ...openmct.layout.$refs.browseBar.domainObject,
+          name: newName
+        };
       }
     }
 
@@ -80,7 +84,11 @@ define([], function () {
         let currentProvider = openmct.objectViews.getByProviderKey(currentViewKey);
         document.title = browseObject.name; //change document title to current object in main view
         // assign listener to global for later clearing
-        unobserve = openmct.objects.observe(browseObject, '*', updateDocumentTitleOnNameMutation);
+        unobserve = openmct.objects.observe(
+          browseObject,
+          'name',
+          updateDocumentTitleOnNameMutation
+        );
 
         if (currentProvider && currentProvider.canView(browseObject, openmct.router.path)) {
           viewObject(browseObject, currentProvider);