Merge pull request #6004 from hayemaxi/notifications-3

feat(notification): misc logging, types, fixes

Author: hayemaxi

packages/core/src/notifications/activation.ts

@@ -38,8 +38,8 @@ export async function activate(
     const controller = new NotificationsController(panelNode)
     const engine = new RuleEngine(await getRuleContext(context, initialState))
 
-    void controller.pollForStartUp(engine)
-    void controller.pollForEmergencies(engine)
+    await controller.pollForStartUp(engine)
+    await controller.pollForEmergencies(engine)
 
     globals.clock.setInterval(async () => {
         const ruleContext = await getRuleContext(context, await authStateFn())

packages/core/src/notifications/controller.ts

@@ -40,24 +40,22 @@ 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
 
     static #instance: NotificationsController | undefined
 
     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.
             registerDismissCommand()
         }
         NotificationsController.#instance = this
 
-        this.storageKey = 'aws.notifications'
         this.state = globals.globalState.tryGet<NotificationsState>(this.storageKey, NotificationsStateConstructor, {
             startUp: {},
             emergency: {},
@@ -94,7 +92,7 @@ export class NotificationsController {
             ruleEngine.shouldDisplayNotification(n)
         )
 
-        NotificationsNode.instance.setNotifications(startUp, emergency)
+        await NotificationsNode.instance.setNotifications(startUp, emergency)
 
         // Emergency notifications can't be dismissed, but if the user minimizes the panel then
         // we don't want to focus it each time we set the notification nodes.
@@ -128,7 +126,7 @@ export class NotificationsController {
         this.state.dismissed.push(notificationId)
         await this.writeState()
 
-        NotificationsNode.instance.dismissStartUpNotification(notificationId)
+        await NotificationsNode.instance.dismissStartUpNotification(notificationId)
     }
 
     /**

packages/core/src/notifications/panelNode.ts

@@ -4,24 +4,31 @@
  */
 
 import * as vscode from 'vscode'
+import * as nls from 'vscode-nls'
 import { ResourceTreeDataProvider, TreeNode } from '../shared/treeview/resourceTreeDataProvider'
 import { Command, Commands } from '../shared/vscode/commands2'
-import { Icon, IconPath, getIcon } from '../shared/icons'
+import { Icon, getIcon } from '../shared/icons'
 import { contextKey, setContext } from '../shared/vscode/setContext'
-import { NotificationType, ToolkitNotification, getNotificationTelemetryId } from './types'
+import { NotificationType, OnReceiveType, ToolkitNotification, getNotificationTelemetryId } from './types'
 import { ToolkitError } from '../shared/errors'
 import { isAmazonQ } from '../shared/extensionUtilities'
 import { getLogger } from '../shared/logger/logger'
 import { registerToolView } from '../awsexplorer/activationShared'
 import { readonlyDocument } from '../shared/utilities/textDocumentUtilities'
 import { openUrl } from '../shared/utilities/vsCodeUtils'
 import { telemetry } from '../shared/telemetry/telemetry'
+import { globals } from '../shared'
+
+const localize = nls.loadMessageBundle()
+const logger = getLogger('notifications')
 
 /**
  * Controls the "Notifications" side panel/tree in each extension. It takes purely UX actions
  * and does not determine what notifications to dispaly or how to fetch and store them.
  */
 export class NotificationsNode implements TreeNode {
+    public static readonly title = localize('AWS.notifications.title', 'Notifications')
+
     public readonly id = 'notifications'
     public readonly resource = this
     public provider?: ResourceTreeDataProvider
@@ -34,6 +41,7 @@ export class NotificationsNode implements TreeNode {
     private readonly showContextStr: contextKey
     private readonly startUpNodeContext: string
     private readonly emergencyNodeContext: string
+    private view: vscode.TreeView<TreeNode> | undefined
 
     static #instance: NotificationsNode
 
@@ -62,31 +70,49 @@ export class NotificationsNode implements TreeNode {
     }
 
     public getTreeItem() {
-        const item = new vscode.TreeItem('Notifications')
+        const item = new vscode.TreeItem(NotificationsNode.title)
         item.collapsibleState = vscode.TreeItemCollapsibleState.Collapsed
         item.contextValue = 'notifications'
 
         return item
     }
 
-    public refresh(): void {
-        const hasNotifications = this.startUpNotifications.length > 0 || this.emergencyNotifications.length > 0
-        void setContext(this.showContextStr, hasNotifications)
+    public refresh() {
+        const totalNotifications = this.notificationCount()
+        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
+            }
+        } else {
+            logger.warn('NotificationsNode was refreshed but the view was not initialized!')
+        }
 
         this.provider?.refresh()
+        return setContext(this.showContextStr, totalNotifications > 0)
     }
 
     public getChildren() {
         const buildNode = (n: ToolkitNotification, type: NotificationType) => {
-            const icon: Icon | IconPath =
-                type === 'startUp'
-                    ? getIcon('vscode-question')
-                    : { ...getIcon('vscode-alert'), color: new vscode.ThemeColor('errorForeground') }
+            const icon: Icon =
+                type === 'emergency'
+                    ? Object.assign(getIcon('vscode-alert') as Icon, {
+                          color: new vscode.ThemeColor('errorForeground'),
+                      })
+                    : (getIcon('vscode-question') as Icon)
+
+            const title = n.uiRenderInstructions.content['en-US'].title
             return this.openNotificationCmd.build(n).asTreeNode({
-                label: n.uiRenderInstructions.content['en-US'].title,
+                label: title,
+                tooltip: title,
                 iconPath: icon,
                 contextValue: type === 'startUp' ? this.startUpNodeContext : this.emergencyNodeContext,
-                tooltip: 'Click to open',
             })
         }
 
@@ -100,10 +126,10 @@ export class NotificationsNode implements TreeNode {
      * Sets the current list of notifications. Nodes are generated for each notification.
      * No other processing is done, see NotificationController.
      */
-    public setNotifications(startUp: ToolkitNotification[], emergency: ToolkitNotification[]) {
+    public async setNotifications(startUp: ToolkitNotification[], emergency: ToolkitNotification[]) {
         this.startUpNotifications = startUp
         this.emergencyNotifications = emergency
-        this.refresh()
+        await this.refresh()
     }
 
     /**
@@ -112,9 +138,9 @@ export class NotificationsNode implements TreeNode {
      *
      * Only dismisses startup notifications.
      */
-    public dismissStartUpNotification(id: string) {
+    public async dismissStartUpNotification(id: string) {
         this.startUpNotifications = this.startUpNotifications.filter((n) => n.id !== id)
-        this.refresh()
+        await this.refresh()
     }
 
     /**
@@ -124,6 +150,10 @@ export class NotificationsNode implements TreeNode {
         return vscode.commands.executeCommand(this.focusCmdStr)
     }
 
+    private notificationCount() {
+        return this.startUpNotifications.length + this.emergencyNotifications.length
+    }
+
     /**
      * Fired when a notification is clicked on in the panel. It will run any rendering
      * instructions included in the notification. See {@link ToolkitNotification.uiRenderInstructions}.
@@ -132,23 +162,23 @@ export class NotificationsNode implements TreeNode {
         switch (notification.uiRenderInstructions.onClick.type) {
             case 'modal':
                 // Render blocking modal
-                getLogger('notifications').verbose(`rendering modal for notificaiton: ${notification.id} ...`)
+                logger.verbose(`rendering modal for notificaiton: ${notification.id} ...`)
                 await this.showInformationWindow(notification, 'modal', false)
                 break
             case 'openUrl':
                 // Show open url option
                 if (!notification.uiRenderInstructions.onClick.url) {
                     throw new ToolkitError('No url provided for onclick open url')
                 }
-                getLogger('notifications').verbose(`opening url for notification: ${notification.id} ...`)
+                logger.verbose(`opening url for notification: ${notification.id} ...`)
                 await openUrl(
                     vscode.Uri.parse(notification.uiRenderInstructions.onClick.url),
                     getNotificationTelemetryId(notification)
                 )
                 break
             case 'openTextDocument':
                 // Display read-only txt document
-                getLogger('notifications').verbose(`showing txt document for notification: ${notification.id} ...`)
+                logger.verbose(`showing txt document for notification: ${notification.id} ...`)
                 await telemetry.toolkit_invokeAction.run(async () => {
                     telemetry.record({ source: getNotificationTelemetryId(notification), action: 'openTxt' })
                     await readonlyDocument.show(
@@ -165,7 +195,11 @@ export class NotificationsNode implements TreeNode {
      * Can be either a blocking modal or a bottom-right corner toast
      * Handles the button click actions based on the button type.
      */
-    private showInformationWindow(notification: ToolkitNotification, type: string = 'toast', passive: boolean = false) {
+    private showInformationWindow(
+        notification: ToolkitNotification,
+        type: OnReceiveType = 'toast',
+        passive: boolean = false
+    ) {
         const isModal = type === 'modal'
 
         // modal has to have defined actions (buttons)
@@ -203,7 +237,10 @@ export class NotificationsNode implements TreeNode {
                                     )
                                     break
                                 case 'updateAndReload':
-                                    await this.updateAndReload(notification.displayIf.extensionId)
+                                    // Give things time to finish executing.
+                                    globals.clock.setTimeout(() => {
+                                        return this.updateAndReload(notification.displayIf.extensionId)
+                                    }, 1000)
                                     break
                                 case 'openUrl':
                                     if (selectedButton.url) {
@@ -228,7 +265,11 @@ export class NotificationsNode implements TreeNode {
     }
 
     private async updateAndReload(id: string) {
-        getLogger('notifications').verbose('Updating and reloading the extension...')
+        logger.verbose('Updating and reloading the extension...')
+
+        // Publish pending telemetry before it is lost to the window reload.
+        await globals.telemetry.flushRecords()
+
         await vscode.commands.executeCommand('workbench.extensions.installExtension', id)
         await vscode.commands.executeCommand('workbench.action.reloadWindow')
     }
@@ -258,14 +299,13 @@ export class NotificationsNode implements TreeNode {
     }
 
     registerView(context: vscode.ExtensionContext) {
-        const view = registerToolView(
+        this.view = registerToolView(
             {
                 nodes: [this],
                 view: isAmazonQ() ? 'aws.amazonq.notifications' : 'aws.toolkit.notifications',
                 refreshCommands: [(provider: ResourceTreeDataProvider) => this.registerProvider(provider)],
             },
             context
         )
-        view.message = `New feature announcements and emergency notifications for ${isAmazonQ() ? 'Amazon Q' : 'AWS Toolkit'} will appear here.`
     }
 }

packages/core/src/notifications/rules.ts

@@ -11,6 +11,7 @@ import { getComputeEnvType, getOperatingSystem } from '../shared/telemetry/util'
 import { AuthFormId } from '../login/webview/vue/types'
 import { getLogger } from '../shared/logger/logger'
 
+const logger = getLogger('notifications')
 /**
  * Evaluates if a given version fits into the parameters specified by a notification, e.g:
  *
@@ -72,31 +73,51 @@ export class RuleEngine {
     constructor(private readonly context: RuleContext) {}
 
     public shouldDisplayNotification(payload: ToolkitNotification) {
-        return this.evaluate(payload.displayIf)
+        return this.evaluate(payload.id, payload.displayIf)
     }
 
-    private evaluate(condition: DisplayIf): boolean {
+    private evaluate(id: string, condition: DisplayIf): boolean {
         const currentExt = globals.context.extension.id
         if (condition.extensionId !== currentExt) {
+            logger.verbose(
+                'notification id: (%s) did NOT pass extension id check, actual ext id: (%s), expected ext id: (%s)',
+                id,
+                currentExt,
+                condition.extensionId
+            )
             return false
         }
 
         if (condition.ideVersion) {
             if (!isValidVersion(this.context.ideVersion, condition.ideVersion)) {
+                logger.verbose(
+                    'notification id: (%s) did NOT pass IDE version check, actual version: (%s), expected version: (%s)',
+                    id,
+                    this.context.ideVersion,
+                    condition.ideVersion
+                )
                 return false
             }
         }
         if (condition.extensionVersion) {
             if (!isValidVersion(this.context.extensionVersion, condition.extensionVersion)) {
+                logger.verbose(
+                    'notification id: (%s) did NOT pass extension version check, actual ext version: (%s), expected ext version: (%s)',
+                    id,
+                    this.context.extensionVersion,
+                    condition.extensionVersion
+                )
                 return false
             }
         }
 
         if (condition.additionalCriteria) {
             for (const criteria of condition.additionalCriteria) {
                 if (!this.evaluateRule(criteria)) {
+                    logger.verbose('notification id: (%s) did NOT pass criteria check: %O', id, criteria)
                     return false
                 }
+                logger.debug('notification id: (%s) passed criteria check: %O', id, criteria)
             }
         }
 
@@ -134,8 +155,6 @@ export class RuleEngine {
                 return hasAnyOfExpected(this.context.authStates)
             case 'AuthScopes':
                 return isEqualSetToExpected(this.context.authScopes)
-            case 'InstalledExtensions':
-                return isSuperSetOfExpected(this.context.installedExtensions)
             case 'ActiveExtensions':
                 return isSuperSetOfExpected(this.context.activeExtensions)
             default:
@@ -169,16 +188,16 @@ export async function getRuleContext(context: vscode.ExtensionContext, authState
         computeEnv: await getComputeEnvType(),
         authTypes: [...new Set(authTypes)],
         authScopes: authState.authScopes ? authState.authScopes?.split(',') : [],
-        installedExtensions: vscode.extensions.all.map((e) => e.id),
         activeExtensions: vscode.extensions.all.filter((e) => e.isActive).map((e) => e.id),
 
         // Toolkit (and eventually Q?) may have multiple connections with different regions and states.
         // However, this granularity does not seem useful at this time- only the active connection is considered.
         authRegions: authState.awsRegion ? [authState.awsRegion] : [],
         authStates: [authState.authStatus],
     }
-    const { activeExtensions, installedExtensions, ...loggableRuleContext } = ruleContext
-    getLogger('notifications').debug('getRuleContext() determined rule context: %O', loggableRuleContext)
+
+    const { activeExtensions, ...loggableRuleContext } = ruleContext
+    logger.debug('getRuleContext() determined rule context: %O', loggableRuleContext)
 
     return ruleContext
 }