From 10fbf4b57ff0857903f87d2d2fcf870f21eae3ef Mon Sep 17 00:00:00 2001 From: Jamie V Date: Wed, 21 Jun 2023 12:47:18 -0700 Subject: [PATCH 01/13] adding abortSignal back to composition load --- src/ui/layout/mct-tree.vue | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/ui/layout/mct-tree.vue b/src/ui/layout/mct-tree.vue index ff161788f2b..70636aa4c40 100644 --- a/src/ui/layout/mct-tree.vue +++ b/src/ui/layout/mct-tree.vue @@ -326,12 +326,13 @@ export default { }, async openTreeItem(parentItem) { const parentPath = parentItem.navigationPath; - - this.startItemLoad(parentPath); + const abortSignal = this.startItemLoad(parentPath); + // pass in abort signal when functional const childrenItems = await this.loadAndBuildTreeItemsFor( parentItem.object.identifier, - parentItem.objectPath + parentItem.objectPath, + abortSignal ); const parentIndex = this.treeItems.indexOf(parentItem); From 87d3573e8eb7c67957d3fb42b9e84bd72d795b26 Mon Sep 17 00:00:00 2001 From: Jamie V Date: Fri, 23 Jun 2023 12:51:24 -0700 Subject: [PATCH 02/13] suppress AbortError console.errors from couch, delay requests for test to trigger abort --- e2e/tests/functional/tree.e2e.spec.js | 41 +++++++++++++++++++ .../persistence/couch/CouchObjectProvider.js | 8 +++- src/ui/layout/mct-tree.vue | 2 +- 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/e2e/tests/functional/tree.e2e.spec.js b/e2e/tests/functional/tree.e2e.spec.js index efdd70b6d71..64f744279ad 100644 --- a/e2e/tests/functional/tree.e2e.spec.js +++ b/e2e/tests/functional/tree.e2e.spec.js @@ -174,6 +174,47 @@ test.describe('Main Tree', () => { ]); }); }); + test('Opening and closing an item before the request has been fulfilled will abort the request @couchdb', async ({ + page, + openmctConfig + }) => { + const { myItemsFolderName } = openmctConfig; + let requestWasAborted = false; + + page.on('requestfailed', (request) => { + // check if the request was aborted + console.log('request error text', request.failure().errorText); + if (request.failure().errorText === 'net::ERR_ABORTED') { + requestWasAborted = true; + } + }); + + const fooData = await createDomainObjectWithDefaults(page, { + type: 'Folder', + name: 'Foo' + }); + + // Intercept and delay request + const delayInMs = 500; + + await page.route('**', async (route, request) => { + console.log('request ', request.url(), request.url().endsWith(fooData.uuid)); + await new Promise((resolve) => setTimeout(resolve, delayInMs)); + + route.continue(); + }); + + // Quickly Expand/close the root folder + const mainTree = page.getByRole('tree', { + name: 'Main Tree' + }); + const treeItem = mainTree.getByRole('treeitem', { + myItemsFolderName + }); + await treeItem.locator('.c-disclosure-triangle').dblclick({ delay: 400 }); + + expect(requestWasAborted).toBe(true); + }); }); /** diff --git a/src/plugins/persistence/couch/CouchObjectProvider.js b/src/plugins/persistence/couch/CouchObjectProvider.js index 8982d97bd84..7ba55ff7644 100644 --- a/src/plugins/persistence/couch/CouchObjectProvider.js +++ b/src/plugins/persistence/couch/CouchObjectProvider.js @@ -223,13 +223,17 @@ class CouchObjectProvider { // Network error, CouchDB unreachable. if (response === null) { this.indicator.setIndicatorToState(DISCONNECTED); - console.error(error.message); + + if (!error.name === 'AbortError') { + console.error(error.message); + } + throw new Error(`CouchDB Error - No response"`); } else { if (body?.model && isNotebookOrAnnotationType(body.model)) { // warn since we handle conflicts for notebooks console.warn(error.message); - } else { + } else if (!error.name === 'AbortError') { console.error(error.message); } diff --git a/src/ui/layout/mct-tree.vue b/src/ui/layout/mct-tree.vue index 70636aa4c40..1b5cc04c3b5 100644 --- a/src/ui/layout/mct-tree.vue +++ b/src/ui/layout/mct-tree.vue @@ -327,7 +327,7 @@ export default { async openTreeItem(parentItem) { const parentPath = parentItem.navigationPath; const abortSignal = this.startItemLoad(parentPath); - + // pass in abort signal when functional const childrenItems = await this.loadAndBuildTreeItemsFor( parentItem.object.identifier, From aa655676cdc21ff5f9452d79dfaaad1395a30283 Mon Sep 17 00:00:00 2001 From: Jamie V Date: Fri, 23 Jun 2023 13:37:34 -0700 Subject: [PATCH 03/13] fix bug with logic and remove debug code --- e2e/tests/functional/tree.e2e.spec.js | 5 +---- src/plugins/persistence/couch/CouchObjectProvider.js | 4 ++-- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/e2e/tests/functional/tree.e2e.spec.js b/e2e/tests/functional/tree.e2e.spec.js index 64f744279ad..c07099b4d42 100644 --- a/e2e/tests/functional/tree.e2e.spec.js +++ b/e2e/tests/functional/tree.e2e.spec.js @@ -183,13 +183,12 @@ test.describe('Main Tree', () => { page.on('requestfailed', (request) => { // check if the request was aborted - console.log('request error text', request.failure().errorText); if (request.failure().errorText === 'net::ERR_ABORTED') { requestWasAborted = true; } }); - const fooData = await createDomainObjectWithDefaults(page, { + await createDomainObjectWithDefaults(page, { type: 'Folder', name: 'Foo' }); @@ -198,9 +197,7 @@ test.describe('Main Tree', () => { const delayInMs = 500; await page.route('**', async (route, request) => { - console.log('request ', request.url(), request.url().endsWith(fooData.uuid)); await new Promise((resolve) => setTimeout(resolve, delayInMs)); - route.continue(); }); diff --git a/src/plugins/persistence/couch/CouchObjectProvider.js b/src/plugins/persistence/couch/CouchObjectProvider.js index 7ba55ff7644..1e4fec3ed85 100644 --- a/src/plugins/persistence/couch/CouchObjectProvider.js +++ b/src/plugins/persistence/couch/CouchObjectProvider.js @@ -224,7 +224,7 @@ class CouchObjectProvider { if (response === null) { this.indicator.setIndicatorToState(DISCONNECTED); - if (!error.name === 'AbortError') { + if (error.name !== 'AbortError') { console.error(error.message); } @@ -233,7 +233,7 @@ class CouchObjectProvider { if (body?.model && isNotebookOrAnnotationType(body.model)) { // warn since we handle conflicts for notebooks console.warn(error.message); - } else if (!error.name === 'AbortError') { + } else if (error.name !== 'AbortError') { console.error(error.message); } From c982acaf51c442721c63ffaa33a85d4fbbd35ee0 Mon Sep 17 00:00:00 2001 From: John Hill Date: Thu, 29 Jun 2023 07:52:46 -0700 Subject: [PATCH 04/13] keyboard nav properties --- src/ui/components/viewControl.vue | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ui/components/viewControl.vue b/src/ui/components/viewControl.vue index d7ee3924c32..a0fd118dddd 100644 --- a/src/ui/components/viewControl.vue +++ b/src/ui/components/viewControl.vue @@ -22,7 +22,11 @@ From f54e7e83a20683d97de0fc7d1704db98895aa825 Mon Sep 17 00:00:00 2001 From: Jamie V Date: Fri, 7 Jul 2023 10:58:09 -0700 Subject: [PATCH 05/13] suppress abort errors --- src/api/objects/ObjectAPI.js | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/api/objects/ObjectAPI.js b/src/api/objects/ObjectAPI.js index cc3171b573a..d9336e3635d 100644 --- a/src/api/objects/ObjectAPI.js +++ b/src/api/objects/ObjectAPI.js @@ -242,9 +242,15 @@ export default class ObjectAPI { return domainObject; }) .catch((error) => { - console.warn(`Failed to retrieve ${keystring}:`, error); + let result; + delete this.cache[keystring]; - const result = this.applyGetInterceptors(identifier); + + // suppress abort errors + if (error.name !== 'AbortError') { + console.warn(`Failed to retrieve ${keystring}:`, error); + result = this.applyGetInterceptors(identifier); + } return result; }); From 71dc7f5aace8b00c129b06a2fd957de91c4215eb Mon Sep 17 00:00:00 2001 From: Jamie V Date: Fri, 7 Jul 2023 14:19:51 -0700 Subject: [PATCH 06/13] added some accessibility functionaly to the viewControl component updated tree and tree test to utilize that new feature --- e2e/tests/functional/tree.e2e.spec.js | 7 +++++-- src/ui/components/viewControl.vue | 13 +++++++++++++ src/ui/layout/tree-item.vue | 1 + 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/e2e/tests/functional/tree.e2e.spec.js b/e2e/tests/functional/tree.e2e.spec.js index c07099b4d42..569e43a8fb4 100644 --- a/e2e/tests/functional/tree.e2e.spec.js +++ b/e2e/tests/functional/tree.e2e.spec.js @@ -26,7 +26,7 @@ const { openObjectTreeContextMenu } = require('../../appActions.js'); -test.describe('Main Tree', () => { +test.describe.only('Main Tree', () => { test.beforeEach(async ({ page }) => { await page.goto('./', { waitUntil: 'domcontentloaded' }); }); @@ -208,7 +208,10 @@ test.describe('Main Tree', () => { const treeItem = mainTree.getByRole('treeitem', { myItemsFolderName }); - await treeItem.locator('.c-disclosure-triangle').dblclick({ delay: 400 }); + const treeItemButton = treeItem.getByRole('button', { + name: `Expand ${myItemsFolderName} folder` + }); + await treeItemButton.dblclick({ delay: 400 }); expect(requestWasAborted).toBe(true); }); diff --git a/src/ui/components/viewControl.vue b/src/ui/components/viewControl.vue index a0fd118dddd..0b3bc542841 100644 --- a/src/ui/components/viewControl.vue +++ b/src/ui/components/viewControl.vue @@ -24,6 +24,7 @@ :class="[controlClass, { 'c-disclosure-triangle--expanded': value }, { 'is-enabled': enabled }]" tabindex="0" role="button" + :aria-label="ariaLabelValue" :aria-expanded="value ? 'true' : 'false'" @click="handleClick" @keydown.enter="handleClick" @@ -46,6 +47,18 @@ export default { controlClass: { type: String, default: 'c-disclosure-triangle' + }, + domainObject: { + type: Object, + default: () => {} + } + }, + computed: { + ariaLabelValue() { + const name = this.domainObject.name ? ` ${this.domainObject.name}` : ''; + const type = this.domainObject.type ? ` ${this.domainObject.type}` : ''; + + return `${this.value ? 'Collapse' : 'Expand'}${name}${type}`; } }, methods: { diff --git a/src/ui/layout/tree-item.vue b/src/ui/layout/tree-item.vue index 3e7e2d57600..f26a2871baf 100644 --- a/src/ui/layout/tree-item.vue +++ b/src/ui/layout/tree-item.vue @@ -44,6 +44,7 @@ Date: Fri, 7 Jul 2023 14:20:15 -0700 Subject: [PATCH 07/13] removing .only --- e2e/tests/functional/tree.e2e.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/functional/tree.e2e.spec.js b/e2e/tests/functional/tree.e2e.spec.js index 569e43a8fb4..34e89955b45 100644 --- a/e2e/tests/functional/tree.e2e.spec.js +++ b/e2e/tests/functional/tree.e2e.spec.js @@ -26,7 +26,7 @@ const { openObjectTreeContextMenu } = require('../../appActions.js'); -test.describe.only('Main Tree', () => { +test.describe('Main Tree', () => { test.beforeEach(async ({ page }) => { await page.goto('./', { waitUntil: 'domcontentloaded' }); }); From 030caf292689c6dba4bd99d9cfe7f888faa3e76c Mon Sep 17 00:00:00 2001 From: Jamie V Date: Fri, 7 Jul 2023 14:32:56 -0700 Subject: [PATCH 08/13] better logic for abort in couch provider --- src/plugins/persistence/couch/CouchObjectProvider.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/plugins/persistence/couch/CouchObjectProvider.js b/src/plugins/persistence/couch/CouchObjectProvider.js index 1e4fec3ed85..37110dcac22 100644 --- a/src/plugins/persistence/couch/CouchObjectProvider.js +++ b/src/plugins/persistence/couch/CouchObjectProvider.js @@ -220,20 +220,22 @@ class CouchObjectProvider { return json; } catch (error) { + // abort errors are expected + if (error.name === 'AbortError') { + return; + } + // Network error, CouchDB unreachable. if (response === null) { this.indicator.setIndicatorToState(DISCONNECTED); - - if (error.name !== 'AbortError') { - console.error(error.message); - } + console.error(error.message); throw new Error(`CouchDB Error - No response"`); } else { if (body?.model && isNotebookOrAnnotationType(body.model)) { // warn since we handle conflicts for notebooks console.warn(error.message); - } else if (error.name !== 'AbortError') { + } else { console.error(error.message); } From 734bdc8b7c23483999c66e47db149266efcfac3d Mon Sep 17 00:00:00 2001 From: Jamie V Date: Fri, 7 Jul 2023 14:57:30 -0700 Subject: [PATCH 09/13] simplified test with new selector --- e2e/tests/functional/tree.e2e.spec.js | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/e2e/tests/functional/tree.e2e.spec.js b/e2e/tests/functional/tree.e2e.spec.js index 34e89955b45..338f6658135 100644 --- a/e2e/tests/functional/tree.e2e.spec.js +++ b/e2e/tests/functional/tree.e2e.spec.js @@ -174,7 +174,7 @@ test.describe('Main Tree', () => { ]); }); }); - test('Opening and closing an item before the request has been fulfilled will abort the request @couchdb', async ({ + test.only('Opening and closing an item before the request has been fulfilled will abort the request @couchdb', async ({ page, openmctConfig }) => { @@ -202,16 +202,11 @@ test.describe('Main Tree', () => { }); // Quickly Expand/close the root folder - const mainTree = page.getByRole('tree', { - name: 'Main Tree' - }); - const treeItem = mainTree.getByRole('treeitem', { - myItemsFolderName - }); - const treeItemButton = treeItem.getByRole('button', { - name: `Expand ${myItemsFolderName} folder` - }); - await treeItemButton.dblclick({ delay: 400 }); + await page + .getByRole('button', { + name: `Expand ${myItemsFolderName} folder` + }) + .dblclick({ delay: 400 }); expect(requestWasAborted).toBe(true); }); From 43f94d537cd08cc8fc45a7458648c89a4ab73fe1 Mon Sep 17 00:00:00 2001 From: Jamie V Date: Mon, 10 Jul 2023 10:22:27 -0700 Subject: [PATCH 10/13] removing ".only" --- e2e/tests/functional/tree.e2e.spec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/functional/tree.e2e.spec.js b/e2e/tests/functional/tree.e2e.spec.js index 338f6658135..8eea8d227c1 100644 --- a/e2e/tests/functional/tree.e2e.spec.js +++ b/e2e/tests/functional/tree.e2e.spec.js @@ -174,7 +174,7 @@ test.describe('Main Tree', () => { ]); }); }); - test.only('Opening and closing an item before the request has been fulfilled will abort the request @couchdb', async ({ + test('Opening and closing an item before the request has been fulfilled will abort the request @couchdb', async ({ page, openmctConfig }) => { From ce2a760c79de0a357ee14df1c6dfe70accd6941a Mon Sep 17 00:00:00 2001 From: Jamie V Date: Mon, 10 Jul 2023 16:23:59 -0700 Subject: [PATCH 11/13] modifying error handling in objects.get as well as how the mock provider in the test returns the error --- src/api/objects/ObjectAPI.js | 2 +- src/api/objects/ObjectAPISpec.js | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/src/api/objects/ObjectAPI.js b/src/api/objects/ObjectAPI.js index d9336e3635d..8bd8e4bc0a0 100644 --- a/src/api/objects/ObjectAPI.js +++ b/src/api/objects/ObjectAPI.js @@ -247,7 +247,7 @@ export default class ObjectAPI { delete this.cache[keystring]; // suppress abort errors - if (error.name !== 'AbortError') { + if (!error || error.name !== 'AbortError') { console.warn(`Failed to retrieve ${keystring}:`, error); result = this.applyGetInterceptors(identifier); } diff --git a/src/api/objects/ObjectAPISpec.js b/src/api/objects/ObjectAPISpec.js index 9456440adf7..1d910ae6b59 100644 --- a/src/api/objects/ObjectAPISpec.js +++ b/src/api/objects/ObjectAPISpec.js @@ -248,10 +248,17 @@ describe('The Object API', () => { }); it('displays a notification in the event of an error', () => { - mockProvider.get.and.returnValue(Promise.reject()); + openmct.notifications.warn = jasmine.createSpy('warn'); + mockProvider.get.and.returnValue( + Promise.reject({ + name: 'Error', + status: 404, + statusText: 'Not Found' + }) + ); return objectAPI.get(mockDomainObject.identifier).catch(() => { - expect(openmct.notifications.error).toHaveBeenCalledWith( + expect(openmct.notifications.warn).toHaveBeenCalledWith( `Failed to retrieve object ${TEST_NAMESPACE}:${TEST_KEY}` ); }); From 67885463a95ece7e6e53b1c8c8ed69fa5c82da8f Mon Sep 17 00:00:00 2001 From: Jamie V Date: Mon, 10 Jul 2023 16:36:01 -0700 Subject: [PATCH 12/13] reworked abort error handling logic in object api --- src/api/objects/ObjectAPI.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/api/objects/ObjectAPI.js b/src/api/objects/ObjectAPI.js index 8bd8e4bc0a0..6bb2e803359 100644 --- a/src/api/objects/ObjectAPI.js +++ b/src/api/objects/ObjectAPI.js @@ -242,17 +242,16 @@ export default class ObjectAPI { return domainObject; }) .catch((error) => { - let result; - delete this.cache[keystring]; // suppress abort errors - if (!error || error.name !== 'AbortError') { - console.warn(`Failed to retrieve ${keystring}:`, error); - result = this.applyGetInterceptors(identifier); + if (error.name === 'AbortError') { + return; } - return result; + console.warn(`Failed to retrieve ${keystring}:`, error); + + return this.applyGetInterceptors(identifier); }); this.cache[keystring] = objectPromise; From 7a389fa8c4d9d3006dd92c9314cb56c4cba65b7a Mon Sep 17 00:00:00 2001 From: Jamie V Date: Fri, 14 Jul 2023 10:40:30 -0700 Subject: [PATCH 13/13] swapping out for new tree-item selector --- e2e/tests/functional/forms.e2e.spec.js | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/e2e/tests/functional/forms.e2e.spec.js b/e2e/tests/functional/forms.e2e.spec.js index 793c859ed74..8296ff3ce1a 100644 --- a/e2e/tests/functional/forms.e2e.spec.js +++ b/e2e/tests/functional/forms.e2e.spec.js @@ -192,8 +192,12 @@ test.describe('Persistence operations @couchdb', () => { ]); //Slow down the test a bit - await expect(page.getByRole('treeitem', { name: `  ${myItemsFolderName}` })).toBeVisible(); - await expect(page2.getByRole('treeitem', { name: `  ${myItemsFolderName}` })).toBeVisible(); + await expect( + page.getByRole('button', { name: `Expand ${myItemsFolderName} folder` }) + ).toBeVisible(); + await expect( + page2.getByRole('button', { name: `Expand ${myItemsFolderName} folder` }) + ).toBeVisible(); // Both pages: Click the Create button await Promise.all([