Skip to content

Commit a81e288

Browse files
author
Artem
committed
#RI-4489 UTests (complete) + analytics + rename feature
1 parent ca85515 commit a81e288

25 files changed

+1005
-80
lines changed

.github/CODEOWNERS

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
# Add reviewers for the most sensitive folders
22
33
4+
/redisinsight/api/config/features-config.json [email protected] [email protected] [email protected]

redisinsight/api/config/features-config.json

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,19 @@
11
{
22
"version": 1,
33
"features": {
4-
"liveRecommendations": {
4+
"insightsRecommendations": {
55
"flag": true,
6-
"perc": [[0, 100]],
6+
"perc": [],
77
"filters": [
88
{
99
"name": "agreements.analytics",
1010
"value": true,
1111
"cond": "eq"
12+
},
13+
{
14+
"name": "config.server.buildType",
15+
"value": "ELECTRON",
16+
"cond": "eq"
1217
}
1318
]
1419
}

redisinsight/api/src/__mocks__/feature.ts

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { Feature } from 'src/modules/feature/model/feature';
1010
import { FeatureEntity } from 'src/modules/feature/entities/feature.entity';
1111
import { mockAppSettings } from 'src/__mocks__/app-settings';
1212
import config from 'src/utils/config';
13+
import { KnownFeatures } from 'src/modules/feature/constants';
1314
import * as defaultConfig from '../../config/features-config.json';
1415

1516
export const mockFeaturesConfigId = '1';
@@ -20,7 +21,7 @@ export const mockControlGroup = '7';
2021
export const mockFeaturesConfigJson = {
2122
version: mockFeaturesConfigVersion,
2223
features: {
23-
liveRecommendations: {
24+
[KnownFeatures.InsightsRecommendations]: {
2425
perc: [[1.25, 8.45]],
2526
flag: true,
2627
filters: [
@@ -37,8 +38,8 @@ export const mockFeaturesConfigJson = {
3738
export const mockFeaturesConfigJsonComplex = {
3839
...mockFeaturesConfigJson,
3940
features: {
40-
liveRecommendations: {
41-
...mockFeaturesConfigJson.features.liveRecommendations,
41+
[KnownFeatures.InsightsRecommendations]: {
42+
...mockFeaturesConfigJson.features[KnownFeatures.InsightsRecommendations],
4243
filters: [
4344
{
4445
or: [
@@ -80,10 +81,12 @@ export const mockFeaturesConfigJsonComplex = {
8081
export const mockFeaturesConfigData = Object.assign(new FeaturesConfigData(), {
8182
...mockFeaturesConfigJson,
8283
features: new Map(Object.entries({
83-
liveRecommendations: Object.assign(new FeatureConfig(), {
84-
...mockFeaturesConfigJson.features.liveRecommendations,
84+
[KnownFeatures.InsightsRecommendations]: Object.assign(new FeatureConfig(), {
85+
...mockFeaturesConfigJson.features[KnownFeatures.InsightsRecommendations],
8586
filters: [
86-
Object.assign(new FeatureConfigFilter(), { ...mockFeaturesConfigJson.features.liveRecommendations.filters[0] }),
87+
Object.assign(new FeatureConfigFilter(), {
88+
...mockFeaturesConfigJson.features[KnownFeatures.InsightsRecommendations].filters[0],
89+
}),
8790
],
8891
}),
8992
})),
@@ -92,8 +95,8 @@ export const mockFeaturesConfigData = Object.assign(new FeaturesConfigData(), {
9295
export const mockFeaturesConfigDataComplex = Object.assign(new FeaturesConfigData(), {
9396
...mockFeaturesConfigJson,
9497
features: new Map(Object.entries({
95-
liveRecommendations: Object.assign(new FeatureConfig(), {
96-
...mockFeaturesConfigJson.features.liveRecommendations,
98+
[KnownFeatures.InsightsRecommendations]: Object.assign(new FeatureConfig(), {
99+
...mockFeaturesConfigJson.features[KnownFeatures.InsightsRecommendations],
97100
filters: [
98101
Object.assign(new FeatureConfigFilterOr(), {
99102
or: [
@@ -153,13 +156,18 @@ export const mockFeaturesConfigEntityComplex = Object.assign(new FeaturesConfigE
153156
});
154157

155158
export const mockFeature = Object.assign(new Feature(), {
156-
name: 'liveRecommendations',
159+
name: KnownFeatures.InsightsRecommendations,
160+
flag: true,
161+
});
162+
163+
export const mockUnknownFeature = Object.assign(new Feature(), {
164+
name: 'unknown',
157165
flag: true,
158166
});
159167

160168
export const mockFeatureEntity = Object.assign(new FeatureEntity(), {
161169
id: 'lr-1',
162-
name: 'liveRecommendations',
170+
name: KnownFeatures.InsightsRecommendations,
163171
flag: true,
164172
});
165173

@@ -175,10 +183,37 @@ export const mockFeaturesConfigRepository = jest.fn(() => ({
175183
update: jest.fn().mockResolvedValue(mockFeaturesConfig),
176184
}));
177185

178-
export const mockFeaturesConfigService = () => ({
186+
export const mockFeatureRepository = jest.fn(() => ({
187+
get: jest.fn().mockResolvedValue(mockFeature),
188+
upsert: jest.fn().mockResolvedValue({ updated: 1 }),
189+
list: jest.fn().mockResolvedValue([mockFeature]),
190+
delete: jest.fn().mockResolvedValue({ deleted: 1 }),
191+
}));
192+
193+
export const mockFeaturesConfigService = jest.fn(() => ({
179194
sync: jest.fn(),
180195
getControlInfo: jest.fn().mockResolvedValue({
181196
controlNumber: mockControlNumber,
182197
controlGroup: mockControlGroup,
183198
}),
184-
});
199+
}));
200+
201+
export const mockFeatureService = jest.fn(() => ({
202+
isFeatureEnabled: jest.fn().mockResolvedValue(true),
203+
}));
204+
205+
export const mockFeatureAnalytics = jest.fn(() => ({
206+
sendFeatureFlagConfigUpdated: jest.fn(),
207+
sendFeatureFlagConfigUpdateError: jest.fn(),
208+
sendFeatureFlagInvalidRemoteConfig: jest.fn(),
209+
sendFeatureFlagRecalculated: jest.fn(),
210+
}));
211+
212+
export const mockInsightsRecommendationsFlagStrategy = {
213+
calculate: jest.fn().mockResolvedValue(true),
214+
};
215+
216+
export const mockFeatureFlagProvider = jest.fn(() => ({
217+
getStrategy: jest.fn().mockResolvedValue(mockInsightsRecommendationsFlagStrategy),
218+
calculate: jest.fn().mockResolvedValue(true),
219+
}));

redisinsight/api/src/constants/telemetry-events.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,12 @@ export enum TelemetryEvents {
6464
// Bulk Actions
6565
BulkActionsStarted = 'BULK_ACTIONS_STARTED',
6666
BulkActionsStopped = 'BULK_ACTIONS_STOPPED',
67+
68+
// Feature
69+
FeatureFlagConfigUpdated = 'FEATURE_FLAG_CONFIG_UPDATED',
70+
FeatureFlagConfigUpdateError = 'FEATURE_FLAG_CONFIG_UPDATE_ERROR',
71+
FeatureFlagInvalidRemoteConfig = 'FEATURE_FLAG_INVALID_REMOTE_CONFIG',
72+
FeatureFlagRecalculated = 'FEATURE_FLAG_RECALCULATED',
6773
}
6874

6975
export enum CommandType {

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

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { Test, TestingModule } from '@nestjs/testing';
22
import {
33
mockAppSettings,
4-
mockAppSettingsWithoutPermissions,
4+
mockAppSettingsWithoutPermissions, mockControlGroup, mockControlNumber,
55
mockSettingsService,
66
MockType,
77
} from 'src/__mocks__';
@@ -53,7 +53,13 @@ describe('AnalyticsService', () => {
5353

5454
describe('initialize', () => {
5555
it('should set anonymousId', () => {
56-
service.initialize({ anonymousId: mockAnonymousId, sessionId, appType: AppType.Electron });
56+
service.initialize({
57+
anonymousId: mockAnonymousId,
58+
sessionId,
59+
appType: AppType.Electron,
60+
controlNumber: mockControlNumber,
61+
controlGroup: mockControlGroup,
62+
});
5763

5864
const anonymousId = service.getAnonymousId();
5965

@@ -64,7 +70,13 @@ describe('AnalyticsService', () => {
6470
describe('sendEvent', () => {
6571
beforeEach(() => {
6672
mockAnalyticsTrack = jest.fn();
67-
service.initialize({ anonymousId: mockAnonymousId, sessionId, appType: AppType.Electron });
73+
service.initialize({
74+
anonymousId: mockAnonymousId,
75+
sessionId,
76+
appType: AppType.Electron,
77+
controlNumber: mockControlNumber,
78+
controlGroup: mockControlGroup,
79+
});
6880
});
6981
it('should send event with anonymousId if permission are granted', async () => {
7082
settingsService.getAppSettings.mockResolvedValue(mockAppSettings);
@@ -81,6 +93,8 @@ describe('AnalyticsService', () => {
8193
event: TelemetryEvents.ApplicationStarted,
8294
properties: {
8395
buildType: AppType.Electron,
96+
controlNumber: mockControlNumber,
97+
controlGroup: mockControlGroup,
8498
},
8599
});
86100
});
@@ -110,6 +124,8 @@ describe('AnalyticsService', () => {
110124
event: TelemetryEvents.ApplicationStarted,
111125
properties: {
112126
buildType: AppType.Electron,
127+
controlNumber: mockControlNumber,
128+
controlGroup: mockControlGroup,
113129
},
114130
});
115131
});

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ export interface ITelemetryInitEvent {
1919
anonymousId: string;
2020
sessionId: number;
2121
appType: string;
22-
controlGroup: number;
22+
controlNumber: number;
23+
controlGroup: string;
2324
}
2425

2526
@Injectable()
@@ -30,7 +31,9 @@ export class AnalyticsService {
3031

3132
private appType: string = 'unknown';
3233

33-
private controlGroup: number = -1;
34+
private controlNumber: number = -1;
35+
36+
private controlGroup: string = '-1';
3437

3538
private analytics;
3639

@@ -44,11 +47,14 @@ export class AnalyticsService {
4447

4548
@OnEvent(AppAnalyticsEvents.Initialize)
4649
public initialize(payload: ITelemetryInitEvent) {
47-
const { anonymousId, sessionId, appType, controlGroup } = payload;
50+
const {
51+
anonymousId, sessionId, appType, controlNumber, controlGroup,
52+
} = payload;
4853
this.sessionId = sessionId;
4954
this.anonymousId = anonymousId;
5055
this.appType = appType;
5156
this.controlGroup = controlGroup;
57+
this.controlNumber = controlNumber;
5258
this.analytics = new Analytics(ANALYTICS_CONFIG.writeKey, {
5359
flushInterval: ANALYTICS_CONFIG.flushInterval,
5460
});
@@ -79,6 +85,7 @@ export class AnalyticsService {
7985
properties: {
8086
...eventData,
8187
buildType: this.appType,
88+
controlNumber: this.controlNumber,
8289
controlGroup: this.controlGroup,
8390
},
8491
});

redisinsight/api/src/modules/database-recommendation/scanner/recommendations.scanner.spec.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { Test, TestingModule } from '@nestjs/testing';
22
import { RecommendationScanner } from 'src/modules/database-recommendation/scanner/recommendations.scanner';
33
import { RecommendationProvider } from 'src/modules/database-recommendation/scanner/recommendation.provider';
4+
import { FeatureService } from 'src/modules/feature/feature.service';
5+
import { mockFeatureService, MockType } from 'src/__mocks__';
46

57
const mockRecommendationStrategy = () => ({
68
isRecommendationReached: jest.fn(),
@@ -16,6 +18,7 @@ describe('RecommendationScanner', () => {
1618
let service: RecommendationScanner;
1719
let recommendationProvider;
1820
let recommendationStrategy;
21+
let featureService: MockType<FeatureService>;
1922

2023
beforeEach(async () => {
2124
const module: TestingModule = await Test.createTestingModule({
@@ -25,11 +28,16 @@ describe('RecommendationScanner', () => {
2528
provide: RecommendationProvider,
2629
useFactory: mockRecommendationProvider,
2730
},
31+
{
32+
provide: FeatureService,
33+
useFactory: mockFeatureService,
34+
},
2835
],
2936
}).compile();
3037

3138
service = module.get<RecommendationScanner>(RecommendationScanner);
3239
recommendationProvider = module.get<RecommendationProvider>(RecommendationProvider);
40+
featureService = module.get(FeatureService);
3341
recommendationStrategy = mockRecommendationStrategy();
3442
recommendationProvider.getStrategy.mockReturnValue(recommendationStrategy);
3543
});
@@ -43,6 +51,16 @@ describe('RecommendationScanner', () => {
4351
})).toEqual({ name: 'name' });
4452
});
4553

54+
it('should return null when feature disabled', async () => {
55+
featureService.isFeatureEnabled.mockResolvedValueOnce(false);
56+
57+
recommendationStrategy.isRecommendationReached.mockResolvedValue({ isReached: true });
58+
59+
expect(await service.determineRecommendation('name', {
60+
data: mockData,
61+
})).toEqual(null);
62+
});
63+
4664
it('should return null when isRecommendationReached throw error', async () => {
4765
recommendationStrategy.isRecommendationReached.mockRejectedValueOnce(new Error());
4866

redisinsight/api/src/modules/database-recommendation/scanner/recommendations.scanner.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
11
import { Injectable } from '@nestjs/common';
22
import { RecommendationProvider } from 'src/modules/database-recommendation/scanner/recommendation.provider';
3+
import { FeatureService } from 'src/modules/feature/feature.service';
4+
import { KnownFeatures } from 'src/modules/feature/constants';
35

46
@Injectable()
57
export class RecommendationScanner {
68
constructor(
79
private readonly recommendationProvider: RecommendationProvider,
10+
private readonly featureService: FeatureService,
811
) {}
912

1013
async determineRecommendation(name: string, data: any) {
14+
if (!await this.featureService.isFeatureEnabled(KnownFeatures.InsightsRecommendations)) {
15+
return null;
16+
}
17+
1118
const strategy = this.recommendationProvider.getStrategy(name);
1219
try {
1320
const recommendation = await strategy.isRecommendationReached(data);

redisinsight/api/src/modules/feature/constants/index.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,18 @@ export enum FeatureStorage {
1111
Env = 'env',
1212
Database = 'database',
1313
}
14+
export enum FeatureConfigConfigDestination {
15+
Default = 'default',
16+
Remote = 'remote',
17+
}
1418

15-
export enum FeatureRecalculationStrategy {
16-
LiveRecommendation = 'liveRecommendation',
19+
export enum KnownFeatures {
20+
InsightsRecommendations = 'insightsRecommendations',
1721
}
1822

1923
export const knownFeatures = [
2024
{
21-
name: 'liveRecommendations',
25+
name: KnownFeatures.InsightsRecommendations,
2226
storage: FeatureStorage.Database,
2327
},
2428
];
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
export * from './unable-to-fetch-remote-config.exception';

0 commit comments

Comments
 (0)