Skip to content

Commit 51725e5

Browse files
nkomonen-amazonnkomonen-amazon
authored
fix(messages): "first use" missing fallback metric (#3634)
bug: First extension use missing fallback metric If the user has not had this run on their system yet, they will not have the state variable existing yet. It will assume they are a first time user. Solution: - If they have no state, then fallback to checking if we are aware of existing connections. - Had to move the ExtensionUse class since we now have a circular dependency due to using the Auth class Signed-off-by: Nikolas Komonen <[email protected]>
1 parent b24548a commit 51725e5

File tree

6 files changed

+118
-61
lines changed

6 files changed

+118
-61
lines changed

‎src/auth/activation.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@ import { LoginManager } from './deprecated/loginManager'
1010
import { fromString } from './providers/credentials'
1111
import { registerCommandsWithVSCode } from '../shared/vscode/commands2'
1212
import { AuthCommandBackend, AuthCommandDeclarations } from './commands'
13-
import { ExtensionUse } from '../shared/utilities/vsCodeUtils'
1413
import { getLogger } from '../shared/logger'
1514
import { isInDevEnv } from '../codecatalyst/utils'
15+
import { ExtensionUse } from './utils'
1616

1717
export async function initialize(
1818
extensionContext: vscode.ExtensionContext,
@@ -28,8 +28,6 @@ export async function initialize(
2828
}
2929
})
3030

31-
// TODO: To enable this in prod we need to remove the 'when' clause
32-
// for: '"command": "aws.auth.manageConnections"' in package.json
3331
registerCommandsWithVSCode(
3432
extensionContext,
3533
AuthCommandDeclarations.instance,
@@ -51,8 +49,6 @@ async function showManageConnectionsOnStartup() {
5149
}
5250

5351
if (isInDevEnv()) {
54-
// A dev env will have an existing connection so this scenario is redundant. But keeping
55-
// for reference.
5652
getLogger().debug('firstStartup: Detected we are in Dev Env, skipping showing Add Connections page.')
5753
return
5854
}

‎src/auth/ui/vue/show.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ import {
3232
isIamConnection,
3333
isSsoConnection,
3434
} from '../../connection'
35-
import { tryAddCredentials, signout, showRegionPrompter, promptAndUseConnection } from '../../utils'
35+
import { tryAddCredentials, signout, showRegionPrompter, promptAndUseConnection, ExtensionUse } from '../../utils'
3636
import { Region } from '../../../shared/regions/endpoints'
3737
import { CancellationError } from '../../../shared/utilities/timeoutUtils'
3838
import { validateSsoUrl, validateSsoUrlFormat } from '../../sso/validation'
@@ -41,7 +41,6 @@ import { AuthError, ServiceItemId, userCancelled } from './types'
4141
import { awsIdSignIn } from '../../../codewhisperer/util/showSsoPrompt'
4242
import { connectToEnterpriseSso } from '../../../codewhisperer/util/getStartUrl'
4343
import { trustedDomainCancellation } from '../../sso/model'
44-
import { ExtensionUse } from '../../../shared/utilities/vsCodeUtils'
4544
import { FeatureId, CredentialSourceId, Result, telemetry } from '../../../shared/telemetry/telemetry'
4645
import { AuthFormId, isBuilderIdAuth } from './authForms/types'
4746

‎src/auth/utils.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import { Auth } from './auth'
4343
import { validateIsNewSsoUrl, validateSsoUrlFormat } from './sso/validation'
4444
import { openUrl } from '../shared/utilities/vsCodeUtils'
4545
import { AuthSource } from './ui/vue/show'
46+
import { getLogger } from '../shared/logger'
4647

4748
// TODO: Look to do some refactoring to handle circular dependency later and move this to ./commands.ts
4849
export const showConnectionsPageCommand = 'aws.auth.manageConnections'
@@ -556,3 +557,54 @@ export class AuthNode implements TreeNode<Auth> {
556557
}
557558
}
558559
}
560+
561+
/**
562+
* Class to get info about the user + use of this extension
563+
*
564+
* Why is this extension in this file?
565+
* - Due to circular dependency issues since this class needs to use the {@link Auth}
566+
* instance. If we can find a better spot and not run in to the isssue this should be moved.
567+
*
568+
* Keywords for searchability:
569+
* - new user
570+
* - first time
571+
*/
572+
export class ExtensionUse {
573+
public readonly isExtensionFirstUseKey = 'isExtensionFirstUse'
574+
575+
// The result of if is first use for the remainder of the extension session.
576+
// This will reset on next startup.
577+
private isFirstUseCurrentSession: boolean | undefined
578+
579+
isFirstUse(
580+
state: vscode.Memento = globals.context.globalState,
581+
hasExistingConnections = () => Auth.instance.hasConnections
582+
): boolean {
583+
if (this.isFirstUseCurrentSession !== undefined) {
584+
return this.isFirstUseCurrentSession
585+
}
586+
587+
this.isFirstUseCurrentSession = state.get(this.isExtensionFirstUseKey)
588+
if (this.isFirstUseCurrentSession === undefined) {
589+
// The variable in the store is not defined yet, fallback to checking if they have existing connections.
590+
this.isFirstUseCurrentSession = !hasExistingConnections()
591+
592+
getLogger().debug(
593+
`isFirstUse: State not found, marking user as '${
594+
this.isFirstUseCurrentSession ? '' : 'NOT '
595+
}first use' since they 'did ${this.isFirstUseCurrentSession ? 'NOT ' : ''}have existing connections'.`
596+
)
597+
}
598+
599+
// Update state, so next time it is not first use
600+
state.update(this.isExtensionFirstUseKey, false)
601+
602+
return this.isFirstUseCurrentSession
603+
}
604+
605+
static #instance: ExtensionUse
606+
607+
static get instance(): ExtensionUse {
608+
return (this.#instance ??= new ExtensionUse())
609+
}
610+
}

‎src/shared/utilities/vsCodeUtils.ts‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import { CancellationError, Timeout, waitTimeout, waitUntil } from './timeoutUti
1212
import { telemetry } from '../telemetry/telemetry'
1313
import * as semver from 'semver'
1414
import { isNonNullable } from './tsUtils'
15-
import globals from '../extensionGlobals'
1615

1716
// TODO: Consider NLS initialization/configuration here & have packages to import localize from here
1817
export const localize = nls.loadMessageBundle()
@@ -48,36 +47,6 @@ export async function closeAllEditors() {
4847
}
4948
}
5049

51-
/**
52-
* Class to get info about the extension being first used
53-
*/
54-
export class ExtensionUse {
55-
private readonly isExtensionFirstUseKey = 'isExtensionFirstUse'
56-
57-
// The result of if is first use for the remainder of the extension session.
58-
// This will reset on next startup.
59-
private isFirstUseCurrentSession: boolean | undefined
60-
61-
isFirstUse(state: vscode.Memento = globals.context.globalState): boolean {
62-
if (this.isFirstUseCurrentSession !== undefined) {
63-
return this.isFirstUseCurrentSession
64-
}
65-
66-
this.isFirstUseCurrentSession = state.get(this.isExtensionFirstUseKey, true)
67-
68-
// Update state, so next time it is not first use
69-
state.update(this.isExtensionFirstUseKey, false)
70-
71-
return this.isFirstUseCurrentSession
72-
}
73-
74-
static #instance: ExtensionUse
75-
76-
static get instance(): ExtensionUse {
77-
return (this.#instance ??= new ExtensionUse())
78-
}
79-
}
80-
8150
/**
8251
* Checks if an extension is installed and active.
8352
*/

‎src/test/credentials/utils.test.ts‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/*!
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
* SPDX-License-Identifier: Apache-2.0
4+
*/
5+
import * as vscode from 'vscode'
6+
import * as assert from 'assert'
7+
import { FakeExtensionContext } from '../fakeExtensionContext'
8+
import { ExtensionUse } from '../../auth/utils'
9+
10+
describe('ExtensionUse.isFirstUse()', function () {
11+
let fakeState: vscode.Memento
12+
let instance: ExtensionUse
13+
14+
beforeEach(async function () {
15+
fakeState = (await FakeExtensionContext.create()).globalState
16+
instance = new ExtensionUse()
17+
fakeState.update(ExtensionUse.instance.isExtensionFirstUseKey, true)
18+
})
19+
20+
it('is true only on first startup', function () {
21+
assert.strictEqual(instance.isFirstUse(fakeState), true, 'Failed on first call.')
22+
assert.strictEqual(instance.isFirstUse(fakeState), true, 'Failed on second call.')
23+
24+
const nextStartup = nextExtensionStartup()
25+
assert.strictEqual(nextStartup.isFirstUse(fakeState), false, 'Failed on new startup.')
26+
})
27+
28+
it('true when: (state value not exists + NOT has existing connections)', async function () {
29+
await makeStateValueNotExist()
30+
const notHasExistingConnections = () => false
31+
assert.strictEqual(
32+
instance.isFirstUse(fakeState, notHasExistingConnections),
33+
true,
34+
'No existing connections, should be first use'
35+
)
36+
assert.strictEqual(nextExtensionStartup().isFirstUse(fakeState), false)
37+
})
38+
39+
it('false when: (state value not exists + has existing connections)', async function () {
40+
await makeStateValueNotExist()
41+
const hasExistingConnections = () => true
42+
assert.strictEqual(
43+
instance.isFirstUse(fakeState, hasExistingConnections),
44+
false,
45+
'Found existing connections, should not be first use'
46+
)
47+
assert.strictEqual(nextExtensionStartup().isFirstUse(fakeState), false)
48+
})
49+
50+
/**
51+
* This makes the backend state value: undefined, mimicking a brand new user.
52+
* We use this state value to track if user is a first time user.
53+
*/
54+
async function makeStateValueNotExist() {
55+
await fakeState.update(ExtensionUse.instance.isExtensionFirstUseKey, undefined)
56+
}
57+
58+
/**
59+
* Mimics when the extension startsup a subsequent time (i.e user closes vscode and opens again).
60+
*/
61+
function nextExtensionStartup() {
62+
return new ExtensionUse()
63+
}
64+
})

‎src/test/shared/utilities/vscodeUtils.test.ts‎

Lines changed: 0 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ import * as assert from 'assert'
77
import { VSCODE_EXTENSION_ID } from '../../../shared/extensions'
88
import * as vscodeUtil from '../../../shared/utilities/vsCodeUtils'
99
import * as vscode from 'vscode'
10-
import { FakeExtensionContext } from '../../fakeExtensionContext'
11-
import { ExtensionUse } from '../../../shared/utilities/vsCodeUtils'
1210

1311
describe('vscodeUtils', async function () {
1412
it('activateExtension(), isExtensionActive()', async function () {
@@ -101,24 +99,3 @@ describe('buildMissingExtensionMessage()', function () {
10199
)
102100
})
103101
})
104-
105-
describe('ExtensionUse.isFirstUse()', function () {
106-
let fakeState: vscode.Memento
107-
let instance: ExtensionUse
108-
109-
beforeEach(async function () {
110-
fakeState = (await FakeExtensionContext.create()).globalState
111-
instance = new ExtensionUse()
112-
})
113-
114-
it('is true only on first startup', function () {
115-
assert.strictEqual(instance.isFirstUse(fakeState), true, 'Failed on first call.')
116-
assert.strictEqual(instance.isFirstUse(fakeState), true, 'Failed on second call.')
117-
118-
// Mimic new extension startup, since a new instance
119-
// is created on each load of the extension.
120-
const secondInstance = new ExtensionUse()
121-
122-
assert.strictEqual(secondInstance.isFirstUse(fakeState), false, 'Failed on new startup.')
123-
})
124-
})

0 commit comments

Comments
 (0)