diff --git a/backend/packages/Upgrade/src/api/services/SegmentService.ts b/backend/packages/Upgrade/src/api/services/SegmentService.ts index 4ccdf80d6f..6573b3dea7 100644 --- a/backend/packages/Upgrade/src/api/services/SegmentService.ts +++ b/backend/packages/Upgrade/src/api/services/SegmentService.ts @@ -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'; @@ -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; @@ -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; @@ -380,7 +385,12 @@ export class SegmentService { return queryBuilder; } - public upsertSegment(segment: SegmentInputValidator, logger: UpgradeLogger): Promise { + public async upsertSegment(segment: SegmentInputValidator, logger: UpgradeLogger): Promise { + // 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); } @@ -987,4 +997,27 @@ export class SegmentService { } } } + + async checkIsDuplicateSegmentName(name: string, context: string, logger: UpgradeLogger): Promise { + 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; + } + } } diff --git a/backend/packages/Upgrade/test/unit/services/SegmentService.test.ts b/backend/packages/Upgrade/test/unit/services/SegmentService.test.ts index e0986d2c12..c892e017ec 100644 --- a/backend/packages/Upgrade/test/unit/services/SegmentService.test.ts +++ b/backend/packages/Upgrade/test/unit/services/SegmentService.test.ts @@ -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']; @@ -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(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; }); diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts index 646edfd029..493496949f 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts @@ -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), @@ -334,6 +334,7 @@ export class FeatureFlagsEffects { ) ) ); + exportFeatureFlagsDesign$ = createEffect(() => this.actions$.pipe( ofType(FeatureFlagsActions.actionExportFeatureFlagDesign), diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts index 311df9b2ff..e3326b88df 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts @@ -23,7 +23,7 @@ export interface CoreFeatureFlagDetails { key: string; description?: string; context: string[]; - tags: string[]; + tags?: string[]; status: FEATURE_FLAG_STATUS; filterMode: FILTER_MODE; } diff --git a/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts b/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts index 9d954b5387..91cddf6650 100755 --- a/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts +++ b/frontend/projects/upgrade/src/app/core/local-storage/local-storage.service.ts @@ -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 = { diff --git a/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts b/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts index a91714f924..8780e82e2e 100644 --- a/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts +++ b/frontend/projects/upgrade/src/app/core/segments/segments.data.service.ts @@ -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'; @@ -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); @@ -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) => { diff --git a/frontend/projects/upgrade/src/app/core/segments/segments.service.ts b/frontend/projects/upgrade/src/app/core/segments/segments.service.ts index c8257f2a82..4a966e63d5 100644 --- a/frontend/projects/upgrade/src/app/core/segments/segments.service.ts +++ b/frontend/projects/upgrade/src/app/core/segments/segments.service.ts @@ -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 { @@ -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)); @@ -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(null); + selectSegmentIdAfterNavigation$ = this.store$.pipe(select(selectSegmentIdAfterNavigation)); selectSearchSegmentParams(): Observable> { return combineLatest([this.selectSearchKey$, this.selectSearchString$]).pipe( @@ -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), @@ -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) @@ -213,6 +214,14 @@ 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 })); } @@ -220,4 +229,8 @@ export class SegmentsService { exportSegmentCSV(segmentIds: string[]): Observable { return this.segmentsDataService.exportSegmentCSV(segmentIds); } + + setDuplicateSegmentNameError(error: DuplicateSegmentNameError) { + this.duplicateSegmentNameError$.next(error); + } } diff --git a/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts b/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts index 32d161c3da..866155a6ab 100644 --- a/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts +++ b/frontend/projects/upgrade/src/app/core/segments/store/segments.actions.ts @@ -1,7 +1,9 @@ import { createAction, props } from '@ngrx/store'; import { + AddSegmentRequest, Segment, SegmentInput, + UpdateSegmentRequest, UpsertSegmentType, experimentSegmentInclusionExclusionData, featureFlagSegmentInclusionExclusionData, @@ -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( diff --git a/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.spec.ts b/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.spec.ts index 83945a6b3f..f1c2d6f192 100644 --- a/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.spec.ts +++ b/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.spec.ts @@ -6,12 +6,15 @@ import { SegmentsEffects } from './segments.effects'; import { Segment, SegmentFile, SegmentInput, UpsertSegmentType } from './segments.model'; import { selectAllSegments } from './segments.selectors'; import * as SegmentsActions from './segments.actions'; +import { CommonModalEventsService } from '../../../shared/services/common-modal-event.service'; describe('SegmentsEffects', () => { let store$: any; let actions$: ActionsSubject; let segmentsDataService: any; + let segmentService: any; let router: any; + let commonModalEventService: any; let service: SegmentsEffects; const mockSegment: Segment = { createdAt: 'test', @@ -48,11 +51,21 @@ describe('SegmentsEffects', () => { store$ = new BehaviorSubject({}); store$.dispatch = jest.fn(); segmentsDataService = {}; + commonModalEventService = { + forceCloseModal: jest.fn(), + }; router = { navigate: jest.fn(), }; - service = new SegmentsEffects(store$, actions$, segmentsDataService, router); + service = new SegmentsEffects( + store$, + actions$, + segmentsDataService, + segmentService, + router, + commonModalEventService + ); }); describe('fetchAllSegments$', () => { diff --git a/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.ts b/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.ts index d87f8ea89e..06c7c9dbd7 100644 --- a/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.ts +++ b/frontend/projects/upgrade/src/app/core/segments/store/segments.effects.ts @@ -2,7 +2,7 @@ import { Injectable } from '@angular/core'; import { Router } from '@angular/router'; import { Actions, createEffect, ofType } from '@ngrx/effects'; import { select, Store } from '@ngrx/store'; -import { catchError, filter, first, map, switchMap, tap, withLatestFrom } from 'rxjs/operators'; +import { catchError, concatMap, filter, first, map, switchMap, tap, withLatestFrom } from 'rxjs/operators'; import { AppState } from '../../core.module'; import { SegmentsDataService } from '../segments.data.service'; import * as SegmentsActions from './segments.actions'; @@ -19,6 +19,10 @@ import { selectTotalSegments, } from './segments.selectors'; import JSZip from 'jszip'; +import { of } from 'rxjs'; +import { SERVER_ERROR } from 'upgrade_types'; +import { SegmentsService } from '../segments.service'; +import { CommonModalEventsService } from '../../../shared/services/common-modal-event.service'; @Injectable() export class SegmentsEffects { @@ -26,7 +30,9 @@ export class SegmentsEffects { private store$: Store, private actions$: Actions, private segmentsDataService: SegmentsDataService, - private router: Router + private segmentsService: SegmentsService, + private router: Router, + private commonModalEventService: CommonModalEventsService ) {} fetchSegmentsPaginated$ = createEffect(() => @@ -176,6 +182,46 @@ export class SegmentsEffects { ) ); + addSegment$ = createEffect(() => + this.actions$.pipe( + ofType(SegmentsActions.actionAddSegment), + switchMap((action) => { + return this.segmentsDataService.addSegment(action.addSegmentRequest).pipe( + map((response: Segment) => { + this.commonModalEventService.forceCloseModal(); + return SegmentsActions.actionAddSegmentSuccess({ segment: response }); + }), + catchError((error) => { + if (error?.error?.type === SERVER_ERROR.SEGMENT_DUPLICATE_NAME) { + this.segmentsService.setDuplicateSegmentNameError(error.error); + } + return [SegmentsActions.actionAddSegmentFailure()]; + }) + ); + }) + ) + ); + + updateSegment$ = createEffect(() => + this.actions$.pipe( + ofType(SegmentsActions.actionUpdateSegment), + switchMap((action) => { + return this.segmentsDataService.modifySegment(action.updateSegmentRequest).pipe( + concatMap((response: Segment) => { + this.commonModalEventService.forceCloseModal(); + return [ + SegmentsActions.actionUpdateSegmentSuccess({ segment: response }), + SegmentsActions.actionGetSegmentById({ segmentId: response.id }), + ]; + }), + catchError(() => { + return of(SegmentsActions.actionUpdateSegmentFailure()); + }) + ); + }) + ) + ); + deleteSegment$ = createEffect(() => this.actions$.pipe( ofType(SegmentsActions.actionDeleteSegment), @@ -193,6 +239,17 @@ export class SegmentsEffects { ) ); + navigateToSegmentDetail$ = createEffect( + () => + this.actions$.pipe( + ofType(SegmentsActions.actionAddSegmentSuccess), + tap(({ segment }) => { + this.router.navigate(['/segments', 'detail', segment.id]); + }) + ), + { dispatch: false } + ); + exportSegments$ = createEffect(() => this.actions$.pipe( ofType(SegmentsActions.actionExportSegments), diff --git a/frontend/projects/upgrade/src/app/core/segments/store/segments.model.ts b/frontend/projects/upgrade/src/app/core/segments/store/segments.model.ts index 77ad548efd..3a5eefc7f7 100644 --- a/frontend/projects/upgrade/src/app/core/segments/store/segments.model.ts +++ b/frontend/projects/upgrade/src/app/core/segments/store/segments.model.ts @@ -6,6 +6,7 @@ import { SEGMENT_SEARCH_KEY, SORT_AS_DIRECTION, SEGMENT_SORT_KEY, + FILTER_MODE, FEATURE_FLAG_LIST_FILTER_MODE, } from 'upgrade_types'; export { SEGMENT_STATUS }; @@ -118,6 +119,27 @@ export interface Segment { status: SEGMENT_STATUS; } +export interface CoreSegmentDetails { + id?: string; + name: string; + context: string; + description?: string; + tags?: string[]; + userIds: string[]; + groups: Group[]; + subSegmentIds: string[]; + status?: SEGMENT_STATUS; + type: SEGMENT_TYPE; +} + +// Currently there is no difference between these types, but they semantically different and could diverge later +export type AddSegmentRequest = CoreSegmentDetails; + +// so that we can throw an error if we try to update the id +export interface UpdateSegmentRequest extends AddSegmentRequest { + readonly id: string; +} + export interface SegmentInput { createdAt: string; updatedAt: string; @@ -212,6 +234,7 @@ export enum SEGMENT_LIST_ACTIONS { export interface SegmentState extends EntityState { isLoadingSegments: boolean; + isLoadingUpsertSegment: boolean; // TODO: remove any allExperimentSegmentsInclusion: any; allExperimentSegmentsExclusion: any; @@ -330,3 +353,26 @@ export interface AddPrivateSegmentListRequest extends PrivateSegmentListRequest export interface EditPrivateSegmentListRequest extends PrivateSegmentListRequest { segment: EditPrivateSegmentListDetails; } + +export enum CommonTagInputType { + TAGS = 'tags', + VALUES = 'values', +} + +export interface SegmentFormData { + name: string; + description: string; + appContext: string; + tags: string[]; +} + +export enum UPSERT_SEGMENT_ACTION { + ADD = 'add', + EDIT = 'edit', + DUPLICATE = 'duplicate', +} + +export interface UpsertSegmentParams { + sourceSegment: Segment; + action: UPSERT_SEGMENT_ACTION; +} diff --git a/frontend/projects/upgrade/src/app/core/segments/store/segments.reducer.ts b/frontend/projects/upgrade/src/app/core/segments/store/segments.reducer.ts index f0c01cf2fa..4c2e4b2929 100644 --- a/frontend/projects/upgrade/src/app/core/segments/store/segments.reducer.ts +++ b/frontend/projects/upgrade/src/app/core/segments/store/segments.reducer.ts @@ -25,6 +25,7 @@ export const initialState: SegmentState = adapter.getInitialState({ searchString: null, sortKey: SEGMENT_SORT_KEY.NAME, sortAs: SORT_AS_DIRECTION.ASCENDING, + isLoadingUpsertSegment: false, }); const reducer = createReducer( @@ -91,6 +92,8 @@ const reducer = createReducer( SegmentsActions.actionFetchSegmentsFailure, SegmentsActions.actionUpsertSegmentFailure, SegmentsActions.actionGetSegmentByIdFailure, + SegmentsActions.actionUpdateSegmentSuccess, + SegmentsActions.actionAddSegmentSuccess, (state) => ({ ...state, isLoadingSegments: false }) ), on(SegmentsActions.actionUpsertSegmentSuccess, (state, { segment }) => diff --git a/frontend/projects/upgrade/src/app/core/segments/store/segments.selectors.ts b/frontend/projects/upgrade/src/app/core/segments/store/segments.selectors.ts index 73c1c76986..42602f3f68 100644 --- a/frontend/projects/upgrade/src/app/core/segments/store/segments.selectors.ts +++ b/frontend/projects/upgrade/src/app/core/segments/store/segments.selectors.ts @@ -1,4 +1,4 @@ -import { createSelector, createFeatureSelector } from '@ngrx/store'; +import { createSelector, createFeatureSelector, select } from '@ngrx/store'; import { LIST_OPTION_TYPE, SegmentState, @@ -27,6 +27,8 @@ export const selectGlobalSegments = createSelector(selectGlobalSegmentsState, se export const selectIsLoadingSegments = createSelector(selectSegmentsState, (state) => state.isLoadingSegments); +export const isLoadingUpsertSegment = createSelector(selectSegmentsState, (state) => state.isLoadingUpsertSegment); + export const selectIsLoadingGlobalSegments = createSelector( selectGlobalSegmentsState, (state) => state.isLoadingGlobalSegments @@ -37,6 +39,10 @@ export const selectSegmentById = createSelector( (state, { segmentId }) => state.entities[segmentId] ); +export const selectAppContexts = createSelector(selectContextMetaData, (contextMetaData) => + Object.keys(contextMetaData?.contextMetadata ?? []) +); + export const selectExperimentSegmentsInclusion = createSelector( selectSegmentsState, (state) => state.allExperimentSegmentsInclusion @@ -66,6 +72,26 @@ export const selectAllSegmentEntities = createSelector( }) ); +export const selectSegmentIdFromRoute = createSelector(selectRouterState, (routerState) => { + if (routerState?.state?.params?.segmentId) { + return routerState.state.params.segmentId; + } + return null; +}); + +// Create a selector that only emits after navigation is complete +export const selectSegmentIdAfterNavigation = createSelector( + selectSegmentIdFromRoute, + selectRouterState, + (segmentId, routerState) => { + // Only return the segmentId if we have a completed navigation + if (segmentId && routerState?.state?.url) { + return segmentId; + } + return null; + } +); + export const selectSelectedSegment = createSelector( selectRouterState, selectAllSegmentEntities, @@ -76,6 +102,7 @@ export const selectSelectedSegment = createSelector( } = routerState; return allSegmentEntities[params.segmentId] ? allSegmentEntities[params.segmentId] : undefined; } + return undefined; } ); diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/upsert-feature-flag-modal/upsert-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/upsert-feature-flag-modal/upsert-feature-flag-modal.component.ts index 7213829810..995c3d602e 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/upsert-feature-flag-modal/upsert-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/upsert-feature-flag-modal/upsert-feature-flag-modal.component.ts @@ -198,7 +198,6 @@ export class UpsertFeatureFlagModalComponent { sendRequest(action: UPSERT_FEATURE_FLAG_ACTION, sourceFlag?: FeatureFlag): void { const formData: FeatureFlagFormData = this.featureFlagForm.value; - if (action === UPSERT_FEATURE_FLAG_ACTION.ADD || action === UPSERT_FEATURE_FLAG_ACTION.DUPLICATE) { this.createAddRequest(formData); } else if (action === UPSERT_FEATURE_FLAG_ACTION.EDIT && sourceFlag) { diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component.html b/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component.html index cc87ad298d..c0ec136643 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component.html +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component.html @@ -1 +1,66 @@ -

upsert-segment-modal works!

+ +
+ + Name + + + Name is required + + + + {{ + 'segments.errors.duplicate-name.text' + | translate : { name: errorData.duplicateName, context: errorData.context } + }} + + + + + {{ 'segments.upsert-segment-modal.name-hint.text' | translate }} + + + + + Description (optional) + + + + + App Context + + {{ + context + }} + + + {{ 'segments.upsert-segment-modal.app-context-hint.text' | translate }} + + Learn more + + + + {{ 'segments.upsert-segment-modal.app-context-warning.text' | translate }} + + + + +
+ diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component.ts index da6180dbf8..b3575dfcc1 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component.ts @@ -1,10 +1,292 @@ -import { ChangeDetectionStrategy, Component } from '@angular/core'; +import { ChangeDetectionStrategy, Component, Inject } from '@angular/core'; +import { + CommonModalComponent, + CommonTagsInputComponent, +} from '../../../../../shared-standalone-component-lib/components'; +import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; +import { CommonModule } from '@angular/common'; +import { FormBuilder, FormGroup, ReactiveFormsModule, Validators } from '@angular/forms'; +import { MatButtonModule } from '@angular/material/button'; +import { MatCardModule } from '@angular/material/card'; +import { MatFormFieldModule } from '@angular/material/form-field'; +import { MatInputModule } from '@angular/material/input'; +import { MatSelectModule } from '@angular/material/select'; +import { CommonFormHelpersService } from '../../../../../shared/services/common-form-helpers.service'; +import { + CommonTagInputType, + Segment, + SEGMENT_STATUS, + SegmentFormData, + UPSERT_SEGMENT_ACTION, + UpsertSegmentParams, + AddSegmentRequest, + UpdateSegmentRequest, +} from '../../../../../core/segments/store/segments.model'; +import { SegmentsService } from '../../../../../core/segments/segments.service'; +import { BehaviorSubject, Observable, map, startWith, Subscription, combineLatestWith, withLatestFrom } from 'rxjs'; +import { TranslateModule } from '@ngx-translate/core'; +import { CommonModalConfig } from '../../../../../shared-standalone-component-lib/components/common-modal/common-modal.types'; +import { ExperimentService } from '../../../../../core/experiments/experiments.service'; +import { DuplicateSegmentNameError, SEGMENT_TYPE } from 'upgrade_types'; +import isEqual from 'lodash.isequal'; @Component({ - selector: 'app-upsert-segment-modal', - imports: [], + selector: 'upsert-add-segment-modal', + imports: [ + CommonModalComponent, + MatCardModule, + MatFormFieldModule, + MatInputModule, + MatButtonModule, + MatSelectModule, + CommonModule, + ReactiveFormsModule, + TranslateModule, + CommonTagsInputComponent, + ], templateUrl: './upsert-segment-modal.component.html', styleUrl: './upsert-segment-modal.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) -export class UpsertSegmentModalComponent {} +export class UpsertSegmentModalComponent { + segmentForm: FormGroup; + isLoadingUpsertSegment$ = this.segmentService.isLoadingUpsertSegment$; + isPrimaryButtonDisabled$: Observable; + appContexts$ = this.segmentService.appContexts$; + CommonTagInputType = CommonTagInputType; + initialFormValues$ = new BehaviorSubject(null); + subscriptions = new Subscription(); + isContextChanged = false; + initialContext = ''; + isInitialFormValueChanged$: Observable; + duplicateSegmentNameError$ = this.segmentService.duplicateSegmentNameError$; + + constructor( + @Inject(MAT_DIALOG_DATA) + public config: CommonModalConfig, + private formBuilder: FormBuilder, + private segmentService: SegmentsService, + private experimentService: ExperimentService, + public dialogRef: MatDialogRef + ) {} + + ngOnInit(): void { + this.experimentService.fetchContextMetaData(); + this.createSegmentForm(); + this.listenForIsInitialFormValueChanged(); + this.listenForPrimaryButtonDisabled(); + this.listenOnContext(); + } + + createSegmentForm(): void { + const { sourceSegment, action } = this.config.params; + const initialValues = this.deriveInitialFormValues(sourceSegment, action); + this.initialContext = initialValues.appContext; + + this.segmentForm = this.formBuilder.group({ + name: [initialValues.name, Validators.required], + description: [initialValues.description], + appContext: [initialValues.appContext, Validators.required], + tags: [initialValues.tags], + }); + this.disableRestrictedFields(); + + this.initialFormValues$.next(this.segmentForm.value); + + this.listenForDuplicateNameError(); + this.listenForNameChanges(); + } + + disableRestrictedFields(): void { + if ( + this.config.params.action === UPSERT_SEGMENT_ACTION.EDIT && + this.config.params.sourceSegment.status === SEGMENT_STATUS.USED + ) { + this.appContextControl.disable(); + this.nameControl.disable(); + } + + if (this.config.params.action === UPSERT_SEGMENT_ACTION.DUPLICATE) { + this.appContextControl.enable(); + } + } + + private updateNameControlErrors(shouldSetDuplicateError: boolean, error?: DuplicateSegmentNameError) { + const currentErrors = this.nameControl.errors || {}; + const { duplicateSegmentName, ...otherErrors } = currentErrors; + + if (shouldSetDuplicateError && error) { + this.nameControl.setErrors({ ...otherErrors, duplicateSegmentName: error }); + } else if (currentErrors.duplicateSegmentName) { + this.nameControl.setErrors(Object.keys(otherErrors).length ? otherErrors : null); + } + } + + setDuplicateSegmentNameErrorOnNameControl(error: DuplicateSegmentNameError) { + this.updateNameControlErrors(!!error, error); + } + + handleCheckingDuplicateName(error: DuplicateSegmentNameError, nameInput: string) { + const contextInput = this.appContextControl.value; + const isDuplicate = nameInput === error?.duplicateName && contextInput === error?.context; + this.updateNameControlErrors(isDuplicate, error); + } + + get nameControl() { + return this.segmentForm.get('name'); + } + + get appContextControl() { + return this.segmentForm.get('appContext'); + } + + deriveInitialFormValues(sourceSegment: Segment, action: UPSERT_SEGMENT_ACTION): SegmentFormData { + let name = ''; + if (action === UPSERT_SEGMENT_ACTION.DUPLICATE) { + name = `${sourceSegment?.name} (COPY)`; + } + + if (action === UPSERT_SEGMENT_ACTION.EDIT) { + name = sourceSegment?.name; + } + + const description = sourceSegment?.description || ''; + const appContext = sourceSegment?.context || ''; + const tags = sourceSegment?.tags || []; + return { name, description, appContext, tags }; + } + + listenForPrimaryButtonDisabled() { + this.isPrimaryButtonDisabled$ = this.isLoadingUpsertSegment$.pipe( + combineLatestWith(this.isInitialFormValueChanged$), + map( + ([isLoading, isInitialFormValueChanged]) => + isLoading || + this.segmentForm.invalid || + (!isInitialFormValueChanged && this.config.params.action !== UPSERT_SEGMENT_ACTION.DUPLICATE) + ) + ); + this.subscriptions.add(this.isPrimaryButtonDisabled$.subscribe()); + } + + listenForNameChanges(): void { + this.subscriptions.add( + this.nameControl.valueChanges + .pipe(withLatestFrom(this.duplicateSegmentNameError$)) + .subscribe(([name, error]) => this.handleCheckingDuplicateName(error, name)) + ); + } + + listenForDuplicateNameError(): void { + this.subscriptions.add( + this.duplicateSegmentNameError$.subscribe((error) => this.setDuplicateSegmentNameErrorOnNameControl(error)) + ); + } + + listenOnContext(): void { + this.subscriptions.add( + this.appContextControl?.valueChanges.subscribe((context) => { + this.isContextChanged = context !== this.initialContext; + this.handleCheckingDuplicateName(this.duplicateSegmentNameError$.value, this.nameControl.value); + }) + ); + } + + listenForIsInitialFormValueChanged() { + this.isInitialFormValueChanged$ = this.segmentForm.valueChanges.pipe( + startWith(this.segmentForm.value), + map(() => !isEqual(this.segmentForm.value, this.initialFormValues$.value)) + ); + this.subscriptions.add(this.isInitialFormValueChanged$.subscribe()); + } + + onPrimaryActionBtnClicked() { + if (this.segmentForm.valid) { + this.segmentService.setDuplicateSegmentNameError(null); + this.sendRequest(this.config.params.action, this.config.params.sourceSegment); + } else { + CommonFormHelpersService.triggerTouchedToDisplayErrors(this.segmentForm); + } + } + + sendRequest(action: UPSERT_SEGMENT_ACTION, sourceSegment?: Segment): void { + const formData: SegmentFormData = this.segmentForm.value; + if (action === UPSERT_SEGMENT_ACTION.ADD) { + this.createAddRequest(formData); + } else if (action === UPSERT_SEGMENT_ACTION.EDIT && sourceSegment) { + this.createEditRequest(formData, sourceSegment); + } else if (action === UPSERT_SEGMENT_ACTION.DUPLICATE && sourceSegment) { + this.createDuplicateRequest(formData, sourceSegment); + } else { + console.error('UpsertSegmentModalComponent: sendRequest: Invalid action or missing sourceSegment'); + } + } + + createAddRequest({ name, description, appContext, tags }: SegmentFormData): void { + const segmentRequest: AddSegmentRequest = { + name, + description, + context: appContext, + userIds: [], + groups: [], + subSegmentIds: [], + status: SEGMENT_STATUS.UNUSED, + type: SEGMENT_TYPE.PUBLIC, + tags, + }; + this.segmentService.addSegment(segmentRequest); + } + + createEditRequest({ name, description, appContext, tags }: SegmentFormData, sourceSegment: Segment): void { + const { id, status } = sourceSegment; + const subSegmentIds = sourceSegment.subSegments.map((subSegment) => subSegment.id); + + // Not allow editing segment name and context if segment is in used status: + if (sourceSegment.status === SEGMENT_STATUS.USED) { + name = sourceSegment.name; + appContext = sourceSegment.context; + } + const segmentRequest: UpdateSegmentRequest = { + id, + name, + description, + context: appContext, + userIds: [], + groups: [], + subSegmentIds, + status, + type: SEGMENT_TYPE.PUBLIC, + tags, + }; + this.segmentService.modifySegment(segmentRequest); + } + + createDuplicateRequest({ name, description, tags }: SegmentFormData, sourceSegment: Segment): void { + const subSegmentIds = sourceSegment.subSegments.map((subSegment) => subSegment.id); + + const segmentRequest: AddSegmentRequest = { + name, + description, + context: sourceSegment.context, + userIds: [], + groups: [], + subSegmentIds, + status: SEGMENT_STATUS.UNUSED, + type: SEGMENT_TYPE.PUBLIC, + tags, + }; + this.segmentService.addSegment(segmentRequest); + } + + get UPSERT_SEGMENT_ACTION() { + return UPSERT_SEGMENT_ACTION; + } + + closeModal() { + this.dialogRef.close(); + } + + ngOnDestroy() { + this.subscriptions.unsubscribe(); + } +} diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-details-page-content.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-details-page-content.component.ts index acdfe83fa5..956ad2ea5a 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-details-page-content.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-details-page-content.component.ts @@ -1,13 +1,12 @@ -import { ChangeDetectionStrategy, Component, OnDestroy, OnInit } from '@angular/core'; +import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; import { CommonSectionCardListComponent } from '../../../../../../shared-standalone-component-lib/components'; import { CommonModule } from '@angular/common'; import { SegmentOverviewDetailsSectionCardComponent } from './segment-overview-details-section-card/segment-overview-details-section-card.component'; import { SegmentListsSectionCardComponent } from './segment-lists-section-card/segment-lists-section-card.component'; import { SegmentUsedBySectionCardComponent } from './segment-used-by-section-card/segment-used-by-section-card.component'; import { SegmentsService } from '../../../../../../core/segments/segments.service'; -import { Observable, Subscription } from 'rxjs'; +import { Observable } from 'rxjs'; import { Segment } from '../../../../../../core/segments/store/segments.model'; -import { ActivatedRoute } from '@angular/router'; import { SharedModule } from '../../../../../../shared/shared.module'; @Component({ @@ -24,21 +23,14 @@ import { SharedModule } from '../../../../../../shared/shared.module'; styleUrl: './segment-details-page-content.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) -export class SegmentDetailsPageContentComponent implements OnInit, OnDestroy { +export class SegmentDetailsPageContentComponent implements OnInit { isSectionCardExpanded = true; segment$: Observable; activeTabIndex = 0; // 0 for Lists, 1 for Used By - segmentIdSub: Subscription; - - constructor(private segmentsService: SegmentsService, private _Activatedroute: ActivatedRoute) {} + constructor(private segmentsService: SegmentsService) {} ngOnInit() { - this.segmentIdSub = this._Activatedroute.paramMap.subscribe((params) => { - const segmentIdFromParams = params.get('segmentId'); - this.segmentsService.fetchSegmentById(segmentIdFromParams); - }); - this.segment$ = this.segmentsService.selectedSegment$; } @@ -49,8 +41,4 @@ export class SegmentDetailsPageContentComponent implements OnInit, OnDestroy { onTabChange(tabIndex: number) { this.activeTabIndex = tabIndex; } - - ngOnDestroy() { - this.segmentIdSub.unsubscribe(); - } } diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-overview-details-section-card/segment-overview-details-section-card.component.html b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-overview-details-section-card/segment-overview-details-section-card.component.html index e6d2552478..5c5e75c550 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-overview-details-section-card/segment-overview-details-section-card.component.html +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-overview-details-section-card/segment-overview-details-section-card.component.html @@ -5,6 +5,7 @@ [title]="segment.name" [createdAt]="segment.createdAt" [updatedAt]="segment.updatedAt" + [versionNumber]="segment.versionNumber" [chipClass]="segment.status.toLowerCase()" > diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-overview-details-section-card/segment-overview-details-section-card.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-overview-details-section-card/segment-overview-details-section-card.component.ts index 32f00533d7..33ba4fe330 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-overview-details-section-card/segment-overview-details-section-card.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page-content/segment-overview-details-section-card/segment-overview-details-section-card.component.ts @@ -85,10 +85,10 @@ export class SegmentOverviewDetailsSectionCardComponent implements OnInit, OnDes onMenuButtonItemClick(action: SEGMENT_DETAILS_PAGE_ACTIONS, segment: Segment) { switch (action) { case SEGMENT_DETAILS_PAGE_ACTIONS.EDIT: - // this.dialogService.openEditSegmentModal(segment); + this.dialogService.openEditSegmentModal(segment); break; case SEGMENT_DETAILS_PAGE_ACTIONS.DUPLICATE: - // this.dialogService.openDuplicateSegmentModal(segment); + this.dialogService.openDuplicateSegmentModal(segment); break; case SEGMENT_DETAILS_PAGE_ACTIONS.DELETE: // this.dialogService.openDeleteSegmentModal(segment); diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page.component.ts index 5b3310355a..9b51dc9eb0 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-details-page/segment-details-page.component.ts @@ -5,11 +5,10 @@ import { SegmentDetailsPageHeaderComponent } from './segment-details-page-header import { SegmentDetailsPageContentComponent } from './segment-details-page-content/segment-details-page-content.component'; import { SegmentsModule } from '../../../segments-legacy/segments.module'; import { SegmentsService } from '../../../../../core/segments/segments.service'; -import { ActivatedRoute } from '@angular/router'; +import { ActivatedRoute, NavigationEnd, Router } from '@angular/router'; import { Segment } from '../../../../../core/segments/store/segments.model'; -import { SEGMENT_TYPE } from 'upgrade_types'; -import { Observable, Subscription } from 'rxjs'; -import { filter, map } from 'rxjs/operators'; +import { combineLatest, Observable, Subscription } from 'rxjs'; +import { filter, map, startWith } from 'rxjs/operators'; import { CommonModule } from '@angular/common'; @Component({ @@ -29,14 +28,25 @@ export class SegmentDetailsPageComponent implements OnInit, OnDestroy { segmentIdSub: Subscription; shouldUseLegacyView$: Observable; - constructor(private segmentsService: SegmentsService, private route: ActivatedRoute) {} + constructor(private segmentsService: SegmentsService, private router: Router, private route: ActivatedRoute) {} ngOnInit() { - this.segmentIdSub = this.route.paramMap.subscribe((params) => { - const segmentId = params.get('segmentId'); - if (segmentId) { - this.segmentsService.fetchSegmentById(segmentId); - } + const segmentIdFromRoute$ = this.route.paramMap.pipe( + map((params) => params.get('segmentId')), + filter((segmentId) => !!segmentId) + ); + + const navigationComplete$ = this.router.events.pipe( + filter((event) => event instanceof NavigationEnd), + startWith(null) + ); + + // Combine both observables to ensure we only fetch after navigation completes + // This will ensure no weird router behavior when navigating back and forth + const segmentId$ = combineLatest([segmentIdFromRoute$, navigationComplete$]).pipe(map(([segmentId]) => segmentId)); + + this.segmentIdSub = segmentId$.subscribe((segmentId) => { + this.segmentsService.fetchSegmentById(segmentId); }); this.segment$ = this.segmentsService.selectedSegment$; diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component.html b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component.html index e0beb78d59..02525b82f0 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component.html +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card-table/segment-root-section-card-table.component.html @@ -53,8 +53,13 @@ {{ SEGMENT_TRANSLATION_KEYS.UPDATED_AT | translate }} - {{ segment.updatedAt | date : 'MMM d, y h:mm a' }} - + + {{ '-' }} + + + {{ segment.updatedAt | date : 'MMM d, y h:mm a' }} + + diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component.ts index 5db031a281..e29ac579e4 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/pages/segment-root-page/segment-root-page-content/segment-root-section-card/segment-root-section-card.component.ts @@ -98,7 +98,7 @@ export class SegmentRootSectionCardComponent { } onAddSegmentButtonClick() { - // this.dialogService.openNewSegmentModal(); + this.dialogService.openAddSegmentModal(); } onMenuButtonItemClick(action: string) { diff --git a/frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts b/frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts index c694f649d3..b8d8b3a788 100644 --- a/frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts +++ b/frontend/projects/upgrade/src/app/shared/services/common-dialog.service.ts @@ -5,8 +5,11 @@ import { DeleteFeatureFlagModalComponent } from '../../features/dashboard/featur import { UpsertFeatureFlagModalComponent } from '../../features/dashboard/feature-flags/modals/upsert-feature-flag-modal/upsert-feature-flag-modal.component'; import { UpsertPrivateSegmentListModalComponent } from '../../features/dashboard/segments/modals/upsert-private-segment-list-modal/upsert-private-segment-list-modal.component'; import { + Segment, UPSERT_PRIVATE_SEGMENT_LIST_ACTION, + UPSERT_SEGMENT_ACTION, UpsertPrivateSegmentListParams, + UpsertSegmentParams, } from '../../core/segments/store/segments.model'; import { FEATURE_FLAG_DETAILS_PAGE_ACTIONS, @@ -23,6 +26,7 @@ import { SimpleConfirmationModalParams, } from '../../shared-standalone-component-lib/components/common-modal/common-modal.types'; import { FEATURE_FLAG_LIST_FILTER_MODE } from 'upgrade_types'; +import { UpsertSegmentModalComponent } from '../../features/dashboard/segments/modals/upsert-segment-modal/upsert-segment-modal.component'; import { FEATURE_FLAG_IMPORT_SERVICE, ImportServiceAdapter, @@ -54,7 +58,7 @@ export class DialogService { }); } - // Common modal flags ---------------------------------------- // + // feature flag modal ---------------------------------------- // openAddFeatureFlagModal() { const commonModalConfig: CommonModalConfig = { title: 'Add Feature Flag', @@ -367,6 +371,65 @@ export class DialogService { return this.openSimpleCommonConfirmationModal(commonModalConfig, ModalSize.MEDIUM); } + // segment modal ---------------------------------------- // + openUpsertSegmentModal(commonModalConfig: CommonModalConfig) { + const config: MatDialogConfig = { + data: commonModalConfig, + width: ModalSize.STANDARD, + autoFocus: 'input', + disableClose: true, + }; + return this.dialog.open(UpsertSegmentModalComponent, config); + } + + openAddSegmentModal() { + const commonModalConfig: CommonModalConfig = { + title: 'Add Segment', + tagsLabel: 'segments.upsert-segment-modal.tags-label.text', + tagsPlaceholder: 'segments.upsert-segment-modal.tags-placeholder.text', + primaryActionBtnLabel: 'Create', + primaryActionBtnColor: 'primary', + cancelBtnLabel: 'Cancel', + params: { + sourceSegment: null, + action: UPSERT_SEGMENT_ACTION.ADD, + }, + }; + return this.openUpsertSegmentModal(commonModalConfig); + } + + openEditSegmentModal(sourceSegment: Segment) { + const commonModalConfig: CommonModalConfig = { + title: 'Edit Segment', + tagsLabel: 'segments.upsert-segment-modal.tags-label.text', + tagsPlaceholder: 'segments.upsert-segment-modal.tags-placeholder.text', + primaryActionBtnLabel: 'Save', + primaryActionBtnColor: 'primary', + cancelBtnLabel: 'Cancel', + params: { + sourceSegment: { ...sourceSegment }, + action: UPSERT_SEGMENT_ACTION.EDIT, + }, + }; + return this.openUpsertSegmentModal(commonModalConfig); + } + + openDuplicateSegmentModal(sourceSegment: Segment) { + const commonModalConfig: CommonModalConfig = { + title: 'Duplicate Segment', + tagsLabel: 'segments.upsert-segment-modal.tags-label.text', + tagsPlaceholder: 'segments.upsert-segment-modal.tags-placeholder.text', + primaryActionBtnLabel: 'Create', + primaryActionBtnColor: 'primary', + cancelBtnLabel: 'Cancel', + params: { + sourceSegment: { ...sourceSegment }, + action: UPSERT_SEGMENT_ACTION.DUPLICATE, + }, + }; + return this.openUpsertSegmentModal(commonModalConfig); + } + openImportSegmentModal() { const commonModalConfig: CommonModalConfig = { title: 'segments.import-segment-modal.title.text', diff --git a/frontend/projects/upgrade/src/assets/i18n/en.json b/frontend/projects/upgrade/src/assets/i18n/en.json index d645123567..cdaddde67e 100644 --- a/frontend/projects/upgrade/src/assets/i18n/en.json +++ b/frontend/projects/upgrade/src/assets/i18n/en.json @@ -522,6 +522,12 @@ "segments.segment-experiment-list-type.text": "Type", "segments.segment-experiment-list-usedList.text": "Used List", "segments.segment-experiment-list-zeroState.text": "No Data", + "segments.upsert-segment-modal.name-hint.text": "The name for this segment.", + "segments.upsert-segment-modal.app-context-hint.text": "The App Context indicates the segment belongs to which context.", + "segments.upsert-segment-modal.app-context-warning.text": "Updating the App Context will clear the existing include and exclude lists.", + "segments.upsert-segment-modal.tags-label.text": "Tags (optional)", + "segments.upsert-segment-modal.tags-placeholder.text": "Tags separated by commas", + "segments.errors.duplicate-name.text": "A segment with name \"{{name}}\" already exists on context \"{{context}}\".", "stratifications.global-members-factor.text": "FACTOR", "stratifications.global-members-summary.text": "SUMMARY", "stratifications.global-members-status.text": "STATUS", diff --git a/types/src/Experiment/enums.ts b/types/src/Experiment/enums.ts index 639a8f6057..3f7d2faa4e 100644 --- a/types/src/Experiment/enums.ts +++ b/types/src/Experiment/enums.ts @@ -74,6 +74,7 @@ export enum SERVER_ERROR { UNSUPPORTED_CALIPER = 'Caliper profile or event not supported', DUPLICATE_KEY = 'Feature Flag with same key already exists for this app-context', MISSING_HEADER_USER_ID = 'Missing `User-Id` header', + SEGMENT_DUPLICATE_NAME = 'Segment with same name already exists for this app-context, please edit name to be unique.', } export enum MARKED_DECISION_POINT_STATUS { @@ -213,6 +214,7 @@ export enum SEGMENT_SEARCH_KEY { ALL = 'all', NAME = 'name', TAG = 'tag', + STATUS = 'status', CONTEXT = 'context', } diff --git a/types/src/Experiment/interfaces.ts b/types/src/Experiment/interfaces.ts index 9e292e39b6..f3f9774311 100644 --- a/types/src/Experiment/interfaces.ts +++ b/types/src/Experiment/interfaces.ts @@ -13,6 +13,7 @@ import { FILTER_MODE, FEATURE_FLAG_LIST_FILTER_MODE, IMPORT_COMPATIBILITY_TYPE, + SERVER_ERROR, } from './enums'; export interface IEnrollmentCompleteCondition { userCount: number; @@ -294,3 +295,11 @@ export interface ValidatedImportResponse { compatibilityType: IMPORT_COMPATIBILITY_TYPE; error?: string; } + +export interface DuplicateSegmentNameError { + type: SERVER_ERROR.SEGMENT_DUPLICATE_NAME; + message: string; + duplicateName: string; + context: string; + httpCode: 400; +} diff --git a/types/src/index.ts b/types/src/index.ts index 6f380d9f31..ed3b3e82d9 100644 --- a/types/src/index.ts +++ b/types/src/index.ts @@ -78,6 +78,7 @@ export { FeatureFlagStateChangedData, FeatureFlagDeletedData, ValidatedImportResponse, + DuplicateSegmentNameError, } from './Experiment/interfaces'; export { MoocletPolicyParametersDTO,