feat(notifications): add misc rule, log, type, and test changes (#5989)

- Add types to some RuleContext fields
- Add some informative log statements for debugging
- Add a test for getRuleContext()
- Add case-insenstive rule checks
- Fix getRuleContext to properly capture authTypes
- Remove a few unneeded lines of code
- Add red icon for emergency notifications

---

<!--- REMINDER: Ensure that your PR meets the guidelines in
CONTRIBUTING.md -->

License: I confirm that my contribution is made under the terms of the
Apache 2.0 license.

Author: hayemaxi

packages/core/src/login/webview/vue/types.ts

@@ -81,5 +81,4 @@ export type AuthFormId =
     | 'identityCenterCodeWhisperer'
     | 'identityCenterCodeCatalyst'
     | 'identityCenterExplorer'
-    | 'aggregateExplorer'
     | 'unknown'

packages/core/src/notifications/panelNode.ts

@@ -6,7 +6,7 @@
 import * as vscode from 'vscode'
 import { ResourceTreeDataProvider, TreeNode } from '../shared/treeview/resourceTreeDataProvider'
 import { Command, Commands } from '../shared/vscode/commands2'
-import { getIcon } from '../shared/icons'
+import { Icon, IconPath, getIcon } from '../shared/icons'
 import { contextKey, setContext } from '../shared/vscode/setContext'
 import { NotificationType, ToolkitNotification, getNotificationTelemetryId } from './types'
 import { ToolkitError } from '../shared/errors'
@@ -78,10 +78,15 @@ export class NotificationsNode implements TreeNode {
 
     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') }
             return this.openNotificationCmd.build(n).asTreeNode({
                 label: n.uiRenderInstructions.content['en-US'].title,
-                iconPath: type === 'startUp' ? getIcon('vscode-question') : getIcon('vscode-alert'),
+                iconPath: icon,
                 contextValue: type === 'startUp' ? this.startUpNodeContext : this.emergencyNodeContext,
+                tooltip: 'Click to open',
             })
         }
 

packages/core/src/notifications/rules.ts

@@ -8,6 +8,8 @@ import * as semver from 'semver'
 import globals from '../shared/extensionGlobals'
 import { ConditionalClause, RuleContext, DisplayIf, CriteriaCondition, ToolkitNotification, AuthState } from './types'
 import { getComputeEnvType, getOperatingSystem } from '../shared/telemetry/util'
+import { AuthFormId } from '../login/webview/vue/types'
+import { getLogger } from '../shared/logger/logger'
 
 /**
  * Evaluates if a given version fits into the parameters specified by a notification, e.g:
@@ -26,8 +28,14 @@ import { getComputeEnvType, getOperatingSystem } from '../shared/telemetry/util'
 function isValidVersion(version: string, condition: ConditionalClause): boolean {
     switch (condition.type) {
         case 'range': {
-            const lowerConstraint = !condition.lowerInclusive || semver.gte(version, condition.lowerInclusive)
-            const upperConstraint = !condition.upperExclusive || semver.lt(version, condition.upperExclusive)
+            const lowerConstraint =
+                !condition.lowerInclusive ||
+                condition.lowerInclusive === '-inf' ||
+                semver.gte(version, condition.lowerInclusive)
+            const upperConstraint =
+                !condition.upperExclusive ||
+                condition.upperExclusive === '+inf' ||
+                semver.lt(version, condition.upperExclusive)
             return lowerConstraint && upperConstraint
         }
         case 'exactMatch':
@@ -99,7 +107,6 @@ export class RuleEngine {
         const expected = criteria.values
         const expectedSet = new Set(expected)
 
-        const isExpected = (i: string) => expectedSet.has(i)
         const hasAnyOfExpected = (i: string[]) => i.some((v) => expectedSet.has(v))
         const isSuperSetOfExpected = (i: string[]) => {
             const s = new Set(i)
@@ -116,10 +123,9 @@ export class RuleEngine {
         // So... YAGNI
         switch (criteria.type) {
             case 'OS':
-                // todo: allow lowercase?
-                return isExpected(this.context.os)
+                return hasAnyOfExpected([this.context.os])
             case 'ComputeEnv':
-                return isExpected(this.context.computeEnv)
+                return hasAnyOfExpected([this.context.computeEnv])
             case 'AuthType':
                 return hasAnyOfExpected(this.context.authTypes)
             case 'AuthRegion':
@@ -139,16 +145,40 @@ export class RuleEngine {
 }
 
 export async function getRuleContext(context: vscode.ExtensionContext, authState: AuthState): Promise<RuleContext> {
-    return {
+    const authTypes =
+        authState.authEnabledConnections === ''
+            ? []
+            : // TODO: There is a large disconnect in the codebase with how auth "types" are stored, displayed, sent to telemetry, etc.
+              // For now we have this "hack" (more of an inefficiency) until auth code can be properly refactored.
+              (authState.authEnabledConnections.split(',') as AuthFormId[]).map(
+                  (id): RuleContext['authTypes'][number] => {
+                      if (id.includes('builderId')) {
+                          return 'builderId'
+                      } else if (id.includes('identityCenter')) {
+                          return 'identityCenter'
+                      } else if (id.includes('credentials')) {
+                          return 'credentials'
+                      }
+                      return 'unknown'
+                  }
+              )
+    const ruleContext = {
         ideVersion: vscode.version,
         extensionVersion: context.extension.packageJSON.version,
         os: getOperatingSystem(),
         computeEnv: await getComputeEnvType(),
-        authTypes: authState.authEnabledConnections.split(','),
-        authRegions: authState.awsRegion ? [authState.awsRegion] : [],
-        authStates: [authState.authStatus],
+        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)
+
+    return ruleContext
 }

packages/core/src/notifications/types.ts

@@ -6,7 +6,7 @@
 import * as vscode from 'vscode'
 import { EnvType, OperatingSystem } from '../shared/telemetry/util'
 import { TypeConstructor } from '../shared/utilities/typeConstructors'
-import { AuthUserState } from '../shared/telemetry/telemetry.gen'
+import { AuthUserState, AuthStatus } from '../shared/telemetry/telemetry.gen'
 
 /** Types of information that we can use to determine whether to show a notification or not. */
 export type Criteria =
@@ -128,9 +128,9 @@ export interface RuleContext {
     readonly extensionVersion: string
     readonly os: OperatingSystem
     readonly computeEnv: EnvType
-    readonly authTypes: string[]
+    readonly authTypes: ('credentials' | 'builderId' | 'identityCenter' | 'unknown')[]
     readonly authRegions: string[]
-    readonly authStates: string[]
+    readonly authStates: AuthStatus[]
     readonly authScopes: string[]
     readonly installedExtensions: string[]
     readonly activeExtensions: string[]

packages/core/src/test/notifications/rules.test.ts

@@ -3,10 +3,16 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
+import * as vscode from 'vscode'
 import assert from 'assert'
-import { RuleEngine } from '../../notifications/rules'
+import { RuleEngine, getRuleContext } from '../../notifications/rules'
 import { DisplayIf, ToolkitNotification, RuleContext } from '../../notifications/types'
-import { globals } from '../../shared'
+import globals from '../../shared/extensionGlobals'
+import { Connection, scopesCodeCatalyst } from '../../auth/connection'
+import { getOperatingSystem } from '../../shared/telemetry/util'
+import { getAuthFormIdsFromConnection } from '../../auth/utils'
+import { builderIdStartUrl } from '../../auth/sso/model'
+import { amazonQScopes } from '../../codewhisperer'
 
 describe('Notifications Rule Engine', function () {
     const context: RuleContext = {
@@ -126,6 +132,18 @@ describe('Notifications Rule Engine', function () {
             ),
             true
         )
+        assert.equal(
+            ruleEngine.shouldDisplayNotification(
+                buildNotification({
+                    extensionVersion: {
+                        type: 'range',
+                        lowerInclusive: '-inf',
+                        upperExclusive: '1.23.0',
+                    },
+                })
+            ),
+            true
+        )
 
         assert.equal(
             ruleEngine.shouldDisplayNotification(
@@ -138,6 +156,31 @@ describe('Notifications Rule Engine', function () {
             ),
             true
         )
+        assert.equal(
+            ruleEngine.shouldDisplayNotification(
+                buildNotification({
+                    extensionVersion: {
+                        type: 'range',
+                        lowerInclusive: '1.0.0',
+                        upperExclusive: '+inf',
+                    },
+                })
+            ),
+            true
+        )
+
+        assert.equal(
+            ruleEngine.shouldDisplayNotification(
+                buildNotification({
+                    extensionVersion: {
+                        type: 'range',
+                        lowerInclusive: '-inf',
+                        upperExclusive: '+inf',
+                    },
+                })
+            ),
+            true
+        )
     })
 
     it('should NOT display notification with invalid version range criteria', function () {
@@ -153,6 +196,18 @@ describe('Notifications Rule Engine', function () {
             ),
             false
         )
+        assert.equal(
+            ruleEngine.shouldDisplayNotification(
+                buildNotification({
+                    extensionVersion: {
+                        type: 'range',
+                        lowerInclusive: '-inf',
+                        upperExclusive: '1.20.0',
+                    },
+                })
+            ),
+            false
+        )
     })
 
     it('should display notification with version OR criteria', function () {
@@ -365,7 +420,7 @@ describe('Notifications Rule Engine', function () {
         assert.equal(
             ruleEngine.shouldDisplayNotification(
                 buildNotification({
-                    additionalCriteria: [{ type: 'InstalledExtensions', values: ['ext1', 'ext2', 'unkownExtension'] }],
+                    additionalCriteria: [{ type: 'InstalledExtensions', values: ['ext1', 'ext2', 'unknownExtension'] }],
                 })
             ),
             false
@@ -473,3 +528,56 @@ describe('Notifications Rule Engine', function () {
         )
     })
 })
+
+describe('Notifications getRuleContext()', function () {
+    it('should return the correct rule context', async function () {
+        const context = globals.context
+        context.extension.packageJSON.version = '0.0.1'
+        const authEnabledConnections = (
+            [
+                {
+                    type: 'iam',
+                },
+                {
+                    type: 'sso',
+                    scopes: amazonQScopes,
+                    startUrl: builderIdStartUrl,
+                },
+                {
+                    type: 'sso',
+                    scopes: scopesCodeCatalyst,
+                    startUrl: builderIdStartUrl,
+                },
+                {
+                    type: 'sso',
+                    scopes: amazonQScopes,
+                    startUrl: 'something',
+                },
+                {
+                    type: 'unknown',
+                },
+            ] as Connection[]
+        )
+            .map((c) => getAuthFormIdsFromConnection(c))
+            .join(',')
+
+        const ruleContext = await getRuleContext(context, {
+            authEnabledConnections,
+            authScopes: amazonQScopes.join(','),
+            authStatus: 'connected',
+            awsRegion: 'us-east-1',
+        })
+        assert.deepStrictEqual(ruleContext, {
+            ideVersion: vscode.version,
+            extensionVersion: '0.0.1',
+            os: getOperatingSystem(),
+            computeEnv: 'test',
+            authTypes: ['credentials', 'builderId', 'identityCenter', 'unknown'],
+            authRegions: ['us-east-1'],
+            authStates: ['connected'],
+            authScopes: amazonQScopes,
+            installedExtensions: vscode.extensions.all.map((e) => e.id),
+            activeExtensions: vscode.extensions.all.filter((e) => e.isActive).map((e) => e.id),
+        })
+    })
+})