Skip to content

Commit 280c5fb

Browse files
authored
RI-6864: add reason to ANALYTICS_PERMISSION event. (#4403)
* add reason to `ANALYTICS_PERMISSION` event. If the event is triggered by accepting consents, reason is `install`, if it is because of `sso\social login\oauth` reasons, appropriate strategy is sent `google | github | sso`. One last place where analytics is being enabled is in OAuthAgreement.tsx, where reason is sent as `oauth-agreement` * update some of the test, to ensure, that having or lacking the reason, does not change overall execution of the code * update settings.service.spec.ts * extract ToggleAnalyticsReason as an object * remove references to API constants, because it breaks tests of the UI * replace UI values * remove unused import
1 parent 67f4245 commit 280c5fb

File tree

11 files changed

+103
-25
lines changed

11 files changed

+103
-25
lines changed
Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
export type ToggleAnalyticsReasonType = 'none' | 'oauth-agreement' | 'google' | 'github' | 'sso' | 'user';
2+
3+
export enum ToggleAnalyticsReason {
4+
None = 'none',
5+
OAuthAgreement = 'oauth-agreement',
6+
Google = 'google',
7+
Github = 'github',
8+
Sso = 'sso',
9+
User = 'user',
10+
}

redisinsight/api/src/modules/settings/dto/settings.dto.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { ApiProperty, ApiPropertyOptional } from '@nestjs/swagger';
22
import {
3-
IsBoolean,
3+
IsBoolean, IsEnum,
44
IsInstance,
55
IsInt,
66
IsOptional,
@@ -14,6 +14,7 @@ import { pickDefinedAgreements } from 'src/dto/dto-transformer';
1414
import { Default } from 'src/common/decorators';
1515
import config from 'src/utils/config';
1616
import { IAgreementSpec } from 'src/modules/settings/models/agreements.interface';
17+
import { ToggleAnalyticsReason, ToggleAnalyticsReasonType } from 'src/modules/settings/constants/settings';
1718

1819
const REDIS_SCAN_CONFIG = config.get('redis_scan');
1920
const WORKBENCH_CONFIG = config.get('workbench');
@@ -187,4 +188,14 @@ export class UpdateSettingsDto {
187188
@Transform(pickDefinedAgreements)
188189
@IsBoolean({ each: true })
189190
agreements?: Map<string, boolean>;
191+
192+
@ApiPropertyOptional({
193+
description: 'Reason describing why analytics are enabled',
194+
type: String,
195+
example: 'install',
196+
})
197+
@IsOptional()
198+
@IsString()
199+
@IsEnum(ToggleAnalyticsReason)
200+
analyticsReason?: ToggleAnalyticsReasonType;
190201
}

redisinsight/api/src/modules/settings/models/agreements.interface.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ export interface IAgreement {
99
title?: string;
1010
label?: string;
1111
description?: string;
12+
requiredText?: string;
1213
options?: {
1314
[key: string]: IAgreement;
1415
}

redisinsight/api/src/modules/settings/settings.analytics.spec.ts

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,43 @@ describe('SettingsAnalytics', () => {
4545
},
4646
);
4747
});
48+
it('should emit ANALYTICS_PERMISSION with state enabled and reason undefined', async () => {
49+
service.sendAnalyticsAgreementChange(
50+
mockSessionMetadata,
51+
new Map([['analytics', true]]),
52+
undefined,
53+
undefined,
54+
);
55+
56+
expect(eventEmitter.emit).toHaveBeenCalledWith(
57+
AppAnalyticsEvents.Track,
58+
mockSessionMetadata,
59+
{
60+
event: TelemetryEvents.AnalyticsPermission,
61+
eventData: { reason: undefined, state: 'enabled' },
62+
nonTracking: true,
63+
},
64+
);
65+
});
66+
it('should emit ANALYTICS_PERMISSION with state enabled and reason "test-reason"', async () => {
67+
service.sendAnalyticsAgreementChange(
68+
mockSessionMetadata,
69+
new Map([['analytics', true]]),
70+
undefined,
71+
'test-reason',
72+
);
73+
74+
expect(eventEmitter.emit).toHaveBeenCalledWith(
75+
AppAnalyticsEvents.Track,
76+
mockSessionMetadata,
77+
{
78+
event: TelemetryEvents.AnalyticsPermission,
79+
eventData: { reason: 'test-reason', state: 'enabled' },
80+
nonTracking: true,
81+
82+
},
83+
);
84+
});
4885
it('should emit ANALYTICS_PERMISSION with state disabled on first app launch', async () => {
4986
service.sendAnalyticsAgreementChange(
5087
mockSessionMetadata,
@@ -67,6 +104,7 @@ describe('SettingsAnalytics', () => {
67104
mockSessionMetadata,
68105
new Map([['analytics', false]]),
69106
new Map([['analytics', false]]),
107+
'test-reason',
70108
);
71109

72110
expect(eventEmitter.emit).not.toHaveBeenCalledWith(
@@ -83,14 +121,15 @@ describe('SettingsAnalytics', () => {
83121
mockSessionMetadata,
84122
new Map([['analytics', false]]),
85123
new Map([['analytics', true]]),
124+
'test-reason',
86125
);
87126

88127
expect(eventEmitter.emit).toHaveBeenCalledWith(
89128
AppAnalyticsEvents.Track,
90129
mockSessionMetadata,
91130
{
92131
event: TelemetryEvents.AnalyticsPermission,
93-
eventData: { state: 'disabled' },
132+
eventData: { state: 'disabled', reason: 'test-reason' },
94133
nonTracking: true,
95134
},
96135
);

redisinsight/api/src/modules/settings/settings.analytics.ts

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,8 @@
11
import { Injectable } from '@nestjs/common';
22
import { EventEmitter2 } from '@nestjs/event-emitter';
3-
import {
4-
differenceWith,
5-
isEqual,
6-
has,
7-
} from 'lodash';
8-
import { AppAnalyticsEvents, TelemetryEvents } from 'src/constants';
9-
import { getRangeForNumber, getIsPipelineEnable, SCAN_THRESHOLD_BREAKPOINTS } from 'src/utils';
3+
import { differenceWith, has, isEqual } from 'lodash';
4+
import { AppAnalyticsEvents, TelemetryEvents, ToggleAnalyticsReasonType } from 'src/constants';
5+
import { getIsPipelineEnable, getRangeForNumber, SCAN_THRESHOLD_BREAKPOINTS } from 'src/utils';
106
import { TelemetryBaseService } from 'src/modules/analytics/telemetry.base.service';
117
import { GetAppSettingsResponse } from 'src/modules/settings/dto/settings.dto';
128
import { SessionMetadata } from 'src/common/models';
@@ -42,6 +38,7 @@ export class SettingsAnalytics extends TelemetryBaseService {
4238
sessionMetadata: SessionMetadata,
4339
newAgreements: Map<string, boolean>,
4440
oldAgreements: Map<string, boolean> = new Map(),
41+
reason?: ToggleAnalyticsReasonType,
4542
) {
4643
try {
4744
const newPermission = newAgreements.get('analytics');
@@ -54,6 +51,7 @@ export class SettingsAnalytics extends TelemetryBaseService {
5451
event: TelemetryEvents.AnalyticsPermission,
5552
eventData: {
5653
state: newPermission ? 'enabled' : 'disabled',
54+
reason,
5755
},
5856
nonTracking: true,
5957
},

redisinsight/api/src/modules/settings/settings.service.spec.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
} from 'src/__mocks__';
1111
import { UpdateSettingsDto } from 'src/modules/settings/dto/settings.dto';
1212
import * as AGREEMENTS_SPEC from 'src/constants/agreements-spec.json';
13-
import { AgreementIsNotDefinedException } from 'src/constants';
13+
import { AgreementIsNotDefinedException, ToggleAnalyticsReason } from 'src/constants';
1414
import config from 'src/utils/config';
1515
import { KeytarEncryptionStrategy } from 'src/modules/encryption/strategies/keytar-encryption.strategy';
1616
import { SettingsAnalytics } from 'src/modules/settings/settings.analytics';
@@ -199,6 +199,7 @@ describe('SettingsService', () => {
199199
agreements: new Map(Object.entries({
200200
analytics: false,
201201
})),
202+
analyticsReason: ToggleAnalyticsReason.GitHub,
202203
};
203204

204205
const response = await service.updateAppSettings(mockSessionMetadata, dto);
@@ -220,6 +221,7 @@ describe('SettingsService', () => {
220221
new Map(Object.entries({
221222
...mockAgreements.data,
222223
})),
224+
'some reason',
223225
);
224226
});
225227
it('should update agreements and settings', async () => {

redisinsight/api/src/modules/settings/settings.service.ts

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import { readFile } from 'fs-extra';
1616
import { join } from 'path';
1717
import * as AGREEMENTS_SPEC from 'src/constants/agreements-spec.json';
1818
import config, { Config } from 'src/utils/config';
19-
import { AgreementIsNotDefinedException } from 'src/constants';
19+
import { AgreementIsNotDefinedException, ToggleAnalyticsReasonType } from 'src/constants';
2020
import { KeytarEncryptionStrategy } from 'src/modules/encryption/strategies/keytar-encryption.strategy';
2121
import { KeyEncryptionStrategy } from 'src/modules/encryption/strategies/key-encryption.strategy';
2222
import { SettingsAnalytics } from 'src/modules/settings/settings.analytics';
@@ -80,7 +80,7 @@ export class SettingsService {
8080
dto: UpdateSettingsDto,
8181
): Promise<GetAppSettingsResponse> {
8282
this.logger.debug('Updating application settings.', sessionMetadata);
83-
const { agreements, ...settings } = dto;
83+
const { agreements, analyticsReason, ...settings } = dto;
8484
try {
8585
const oldAppSettings = await this.getAppSettings(sessionMetadata);
8686
if (!isEmpty(settings)) {
@@ -96,7 +96,7 @@ export class SettingsService {
9696
await this.settingsRepository.update(sessionMetadata, toUpdate);
9797
}
9898
if (agreements) {
99-
await this.updateAgreements(sessionMetadata, agreements);
99+
await this.updateAgreements(sessionMetadata, agreements, analyticsReason);
100100
}
101101
this.logger.debug('Succeed to update application settings.', sessionMetadata);
102102
const results = await this.getAppSettings(sessionMetadata);
@@ -193,6 +193,7 @@ export class SettingsService {
193193
private async updateAgreements(
194194
sessionMetadata: SessionMetadata,
195195
dtoAgreements: Map<string, boolean> = new Map(),
196+
analyticsReason?: ToggleAnalyticsReasonType,
196197
): Promise<void> {
197198
this.logger.debug('Updating application agreements.', sessionMetadata);
198199
const oldAgreements = await this.agreementRepository.getOrCreate(sessionMetadata);
@@ -206,7 +207,6 @@ export class SettingsService {
206207
...Object.fromEntries(dtoAgreements),
207208
},
208209
};
209-
210210
// Detect which agreements should be defined according to the settings specification
211211
const diff = difference(
212212
Object.keys(agreementsSpec.agreements),
@@ -226,6 +226,7 @@ export class SettingsService {
226226
sessionMetadata,
227227
dtoAgreements,
228228
new Map(Object.entries(oldAgreements.data || {})),
229+
analyticsReason,
229230
);
230231
}
231232
}

redisinsight/ui/src/components/consents-settings/ConsentsSettings.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,13 +157,15 @@ const ConsentsSettings = ({ onSubmitted }: Props) => {
157157
recommended = false
158158
return false
159159
}
160+
return true
160161
})
161162

162163
forEach(notificationConsents, (consent) => {
163164
if (!formik.values[consent?.agreementName] && !consent.disabled) {
164165
recommended = false
165166
return false
166167
}
168+
return true
167169
})
168170

169171
return recommended
@@ -185,7 +187,11 @@ const ConsentsSettings = ({ onSubmitted }: Props) => {
185187
: TelemetryEvent.SETTINGS_NOTIFICATION_MESSAGES_DISABLED,
186188
})
187189
}
188-
dispatch(updateUserConfigSettingsAction({ agreements: values }, onSubmitted))
190+
const settings: Record<string, any> = { agreements: values }
191+
if (values.analytics) {
192+
settings.analyticsReason = 'user'
193+
}
194+
dispatch(updateUserConfigSettingsAction(settings, onSubmitted))
189195
}
190196

191197
return (

redisinsight/ui/src/components/oauth/shared/oauth-agreement/OAuthAgreement.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const OAuthAgreement = (props: Props) => {
2222

2323
const handleCheck = (e: ChangeEvent<HTMLInputElement>) => {
2424
if (e.target.checked) {
25-
dispatch(enableUserAnalyticsAction())
25+
dispatch(enableUserAnalyticsAction('oauth-agreement'))
2626
}
2727
dispatch(setAgreement(e.target.checked))
2828
localStorageService.set(BrowserStorageItem.OAuthAgreement, e.target.checked)

redisinsight/ui/src/components/oauth/shared/oauth-form/OAuthForm.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ const OAuthForm = ({
3535
const onSocialButtonClick = (authStrategy: OAuthStrategy) => {
3636
setDisabled(true)
3737
setTimeout(() => { setDisabled(false) }, 1000)
38-
dispatch(enableUserAnalyticsAction())
38+
dispatch(enableUserAnalyticsAction(authStrategy))
3939
setAuthStrategy(authStrategy)
4040
onClick?.(authStrategy)
4141

0 commit comments

Comments
 (0)