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 <[email protected]>
nasa · Aug 17, 2023 · 89b4c6e · 89b4c6e
1 parent dedfd3b
commit 89b4c6e
Show file tree

Hide file tree

Showing 14 changed files with 319 additions and 77 deletions.
diff --git a/e2e/appActions.js b/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);
 
@@ -581,6 +581,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,
@@ -599,5 +614,6 @@ module.exports = {
   setTimeConductorBounds,
   setIndependentTimeConductorBounds,
   selectInspectorTab,
-  waitForPlotsToRender
+  waitForPlotsToRender,
+  renameObjectFromContextMenu
 };
diff --git a/e2e/tests/functional/recentObjects.e2e.spec.js b/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(
-    'Navigated objects show up in recents, object renames and deletions are reflected',
-    async ({ page }) => {
-      test.info().annotations.push({
-        type: 'issue',
-        description: 'https://github.com/nasa/openmct/issues/6818'
-      });
+  test('Navigated objects show up in recents, object renames and deletions are reflected', async ({
+    page
+  }) => {
+    test.info().annotations.push({
+      type: 'issue',
+      description: 'https://github.com/nasa/openmct/issues/6818'
+    });
 
-      // Verify that both created objects appear in the list and are in the correct order
-      await assertInitialRecentObjectsListState();
+    // Verify that both created objects appear in the list and are in the correct order
+    await assertInitialRecentObjectsListState();
 
-      // 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.waitForURL(`**/${folderA.uuid}?*`);
-      expect(recentObjectsList.getByRole('listitem').nth(0).getByText(folderA.name)).toBeTruthy();
+    // 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.waitForURL(`**/${folderA.uuid}?*`);
+    expect(recentObjectsList.getByRole('listitem').nth(0).getByText(folderA.name)).toBeTruthy();
 
-      // Rename
-      folderA.name = `${folderA.name}-NEW!`;
-      await page.locator('.l-browse-bar__object-name').fill('');
-      await page.locator('.l-browse-bar__object-name').fill(folderA.name);
-      await page.keyboard.press('Enter');
+    // Rename
+    folderA.name = `${folderA.name}-NEW!`;
+    await page.locator('.l-browse-bar__object-name').fill('');
+    await page.locator('.l-browse-bar__object-name').fill(folderA.name);
+    await page.keyboard.press('Enter');
 
-      // Verify rename has been applied in recent objects list item and objects paths
-      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
+    // Verify rename has been applied in recent objects list item and objects paths
+    expect(
       await page
-        .getByRole('treeitem', { name: new RegExp(folderA.name) })
+        .getByRole('navigation', {
+          name: clock.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();
-    }
-  );
+        .filter({
+          hasText: folderA.name
+        })
+        .count()
+    ).toBeGreaterThan(0);
+    expect(recentObjectsList.getByRole('listitem', { name: folderA.name })).toBeTruthy();
+
+    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) })
+      .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,

diff --git a/e2e/tests/functional/renaming.e2e.spec.js b/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);
+  });
+});
diff --git a/e2e/tests/functional/tree.e2e.spec.js b/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"]');
-}
diff --git a/src/api/forms/components/FormRow.vue b/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>

diff --git a/src/api/forms/components/controls/TextField.vue b/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>

diff --git a/src/plugins/inspectorViews/properties/DetailText.vue b/src/plugins/inspectorViews/properties/DetailText.vue
@@ -21,11 +21,11 @@
 -->
 
 <template>
-  <li class="c-inspect-properties__row">
-    <div class="c-inspect-properties__label">
+  <li class="c-inspect-properties__row" :aria-label="`${detail.name} inspector properties`">
+    <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>

diff --git a/src/plugins/inspectorViews/properties/Location.vue b/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);
+      });
     }
   }
 };

diff --git a/src/plugins/inspectorViews/properties/Properties.vue b/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 };
+      });
     }
   }
 };