Skip to content

add-edit-duplicate segment modal functionality #2320

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
danoswaltCL merged 14 commits into from
Apr 8, 2025
Merged

add-edit-duplicate segment modal functionality #2320

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
bc0c739
add-edit-duplicate segment modal functionality
ppratikcr7 Mar 29, 2025
5f92379
merge dev
danoswaltCL Apr 1, 2025
792a807
modify actions strategy a bit to get this working
danoswaltCL Apr 1, 2025
805cc17
prevent GET segments from triggering close, it breaks the add-list mo…
danoswaltCL Apr 2, 2025
d92d3df
updated at uniformity for new segments UI
ppratikcr7 Apr 2, 2025
226e1af
Merge branch 'dev' into feature/add-segment-modal
danoswaltCL Apr 2, 2025
033c35d
merge dev
danoswaltCL Apr 3, 2025
e7d0328
add duplicate name segment validation in backend and mat-error hookup…
danoswaltCL Apr 4, 2025
48ead21
fix duplicate modal issues, fetch-by-id timing
danoswaltCL Apr 4, 2025
e84c10e
hook up full list details on duplicate create
danoswaltCL Apr 4, 2025
3b08580
disable context on dupe, remove comments
danoswaltCL Apr 7, 2025
ab7e985
merge dev
danoswaltCL Apr 8, 2025
d418305
fix edit dupe name check, use modal event service to close modals
danoswaltCL Apr 8, 2025
2526135
make sure subsegments copied over for upsert
danoswaltCL Apr 8, 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
35 changes: 34 additions & 1 deletion backend/packages/Upgrade/src/api/services/SegmentService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
IMPORT_COMPATIBILITY_TYPE,
ValidatedImportResponse,
SEGMENT_SEARCH_KEY,
DuplicateSegmentNameError,
} from 'upgrade_types';
import { In } from 'typeorm';
import { EntityManager, DataSource } from 'typeorm';
Expand Down Expand Up @@ -249,6 +250,9 @@ export class SegmentService {
case SEGMENT_SEARCH_KEY.NAME:
searchString.push("coalesce(segment.name::TEXT,'')");
break;
case SEGMENT_SEARCH_KEY.STATUS:
searchString.push("coalesce(segment.status::TEXT,'')");
break;
case SEGMENT_SEARCH_KEY.CONTEXT:
searchString.push("coalesce(segment.context::TEXT,'')");
break;
Expand All @@ -257,6 +261,7 @@ export class SegmentService {
break;
default:
searchString.push("coalesce(segment.name::TEXT,'')");
searchString.push("coalesce(feature_flag.status::TEXT,'')");
searchString.push("coalesce(segment.context::TEXT,'')");
searchString.push("coalesce(segment.tags::TEXT,'')");
break;
Expand Down Expand Up @@ -380,7 +385,12 @@ export class SegmentService {
return queryBuilder;
}

public upsertSegment(segment: SegmentInputValidator, logger: UpgradeLogger): Promise<Segment> {
public async upsertSegment(segment: SegmentInputValidator, logger: UpgradeLogger): Promise<Segment> {
// skip dupe check if segment has id, which means it's an update not an add
if (!segment.id) {
await this.checkIsDuplicateSegmentName(segment.name, segment.context, logger);
}

logger.info({ message: `Upsert segment => ${JSON.stringify(segment, undefined, 2)}` });
return this.addSegmentDataInDB(segment, logger);
}
Expand Down Expand Up @@ -987,4 +997,27 @@ export class SegmentService {
}
}
}

async checkIsDuplicateSegmentName(name: string, context: string, logger: UpgradeLogger): Promise<boolean> {
logger.info({ message: `Check for duplicate segment name ${name} in context ${context}` });
const sameNameSegment = await this.segmentRepository.find({ where: { name, context } });

if (sameNameSegment.length) {
logger.error({
message: `Segment name ${name} already exists in context ${context}`,
});

const error: DuplicateSegmentNameError = {
type: SERVER_ERROR.SEGMENT_DUPLICATE_NAME,
message: `Segment name ${name} already exists in context ${context}`,
duplicateName: name,
context,
httpCode: 400,
};

throw error;
} else {
return false;
}
}
}
44 changes: 42 additions & 2 deletions backend/packages/Upgrade/test/unit/services/SegmentService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -421,12 +421,14 @@ describe('Segment Service Testing', () => {
expect(segments).toEqual(ff_include);
});

it('should upsert a segment', async () => {
it('should upsert a segment with id (edit)', async () => {
service.checkIsDuplicateSegmentName = jest.fn().mockResolvedValue(false);
const segments = await service.upsertSegment(segVal, logger);
expect(segments).toEqual(seg1);
});

it('should upsert a segment with no id', async () => {
it('should upsert a segment with no id (add)', async () => {
service.checkIsDuplicateSegmentName = jest.fn().mockResolvedValue(false);
const err = new Error('error');
const segment = new SegmentInputValidator();
segment.subSegmentIds = ['seg1'];
Expand All @@ -437,24 +439,62 @@ describe('Segment Service Testing', () => {
indivRepo.insertIndividualForSegment = jest.fn().mockImplementation(() => {
throw err;
});
repo.find = jest.fn().mockResolvedValue([]);
expect(async () => {
await service.upsertSegment(segment, logger);
}).rejects.toThrow(new Error('Error in creating individualDocs, groupDocs in "addSegmentInDB"'));
});

it('should throw an error if the segment has a duplicate name', async () => {
const dupeError = new Error(
JSON.stringify({
type: SERVER_ERROR.SEGMENT_DUPLICATE_NAME,
message: `Segment name ${segVal.name} already exists in context ${segVal.context}`,
duplicateName: segVal.name,
context: segVal.context, // Fix: Make sure this is a string, not the segment object
httpCode: 400,
})
);

const addSegment = { ...segVal, id: undefined };

// Make sure this mock is on the service instance being tested
service.checkIsDuplicateSegmentName = jest.fn().mockImplementation(() => {
throw dupeError;
});

// Also verify that addSegmentDataInDB isn't being called
service.addSegmentDataInDB = jest.fn();

expect(async () => {
await service.upsertSegment(addSegment, logger);
}).rejects.toThrow(dupeError);

// Verify checkIsDuplicateSegmentName was called with correct parameters
expect(service.checkIsDuplicateSegmentName).toHaveBeenCalledWith(segVal.name, segVal.context, logger);

// Verify addSegmentDataInDB was never called
expect(service.addSegmentDataInDB).not.toHaveBeenCalled();
});

it('should throw an error when unable to delete segment', async () => {
service.checkIsDuplicateSegmentName = jest.fn().mockResolvedValue(false);
const err = new Error('error');
const indivRepo = module.get<IndividualForSegmentRepository>(getRepositoryToken(IndividualForSegmentRepository));
indivRepo.insertIndividualForSegment = jest.fn().mockImplementation(() => {
throw err;
});
repo.find = jest.fn().mockResolvedValue([]);

expect(async () => {
await service.upsertSegment(segVal, logger);
}).rejects.toThrow(err);
});

it('should throw an error when unable to save segment', async () => {
service.checkIsDuplicateSegmentName = jest.fn().mockResolvedValue(false);
const err = new Error('error');
repo.find = jest.fn().mockResolvedValue([]);
repo.save = jest.fn().mockImplementation(() => {
throw err;
});
Expand Down
3 changes: 2 additions & 1 deletion frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class FeatureFlagsEffects {
)
);

// actionCreateFeatureFlag dispatch POST feature flag
// actionAddFeatureFlag dispatch POST feature flag
addFeatureFlag$ = createEffect(() =>
this.actions$.pipe(
ofType(FeatureFlagsActions.actionAddFeatureFlag),
Expand Down Expand Up @@ -334,6 +334,7 @@ export class FeatureFlagsEffects {
)
)
);

exportFeatureFlagsDesign$ = createEffect(() =>
this.actions$.pipe(
ofType(FeatureFlagsActions.actionExportFeatureFlagDesign),
Expand Down
2 changes: 1 addition & 1 deletion frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export interface CoreFeatureFlagDetails {
key: string;
description?: string;
context: string[];
tags: string[];
tags?: string[];
status: FEATURE_FLAG_STATUS;
filterMode: FILTER_MODE;
}
Expand Down
1 change: 1 addition & 0 deletions frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ export class LocalStorageService {
searchString: segmentSearchString || null,
sortKey: (segmentSortKey as SEGMENT_SORT_KEY) || SEGMENT_SORT_KEY.NAME,
sortAs: (segmentSortType as SORT_AS_DIRECTION) || SORT_AS_DIRECTION.ASCENDING,
isLoadingUpsertSegment: false,
};

const state = {
Expand Down
18 changes: 17 additions & 1 deletion frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
import { Inject, Injectable } from '@angular/core';
import { SegmentFile, SegmentInput, SegmentsPaginationInfo, SegmentsPaginationParams } from './store/segments.model';
import {
AddSegmentRequest,
SegmentFile,
SegmentInput,
SegmentsPaginationInfo,
SegmentsPaginationParams,
} from './store/segments.model';
import { Observable } from 'rxjs';
import { HttpClient, HttpParams } from '@angular/common/http';
import { ENV, Environment } from '../../../environments/environment-types';
Expand Down Expand Up @@ -28,6 +34,11 @@ export class SegmentsDataService {
return this.http.post(url, segment);
}

addSegment(segment: AddSegmentRequest) {
const url = this.environment.api.segments;
return this.http.post(url, segment);
}

getSegmentById(id: string) {
const url = `${this.environment.api.segments}/status/${id}`;
return this.http.get(url);
Expand All @@ -43,6 +54,11 @@ export class SegmentsDataService {
return this.http.post(url, segment);
}

modifySegment(segment: AddSegmentRequest) {
const url = this.environment.api.segments;
return this.http.post(url, segment);
}

exportSegments(segmentIds: string[]) {
let ids = new HttpParams();
segmentIds.forEach((id) => {
Expand Down
45 changes: 29 additions & 16 deletions frontend/projects/upgrade/src/app/core/segments/segments.service.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,39 @@ import {
selectExperimentSegmentsExclusion,
selectFeatureFlagSegmentsInclusion,
selectFeatureFlagSegmentsExclusion,
selectSegmentById,
selectSearchString,
selectSearchKey,
selectSortKey,
selectSortAs,
selectSegmentLists,
selectAppContexts,
selectSegmentUsageData,
selectIsLoadingGlobalSegments,
selectGlobalTableState,
selectGlobalSortKey,
selectGlobalSortAs,
isLoadingUpsertSegment,
selectSegmentIdAfterNavigation,
} from './store/segments.selectors';
import {
AddSegmentRequest,
LIST_OPTION_TYPE,
Segment,
SegmentInput,
SegmentLocalStorageKeys,
UpdateSegmentRequest,
UpsertSegmentType,
} from './store/segments.model';
import { filter, map, tap, withLatestFrom } from 'rxjs/operators';
import { Observable, combineLatest } from 'rxjs';
import { filter, map, withLatestFrom } from 'rxjs/operators';
import { BehaviorSubject, Observable, combineLatest } from 'rxjs';
import { SegmentsDataService } from './segments.data.service';
import { SEGMENT_SEARCH_KEY, SORT_AS_DIRECTION, SEGMENT_SORT_KEY } from 'upgrade_types';
import { SEGMENT_SEARCH_KEY, SORT_AS_DIRECTION, SEGMENT_SORT_KEY, DuplicateSegmentNameError } from 'upgrade_types';
import { LocalStorageService } from '../local-storage/local-storage.service';
import { selectShouldUseLegacyUI } from './store/segments.selectors';
import { selectContextMetaData } from '../experiments/store/experiments.selectors';
import { selectSelectedFeatureFlag } from '../feature-flags/store/feature-flags.selectors';
import { CommonTextHelpersService } from '../../shared/services/common-text-helpers.service';
import { actionFetchContextMetaData } from '../experiments/store/experiments.actions';

@Injectable({ providedIn: 'root' })
export class SegmentsService {
Expand All @@ -50,6 +55,7 @@ export class SegmentsService {
) {}

isLoadingSegments$ = this.store$.pipe(select(selectIsLoadingSegments));
isLoadingUpsertSegment$ = this.store$.pipe(select(isLoadingUpsertSegment));
setIsLoadingImportSegment$ = this.store$.pipe(select(selectIsLoadingSegments));
isLoadingGlobalSegments$ = this.store$.pipe(select(selectIsLoadingGlobalSegments));
selectAllSegments$ = this.store$.pipe(select(selectAllSegments));
Expand All @@ -69,11 +75,14 @@ export class SegmentsService {
select(selectSegmentLists),
map((lists) => lists.length)
);
appContexts$ = this.store$.pipe(select(selectAppContexts));
allExperimentSegmentsInclusion$ = this.store$.pipe(select(selectExperimentSegmentsInclusion));
allExperimentSegmentsExclusion$ = this.store$.pipe(select(selectExperimentSegmentsExclusion));
allFeatureFlagSegmentsExclusion$ = this.store$.pipe(select(selectFeatureFlagSegmentsExclusion));
allFeatureFlagSegmentsInclusion$ = this.store$.pipe(select(selectFeatureFlagSegmentsInclusion));
segmentUsageData$ = this.store$.pipe(select(selectSegmentUsageData));
duplicateSegmentNameError$ = new BehaviorSubject<DuplicateSegmentNameError>(null);
selectSegmentIdAfterNavigation$ = this.store$.pipe(select(selectSegmentIdAfterNavigation));

selectSearchSegmentParams(): Observable<Record<string, unknown>> {
return combineLatest([this.selectSearchKey$, this.selectSearchString$]).pipe(
Expand All @@ -82,18 +91,6 @@ export class SegmentsService {
);
}

selectSegmentById(segmentId: string) {
return this.store$.pipe(
select(selectSegmentById, { segmentId }),
tap((segment) => {
if (!segment) {
this.fetchSegmentById(segmentId);
}
}),
map((segment) => ({ ...segment }))
);
}

allSegments$ = this.store$.pipe(
select(selectAllSegments),
filter((allSegments) => !!allSegments),
Expand Down Expand Up @@ -145,6 +142,10 @@ export class SegmentsService {
this.store$.dispatch(SegmentsActions.actionGetSegmentById({ segmentId }));
}

fetchContextMetaData() {
this.store$.dispatch(actionFetchContextMetaData({ isLoadingContextMetaData: true }));
}

isInitialSegmentsLoading() {
return combineLatest(this.store$.pipe(select(selectIsLoadingSegments)), this.allSegments$).pipe(
map(([isLoading, segments]) => !isLoading || !!segments.length)
Expand Down Expand Up @@ -213,11 +214,23 @@ export class SegmentsService {
);
}

addSegment(addSegmentRequest: AddSegmentRequest) {
this.store$.dispatch(SegmentsActions.actionAddSegment({ addSegmentRequest }));
}

modifySegment(updateSegmentRequest: UpdateSegmentRequest) {
this.store$.dispatch(SegmentsActions.actionUpdateSegment({ updateSegmentRequest }));
}

exportSegments(segmentIds: string[]) {
this.store$.dispatch(SegmentsActions.actionExportSegments({ segmentIds }));
}

exportSegmentCSV(segmentIds: string[]): Observable<any> {
return this.segmentsDataService.exportSegmentCSV(segmentIds);
}

setDuplicateSegmentNameError(error: DuplicateSegmentNameError) {
this.duplicateSegmentNameError$.next(error);
}
}
23 changes: 23 additions & 0 deletions frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import { createAction, props } from '@ngrx/store';
import {
AddSegmentRequest,
Segment,
SegmentInput,
UpdateSegmentRequest,
UpsertSegmentType,
experimentSegmentInclusionExclusionData,
featureFlagSegmentInclusionExclusionData,
Expand Down Expand Up @@ -70,6 +72,27 @@ export const actionUpsertSegmentSuccess = createAction(

export const actionUpsertSegmentFailure = createAction('[Segments] Upsert Segment Failure');

export const actionAddSegment = createAction(
'[Segments] Add Segment',
props<{ addSegmentRequest: AddSegmentRequest }>()
);

export const actionAddSegmentSuccess = createAction('[Segments] Add Segment Success', props<{ segment: Segment }>());

export const actionAddSegmentFailure = createAction('[Segments] Add Segment Failure');

export const actionUpdateSegment = createAction(
'[Segments] Update Segment',
props<{ updateSegmentRequest: UpdateSegmentRequest }>()
);

export const actionUpdateSegmentSuccess = createAction(
'[Segments] Update Segment Success',
props<{ segment: Segment }>()
);

export const actionUpdateSegmentFailure = createAction('[Segments] Update Segment Failure');

export const actionGetSegmentById = createAction('[Segments] Get Segment By Id', props<{ segmentId: string }>());

export const actionGetSegmentByIdSuccess = createAction(
Expand Down
Loading
Loading