Skip to content

feat(connection-form): add connection and anonymous id to app name for atlas connections COMPASS-8591 #6750

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 3, 2025
Merged

feat(connection-form): add connection and anonymous id to app name for atlas connections COMPASS-8591 #6750

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3b12b40
feat(connection-form): add connection and anonymous id to app name
gagik Feb 25, 2025
b23fceb
test: update connectionString assertion
gagik Feb 27, 2025
0d21b46
feat: append connection and telemetry id only for Atlas connections
gagik Mar 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/compass-connections/src/stores/connections-store-redux.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1492,6 +1492,7 @@ const connectWithOptions = (
forceConnectionOptions,
browserCommandForOIDCAuth,
maximumNumberOfActiveConnections,
telemetryAnonymousId,
} = preferences.getPreferences();

const connectionProgress = getNotificationTriggers();
Expand Down Expand Up @@ -1553,10 +1554,12 @@ const connectWithOptions = (
cloneDeep(connectionOptions),
SecretsForConnection.get(connectionInfo.id) ?? {}
),
connectionId: connectionInfo.id,
defaultAppName: appName,
preferences: {
forceConnectionOptions: forceConnectionOptions ?? [],
browserCommandForOIDCAuth,
telemetryAnonymousId,
},
notifyDeviceFlow: (deviceFlowInfo) => {
connectionProgress.openNotifyDeviceAuthModal(
Expand Down
17 changes: 12 additions & 5 deletions packages/compass-web/src/entrypoint.spec.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ describe('CompassWeb', function () {
Sinon.resetHistory();
});

const testAnonymousId = 'test-anonymous-id';

async function renderCompassWebAndConnect(
props: Partial<React.ComponentProps<typeof CompassWeb>> = {},
connectFn = mockConnectFn
Expand All @@ -72,6 +74,7 @@ describe('CompassWeb', function () {
{...props}
initialPreferences={{
enableCreatingNewConnections: true,
telemetryAnonymousId: testAnonymousId,
...props.initialPreferences,
}}
onFailToLoadConnections={() => {}}
Expand All @@ -97,16 +100,20 @@ describe('CompassWeb', function () {
screen.getByText('Connecting to localhost:27017');
});

expect(mockConnectFn.getCall(0).args[0].connectionOptions).to.have.property(
'connectionString',
'mongodb://localhost:27017/?appName=Compass+Web'
);

await waitFor(() => {
screen.getByText('Connected to localhost:27017');
});

expect(onTrackSpy).to.have.been.calledWith('New Connection');

const connectionId = onTrackSpy.firstCall.args[1][
Copy link
Contributor Author

@gagik gagik Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit weird to get it from there though has added benefit of making sure tracking + the URL are consistent.

'connection_id'
] as string;

expect(mockConnectFn.getCall(0).args[0].connectionOptions).to.have.property(
'connectionString',
`mongodb://localhost:27017/?appName=Compass+Web-${testAnonymousId}-${connectionId}`
);
});

it('should render error state if connection fails', async function () {
Expand Down
9 changes: 8 additions & 1 deletion packages/connection-form/src/hooks/use-connect-form.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -843,18 +843,21 @@ function setInitialState({

export function adjustConnectionOptionsBeforeConnect({
connectionOptions,
connectionId,
defaultAppName,
notifyDeviceFlow,
preferences,
}: {
connectionOptions: Readonly<ConnectionOptions>;
connectionId: string;
defaultAppName?: string;
notifyDeviceFlow?: (deviceFlowInformation: {
verificationUrl: string;
userCode: string;
}) => void;
preferences: {
browserCommandForOIDCAuth?: string;
telemetryAnonymousId?: string;
forceConnectionOptions: [string, string][];
};
}): ConnectionOptions {
Expand All @@ -863,7 +866,11 @@ export function adjustConnectionOptionsBeforeConnect({
) => ConnectionOptions)[] = [
adjustCSFLEParams,
unsetFleOptionsIfEmptyAutoEncryption,
setAppNameParamIfMissing(defaultAppName),
setAppNameParamIfMissing({
defaultAppName,
connectionId,
telemetryAnonymousId: preferences.telemetryAnonymousId,
}),
adjustOIDCConnectionOptionsBeforeConnect({
browserCommandForOIDCAuth: preferences.browserCommandForOIDCAuth,
notifyDeviceFlow,
Expand Down
19 changes: 15 additions & 4 deletions packages/connection-form/src/utils/set-app-name-if-missing.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@ import { setAppNameParamIfMissing } from './set-app-name-if-missing';
describe('setAppNameParamIfMissing', function () {
it('leaves options unchanged if no default appName was specified', function () {
expect(
setAppNameParamIfMissing()({
setAppNameParamIfMissing({
connectionId: '123',
telemetryAnonymousId: '789',
})({
connectionString: 'mongodb://localhost/',
})
).to.deep.equal({
Expand All @@ -14,7 +17,11 @@ describe('setAppNameParamIfMissing', function () {

it('leaves options unchanged if appName was already part of the connection string', function () {
expect(
setAppNameParamIfMissing('defaultAppName')({
setAppNameParamIfMissing({
defaultAppName: 'defaultAppName',
connectionId: '123',
telemetryAnonymousId: '789',
})({
connectionString: 'mongodb://localhost/?appName=foobar',
})
).to.deep.equal({
Expand All @@ -24,11 +31,15 @@ describe('setAppNameParamIfMissing', function () {

it('sets appName to a default value if not already set', function () {
expect(
setAppNameParamIfMissing('defaultAppName')({
setAppNameParamIfMissing({
defaultAppName: 'defaultAppName',
connectionId: '123',
telemetryAnonymousId: '789',
})({
connectionString: 'mongodb://localhost/',
})
).to.deep.equal({
connectionString: 'mongodb://localhost/?appName=defaultAppName',
connectionString: 'mongodb://localhost/?appName=defaultAppName-789-123',
});
});
});
18 changes: 14 additions & 4 deletions packages/connection-form/src/utils/set-app-name-if-missing.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,15 @@ import type { MongoClientOptions } from 'mongodb';
import { ConnectionString } from 'mongodb-connection-string-url';
import type { ConnectionOptions } from 'mongodb-data-service';

export function setAppNameParamIfMissing(
defaultAppName?: string
): (connectionOptions: Readonly<ConnectionOptions>) => ConnectionOptions {
export function setAppNameParamIfMissing({
defaultAppName,
telemetryAnonymousId,
connectionId,
}: {
defaultAppName?: string;
telemetryAnonymousId?: string;
connectionId: string;
}): (connectionOptions: Readonly<ConnectionOptions>) => ConnectionOptions {
return (connectionOptions) => {
const connectionStringUrl = new ConnectionString(
connectionOptions.connectionString
Expand All @@ -14,7 +20,11 @@ export function setAppNameParamIfMissing(
const searchParams =
connectionStringUrl.typedSearchParams<MongoClientOptions>();
if (!searchParams.has('appName') && defaultAppName !== undefined) {
searchParams.set('appName', defaultAppName);
const appName = `${defaultAppName}${
Copy link
Contributor Author

@gagik gagik Feb 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used - as a separator, open to suggestions if this can be problem causing in ways

telemetryAnonymousId ? `-${telemetryAnonymousId}` : ''
}-${connectionId}`;

searchParams.set('appName', appName);
connectionOptions = {
...cloneDeep(connectionOptions),
connectionString: connectionStringUrl.toString(),
Expand Down
Loading