From 7b7a2a706cc751f2842aea64deecfe6be2a9020b Mon Sep 17 00:00:00 2001 From: Jaanus Sellin Date: Thu, 12 Oct 2023 13:43:43 +0300 Subject: [PATCH] fix: enable segment importing for oss (#5010) --- .../createExportImportService.ts | 10 +++++ .../export-import-permissions.e2e.test.ts | 37 +++++++++++++--- .../export-import-service.ts | 42 +++++++++++++++++++ .../export-import.e2e.test.ts | 5 +++ .../import-validation-messages.ts | 10 +++++ src/lib/openapi/spec/export-result-schema.ts | 2 +- 6 files changed, 99 insertions(+), 7 deletions(-) diff --git a/src/lib/features/export-import-toggles/createExportImportService.ts b/src/lib/features/export-import-toggles/createExportImportService.ts index 62e1ff4724..7d34a4b245 100644 --- a/src/lib/features/export-import-toggles/createExportImportService.ts +++ b/src/lib/features/export-import-toggles/createExportImportService.ts @@ -46,6 +46,10 @@ import { import { DbServiceFactory } from 'lib/db/transaction'; import { DependentFeaturesReadModel } from '../dependent-features/dependent-features-read-model'; import { FakeDependentFeaturesReadModel } from '../dependent-features/fake-dependent-features-read-model'; +import { + createFakeSegmentService, + createSegmentService, +} from '../segment/createSegmentService'; export const createFakeExportImportTogglesService = ( config: IUnleashConfig, @@ -106,6 +110,8 @@ export const createFakeExportImportTogglesService = ( ); const dependentFeaturesReadModel = new FakeDependentFeaturesReadModel(); + const segmentService = createFakeSegmentService(config); + const exportImportService = new ExportImportService( { importTogglesStore, @@ -126,6 +132,7 @@ export const createFakeExportImportTogglesService = ( contextService, strategyService, tagTypeService, + segmentService, }, dependentFeaturesReadModel, ); @@ -220,6 +227,8 @@ export const deferredExportImportTogglesService = ( ); const dependentFeaturesReadModel = new DependentFeaturesReadModel(db); + const segmentService = createSegmentService(db, config); + const exportImportService = new ExportImportService( { importTogglesStore, @@ -240,6 +249,7 @@ export const deferredExportImportTogglesService = ( contextService, strategyService, tagTypeService, + segmentService, }, dependentFeaturesReadModel, ); diff --git a/src/lib/features/export-import-toggles/export-import-permissions.e2e.test.ts b/src/lib/features/export-import-toggles/export-import-permissions.e2e.test.ts index bd868722f1..a5e8327c74 100644 --- a/src/lib/features/export-import-toggles/export-import-permissions.e2e.test.ts +++ b/src/lib/features/export-import-toggles/export-import-permissions.e2e.test.ts @@ -170,9 +170,18 @@ const tags = [ ]; const tagTypes = [ - { name: 'bestt', description: 'test' }, - { name: 'special_tag', description: 'this is my special tag' }, - { name: 'special_tag', description: 'this is my special tag' }, // deliberate duplicate + { + name: 'bestt', + description: 'test', + }, + { + name: 'special_tag', + description: 'this is my special tag', + }, + { + name: 'special_tag', + description: 'this is my special tag', + }, // deliberate duplicate ]; const importPayload: ImportTogglesSchema = { @@ -199,13 +208,19 @@ const importPayload: ImportTogglesSchema = { const createUserEditorAccess = async (name, email) => { const { userStore } = stores; - const user = await userStore.insert({ name, email }); + const user = await userStore.insert({ + name, + email, + }); return user; }; const createUserAdminAccess = async (name, email) => { const { userStore } = stores; - const user = await userStore.insert({ name, email }); + const user = await userStore.insert({ + name, + email, + }); await accessService.addUserToRole(user.id, adminRole.id, 'default'); return user; }; @@ -301,7 +316,12 @@ test('validate import data', async () => { data: { ...importPayload.data, featureStrategies: [{ name: 'customStrategy' }], - segments: [{ id: 1, name: 'customSegment' }], + segments: [ + { + id: 1, + name: 'customSegment', + }, + ], contextFields: [ { ...contextField, @@ -328,6 +348,11 @@ test('validate import data', async () => { 'We detected the following context fields that do not have matching legal values with the imported ones:', affectedItems: [contextField.name], }, + { + affectedItems: ['customSegment'], + message: + 'We detected the following segments in the import file that need to be created first:', + }, ], warnings: [ { diff --git a/src/lib/features/export-import-toggles/export-import-service.ts b/src/lib/features/export-import-toggles/export-import-service.ts index 8b8435efe3..2594701785 100644 --- a/src/lib/features/export-import-toggles/export-import-service.ts +++ b/src/lib/features/export-import-toggles/export-import-service.ts @@ -51,6 +51,7 @@ import { findDuplicates } from '../../util/findDuplicates'; import { FeatureNameCheckResultWithFeaturePattern } from '../feature-toggle/feature-toggle-service'; import { IDependentFeaturesReadModel } from '../dependent-features/dependent-features-read-model-type'; import groupBy from 'lodash.groupby'; +import { ISegmentService } from '../../segments/segment-service-interface'; export type IImportService = { validate( @@ -103,6 +104,8 @@ export default class ExportImportService private tagTypeService: TagTypeService; + private segmentService: ISegmentService; + private featureTagService: FeatureTagService; private importPermissionsService: ImportPermissionsService; @@ -133,6 +136,7 @@ export default class ExportImportService eventService, tagTypeService, featureTagService, + segmentService, }: Pick< IUnleashServices, | 'featureToggleService' @@ -142,6 +146,7 @@ export default class ExportImportService | 'eventService' | 'tagTypeService' | 'featureTagService' + | 'segmentService' >, dependentFeaturesReadModel: IDependentFeaturesReadModel, ) { @@ -158,6 +163,7 @@ export default class ExportImportService this.strategyService = strategyService; this.contextService = contextService; this.accessService = accessService; + this.segmentService = segmentService; this.eventService = eventService; this.tagTypeService = tagTypeService; this.featureTagService = featureTagService; @@ -187,6 +193,7 @@ export default class ExportImportService duplicateFeatures, featureNameCheckResult, featureLimitResult, + unsupportedSegments, ] = await Promise.all([ this.getUnsupportedStrategies(dto), this.getUsedCustomStrategies(dto), @@ -202,6 +209,7 @@ export default class ExportImportService this.getDuplicateFeatures(dto), this.getInvalidFeatureNames(dto), this.getFeatureLimit(dto), + this.getUnsupportedSegments(dto), ]); const errors = ImportValidationMessages.compileErrors({ @@ -212,6 +220,7 @@ export default class ExportImportService duplicateFeatures, featureNameCheckResult, featureLimitResult, + segments: unsupportedSegments, }); const warnings = ImportValidationMessages.compileWarnings({ archivedFeatures, @@ -240,6 +249,7 @@ export default class ExportImportService this.verifyContextFields(dto), this.importPermissionsService.verifyPermissions(dto, user, mode), this.verifyFeatures(dto), + this.verifySegments(dto), ]); } @@ -426,6 +436,38 @@ export default class ExportImportService } } + private async getUnsupportedSegments( + dto: ImportTogglesSchema, + ): Promise { + const supportedSegments = await this.segmentService.getAll(); + const targetProject = dto.project; + return dto.data.segments + ? dto.data.segments + .filter( + (importingSegment) => + !supportedSegments.find( + (existingSegment) => + importingSegment.name === + existingSegment.name && + (!existingSegment.project || + existingSegment.project === + targetProject), + ), + ) + + .map((it) => it.name) + : []; + } + + private async verifySegments(dto: ImportTogglesSchema) { + const unsupportedSegments = await this.getUnsupportedSegments(dto); + if (unsupportedSegments.length > 0) { + throw new BadDataError( + `Unsupported segments: ${unsupportedSegments.join(', ')}`, + ); + } + } + private async verifyContextFields(dto: ImportTogglesSchema) { const unsupportedContextFields = await this.getUnsupportedContextFields( dto, diff --git a/src/lib/features/export-import-toggles/export-import.e2e.test.ts b/src/lib/features/export-import-toggles/export-import.e2e.test.ts index 335d4aedaa..7c8824a86e 100644 --- a/src/lib/features/export-import-toggles/export-import.e2e.test.ts +++ b/src/lib/features/export-import-toggles/export-import.e2e.test.ts @@ -1018,6 +1018,11 @@ test('validate import data', async () => { 'We detected you want to create 2 new features to a project that already has 0 existing features, exceeding the maximum limit of 1.', affectedItems: [], }, + { + message: + 'We detected the following segments in the import file that need to be created first:', + affectedItems: ['customSegment'], + }, ], warnings: [ { diff --git a/src/lib/features/export-import-toggles/import-validation-messages.ts b/src/lib/features/export-import-toggles/import-validation-messages.ts index 07bc206551..450a64f14d 100644 --- a/src/lib/features/export-import-toggles/import-validation-messages.ts +++ b/src/lib/features/export-import-toggles/import-validation-messages.ts @@ -14,6 +14,7 @@ export interface IErrorsParams { duplicateFeatures: string[]; featureNameCheckResult: FeatureNameCheckResultWithFeaturePattern; featureLimitResult: ProjectFeaturesLimit; + segments: string[]; } export interface IWarningParams { @@ -46,6 +47,7 @@ export class ImportValidationMessages { duplicateFeatures, featureNameCheckResult, featureLimitResult, + segments, }: IErrorsParams): ImportTogglesValidateItemSchema[] { const errors: ImportTogglesValidateItemSchema[] = []; @@ -106,6 +108,14 @@ export class ImportValidationMessages { }); } + if (segments.length > 0) { + errors.push({ + message: + 'We detected the following segments in the import file that need to be created first:', + affectedItems: segments, + }); + } + return errors; } diff --git a/src/lib/openapi/spec/export-result-schema.ts b/src/lib/openapi/spec/export-result-schema.ts index 377f1e6851..75d3d2c5b4 100644 --- a/src/lib/openapi/spec/export-result-schema.ts +++ b/src/lib/openapi/spec/export-result-schema.ts @@ -142,7 +142,7 @@ export const exportResultSchema = { items: { type: 'object', additionalProperties: false, - required: ['id'], + required: ['id', 'name'], properties: { id: { type: 'number',