From f388d9a548d308a8e9ea617e0e7cacf323176ad9 Mon Sep 17 00:00:00 2001 From: Jesse Mazzella Date: Mon, 6 Mar 2023 14:01:25 -0800 Subject: [PATCH] fix(#5975): Transaction-ify the CreateAction (#6306) * fix: Transaction-ify the CreateAction * test: add regression test and update tree locators * test: make object search test less flaky * test: revert locator --- e2e/tests/functional/search.e2e.spec.js | 3 +- e2e/tests/functional/tree.e2e.spec.js | 50 ++++++++++++++++++++++--- src/plugins/formActions/CreateAction.js | 28 +++++++++++++- 3 files changed, 73 insertions(+), 8 deletions(-) diff --git a/e2e/tests/functional/search.e2e.spec.js b/e2e/tests/functional/search.e2e.spec.js index 03891977cf..be137fa7cd 100644 --- a/e2e/tests/functional/search.e2e.spec.js +++ b/e2e/tests/functional/search.e2e.spec.js @@ -133,6 +133,7 @@ test.describe('Grand Search', () => { const searchResults = page.locator(searchResultSelector); // Verify that one result is found + await expect(searchResults).toBeVisible(); expect(await searchResults.count()).toBe(1); await expect(searchResults).toHaveText(folderName); }); @@ -194,7 +195,7 @@ test.describe('Grand Search', () => { // Wait for search to finish await waitForSearchCompletion(page); - const searchResultDropDown = await page.locator(searchResultDropDownSelector); + const searchResultDropDown = page.locator(searchResultDropDownSelector); // Verify that the search result/s correctly match the search query await expect(searchResultDropDown).toContainText(folderName1); diff --git a/e2e/tests/functional/tree.e2e.spec.js b/e2e/tests/functional/tree.e2e.spec.js index e0eed0e5e7..b4e7ce13e9 100644 --- a/e2e/tests/functional/tree.e2e.spec.js +++ b/e2e/tests/functional/tree.e2e.spec.js @@ -26,10 +26,35 @@ const { openObjectTreeContextMenu } = require('../../appActions.js'); -test.describe('Tree operations', () => { +test.describe('Main Tree', () => { + test.beforeEach(async ({ page }) => { + await page.goto('./', { waitUntil: 'networkidle' }); + }); + + test('Creating a child object within a folder and immediately opening it shows the created object in the tree @couchdb', async ({ page }) => { + test.info().annotations.push({ + type: 'issue', + description: 'https://github.com/nasa/openmct/issues/5975' + }); + + const folder = await createDomainObjectWithDefaults(page, { + type: 'Folder' + }); + + await page.getByTitle('Show selected item in tree').click(); + + const clock = await createDomainObjectWithDefaults(page, { + type: 'Clock', + parent: folder.uuid + }); + + await expandTreePaneItemByName(page, folder.name); + await assertTreeItemIsVisible(page, clock.name); + + }); + test('Renaming an object reorders the tree @unstable', async ({ page, openmctConfig }) => { const { myItemsFolderName } = openmctConfig; - await page.goto('./', { waitUntil: 'networkidle' }); await createDomainObjectWithDefaults(page, { type: 'Folder', @@ -111,17 +136,30 @@ async function getAndAssertTreeItems(page, expected) { expect(allTexts).toEqual(expected); } +async function assertTreeItemIsVisible(page, name) { + const mainTree = page.getByRole('tree', { + name: 'Main Tree' + }); + const treeItem = mainTree.getByRole('treeitem', { + name + }); + + await expect(treeItem).toBeVisible(); +} + /** * @param {import('@playwright/test').Page} page * @param {string} name */ async function expandTreePaneItemByName(page, name) { - const treePane = page.getByRole('tree', { + const mainTree = page.getByRole('tree', { name: 'Main Tree' }); - const treeItem = treePane.locator(`role=treeitem[expanded=false][name=/${name}/]`); - const expandTriangle = treeItem.locator('.c-disclosure-triangle'); - await expandTriangle.click(); + const treeItem = mainTree.getByRole('treeitem', { + name, + expanded: false + }); + await treeItem.locator('.c-disclosure-triangle').click(); } /** diff --git a/src/plugins/formActions/CreateAction.js b/src/plugins/formActions/CreateAction.js index 8f455f4512..950987898b 100644 --- a/src/plugins/formActions/CreateAction.js +++ b/src/plugins/formActions/CreateAction.js @@ -27,11 +27,14 @@ import { v4 as uuid } from 'uuid'; import _ from 'lodash'; export default class CreateAction extends PropertiesAction { + #transaction; + constructor(openmct, type, parentDomainObject) { super(openmct); this.type = type; this.parentDomainObject = parentDomainObject; + this.#transaction = null; } invoke() { @@ -77,6 +80,7 @@ export default class CreateAction extends PropertiesAction { await this.openmct.objects.save(this.domainObject); const compositionCollection = await this.openmct.composition.get(parentDomainObject); compositionCollection.add(this.domainObject); + await this.saveTransaction(); this._navigateAndEdit(this.domainObject, parentDomainObjectPath); @@ -95,8 +99,12 @@ export default class CreateAction extends PropertiesAction { * @private */ _onCancel() { - //do Nothing + this.#transaction.cancel().then(() => { + this.openmct.objects.endTransaction(); + this.#transaction = null; + }); } + /** * @private */ @@ -153,6 +161,8 @@ export default class CreateAction extends PropertiesAction { const formStructure = createWizard.getFormStructure(true); formStructure.title = 'Create a New ' + definition.name; + this.startTransaction(); + this.openmct.forms.showForm(formStructure) .then(this._onSave.bind(this)) .catch(this._onCancel.bind(this)) @@ -160,4 +170,20 @@ export default class CreateAction extends PropertiesAction { this.openmct.objects.destroyMutable(this.domainObject); }); } + + startTransaction() { + if (!this.openmct.objects.isTransactionActive()) { + this.#transaction = this.openmct.objects.startTransaction(); + } + } + + async saveTransaction() { + if (!this.#transaction) { + return; + } + + await this.#transaction.commit(); + this.openmct.objects.endTransaction(); + this.#transaction = null; + } }