Skip to content

Commit f225d6e

Browse files
committed
feat(notifications): add misc rule, log, type, and test changes
- Add types to some RuleContext fields - Add some informative log statements for debugging - Add a test for getRuleContext() - Fix getRuleContext to properly capture authTypes - Add case-insenstive rule checks - Remove a few unneeded lines of code - Add red icon for emergency notifications
1 parent 3ca78ba commit f225d6e

File tree

5 files changed

+117
-26
lines changed

5 files changed

+117
-26
lines changed

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -81,5 +81,4 @@ export type AuthFormId =
8181
| 'identityCenterCodeWhisperer'
8282
| 'identityCenterCodeCatalyst'
8383
| 'identityCenterExplorer'
84-
| 'aggregateExplorer'
8584
| 'unknown'

packages/core/src/notifications/panelNode.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import * as vscode from 'vscode'
77
import { ResourceTreeDataProvider, TreeNode } from '../shared/treeview/resourceTreeDataProvider'
88
import { Command, Commands } from '../shared/vscode/commands2'
9-
import { getIcon } from '../shared/icons'
9+
import { Icon, getIcon } from '../shared/icons'
1010
import { contextKey, setContext } from '../shared/vscode/setContext'
1111
import { NotificationType, ToolkitNotification, getNotificationTelemetryId } from './types'
1212
import { ToolkitError } from '../shared/errors'
@@ -78,10 +78,19 @@ export class NotificationsNode implements TreeNode {
7878

7979
public getChildren() {
8080
const buildNode = (n: ToolkitNotification, type: NotificationType) => {
81+
let icon: Icon & { color?: vscode.ThemeColor }
82+
if (type === 'startUp') {
83+
icon = getIcon('vscode-question') as Icon
84+
} else {
85+
icon = getIcon('vscode-alert') as Icon
86+
icon.color = new vscode.ThemeColor('errorForeground')
87+
}
88+
8189
return this.openNotificationCmd.build(n).asTreeNode({
8290
label: n.uiRenderInstructions.content['en-US'].title,
83-
iconPath: type === 'startUp' ? getIcon('vscode-question') : getIcon('vscode-alert'),
91+
iconPath: icon,
8492
contextValue: type === 'startUp' ? this.startUpNodeContext : this.emergencyNodeContext,
93+
tooltip: 'Click to open',
8594
})
8695
}
8796

packages/core/src/notifications/rules.ts

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import * as semver from 'semver'
88
import globals from '../shared/extensionGlobals'
99
import { ConditionalClause, RuleContext, DisplayIf, CriteriaCondition, ToolkitNotification, AuthState } from './types'
1010
import { getComputeEnvType, getOperatingSystem } from '../shared/telemetry/util'
11+
import { AuthFormId } from '../login/webview/vue/types'
12+
import { getLogger } from '../shared/logger/logger'
1113

1214
/**
1315
* Evaluates if a given version fits into the parameters specified by a notification, e.g:
@@ -96,17 +98,16 @@ export class RuleEngine {
9698
}
9799

98100
private evaluateRule(criteria: CriteriaCondition) {
99-
const expected = criteria.values
101+
const expected = criteria.values.map((v) => v.toLowerCase())
100102
const expectedSet = new Set(expected)
101103

102-
const isExpected = (i: string) => expectedSet.has(i)
103-
const hasAnyOfExpected = (i: string[]) => i.some((v) => expectedSet.has(v))
104+
const hasAnyOfExpected = (i: string[]) => i.map((v) => v.toLowerCase()).some((v) => expectedSet.has(v))
104105
const isSuperSetOfExpected = (i: string[]) => {
105-
const s = new Set(i)
106+
const s = new Set(i.map((v) => v.toLowerCase()))
106107
return expected.every((v) => s.has(v))
107108
}
108109
const isEqualSetToExpected = (i: string[]) => {
109-
const s = new Set(i)
110+
const s = new Set(i.map((v) => v.toLowerCase()))
110111
return expected.every((v) => s.has(v)) && i.every((v) => expectedSet.has(v))
111112
}
112113

@@ -116,10 +117,9 @@ export class RuleEngine {
116117
// So... YAGNI
117118
switch (criteria.type) {
118119
case 'OS':
119-
// todo: allow lowercase?
120-
return isExpected(this.context.os)
120+
return hasAnyOfExpected([this.context.os])
121121
case 'ComputeEnv':
122-
return isExpected(this.context.computeEnv)
122+
return hasAnyOfExpected([this.context.computeEnv])
123123
case 'AuthType':
124124
return hasAnyOfExpected(this.context.authTypes)
125125
case 'AuthRegion':
@@ -139,16 +139,40 @@ export class RuleEngine {
139139
}
140140

141141
export async function getRuleContext(context: vscode.ExtensionContext, authState: AuthState): Promise<RuleContext> {
142-
return {
142+
const authTypes =
143+
authState.authEnabledConnections === ''
144+
? []
145+
: // TODO: There is a large disconnect in the codebase with how auth "types" are stored, displayed, sent to telemetry, etc.
146+
// For now we have this "hack" (more of an inefficiency) until auth code can be properly refactored.
147+
(authState.authEnabledConnections.split(',') as AuthFormId[]).map(
148+
(id): RuleContext['authTypes'][number] => {
149+
if (id.includes('builderId')) {
150+
return 'builderId'
151+
} else if (id.includes('identityCenter')) {
152+
return 'identityCenter'
153+
} else if (id.includes('credentials')) {
154+
return 'credentials'
155+
}
156+
return 'unknown'
157+
}
158+
)
159+
const ruleContext = {
143160
ideVersion: vscode.version,
144161
extensionVersion: context.extension.packageJSON.version,
145162
os: getOperatingSystem(),
146163
computeEnv: await getComputeEnvType(),
147-
authTypes: authState.authEnabledConnections.split(','),
148-
authRegions: authState.awsRegion ? [authState.awsRegion] : [],
149-
authStates: [authState.authStatus],
164+
authTypes: [...new Set(authTypes)],
150165
authScopes: authState.authScopes ? authState.authScopes?.split(',') : [],
151166
installedExtensions: vscode.extensions.all.map((e) => e.id),
152167
activeExtensions: vscode.extensions.all.filter((e) => e.isActive).map((e) => e.id),
168+
169+
// Toolkit (and eventually Q?) may have multiple connections with different regions and states.
170+
// However, this granularity does not seem useful at this time- only the active connection is considered.
171+
authRegions: authState.awsRegion ? [authState.awsRegion] : [],
172+
authStates: [authState.authStatus],
153173
}
174+
const { activeExtensions, installedExtensions, ...loggableRuleContext } = ruleContext
175+
getLogger('notifications').debug('getRuleContext() determined rule context: %O', loggableRuleContext)
176+
177+
return ruleContext
154178
}

packages/core/src/notifications/types.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import * as vscode from 'vscode'
77
import { EnvType, OperatingSystem } from '../shared/telemetry/util'
88
import { TypeConstructor } from '../shared/utilities/typeConstructors'
9-
import { AuthUserState } from '../shared/telemetry/telemetry.gen'
9+
import { AuthUserState, AuthStatus } from '../shared/telemetry/telemetry.gen'
1010

1111
/** Types of information that we can use to determine whether to show a notification or not. */
1212
export type Criteria =
@@ -128,9 +128,9 @@ export interface RuleContext {
128128
readonly extensionVersion: string
129129
readonly os: OperatingSystem
130130
readonly computeEnv: EnvType
131-
readonly authTypes: string[]
131+
readonly authTypes: ('credentials' | 'builderId' | 'identityCenter' | 'unknown')[]
132132
readonly authRegions: string[]
133-
readonly authStates: string[]
133+
readonly authStates: AuthStatus[]
134134
readonly authScopes: string[]
135135
readonly installedExtensions: string[]
136136
readonly activeExtensions: string[]

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

Lines changed: 67 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,16 @@
33
* SPDX-License-Identifier: Apache-2.0
44
*/
55

6+
import * as vscode from 'vscode'
67
import assert from 'assert'
7-
import { RuleEngine } from '../../notifications/rules'
8+
import { RuleEngine, getRuleContext } from '../../notifications/rules'
89
import { DisplayIf, ToolkitNotification, RuleContext } from '../../notifications/types'
9-
import { globals } from '../../shared'
10+
import globals from '../../shared/extensionGlobals'
11+
import { Connection, scopesCodeCatalyst } from '../../auth/connection'
12+
import { getOperatingSystem } from '../../shared/telemetry/util'
13+
import { getAuthFormIdsFromConnection } from '../../auth/utils'
14+
import { builderIdStartUrl } from '../../auth/sso/model'
15+
import { amazonQScopes } from '../../codewhisperer'
1016

1117
describe('Notifications Rule Engine', function () {
1218
const context: RuleContext = {
@@ -365,7 +371,7 @@ describe('Notifications Rule Engine', function () {
365371
assert.equal(
366372
ruleEngine.shouldDisplayNotification(
367373
buildNotification({
368-
additionalCriteria: [{ type: 'InstalledExtensions', values: ['ext1', 'ext2', 'unkownExtension'] }],
374+
additionalCriteria: [{ type: 'InstalledExtensions', values: ['ext1', 'ext2', 'unknownExtension'] }],
369375
})
370376
),
371377
false
@@ -394,7 +400,7 @@ describe('Notifications Rule Engine', function () {
394400
)
395401
})
396402

397-
it('should display notification for combined criteria', function () {
403+
it('should display notification for combined case-insensitive criteria', function () {
398404
assert.equal(
399405
ruleEngine.shouldDisplayNotification(
400406
buildNotification({
@@ -418,11 +424,11 @@ describe('Notifications Rule Engine', function () {
418424
values: ['1.19.0', '1.20.0'],
419425
},
420426
additionalCriteria: [
421-
{ type: 'OS', values: ['LINUX', 'MAC'] },
427+
{ type: 'OS', values: ['LINUX', 'mac'] },
422428
{ type: 'ComputeEnv', values: ['local', 'ec2'] },
423-
{ type: 'AuthType', values: ['builderId', 'iamIdentityCenter'] },
424-
{ type: 'AuthRegion', values: ['us-east-1', 'us-west-2'] },
425-
{ type: 'AuthState', values: ['connected'] },
429+
{ type: 'AuthType', values: ['builderid', 'iamIdentityCenter'] },
430+
{ type: 'AuthRegion', values: ['US-EAST-1', 'us-west-2'] },
431+
{ type: 'AuthState', values: ['Connected'] },
426432
{ type: 'AuthScopes', values: ['codewhisperer:completions', 'codewhisperer:analysis'] },
427433
{ type: 'InstalledExtensions', values: ['ext1', 'ext2'] },
428434
{ type: 'ActiveExtensions', values: ['ext1', 'ext2'] },
@@ -473,3 +479,56 @@ describe('Notifications Rule Engine', function () {
473479
)
474480
})
475481
})
482+
483+
describe('Notifications getRuleContext()', function () {
484+
it('should return the correct rule context', async function () {
485+
const context = globals.context
486+
context.extension.packageJSON.version = '0.0.1'
487+
const authEnabledConnections = (
488+
[
489+
{
490+
type: 'iam',
491+
},
492+
{
493+
type: 'sso',
494+
scopes: amazonQScopes,
495+
startUrl: builderIdStartUrl,
496+
},
497+
{
498+
type: 'sso',
499+
scopes: scopesCodeCatalyst,
500+
startUrl: builderIdStartUrl,
501+
},
502+
{
503+
type: 'sso',
504+
scopes: amazonQScopes,
505+
startUrl: 'something',
506+
},
507+
{
508+
type: 'unknown',
509+
},
510+
] as Connection[]
511+
)
512+
.map((c) => getAuthFormIdsFromConnection(c))
513+
.join(',')
514+
515+
const ruleContext = await getRuleContext(context, {
516+
authEnabledConnections,
517+
authScopes: amazonQScopes.join(','),
518+
authStatus: 'connected',
519+
awsRegion: 'us-east-1',
520+
})
521+
assert.deepStrictEqual(ruleContext, {
522+
ideVersion: vscode.version,
523+
extensionVersion: '0.0.1',
524+
os: getOperatingSystem(),
525+
computeEnv: 'test',
526+
authTypes: ['credentials', 'builderId', 'identityCenter', 'unknown'],
527+
authRegions: ['us-east-1'],
528+
authStates: ['connected'],
529+
authScopes: amazonQScopes,
530+
installedExtensions: vscode.extensions.all.map((e) => e.id),
531+
activeExtensions: vscode.extensions.all.filter((e) => e.isActive).map((e) => e.id),
532+
})
533+
})
534+
})

0 commit comments

Comments
 (0)