From 84387a997f6d3cf5ca723adacac0bc31c8f5340d Mon Sep 17 00:00:00 2001 From: pratik Date: Fri, 26 Jul 2024 14:48:25 +0530 Subject: [PATCH 01/28] import ff validation api and frontend integration part1 WIP --- .../api/controllers/FeatureFlagController.ts | 96 ++++++++++++++++++- .../FeatureFlagsPaginatedParamsValidator.ts | 16 ++++ .../src/api/services/FeatureFlagService.ts | 39 ++++++++ .../feature-flags.data.service.ts | 7 +- .../import-feature-flag-modal.component.html | 2 +- .../import-feature-flag-modal.component.ts | 59 ++++++++++-- .../src/environments/environment-types.ts | 1 + .../src/environments/environment.bsnl.ts | 1 + .../src/environments/environment.demo.prod.ts | 1 + .../src/environments/environment.prod.ts | 1 + .../src/environments/environment.staging.ts | 1 + .../upgrade/src/environments/environment.ts | 1 + 12 files changed, 212 insertions(+), 13 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index 889d655682..30bf32fab7 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -4,7 +4,7 @@ import { FeatureFlag } from '../models/FeatureFlag'; import { FeatureFlagSegmentExclusion } from '../models/FeatureFlagSegmentExclusion'; import { FeatureFlagSegmentInclusion } from '../models/FeatureFlagSegmentInclusion'; import { FeatureFlagStatusUpdateValidator } from './validators/FeatureFlagStatusUpdateValidator'; -import { FeatureFlagPaginatedParamsValidator } from './validators/FeatureFlagsPaginatedParamsValidator'; +import { FeatureFlagFile, FeatureFlagPaginatedParamsValidator, ValidatedFeatureFlagsError } from './validators/FeatureFlagsPaginatedParamsValidator'; import { AppRequest, PaginationResponse } from '../../types'; import { SERVER_ERROR } from 'upgrade_types'; import { FeatureFlagValidation, IdValidator, UserParamsValidator } from './validators/FeatureFlagValidator'; @@ -599,4 +599,98 @@ export class FeatureFlagsController { ): Promise { return this.featureFlagService.deleteList(id, request.logger); } + + /** + * @swagger + * /experiments/{validation}: + * post: + * description: Validating Experiment + * consumes: + * - application/json + * parameters: + * - in: body + * name: experiments + * required: true + * schema: + * type: array + * items: + * type: object + * properties: + * fileName: + * type: string + * fileContent: + * type: string + * description: Experiment Files + * tags: + * - Experiments + * produces: + * - application/json + * responses: + * '200': + * description: Validations are completed + * schema: + * type: array + * items: + * type: object + * properties: + * fileName: + * type: string + * error: + * type: string + * '401': + * description: AuthorizationRequiredError + * '500': + * description: Internal Server Error + */ + + /** + * @swagger + * /flags/{validation}: + * post: + * description: Validating Feature Flag + * consumes: + * - application/json + * parameters: + * - in: body + * name: featureFlags + * required: true + * schema: + * type: array + * items: + * type: object + * properties: + * fileName: + * type: string + * fileContent: + * type: string + * description: Import FeatureFlag Files + * tags: + * - Feature Flags + * produces: + * - application/json + * responses: + * '200': + * description: Validations are completed + * schema: + * type: array + * items: + * type: object + * properties: + * fileName: + * type: string + * compatibilityType: + * type: string + * enum: [compatible, warning, incompatible] + * '401': + * description: AuthorizationRequiredError + * '500': + * description: Internal Server Error + */ + @Post('/import/validation') + public async validateImportFeatureFlags( + @Body({ validate: true }) featureFlags: FeatureFlagFile[], + @Req() request: AppRequest + ): Promise { + return await this.featureFlagService.validateImportFeatureFlags(featureFlags, request.logger); + } } diff --git a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagsPaginatedParamsValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagsPaginatedParamsValidator.ts index bfd0c2d54f..1aed56fd47 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagsPaginatedParamsValidator.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagsPaginatedParamsValidator.ts @@ -12,6 +12,22 @@ export interface IFeatureFlagSortParams { sortAs: SORT_AS_DIRECTION; } +export interface FeatureFlagFile { + fileName: string; + fileContent: string; +} + +export interface ValidatedFeatureFlagsError { + fileName: string; + compatibilityType: FF_COMPATIBILITY_TYPE; +} + +export enum FF_COMPATIBILITY_TYPE { + COMPATIBLE = 'compatible', + WARNING = 'warning', + INCOMPATIBLE = 'incompatible', +} + export enum FLAG_SORT_KEY { NAME = 'name', KEY = 'key', diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 7865548fe6..f823251afa 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -13,6 +13,7 @@ import { IFeatureFlagSearchParams, IFeatureFlagSortParams, FLAG_SEARCH_KEY, + ValidatedFeatureFlagsError, } from '../controllers/validators/FeatureFlagsPaginatedParamsValidator'; import { FeatureFlagListValidator } from '../controllers/validators/FeatureFlagListValidator'; import { SERVER_ERROR, FEATURE_FLAG_STATUS, FILTER_MODE, SEGMENT_TYPE } from 'upgrade_types'; @@ -333,4 +334,42 @@ export class FeatureFlagService { const includedFeatureFlags = featureFlags.filter(({ id }) => includedFeatureFlagIds.includes(id)); return includedFeatureFlags; } + + public async validateImportFeatureFlags(featureFlagFiles: any, logger: UpgradeLogger): Promise { + logger.info({ message: 'Validate feature flags' }); + const validationErrors = await Promise.allSettled( + featureFlagFiles.map(async (featureFlagFile) => { + let featureFlag = JSON.parse(featureFlagFile.fileContent); + // const newFeatureFlag = plainToClass(FeatureFlag, featureFlag); + const error = this.validateImportFeatureFlag(featureFlagFile.fileName, featureFlag); + return error; + }) + ); + // Filter out the files that have no promise rejection errors + return validationErrors + .map((result) => { + if (result.status === 'fulfilled') { + return result.value; + } else { + const { fileName, compatibilityType } = result.reason; + return { fileName: fileName, compatibilityType: compatibilityType }; + } + }) + .filter((error) => error !== null); + } + + private validateImportFeatureFlag(fileName: string, flag: any) { + let compatibilityType; + + if (!flag.name || !flag.key || !flag.context) { + compatibilityType = 'incompatible'; + } else { + compatibilityType = 'compatible'; + } + // when should we get the 'warning' compatibility type? + return { + fileName: fileName, + compatibilityType: compatibilityType, + }; + } } diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts index dd9757c9e4..dbd3f71960 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts @@ -9,7 +9,7 @@ import { UpdateFeatureFlagStatusRequest, } from './store/feature-flags.model'; import { Observable } from 'rxjs'; -import { FEATURE_FLAG_STATUS, FILTER_MODE } from '../../../../../../../types/src'; +import { FeatureFlagFile } from '../../features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component'; @Injectable() export class FeatureFlagsDataService { @@ -46,4 +46,9 @@ export class FeatureFlagsDataService { const url = `${this.environment.api.featureFlag}/${flag.id}`; return this.http.put(url, flag); } + + validateFeatureFlag(featureFlag: FeatureFlagFile[]) { + const url = this.environment.api.validateFeatureFlag; + return this.http.post(url, featureFlag); + } } diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html index 6632f185b8..64254ee08b 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html @@ -11,7 +11,7 @@ diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts index fced0d1298..8cec40e111 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts @@ -6,7 +6,17 @@ import { BehaviorSubject } from 'rxjs'; import { CommonModule } from '@angular/common'; import { SharedModule } from '../../../../../shared/shared.module'; import { CommonImportContainerComponent } from '../../../../../shared-standalone-component-lib/components/common-import-container/common-import-container.component'; +import { FeatureFlagsDataService } from '../../../../../core/feature-flags/feature-flags.data.service'; +export interface FeatureFlagFile { + fileName: string; + fileContent: string | ArrayBuffer; +} + +export interface ValidateFeatureFlagError { + fileName: string; + compatibilityType: string; +} @Component({ selector: 'app-import-feature-flag-modal', standalone: true, @@ -18,29 +28,58 @@ import { CommonImportContainerComponent } from '../../../../../shared-standalone export class ImportFeatureFlagModalComponent { isImportActionBtnDisabled = new BehaviorSubject(true); + importFileErrors: { fileName: string; compatibilityType: string }[] = []; + allFeatureFlags: FeatureFlagFile[] = []; + uploadedFileCount = 0; constructor( @Inject(MAT_DIALOG_DATA) public data: CommonModalConfig, public dialog: MatDialog, + public featureFlagsDataService: FeatureFlagsDataService, public dialogRef: MatDialogRef ) {} - handleFilesSelected(files: File[]) { - if(files.length>0) { + async handleFilesSelected(event) { + if(event.target.files.length>0) { this.isImportActionBtnDisabled.next(false); } - console.log('Selected files:', files); //Send files to validation endpoint to receive data for table + this.uploadedFileCount = event.target.files.length; + this.importFileErrors = []; + this.allFeatureFlags = []; + + if (this.uploadedFileCount === 0) return; + + // Set loading to true before processing the files + // this.isLoadingExperiments$ = true; + + const filesArray = Array.from(event.target.files) as any[]; + await Promise.all( + filesArray.map((file) => { + return new Promise((resolve) => { + const reader = new FileReader(); + reader.onload = (e) => { + const fileContent = e.target.result; + this.allFeatureFlags.push({ fileName: file.name, fileContent: fileContent }); + resolve(); + }; + reader.readAsText(file); + }); + }) + ); + + await this.checkValidation(this.allFeatureFlags); + // this.isLoadingFeatureFlag$ = false; + const compatibility = this.checkValidation(event.target.files); } - handleFileInput(file: File) { - const reader = new FileReader(); - reader.onload = (e: any) => { - const jsonContent = e.target.result; - console.log(JSON.parse(jsonContent)); - }; - reader.readAsText(file); + async checkValidation(files) { + this.importFileErrors = + ( + (await this.featureFlagsDataService.validateFeatureFlag(files).toPromise()) as ValidateFeatureFlagError[] + ).filter((data) => data.compatibilityType != null) || []; + console.log("Import file errors", this.importFileErrors); } importFiles() { diff --git a/frontend/projects/upgrade/src/environments/environment-types.ts b/frontend/projects/upgrade/src/environments/environment-types.ts index 3580d10186..1919215e6f 100644 --- a/frontend/projects/upgrade/src/environments/environment-types.ts +++ b/frontend/projects/upgrade/src/environments/environment-types.ts @@ -29,6 +29,7 @@ export interface APIEndpoints { allPartitions: string; allExperimentNames: string; featureFlag: string; + validateFeatureFlag: string; updateFlagStatus: string; getPaginatedFlags: string; setting: string; diff --git a/frontend/projects/upgrade/src/environments/environment.bsnl.ts b/frontend/projects/upgrade/src/environments/environment.bsnl.ts index 78d11f14ff..445b7febaf 100644 --- a/frontend/projects/upgrade/src/environments/environment.bsnl.ts +++ b/frontend/projects/upgrade/src/environments/environment.bsnl.ts @@ -44,6 +44,7 @@ export const environment = { featureFlag: '/flags', updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', + validateFeatureFlag: '/flags/import/validation', setting: '/setting', metrics: '/metric', metricsSave: '/metric/save', diff --git a/frontend/projects/upgrade/src/environments/environment.demo.prod.ts b/frontend/projects/upgrade/src/environments/environment.demo.prod.ts index 075a02037f..184388e166 100755 --- a/frontend/projects/upgrade/src/environments/environment.demo.prod.ts +++ b/frontend/projects/upgrade/src/environments/environment.demo.prod.ts @@ -44,6 +44,7 @@ export const environment = { featureFlag: '/flags', updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', + validateFeatureFlag: '/flags/import/validation', setting: '/setting', metrics: '/metric', metricsSave: '/metric/save', diff --git a/frontend/projects/upgrade/src/environments/environment.prod.ts b/frontend/projects/upgrade/src/environments/environment.prod.ts index fb7aad0702..9bf437d5ca 100755 --- a/frontend/projects/upgrade/src/environments/environment.prod.ts +++ b/frontend/projects/upgrade/src/environments/environment.prod.ts @@ -44,6 +44,7 @@ export const environment = { featureFlag: '/flags', updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', + validateFeatureFlag: '/flags/import/validation', setting: '/setting', metrics: '/metric', metricsSave: '/metric/save', diff --git a/frontend/projects/upgrade/src/environments/environment.staging.ts b/frontend/projects/upgrade/src/environments/environment.staging.ts index 144bb38cdf..02cfd262e5 100644 --- a/frontend/projects/upgrade/src/environments/environment.staging.ts +++ b/frontend/projects/upgrade/src/environments/environment.staging.ts @@ -44,6 +44,7 @@ export const environment = { featureFlag: '/flags', updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', + validateFeatureFlag: '/flags/import/validation', setting: '/setting', metrics: '/metric', metricsSave: '/metric/save', diff --git a/frontend/projects/upgrade/src/environments/environment.ts b/frontend/projects/upgrade/src/environments/environment.ts index 774d1998ab..ed99e3716b 100755 --- a/frontend/projects/upgrade/src/environments/environment.ts +++ b/frontend/projects/upgrade/src/environments/environment.ts @@ -48,6 +48,7 @@ export const environment = { featureFlag: '/flags', updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', + validateFeatureFlag: '/flags/import/validation', setting: '/setting', metrics: '/metric', metricsSave: '/metric/save', From 855ee4d28b358881cb56207ff37f278499038e74 Mon Sep 17 00:00:00 2001 From: Yagnik Date: Fri, 26 Jul 2024 15:11:46 +0530 Subject: [PATCH 02/28] merge conflict resolved --- .../import-feature-flag-modal.component.html | 27 ------------------- 1 file changed, 27 deletions(-) diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html index 17b386b19a..c11a9dd267 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html @@ -1,29 +1,3 @@ -<<<<<<< HEAD -
- -
- - -

- {{ 'feature-flags.import-feature-flag.message.text' | translate }} - Learn More -

-
- -
-======= ->>>>>>> f28434a62d6bcb9c975cbf41eb22fc6270fc0ea0 From 61ae2892b89691e17fdf320d090c3dd7b8161836 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Mon, 29 Jul 2024 19:17:11 +0530 Subject: [PATCH 03/28] import validation to return compatibility --- .../api/controllers/FeatureFlagController.ts | 22 +++++++----- .../src/api/services/FeatureFlagService.ts | 36 +++++++++++++------ .../import-feature-flag-modal.component.html | 2 +- .../import-feature-flag-modal.component.ts | 5 ++- 4 files changed, 41 insertions(+), 24 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index 30bf32fab7..d0d54198cb 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -4,7 +4,11 @@ import { FeatureFlag } from '../models/FeatureFlag'; import { FeatureFlagSegmentExclusion } from '../models/FeatureFlagSegmentExclusion'; import { FeatureFlagSegmentInclusion } from '../models/FeatureFlagSegmentInclusion'; import { FeatureFlagStatusUpdateValidator } from './validators/FeatureFlagStatusUpdateValidator'; -import { FeatureFlagFile, FeatureFlagPaginatedParamsValidator, ValidatedFeatureFlagsError } from './validators/FeatureFlagsPaginatedParamsValidator'; +import { + FeatureFlagFile, + FeatureFlagPaginatedParamsValidator, + ValidatedFeatureFlagsError, +} from './validators/FeatureFlagsPaginatedParamsValidator'; import { AppRequest, PaginationResponse } from '../../types'; import { SERVER_ERROR } from 'upgrade_types'; import { FeatureFlagValidation, IdValidator, UserParamsValidator } from './validators/FeatureFlagValidator'; @@ -600,7 +604,7 @@ export class FeatureFlagsController { return this.featureFlagService.deleteList(id, request.logger); } - /** + /** * @swagger * /experiments/{validation}: * post: @@ -643,7 +647,7 @@ export class FeatureFlagsController { * description: Internal Server Error */ - /** + /** * @swagger * /flags/{validation}: * post: @@ -686,11 +690,11 @@ export class FeatureFlagsController { * '500': * description: Internal Server Error */ - @Post('/import/validation') - public async validateImportFeatureFlags( - @Body({ validate: true }) featureFlags: FeatureFlagFile[], - @Req() request: AppRequest - ): Promise { + @Post('/import/validation') + public async validateImportFeatureFlags( + @Body({ validate: true }) featureFlags: FeatureFlagFile[], + @Req() request: AppRequest + ): Promise { return await this.featureFlagService.validateImportFeatureFlags(featureFlags, request.logger); - } + } } diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index f823251afa..3076934f99 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -14,6 +14,7 @@ import { IFeatureFlagSortParams, FLAG_SEARCH_KEY, ValidatedFeatureFlagsError, + FF_COMPATIBILITY_TYPE, } from '../controllers/validators/FeatureFlagsPaginatedParamsValidator'; import { FeatureFlagListValidator } from '../controllers/validators/FeatureFlagListValidator'; import { SERVER_ERROR, FEATURE_FLAG_STATUS, FILTER_MODE, SEGMENT_TYPE } from 'upgrade_types'; @@ -335,13 +336,16 @@ export class FeatureFlagService { return includedFeatureFlags; } - public async validateImportFeatureFlags(featureFlagFiles: any, logger: UpgradeLogger): Promise { + public async validateImportFeatureFlags( + featureFlagFiles: any, + logger: UpgradeLogger + ): Promise { logger.info({ message: 'Validate feature flags' }); const validationErrors = await Promise.allSettled( featureFlagFiles.map(async (featureFlagFile) => { - let featureFlag = JSON.parse(featureFlagFile.fileContent); + const featureFlag = JSON.parse(featureFlagFile.fileContent); // const newFeatureFlag = plainToClass(FeatureFlag, featureFlag); - const error = this.validateImportFeatureFlag(featureFlagFile.fileName, featureFlag); + const error = await this.validateImportFeatureFlag(featureFlagFile.fileName, featureFlag); return error; }) ); @@ -358,18 +362,28 @@ export class FeatureFlagService { .filter((error) => error !== null); } - private validateImportFeatureFlag(fileName: string, flag: any) { - let compatibilityType; + private async validateImportFeatureFlag(fileName: string, flag: any) { + let compatibilityType = FF_COMPATIBILITY_TYPE.COMPATIBLE; if (!flag.name || !flag.key || !flag.context) { - compatibilityType = 'incompatible'; + compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; } else { - compatibilityType = 'compatible'; + const segmentIds = [ + ...flag.featureFlagSegmentInclusion.map((segmentInclusion) => segmentInclusion.segment.id), + ...flag.featureFlagSegmentExclusion.map((segmentExclusion) => segmentExclusion.segment.id), + ]; + + const segments = await this.segmentService.getSegmentByIds(segmentIds); + segments.forEach((segment) => { + if (segment == undefined) { + compatibilityType = FF_COMPATIBILITY_TYPE.WARNING; + } + }); } - // when should we get the 'warning' compatibility type? + return { - fileName: fileName, - compatibilityType: compatibilityType, - }; + fileName: fileName, + compatibilityType: compatibilityType, + }; } } diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html index c11a9dd267..b8388ceb87 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html @@ -10,7 +10,7 @@

diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts index 8cec40e111..8353501b04 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts @@ -26,7 +26,6 @@ export interface ValidateFeatureFlagError { changeDetection: ChangeDetectionStrategy.OnPush, }) export class ImportFeatureFlagModalComponent { - isImportActionBtnDisabled = new BehaviorSubject(true); importFileErrors: { fileName: string; compatibilityType: string }[] = []; allFeatureFlags: FeatureFlagFile[] = []; @@ -41,7 +40,7 @@ export class ImportFeatureFlagModalComponent { ) {} async handleFilesSelected(event) { - if(event.target.files.length>0) { + if (event.target.files.length > 0) { this.isImportActionBtnDisabled.next(false); } //Send files to validation endpoint to receive data for table @@ -79,7 +78,7 @@ export class ImportFeatureFlagModalComponent { ( (await this.featureFlagsDataService.validateFeatureFlag(files).toPromise()) as ValidateFeatureFlagError[] ).filter((data) => data.compatibilityType != null) || []; - console.log("Import file errors", this.importFileErrors); + console.log('Import file errors', this.importFileErrors); } importFiles() { From 480e16e4d2e959f76511bdf87d7f762fd833438f Mon Sep 17 00:00:00 2001 From: Yagnik Date: Mon, 5 Aug 2024 17:11:49 +0530 Subject: [PATCH 04/28] validation req, table, loading spinner and import action button code --- .../feature-flags/feature-flags.service.ts | 8 +- .../store/feature-flags.actions.ts | 5 ++ .../store/feature-flags.model.ts | 1 + .../store/feature-flags.reducer.ts | 5 ++ .../store/feature-flags.selectors.ts | 5 ++ .../delete-feature-flag-modal.component.ts | 4 +- .../import-feature-flag-modal.component.html | 77 +++++++++++++++--- .../import-feature-flag-modal.component.scss | 80 ++++++++++++++++++- .../import-feature-flag-modal.component.ts | 67 ++++++++++------ .../import-segment.component.ts | 6 +- ...ommon-status-indicator-chip.component.scss | 6 +- .../projects/upgrade/src/assets/i18n/en.json | 2 + types/src/Experiment/enums.ts | 3 + 13 files changed, 225 insertions(+), 44 deletions(-) diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts index 365aaa941b..e9fc9b9d9f 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts @@ -22,6 +22,7 @@ import { selectSortKey, selectSortAs, selectAppContexts, + selectIsLoadingImportFeatureFlag, } from './store/feature-flags.selectors'; import * as FeatureFlagsActions from './store/feature-flags.actions'; import { actionFetchContextMetaData } from '../experiments/store/experiments.actions'; @@ -52,7 +53,8 @@ export class FeatureFlagsService { sortKey$ = this.store$.pipe(select(selectSortKey)); sortAs$ = this.store$.pipe(select(selectSortAs)); isLoadingUpsertFeatureFlag$ = this.store$.pipe(select(selectIsLoadingUpsertFeatureFlag)); - IsLoadingFeatureFlagDelete$ = this.store$.pipe(select(selectIsLoadingFeatureFlagDelete)); + isLoadingFeatureFlagDelete$ = this.store$.pipe(select(selectIsLoadingFeatureFlagDelete)); + isLoadingImportFeatureFlag$ = this.store$.pipe(select(selectIsLoadingImportFeatureFlag)); hasFeatureFlagsCountChanged$ = this.allFeatureFlags$.pipe( pairwise(), @@ -124,6 +126,10 @@ export class FeatureFlagsService { this.store$.dispatch(FeatureFlagsActions.actionDeleteFeatureFlag({ flagId })); } + setIsLoadingImportFeatureFlag(isLoadingImportFeatureFlag: boolean) { + this.store$.dispatch(FeatureFlagsActions.actionSetIsLoadingImportFeatureFlag({ isLoadingImportFeatureFlag })); + } + setSearchKey(searchKey: FLAG_SEARCH_KEY) { this.store$.dispatch(FeatureFlagsActions.actionSetSearchKey({ searchKey })); } diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts index c69b5d02f2..a6bab58314 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts @@ -65,6 +65,11 @@ export const actionUpdateFeatureFlagSuccess = createAction( export const actionUpdateFeatureFlagFailure = createAction('[Feature Flags] Update Feature Flag Failure'); +export const actionSetIsLoadingImportFeatureFlag = createAction( + '[Feature Flags] Set Is Loading for Flag Import', + props<{ isLoadingImportFeatureFlag: boolean }>() +); + export const actionSetIsLoadingFeatureFlags = createAction( '[Feature Flags] Set Is Loading Flags', props<{ isLoadingFeatureFlags: boolean }>() 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 f33c729141..1d47661d20 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 @@ -161,6 +161,7 @@ export const FLAG_ROOT_DISPLAYED_COLUMNS = Object.values(FLAG_ROOT_COLUMN_NAMES) export interface FeatureFlagState extends EntityState { isLoadingUpsertFeatureFlag: boolean; + isLoadingImportFeatureFlag: boolean; isLoadingSelectedFeatureFlag: boolean; isLoadingFeatureFlags: boolean; isLoadingUpdateFeatureFlagStatus: boolean; diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts index 3eefa1f6c8..6c82d9bfee 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.reducer.ts @@ -11,6 +11,7 @@ export const { selectIds, selectEntities, selectAll, selectTotal } = adapter.get export const initialState: FeatureFlagState = adapter.getInitialState({ isLoadingUpsertFeatureFlag: false, + isLoadingImportFeatureFlag: false, isLoadingFeatureFlags: false, isLoadingUpdateFeatureFlagStatus: false, isLoadingFeatureFlagDetail: false, @@ -109,6 +110,10 @@ const reducer = createReducer( ...state, isLoadingFeatureFlags, })), + on(FeatureFlagsActions.actionSetIsLoadingImportFeatureFlag, (state, { isLoadingImportFeatureFlag }) => ({ + ...state, + isLoadingImportFeatureFlag, + })), on(FeatureFlagsActions.actionSetSkipFlags, (state, { skipFlags }) => ({ ...state, skipFlags })), on(FeatureFlagsActions.actionSetSearchKey, (state, { searchKey }) => ({ ...state, searchKey })), on(FeatureFlagsActions.actionSetSearchString, (state, { searchString }) => ({ ...state, searchValue: searchString })), diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts index f36a2a67e0..1f5c2824e5 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.selectors.ts @@ -113,6 +113,11 @@ export const selectFeatureFlagsListLength = createSelector( (featureFlags) => featureFlags.length ); +export const selectIsLoadingImportFeatureFlag = createSelector( + selectFeatureFlagsState, + (state) => state.isLoadingImportFeatureFlag +); + export const selectIsLoadingUpdateFeatureFlagStatus = createSelector( selectFeatureFlagsState, (state) => state.isLoadingUpdateFeatureFlagStatus diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/delete-feature-flag-modal/delete-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/delete-feature-flag-modal/delete-feature-flag-modal.component.ts index 678d70054b..61ff39c9d4 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/delete-feature-flag-modal/delete-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/delete-feature-flag-modal/delete-feature-flag-modal.component.ts @@ -41,7 +41,7 @@ export class DeleteFeatureFlagModalComponent { inputValue = ''; subscriptions = new Subscription(); isSelectedFeatureFlagRemoved$ = this.featureFlagsService.isSelectedFeatureFlagRemoved$; - IsLoadingFeatureFlagDelete$ = this.featureFlagsService.IsLoadingFeatureFlagDelete$; + isLoadingFeatureFlagDelete$ = this.featureFlagsService.isLoadingFeatureFlagDelete$; private inputSubject: BehaviorSubject = new BehaviorSubject(''); // Observable that emits true if inputValue is 'delete', false otherwise @@ -49,7 +49,7 @@ export class DeleteFeatureFlagModalComponent { isDeleteActionBtnDisabled$: Observable = combineLatest([ this.isDeleteNotTyped$, - this.IsLoadingFeatureFlagDelete$, + this.isLoadingFeatureFlagDelete$, ]).pipe(map(([isDeleteNotTyped, isLoading]) => isDeleteNotTyped || isLoading)); constructor( diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html index b8388ceb87..5105d6abce 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html @@ -7,15 +7,72 @@ (primaryActionBtnClicked)="importFiles()" >

- - -

- {{ 'feature-flags.import-feature-flag.message.text' | translate }} - Learn more -

+ + + + +

+ {{ 'feature-flags.import-feature-flag.message.text' | translate }} + Learn more +

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ + File Name {{element.fileName}} Compatibility Type + + +
+
+ {{'feature-flags.import-flag-modal.compatibility-description.incompatible.text' | translate}} +
+
+ {{'feature-flags.import-flag-modal.compatibility-description.warning.text' | translate}} +
+
+
+ + + + +
diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.scss b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.scss index d517e78705..0f8a42ddf3 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.scss +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.scss @@ -8,5 +8,83 @@ text-indent: 16px; margin-bottom: 0; } -} + .spinner { + width: 100%; + margin: auto; + } + + .file-compatibility-table { + ::ng-deep thead { + background-color: var(--zircon); + + tr.mat-mdc-header-row { + height: 48px; + border: 0; + + th { + padding-left: 0; + color: var(--darker-grey); + + &:first-child { + border-top-left-radius: 4px; + } + + &:last-child { + border-top-right-radius: 4px; + } + } + } + } + + ::ng-deep tbody { + tr.mat-mdc-row { + height: 56px; + + td { + min-width: 96px; + padding-left: 0; + color: var(--black-2); + } + } + + tr.mat-mdc-no-data-row { + td { + height: 48px; + text-align: center; + border: 1.5px dashed var(--light-grey-2); + color: var(--dark-grey); + } + } + } + + &.disabled { + opacity: 0.5; + cursor: default; + } + + .cdk-column-actions { + width: 10%; + } + + .cdk-column-fileName { + width: 60%; + } + + .cdk-column-compatibilityType { + width: 30%; + } + + .expandable-row { + padding: 0; + width: 100%; + + .compatibility-details-container { + min-height: 48px; + padding: 12px 5% 12px 10%; + display: flex; + align-items: center; + } + } + } +} diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts index e5dc6f99c7..0ce3899ad6 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts @@ -1,12 +1,14 @@ -import { ChangeDetectionStrategy, Component, ElementRef, Inject, OnInit, ViewChild } from '@angular/core'; -import { CommonModalComponent } from '../../../../../shared-standalone-component-lib/components'; -import { MAT_DIALOG_DATA, MatDialog, MatDialogRef } from '@angular/material/dialog'; -import { BehaviorSubject } from 'rxjs'; +import { ChangeDetectionStrategy, Component, Inject } from '@angular/core'; +import { CommonModalComponent, CommonStatusIndicatorChipComponent } from '../../../../../shared-standalone-component-lib/components'; +import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; +import { BehaviorSubject, Observable, combineLatest, map } from 'rxjs'; import { CommonModule } from '@angular/common'; import { SharedModule } from '../../../../../shared/shared.module'; import { CommonImportContainerComponent } from '../../../../../shared-standalone-component-lib/components/common-import-container/common-import-container.component'; import { FeatureFlagsDataService } from '../../../../../core/feature-flags/feature-flags.data.service'; import { CommonModalConfig } from '../../../../../shared-standalone-component-lib/components/common-modal/common-modal.types'; +import { FeatureFlagsService } from '../../../../../core/feature-flags/feature-flags.service'; +import { MatTableDataSource } from '@angular/material/table'; export interface FeatureFlagFile { fileName: string; @@ -17,24 +19,33 @@ export interface ValidateFeatureFlagError { fileName: string; compatibilityType: string; } + @Component({ selector: 'app-import-feature-flag-modal', standalone: true, - imports: [CommonModalComponent, CommonModule, SharedModule, CommonImportContainerComponent], + imports: [CommonModalComponent, CommonModule, SharedModule, CommonImportContainerComponent, CommonStatusIndicatorChipComponent], templateUrl: './import-feature-flag-modal.component.html', styleUrls: ['./import-feature-flag-modal.component.scss'], changeDetection: ChangeDetectionStrategy.OnPush, }) export class ImportFeatureFlagModalComponent { + isDescriptionExpanded = false; + compatibilityDescription = ''; + displayedColumns: string[] = ['actions', 'fileName', 'compatibilityType']; isImportActionBtnDisabled = new BehaviorSubject(true); - importFileErrors: { fileName: string; compatibilityType: string }[] = []; - allFeatureFlags: FeatureFlagFile[] = []; - uploadedFileCount = 0; + importFileErrorsDataSource = new MatTableDataSource(); + importFileErrors: ValidateFeatureFlagError[] = []; + fileData: FeatureFlagFile[] = []; + uploadedFileCount = new BehaviorSubject(0); + isLoadingImportFeatureFlag$ = this.featureFlagsService.isLoadingImportFeatureFlag$; + isImportActionBtnDisabled$: Observable = combineLatest([ + this.uploadedFileCount, + this.isLoadingImportFeatureFlag$, + ]).pipe(map(([uploadedCount, isLoading]) => isLoading || uploadedCount === 0)); constructor( - @Inject(MAT_DIALOG_DATA) - public data: CommonModalConfig, - public dialog: MatDialog, + @Inject(MAT_DIALOG_DATA) public data: CommonModalConfig, + public featureFlagsService: FeatureFlagsService, public featureFlagsDataService: FeatureFlagsDataService, public dialogRef: MatDialogRef ) {} @@ -42,16 +53,14 @@ export class ImportFeatureFlagModalComponent { async handleFilesSelected(event) { if (event.target.files.length > 0) { this.isImportActionBtnDisabled.next(false); + this.featureFlagsService.setIsLoadingImportFeatureFlag(true); } - //Send files to validation endpoint to receive data for table - this.uploadedFileCount = event.target.files.length; - this.importFileErrors = []; - this.allFeatureFlags = []; - if (this.uploadedFileCount === 0) return; + this.uploadedFileCount.next(event.target.files.length); + this.importFileErrors = []; + this.fileData = []; - // Set loading to true before processing the files - // this.isLoadingExperiments$ = true; + if (event.target.files.length === 0) return; const filesArray = Array.from(event.target.files) as any[]; await Promise.all( @@ -60,7 +69,7 @@ export class ImportFeatureFlagModalComponent { const reader = new FileReader(); reader.onload = (e) => { const fileContent = e.target.result; - this.allFeatureFlags.push({ fileName: file.name, fileContent: fileContent }); + this.fileData.push({ fileName: file.name, fileContent: fileContent }); resolve(); }; reader.readAsText(file); @@ -68,17 +77,27 @@ export class ImportFeatureFlagModalComponent { }) ); - await this.checkValidation(this.allFeatureFlags); - // this.isLoadingFeatureFlag$ = false; - const compatibility = this.checkValidation(event.target.files); + await this.checkValidation(this.fileData); } - async checkValidation(files) { + async checkValidation(files: FeatureFlagFile[]) { this.importFileErrors = ( (await this.featureFlagsDataService.validateFeatureFlag(files).toPromise()) as ValidateFeatureFlagError[] ).filter((data) => data.compatibilityType != null) || []; - console.log('Import file errors', this.importFileErrors); + this.importFileErrorsDataSource.data = this.importFileErrors; + this.featureFlagsService.setIsLoadingImportFeatureFlag(false); + if (this.importFileErrors.length > 0) { + this.importFileErrors.forEach(error => { + if (error.compatibilityType === 'incompatible') { + this.isImportActionBtnDisabled.next(true); + }; + }); + } + } + + toggleExpand() { + this.isDescriptionExpanded = !this.isDescriptionExpanded; } importFiles() { diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts index 95da8f2329..67e8b48ca5 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts @@ -27,7 +27,7 @@ export class ImportSegmentComponent { private segmentsService: SegmentsService, private translate: TranslateService, private notificationService: NotificationService, - private segmentdataService: SegmentsDataService, + private segmentDataService: SegmentsDataService, public dialogRef: MatDialogRef, @Inject(MAT_DIALOG_DATA) public data: any ) {} @@ -38,7 +38,7 @@ export class ImportSegmentComponent { async importSegments() { this.onCancelClick(); - const importResult = (await this.segmentdataService + const importResult = (await this.segmentDataService .importSegments(this.fileData) .toPromise()) as SegmentImportError[]; @@ -90,7 +90,7 @@ export class ImportSegmentComponent { async validateFiles() { this.importFileErrors = - ((await this.segmentdataService.validateSegments(this.fileData).toPromise()) as SegmentImportError[]) || []; + ((await this.segmentDataService.validateSegments(this.fileData).toPromise()) as SegmentImportError[]) || []; this.importFileErrorsDataSource.data = this.importFileErrors; this.nonErrorSegments = this.uploadedFileCount - this.importFileErrors.length; diff --git a/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-status-indicator-chip/common-status-indicator-chip.component.scss b/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-status-indicator-chip/common-status-indicator-chip.component.scss index 3d96cbb1e3..e99b65732d 100644 --- a/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-status-indicator-chip/common-status-indicator-chip.component.scss +++ b/frontend/projects/upgrade/src/app/shared-standalone-component-lib/components/common-status-indicator-chip/common-status-indicator-chip.component.scss @@ -24,7 +24,7 @@ background-color: var(--status-draft-alpha); } - &.inactive { + &.inactive, &.warning { --mdc-chip-label-text-color: var(--status-inactive); border: 1px solid var(--status-inactive); background-color: var(--status-inactive-alpha); @@ -36,13 +36,13 @@ background-color: var(--status-enrolling-alpha); } - &.enrollment-complete { + &.enrollment-complete, &.compatible { --mdc-chip-label-text-color: var(--status-enrollment-complete); border: 1px solid var(--status-enrollment-complete); background-color: var(--status-enrollment-complete-alpha); } - &.cancelled { + &.cancelled, &.incompatible { --mdc-chip-label-text-color: var(--status-cancelled); border: 1px solid var(--status-cancelled); background-color: var(--status-cancelled-alpha); diff --git a/frontend/projects/upgrade/src/assets/i18n/en.json b/frontend/projects/upgrade/src/assets/i18n/en.json index 9b2f9950c9..d614dc437e 100644 --- a/frontend/projects/upgrade/src/assets/i18n/en.json +++ b/frontend/projects/upgrade/src/assets/i18n/en.json @@ -356,6 +356,8 @@ "feature-flags.no-flags-in-table.text": "No feature flags is defined. Create a new feature flag.", "feature-flags.delete-flag.message.text": "Type the name of feature flag to confirm deletion:", "feature-flags.delete-flag.input-placeholder.text": "Feature flag name", + "feature-flags.import-flag-modal.compatibility-description.incompatible.text": "This JSON file cannot be imported because it lacks required properties or it incorrectly formatted for the feature flag. Please ensure it is the correct file.", + "feature-flags.import-flag-modal.compatibility-description.warning.text": "This JSON file can be imported, but it may contain outdated or missing properties/features. Please review the feature flag details post-import.", "feature-flags.upsert-flag-modal.name-hint.text": "The name for this feature flag.", "feature-flags.upsert-flag-modal.key-hint.text": "A unique key used to retrieve this feature flag from the client application.", "feature-flags.upsert-flag-modal.app-context-hint.text": "The App Context indicates where the feature flag will run, known to UpGrade.", diff --git a/types/src/Experiment/enums.ts b/types/src/Experiment/enums.ts index d7a4228b5c..12d7f50c3a 100644 --- a/types/src/Experiment/enums.ts +++ b/types/src/Experiment/enums.ts @@ -265,6 +265,9 @@ export enum STATUS_INDICATOR_CHIP_TYPE { ENROLLMENT_COMPLETE = 'enrollment-complete', CANCELLED = 'cancelled', SCHEDULED = 'scheduled', + COMPATIBLE = 'compatible', + INCOMPATIBLE = 'incompatible', + WARNING = 'warning', } export enum FEATURE_FLAG_PARTICIPANT_LIST_KEY { From 042415b2b5a4f0db192a564d8779bfc6b3deab11 Mon Sep 17 00:00:00 2001 From: Yagnik Date: Mon, 5 Aug 2024 17:13:58 +0530 Subject: [PATCH 05/28] resolved merge conflicts --- .../src/app/core/feature-flags/feature-flags.data.service.ts | 3 --- 1 file changed, 3 deletions(-) diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts index bc3e3f73f7..03eab72567 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts @@ -11,11 +11,8 @@ import { UpdateFeatureFlagStatusRequest, } from './store/feature-flags.model'; import { Observable } from 'rxjs'; -<<<<<<< HEAD import { FeatureFlagFile } from '../../features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component'; -======= import { AddPrivateSegmentListRequest, EditPrivateSegmentListRequest } from '../segments/store/segments.model'; ->>>>>>> cd2e80f22666b0e4270b9f12ab77aa8b195f8cba @Injectable() export class FeatureFlagsDataService { From 2807b756e795b96b2645492a75ceb5727618d69e Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Tue, 6 Aug 2024 11:32:53 +0530 Subject: [PATCH 06/28] solve git comments --- .../src/api/services/FeatureFlagService.ts | 22 ++++++++++++++----- 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index dfa69b73d8..5aeced8630 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -15,6 +15,7 @@ import { FLAG_SEARCH_KEY, ValidatedFeatureFlagsError, FF_COMPATIBILITY_TYPE, + FeatureFlagFile, } from '../controllers/validators/FeatureFlagsPaginatedParamsValidator'; import { FeatureFlagListValidator } from '../controllers/validators/FeatureFlagListValidator'; import { SERVER_ERROR, FEATURE_FLAG_STATUS, FILTER_MODE, SEGMENT_TYPE } from 'upgrade_types'; @@ -276,12 +277,12 @@ export class FeatureFlagService { if (filterType === 'inclusion') { existingRecord = await this.featureFlagSegmentInclusionRepository.findOne({ where: { featureFlag: { id: listInput.flagId }, segment: { id: listInput.list.id } }, - relations: ['featureFlag', 'segment'] + relations: ['featureFlag', 'segment'], }); } else { existingRecord = await this.featureFlagSegmentExclusionRepository.findOne({ where: { featureFlag: { id: listInput.flagId }, segment: { id: listInput.list.id } }, - relations: ['featureFlag', 'segment'] + relations: ['featureFlag', 'segment'], }); } @@ -401,14 +402,23 @@ export class FeatureFlagService { } public async validateImportFeatureFlags( - featureFlagFiles: any, + featureFlagFiles: FeatureFlagFile[], logger: UpgradeLogger ): Promise { logger.info({ message: 'Validate feature flags' }); const validationErrors = await Promise.allSettled( featureFlagFiles.map(async (featureFlagFile) => { - const featureFlag = JSON.parse(featureFlagFile.fileContent); - // const newFeatureFlag = plainToClass(FeatureFlag, featureFlag); + let featureFlag: FeatureFlag; + try { + featureFlag = JSON.parse(featureFlagFile.fileContent); + } catch (parseError) { + logger.error({ message: 'Error in parsing feature flag file', details: parseError }); + return { + fileName: featureFlagFile.fileName, + compatibilityType: FF_COMPATIBILITY_TYPE.INCOMPATIBLE, + }; + } + const error = await this.validateImportFeatureFlag(featureFlagFile.fileName, featureFlag); return error; }) @@ -426,7 +436,7 @@ export class FeatureFlagService { .filter((error) => error !== null); } - private async validateImportFeatureFlag(fileName: string, flag: any) { + private async validateImportFeatureFlag(fileName: string, flag: FeatureFlag) { let compatibilityType = FF_COMPATIBILITY_TYPE.COMPATIBLE; if (!flag.name || !flag.key || !flag.context) { From bd3b41b0acc8ff1b34251753603b6592b086a705 Mon Sep 17 00:00:00 2001 From: Yagnik Date: Wed, 7 Aug 2024 10:10:42 +0530 Subject: [PATCH 07/28] resolved the review cmt --- .../core/feature-flags/feature-flags.data.service.ts | 2 +- .../core/feature-flags/store/feature-flags.model.ts | 10 ++++++++++ .../import-feature-flag-modal.component.ts | 11 +---------- 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts index eff6826867..248a21e8fa 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts @@ -4,13 +4,13 @@ import { HttpClient, HttpParams } from '@angular/common/http'; import { AddFeatureFlagRequest, FeatureFlag, + FeatureFlagFile, FeatureFlagSegmentListDetails, FeatureFlagsPaginationInfo, FeatureFlagsPaginationParams, UpdateFeatureFlagRequest, UpdateFeatureFlagStatusRequest, } from './store/feature-flags.model'; -import { FeatureFlagFile } from '../../features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component'; import { Observable, delay, of } from 'rxjs'; import { AddPrivateSegmentListRequest, EditPrivateSegmentListRequest } from '../segments/store/segments.model'; 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 204601d3c1..5342a451f7 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 @@ -58,6 +58,16 @@ export enum UPSERT_FEATURE_FLAG_LIST_ACTION { EDIT = 'edit', } +export interface FeatureFlagFile { + fileName: string; + fileContent: string | ArrayBuffer; +} + +export interface ValidateFeatureFlagError { + fileName: string; + compatibilityType: string; +} + export interface FeatureFlagsPaginationInfo { nodes: FeatureFlag[]; total: number; diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts index 0ce3899ad6..e047248fd1 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts @@ -9,16 +9,7 @@ import { FeatureFlagsDataService } from '../../../../../core/feature-flags/featu import { CommonModalConfig } from '../../../../../shared-standalone-component-lib/components/common-modal/common-modal.types'; import { FeatureFlagsService } from '../../../../../core/feature-flags/feature-flags.service'; import { MatTableDataSource } from '@angular/material/table'; - -export interface FeatureFlagFile { - fileName: string; - fileContent: string | ArrayBuffer; -} - -export interface ValidateFeatureFlagError { - fileName: string; - compatibilityType: string; -} +import { ValidateFeatureFlagError, FeatureFlagFile } from '../../../../../core/feature-flags/store/feature-flags.model'; @Component({ selector: 'app-import-feature-flag-modal', From f96ee40718d5c7854ef1484f3666b492a15a53cf Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Wed, 7 Aug 2024 10:43:22 +0530 Subject: [PATCH 08/28] update compatibility warning code --- .../Upgrade/src/api/services/FeatureFlagService.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 5aeced8630..4b95d5bece 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -443,8 +443,12 @@ export class FeatureFlagService { compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; } else { const segmentIds = [ - ...flag.featureFlagSegmentInclusion.map((segmentInclusion) => segmentInclusion.segment.id), - ...flag.featureFlagSegmentExclusion.map((segmentExclusion) => segmentExclusion.segment.id), + ...flag.featureFlagSegmentInclusion.flatMap((segmentInclusion) => + segmentInclusion.segment.subSegments.map((subSegment) => subSegment.id) + ), + ...flag.featureFlagSegmentExclusion.flatMap((segmentExclusion) => + segmentExclusion.segment.subSegments.map((subSegment) => subSegment.id) + ), ]; const segments = await this.segmentService.getSegmentByIds(segmentIds); From 27a01742a49efd0a1377b3b044183a635584b7d3 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Wed, 7 Aug 2024 19:16:19 +0530 Subject: [PATCH 09/28] segment checks to return incompatible --- .../src/api/services/FeatureFlagService.ts | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 4b95d5bece..96d6953899 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -439,6 +439,41 @@ export class FeatureFlagService { private async validateImportFeatureFlag(fileName: string, flag: FeatureFlag) { let compatibilityType = FF_COMPATIBILITY_TYPE.COMPATIBLE; + // check for subSegmentIds + let segmentValidator = false; + flag.featureFlagSegmentInclusion.forEach((segmentInclusion) => { + if (!segmentInclusion.segment.subSegments) { + segmentValidator = true; + return; + } + segmentInclusion.segment.subSegments.forEach((subSegment) => { + if (subSegment.id == undefined) { + segmentValidator = true; + return; + } + }); + }); + + flag.featureFlagSegmentExclusion.forEach((segmentExclusion) => { + if (!segmentExclusion.segment.subSegments) { + segmentValidator = true; + return; + } + segmentExclusion.segment.subSegments.forEach((subSegment) => { + if (subSegment.id == undefined) { + segmentValidator = true; + return; + } + }); + }); + + if (segmentValidator) { + return { + fileName: fileName, + compatibilityType: FF_COMPATIBILITY_TYPE.INCOMPATIBLE, + }; + } + if (!flag.name || !flag.key || !flag.context) { compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; } else { From 5b6b920f7d3bf2865947d3f0052d36ea583f6ab2 Mon Sep 17 00:00:00 2001 From: Yagnik Date: Wed, 7 Aug 2024 21:42:35 +0530 Subject: [PATCH 10/28] resolved the review comment --- .../feature-flags.data.service.ts | 5 ++ .../app/core/segments/store/segments.model.ts | 7 +- .../import-feature-flag-modal.component.html | 4 +- .../import-feature-flag-modal.component.ts | 81 +++++++++++++------ .../import-segment.component.ts | 12 +-- .../src/environments/environment-types.ts | 1 + .../src/environments/environment.bsnl.ts | 1 + .../src/environments/environment.demo.prod.ts | 1 + .../src/environments/environment.prod.ts | 1 + .../src/environments/environment.staging.ts | 1 + .../upgrade/src/environments/environment.ts | 1 + 11 files changed, 78 insertions(+), 37 deletions(-) diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts index 248a21e8fa..ba5d1e9907 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts @@ -49,6 +49,11 @@ export class FeatureFlagsDataService { return this.http.post(url, featureFlag); } + importFeatureFlag(featureFlag: FeatureFlagFile[]) { + const url = this.environment.api.importFeatureFlag; + return this.http.post(url, featureFlag); + } + emailFeatureFlagData(flagId: string, email: string){ let featureFlagInfoParams = new HttpParams(); featureFlagInfoParams = featureFlagInfoParams.append('experimentId', flagId); 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 d13f7a4ced..43253eee82 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 @@ -122,12 +122,7 @@ export interface SegmentFile { fileContent: string | ArrayBuffer; } -export interface SegmentReturnedObj { - segments: Segment[]; - importErrors: SegmentImportError[]; -} - -export interface SegmentImportError { +export interface importError { fileName: string; error: string; } diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html index 5105d6abce..7f1581c123 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.html @@ -12,7 +12,7 @@

@@ -21,7 +21,7 @@

- +
diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts index e047248fd1..163437e841 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts @@ -1,7 +1,7 @@ import { ChangeDetectionStrategy, Component, Inject } from '@angular/core'; import { CommonModalComponent, CommonStatusIndicatorChipComponent } from '../../../../../shared-standalone-component-lib/components'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; -import { BehaviorSubject, Observable, combineLatest, map } from 'rxjs'; +import { BehaviorSubject, Observable, combineLatest, firstValueFrom, map } from 'rxjs'; import { CommonModule } from '@angular/common'; import { SharedModule } from '../../../../../shared/shared.module'; import { CommonImportContainerComponent } from '../../../../../shared-standalone-component-lib/components/common-import-container/common-import-container.component'; @@ -10,6 +10,8 @@ import { CommonModalConfig } from '../../../../../shared-standalone-component-li import { FeatureFlagsService } from '../../../../../core/feature-flags/feature-flags.service'; import { MatTableDataSource } from '@angular/material/table'; import { ValidateFeatureFlagError, FeatureFlagFile } from '../../../../../core/feature-flags/store/feature-flags.model'; +import { importError } from '../../../../../core/segments/store/segments.model'; +import { NotificationService } from '../../../../../core/notifications/notification.service'; @Component({ selector: 'app-import-feature-flag-modal', @@ -24,8 +26,8 @@ export class ImportFeatureFlagModalComponent { compatibilityDescription = ''; displayedColumns: string[] = ['actions', 'fileName', 'compatibilityType']; isImportActionBtnDisabled = new BehaviorSubject(true); - importFileErrorsDataSource = new MatTableDataSource(); - importFileErrors: ValidateFeatureFlagError[] = []; + fileValidationErrorDataSource = new MatTableDataSource(); + fileValidationErrors: ValidateFeatureFlagError[] = []; fileData: FeatureFlagFile[] = []; uploadedFileCount = new BehaviorSubject(0); isLoadingImportFeatureFlag$ = this.featureFlagsService.isLoadingImportFeatureFlag$; @@ -38,22 +40,23 @@ export class ImportFeatureFlagModalComponent { @Inject(MAT_DIALOG_DATA) public data: CommonModalConfig, public featureFlagsService: FeatureFlagsService, public featureFlagsDataService: FeatureFlagsDataService, - public dialogRef: MatDialogRef + public dialogRef: MatDialogRef, + private notificationService: NotificationService, ) {} async handleFilesSelected(event) { - if (event.target.files.length > 0) { + if (event.length > 0) { this.isImportActionBtnDisabled.next(false); this.featureFlagsService.setIsLoadingImportFeatureFlag(true); } - this.uploadedFileCount.next(event.target.files.length); - this.importFileErrors = []; + this.uploadedFileCount.next(event.length); + this.fileValidationErrors = []; this.fileData = []; - if (event.target.files.length === 0) return; + if (event.length === 0) return; - const filesArray = Array.from(event.target.files) as any[]; + const filesArray = Array.from(event) as any[]; await Promise.all( filesArray.map((file) => { return new Promise((resolve) => { @@ -72,18 +75,22 @@ export class ImportFeatureFlagModalComponent { } async checkValidation(files: FeatureFlagFile[]) { - this.importFileErrors = - ( - (await this.featureFlagsDataService.validateFeatureFlag(files).toPromise()) as ValidateFeatureFlagError[] - ).filter((data) => data.compatibilityType != null) || []; - this.importFileErrorsDataSource.data = this.importFileErrors; - this.featureFlagsService.setIsLoadingImportFeatureFlag(false); - if (this.importFileErrors.length > 0) { - this.importFileErrors.forEach(error => { - if (error.compatibilityType === 'incompatible') { - this.isImportActionBtnDisabled.next(true); - }; - }); + try { + const validationErrors = await firstValueFrom(this.featureFlagsDataService.validateFeatureFlag(files)) as ValidateFeatureFlagError[]; + this.fileValidationErrors = validationErrors.filter((data) => data.compatibilityType != null) || []; + this.fileValidationErrorDataSource.data = this.fileValidationErrors; + this.featureFlagsService.setIsLoadingImportFeatureFlag(false); + + if (this.fileValidationErrors.length > 0) { + this.fileValidationErrors.forEach(error => { + if (error.compatibilityType === 'incompatible') { + this.isImportActionBtnDisabled.next(true); + } + }); + } + } catch (error) { + console.error('Error during validation:', error); + this.featureFlagsService.setIsLoadingImportFeatureFlag(false); } } @@ -91,8 +98,36 @@ export class ImportFeatureFlagModalComponent { this.isDescriptionExpanded = !this.isDescriptionExpanded; } - importFiles() { - console.log('Import feature flags'); + async importFiles() { + try { + this.isImportActionBtnDisabled.next(true); + const importResult = await firstValueFrom(this.featureFlagsDataService.importFeatureFlag(this.fileData)) as importError[]; + + this.showNotification(importResult); + this.isImportActionBtnDisabled.next(false); + this.uploadedFileCount.next(0); + this.fileData = []; + } catch (error) { + console.error('Error during import:', error); + this.isImportActionBtnDisabled.next(false); + } + } + + showNotification(importResult: importError[]) { + const importSuccessFiles = importResult.filter((data) => data.error == null).map((data) => data.fileName); + + let importSuccessMsg = ''; + if ( importSuccessFiles.length > 0) { + importSuccessMsg =`Successfully imported ${importSuccessFiles.length} file/s: ${importSuccessFiles.join(', ')}` + this.closeModal(); + } + + this.notificationService.showSuccess(importSuccessMsg); + + const importFailedFiles = importResult.filter((data) => data.error != null); + importFailedFiles.forEach((data) => { + this.notificationService.showError(`Failed to import ${data.fileName}: ${data.error}`); + }); } closeModal() { diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts index 67e8b48ca5..c806e973dd 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts @@ -1,7 +1,7 @@ import { Component, Inject } from '@angular/core'; import { MatDialogRef, MAT_DIALOG_DATA } from '@angular/material/dialog'; import { SegmentsService } from '../../../../../../core/segments/segments.service'; -import { Segment, SegmentFile, SegmentImportError } from '../../../../../../core/segments/store/segments.model'; +import { Segment, SegmentFile, importError } from '../../../../../../core/segments/store/segments.model'; import { MatTableDataSource } from '@angular/material/table'; import { TranslateService } from '@ngx-translate/core'; import { SegmentsDataService } from '../../../../../../core/segments/segments.data.service'; @@ -19,8 +19,8 @@ export class ImportSegmentComponent { isLoadingSegments$ = false; fileData: SegmentFile[] = []; nonErrorSegments: number; - importFileErrorsDataSource = new MatTableDataSource(); - importFileErrors: SegmentImportError[] = []; + importFileErrorsDataSource = new MatTableDataSource(); + importFileErrors: importError[] = []; displayedColumns: string[] = ['File Name', 'Error']; constructor( @@ -40,14 +40,14 @@ export class ImportSegmentComponent { this.onCancelClick(); const importResult = (await this.segmentDataService .importSegments(this.fileData) - .toPromise()) as SegmentImportError[]; + .toPromise()) as importError[]; this.showNotification(importResult); this.segmentsService.fetchSegments(true); } - showNotification(importResult: SegmentImportError[]) { + showNotification(importResult: importError[]) { const importSuccessFiles = importResult.filter((data) => data.error == null).map((data) => data.fileName); const importSuccessMsg = @@ -90,7 +90,7 @@ export class ImportSegmentComponent { async validateFiles() { this.importFileErrors = - ((await this.segmentDataService.validateSegments(this.fileData).toPromise()) as SegmentImportError[]) || []; + ((await this.segmentDataService.validateSegments(this.fileData).toPromise()) as importError[]) || []; this.importFileErrorsDataSource.data = this.importFileErrors; this.nonErrorSegments = this.uploadedFileCount - this.importFileErrors.length; diff --git a/frontend/projects/upgrade/src/environments/environment-types.ts b/frontend/projects/upgrade/src/environments/environment-types.ts index b384e73256..89224a2739 100644 --- a/frontend/projects/upgrade/src/environments/environment-types.ts +++ b/frontend/projects/upgrade/src/environments/environment-types.ts @@ -30,6 +30,7 @@ export interface APIEndpoints { allExperimentNames: string; featureFlag: string; validateFeatureFlag: string; + importFeatureFlag: string; updateFlagStatus: string; getPaginatedFlags: string; exportFlagsDesign: string, diff --git a/frontend/projects/upgrade/src/environments/environment.bsnl.ts b/frontend/projects/upgrade/src/environments/environment.bsnl.ts index 05cf70a2fc..c0d870d19c 100644 --- a/frontend/projects/upgrade/src/environments/environment.bsnl.ts +++ b/frontend/projects/upgrade/src/environments/environment.bsnl.ts @@ -45,6 +45,7 @@ export const environment = { updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', validateFeatureFlag: '/flags/import/validation', + importFeatureFlag: '/flags/import', exportFlagsDesign: '/flags/export', emailFlagData: '/flags/mail', addFlagInclusionList: '/flags/inclusionList', diff --git a/frontend/projects/upgrade/src/environments/environment.demo.prod.ts b/frontend/projects/upgrade/src/environments/environment.demo.prod.ts index b0c19d8ede..5d271c7d42 100755 --- a/frontend/projects/upgrade/src/environments/environment.demo.prod.ts +++ b/frontend/projects/upgrade/src/environments/environment.demo.prod.ts @@ -45,6 +45,7 @@ export const environment = { updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', validateFeatureFlag: '/flags/import/validation', + importFeatureFlag: '/flags/import', exportFlagsDesign: '/flags/export', emailFlagData: '/flags/mail', addFlagInclusionList: '/flags/inclusionList', diff --git a/frontend/projects/upgrade/src/environments/environment.prod.ts b/frontend/projects/upgrade/src/environments/environment.prod.ts index bda5ee1a65..685035e337 100755 --- a/frontend/projects/upgrade/src/environments/environment.prod.ts +++ b/frontend/projects/upgrade/src/environments/environment.prod.ts @@ -45,6 +45,7 @@ export const environment = { updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', validateFeatureFlag: '/flags/import/validation', + importFeatureFlag: '/flags/import', exportFlagsDesign: '/flags/export', emailFlagData: '/flags/mail', addFlagInclusionList: '/flags/inclusionList', diff --git a/frontend/projects/upgrade/src/environments/environment.staging.ts b/frontend/projects/upgrade/src/environments/environment.staging.ts index 9a5c5e0137..dbe6d85fd8 100644 --- a/frontend/projects/upgrade/src/environments/environment.staging.ts +++ b/frontend/projects/upgrade/src/environments/environment.staging.ts @@ -45,6 +45,7 @@ export const environment = { updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', validateFeatureFlag: '/flags/import/validation', + importFeatureFlag: '/flags/import', exportFlagsDesign: '/flags/export', emailFlagData: '/flags/mail', addFlagInclusionList: '/flags/inclusionList', diff --git a/frontend/projects/upgrade/src/environments/environment.ts b/frontend/projects/upgrade/src/environments/environment.ts index 427e9e3f3d..006142f6b0 100755 --- a/frontend/projects/upgrade/src/environments/environment.ts +++ b/frontend/projects/upgrade/src/environments/environment.ts @@ -50,6 +50,7 @@ export const environment = { updateFlagStatus: '/flags/status', getPaginatedFlags: '/flags/paginated', validateFeatureFlag: '/flags/import/validation', + importFeatureFlag: '/flags/import', exportFlagsDesign: '/flags/export', emailFlagData: '/flags/mail', addFlagInclusionList: '/flags/inclusionList', From fa168dd6ee205ccf247a9ef1382b6032270332d9 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Mon, 12 Aug 2024 11:33:32 +0530 Subject: [PATCH 11/28] adding segment list for import --- .../api/controllers/FeatureFlagController.ts | 30 +++--- .../src/api/services/FeatureFlagService.ts | 97 ++++++++++++++----- 2 files changed, 92 insertions(+), 35 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index 0c69cad04d..aae9ad972a 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -659,14 +659,14 @@ export class FeatureFlagsController { /** * @swagger - * /experiments/{validation}: + * /flags/import/validation: * post: - * description: Validating Experiment + * description: Validating Feature Flag * consumes: * - application/json * parameters: * - in: body - * name: experiments + * name: featureFlags * required: true * schema: * type: array @@ -677,9 +677,9 @@ export class FeatureFlagsController { * type: string * fileContent: * type: string - * description: Experiment Files + * description: Import FeatureFlag Files * tags: - * - Experiments + * - Feature Flags * produces: * - application/json * responses: @@ -692,17 +692,25 @@ export class FeatureFlagsController { * properties: * fileName: * type: string - * error: + * compatibilityType: * type: string + * enum: [compatible, warning, incompatible] * '401': * description: AuthorizationRequiredError * '500': * description: Internal Server Error */ + @Post('/import/validation') + public async validateImportFeatureFlags( + @Body({ validate: true }) featureFlags: FeatureFlagFile[], + @Req() request: AppRequest + ): Promise { + return await this.featureFlagService.validateImportFeatureFlags(featureFlags, request.logger); + } /** * @swagger - * /flags/{validation}: + * /flags/import: * post: * description: Validating Feature Flag * consumes: @@ -743,11 +751,11 @@ export class FeatureFlagsController { * '500': * description: Internal Server Error */ - @Post('/import/validation') - public async validateImportFeatureFlags( + @Post('/import') + public async importFeatureFlags( @Body({ validate: true }) featureFlags: FeatureFlagFile[], @Req() request: AppRequest - ): Promise { - return await this.featureFlagService.validateImportFeatureFlags(featureFlags, request.logger); + ): Promise { + return await this.featureFlagService.importFeatureFlags(featureFlags, request.logger); } } diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 389d5854e6..fe4abec486 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -258,7 +258,6 @@ export class FeatureFlagService { throw error; } featureFlagSegmentInclusionOrExclusion.segment = newSegment; - // } try { if (filterType === 'inclusion') { @@ -425,6 +424,56 @@ export class FeatureFlagService { return includedFeatureFlags; } + public async importFeatureFlags(featureFlagFiles: FeatureFlagFile[], logger: UpgradeLogger): Promise { + logger.info({ message: 'Import feature flags' }); + const validatedFlags = await this.validateImportFeatureFlags(featureFlagFiles, logger); + + const validFiles: FeatureFlag[] = featureFlagFiles + .filter((file) => { + const validation = validatedFlags.find((error) => error.fileName === file.fileName); + return validation && validation.compatibilityType !== FF_COMPATIBILITY_TYPE.INCOMPATIBLE; + }) + .map((featureFlagFile) => { + return JSON.parse(featureFlagFile.fileContent); + }); + + const createdFlags = await Promise.all( + validFiles.map(async (featureFlag) => { + const { featureFlagSegmentInclusion, featureFlagSegmentExclusion, ...rest } = featureFlag; + const newFlag = await this.addFeatureFlagInDB(rest as any, logger); + + const inclusionDoc = await Promise.all( + featureFlagSegmentInclusion.map(async (segmentInclusion) => { + segmentInclusion.segment.id = uuid(); + const listInput = { + flagId: newFlag.id, + enabled: false, + listType: segmentInclusion.listType, + list: this.segmentService.convertJSONStringToSegInputValFormat(JSON.stringify(segmentInclusion.segment)), + }; + return await this.addList(listInput, 'inclusion', logger); + }) + ); + + const exclusionDoc = await Promise.all( + featureFlagSegmentExclusion.map(async (segmentExclusion) => { + segmentExclusion.segment.id = uuid(); + const listInput = { + flagId: newFlag.id, + enabled: false, + listType: segmentExclusion.listType, + list: this.segmentService.convertJSONStringToSegInputValFormat(JSON.stringify(segmentExclusion.segment)), + }; + return await this.addList(listInput, 'exclusion', logger); + }) + ); + + return { ...newFlag, featureFlagSegmentInclusion: inclusionDoc, featureFlagSegmentExclusion: exclusionDoc }; + }) + ); + + return createdFlags; + } public async validateImportFeatureFlags( featureFlagFiles: FeatureFlagFile[], logger: UpgradeLogger @@ -491,31 +540,31 @@ export class FeatureFlagService { }); }); - if (segmentValidator) { - return { - fileName: fileName, - compatibilityType: FF_COMPATIBILITY_TYPE.INCOMPATIBLE, - }; - } - - if (!flag.name || !flag.key || !flag.context) { + // check for missing fields + if (!flag.name || !flag.key || !flag.context || segmentValidator) { compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; } else { - const segmentIds = [ - ...flag.featureFlagSegmentInclusion.flatMap((segmentInclusion) => - segmentInclusion.segment.subSegments.map((subSegment) => subSegment.id) - ), - ...flag.featureFlagSegmentExclusion.flatMap((segmentExclusion) => - segmentExclusion.segment.subSegments.map((subSegment) => subSegment.id) - ), - ]; - - const segments = await this.segmentService.getSegmentByIds(segmentIds); - segments.forEach((segment) => { - if (segment == undefined) { - compatibilityType = FF_COMPATIBILITY_TYPE.WARNING; - } - }); + const keyExists = await this.featureFlagRepository.findOne({ key: flag.key }); + + if (keyExists) { + compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; + } else { + const segmentIds = [ + ...flag.featureFlagSegmentInclusion.flatMap((segmentInclusion) => + segmentInclusion.segment.subSegments.map((subSegment) => subSegment.id) + ), + ...flag.featureFlagSegmentExclusion.flatMap((segmentExclusion) => + segmentExclusion.segment.subSegments.map((subSegment) => subSegment.id) + ), + ]; + + const segments = await this.segmentService.getSegmentByIds(segmentIds); + segments.forEach((segment) => { + if (segment == undefined) { + compatibilityType = FF_COMPATIBILITY_TYPE.WARNING; + } + }); + } } return { From 0b15ae291e98f4800fdab8259e7af25ee93f106d Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Mon, 12 Aug 2024 12:45:35 +0530 Subject: [PATCH 12/28] Add IFeatureFlagFile interface for feature flag file handling --- .../api/controllers/FeatureFlagController.ts | 50 ++----------------- .../FeatureFlagsPaginatedParamsValidator.ts | 5 -- .../src/api/services/FeatureFlagService.ts | 7 ++- .../feature-flags.data.service.ts | 6 +-- .../store/feature-flags.model.ts | 5 -- .../import-feature-flag-modal.component.ts | 36 +++++++++---- types/src/Experiment/interfaces.ts | 5 ++ types/src/index.ts | 1 + 8 files changed, 40 insertions(+), 75 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index 0c69cad04d..76c14f4f14 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -5,13 +5,12 @@ import { FeatureFlagSegmentExclusion } from '../models/FeatureFlagSegmentExclusi import { FeatureFlagSegmentInclusion } from '../models/FeatureFlagSegmentInclusion'; import { FeatureFlagStatusUpdateValidator } from './validators/FeatureFlagStatusUpdateValidator'; import { - FeatureFlagFile, FeatureFlagPaginatedParamsValidator, ValidatedFeatureFlagsError, } from './validators/FeatureFlagsPaginatedParamsValidator'; import { FeatureFlagFilterModeUpdateValidator } from './validators/FeatureFlagFilterModeUpdateValidator'; import { AppRequest, PaginationResponse } from '../../types'; -import { SERVER_ERROR } from 'upgrade_types'; +import { SERVER_ERROR, IFeatureFlagFile } from 'upgrade_types'; import { FeatureFlagValidation, IdValidator, UserParamsValidator } from './validators/FeatureFlagValidator'; import { ExperimentUserService } from '../services/ExperimentUserService'; import { FeatureFlagListValidator } from '../controllers/validators/FeatureFlagListValidator'; @@ -659,50 +658,7 @@ export class FeatureFlagsController { /** * @swagger - * /experiments/{validation}: - * post: - * description: Validating Experiment - * consumes: - * - application/json - * parameters: - * - in: body - * name: experiments - * required: true - * schema: - * type: array - * items: - * type: object - * properties: - * fileName: - * type: string - * fileContent: - * type: string - * description: Experiment Files - * tags: - * - Experiments - * produces: - * - application/json - * responses: - * '200': - * description: Validations are completed - * schema: - * type: array - * items: - * type: object - * properties: - * fileName: - * type: string - * error: - * type: string - * '401': - * description: AuthorizationRequiredError - * '500': - * description: Internal Server Error - */ - - /** - * @swagger - * /flags/{validation}: + * /flags/import/validation: * post: * description: Validating Feature Flag * consumes: @@ -745,7 +701,7 @@ export class FeatureFlagsController { */ @Post('/import/validation') public async validateImportFeatureFlags( - @Body({ validate: true }) featureFlags: FeatureFlagFile[], + @Body({ validate: true }) featureFlags: IFeatureFlagFile[], @Req() request: AppRequest ): Promise { return await this.featureFlagService.validateImportFeatureFlags(featureFlags, request.logger); diff --git a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagsPaginatedParamsValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagsPaginatedParamsValidator.ts index 1aed56fd47..a40aa5facf 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagsPaginatedParamsValidator.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagsPaginatedParamsValidator.ts @@ -12,11 +12,6 @@ export interface IFeatureFlagSortParams { sortAs: SORT_AS_DIRECTION; } -export interface FeatureFlagFile { - fileName: string; - fileContent: string; -} - export interface ValidatedFeatureFlagsError { fileName: string; compatibilityType: FF_COMPATIBILITY_TYPE; diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 389d5854e6..e78863b999 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -15,10 +15,9 @@ import { FLAG_SEARCH_KEY, ValidatedFeatureFlagsError, FF_COMPATIBILITY_TYPE, - FeatureFlagFile, } from '../controllers/validators/FeatureFlagsPaginatedParamsValidator'; import { FeatureFlagListValidator } from '../controllers/validators/FeatureFlagListValidator'; -import { SERVER_ERROR, FEATURE_FLAG_STATUS, FILTER_MODE, SEGMENT_TYPE } from 'upgrade_types'; +import { SERVER_ERROR, FEATURE_FLAG_STATUS, FILTER_MODE, SEGMENT_TYPE, IFeatureFlagFile } from 'upgrade_types'; import { UpgradeLogger } from '../../lib/logger/UpgradeLogger'; import { FeatureFlagValidation } from '../controllers/validators/FeatureFlagValidator'; import { ExperimentUser } from '../models/ExperimentUser'; @@ -426,7 +425,7 @@ export class FeatureFlagService { } public async validateImportFeatureFlags( - featureFlagFiles: FeatureFlagFile[], + featureFlagFiles: IFeatureFlagFile[], logger: UpgradeLogger ): Promise { logger.info({ message: 'Validate feature flags' }); @@ -434,7 +433,7 @@ export class FeatureFlagService { featureFlagFiles.map(async (featureFlagFile) => { let featureFlag: FeatureFlag; try { - featureFlag = JSON.parse(featureFlagFile.fileContent); + featureFlag = JSON.parse(featureFlagFile.fileContent as string); } catch (parseError) { logger.error({ message: 'Error in parsing feature flag file', details: parseError }); return { diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts index a764e2ac14..463fc997bc 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts @@ -4,7 +4,6 @@ import { HttpClient, HttpParams } from '@angular/common/http'; import { AddFeatureFlagRequest, FeatureFlag, - FeatureFlagFile, FeatureFlagSegmentListDetails, FeatureFlagsPaginationInfo, FeatureFlagsPaginationParams, @@ -14,6 +13,7 @@ import { } from './store/feature-flags.model'; import { Observable, delay, of } from 'rxjs'; import { AddPrivateSegmentListRequest, EditPrivateSegmentListRequest } from '../segments/store/segments.model'; +import { IFeatureFlagFile } from 'upgrade_types'; @Injectable() export class FeatureFlagsDataService { @@ -45,12 +45,12 @@ export class FeatureFlagsDataService { return this.http.put(url, flag); } - validateFeatureFlag(featureFlag: FeatureFlagFile[]) { + validateFeatureFlag(featureFlag: IFeatureFlagFile[]) { const url = this.environment.api.validateFeatureFlag; return this.http.post(url, featureFlag); } - importFeatureFlag(featureFlag: FeatureFlagFile[]) { + importFeatureFlag(featureFlag: IFeatureFlagFile[]) { const url = this.environment.api.importFeatureFlag; return this.http.post(url, featureFlag); } 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 0101a84cb0..13f45f77bd 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 @@ -58,11 +58,6 @@ export enum UPSERT_FEATURE_FLAG_LIST_ACTION { EDIT = 'edit', } -export interface FeatureFlagFile { - fileName: string; - fileContent: string | ArrayBuffer; -} - export interface ValidateFeatureFlagError { fileName: string; compatibilityType: string; diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts index 163437e841..a1cb87ae51 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts @@ -1,5 +1,8 @@ import { ChangeDetectionStrategy, Component, Inject } from '@angular/core'; -import { CommonModalComponent, CommonStatusIndicatorChipComponent } from '../../../../../shared-standalone-component-lib/components'; +import { + CommonModalComponent, + CommonStatusIndicatorChipComponent, +} from '../../../../../shared-standalone-component-lib/components'; import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { BehaviorSubject, Observable, combineLatest, firstValueFrom, map } from 'rxjs'; import { CommonModule } from '@angular/common'; @@ -9,14 +12,21 @@ import { FeatureFlagsDataService } from '../../../../../core/feature-flags/featu import { CommonModalConfig } from '../../../../../shared-standalone-component-lib/components/common-modal/common-modal.types'; import { FeatureFlagsService } from '../../../../../core/feature-flags/feature-flags.service'; import { MatTableDataSource } from '@angular/material/table'; -import { ValidateFeatureFlagError, FeatureFlagFile } from '../../../../../core/feature-flags/store/feature-flags.model'; +import { ValidateFeatureFlagError } from '../../../../../core/feature-flags/store/feature-flags.model'; import { importError } from '../../../../../core/segments/store/segments.model'; import { NotificationService } from '../../../../../core/notifications/notification.service'; +import { IFeatureFlagFile } from 'upgrade_types'; @Component({ selector: 'app-import-feature-flag-modal', standalone: true, - imports: [CommonModalComponent, CommonModule, SharedModule, CommonImportContainerComponent, CommonStatusIndicatorChipComponent], + imports: [ + CommonModalComponent, + CommonModule, + SharedModule, + CommonImportContainerComponent, + CommonStatusIndicatorChipComponent, + ], templateUrl: './import-feature-flag-modal.component.html', styleUrls: ['./import-feature-flag-modal.component.scss'], changeDetection: ChangeDetectionStrategy.OnPush, @@ -28,7 +38,7 @@ export class ImportFeatureFlagModalComponent { isImportActionBtnDisabled = new BehaviorSubject(true); fileValidationErrorDataSource = new MatTableDataSource(); fileValidationErrors: ValidateFeatureFlagError[] = []; - fileData: FeatureFlagFile[] = []; + fileData: IFeatureFlagFile[] = []; uploadedFileCount = new BehaviorSubject(0); isLoadingImportFeatureFlag$ = this.featureFlagsService.isLoadingImportFeatureFlag$; isImportActionBtnDisabled$: Observable = combineLatest([ @@ -41,7 +51,7 @@ export class ImportFeatureFlagModalComponent { public featureFlagsService: FeatureFlagsService, public featureFlagsDataService: FeatureFlagsDataService, public dialogRef: MatDialogRef, - private notificationService: NotificationService, + private notificationService: NotificationService ) {} async handleFilesSelected(event) { @@ -74,15 +84,17 @@ export class ImportFeatureFlagModalComponent { await this.checkValidation(this.fileData); } - async checkValidation(files: FeatureFlagFile[]) { + async checkValidation(files: IFeatureFlagFile[]) { try { - const validationErrors = await firstValueFrom(this.featureFlagsDataService.validateFeatureFlag(files)) as ValidateFeatureFlagError[]; + const validationErrors = (await firstValueFrom( + this.featureFlagsDataService.validateFeatureFlag(files) + )) as ValidateFeatureFlagError[]; this.fileValidationErrors = validationErrors.filter((data) => data.compatibilityType != null) || []; this.fileValidationErrorDataSource.data = this.fileValidationErrors; this.featureFlagsService.setIsLoadingImportFeatureFlag(false); if (this.fileValidationErrors.length > 0) { - this.fileValidationErrors.forEach(error => { + this.fileValidationErrors.forEach((error) => { if (error.compatibilityType === 'incompatible') { this.isImportActionBtnDisabled.next(true); } @@ -101,7 +113,9 @@ export class ImportFeatureFlagModalComponent { async importFiles() { try { this.isImportActionBtnDisabled.next(true); - const importResult = await firstValueFrom(this.featureFlagsDataService.importFeatureFlag(this.fileData)) as importError[]; + const importResult = (await firstValueFrom( + this.featureFlagsDataService.importFeatureFlag(this.fileData) + )) as importError[]; this.showNotification(importResult); this.isImportActionBtnDisabled.next(false); @@ -117,8 +131,8 @@ export class ImportFeatureFlagModalComponent { const importSuccessFiles = importResult.filter((data) => data.error == null).map((data) => data.fileName); let importSuccessMsg = ''; - if ( importSuccessFiles.length > 0) { - importSuccessMsg =`Successfully imported ${importSuccessFiles.length} file/s: ${importSuccessFiles.join(', ')}` + if (importSuccessFiles.length > 0) { + importSuccessMsg = `Successfully imported ${importSuccessFiles.length} file/s: ${importSuccessFiles.join(', ')}`; this.closeModal(); } diff --git a/types/src/Experiment/interfaces.ts b/types/src/Experiment/interfaces.ts index 8fb8c4fa1e..9345bf5f0a 100644 --- a/types/src/Experiment/interfaces.ts +++ b/types/src/Experiment/interfaces.ts @@ -245,3 +245,8 @@ export interface IMenuButtonItem { name: string; disabled: boolean; } + +export interface IFeatureFlagFile { + fileName: string; + fileContent: string | ArrayBuffer; +} diff --git a/types/src/index.ts b/types/src/index.ts index 2d92cf032e..e5df9187fc 100644 --- a/types/src/index.ts +++ b/types/src/index.ts @@ -66,4 +66,5 @@ export { ILogMetrics, ILogGroupMetrics, IMenuButtonItem, + IFeatureFlagFile, } from './Experiment/interfaces'; From c5e397544e9b8957d93c0fa91cd3ed995e0f3d2d Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Tue, 13 Aug 2024 11:05:49 +0530 Subject: [PATCH 13/28] Update import return format for feature flag --- .../api/controllers/FeatureFlagController.ts | 4 +- .../src/api/services/FeatureFlagService.ts | 37 ++++++++++++++----- types/src/Experiment/interfaces.ts | 5 +++ types/src/index.ts | 1 + 4 files changed, 36 insertions(+), 11 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index f5fabb202d..d67bb53522 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -10,7 +10,7 @@ import { } from './validators/FeatureFlagsPaginatedParamsValidator'; import { FeatureFlagFilterModeUpdateValidator } from './validators/FeatureFlagFilterModeUpdateValidator'; import { AppRequest, PaginationResponse } from '../../types'; -import { SERVER_ERROR, IFeatureFlagFile } from 'upgrade_types'; +import { SERVER_ERROR, IFeatureFlagFile, IImportError } from 'upgrade_types'; import { FeatureFlagValidation, IdValidator, UserParamsValidator } from './validators/FeatureFlagValidator'; import { ExperimentUserService } from '../services/ExperimentUserService'; import { FeatureFlagListValidator } from '../controllers/validators/FeatureFlagListValidator'; @@ -754,7 +754,7 @@ export class FeatureFlagsController { public async importFeatureFlags( @Body({ validate: true }) featureFlags: IFeatureFlagFile[], @Req() request: AppRequest - ): Promise { + ): Promise { return await this.featureFlagService.importFeatureFlags(featureFlags, request.logger); } } diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 48486cfde4..48d3299863 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -17,7 +17,14 @@ import { FF_COMPATIBILITY_TYPE, } from '../controllers/validators/FeatureFlagsPaginatedParamsValidator'; import { FeatureFlagListValidator } from '../controllers/validators/FeatureFlagListValidator'; -import { SERVER_ERROR, FEATURE_FLAG_STATUS, FILTER_MODE, SEGMENT_TYPE, IFeatureFlagFile } from 'upgrade_types'; +import { + SERVER_ERROR, + FEATURE_FLAG_STATUS, + FILTER_MODE, + SEGMENT_TYPE, + IFeatureFlagFile, + IImportError, +} from 'upgrade_types'; import { UpgradeLogger } from '../../lib/logger/UpgradeLogger'; import { FeatureFlagValidation } from '../controllers/validators/FeatureFlagValidator'; import { ExperimentUser } from '../models/ExperimentUser'; @@ -423,16 +430,27 @@ export class FeatureFlagService { return includedFeatureFlags; } - public async importFeatureFlags(featureFlagFiles: IFeatureFlagFile[], logger: UpgradeLogger): Promise { + public async importFeatureFlags( + featureFlagFiles: IFeatureFlagFile[], + logger: UpgradeLogger + ): Promise { logger.info({ message: 'Import feature flags' }); const validatedFlags = await this.validateImportFeatureFlags(featureFlagFiles, logger); - const validFiles: FeatureFlag[] = featureFlagFiles - .filter((file) => { - const validation = validatedFlags.find((error) => error.fileName === file.fileName); - return validation && validation.compatibilityType !== FF_COMPATIBILITY_TYPE.INCOMPATIBLE; - }) - .map((featureFlagFile) => { + const fileStatusArray = featureFlagFiles.map((file) => { + const validation = validatedFlags.find((error) => error.fileName === file.fileName); + const isCompatible = validation && validation.compatibilityType !== FF_COMPATIBILITY_TYPE.INCOMPATIBLE; + + return { + fileName: file.fileName, + error: isCompatible ? null : FF_COMPATIBILITY_TYPE.INCOMPATIBLE, + }; + }); + + const validFiles: FeatureFlag[] = fileStatusArray + .filter((fileStatus) => fileStatus.error === null) + .map((fileStatus) => { + const featureFlagFile = featureFlagFiles.find((file) => file.fileName === fileStatus.fileName); return JSON.parse(featureFlagFile.fileContent as string); }); @@ -470,8 +488,9 @@ export class FeatureFlagService { return { ...newFlag, featureFlagSegmentInclusion: inclusionDoc, featureFlagSegmentExclusion: exclusionDoc }; }) ); + logger.info({ message: 'Imported feature flags', details: createdFlags }); - return createdFlags; + return fileStatusArray; } public async validateImportFeatureFlags( featureFlagFiles: IFeatureFlagFile[], diff --git a/types/src/Experiment/interfaces.ts b/types/src/Experiment/interfaces.ts index 9345bf5f0a..2d5faee2b3 100644 --- a/types/src/Experiment/interfaces.ts +++ b/types/src/Experiment/interfaces.ts @@ -250,3 +250,8 @@ export interface IFeatureFlagFile { fileName: string; fileContent: string | ArrayBuffer; } + +export interface IImportError { + fileName: string; + error: string | null; +} diff --git a/types/src/index.ts b/types/src/index.ts index e5df9187fc..aa5cb8b85c 100644 --- a/types/src/index.ts +++ b/types/src/index.ts @@ -67,4 +67,5 @@ export { ILogGroupMetrics, IMenuButtonItem, IFeatureFlagFile, + IImportError, } from './Experiment/interfaces'; From 711138363c4c262fd29f56b4c39ed0c34a18fc36 Mon Sep 17 00:00:00 2001 From: Yagnik Date: Tue, 13 Aug 2024 16:50:28 +0530 Subject: [PATCH 14/28] fetch feature flags after successful import --- .../upgrade/src/app/core/feature-flags/feature-flags.service.ts | 1 - .../import-feature-flag-modal.component.ts | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts index 88127799e8..47e5530144 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.service.ts @@ -60,7 +60,6 @@ export class FeatureFlagsService { searchKey$ = this.store$.pipe(select(selectSearchKey)); sortKey$ = this.store$.pipe(select(selectSortKey)); sortAs$ = this.store$.pipe(select(selectSortAs)); - hasFeatureFlagsCountChanged$ = this.allFeatureFlags$.pipe( pairwise(), diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts index a1cb87ae51..e17fc9430a 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts @@ -134,6 +134,7 @@ export class ImportFeatureFlagModalComponent { if (importSuccessFiles.length > 0) { importSuccessMsg = `Successfully imported ${importSuccessFiles.length} file/s: ${importSuccessFiles.join(', ')}`; this.closeModal(); + this.featureFlagsService.fetchFeatureFlags(true); } this.notificationService.showSuccess(importSuccessMsg); From 92377f0abf1daff42e0fe1a4725b143c209da0eb Mon Sep 17 00:00:00 2001 From: Yagnik Date: Tue, 13 Aug 2024 16:51:11 +0530 Subject: [PATCH 15/28] removed topromise from import segment comp --- .../import-segment/import-segment.component.ts | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts index c806e973dd..13992e755e 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/components/modal/import-segment/import-segment.component.ts @@ -6,6 +6,7 @@ import { MatTableDataSource } from '@angular/material/table'; import { TranslateService } from '@ngx-translate/core'; import { SegmentsDataService } from '../../../../../../core/segments/segments.data.service'; import { NotificationService } from '../../../../../../core/notifications/notification.service'; +import { firstValueFrom } from 'rxjs'; @Component({ selector: 'app-import-segment', @@ -37,14 +38,14 @@ export class ImportSegmentComponent { } async importSegments() { - this.onCancelClick(); - const importResult = (await this.segmentDataService - .importSegments(this.fileData) - .toPromise()) as importError[]; - - this.showNotification(importResult); - - this.segmentsService.fetchSegments(true); + try { + this.onCancelClick(); + const importResult = await firstValueFrom(this.segmentDataService.importSegments(this.fileData)) as importError[]; + this.showNotification(importResult); + this.segmentsService.fetchSegments(true); + } catch (error) { + console.error('Error during segment import:', error); + } } showNotification(importResult: importError[]) { From 674f8159f733fcb5cc9f8f0eb2f2c25c410dbe71 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Tue, 20 Aug 2024 12:24:36 +0530 Subject: [PATCH 16/28] added validator to validate import json --- .../Upgrade/src/api/DTO/ExperimentDTO.ts | 9 +-- .../validators/FeatureFlagValidator.ts | 16 ++--- .../src/api/services/FeatureFlagService.ts | 62 +++++-------------- 3 files changed, 27 insertions(+), 60 deletions(-) diff --git a/backend/packages/Upgrade/src/api/DTO/ExperimentDTO.ts b/backend/packages/Upgrade/src/api/DTO/ExperimentDTO.ts index b4158ca568..5d89189088 100644 --- a/backend/packages/Upgrade/src/api/DTO/ExperimentDTO.ts +++ b/backend/packages/Upgrade/src/api/DTO/ExperimentDTO.ts @@ -43,6 +43,7 @@ export { EXPERIMENT_SORT_KEY, IExperimentSearchParams, IExperimentSortParams, + SegmentValidator, }; class PayloadValidator { @@ -296,14 +297,6 @@ export class ParticipantsValidator { public segment: SegmentValidator; } -export class ParticipantsArrayValidator { - @IsNotEmpty() - @IsArray() - @ValidateNested({ each: true }) - @Type(() => SegmentValidator) - public segments: SegmentValidator[]; -} - class StateTimeLogValidator { @IsNotEmpty() @IsString() diff --git a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts index 7dfa2c4095..7a700c8fa4 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts @@ -1,8 +1,8 @@ import { IsNotEmpty, IsDefined, IsString, IsArray, IsEnum, IsOptional, ValidateNested, IsUUID } from 'class-validator'; -import { ParticipantsArrayValidator } from '../../DTO/ExperimentDTO'; import { FILTER_MODE } from 'upgrade_types'; import { FEATURE_FLAG_STATUS } from 'upgrade_types'; import { Type } from 'class-transformer'; +import { FeatureFlagListValidator } from './FeatureFlagListValidator'; export class FeatureFlagValidation { @IsOptional() @@ -43,14 +43,16 @@ export class FeatureFlagValidation { public tags: string[]; @IsOptional() - @ValidateNested() - @Type(() => ParticipantsArrayValidator) - public featureFlagSegmentInclusion?: ParticipantsArrayValidator; + @IsArray() + @ValidateNested({ each: true }) + @Type(() => FeatureFlagListValidator) + public featureFlagSegmentInclusion?: FeatureFlagListValidator[]; @IsOptional() - @ValidateNested() - @Type(() => ParticipantsArrayValidator) - public featureFlagSegmentExclusion?: ParticipantsArrayValidator; + @IsArray() + @ValidateNested({ each: true }) + @Type(() => FeatureFlagListValidator) + public featureFlagSegmentExclusion?: FeatureFlagListValidator[]; } export class UserParamsValidator { diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index df6a82e730..a46936c56b 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -25,6 +25,8 @@ import { ExperimentAssignmentService } from './ExperimentAssignmentService'; import { SegmentService } from './SegmentService'; import { ErrorWithType } from '../errors/ErrorWithType'; import { RequestedExperimentUser } from '../controllers/validators/ExperimentUserValidator'; +import { validate } from 'class-validator'; +import { plainToClass } from 'class-transformer'; @Service() export class FeatureFlagService { @@ -140,7 +142,7 @@ export class FeatureFlagService { await transactionalEntityManager.getRepository(Segment).delete(segmentInclusion.segment.id); } catch (err) { const error = err as ErrorWithType; - error.details = 'Error in deleting Feature Flag Included Segment fron DB'; + error.details = 'Error in deleting Feature Flag Included Segment from DB'; error.type = SERVER_ERROR.QUERY_FAILED; logger.error(error); throw error; @@ -151,7 +153,7 @@ export class FeatureFlagService { await transactionalEntityManager.getRepository(Segment).delete(segmentExclusion.segment.id); } catch (err) { const error = err as ErrorWithType; - error.details = 'Error in deleting Feature Flag Excluded Segment fron DB'; + error.details = 'Error in deleting Feature Flag Excluded Segment from DB'; error.type = SERVER_ERROR.QUERY_FAILED; logger.error(error); throw error; @@ -450,7 +452,7 @@ export class FeatureFlagService { logger.info({ message: 'Validate feature flags' }); const validationErrors = await Promise.allSettled( featureFlagFiles.map(async (featureFlagFile) => { - let featureFlag: FeatureFlag; + let featureFlag: FeatureFlagValidation; try { featureFlag = JSON.parse(featureFlagFile.fileContent as string); } catch (parseError) { @@ -478,54 +480,24 @@ export class FeatureFlagService { .filter((error) => error !== null); } - private async validateImportFeatureFlag(fileName: string, flag: FeatureFlag) { + private async validateImportFeatureFlag(fileName: string, flag: FeatureFlagValidation) { let compatibilityType = FF_COMPATIBILITY_TYPE.COMPATIBLE; - // check for subSegmentIds - let segmentValidator = false; - flag.featureFlagSegmentInclusion.forEach((segmentInclusion) => { - if (!segmentInclusion.segment.subSegments) { - segmentValidator = true; - return; + flag = plainToClass(FeatureFlagValidation, flag); + await validate(flag).then((errors) => { + if (errors.length > 0) { + compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; } - segmentInclusion.segment.subSegments.forEach((subSegment) => { - if (subSegment.id == undefined) { - segmentValidator = true; - return; - } - }); - }); - - flag.featureFlagSegmentExclusion.forEach((segmentExclusion) => { - if (!segmentExclusion.segment.subSegments) { - segmentValidator = true; - return; - } - segmentExclusion.segment.subSegments.forEach((subSegment) => { - if (subSegment.id == undefined) { - segmentValidator = true; - return; - } - }); }); - if (segmentValidator) { - return { - fileName: fileName, - compatibilityType: FF_COMPATIBILITY_TYPE.INCOMPATIBLE, - }; - } - - if (!flag.name || !flag.key || !flag.context) { - compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; - } else { + if (compatibilityType === FF_COMPATIBILITY_TYPE.COMPATIBLE) { const segmentIds = [ - ...flag.featureFlagSegmentInclusion.flatMap((segmentInclusion) => - segmentInclusion.segment.subSegments.map((subSegment) => subSegment.id) - ), - ...flag.featureFlagSegmentExclusion.flatMap((segmentExclusion) => - segmentExclusion.segment.subSegments.map((subSegment) => subSegment.id) - ), + ...flag.featureFlagSegmentInclusion.flatMap((segmentInclusion) => { + return segmentInclusion.list.subSegmentIds; + }), + ...flag.featureFlagSegmentExclusion.flatMap((segmentExclusion) => { + return segmentExclusion.list.subSegmentIds; + }), ]; const segments = await this.segmentService.getSegmentByIds(segmentIds); From 3828069b160f678f70099f855233a1b20b84b250 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Tue, 20 Aug 2024 14:32:16 +0530 Subject: [PATCH 17/28] update import API validations --- .../api/controllers/FeatureFlagController.ts | 19 ++++++++++++------- .../validators/FeatureFlagValidator.ts | 19 +++++++++++++++++++ .../feature-flags.data.service.ts | 4 ++-- .../import-feature-flag-modal.component.ts | 6 +++--- 4 files changed, 36 insertions(+), 12 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index 76c14f4f14..0fe579627c 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -10,8 +10,13 @@ import { } from './validators/FeatureFlagsPaginatedParamsValidator'; import { FeatureFlagFilterModeUpdateValidator } from './validators/FeatureFlagFilterModeUpdateValidator'; import { AppRequest, PaginationResponse } from '../../types'; -import { SERVER_ERROR, IFeatureFlagFile } from 'upgrade_types'; -import { FeatureFlagValidation, IdValidator, UserParamsValidator } from './validators/FeatureFlagValidator'; +import { SERVER_ERROR } from 'upgrade_types'; +import { + FeatureFlagImportValidation, + FeatureFlagValidation, + IdValidator, + UserParamsValidator, +} from './validators/FeatureFlagValidator'; import { ExperimentUserService } from '../services/ExperimentUserService'; import { FeatureFlagListValidator } from '../controllers/validators/FeatureFlagListValidator'; import { Segment } from 'src/api/models/Segment'; @@ -314,7 +319,7 @@ export class FeatureFlagsController { * parameters: * - in: body * name: statusUpdate - * description: Updating the featur flag's status + * description: Updating the feature flag's status * schema: * type: object * required: @@ -352,7 +357,7 @@ export class FeatureFlagsController { * parameters: * - in: body * name: updateFilterMode - * description: Updating the featur flag's filter mode + * description: Updating the feature flag's filter mode * schema: * type: object * required: @@ -457,7 +462,7 @@ export class FeatureFlagsController { * - application/json * parameters: * - in: body - * name: addinclusionList + * name: add inclusionList * description: Adding an inclusion list to the feature flag * schema: * type: object @@ -701,9 +706,9 @@ export class FeatureFlagsController { */ @Post('/import/validation') public async validateImportFeatureFlags( - @Body({ validate: true }) featureFlags: IFeatureFlagFile[], + @Body({ validate: true }) featureFlags: FeatureFlagImportValidation, @Req() request: AppRequest ): Promise { - return await this.featureFlagService.validateImportFeatureFlags(featureFlags, request.logger); + return await this.featureFlagService.validateImportFeatureFlags(featureFlags.files, request.logger); } } diff --git a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts index 7a700c8fa4..55650c2145 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts @@ -73,3 +73,22 @@ export class IdValidator { @IsUUID() public id: string; } + +export class FeatureFlagImportValidation { + @IsArray() + @ValidateNested({ each: true }) + @Type(() => FeatureFlagFile) + public files: FeatureFlagFile[]; +} + +class FeatureFlagFile { + @IsString() + @IsNotEmpty() + @IsDefined() + public fileName: string; + + @IsString() + @IsNotEmpty() + @IsDefined() + public fileContent: string; +} diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts index a3ccf47e50..9e47713dfb 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts @@ -45,12 +45,12 @@ export class FeatureFlagsDataService { return this.http.put(url, flag); } - validateFeatureFlag(featureFlag: IFeatureFlagFile[]) { + validateFeatureFlag(featureFlag: { files: IFeatureFlagFile[] }) { const url = this.environment.api.validateFeatureFlag; return this.http.post(url, featureFlag); } - importFeatureFlag(featureFlag: IFeatureFlagFile[]) { + importFeatureFlag(featureFlag: { files: IFeatureFlagFile[] }) { const url = this.environment.api.importFeatureFlag; return this.http.post(url, featureFlag); } diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts index 1604c18bc7..4176014fb8 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/modals/import-feature-flag-modal/import-feature-flag-modal.component.ts @@ -4,7 +4,7 @@ import { CommonStatusIndicatorChipComponent, } from '../../../../../shared-standalone-component-lib/components'; import { BehaviorSubject, Observable, combineLatest, firstValueFrom, map } from 'rxjs'; -import { MAT_DIALOG_DATA, MatDialog, MatDialogRef } from '@angular/material/dialog'; +import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { CommonModule } from '@angular/common'; import { SharedModule } from '../../../../../shared/shared.module'; import { CommonImportContainerComponent } from '../../../../../shared-standalone-component-lib/components/common-import-container/common-import-container.component'; @@ -87,7 +87,7 @@ export class ImportFeatureFlagModalComponent { async checkValidation(files: IFeatureFlagFile[]) { try { const validationErrors = (await firstValueFrom( - this.featureFlagsDataService.validateFeatureFlag(files) + this.featureFlagsDataService.validateFeatureFlag({ files: files }) )) as ValidateFeatureFlagError[]; this.fileValidationErrors = validationErrors.filter((data) => data.compatibilityType != null) || []; this.fileValidationErrorDataSource.data = this.fileValidationErrors; @@ -114,7 +114,7 @@ export class ImportFeatureFlagModalComponent { try { this.isImportActionBtnDisabled.next(true); const importResult = (await firstValueFrom( - this.featureFlagsDataService.importFeatureFlag(this.fileData) + this.featureFlagsDataService.importFeatureFlag({ files: this.fileData }) )) as importError[]; this.showNotification(importResult); From cc21f55b62dc41bdc83adf1ddf2c1d714c85c7f5 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Thu, 22 Aug 2024 14:29:25 +0530 Subject: [PATCH 18/28] make all function of import in a transaction --- .../api/controllers/FeatureFlagController.ts | 4 +- .../FeatureFlagSegmentExclusionRepository.ts | 2 +- .../FeatureFlagSegmentInclusionRepository.ts | 2 +- .../src/api/services/FeatureFlagService.ts | 173 +++++++++++------- .../FeatureFlagInclusionExclusion.ts | 4 +- .../unit/services/FeatureFlagService.test.ts | 2 +- 6 files changed, 112 insertions(+), 75 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index f19e109210..179e01443b 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -480,7 +480,7 @@ export class FeatureFlagsController { @Body({ validate: true }) inclusionList: FeatureFlagListValidator, @Req() request: AppRequest ): Promise { - return this.featureFlagService.addList(inclusionList, 'inclusion', request.logger); + return this.featureFlagService.addList([inclusionList], 'inclusion', request.logger)[0]; } /** @@ -510,7 +510,7 @@ export class FeatureFlagsController { @Body({ validate: true }) exclusionList: FeatureFlagListValidator, @Req() request: AppRequest ): Promise { - return this.featureFlagService.addList(exclusionList, 'exclusion', request.logger); + return this.featureFlagService.addList([exclusionList], 'exclusion', request.logger)[0]; } /** diff --git a/backend/packages/Upgrade/src/api/repositories/FeatureFlagSegmentExclusionRepository.ts b/backend/packages/Upgrade/src/api/repositories/FeatureFlagSegmentExclusionRepository.ts index 95f36816d2..5da260510a 100644 --- a/backend/packages/Upgrade/src/api/repositories/FeatureFlagSegmentExclusionRepository.ts +++ b/backend/packages/Upgrade/src/api/repositories/FeatureFlagSegmentExclusionRepository.ts @@ -6,7 +6,7 @@ import { FeatureFlagSegmentExclusion } from '../models/FeatureFlagSegmentExclusi @EntityRepository(FeatureFlagSegmentExclusion) export class FeatureFlagSegmentExclusionRepository extends Repository { public async insertData( - data: Partial, + data: Partial[], logger: UpgradeLogger, entityManager: EntityManager ): Promise { diff --git a/backend/packages/Upgrade/src/api/repositories/FeatureFlagSegmentInclusionRepository.ts b/backend/packages/Upgrade/src/api/repositories/FeatureFlagSegmentInclusionRepository.ts index ab032dd213..1d86369e30 100644 --- a/backend/packages/Upgrade/src/api/repositories/FeatureFlagSegmentInclusionRepository.ts +++ b/backend/packages/Upgrade/src/api/repositories/FeatureFlagSegmentInclusionRepository.ts @@ -6,7 +6,7 @@ import { FeatureFlagSegmentInclusion } from '../models/FeatureFlagSegmentInclusi @EntityRepository(FeatureFlagSegmentInclusion) export class FeatureFlagSegmentInclusionRepository extends Repository { public async insertData( - data: Partial, + data: Partial[], logger: UpgradeLogger, entityManager: EntityManager ): Promise { diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 5785b56954..8c20212a32 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -7,7 +7,7 @@ import { InjectRepository } from 'typeorm-typedi-extensions'; import { FeatureFlagRepository } from '../repositories/FeatureFlagRepository'; import { FeatureFlagSegmentInclusionRepository } from '../repositories/FeatureFlagSegmentInclusionRepository'; import { FeatureFlagSegmentExclusionRepository } from '../repositories/FeatureFlagSegmentExclusionRepository'; -import { getConnection } from 'typeorm'; +import { EntityManager, getConnection } from 'typeorm'; import { v4 as uuid } from 'uuid'; import { IFeatureFlagSearchParams, @@ -205,25 +205,36 @@ export class FeatureFlagService { return this.updateFeatureFlagInDB(this.featureFlagValidatorToFlag(flagDTO), logger); } - private async addFeatureFlagInDB(flag: FeatureFlag, logger: UpgradeLogger): Promise { + private async addFeatureFlagInDB( + flag: FeatureFlag, + logger: UpgradeLogger, + entityManager?: EntityManager + ): Promise { flag.id = uuid(); // saving feature flag doc - let featureFlagDoc: FeatureFlag; - await getConnection().transaction(async (transactionalEntityManager) => { + + const executeTransaction = async (manager: EntityManager): Promise => { try { - featureFlagDoc = ( - await this.featureFlagRepository.insertFeatureFlag(flag as any, transactionalEntityManager) - )[0]; + const featureFlagDoc = (await this.featureFlagRepository.insertFeatureFlag(flag as any, manager))[0]; + return featureFlagDoc; } catch (err) { const error = new Error(`Error in creating feature flag document "addFeatureFlagInDB" ${err}`); (error as any).type = SERVER_ERROR.QUERY_FAILED; logger.error(error); throw error; } - }); + }; // TODO: Add log for feature flag creation - return featureFlagDoc; + if (entityManager) { + // Use the provided entity manager + return await executeTransaction(entityManager); + } else { + // Create a new transaction if no entity manager is provided + return await getConnection().transaction(async (manager) => { + return await executeTransaction(manager); + }); + } } private async updateFeatureFlagInDB(flag: FeatureFlag, logger: UpgradeLogger): Promise { @@ -255,28 +266,24 @@ export class FeatureFlagService { } public async addList( - listInput: FeatureFlagListValidator, + listsInput: FeatureFlagListValidator[], filterType: string, - logger: UpgradeLogger - ): Promise { + logger: UpgradeLogger, + transactionalEntityManager?: EntityManager + ): Promise<(FeatureFlagSegmentInclusion | FeatureFlagSegmentExclusion)[]> { logger.info({ message: `Add ${filterType} list to feature flag` }); - const createdList = await getConnection().transaction(async (transactionalEntityManager) => { - const featureFlagSegmentInclusionOrExclusion = - filterType === 'inclusion' ? new FeatureFlagSegmentInclusion() : new FeatureFlagSegmentExclusion(); - featureFlagSegmentInclusionOrExclusion.enabled = listInput.enabled; - featureFlagSegmentInclusionOrExclusion.listType = listInput.listType; - const featureFlag = await this.featureFlagRepository.findOne(listInput.flagId); - - featureFlagSegmentInclusionOrExclusion.featureFlag = featureFlag; + const executeTransaction = async (manager: EntityManager) => { // create a new private segment - listInput.list.type = SEGMENT_TYPE.PRIVATE; - let newSegment: Segment; + const segmentsToCreate = listsInput.map((listInput) => { + listInput.list.type = SEGMENT_TYPE.PRIVATE; + return listInput.list; + }); + + let newSegments: Segment[]; try { - newSegment = await this.segmentService.upsertSegmentInPipeline( - listInput.list, - logger, - transactionalEntityManager + newSegments = await Promise.all( + segmentsToCreate.map((segment) => this.segmentService.upsertSegmentInPipeline(segment, logger, manager)) ); } catch (err) { const error = new Error(`Error in creating private segment for feature flag ${filterType} list ${err}`); @@ -284,20 +291,35 @@ export class FeatureFlagService { logger.error(error); throw error; } - featureFlagSegmentInclusionOrExclusion.segment = newSegment; + + const featureFlags = await manager + .getRepository(FeatureFlag) + .findByIds(listsInput.map((listInput) => listInput.flagId)); + + const featureFlagSegmentInclusionOrExclusionArray = listsInput.map((listInput) => { + const featureFlagSegmentInclusionOrExclusion = + filterType === 'inclusion' ? new FeatureFlagSegmentInclusion() : new FeatureFlagSegmentExclusion(); + featureFlagSegmentInclusionOrExclusion.enabled = listInput.enabled; + featureFlagSegmentInclusionOrExclusion.listType = listInput.listType; + featureFlagSegmentInclusionOrExclusion.featureFlag = featureFlags.find((flag) => flag.id === listInput.flagId); + featureFlagSegmentInclusionOrExclusion.segment = newSegments.find( + (segment) => segment.id === listInput.list.id + ); + return featureFlagSegmentInclusionOrExclusion; + }); try { if (filterType === 'inclusion') { await this.featureFlagSegmentInclusionRepository.insertData( - featureFlagSegmentInclusionOrExclusion, + featureFlagSegmentInclusionOrExclusionArray, logger, - transactionalEntityManager + manager ); } else { await this.featureFlagSegmentExclusionRepository.insertData( - featureFlagSegmentInclusionOrExclusion, + featureFlagSegmentInclusionOrExclusionArray, logger, - transactionalEntityManager + manager ); } } catch (err) { @@ -306,9 +328,18 @@ export class FeatureFlagService { logger.error(error); throw error; } - return featureFlagSegmentInclusionOrExclusion; - }); - return createdList; + return featureFlagSegmentInclusionOrExclusionArray; + }; + + if (transactionalEntityManager) { + // Use the provided entity manager + return await executeTransaction(transactionalEntityManager); + } else { + // Create a new transaction if no entity manager is provided + return await getConnection().transaction(async (manager) => { + return await executeTransaction(manager); + }); + } } public async updateList( @@ -468,49 +499,55 @@ export class FeatureFlagService { }; }); - const validFiles: FeatureFlag[] = fileStatusArray + const validFiles: FeatureFlagValidation[] = fileStatusArray .filter((fileStatus) => fileStatus.error === null) .map((fileStatus) => { const featureFlagFile = featureFlagFiles.find((file) => file.fileName === fileStatus.fileName); return JSON.parse(featureFlagFile.fileContent as string); }); - const createdFlags = await Promise.all( - validFiles.map(async (featureFlag) => { - const { featureFlagSegmentInclusion, featureFlagSegmentExclusion, ...rest } = featureFlag; - const newFlag = await this.addFeatureFlagInDB(rest as any, logger); - - const inclusionDoc = await Promise.all( - featureFlagSegmentInclusion.map(async (segmentInclusion) => { - segmentInclusion.segment.id = uuid(); - const listInput = { - flagId: newFlag.id, - enabled: false, - listType: segmentInclusion.listType, - list: this.segmentService.convertJSONStringToSegInputValFormat(JSON.stringify(segmentInclusion.segment)), - }; - return await this.addList(listInput, 'inclusion', logger); - }) - ); + const createdFlags = await getConnection().transaction(async (transactionalEntityManager) => { + return await Promise.all( + validFiles.map(async (featureFlag) => { + const newFlag = await this.addFeatureFlagInDB( + this.featureFlagValidatorToFlag(featureFlag), + logger, + transactionalEntityManager + ); - const exclusionDoc = await Promise.all( - featureFlagSegmentExclusion.map(async (segmentExclusion) => { - segmentExclusion.segment.id = uuid(); - const listInput = { - flagId: newFlag.id, - enabled: false, - listType: segmentExclusion.listType, - list: this.segmentService.convertJSONStringToSegInputValFormat(JSON.stringify(segmentExclusion.segment)), - }; - return await this.addList(listInput, 'exclusion', logger); - }) - ); + const featureFlagSegmentInclusionList = featureFlag.featureFlagSegmentInclusion.map( + (segmentInclusionList) => { + segmentInclusionList.list.id = uuid(); + segmentInclusionList.flagId = newFlag.id; + return segmentInclusionList; + } + ); + const inclusionDoc = await this.addList( + featureFlagSegmentInclusionList, + 'inclusion', + logger, + transactionalEntityManager + ); - return { ...newFlag, featureFlagSegmentInclusion: inclusionDoc, featureFlagSegmentExclusion: exclusionDoc }; - }) - ); - logger.info({ message: 'Imported feature flags', details: createdFlags }); + const featureFlagSegmentExclusionList = featureFlag.featureFlagSegmentExclusion.map( + (segmentExclusionList) => { + segmentExclusionList.list.id = uuid(); + segmentExclusionList.flagId = newFlag.id; + return segmentExclusionList; + } + ); + const exclusionDoc = await await this.addList( + featureFlagSegmentExclusionList, + 'exclusion', + logger, + transactionalEntityManager + ); + return { ...newFlag, featureFlagSegmentInclusion: inclusionDoc, featureFlagSegmentExclusion: exclusionDoc }; + }) + ); + }); + logger.info({ message: 'Imported feature flags', details: createdFlags }); return fileStatusArray; } public async validateImportFeatureFlags( diff --git a/backend/packages/Upgrade/test/integration/FeatureFlags/FeatureFlagInclusionExclusion.ts b/backend/packages/Upgrade/test/integration/FeatureFlags/FeatureFlagInclusionExclusion.ts index 18aef804dc..24515ab1bd 100644 --- a/backend/packages/Upgrade/test/integration/FeatureFlags/FeatureFlagInclusionExclusion.ts +++ b/backend/packages/Upgrade/test/integration/FeatureFlags/FeatureFlagInclusionExclusion.ts @@ -46,8 +46,8 @@ export default async function FeatureFlagInclusionExclusionLogic(): Promise { }); it('should add an include list', async () => { - const result = await service.addList(mockList, 'include', logger); + const result = await service.addList([mockList], 'include', logger); expect(result).toBeTruthy(); }); From 9bc40e3b8b9fe60f93e5d258e6b8c81105b28649 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Fri, 23 Aug 2024 00:56:50 +0530 Subject: [PATCH 19/28] Solve failing testcases --- .../src/api/controllers/FeatureFlagController.ts | 4 ++-- .../unit/controllers/mocks/FeatureFlagServiceMock.ts | 6 ++++-- .../test/unit/services/FeatureFlagService.test.ts | 10 +++++++--- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index 179e01443b..a45e2c404e 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -480,7 +480,7 @@ export class FeatureFlagsController { @Body({ validate: true }) inclusionList: FeatureFlagListValidator, @Req() request: AppRequest ): Promise { - return this.featureFlagService.addList([inclusionList], 'inclusion', request.logger)[0]; + return (await this.featureFlagService.addList([inclusionList], 'inclusion', request.logger))[0]; } /** @@ -510,7 +510,7 @@ export class FeatureFlagsController { @Body({ validate: true }) exclusionList: FeatureFlagListValidator, @Req() request: AppRequest ): Promise { - return this.featureFlagService.addList([exclusionList], 'exclusion', request.logger)[0]; + return (await this.featureFlagService.addList([exclusionList], 'exclusion', request.logger))[0]; } /** diff --git a/backend/packages/Upgrade/test/unit/controllers/mocks/FeatureFlagServiceMock.ts b/backend/packages/Upgrade/test/unit/controllers/mocks/FeatureFlagServiceMock.ts index e1707edf6b..ef32dbed70 100644 --- a/backend/packages/Upgrade/test/unit/controllers/mocks/FeatureFlagServiceMock.ts +++ b/backend/packages/Upgrade/test/unit/controllers/mocks/FeatureFlagServiceMock.ts @@ -7,6 +7,7 @@ import { UpgradeLogger } from '../../../../src/lib/logger/UpgradeLogger'; import { FeatureFlagValidation } from '../../../../src/api/controllers/validators/FeatureFlagValidator'; import { RequestedExperimentUser } from '../../../../src/api/controllers/validators/ExperimentUserValidator'; import { FeatureFlagListValidator } from '../../../../src/api/controllers/validators/FeatureFlagListValidator'; +import { FeatureFlagSegmentInclusion } from '../../../../src/api/models/FeatureFlagSegmentInclusion'; @Service() export default class FeatureFlagServiceMock { @@ -49,8 +50,9 @@ export default class FeatureFlagServiceMock { return Promise.resolve([]); } - public addList(listInput: FeatureFlagListValidator, filterType: string, logger: UpgradeLogger): Promise<[]> { - return Promise.resolve([]); + // eslint-disable-next-line @typescript-eslint/ban-types + public addList(listInput: FeatureFlagListValidator[], filterType: string, logger: UpgradeLogger): Promise<{}[]> { + return Promise.resolve([{}]); } public deleteList(segmentId: string, logger: UpgradeLogger): Promise<[]> { diff --git a/backend/packages/Upgrade/test/unit/services/FeatureFlagService.test.ts b/backend/packages/Upgrade/test/unit/services/FeatureFlagService.test.ts index 783f872f11..af247650af 100644 --- a/backend/packages/Upgrade/test/unit/services/FeatureFlagService.test.ts +++ b/backend/packages/Upgrade/test/unit/services/FeatureFlagService.test.ts @@ -1,5 +1,5 @@ import * as sinon from 'sinon'; -import { Connection, ConnectionManager } from 'typeorm'; +import { Connection, ConnectionManager, getRepository } from 'typeorm'; import { Test, TestingModuleBuilder } from '@nestjs/testing'; import { getRepositoryToken } from '@nestjs/typeorm'; @@ -56,7 +56,7 @@ describe('Feature Flag Service Testing', () => { const mockList = new FeatureFlagListValidator(); mockList.enabled = true; - mockList.flagId = uuid(); + mockList.flagId = mockFlag1.id; mockList.listType = 'individual'; mockList.list = { name: 'name', @@ -86,7 +86,11 @@ describe('Feature Flag Service Testing', () => { getMany: jest.fn().mockResolvedValue(mockFlagArr), }; - const entityManagerMock = { createQueryBuilder: () => queryBuilderMock }; + const entityManagerMock = { + createQueryBuilder: () => queryBuilderMock, + getRepository: jest.fn().mockReturnThis(), + findByIds: jest.fn().mockResolvedValue([mockFlag1]), + }; const sandbox = sinon.createSandbox(); sandbox.stub(ConnectionManager.prototype, 'get').returns({ transaction: jest.fn(async (passedFunction) => await passedFunction(entityManagerMock)), From 661b1a83ad4e099c848128242eb2bbe0a29d4893 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Fri, 23 Aug 2024 12:11:10 +0530 Subject: [PATCH 20/28] Update import in FeatureFlagService.ts --- .../src/api/services/FeatureFlagService.ts | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 8c20212a32..2a49724016 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -7,7 +7,7 @@ import { InjectRepository } from 'typeorm-typedi-extensions'; import { FeatureFlagRepository } from '../repositories/FeatureFlagRepository'; import { FeatureFlagSegmentInclusionRepository } from '../repositories/FeatureFlagSegmentInclusionRepository'; import { FeatureFlagSegmentExclusionRepository } from '../repositories/FeatureFlagSegmentExclusionRepository'; -import { EntityManager, getConnection } from 'typeorm'; +import { EntityManager, getConnection, In } from 'typeorm'; import { v4 as uuid } from 'uuid'; import { IFeatureFlagSearchParams, @@ -522,12 +522,6 @@ export class FeatureFlagService { return segmentInclusionList; } ); - const inclusionDoc = await this.addList( - featureFlagSegmentInclusionList, - 'inclusion', - logger, - transactionalEntityManager - ); const featureFlagSegmentExclusionList = featureFlag.featureFlagSegmentExclusion.map( (segmentExclusionList) => { @@ -536,12 +530,11 @@ export class FeatureFlagService { return segmentExclusionList; } ); - const exclusionDoc = await await this.addList( - featureFlagSegmentExclusionList, - 'exclusion', - logger, - transactionalEntityManager - ); + + const [inclusionDoc, exclusionDoc] = await Promise.all([ + this.addList(featureFlagSegmentInclusionList, 'inclusion', logger, transactionalEntityManager), + this.addList(featureFlagSegmentExclusionList, 'exclusion', logger, transactionalEntityManager), + ]); return { ...newFlag, featureFlagSegmentInclusion: inclusionDoc, featureFlagSegmentExclusion: exclusionDoc }; }) @@ -555,6 +548,18 @@ export class FeatureFlagService { logger: UpgradeLogger ): Promise { logger.info({ message: 'Validate feature flags' }); + + const featureFlagsIds = featureFlagFiles + .map((featureFlagFile) => { + try { + return JSON.parse(featureFlagFile.fileContent as string).key; + } catch (parseError) { + return null; + } + }) + .filter((key) => key !== null); + const existingFeatureFlags = await this.featureFlagRepository.find({ key: In(featureFlagsIds) }); + const validationErrors = await Promise.allSettled( featureFlagFiles.map(async (featureFlagFile) => { let featureFlag: FeatureFlagValidation; @@ -568,7 +573,7 @@ export class FeatureFlagService { }; } - const error = await this.validateImportFeatureFlag(featureFlagFile.fileName, featureFlag); + const error = await this.validateImportFeatureFlag(featureFlagFile.fileName, featureFlag, existingFeatureFlags); return error; }) ); @@ -585,7 +590,11 @@ export class FeatureFlagService { .filter((error) => error !== null); } - private async validateImportFeatureFlag(fileName: string, flag: FeatureFlagValidation) { + private async validateImportFeatureFlag( + fileName: string, + flag: FeatureFlagValidation, + existingFeatureFlags: FeatureFlag[] + ) { let compatibilityType = FF_COMPATIBILITY_TYPE.COMPATIBLE; flag = plainToClass(FeatureFlagValidation, flag); @@ -596,7 +605,7 @@ export class FeatureFlagService { }); if (compatibilityType === FF_COMPATIBILITY_TYPE.COMPATIBLE) { - const keyExists = await this.featureFlagRepository.findOne({ key: flag.key }); + const keyExists = existingFeatureFlags.find((existingFlag) => existingFlag.key === flag.key); if (keyExists) { compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; From 07f7755a9cd0c1237d2efb0d08617f19a1ed77e8 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Mon, 26 Aug 2024 10:14:41 +0530 Subject: [PATCH 21/28] updated code to have transaction per import file --- .../src/api/services/FeatureFlagService.ts | 58 +++++++++---------- 1 file changed, 28 insertions(+), 30 deletions(-) diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 2a49724016..598d0bfdc1 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -506,40 +506,38 @@ export class FeatureFlagService { return JSON.parse(featureFlagFile.fileContent as string); }); - const createdFlags = await getConnection().transaction(async (transactionalEntityManager) => { - return await Promise.all( - validFiles.map(async (featureFlag) => { - const newFlag = await this.addFeatureFlagInDB( - this.featureFlagValidatorToFlag(featureFlag), - logger, - transactionalEntityManager - ); + const createdFlags = []; - const featureFlagSegmentInclusionList = featureFlag.featureFlagSegmentInclusion.map( - (segmentInclusionList) => { - segmentInclusionList.list.id = uuid(); - segmentInclusionList.flagId = newFlag.id; - return segmentInclusionList; - } - ); + for (const featureFlag of validFiles) { + const createdFlag = await getConnection().transaction(async (transactionalEntityManager) => { + const newFlag = await this.addFeatureFlagInDB( + this.featureFlagValidatorToFlag(featureFlag), + logger, + transactionalEntityManager + ); - const featureFlagSegmentExclusionList = featureFlag.featureFlagSegmentExclusion.map( - (segmentExclusionList) => { - segmentExclusionList.list.id = uuid(); - segmentExclusionList.flagId = newFlag.id; - return segmentExclusionList; - } - ); + const featureFlagSegmentInclusionList = featureFlag.featureFlagSegmentInclusion.map((segmentInclusionList) => { + segmentInclusionList.list.id = uuid(); + segmentInclusionList.flagId = newFlag.id; + return segmentInclusionList; + }); - const [inclusionDoc, exclusionDoc] = await Promise.all([ - this.addList(featureFlagSegmentInclusionList, 'inclusion', logger, transactionalEntityManager), - this.addList(featureFlagSegmentExclusionList, 'exclusion', logger, transactionalEntityManager), - ]); + const featureFlagSegmentExclusionList = featureFlag.featureFlagSegmentExclusion.map((segmentExclusionList) => { + segmentExclusionList.list.id = uuid(); + segmentExclusionList.flagId = newFlag.id; + return segmentExclusionList; + }); - return { ...newFlag, featureFlagSegmentInclusion: inclusionDoc, featureFlagSegmentExclusion: exclusionDoc }; - }) - ); - }); + const [inclusionDoc, exclusionDoc] = await Promise.all([ + this.addList(featureFlagSegmentInclusionList, 'inclusion', logger, transactionalEntityManager), + this.addList(featureFlagSegmentExclusionList, 'exclusion', logger, transactionalEntityManager), + ]); + + return { ...newFlag, featureFlagSegmentInclusion: inclusionDoc, featureFlagSegmentExclusion: exclusionDoc }; + }); + + createdFlags.push(createdFlag); + } logger.info({ message: 'Imported feature flags', details: createdFlags }); return fileStatusArray; } From 8bd7cb1430a7545c2a0bcbd2a3c76a9fe02549fa Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Thu, 29 Aug 2024 12:08:30 +0530 Subject: [PATCH 22/28] Rename 'list' to 'segment' in FeatureFlagListValidator and related files --- .../api/controllers/FeatureFlagController.ts | 8 +++---- .../validators/FeatureFlagListValidator.ts | 2 +- .../src/api/services/FeatureFlagService.ts | 22 +++++++++---------- .../feature-flags.data.service.ts | 4 ++-- .../app/core/segments/store/segments.model.ts | 6 ++--- .../upsert-feature-flag-modal.component.ts | 6 ++--- ...-flag-inclusions-section-card.component.ts | 2 +- ...rt-private-segment-list-modal.component.ts | 6 ++--- 8 files changed, 28 insertions(+), 28 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index a45e2c404e..65ab2db9c7 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -547,10 +547,10 @@ export class FeatureFlagsController { @Body({ validate: true }) exclusionList: FeatureFlagListValidator, @Req() request: AppRequest ): Promise { - if (id !== exclusionList.list.id) { + if (id !== exclusionList.segment.id) { return Promise.reject( new Error( - `${SERVER_ERROR.INCORRECT_PARAM_FORMAT}: The id in the URL (${id}) does not match the list id in the request body (${exclusionList.list.id}).` + `${SERVER_ERROR.INCORRECT_PARAM_FORMAT}: The id in the URL (${id}) does not match the list id in the request body (${exclusionList.segment.id}).` ) ); } @@ -591,10 +591,10 @@ export class FeatureFlagsController { @Body({ validate: true }) inclusionList: FeatureFlagListValidator, @Req() request: AppRequest ): Promise { - if (id !== inclusionList.list.id) { + if (id !== inclusionList.segment.id) { return Promise.reject( new Error( - `${SERVER_ERROR.INCORRECT_PARAM_FORMAT}: The id in the URL (${id}) does not match the list id in the request body (${inclusionList.list.id}).` + `${SERVER_ERROR.INCORRECT_PARAM_FORMAT}: The id in the URL (${id}) does not match the list id in the request body (${inclusionList.segment.id}).` ) ); } diff --git a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagListValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagListValidator.ts index 9352688e90..0babfac3a2 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagListValidator.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagListValidator.ts @@ -18,5 +18,5 @@ export class FeatureFlagListValidator { @ValidateNested() @Type(() => SegmentInputValidator) - public list: SegmentInputValidator; + public segment: SegmentInputValidator; } diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 598d0bfdc1..225ffbc673 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -276,8 +276,8 @@ export class FeatureFlagService { const executeTransaction = async (manager: EntityManager) => { // create a new private segment const segmentsToCreate = listsInput.map((listInput) => { - listInput.list.type = SEGMENT_TYPE.PRIVATE; - return listInput.list; + listInput.segment.type = SEGMENT_TYPE.PRIVATE; + return listInput.segment; }); let newSegments: Segment[]; @@ -303,7 +303,7 @@ export class FeatureFlagService { featureFlagSegmentInclusionOrExclusion.listType = listInput.listType; featureFlagSegmentInclusionOrExclusion.featureFlag = featureFlags.find((flag) => flag.id === listInput.flagId); featureFlagSegmentInclusionOrExclusion.segment = newSegments.find( - (segment) => segment.id === listInput.list.id + (segment) => segment.id === listInput.segment.id ); return featureFlagSegmentInclusionOrExclusion; }); @@ -353,19 +353,19 @@ export class FeatureFlagService { let existingRecord: FeatureFlagSegmentInclusion | FeatureFlagSegmentExclusion; if (filterType === 'inclusion') { existingRecord = await this.featureFlagSegmentInclusionRepository.findOne({ - where: { featureFlag: { id: listInput.flagId }, segment: { id: listInput.list.id } }, + where: { featureFlag: { id: listInput.flagId }, segment: { id: listInput.segment.id } }, relations: ['featureFlag', 'segment'], }); } else { existingRecord = await this.featureFlagSegmentExclusionRepository.findOne({ - where: { featureFlag: { id: listInput.flagId }, segment: { id: listInput.list.id } }, + where: { featureFlag: { id: listInput.flagId }, segment: { id: listInput.segment.id } }, relations: ['featureFlag', 'segment'], }); } if (!existingRecord) { throw new Error( - `No existing ${filterType} record found for feature flag ${listInput.flagId} and segment ${listInput.list.id}` + `No existing ${filterType} record found for feature flag ${listInput.flagId} and segment ${listInput.segment.id}` ); } @@ -376,7 +376,7 @@ export class FeatureFlagService { // Update the segment try { const updatedSegment = await this.segmentService.upsertSegmentInPipeline( - listInput.list, + listInput.segment, logger, transactionalEntityManager ); @@ -517,13 +517,13 @@ export class FeatureFlagService { ); const featureFlagSegmentInclusionList = featureFlag.featureFlagSegmentInclusion.map((segmentInclusionList) => { - segmentInclusionList.list.id = uuid(); + segmentInclusionList.segment.id = uuid(); segmentInclusionList.flagId = newFlag.id; return segmentInclusionList; }); const featureFlagSegmentExclusionList = featureFlag.featureFlagSegmentExclusion.map((segmentExclusionList) => { - segmentExclusionList.list.id = uuid(); + segmentExclusionList.segment.id = uuid(); segmentExclusionList.flagId = newFlag.id; return segmentExclusionList; }); @@ -610,10 +610,10 @@ export class FeatureFlagService { } else { const segmentIds = [ ...flag.featureFlagSegmentInclusion.flatMap((segmentInclusion) => { - return segmentInclusion.list.subSegmentIds; + return segmentInclusion.segment.subSegmentIds; }), ...flag.featureFlagSegmentExclusion.flatMap((segmentExclusion) => { - return segmentExclusion.list.subSegmentIds; + return segmentExclusion.segment.subSegmentIds; }), ]; diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts index 9e47713dfb..e11d0e49b0 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/feature-flags.data.service.ts @@ -87,7 +87,7 @@ export class FeatureFlagsDataService { } updateInclusionList(list: EditPrivateSegmentListRequest): Observable { - const url = `${this.environment.api.addFlagInclusionList}/${list.list.id}`; + const url = `${this.environment.api.addFlagInclusionList}/${list.segment.id}`; return this.http.put(url, list); } @@ -102,7 +102,7 @@ export class FeatureFlagsDataService { } updateExclusionList(list: EditPrivateSegmentListRequest): Observable { - const url = `${this.environment.api.addFlagExclusionList}/${list.list.id}`; + const url = `${this.environment.api.addFlagExclusionList}/${list.segment.id}`; return this.http.put(url, list); } 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 43253eee82..a7beade03c 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 @@ -196,13 +196,13 @@ export interface PrivateSegmentListRequest { flagId: string; enabled: boolean; listType: string; - list: AddPrivateSegmentListRequestDetails | EditPrivateSegmentListDetails; + segment: AddPrivateSegmentListRequestDetails | EditPrivateSegmentListDetails; } export interface AddPrivateSegmentListRequest extends PrivateSegmentListRequest { - list: AddPrivateSegmentListRequestDetails; + segment: AddPrivateSegmentListRequestDetails; } export interface EditPrivateSegmentListRequest extends PrivateSegmentListRequest { - list: EditPrivateSegmentListDetails; + segment: EditPrivateSegmentListDetails; } 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 1b0fe3feac..7ed642078d 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 @@ -195,7 +195,7 @@ export class UpsertFeatureFlagModalComponent { createEditRequest( { name, key, description, appContext, tags }: FeatureFlagFormData, - { id, status, filterMode, featureFlagSegmentInclusion, featureFlagSegmentExclusion }: FeatureFlag + { id, status, filterMode }: FeatureFlag ): void { const flagRequest: UpdateFeatureFlagRequest = { id, @@ -206,8 +206,8 @@ export class UpsertFeatureFlagModalComponent { tags, status, filterMode, - featureFlagSegmentInclusion, - featureFlagSegmentExclusion, + featureFlagSegmentInclusion: [], + featureFlagSegmentExclusion: [], }; this.featureFlagsService.updateFeatureFlag(flagRequest); diff --git a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-details-page/feature-flag-details-page-content/feature-flag-inclusions-section-card/feature-flag-inclusions-section-card.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-details-page/feature-flag-details-page-content/feature-flag-inclusions-section-card/feature-flag-inclusions-section-card.component.ts index 98b0bc2a01..55de97cf29 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-details-page/feature-flag-details-page-content/feature-flag-inclusions-section-card/feature-flag-inclusions-section-card.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/feature-flags/pages/feature-flag-details-page/feature-flag-details-page-content/feature-flag-inclusions-section-card/feature-flag-inclusions-section-card.component.ts @@ -169,7 +169,7 @@ export class FeatureFlagInclusionsSectionCardComponent { flagId, enabled, listType, - list, + segment: list, }; this.sendUpdateFeatureFlagInclusionRequest(listRequest); diff --git a/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-private-segment-list-modal/upsert-private-segment-list-modal.component.ts b/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-private-segment-list-modal/upsert-private-segment-list-modal.component.ts index 6f7d73e22e..970a555593 100644 --- a/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-private-segment-list-modal/upsert-private-segment-list-modal.component.ts +++ b/frontend/projects/upgrade/src/app/features/dashboard/segments/modals/upsert-private-segment-list-modal/upsert-private-segment-list-modal.component.ts @@ -286,14 +286,14 @@ export class UpsertPrivateSegmentListModalComponent { flagId: this.config.params.flagId, enabled: this.config.params.sourceList?.enabled || isExcludeList, // Maintain existing status for edits, default to false for new include lists, true for all exclude lists listType, - list, + segment: list, }; const addListRequest: AddPrivateSegmentListRequest = listRequest; const editRequest: EditPrivateSegmentListRequest = { ...listRequest, - list: { - ...listRequest.list, + segment: { + ...listRequest.segment, id: this.config.params.sourceList?.segment?.id, }, }; From fb02b2a21fbf02508114fa6b97b19faa9644ca01 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Fri, 30 Aug 2024 09:11:46 +0530 Subject: [PATCH 23/28] revert unwanted changes --- .../upsert-feature-flag-modal.component.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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 7ed642078d..1b0fe3feac 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 @@ -195,7 +195,7 @@ export class UpsertFeatureFlagModalComponent { createEditRequest( { name, key, description, appContext, tags }: FeatureFlagFormData, - { id, status, filterMode }: FeatureFlag + { id, status, filterMode, featureFlagSegmentInclusion, featureFlagSegmentExclusion }: FeatureFlag ): void { const flagRequest: UpdateFeatureFlagRequest = { id, @@ -206,8 +206,8 @@ export class UpsertFeatureFlagModalComponent { tags, status, filterMode, - featureFlagSegmentInclusion: [], - featureFlagSegmentExclusion: [], + featureFlagSegmentInclusion, + featureFlagSegmentExclusion, }; this.featureFlagsService.updateFeatureFlag(flagRequest); From 0aac418165e183e8c3e086c9b5171d2289498914 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Fri, 30 Aug 2024 09:47:17 +0530 Subject: [PATCH 24/28] solve failing testcases --- .../FeatureFlags/FeatureFlagInclusionExclusion.ts | 4 ++-- .../Upgrade/test/unit/services/FeatureFlagService.test.ts | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/backend/packages/Upgrade/test/integration/FeatureFlags/FeatureFlagInclusionExclusion.ts b/backend/packages/Upgrade/test/integration/FeatureFlags/FeatureFlagInclusionExclusion.ts index 8de3731111..9508946425 100644 --- a/backend/packages/Upgrade/test/integration/FeatureFlags/FeatureFlagInclusionExclusion.ts +++ b/backend/packages/Upgrade/test/integration/FeatureFlags/FeatureFlagInclusionExclusion.ts @@ -20,7 +20,7 @@ export default async function FeatureFlagInclusionExclusionLogic(): Promise { mockList.enabled = true; mockList.flagId = mockFlag1.id; mockList.listType = 'individual'; - mockList.list = { + mockList.segment = { name: 'name', id: uuid(), context: 'context', @@ -363,7 +363,7 @@ describe('Feature Flag Service Testing', () => { }); it('should delete an include list', async () => { - const result = await service.deleteList(mockList.list.id, logger); + const result = await service.deleteList(mockList.segment.id, logger); expect(result).toBeTruthy(); }); From 17cd315b7791c4093a2e92b755b618c4fc1ec145 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Mon, 2 Sep 2024 09:25:22 +0530 Subject: [PATCH 25/28] refractor the import data format to match exported file format --- .../validators/FeatureFlagImportValidator.ts | 156 ++++++++++++++++++ .../src/api/services/FeatureFlagService.ts | 51 ++++-- 2 files changed, 196 insertions(+), 11 deletions(-) create mode 100644 backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagImportValidator.ts diff --git a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagImportValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagImportValidator.ts new file mode 100644 index 0000000000..9b0c7df50d --- /dev/null +++ b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagImportValidator.ts @@ -0,0 +1,156 @@ +import { + IsNotEmpty, + IsDefined, + IsString, + IsArray, + IsEnum, + IsOptional, + ValidateNested, + IsUUID, + IsBoolean, +} from 'class-validator'; +import { FILTER_MODE, SEGMENT_TYPE } from 'upgrade_types'; +import { FEATURE_FLAG_STATUS } from 'upgrade_types'; +import { Type } from 'class-transformer'; + +class IndividualValidator { + @IsNotEmpty() + @IsString() + public userId: string; +} + +class GroupValidator { + @IsNotEmpty() + @IsString() + public groupId: string; + + @IsNotEmpty() + @IsString() + public type: string; +} + +class SegmentValidator { + @IsOptional() + @IsUUID() + @IsString() + public id?: string; + + @IsNotEmpty() + @IsString() + public name: string; + + @IsString() + @IsOptional() + public description?: string; + + @IsNotEmpty() + @IsString() + public context: string; + + @IsNotEmpty() + @IsEnum(SEGMENT_TYPE) + public type: SEGMENT_TYPE; +} + +class SegmentImportValidator { + @IsOptional() + @IsUUID() + @IsString() + public id?: string; + + @IsNotEmpty() + @IsString() + public name: string; + + @IsString() + @IsOptional() + public description?: string; + + @IsNotEmpty() + @IsString() + public context: string; + + @IsNotEmpty() + @IsEnum(SEGMENT_TYPE) + public type: SEGMENT_TYPE; + + @IsArray() + @ValidateNested() + @Type(() => IndividualValidator) + public individualForSegment: IndividualValidator[]; + + @IsArray() + @ValidateNested({ each: true }) + @Type(() => GroupValidator) + public groupForSegment: GroupValidator[]; + + @IsArray() + @ValidateNested({ each: true }) + @Type(() => SegmentValidator) + public subSegments: SegmentValidator[]; +} + +class FeatureFlagListImportValidator { + @IsDefined() + @IsBoolean() + public enabled: boolean; + + @IsNotEmpty() + @IsDefined() + public listType: string; + + @ValidateNested() + @Type(() => SegmentImportValidator) + public segment: SegmentImportValidator; +} + +export class FeatureFlagImportDataValidation { + @IsOptional() + @IsString() + id: string; + + @IsNotEmpty() + @IsDefined() + @IsString() + name: string; + + @IsOptional() + @IsString() + description: string; + + @IsNotEmpty() + @IsDefined() + @IsString() + key: string; + + @IsNotEmpty() + @IsDefined() + @IsEnum(FEATURE_FLAG_STATUS) + status: FEATURE_FLAG_STATUS; + + @IsNotEmpty() + @IsArray() + @IsString({ each: true }) + public context: string[]; + + @IsDefined() + @IsEnum(FILTER_MODE) + filterMode: FILTER_MODE; + + @IsNotEmpty() + @IsArray() + @IsString({ each: true }) + public tags: string[]; + + @IsOptional() + @IsArray() + @ValidateNested({ each: true }) + @Type(() => FeatureFlagListImportValidator) + public featureFlagSegmentInclusion?: FeatureFlagListImportValidator[]; + + @IsOptional() + @IsArray() + @ValidateNested({ each: true }) + @Type(() => FeatureFlagListImportValidator) + public featureFlagSegmentExclusion?: FeatureFlagListImportValidator[]; +} diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index 225ffbc673..e8e21460a9 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -34,6 +34,7 @@ import { ErrorWithType } from '../errors/ErrorWithType'; import { RequestedExperimentUser } from '../controllers/validators/ExperimentUserValidator'; import { validate } from 'class-validator'; import { plainToClass } from 'class-transformer'; +import { FeatureFlagImportDataValidation } from '../controllers/validators/FeatureFlagImportValidator'; @Service() export class FeatureFlagService { @@ -433,7 +434,7 @@ export class FeatureFlagService { return searchStringConcatenated; } - private featureFlagValidatorToFlag(flagDTO: FeatureFlagValidation) { + private featureFlagValidatorToFlag(flagDTO: FeatureFlagValidation | FeatureFlagImportDataValidation) { const featureFlag = new FeatureFlag(); featureFlag.name = flagDTO.name; featureFlag.description = flagDTO.description; @@ -499,7 +500,7 @@ export class FeatureFlagService { }; }); - const validFiles: FeatureFlagValidation[] = fileStatusArray + const validFiles: FeatureFlagImportDataValidation[] = fileStatusArray .filter((fileStatus) => fileStatus.error === null) .map((fileStatus) => { const featureFlagFile = featureFlagFiles.find((file) => file.fileName === fileStatus.fileName); @@ -518,14 +519,42 @@ export class FeatureFlagService { const featureFlagSegmentInclusionList = featureFlag.featureFlagSegmentInclusion.map((segmentInclusionList) => { segmentInclusionList.segment.id = uuid(); - segmentInclusionList.flagId = newFlag.id; - return segmentInclusionList; + + const userIds = segmentInclusionList.segment.individualForSegment.map((individual) => + individual.userId ? individual.userId : null + ); + const subSegmentIds = segmentInclusionList.segment.subSegments.map((subSegment) => + subSegment.id ? subSegment.id : null + ); + const groups = segmentInclusionList.segment.groupForSegment.map((group) => { + return group.type && group.groupId ? { type: group.type, groupId: group.groupId } : null; + }); + + return { + ...segmentInclusionList, + flagId: newFlag.id, + segment: { ...segmentInclusionList.segment, userIds, subSegmentIds, groups }, + }; }); const featureFlagSegmentExclusionList = featureFlag.featureFlagSegmentExclusion.map((segmentExclusionList) => { segmentExclusionList.segment.id = uuid(); - segmentExclusionList.flagId = newFlag.id; - return segmentExclusionList; + + const userIds = segmentExclusionList.segment.individualForSegment.map((individual) => + individual.userId ? individual.userId : null + ); + const subSegmentIds = segmentExclusionList.segment.subSegments.map((subSegment) => + subSegment.id ? subSegment.id : null + ); + const groups = segmentExclusionList.segment.groupForSegment.map((group) => { + return group.type && group.groupId ? { type: group.type, groupId: group.groupId } : null; + }); + + return { + ...segmentExclusionList, + flagId: newFlag.id, + segment: { ...segmentExclusionList.segment, userIds, subSegmentIds, groups }, + }; }); const [inclusionDoc, exclusionDoc] = await Promise.all([ @@ -560,7 +589,7 @@ export class FeatureFlagService { const validationErrors = await Promise.allSettled( featureFlagFiles.map(async (featureFlagFile) => { - let featureFlag: FeatureFlagValidation; + let featureFlag: FeatureFlagImportDataValidation; try { featureFlag = JSON.parse(featureFlagFile.fileContent as string); } catch (parseError) { @@ -590,12 +619,12 @@ export class FeatureFlagService { private async validateImportFeatureFlag( fileName: string, - flag: FeatureFlagValidation, + flag: FeatureFlagImportDataValidation, existingFeatureFlags: FeatureFlag[] ) { let compatibilityType = FF_COMPATIBILITY_TYPE.COMPATIBLE; - flag = plainToClass(FeatureFlagValidation, flag); + flag = plainToClass(FeatureFlagImportDataValidation, flag); await validate(flag).then((errors) => { if (errors.length > 0) { compatibilityType = FF_COMPATIBILITY_TYPE.INCOMPATIBLE; @@ -610,10 +639,10 @@ export class FeatureFlagService { } else { const segmentIds = [ ...flag.featureFlagSegmentInclusion.flatMap((segmentInclusion) => { - return segmentInclusion.segment.subSegmentIds; + return segmentInclusion.segment.subSegments.map((subSegment) => subSegment.id); }), ...flag.featureFlagSegmentExclusion.flatMap((segmentExclusion) => { - return segmentExclusion.segment.subSegmentIds; + return segmentExclusion.segment.subSegments.map((subSegment) => subSegment.id); }), ]; From b7baef58e95d2d960876881b8bde400a59368558 Mon Sep 17 00:00:00 2001 From: Ridham Shah <49234788+RidhamShah@users.noreply.github.com> Date: Tue, 3 Sep 2024 14:10:57 +0530 Subject: [PATCH 26/28] Solve feature flag throwing error on update (#1885) * solve feature flag throwing error update * Removed unused properties from featureFlags related interfaces --------- Co-authored-by: RidhamShah --- .../core/feature-flags/store/feature-flags.actions.ts | 1 - .../app/core/feature-flags/store/feature-flags.model.ts | 9 ++++++--- .../upsert-feature-flag-modal.component.ts | 6 +----- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts index 346cebfe03..ce4d86cee1 100644 --- a/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts +++ b/frontend/projects/upgrade/src/app/core/feature-flags/store/feature-flags.actions.ts @@ -71,7 +71,6 @@ export const actionSetIsLoadingImportFeatureFlag = createAction( props<{ isLoadingImportFeatureFlag: boolean }>() ); - export const actionEmailFeatureFlagData = createAction( '[Feature Flags] Email Feature Flag Data', props<{ featureFlagId: string }>() 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 e42c81bc18..e088dc998f 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 @@ -10,8 +10,7 @@ export interface GeneralCRUDResponseFields { versionNumber: number; } -// Fields belonging to the FeatureFlag entity itself that are not part of the CRUD response -export interface BaseFeatureFlag { +export interface CoreFeatureFlagDetails { id?: string; name: string; key: string; @@ -20,6 +19,10 @@ export interface BaseFeatureFlag { tags: string[]; status: FEATURE_FLAG_STATUS; filterMode: FILTER_MODE; +} + +// Fields belonging to the FeatureFlag entity itself that are not part of the CRUD response +export interface BaseFeatureFlag extends CoreFeatureFlagDetails { featureFlagSegmentInclusion: FeatureFlagSegmentListDetails[]; featureFlagSegmentExclusion: FeatureFlagSegmentListDetails[]; } @@ -28,7 +31,7 @@ export interface BaseFeatureFlag { export type FeatureFlag = BaseFeatureFlag & GeneralCRUDResponseFields; // Currently there is no difference between these types, but they semantically different and could diverge later -export type AddFeatureFlagRequest = BaseFeatureFlag; +export type AddFeatureFlagRequest = CoreFeatureFlagDetails; // so that we can throw an error if we try to update the id export interface UpdateFeatureFlagRequest extends AddFeatureFlagRequest { 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 1b0fe3feac..e48ae26312 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 @@ -186,8 +186,6 @@ export class UpsertFeatureFlagModalComponent { tags, status: FEATURE_FLAG_STATUS.DISABLED, filterMode: FILTER_MODE.EXCLUDE_ALL, - featureFlagSegmentInclusion: [], - featureFlagSegmentExclusion: [], }; this.featureFlagsService.addFeatureFlag(flagRequest); @@ -195,7 +193,7 @@ export class UpsertFeatureFlagModalComponent { createEditRequest( { name, key, description, appContext, tags }: FeatureFlagFormData, - { id, status, filterMode, featureFlagSegmentInclusion, featureFlagSegmentExclusion }: FeatureFlag + { id, status, filterMode }: FeatureFlag ): void { const flagRequest: UpdateFeatureFlagRequest = { id, @@ -206,8 +204,6 @@ export class UpsertFeatureFlagModalComponent { tags, status, filterMode, - featureFlagSegmentInclusion, - featureFlagSegmentExclusion, }; this.featureFlagsService.updateFeatureFlag(flagRequest); From a4f6025d18f5aa30cd0c3a3a095cb1836a2fdd5b Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Wed, 4 Sep 2024 10:29:04 +0530 Subject: [PATCH 27/28] Make interface for common properties of validator --- .../validators/FeatureFlagImportValidator.ts | 43 ++----------------- .../validators/FeatureFlagValidator.ts | 16 ++++--- 2 files changed, 12 insertions(+), 47 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagImportValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagImportValidator.ts index 9b0c7df50d..403a4db4af 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagImportValidator.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagImportValidator.ts @@ -9,9 +9,9 @@ import { IsUUID, IsBoolean, } from 'class-validator'; -import { FILTER_MODE, SEGMENT_TYPE } from 'upgrade_types'; -import { FEATURE_FLAG_STATUS } from 'upgrade_types'; +import { SEGMENT_TYPE } from 'upgrade_types'; import { Type } from 'class-transformer'; +import { FeatureFlagCoreValidation } from './FeatureFlagValidator'; class IndividualValidator { @IsNotEmpty() @@ -104,44 +104,7 @@ class FeatureFlagListImportValidator { public segment: SegmentImportValidator; } -export class FeatureFlagImportDataValidation { - @IsOptional() - @IsString() - id: string; - - @IsNotEmpty() - @IsDefined() - @IsString() - name: string; - - @IsOptional() - @IsString() - description: string; - - @IsNotEmpty() - @IsDefined() - @IsString() - key: string; - - @IsNotEmpty() - @IsDefined() - @IsEnum(FEATURE_FLAG_STATUS) - status: FEATURE_FLAG_STATUS; - - @IsNotEmpty() - @IsArray() - @IsString({ each: true }) - public context: string[]; - - @IsDefined() - @IsEnum(FILTER_MODE) - filterMode: FILTER_MODE; - - @IsNotEmpty() - @IsArray() - @IsString({ each: true }) - public tags: string[]; - +export class FeatureFlagImportDataValidation extends FeatureFlagCoreValidation { @IsOptional() @IsArray() @ValidateNested({ each: true }) diff --git a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts index 55650c2145..88a8a411b2 100644 --- a/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts +++ b/backend/packages/Upgrade/src/api/controllers/validators/FeatureFlagValidator.ts @@ -4,29 +4,29 @@ import { FEATURE_FLAG_STATUS } from 'upgrade_types'; import { Type } from 'class-transformer'; import { FeatureFlagListValidator } from './FeatureFlagListValidator'; -export class FeatureFlagValidation { +export class FeatureFlagCoreValidation { @IsOptional() @IsString() - id: string; + public id: string; @IsNotEmpty() @IsDefined() @IsString() - name: string; + public name: string; @IsOptional() @IsString() - description: string; + public description: string; @IsNotEmpty() @IsDefined() @IsString() - key: string; + public key: string; @IsNotEmpty() @IsDefined() @IsEnum(FEATURE_FLAG_STATUS) - status: FEATURE_FLAG_STATUS; + public status: FEATURE_FLAG_STATUS; @IsNotEmpty() @IsArray() @@ -35,13 +35,15 @@ export class FeatureFlagValidation { @IsDefined() @IsEnum(FILTER_MODE) - filterMode: FILTER_MODE; + public filterMode: FILTER_MODE; @IsNotEmpty() @IsArray() @IsString({ each: true }) public tags: string[]; +} +export class FeatureFlagValidation extends FeatureFlagCoreValidation { @IsOptional() @IsArray() @ValidateNested({ each: true }) From e19b162d751178efe0e0d83ba2bfe02bad2318e1 Mon Sep 17 00:00:00 2001 From: RidhamShah Date: Fri, 6 Sep 2024 11:40:00 +0530 Subject: [PATCH 28/28] Solve error and merge conflicts for import --- .../src/api/controllers/FeatureFlagController.ts | 3 ++- .../Upgrade/src/api/services/FeatureFlagService.ts | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts index 1d96ffe3d2..6f7c3b8438 100644 --- a/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts +++ b/backend/packages/Upgrade/src/api/controllers/FeatureFlagController.ts @@ -791,9 +791,10 @@ export class FeatureFlagsController { @Post('/import') public async importFeatureFlags( @Body({ validate: true }) featureFlags: FeatureFlagImportValidation, + @CurrentUser() currentUser: User, @Req() request: AppRequest ): Promise { - return await this.featureFlagService.importFeatureFlags(featureFlags.files, request.logger); + return await this.featureFlagService.importFeatureFlags(featureFlags.files, currentUser, request.logger); } /** * @swagger diff --git a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts index d3ff3c737a..0a753129cc 100644 --- a/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts +++ b/backend/packages/Upgrade/src/api/services/FeatureFlagService.ts @@ -7,7 +7,7 @@ import { FeatureFlagSegmentExclusion } from '../models/FeatureFlagSegmentExclusi import { FeatureFlagRepository } from '../repositories/FeatureFlagRepository'; import { FeatureFlagSegmentInclusionRepository } from '../repositories/FeatureFlagSegmentInclusionRepository'; import { FeatureFlagSegmentExclusionRepository } from '../repositories/FeatureFlagSegmentExclusionRepository'; -import { EntityManager, getConnection, In, DataSource } from 'typeorm'; +import { EntityManager, In, DataSource } from 'typeorm'; import { InjectDataSource, InjectRepository } from '../../typeorm-typedi-extensions'; import { v4 as uuid } from 'uuid'; import { @@ -331,7 +331,7 @@ export class FeatureFlagService { return await executeTransaction(entityManager); } else { // Create a new transaction if no entity manager is provided - return await getConnection().transaction(async (manager) => { + return await this.dataSource.transaction(async (manager) => { return await executeTransaction(manager); }); } @@ -505,7 +505,7 @@ export class FeatureFlagService { return await executeTransaction(transactionalEntityManager); } else { // Create a new transaction if no entity manager is provided - return await getConnection().transaction(async (manager) => { + return await this.dataSource.transaction(async (manager) => { return await executeTransaction(manager); }); } @@ -708,10 +708,10 @@ export class FeatureFlagService { public async importFeatureFlags( featureFlagFiles: IFeatureFlagFile[], + currentUser: User, logger: UpgradeLogger ): Promise { logger.info({ message: 'Import feature flags' }); - const currentUser: User; // TODO: Get the current user from the request const validatedFlags = await this.validateImportFeatureFlags(featureFlagFiles, logger); const fileStatusArray = featureFlagFiles.map((file) => { @@ -734,7 +734,7 @@ export class FeatureFlagService { const createdFlags = []; for (const featureFlag of validFiles) { - const createdFlag = await getConnection().transaction(async (transactionalEntityManager) => { + const createdFlag = await this.dataSource.transaction(async (transactionalEntityManager) => { const newFlag = await this.addFeatureFlagInDB( this.featureFlagValidatorToFlag(featureFlag), currentUser, @@ -822,7 +822,7 @@ export class FeatureFlagService { } }) .filter((key) => key !== null); - const existingFeatureFlags = await this.featureFlagRepository.find({ key: In(featureFlagsIds) }); + const existingFeatureFlags = await this.featureFlagRepository.findBy({ key: In(featureFlagsIds) }); const validationErrors = await Promise.allSettled( featureFlagFiles.map(async (featureFlagFile) => {