Skip to content

Commit

Permalink
feat(notifications): add misc rule, log, type, and test changes
Browse files Browse the repository at this point in the history
- Add types to some RuleContext fields
- Add some informative log statements for debugging
- Add a test for getRuleContext()
- Fix getRuleContext to properly capture authTypes
- Remove a few unneeded lines of code
  • Loading branch information
hayemaxi committed Nov 12, 2024
1 parent 3ca78ba commit 0a130f9
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 26 deletions.
1 change: 0 additions & 1 deletion packages/core/src/login/webview/vue/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,4 @@ export type AuthFormId =
| 'identityCenterCodeWhisperer'
| 'identityCenterCodeCatalyst'
| 'identityCenterExplorer'
| 'aggregateExplorer'
| 'unknown'
13 changes: 11 additions & 2 deletions packages/core/src/notifications/panelNode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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, getIcon } from '../shared/icons'
import { contextKey, setContext } from '../shared/vscode/setContext'
import { NotificationType, ToolkitNotification, getNotificationTelemetryId } from './types'
import { ToolkitError } from '../shared/errors'
Expand Down Expand Up @@ -78,10 +78,19 @@ export class NotificationsNode implements TreeNode {

public getChildren() {
const buildNode = (n: ToolkitNotification, type: NotificationType) => {
let icon: Icon & { color?: vscode.ThemeColor }
if (type === 'startUp') {
icon = getIcon('vscode-question') as Icon
} else {
icon = getIcon('vscode-alert') as Icon
icon.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',
})
}

Expand Down
48 changes: 36 additions & 12 deletions packages/core/src/notifications/rules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -96,17 +98,16 @@ export class RuleEngine {
}

private evaluateRule(criteria: CriteriaCondition) {
const expected = criteria.values
const expected = criteria.values.map((v) => v.toLowerCase())
const expectedSet = new Set(expected)

const isExpected = (i: string) => expectedSet.has(i)
const hasAnyOfExpected = (i: string[]) => i.some((v) => expectedSet.has(v))
const hasAnyOfExpected = (i: string[]) => i.map((v) => v.toLowerCase()).some((v) => expectedSet.has(v))
const isSuperSetOfExpected = (i: string[]) => {
const s = new Set(i)
const s = new Set(i.map((v) => v.toLowerCase()))
return expected.every((v) => s.has(v))
}
const isEqualSetToExpected = (i: string[]) => {
const s = new Set(i)
const s = new Set(i.map((v) => v.toLowerCase()))
return expected.every((v) => s.has(v)) && i.every((v) => expectedSet.has(v))
}

Expand All @@ -116,10 +117,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':
Expand All @@ -139,16 +139,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
}
6 changes: 3 additions & 3 deletions packages/core/src/notifications/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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[]
Expand Down
75 changes: 67 additions & 8 deletions packages/core/src/test/notifications/rules.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -365,7 +371,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
Expand Down Expand Up @@ -394,7 +400,7 @@ describe('Notifications Rule Engine', function () {
)
})

it('should display notification for combined criteria', function () {
it('should display notification for combined case-insensitive criteria', function () {
assert.equal(
ruleEngine.shouldDisplayNotification(
buildNotification({
Expand All @@ -418,11 +424,11 @@ describe('Notifications Rule Engine', function () {
values: ['1.19.0', '1.20.0'],
},
additionalCriteria: [
{ type: 'OS', values: ['LINUX', 'MAC'] },
{ type: 'OS', values: ['LINUX', 'mac'] },
{ type: 'ComputeEnv', values: ['local', 'ec2'] },
{ type: 'AuthType', values: ['builderId', 'iamIdentityCenter'] },
{ type: 'AuthRegion', values: ['us-east-1', 'us-west-2'] },
{ type: 'AuthState', values: ['connected'] },
{ type: 'AuthType', values: ['builderid', 'iamIdentityCenter'] },
{ type: 'AuthRegion', values: ['US-EAST-1', 'us-west-2'] },
{ type: 'AuthState', values: ['Connected'] },
{ type: 'AuthScopes', values: ['codewhisperer:completions', 'codewhisperer:analysis'] },
{ type: 'InstalledExtensions', values: ['ext1', 'ext2'] },
{ type: 'ActiveExtensions', values: ['ext1', 'ext2'] },
Expand Down Expand Up @@ -473,3 +479,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),
})
})
})

0 comments on commit 0a130f9

Please sign in to comment.