Merge pull request #1786 from CarnegieLearningWeb/feature/-1762-valid…

…ation-and-409-error

Adding validation and 409 error to flags

backend/packages/Upgrade/src/api/middlewares/ErrorHandlerMiddleware.ts

@@ -97,6 +97,10 @@ export class ErrorHandlerMiddleware implements ExpressErrorMiddlewareInterface {
             message = error.message;
             type = SERVER_ERROR.QUERY_FAILED;
             break;
+          case 409:
+            message = error.message;
+            type = SERVER_ERROR.DUPLICATE_KEY;
+            break;
           case 422:
             message = error.message;
             type = SERVER_ERROR.UNSUPPORTED_CALIPER;

backend/packages/Upgrade/src/api/models/FeatureFlag.ts

@@ -1,4 +1,4 @@
-import { Column, Entity, PrimaryColumn, OneToMany } from 'typeorm';
+import { Column, Entity, PrimaryColumn, OneToMany, Unique } from 'typeorm';
 import { IsNotEmpty } from 'class-validator';
 import { BaseModel } from './base/BaseModel';
 import { Type } from 'class-transformer';
@@ -7,6 +7,7 @@ import { FeatureFlagSegmentInclusion } from './FeatureFlagSegmentInclusion';
 import { FeatureFlagSegmentExclusion } from './FeatureFlagSegmentExclusion';
 import { FeatureFlagExposure } from './FeatureFlagExposure';
 @Entity()
+@Unique(['key', 'context'])
 export class FeatureFlag extends BaseModel {
   @PrimaryColumn('uuid')
   public id: string;
@@ -15,7 +16,7 @@ export class FeatureFlag extends BaseModel {
   @Column()
   public name: string;
 
-  @Column('text', { unique: true })
+  @Column('text')
   public key: string;
 
   @Column()

backend/packages/Upgrade/src/api/repositories/FeatureFlagRepository.ts

@@ -3,6 +3,7 @@ import { EntityRepository } from '../../typeorm-typedi-extensions';
 import { FeatureFlag } from '../models/FeatureFlag';
 import repositoryError from './utils/repositoryError';
 import { FEATURE_FLAG_STATUS, FILTER_MODE } from 'upgrade_types';
+import { FeatureFlagValidation } from '../controllers/validators/FeatureFlagValidator';
 
 @EntityRepository(FeatureFlag)
 export class FeatureFlagRepository extends Repository<FeatureFlag> {
@@ -111,4 +112,17 @@ export class FeatureFlagRepository extends Repository<FeatureFlag> {
 
     return result;
   }
+
+  public async validateUniqueKey(flagDTO: FeatureFlagValidation) {
+    const queryBuilder = this.createQueryBuilder('feature_flag')
+      .where('feature_flag.key = :key', { key: flagDTO.key })
+      .andWhere('feature_flag.context = :context', { context: flagDTO.context });
+
+    if (flagDTO.id) {
+      queryBuilder.andWhere('feature_flag.id != :id', { id: flagDTO.id });
+    }
+
+    const result = await queryBuilder.getOne();
+    return result;
+  }
 }

backend/packages/Upgrade/src/api/services/FeatureFlagService.ts

@@ -142,8 +142,17 @@ export class FeatureFlagService {
     return featureFlag;
   }
 
-  public create(flagDTO: FeatureFlagValidation, currentUser: User, logger: UpgradeLogger): Promise<FeatureFlag> {
+  public async create(flagDTO: FeatureFlagValidation, currentUser: User, logger: UpgradeLogger): Promise<FeatureFlag> {
     logger.info({ message: 'Create a new feature flag', details: flagDTO });
+    const result = await this.featureFlagRepository.validateUniqueKey(flagDTO);
+
+    if (result) {
+      const error = new Error(`A flag with this key already exists for this app-context`);
+      (error as any).type = SERVER_ERROR.DUPLICATE_KEY;
+      (error as any).httpCode = 409;
+      throw error;
+    }
+
     return this.addFeatureFlagInDB(this.featureFlagValidatorToFlag(flagDTO), currentUser, logger);
   }
 
@@ -295,8 +304,16 @@ export class FeatureFlagService {
     return featureFlag;
   }
 
-  public update(flagDTO: FeatureFlagValidation, currentUser: User, logger: UpgradeLogger): Promise<FeatureFlag> {
+  public async update(flagDTO: FeatureFlagValidation, currentUser: User, logger: UpgradeLogger): Promise<FeatureFlag> {
     logger.info({ message: `Update a Feature Flag => ${flagDTO.toString()}` });
+    const result = await this.featureFlagRepository.validateUniqueKey(flagDTO);
+
+    if (result) {
+      const error = new Error(`A flag with this key already exists for this app-context`);
+      (error as any).type = SERVER_ERROR.DUPLICATE_KEY;
+      (error as any).httpCode = 409;
+      throw error;
+    }
     // TODO add entry in log of updating feature flag
     return this.updateFeatureFlagInDB(this.featureFlagValidatorToFlag(flagDTO), currentUser, logger);
   }
@@ -902,7 +919,9 @@ export class FeatureFlagService {
     }
 
     if (compatibilityType === FF_COMPATIBILITY_TYPE.COMPATIBLE) {
-      const keyExists = existingFeatureFlags.find((existingFlag) => existingFlag.key === flag.key);
+      const keyExists = existingFeatureFlags?.find(
+        (existingFlag) => existingFlag.key === flag.key && existingFlag.context === flag.context
+      );
 
       if (keyExists) {
         compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE;

backend/packages/Upgrade/src/database/migrations/1721969267641-UniquePairFlag.ts

@@ -0,0 +1,43 @@
+import { MigrationInterface, QueryRunner } from 'typeorm';
+
+export class UniquePairFlag1721969267641 implements MigrationInterface {
+  name = 'UniquePairFlag1721969267641';
+
+  public async up(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`ALTER TABLE "feature_flag_segment_exclusion" ALTER COLUMN "enabled" SET DEFAULT false`);
+    await queryRunner.query(`ALTER TABLE "feature_flag" DROP CONSTRAINT "UQ_960310efa932f7a29eec57350b3"`);
+    await queryRunner.query(`ALTER TABLE "feature_flag_segment_inclusion" ALTER COLUMN "enabled" SET DEFAULT false`);
+    await queryRunner.query(
+      `ALTER TYPE "public"."experiment_error_type_enum" RENAME TO "experiment_error_type_enum_old"`
+    );
+    await queryRunner.query(
+      `CREATE TYPE "public"."experiment_error_type_enum" AS ENUM('Database not reachable', 'Database auth fail', 'Error in the assignment algorithm', 'Parameter missing in the client request', 'Parameter not in the correct format', 'User ID not found', 'Query Failed', 'Error reported from client', 'Experiment user not defined', 'Experiment user group not defined', 'Working group is not a subset of user group', 'Invalid token', 'Token is not present in request', 'JWT Token validation failed', 'Error in migration', 'Email send error', 'Condition not found', 'Experiment ID not provided for shared Decision Point', 'Experiment ID provided is invalid for shared Decision Point', 'Caliper profile or event not supported', 'Feature Flag with same key already exists for this app-context')`
+    );
+    await queryRunner.query(
+      `ALTER TABLE "experiment_error" ALTER COLUMN "type" TYPE "public"."experiment_error_type_enum" USING "type"::"text"::"public"."experiment_error_type_enum"`
+    );
+    await queryRunner.query(`DROP TYPE "public"."experiment_error_type_enum_old"`);
+    await queryRunner.query(
+      `ALTER TABLE "feature_flag" ADD CONSTRAINT "UQ_653d98ebae725596b4052f80a65" UNIQUE ("key", "context")`
+    );
+  }
+
+  public async down(queryRunner: QueryRunner): Promise<void> {
+    await queryRunner.query(`ALTER TABLE "feature_flag" DROP CONSTRAINT "UQ_653d98ebae725596b4052f80a65"`);
+    await queryRunner.query(
+      `CREATE TYPE "public"."experiment_error_type_enum_old" AS ENUM('Caliper profile or event not supported', 'Condition not found', 'Database auth fail', 'Database not reachable', 'Email send error', 'Error in migration', 'Error in the assignment algorithm', 'Error reported from client', 'Experiment ID not provided for shared Decision Point', 'Experiment ID provided is invalid for shared Decision Point', 'Experiment user group not defined', 'Experiment user not defined', 'Invalid token', 'JWT Token validation failed', 'Parameter missing in the client request', 'Parameter not in the correct format', 'Query Failed', 'Token is not present in request', 'User ID not found', 'Working group is not a subset of user group')`
+    );
+    await queryRunner.query(
+      `ALTER TABLE "experiment_error" ALTER COLUMN "type" TYPE "public"."experiment_error_type_enum_old" USING "type"::"text"::"public"."experiment_error_type_enum_old"`
+    );
+    await queryRunner.query(`DROP TYPE "public"."experiment_error_type_enum"`);
+    await queryRunner.query(
+      `ALTER TYPE "public"."experiment_error_type_enum_old" RENAME TO "experiment_error_type_enum"`
+    );
+    await queryRunner.query(`ALTER TABLE "feature_flag_segment_inclusion" ALTER COLUMN "enabled" DROP DEFAULT`);
+    await queryRunner.query(
+      `ALTER TABLE "feature_flag" ADD CONSTRAINT "UQ_960310efa932f7a29eec57350b3" UNIQUE ("key")`
+    );
+    await queryRunner.query(`ALTER TABLE "feature_flag_segment_exclusion" ALTER COLUMN "enabled" DROP DEFAULT`);
+  }
+}

backend/packages/Upgrade/test/unit/services/FeatureFlagService.test.ts

@@ -194,6 +194,7 @@ describe('Feature Flag Service Testing', () => {
               addOrderBy: addOrderBySpy,
               setParameter: setParameterSpy,
               where: jest.fn().mockReturnThis(),
+              andWhere: jest.fn().mockReturnThis(),
               offset: offsetSpy,
               limit: limitSpy,
               innerJoinAndSelect: jest.fn().mockReturnThis(),
@@ -202,6 +203,7 @@ describe('Feature Flag Service Testing', () => {
               getMany: jest.fn().mockResolvedValue(mockFlagArr),
               getOne: jest.fn().mockResolvedValue(mockFlag1),
             })),
+            validateUniqueKey: jest.fn().mockResolvedValue(null),
           },
         },
         {

frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts

@@ -26,6 +26,7 @@ import {
   selectFeatureFlagIds,
   selectShouldShowWarningForSelectedFlag,
   selectWarningStatusForAllFlags,
+  selectDuplicateKeyFound,
 } from './store/feature-flags.selectors';
 import * as FeatureFlagsActions from './store/feature-flags.actions';
 import { actionFetchContextMetaData } from '../experiments/store/experiments.actions';
@@ -51,6 +52,7 @@ export class FeatureFlagsService {
   isLoadingFeatureFlags$ = this.store$.pipe(select(selectIsLoadingFeatureFlags));
   isLoadingSelectedFeatureFlag$ = this.store$.pipe(select(selectIsLoadingSelectedFeatureFlag));
   isLoadingUpsertFeatureFlag$ = this.store$.pipe(select(selectIsLoadingUpsertFeatureFlag));
+  isDuplicateKeyFound$ = this.store$.pipe(select(selectDuplicateKeyFound));
   isLoadingFeatureFlagDelete$ = this.store$.pipe(select(selectIsLoadingFeatureFlagDelete));
   isLoadingImportFeatureFlag$ = this.store$.pipe(select(selectIsLoadingImportFeatureFlag));
   isLoadingUpdateFeatureFlagStatus$ = this.store$.pipe(select(selectIsLoadingUpdateFeatureFlagStatus));
@@ -140,6 +142,10 @@ export class FeatureFlagsService {
     this.store$.dispatch(FeatureFlagsActions.actionUpdateFilterMode({ updateFilterModeRequest }));
   }
 
+  setIsDuplicateKey(duplicateKeyFound: boolean) {
+    this.store$.dispatch(FeatureFlagsActions.actionSetIsDuplicateKey({ duplicateKeyFound }));
+  }
+
   deleteFeatureFlag(flagId: string) {
     this.store$.dispatch(FeatureFlagsActions.actionDeleteFeatureFlag({ flagId }));
   }

frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts

@@ -45,6 +45,11 @@ export const actionAddFeatureFlagSuccess = createAction(
 
 export const actionAddFeatureFlagFailure = createAction('[Feature Flags] Add Feature Flag Failure');
 
+export const actionSetIsDuplicateKey = createAction(
+  '[Feature Flags] Upsert Feature Flag Failure, Duplicate Key found',
+  props<{ duplicateKeyFound: boolean }>()
+);
+
 export const actionDeleteFeatureFlag = createAction('[Feature Flags] Delete Feature Flag', props<{ flagId: string }>());
 
 export const actionDeleteFeatureFlagSuccess = createAction(

frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.effects.ts

@@ -15,12 +15,11 @@ import {
   selectSortAs,
   selectSearchString,
   selectIsAllFlagsFetched,
-  selectSelectedFeatureFlag,
 } from './feature-flags.selectors';
 import { selectCurrentUser } from '../../auth/store/auth.selectors';
 import { CommonExportHelpersService } from '../../../shared/services/common-export-helpers.service';
 import { of } from 'rxjs';
-
+import { SERVER_ERROR } from 'upgrade_types';
 @Injectable()
 export class FeatureFlagsEffects {
   constructor(
@@ -103,7 +102,12 @@ export class FeatureFlagsEffects {
           tap(({ response }) => {
             this.router.navigate(['/featureflags', 'detail', response.id]);
           }),
-          catchError(() => [FeatureFlagsActions.actionAddFeatureFlagFailure()])
+          catchError((res) => {
+            if (res.error.type == SERVER_ERROR.DUPLICATE_KEY) {
+              return [FeatureFlagsActions.actionSetIsDuplicateKey({ duplicateKeyFound: true })];
+            }
+            return [FeatureFlagsActions.actionAddFeatureFlagFailure()];
+          })
         );
       })
     )
@@ -117,7 +121,12 @@ export class FeatureFlagsEffects {
           map((response) => {
             return FeatureFlagsActions.actionUpdateFeatureFlagSuccess({ response });
           }),
-          catchError(() => [FeatureFlagsActions.actionUpdateFeatureFlagFailure()])
+          catchError((res) => {
+            if (res.error.type == SERVER_ERROR.DUPLICATE_KEY) {
+              return [FeatureFlagsActions.actionSetIsDuplicateKey({ duplicateKeyFound: true })];
+            }
+            return [FeatureFlagsActions.actionAddFeatureFlagFailure()];
+          })
         );
       })
     )

frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.model.ts

@@ -186,6 +186,7 @@ export interface FeatureFlagState extends EntityState<FeatureFlag> {
   isLoadingFeatureFlagDelete: boolean;
   isLoadingUpsertPrivateSegmentList: boolean;
   hasInitialFeatureFlagsDataLoaded: boolean;
+  duplicateKeyFound: boolean;
   activeDetailsTabIndex: number;
   skipFlags: number;
   totalFlags: number;

frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts

@@ -20,6 +20,7 @@ export const initialState: FeatureFlagState = adapter.getInitialState({
   isLoadingSelectedFeatureFlag: false,
   isLoadingUpsertPrivateSegmentList: false,
   hasInitialFeatureFlagsDataLoaded: false,
+  duplicateKeyFound: false,
   activeDetailsTabIndex: 0,
   skipFlags: 0,
   totalFlags: null,
@@ -75,6 +76,11 @@ const reducer = createReducer(
     ...state,
     isLoadingUpsertFeatureFlag: false,
   })),
+  on(FeatureFlagsActions.actionSetIsDuplicateKey, (state, { duplicateKeyFound }) => ({
+    ...state,
+    duplicateKeyFound,
+    isLoadingUpsertFeatureFlag: false,
+  })),
 
   // Feature Flag Delete Actions
   on(FeatureFlagsActions.actionDeleteFeatureFlag, (state) => ({ ...state, isLoadingFeatureFlagDelete: true })),

frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts

@@ -47,6 +47,8 @@ export const selectIsLoadingUpsertFeatureFlag = createSelector(
   (state) => state.isLoadingUpsertFeatureFlag
 );
 
+export const selectDuplicateKeyFound = createSelector(selectFeatureFlagsState, (state) => state.duplicateKeyFound);
+
 export const selectSelectedFeatureFlag = createSelector(
   selectRouterState,
   selectFeatureFlagsState,

frontend/projects/upgrade/src/app/core/http-interceptors/http-error.interceptor.ts

@@ -5,6 +5,7 @@ import { Observable, throwError } from 'rxjs';
 import { catchError } from 'rxjs/operators';
 import { ENV, Environment } from '../../../environments/environment-types';
 import { AuthService } from '../auth/auth.service';
+import { SERVER_ERROR } from 'upgrade_types';
 
 @Injectable()
 export class HttpErrorInterceptor implements HttpInterceptor {
@@ -21,7 +22,9 @@ export class HttpErrorInterceptor implements HttpInterceptor {
       content: error.url,
       animate: 'fromRight',
     };
-    this._notifications.create(temp.title, temp.content, temp.type, temp);
+    if (error.error?.type !== SERVER_ERROR.DUPLICATE_KEY) {
+      this._notifications.create(temp.title, temp.content, temp.type, temp);
+    }
   }
 
   intercept(request: HttpRequest<any>, next: HttpHandler): Observable<HttpEvent<any>> {

frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/upsert-feature-flag-modal/upsert-feature-flag-modal.component.html

@@ -5,38 +5,53 @@
   [primaryActionBtnColor]="config.primaryActionBtnColor"
   [primaryActionBtnDisabled]="isPrimaryButtonDisabled$ | async"
   (primaryActionBtnClicked)="onPrimaryActionBtnClicked()"
-  >
+>
   <form [formGroup]="featureFlagForm" class="form-standard dense-3">
     <mat-form-field appearance="outline">
       <mat-label class="ft-14-400">Name</mat-label>
-      <input matInput formControlName="name" placeholder="e.g., My feature flag" class="ft-14-400"/>
+      <input matInput formControlName="name" placeholder="e.g., My feature flag" class="ft-14-400" />
       <mat-hint class="form-hint ft-12-400">
-        {{'feature-flags.upsert-flag-modal.name-hint.text' | translate}}
+        {{ 'feature-flags.upsert-flag-modal.name-hint.text' | translate }}
       </mat-hint>
     </mat-form-field>
 
     <mat-form-field appearance="outline">
       <mat-label class="ft-14-400">Key</mat-label>
-      <input matInput formControlName="key" placeholder="Key for this feature flag" class="ft-14-400"/>
+      <input matInput formControlName="key" placeholder="Key for this feature flag" class="ft-14-400" />
       <mat-hint class="form-hint ft-12-400">
-        {{'feature-flags.upsert-flag-modal.key-hint.text' | translate}}
-        <a class="learn-more-link" href="https://www.upgradeplatform.org/" target="_blank" rel="noopener noreferrer">Learn more</a>
+        <ng-container *ngIf="!validationError; else duplicateError">
+          <span class="key-hint">
+            {{ 'feature-flags.upsert-flag-modal.key-hint.text' | translate }}
+            <a class="learn-more-link" href="https://www.upgradeplatform.org/" target="_blank" rel="noopener noreferrer"
+              >Learn more</a
+            >
+          </span>
+        </ng-container>
+        <ng-template #duplicateError>
+          <span class="ft-14-500 duplicate-error-message">
+            {{ 'feature-flags.upsert-flag-modal.duplicate-key-error.text' | translate }}
+          </span>
+        </ng-template>
       </mat-hint>
     </mat-form-field>
 
     <mat-form-field appearance="outline">
       <mat-label class="ft-14-400">Description (optional)</mat-label>
-      <input matInput formControlName="description" placeholder="Describe your feature flag" class="ft-14-400"/>
+      <input matInput formControlName="description" placeholder="Describe your feature flag" class="ft-14-400" />
     </mat-form-field>
 
     <mat-form-field appearance="outline">
       <mat-label class="ft-14-400">App Context</mat-label>
       <mat-select formControlName="appContext" class="ft-14-400">
-        <mat-option *ngFor="let context of appContexts$ | async" [value]="context" class="ft-14-400">{{ context }}</mat-option>
+        <mat-option *ngFor="let context of appContexts$ | async" [value]="context" class="ft-14-400">{{
+          context
+        }}</mat-option>
       </mat-select>
       <mat-hint class="form-hint ft-12-400">
-        {{ 'feature-flags.upsert-flag-modal.app-context-hint.text' | translate}}
-        <a class="learn-more-link" href="https://www.upgradeplatform.org/" target="_blank" rel="noopener noreferrer">Learn more</a>
+        {{ 'feature-flags.upsert-flag-modal.app-context-hint.text' | translate }}
+        <a class="learn-more-link" href="https://www.upgradeplatform.org/" target="_blank" rel="noopener noreferrer"
+          >Learn more</a
+        >
       </mat-hint>
     </mat-form-field>
 

frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/upsert-feature-flag-modal/upsert-feature-flag-modal.component.scss

@@ -0,0 +1,3 @@
+.duplicate-error-message {
+    color: var(--red);
+}