Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cherry-pick(#6927): Recent objects do not update when object names are changed #6949

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions e2e/appActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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,
Expand All @@ -599,5 +614,6 @@ module.exports = {
setTimeConductorBounds,
setIndependentTimeConductorBounds,
selectInspectorTab,
waitForPlotsToRender
waitForPlotsToRender,
renameObjectFromContextMenu
};
92 changes: 45 additions & 47 deletions e2e/tests/functional/recentObjects.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
78 changes: 78 additions & 0 deletions e2e/tests/functional/renaming.e2e.spec.js
Original file line number Diff line number Diff line change
@@ -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);
});
});
17 changes: 1 addition & 16 deletions e2e/tests/functional/tree.e2e.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
const { test, expect } = require('../../pluginFixtures.js');
const {
createDomainObjectWithDefaults,
openObjectTreeContextMenu
renameObjectFromContextMenu
} = require('../../appActions.js');

test.describe('Main Tree', () => {
Expand Down Expand Up @@ -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"]');
}
4 changes: 2 additions & 2 deletions src/api/forms/components/FormRow.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
9 changes: 8 additions & 1 deletion src/api/forms/components/controls/TextField.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
6 changes: 3 additions & 3 deletions src/plugins/inspectorViews/properties/DetailText.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand Down
42 changes: 42 additions & 0 deletions src/plugins/inspectorViews/properties/Location.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
Expand All @@ -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);
});
}
}
};
Expand Down
15 changes: 15 additions & 0 deletions src/plugins/inspectorViews/properties/Properties.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
});
}
}
};
Expand Down
Loading