Skip to content
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.

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

Include feature flags in segment status #1974

Merged
merged 4 commits into from
Sep 24, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class FeatureFlagSegmentExclusionRepository extends Repository<FeatureFla
.leftJoin('featureFlagSegmentExclusion.segment', 'segment')
.leftJoinAndSelect('segment.subSegments', 'subSegments')
.addSelect('featureFlag.name')
.addSelect('featureFlag.state')
.addSelect('featureFlag.status')
.addSelect('featureFlag.context')
.addSelect('segment.id')
.getMany()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class FeatureFlagSegmentInclusionRepository extends Repository<FeatureFla
.leftJoin('featureFlagSegmentInclusion.segment', 'segment')
.leftJoinAndSelect('segment.subSegments', 'subSegments')
.addSelect('featureFlag.name')
.addSelect('featureFlag.state')
.addSelect('featureFlag.status')
.addSelect('featureFlag.context')
.addSelect('segment.id')
.getMany()
Expand Down
41 changes: 40 additions & 1 deletion backend/packages/Upgrade/src/api/services/SegmentService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ import {
} from '../controllers/validators/SegmentInputValidator';
import { ExperimentSegmentExclusionRepository } from '../repositories/ExperimentSegmentExclusionRepository';
import { ExperimentSegmentInclusionRepository } from '../repositories/ExperimentSegmentInclusionRepository';
import { FeatureFlagSegmentExclusionRepository } from '../repositories/FeatureFlagSegmentExclusionRepository';
import { FeatureFlagSegmentInclusionRepository } from '../repositories/FeatureFlagSegmentInclusionRepository';
import { getSegmentData } from '../controllers/SegmentController';
import { globalExcludeSegment } from '../../init/seed/globalExcludeSegment';
import { CacheService } from './CacheService';
Expand Down Expand Up @@ -62,6 +64,10 @@ export class SegmentService {
private experimentSegmentExclusionRepository: ExperimentSegmentExclusionRepository,
@InjectRepository()
private experimentSegmentInclusionRepository: ExperimentSegmentInclusionRepository,
@InjectRepository()
private featureFlagSegmentExclusionRepository: FeatureFlagSegmentExclusionRepository,
@InjectRepository()
private featureFlagSegmentInclusionRepository: FeatureFlagSegmentInclusionRepository,
private cacheService: CacheService
) {}

Expand Down Expand Up @@ -145,9 +151,16 @@ export class SegmentService {
public async getSegmentStatus(segmentsData: Segment[]): Promise<getSegmentData> {
const connection = this.dataSource.manager.connection;
const segmentsDataWithStatus = await connection.transaction(async () => {
const [allExperimentSegmentsInclusion, allExperimentSegmentsExclusion] = await Promise.all([
const [
allExperimentSegmentsInclusion,
allExperimentSegmentsExclusion,
allFeatureFlagSegmentsInclusion,
allFeatureFlagSegmentsExclusion,
] = await Promise.all([
this.getExperimentSegmentInclusionData(),
this.getExperimentSegmentExclusionData(),
this.getFeatureFlagSegmentInclusionData(),
this.getFeatureFlagSegmentExclusionData(),
]);

const segmentMap = new Map<string, string[]>();
Expand Down Expand Up @@ -183,6 +196,20 @@ export class SegmentService {
});
}

if (allFeatureFlagSegmentsInclusion) {
allFeatureFlagSegmentsInclusion.forEach((ele) => {
collectSegmentIds(ele.segment.id);
ele.segment.subSegments.forEach((subSegment) => collectSegmentIds(subSegment.id));
});
}

if (allFeatureFlagSegmentsExclusion) {
allFeatureFlagSegmentsExclusion.forEach((ele) => {
collectSegmentIds(ele.segment.id);
ele.segment.subSegments.forEach((subSegment) => collectSegmentIds(subSegment.id));
});
}

const segmentsDataWithStatus = segmentsData.map((segment) => {
if (segment.id === globalExcludeSegment.id) {
return { ...segment, status: SEGMENT_STATUS.GLOBAL };
Expand All @@ -197,6 +224,8 @@ export class SegmentService {
segmentsData: segmentsDataWithStatus,
experimentSegmentInclusionData: allExperimentSegmentsInclusion,
experimentSegmentExclusionData: allExperimentSegmentsExclusion,
featureFlagSegmentInclusionData: allFeatureFlagSegmentsInclusion,
featureFlagSegmentExclusionData: allFeatureFlagSegmentsExclusion,
};
});

Expand All @@ -213,6 +242,16 @@ export class SegmentService {
return queryBuilder;
}

public async getFeatureFlagSegmentExclusionData() {
const queryBuilder = await this.featureFlagSegmentExclusionRepository.getFeatureFlagSegmentExclusionData();
return queryBuilder;
}

public async getFeatureFlagSegmentInclusionData() {
const queryBuilder = await this.featureFlagSegmentInclusionRepository.getFeatureFlagSegmentInclusionData();
return queryBuilder;
}

public upsertSegment(segment: SegmentInputValidator, logger: UpgradeLogger): Promise<Segment> {
logger.info({ message: `Upsert segment => ${JSON.stringify(segment, undefined, 2)}` });
return this.addSegmentDataInDB(segment, logger);
Expand Down
71 changes: 65 additions & 6 deletions backend/packages/Upgrade/test/unit/services/SegmentService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,32 @@ import { IndividualForSegmentRepository } from '../../../src/api/repositories/In
import { GroupForSegmentRepository } from '../../../src/api/repositories/GroupForSegmentRepository';
import { ExperimentSegmentExclusionRepository } from '../../../src/api/repositories/ExperimentSegmentExclusionRepository';
import { ExperimentSegmentInclusionRepository } from '../../../src/api/repositories/ExperimentSegmentInclusionRepository';
import { FeatureFlagSegmentExclusionRepository } from '../../../src/api/repositories/FeatureFlagSegmentExclusionRepository';
import { FeatureFlagSegmentInclusionRepository } from '../../../src/api/repositories/FeatureFlagSegmentInclusionRepository';
import { CacheService } from '../../../src/api/services/CacheService';
import { SegmentFile, SegmentInputValidator } from '../../../src/api/controllers/validators/SegmentInputValidator';
import { IndividualForSegment } from '../../../src/api/models/IndividualForSegment';
import { GroupForSegment } from '../../../src/api/models/GroupForSegment';
import { Experiment } from '../../../src/api/models/Experiment';
import { SEGMENT_TYPE, SERVER_ERROR, EXPERIMENT_STATE } from 'upgrade_types';
import { FeatureFlag } from '../../../src/api/models/FeatureFlag';
import { SEGMENT_TYPE, SERVER_ERROR, EXPERIMENT_STATE, FEATURE_FLAG_STATUS } from 'upgrade_types';
import { configureLogger } from '../../utils/logger';
import { ExperimentSegmentExclusion } from '../../../src/api/models/ExperimentSegmentExclusion';
import { ExperimentSegmentInclusion } from '../../../src/api/models/ExperimentSegmentInclusion';
import { FeatureFlagSegmentExclusion } from '../../../src/api/models/FeatureFlagSegmentExclusion';
import { FeatureFlagSegmentInclusion } from '../../../src/api/models/FeatureFlagSegmentInclusion';
import { Container } from '../../../src/typeorm-typedi-extensions';

const exp = new Experiment();
const ff = new FeatureFlag();
const seg2 = new Segment();
const seg1 = new Segment();
const segValSegment = new Segment();
const logger = new UpgradeLogger();
const segmentArr = [seg1, seg2];
const segVal = new SegmentInputValidator();
const include = [{ segment: seg1, experiment: exp }];
const ff_include = [{ segment: seg1, featureFlag: ff }];
const segValImportFile: SegmentFile = {
fileName: 'seg1.json',
fileContent: '',
Expand Down Expand Up @@ -65,6 +72,8 @@ describe('Segment Service Testing', () => {
Segment,
ExperimentSegmentExclusion,
ExperimentSegmentInclusion,
FeatureFlagSegmentExclusion,
FeatureFlagSegmentInclusion,
],
synchronize: true,
});
Expand All @@ -75,6 +84,8 @@ describe('Segment Service Testing', () => {

exp.id = 'exp1';
exp.state = EXPERIMENT_STATE.ENROLLING;
ff.id = 'ff1';
ff.status = FEATURE_FLAG_STATUS.ENABLED;
seg2.subSegments = [];
seg2.id = 'seg2';
seg2.subSegments = [];
Expand Down Expand Up @@ -133,6 +144,8 @@ describe('Segment Service Testing', () => {
GroupForSegmentRepository,
ExperimentSegmentExclusionRepository,
ExperimentSegmentInclusionRepository,
FeatureFlagSegmentExclusionRepository,
FeatureFlagSegmentInclusionRepository,
CacheService,
SegmentRepository,
{
Expand Down Expand Up @@ -190,6 +203,36 @@ describe('Segment Service Testing', () => {
})),
},
},
{
provide: getRepositoryToken(FeatureFlagSegmentExclusionRepository),
useValue: {
findOne: jest.fn().mockResolvedValue(seg1),
save: jest.fn().mockResolvedValue(seg1),
delete: jest.fn(),
getFeatureFlagSegmentExclusionData: jest.fn().mockResolvedValue(ff_include),
createQueryBuilder: jest.fn(() => ({
insert: jest.fn().mockReturnThis(),
leftJoinAndSelect: jest.fn().mockReturnThis(),
where: jest.fn().mockReturnThis(),
getMany: jest.fn().mockResolvedValue(segmentArr),
})),
},
},
{
provide: getRepositoryToken(FeatureFlagSegmentInclusionRepository),
useValue: {
findOne: jest.fn().mockResolvedValue(seg1),
save: jest.fn().mockResolvedValue(seg1),
delete: jest.fn(),
getFeatureFlagSegmentInclusionData: jest.fn().mockResolvedValue(ff_include),
createQueryBuilder: jest.fn(() => ({
insert: jest.fn().mockReturnThis(),
leftJoinAndSelect: jest.fn().mockReturnThis(),
where: jest.fn().mockReturnThis(),
getMany: jest.fn().mockResolvedValue(segmentArr),
})),
},
},
{
provide: getRepositoryToken(IndividualForSegmentRepository),
useValue: {
Expand Down Expand Up @@ -254,7 +297,7 @@ describe('Segment Service Testing', () => {
expect(segments).toEqual([seg1]);
});

it('should return all segments with status for enrolling experiments', async () => {
it('should return all segments with status for enrolling experiments and feature flags', async () => {
const res = {
segmentsData: [
{
Expand All @@ -269,12 +312,14 @@ describe('Segment Service Testing', () => {
],
experimentSegmentExclusionData: [{ experiment: exp, segment: seg1 }],
experimentSegmentInclusionData: [{ experiment: exp, segment: seg1 }],
featureFlagSegmentExclusionData: [{ featureFlag: ff, segment: seg1 }],
featureFlagSegmentInclusionData: [{ featureFlag: ff, segment: seg1 }],
};
const segments = await service.getAllSegmentWithStatus(logger);
expect(segments).toEqual(res);
});

it('should return all segments with status for not enrolling experiments', async () => {
it('should return all segments with status for not enrolling experiments and feature flags', async () => {
exp.state = EXPERIMENT_STATE.ENROLLMENT_COMPLETE;
const res = {
segmentsData: [
Expand All @@ -290,6 +335,8 @@ describe('Segment Service Testing', () => {
],
experimentSegmentExclusionData: [{ experiment: exp, segment: seg1 }],
experimentSegmentInclusionData: [{ experiment: exp, segment: seg1 }],
featureFlagSegmentExclusionData: [{ featureFlag: ff, segment: seg1 }],
featureFlagSegmentInclusionData: [{ featureFlag: ff, segment: seg1 }],
};
const segments = await service.getAllSegmentWithStatus(logger);
expect(segments).toEqual(res);
Expand Down Expand Up @@ -324,21 +371,33 @@ describe('Segment Service Testing', () => {
],
experimentSegmentExclusionData: [{ experiment: exp, segment: seg1 }],
experimentSegmentInclusionData: [{ experiment: exp, segment: seg1 }],
featureFlagSegmentExclusionData: [{ featureFlag: ff, segment: seg1 }],
featureFlagSegmentInclusionData: [{ featureFlag: ff, segment: seg1 }],
};
const segments = await service.getAllSegmentWithStatus(logger);
expect(segments).toEqual(res);
});

it('should return segment exclusion data', async () => {
it('should return experiment segment exclusion data', async () => {
const segments = await service.getExperimentSegmentExclusionData();
expect(segments).toEqual(include);
});

it('should return segment inclusion data', async () => {
it('should return experiment segment inclusion data', async () => {
const segments = await service.getExperimentSegmentInclusionData();
expect(segments).toEqual(include);
});

it('should return feture flag segment exclusion data', async () => {
const segments = await service.getFeatureFlagSegmentExclusionData();
expect(segments).toEqual(ff_include);
});

it('should return feature flag segment inclusion data', async () => {
const segments = await service.getFeatureFlagSegmentInclusionData();
expect(segments).toEqual(ff_include);
});

it('should upsert a segment', async () => {
const segments = await service.upsertSegment(segVal, logger);
expect(segments).toEqual(seg1);
Expand Down Expand Up @@ -426,4 +485,4 @@ describe('Segment Service Testing', () => {
await service.exportSegments([seg1.id], logger);
}).rejects.toThrow(new Error(SERVER_ERROR.QUERY_FAILED));
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ export class LocalStorageService {
isLoadingSegments: false,
allExperimentSegmentsInclusion: null,
allExperimentSegmentsExclusion: null,
allFeatureFlagSegmentsInclusion: null,
allFeatureFlagSegmentsExclusion: null,
searchKey: segmentSearchKey as SEGMENT_SEARCH_KEY,
searchString: segmentSearchString || null,
sortKey: (segmentSortKey as SEGMENT_SORT_KEY) || SEGMENT_SORT_KEY.NAME,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import {
selectSelectedSegment,
selectExperimentSegmentsInclusion,
selectExperimentSegmentsExclusion,
selectFeatureFlagSegmentsInclusion,
selectFeatureFlagSegmentsExclusion,
selectSegmentById,
selectSearchString,
selectSearchKey,
Expand Down Expand Up @@ -47,6 +49,8 @@ export class SegmentsService {
selectSegmentSortAs$ = this.store$.pipe(select(selectSortAs));
allExperimentSegmentsInclusion$ = this.store$.pipe(select(selectExperimentSegmentsInclusion));
allExperimentSegmentsExclusion$ = this.store$.pipe(select(selectExperimentSegmentsExclusion));
allFeatureFlagSegmentsExclusion$ = this.store$.pipe(select(selectFeatureFlagSegmentsExclusion));
allFeatureFlagSegmentsInclusion$ = this.store$.pipe(select(selectFeatureFlagSegmentsInclusion));

selectSearchSegmentParams(): Observable<Record<string, unknown>> {
return combineLatest([this.selectSearchKey$, this.selectSearchString$]).pipe(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { createAction, props } from '@ngrx/store';
import { Segment, SegmentInput, UpsertSegmentType, experimentSegmentInclusionExclusionData } from './segments.model';
import {
Segment,
SegmentInput,
UpsertSegmentType,
experimentSegmentInclusionExclusionData,
featureFlagSegmentInclusionExclusionData,
} from './segments.model';
import {
SEGMENT_SEARCH_KEY,
SORT_AS_DIRECTION,
Expand All @@ -14,6 +20,8 @@ export const actionFetchSegmentsSuccess = createAction(
segments: Segment[];
experimentSegmentInclusion: experimentSegmentInclusionExclusionData[];
experimentSegmentExclusion: experimentSegmentInclusionExclusionData[];
featureFlagSegmentInclusion: featureFlagSegmentInclusionExclusionData[];
featureFlagSegmentExclusion: featureFlagSegmentInclusionExclusionData[];
}>()
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ describe('SegmentsEffects', () => {
segmentsData: [{ ...mockSegment }],
experimentSegmentInclusionData: [],
experimentSegmentExclusionData: [],
featureFlagSegmentInclusionData: [],
featureFlagSegmentExclusionData: [],
})
);
selectAllSegments.setResult([{ ...mockSegment }]);
Expand All @@ -69,6 +71,8 @@ describe('SegmentsEffects', () => {
segments: [{ ...mockSegment }],
experimentSegmentInclusion: [],
experimentSegmentExclusion: [],
featureFlagSegmentInclusion: [],
featureFlagSegmentExclusion: [],
});

service.fetchSegments$.subscribe((result) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ export class SegmentsEffects {
segments: data.segmentsData,
experimentSegmentInclusion: data.experimentSegmentInclusionData,
experimentSegmentExclusion: data.experimentSegmentExclusionData,
featureFlagSegmentInclusion: data.featureFlagSegmentInclusionData,
featureFlagSegmentExclusion: data.featureFlagSegmentExclusionData,
})
),
catchError(() => [SegmentsActions.actionFetchSegmentsFailure()])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,21 @@ export interface experimentSegmentInclusionExclusionData {
};
}

export interface featureFlagSegmentInclusionExclusionData {
createdAt: string;
updatedAt: string;
versionNumber: number;
featureFlag: {
name: string;
context: any[];
status: string;
};
segment: {
id: string;
subSegments: any[];
};
}

export interface Group {
groupId: string;
type: string;
Expand Down Expand Up @@ -107,6 +122,8 @@ export interface SegmentState extends EntityState<Segment> {
// TODO: remove any
allExperimentSegmentsInclusion: any;
allExperimentSegmentsExclusion: any;
allFeatureFlagSegmentsInclusion: any;
allFeatureFlagSegmentsExclusion: any;
searchKey: SEGMENT_SEARCH_KEY;
searchString: string;
sortKey: SEGMENT_SORT_KEY;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ describe('SegmentsReducer', () => {
segments: [],
experimentSegmentExclusion: [],
experimentSegmentInclusion: [],
featureFlagSegmentExclusion: [],
featureFlagSegmentInclusion: [],
});

const newState = segmentsReducer(previousState, testAction);
Expand Down
Loading
Loading