Repository: nasa/openmct (language: javascript)

Issue:
[Notebook] Changing notebook name in browse bar can cause any changes in notebook to disappear
<!--- Focus on user impact in the title. Use the Summary Field to -->
<!--- describe the problem technically. -->
VIPERGC-702

#### Summary
<!--- A description of the issue encountered. When possible, a description -->
<!--- of the impact of the issue. What use case does it impede?-->
Editing a notebook name in the browse bar and making changes, then editing it again will cause any changes to be erased.

#### Expected vs Current Behavior
<!--- Tell us what should have happened -->
No notebook changes should disappear without explicitly deleting them.

#### Steps to Reproduce
<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug. Include code to reproduce, if relevant -->
1. Create or visit an existing notebook
2. Edit the name of the notebook in the Browse Bar
3. Make ANY chnages to the notebook (add entries, add pages, sections, etc)
4. Edit the name of the notebook in the browse bar again
5. Notice all changes made have been reverted

#### Impact Check List
<!--- Please select from the following options -->
- [x] Data loss or misrepresented data?
- [ ] Regression? Did this used to work or has it always been broken?
- [x] Is there a workaround available?
- [x] Does this impact a critical component?
- [ ] Is this just a visual bug with no functional impact?
- [ ] Does this block the execution of e2e tests?
- [ ] Does this have an impact on Performance?

#### Additional Information
<!--- Include any screenshots, gifs, or logs which will expedite triage -->



Discussion / hints:

Fix patch:
diff --git a/src/ui/layout/BrowseBar.vue b/src/ui/layout/BrowseBar.vue
index 088f60e01c2..59dad57a316 100644
--- a/src/ui/layout/BrowseBar.vue
+++ b/src/ui/layout/BrowseBar.vue
@@ -20,7 +20,7 @@
  at runtime from the About dialog for additional information.
 -->
 <template>
-  <div class="l-browse-bar">
+  <div class="l-browse-bar" aria-label="Browse bar">
     <div class="l-browse-bar__start">
       <button
         v-if="hasParent"
@@ -35,6 +35,7 @@
         </div>
         <span
           ref="objectName"
+          aria-label="Browse bar object name"
           class="l-browse-bar__object-name c-object-label__name"
           :class="{ 'c-input-inline': isPersistable }"
           :contenteditable="isNameEditable"
diff --git a/src/ui/router/Browse.js b/src/ui/router/Browse.js
index 8a996ae1638..1d842519ba0 100644
--- a/src/ui/router/Browse.js
+++ b/src/ui/router/Browse.js
@@ -80,13 +80,11 @@ class Browse {
     this.#openmct.layout.$refs.browseBar.viewKey = viewProvider.key;
   }
 
-  #updateDocumentTitleOnNameMutation(newName) {
-    if (typeof newName === 'string' && newName !== document.title) {
-      document.title = newName;
-      this.#openmct.layout.$refs.browseBar.domainObject = {
-        ...this.#openmct.layout.$refs.browseBar.domainObject,
-        name: newName
-      };
+  #handleBrowseObjectUpdate(newObject) {
+    this.#openmct.layout.$refs.browseBar.domainObject = newObject;
+
+    if (typeof newObject.name === 'string' && newObject.name !== document.title) {
+      document.title = newObject.name;
     }
   }
 
@@ -120,8 +118,8 @@ class Browse {
     document.title = this.#browseObject.name; //change document title to current object in main view
     this.#unobserve = this.#openmct.objects.observe(
       this.#browseObject,
-      'name',
-      this.#updateDocumentTitleOnNameMutation.bind(this)
+      '*',
+      this.#handleBrowseObjectUpdate.bind(this)
     );
     const currentProvider = this.#openmct.objectViews.getByProviderKey(currentViewKey);
     if (currentProvider && currentProvider.canView(this.#browseObject, this.#openmct.router.path)) {


Test patch:
diff --git a/e2e/appActions.js b/e2e/appActions.js
index 99ee63cdb5d..7bb82726f20 100644
--- a/e2e/appActions.js
+++ b/e2e/appActions.js
@@ -682,6 +682,21 @@ async function linkParameterToObject(page, parameterName, objectName) {
   await page.getByLabel('Save').click();
 }
 
+/**
+ * Rename the currently viewed `domainObject` from the browse bar.
+ *
+ * @param {import('@playwright/test').Page} page
+ * @param {string} newName
+ */
+async function renameCurrentObjectFromBrowseBar(page, newName) {
+  const nameInput = page.getByLabel('Browse bar object name');
+  await nameInput.click();
+  await nameInput.fill('');
+  await nameInput.fill(newName);
+  // Click the browse bar container to save changes
+  await page.getByLabel('Browse bar', { exact: true }).click();
+}
+
 export {
   createDomainObjectWithDefaults,
   createExampleTelemetryObject,
@@ -693,6 +708,7 @@ export {
   linkParameterToObject,
   navigateToObjectWithFixedTimeBounds,
   navigateToObjectWithRealTime,
+  renameCurrentObjectFromBrowseBar,
   setEndOffset,
   setFixedIndependentTimeConductorBounds,
   setFixedTimeMode,
diff --git a/e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js b/e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js
index 070a99058e9..c6aaa9e99a1 100644
--- a/e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js
+++ b/e2e/tests/functional/plugins/notebook/notebook.e2e.spec.js
@@ -26,7 +26,10 @@ This test suite is dedicated to tests which verify the basic operations surround
 
 import { fileURLToPath } from 'url';
 
-import { createDomainObjectWithDefaults } from '../../../../appActions.js';
+import {
+  createDomainObjectWithDefaults,
+  renameCurrentObjectFromBrowseBar
+} from '../../../../appActions.js';
 import { copy, paste, selectAll } from '../../../../helper/hotkeys/hotkeys.js';
 import * as nbUtils from '../../../../helper/notebookUtils.js';
 import { expect, streamToString, test } from '../../../../pluginFixtures.js';
@@ -596,4 +599,61 @@ test.describe('Notebook entry tests', () => {
     await expect(await page.locator(`text="${TEST_TEXT.repeat(1)}"`).count()).toEqual(1);
     await expect(await page.locator(`text="${TEST_TEXT.repeat(2)}"`).count()).toEqual(0);
   });
+
+  test('When changing the name of a notebook in the browse bar, new notebook changes are not lost', async ({
+    page
+  }) => {
+    const TEST_TEXT = 'Do not lose me!';
+    const FIRST_NEW_NAME = 'New Name';
+    const SECOND_NEW_NAME = 'Second New Name';
+
+    await page.goto(notebookObject.url);
+
+    await page.getByLabel('Expand My Items folder').click();
+
+    await renameCurrentObjectFromBrowseBar(page, FIRST_NEW_NAME);
+
+    // verify the name change in tree and browse bar
+    await verifyNameChange(page, FIRST_NEW_NAME);
+
+    // enter one entry
+    await enterAndCommitTextEntry(page, TEST_TEXT);
+
+    // verify the entry is present
+    await expect(await page.locator(`text="${TEST_TEXT}"`).count()).toEqual(1);
+
+    // change the name
+    await renameCurrentObjectFromBrowseBar(page, SECOND_NEW_NAME);
+
+    // verify the name change in tree and browse bar
+    await verifyNameChange(page, SECOND_NEW_NAME);
+
+    // verify the entry is still present
+    await expect(await page.locator(`text="${TEST_TEXT}"`).count()).toEqual(1);
+  });
 });
+
+/**
+ * Enter text into the last notebook entry and commit it.
+ *
+ * @param {import('@playwright/test').Page} page
+ * @param {string} text
+ */
+async function enterAndCommitTextEntry(page, text) {
+  await nbUtils.addNotebookEntry(page);
+  await nbUtils.enterTextInLastEntry(page, text);
+  await nbUtils.commitEntry(page);
+}
+
+/**
+ * Verify the name change in the tree and browse bar.
+ *
+ * @param {import('@playwright/test').Page} page
+ * @param {string} newName
+ */
+async function verifyNameChange(page, newName) {
+  await expect(
+    page.getByRole('treeitem').locator('.is-navigated-object .c-tree__item__name')
+  ).toHaveText(newName);
+  await expect(page.getByLabel('Browse bar object name')).toHaveText(newName);
+}