Skip to content

Commit f139fa1

Browse files
gsweetwoodgsweetwood
authored
Fixes user setting for AWS telemetry, auto converts to bool (#1164)
* added function to convert telemetry setting to a bool * added function type * Restored isTelemetryEnabled and isolated unit test * update with master, fix if statement * throw error if incorrect type * update with master * add sanitize telemetry setting * Log errors, revised sanitize test
1 parent cda7451 commit f139fa1

File tree

3 files changed

+101
-18
lines changed

3 files changed

+101
-18
lines changed

‎package.nls.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,7 @@
183183
"AWS.message.error.schemas.downloadCodeBindings.cancelled:": "Download code bindings cancelled",
184184
"AWS.message.error.schemas.downloadCodeBindings.invalid_code_generation_status": "Invalid Code generation status {0}",
185185
"AWS.message.error.schemas.downloadCodeBindings.timout": "Failed to download code for schema {0} before timeout. Please try again later",
186+
"AWS.message.error.settings.telemetry.invalid_type": "The aws.telemetry value must be a boolean",
186187
"AWS.message.info.schemas.downloadCodeBindings.start": "Downloading code for schema {0}...",
187188
"AWS.message.info.schemas.downloadCodeBindings.generate": "{0}: Generating code (this may take a few seconds the first time)...",
188189
"AWS.message.info.schemas.downloadCodeBindings.extracting": "{0}: Extracting/copying code...",

‎src/shared/telemetry/activation.ts‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ import * as nls from 'vscode-nls'
1515
const localize = nls.loadMessageBundle()
1616

1717
const LEGACY_SETTINGS_TELEMETRY_VALUE_DISABLE = 'Disable'
18+
const LEGACY_SETTINGS_TELEMETRY_VALUE_ENABLE = 'Enable'
19+
const TELEMETRY_SETTING_DEFAULT = true
1820

1921
const telemetryNoticeText: string = localize(
2022
'AWS.telemetry.notificationMessage',
@@ -43,6 +45,9 @@ export async function activate(activateArguments: {
4345
}) {
4446
ext.telemetry = new DefaultTelemetryService(activateArguments.extensionContext, activateArguments.awsContext)
4547

48+
// Convert setting to boolean if it is not already
49+
await sanitizeTelemetrySetting(activateArguments.toolkitSettings)
50+
4651
// Configure telemetry based on settings, and default to enabled
4752
applyTelemetryEnabledState(ext.telemetry, activateArguments.toolkitSettings)
4853

@@ -65,6 +70,7 @@ export async function activate(activateArguments: {
6570
return
6671
}
6772

73+
validateTelemetrySettingType(activateArguments.toolkitSettings)
6874
applyTelemetryEnabledState(ext.telemetry, activateArguments.toolkitSettings)
6975
}
7076
},
@@ -73,6 +79,24 @@ export async function activate(activateArguments: {
7379
)
7480
}
7581

82+
/*
83+
* Formats the AWS telemetry setting to a boolean: false if setting value was 'Disable' or false, true for everything else
84+
*/
85+
export async function sanitizeTelemetrySetting(toolkitSettings: SettingsConfiguration): Promise<void> {
86+
const value = toolkitSettings.readSetting<any>(AWS_TELEMETRY_KEY)
87+
88+
if (typeof value === 'boolean') {
89+
return
90+
}
91+
92+
// Set telemetry value to boolean if the current value matches the legacy value
93+
if (value === LEGACY_SETTINGS_TELEMETRY_VALUE_DISABLE) {
94+
await toolkitSettings.writeSetting<any>(AWS_TELEMETRY_KEY, false, vscode.ConfigurationTarget.Global)
95+
} else if (value === LEGACY_SETTINGS_TELEMETRY_VALUE_ENABLE) {
96+
await toolkitSettings.writeSetting<any>(AWS_TELEMETRY_KEY, true, vscode.ConfigurationTarget.Global)
97+
}
98+
}
99+
76100
export function isTelemetryEnabled(toolkitSettings: SettingsConfiguration): boolean {
77101
// Setting used to be an enum, but is now a boolean.
78102
// We don't have api-based strong type support, so we have to process this value manually.
@@ -83,13 +107,21 @@ export function isTelemetryEnabled(toolkitSettings: SettingsConfiguration): bool
83107
return false
84108
}
85109

86-
// Current value is expected to be a boolean
87110
if (typeof value === 'boolean') {
88111
return value
112+
} else {
113+
return TELEMETRY_SETTING_DEFAULT
89114
}
115+
}
90116

91-
// Treat anything else (unexpected values, datatypes, or undefined) as opt-in
92-
return true
117+
function validateTelemetrySettingType(toolkitSettings: SettingsConfiguration): void {
118+
const value = toolkitSettings.readSetting<any>(AWS_TELEMETRY_KEY)
119+
if (typeof value !== 'boolean') {
120+
getLogger().error('In settings.json, aws.telemetry value must be a boolean')
121+
vscode.window.showErrorMessage(
122+
localize('AWS.message.error.settings.telemetry.invalid_type', 'The aws.telemetry value must be a boolean')
123+
)
124+
}
93125
}
94126

95127
function applyTelemetryEnabledState(telemetry: TelemetryService, toolkitSettings: SettingsConfiguration) {

‎src/test/shared/telemetry/activation.test.ts‎

Lines changed: 65 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
TELEMETRY_NOTICE_VERSION_ACKNOWLEDGED,
1717
hasUserSeenTelemetryNotice,
1818
setHasUserSeenTelemetryNotice,
19+
sanitizeTelemetrySetting,
1920
} from '../../../shared/telemetry/activation'
2021
import { DefaultSettingsConfiguration } from '../../../shared/settingsConfiguration'
2122
import { extensionSettingsPrefix } from '../../../shared/constants'
@@ -70,7 +71,7 @@ describe('handleTelemetryNoticeResponse', () => {
7071
})
7172
})
7273

73-
describe('isTelemetryEnabled', () => {
74+
describe('Telemetry on activation', () => {
7475
let settings: vscode.WorkspaceConfiguration
7576
const toolkitSettings = new DefaultSettingsConfiguration(extensionSettingsPrefix)
7677

@@ -88,23 +89,72 @@ describe('isTelemetryEnabled', () => {
8889
})
8990

9091
const scenarios = [
91-
{ initialSettingValue: 'Enable', expectedIsEnabledValue: true, desc: 'Original opt-in value' },
92-
{ initialSettingValue: 'Disable', expectedIsEnabledValue: false, desc: 'Original opt-out value' },
93-
{ initialSettingValue: 'Use IDE settings', expectedIsEnabledValue: true, desc: 'Original deferral value' },
94-
{ initialSettingValue: true, expectedIsEnabledValue: true, desc: 'Opt in' },
95-
{ initialSettingValue: false, expectedIsEnabledValue: false, desc: 'Opt out' },
96-
{ initialSettingValue: 1234, expectedIsEnabledValue: true, desc: 'Unexpected numbers' },
97-
{ initialSettingValue: { label: 'garbageData' }, expectedIsEnabledValue: true, desc: 'Unexpected object' },
98-
{ initialSettingValue: [{ label: 'garbageDataList' }], expectedIsEnabledValue: true, desc: 'Unexpected array' },
99-
{ initialSettingValue: undefined, expectedIsEnabledValue: true, desc: 'Unset value' },
92+
{
93+
initialSettingValue: 'Enable',
94+
expectedIsEnabledValue: true,
95+
desc: 'Original opt-in value',
96+
expectedSanitizedValue: true,
97+
},
98+
{
99+
initialSettingValue: 'Disable',
100+
expectedIsEnabledValue: false,
101+
desc: 'Original opt-out value',
102+
expectedSanitizedValue: false,
103+
},
104+
{
105+
initialSettingValue: 'Use IDE settings',
106+
expectedIsEnabledValue: true,
107+
desc: 'Original deferral value',
108+
expectedSanitizedValue: 'Use IDE settings',
109+
},
110+
{ initialSettingValue: true, expectedIsEnabledValue: true, desc: 'Opt in', expectedSanitizedValue: true },
111+
{ initialSettingValue: false, expectedIsEnabledValue: false, desc: 'Opt out', expectedSanitizedValue: false },
112+
{
113+
initialSettingValue: 1234,
114+
expectedIsEnabledValue: true,
115+
desc: 'Unexpected numbers',
116+
expectedSanitizedValue: 1234,
117+
},
118+
{
119+
initialSettingValue: { label: 'garbageData' },
120+
expectedIsEnabledValue: true,
121+
desc: 'Unexpected object',
122+
expectedSanitizedValue: { label: 'garbageData' },
123+
},
124+
{
125+
initialSettingValue: [{ label: 'garbageDataList' }],
126+
expectedIsEnabledValue: true,
127+
desc: 'Unexpected array',
128+
expectedSanitizedValue: [{ label: 'garbageDataList' }],
129+
},
130+
{
131+
initialSettingValue: undefined,
132+
expectedIsEnabledValue: true,
133+
desc: 'Unset value',
134+
expectedSanitizedValue: true,
135+
}, // The 'expectedSanitizedValue' is true based on the package.json configuration declaration
100136
]
101137

102-
scenarios.forEach(scenario => {
103-
it(scenario.desc, async () => {
104-
await settings.update('telemetry', scenario.initialSettingValue, vscode.ConfigurationTarget.Global)
138+
describe('isTelemetryEnabled', () => {
139+
scenarios.forEach(scenario => {
140+
it(scenario.desc, async () => {
141+
await settings.update('telemetry', scenario.initialSettingValue, vscode.ConfigurationTarget.Global)
142+
143+
const isEnabled = isTelemetryEnabled(toolkitSettings)
144+
assert.strictEqual(isEnabled, scenario.expectedIsEnabledValue)
145+
})
146+
})
147+
})
148+
149+
describe('sanitizeTelemetrySetting', () => {
150+
scenarios.forEach(scenario => {
151+
it(scenario.desc, async () => {
152+
await settings.update('telemetry', scenario.initialSettingValue, vscode.ConfigurationTarget.Global)
105153

106-
const isEnabled = isTelemetryEnabled(toolkitSettings)
107-
assert.strictEqual(isEnabled, scenario.expectedIsEnabledValue)
154+
await sanitizeTelemetrySetting(toolkitSettings)
155+
const sanitizedSetting = toolkitSettings.readSetting<any>('telemetry')
156+
assert.deepStrictEqual(sanitizedSetting, scenario.expectedSanitizedValue)
157+
})
108158
})
109159
})
110160
})

0 commit comments

Comments
 (0)