diff --git a/packages/core/src/notifications/controller.ts b/packages/core/src/notifications/controller.ts index 9d1636c7a24..735721ad953 100644 --- a/packages/core/src/notifications/controller.ts +++ b/packages/core/src/notifications/controller.ts @@ -40,8 +40,6 @@ import { telemetry } from '../shared/telemetry/telemetry' export class NotificationsController { public static readonly suggestedPollIntervalMs = 1000 * 60 * 10 // 10 minutes - public readonly storageKey: globalKey - /** Internal memory state that is written to global state upon modification. */ private readonly state: NotificationsState @@ -49,7 +47,8 @@ export class NotificationsController { constructor( private readonly notificationsNode: NotificationsNode, - private readonly fetcher: NotificationFetcher = new RemoteFetcher() + private readonly fetcher: NotificationFetcher = new RemoteFetcher(), + public readonly storageKey: globalKey = 'aws.notifications' ) { if (!NotificationsController.#instance) { // Register on first creation only. @@ -57,7 +56,6 @@ export class NotificationsController { } NotificationsController.#instance = this - this.storageKey = 'aws.notifications' this.state = globals.globalState.tryGet(this.storageKey, NotificationsStateConstructor, { startUp: {}, emergency: {}, diff --git a/packages/core/src/notifications/panelNode.ts b/packages/core/src/notifications/panelNode.ts index cfcc0ad5eac..d3e93109da9 100644 --- a/packages/core/src/notifications/panelNode.ts +++ b/packages/core/src/notifications/panelNode.ts @@ -78,24 +78,23 @@ export class NotificationsNode implements TreeNode { } public refresh(): void { - if (!this.view) { - throw new ToolkitError('NotificationsNode: TreeView accessed without being registered.') - } - const totalNotifications = this.notificationCount() - if (totalNotifications > 0) { - this.view.badge = { - tooltip: `${totalNotifications} notification${totalNotifications > 1 ? 's' : ''}`, - value: totalNotifications, + if (this.view) { + if (totalNotifications > 0) { + this.view.badge = { + tooltip: `${totalNotifications} notification${totalNotifications > 1 ? 's' : ''}`, + value: totalNotifications, + } + this.view.title = `${NotificationsNode.title} (${totalNotifications})` + } else { + this.view.badge = undefined + this.view.title = NotificationsNode.title } - this.view.title = `${NotificationsNode.title} (${totalNotifications})` - void setContext(this.showContextStr, true) } else { - this.view.badge = undefined - this.view.title = NotificationsNode.title - void setContext(this.showContextStr, false) + logger.warn('NotificationsNode was refreshed but the view was not initialized!') } + void setContext(this.showContextStr, totalNotifications > 0) this.provider?.refresh() } @@ -240,7 +239,7 @@ export class NotificationsNode implements TreeNode { case 'updateAndReload': // Give things time to finish executing. globals.clock.setTimeout(() => { - void this.updateAndReload(notification.displayIf.extensionId) + return this.updateAndReload(notification.displayIf.extensionId) }, 1000) break case 'openUrl': diff --git a/packages/core/src/test/notifications/controller.test.ts b/packages/core/src/test/notifications/controller.test.ts index 669700a9cde..1f097fe7b9f 100644 --- a/packages/core/src/test/notifications/controller.test.ts +++ b/packages/core/src/test/notifications/controller.test.ts @@ -8,7 +8,8 @@ import * as FakeTimers from '@sinonjs/fake-timers' import assert from 'assert' import sinon from 'sinon' import globals from '../../shared/extensionGlobals' -import { randomUUID } from '../../shared' +import { randomUUID } from '../../shared/crypto' +import * as setContext from '../../shared/vscode/setContext' import { assertTelemetry, installFakeClock } from '../testUtil' import { NotificationFetcher, @@ -89,7 +90,7 @@ describe('Notifications Controller', function () { beforeEach(async function () { panelNode.setNotifications([], []) fetcher = new TestFetcher() - controller = new NotificationsController(panelNode, fetcher) + controller = new NotificationsController(panelNode, fetcher, '_aws.test.notification' as any) ruleEngineSpy = sinon.spy(ruleEngine, 'shouldDisplayNotification') focusPanelSpy = sinon.spy(panelNode, 'focusPanel') @@ -107,6 +108,10 @@ describe('Notifications Controller', function () { }) it('can fetch and store startup notifications', async function () { + // There seems to be a race condition with having a global spy object that is reset after each + // test. Doesn't seem to affect the other ones, for some reason. + const setContextSpy = sinon.spy(setContext, 'setContext') + const eTag = randomUUID() const content = { schemaVersion: '1.x', @@ -135,9 +140,14 @@ describe('Notifications Controller', function () { assert.deepStrictEqual(panelNode.startUpNotifications, [content.notifications[0]]) assert.equal(panelNode.getChildren().length, 1) assert.equal(focusPanelSpy.callCount, 0) + assert.ok(setContextSpy.calledWithExactly('aws.toolkit.notifications.show', true)) + + setContextSpy.restore() }) it('can fetch and store emergency notifications', async function () { + const setContextSpy = sinon.spy(setContext, 'setContext') + const eTag = randomUUID() const content = { schemaVersion: '1.x', @@ -166,9 +176,14 @@ describe('Notifications Controller', function () { assert.deepStrictEqual(panelNode.emergencyNotifications, [content.notifications[0]]) assert.equal(panelNode.getChildren().length, 1) assert.equal(focusPanelSpy.callCount, 1) + assert.ok(setContextSpy.calledWithExactly('aws.toolkit.notifications.show', true)) + + setContextSpy.restore() }) it('can fetch and store both startup and emergency notifications', async function () { + const setContextSpy = sinon.spy(setContext, 'setContext') + const eTag1 = randomUUID() const eTag2 = randomUUID() const startUpContent = { @@ -224,9 +239,14 @@ describe('Notifications Controller', function () { assert.deepStrictEqual(panelNode.emergencyNotifications, [emergencyContent.notifications[0]]) assert.equal(panelNode.getChildren().length, 2) assert.equal(focusPanelSpy.callCount, 1) + assert.ok(setContextSpy.calledWithExactly('aws.toolkit.notifications.show', true)) + + setContextSpy.restore() }) it('dismisses a startup notification', async function () { + const setContextSpy = sinon.spy(setContext, 'setContext') + const eTag = randomUUID() const content = { schemaVersion: '1.x', @@ -241,6 +261,7 @@ describe('Notifications Controller', function () { assert.equal(panelNode.getChildren().length, 2) assert.equal(panelNode.startUpNotifications.length, 2) + assert.ok(setContextSpy.calledWithExactly('aws.toolkit.notifications.show', true)) assert.deepStrictEqual(await globals.globalState.get(controller.storageKey), { startUp: { @@ -267,9 +288,13 @@ describe('Notifications Controller', function () { assert.equal(panelNode.getChildren().length, 1) assert.equal(panelNode.startUpNotifications.length, 1) + + setContextSpy.restore() }) it('does not redisplay dismissed notifications', async function () { + const setContextSpy = sinon.spy(setContext, 'setContext') + const content = { schemaVersion: '1.x', notifications: [getValidTestNotification('id:startup1')], @@ -281,9 +306,11 @@ describe('Notifications Controller', function () { await controller.pollForStartUp(ruleEngine) assert.equal(panelNode.getChildren().length, 1) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', true]) await dismissNotification(content.notifications[0]) assert.equal(panelNode.getChildren().length, 0) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', false]) content.notifications.push(getValidTestNotification('id:startup2')) fetcher.setStartUpContent({ @@ -305,6 +332,9 @@ describe('Notifications Controller', function () { }) assert.equal(panelNode.getChildren().length, 1) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', true]) + + setContextSpy.restore() }) it('does not refocus emergency notifications', async function () { @@ -376,6 +406,8 @@ describe('Notifications Controller', function () { }) it('cleans out dismissed state', async function () { + const setContextSpy = sinon.spy(setContext, 'setContext') + const startUpContent = { schemaVersion: '1.x', notifications: [getValidTestNotification('id:startup1')], @@ -434,6 +466,7 @@ describe('Notifications Controller', function () { newlyReceived: [], }) assert.equal(panelNode.getChildren().length, 1) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', true]) fetcher.setEmergencyContent({ eTag: '1', @@ -455,6 +488,9 @@ describe('Notifications Controller', function () { }) assert.equal(panelNode.getChildren().length, 0) + assert.deepStrictEqual(setContextSpy.lastCall.args, ['aws.toolkit.notifications.show', false]) + + setContextSpy.restore() }) it('does not rethrow errors when fetching', async function () { diff --git a/packages/core/src/test/notifications/rendering.test.ts b/packages/core/src/test/notifications/rendering.test.ts index 4df1caddc56..e446e53fdba 100644 --- a/packages/core/src/test/notifications/rendering.test.ts +++ b/packages/core/src/test/notifications/rendering.test.ts @@ -12,28 +12,20 @@ import { panelNode } from './controller.test' import { getTestWindow } from '../shared/vscode/window' import * as VsCodeUtils from '../../shared/utilities/vsCodeUtils' import { assertTextEditorContains, installFakeClock } from '../testUtil' +import { waitUntil } from '../../shared/utilities/timeoutUtils' +import globals from '../../shared/extensionGlobals' describe('Notifications Rendering', function () { let sandbox: sinon.SinonSandbox - let clock: FakeTimers.InstalledClock - - before(function () { - clock = installFakeClock() - }) beforeEach(function () { sandbox = sinon.createSandbox() }) afterEach(function () { - clock.reset() sandbox.restore() }) - after(function () { - clock.uninstall() - }) - // util to test txt pop-up under different senarios async function verifyTxtNotification(notification: ToolkitNotification) { const expectedContent = notification.uiRenderInstructions.content['en-US'].description @@ -103,14 +95,21 @@ describe('Notifications Rendering', function () { message.selectItem('Update and Reload') }) const excuteCommandStub = sandbox.stub(vscode.commands, 'executeCommand').resolves() + const telemetrySpy = sandbox.spy(globals.telemetry, 'flushRecords') const notification = getModalNotification() - await panelNode.openNotification(notification) // Update and Reload is put on a timer so that other methods (e.g. telemetry) can finish. - await clock.tickAsync(2000) + const clock: FakeTimers.InstalledClock = installFakeClock() + await panelNode.openNotification(notification) + + await clock.tickAsync(1000) + clock.uninstall() + + await waitUntil(async () => excuteCommandStub.called, { interval: 5, timeout: 5000 }) assert.ok(excuteCommandStub.calledWith('workbench.extensions.installExtension', 'aws.toolkit.fake.extension')) assert.ok(excuteCommandStub.calledWith('workbench.action.reloadWindow')) + assert.ok(telemetrySpy.calledOnce) }) it('executes openURL type button', async function () {