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 cd70e7886f..674f8bc26a 100644 --- a/src/lib/features/export-import-toggles/export-import-service.ts +++ b/src/lib/features/export-import-toggles/export-import-service.ts @@ -42,8 +42,9 @@ import { } from '../../services'; import { isValidField } from './import-context-validation'; import { IImportTogglesStore } from './import-toggles-store-type'; -import { ImportPermissionsService } from './import-permissions-service'; +import { ImportPermissionsService, Mode } from './import-permissions-service'; import { ImportValidationMessages } from './import-validation-messages'; +import { findDuplicates } from '../../util/findDuplicates'; export default class ExportImportService { private logger: Logger; @@ -144,6 +145,7 @@ export default class ExportImportService { async validate( dto: ImportTogglesSchema, user: User, + mode = 'regular' as Mode, ): Promise { const [ unsupportedStrategies, @@ -153,6 +155,7 @@ export default class ExportImportService { otherProjectFeatures, existingProjectFeatures, missingPermissions, + duplicateFeatures, ] = await Promise.all([ this.getUnsupportedStrategies(dto), this.getUsedCustomStrategies(dto), @@ -163,23 +166,23 @@ export default class ExportImportService { this.importPermissionsService.getMissingPermissions( dto, user, - 'regular', + mode, ), + this.getDuplicateFeatures(dto), ]); - const errors = ImportValidationMessages.compileErrors( - dto.project, - unsupportedStrategies, - unsupportedContextFields || [], - [], + const errors = ImportValidationMessages.compileErrors({ + projectName: dto.project, + strategies: unsupportedStrategies, + contextFields: unsupportedContextFields || [], otherProjectFeatures, - false, - ); - const warnings = ImportValidationMessages.compileWarnings( - usedCustomStrategies, + duplicateFeatures, + }); + const warnings = ImportValidationMessages.compileWarnings({ archivedFeatures, - existingProjectFeatures, - ); + existingFeatures: existingProjectFeatures, + usedCustomStrategies, + }); const permissions = ImportValidationMessages.compilePermissionErrors( missingPermissions, @@ -192,24 +195,36 @@ export default class ExportImportService { }; } + async importVerify( + dto: ImportTogglesSchema, + user: User, + mode = 'regular' as Mode, + ): Promise { + await Promise.all([ + this.verifyStrategies(dto), + this.verifyContextFields(dto), + this.importPermissionsService.verifyPermissions(dto, user, mode), + this.verifyFeatures(dto), + ]); + } + + async importToggleLevelInfo( + dto: ImportTogglesSchema, + user: User, + ): Promise { + await this.createOrUpdateToggles(dto, user); + await this.importToggleVariants(dto, user); + await this.importTagTypes(dto, user); + await this.importTags(dto, user); + await this.importContextFields(dto, user); + } + async import(dto: ImportTogglesSchema, user: User): Promise { const cleanedDto = await this.cleanData(dto); - await Promise.all([ - this.verifyStrategies(cleanedDto), - this.verifyContextFields(cleanedDto), - this.importPermissionsService.verifyPermissions( - dto, - user, - 'regular', - ), - this.verifyFeatures(dto), - ]); - await this.createOrUpdateToggles(cleanedDto, user); - await this.importToggleVariants(dto, user); - await this.importTagTypes(cleanedDto, user); - await this.importTags(cleanedDto, user); - await this.importContextFields(dto, user); + await this.importVerify(cleanedDto, user); + + await this.importToggleLevelInfo(cleanedDto, user); await this.importDefault(cleanedDto, user); await this.eventStore.store({ @@ -220,7 +235,7 @@ export default class ExportImportService { }); } - private async importDefault(dto: ImportTogglesSchema, user: User) { + async importDefault(dto: ImportTogglesSchema, user: User): Promise { await this.deleteStrategies(dto); await this.importStrategies(dto, user); await this.importToggleStatuses(dto, user); @@ -429,7 +444,9 @@ export default class ExportImportService { }; } - private async removeArchivedFeatures(dto: ImportTogglesSchema) { + async removeArchivedFeatures( + dto: ImportTogglesSchema, + ): Promise { const archivedFeatures = await this.getArchivedFeatures(dto); const featureTags = dto.data.featureTags?.filter( @@ -549,6 +566,10 @@ export default class ExportImportService { ); } + private getDuplicateFeatures(dto: ImportTogglesSchema) { + return findDuplicates(dto.data.features.map((feature) => feature.name)); + } + private async getNewTagTypes(dto: ImportTogglesSchema) { const existingTagTypes = (await this.tagTypeService.getAll()).map( (tagType) => tagType.name, 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 3f867bd195..2c812adfef 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 @@ -51,6 +51,8 @@ const defaultContext: ContextFieldSchema = { ], }; +const defaultFeatureName = 'first_feature'; + const createToggle = async ( toggle: FeatureToggleDTO, strategy: Omit = defaultStrategy, @@ -195,7 +197,7 @@ describe('import-export for project-specific segments', () => { }; await createToggle( { - name: 'first_feature', + name: defaultFeatureName, description: 'the #1 feature', }, strategy, @@ -205,7 +207,7 @@ describe('import-export for project-specific segments', () => { const { body } = await app.request .post('/api/admin/features-batch/export') .send({ - features: ['first_feature'], + features: [defaultFeatureName], environment: 'default', }) .set('Content-Type', 'application/json') @@ -215,7 +217,7 @@ describe('import-export for project-specific segments', () => { expect(body).toMatchObject({ features: [ { - name: 'first_feature', + name: defaultFeatureName, }, ], featureStrategies: [resultStrategy], @@ -223,7 +225,7 @@ describe('import-export for project-specific segments', () => { { enabled: false, environment: 'default', - featureName: 'first_feature', + featureName: defaultFeatureName, }, ], segments: [ @@ -254,7 +256,7 @@ test('exports features', async () => { }; await createToggle( { - name: 'first_feature', + name: defaultFeatureName, description: 'the #1 feature', }, strategy, @@ -269,7 +271,7 @@ test('exports features', async () => { const { body } = await app.request .post('/api/admin/features-batch/export') .send({ - features: ['first_feature'], + features: [defaultFeatureName], environment: 'default', }) .set('Content-Type', 'application/json') @@ -279,7 +281,7 @@ test('exports features', async () => { expect(body).toMatchObject({ features: [ { - name: 'first_feature', + name: defaultFeatureName, }, ], featureStrategies: [resultStrategy], @@ -287,7 +289,7 @@ test('exports features', async () => { { enabled: false, environment: 'default', - featureName: 'first_feature', + featureName: defaultFeatureName, }, ], segments: [ @@ -314,7 +316,7 @@ test('exports features by tag', async () => { }; await createToggle( { - name: 'first_feature', + name: defaultFeatureName, description: 'the #1 feature', }, strategy, @@ -341,7 +343,7 @@ test('exports features by tag', async () => { expect(body).toMatchObject({ features: [ { - name: 'first_feature', + name: defaultFeatureName, }, ], featureStrategies: [resultStrategy], @@ -349,7 +351,7 @@ test('exports features by tag', async () => { { enabled: false, environment: 'default', - featureName: 'first_feature', + featureName: defaultFeatureName, }, ], }); @@ -387,7 +389,7 @@ test('should export custom context fields from strategies and variants', async ( }; await createToggle( { - name: 'first_feature', + name: defaultFeatureName, description: 'the #1 feature', }, strategy, @@ -408,7 +410,7 @@ test('should export custom context fields from strategies and variants', async ( }; await createContext(variantStickinessContext); await createContext(variantOverridesContext); - await createVariants('first_feature', [ + await createVariants(defaultFeatureName, [ { name: 'irrelevant', weight: 1000, @@ -426,7 +428,7 @@ test('should export custom context fields from strategies and variants', async ( const { body } = await app.request .post('/api/admin/features-batch/export') .send({ - features: ['first_feature'], + features: [defaultFeatureName], environment: 'default', }) .set('Content-Type', 'application/json') @@ -436,7 +438,7 @@ test('should export custom context fields from strategies and variants', async ( expect(body).toMatchObject({ features: [ { - name: 'first_feature', + name: defaultFeatureName, }, ], featureStrategies: [resultStrategy], @@ -444,7 +446,7 @@ test('should export custom context fields from strategies and variants', async ( { enabled: false, environment: 'default', - featureName: 'first_feature', + featureName: defaultFeatureName, }, ], contextFields: [ @@ -457,7 +459,7 @@ test('should export custom context fields from strategies and variants', async ( }); test('should export tags', async () => { - const featureName = 'first_feature'; + const featureName = defaultFeatureName; await createProjects(); await createToggle( { @@ -471,7 +473,7 @@ test('should export tags', async () => { const { body } = await app.request .post('/api/admin/features-batch/export') .send({ - features: ['first_feature'], + features: [defaultFeatureName], environment: 'default', }) .set('Content-Type', 'application/json') @@ -481,7 +483,7 @@ test('should export tags', async () => { expect(body).toMatchObject({ features: [ { - name: 'first_feature', + name: defaultFeatureName, }, ], featureStrategies: [resultStrategy], @@ -489,7 +491,7 @@ test('should export tags', async () => { { enabled: false, environment: 'default', - featureName: 'first_feature', + featureName: defaultFeatureName, }, ], featureTags: [{ featureName, tagValue: 'tag1' }], @@ -499,7 +501,7 @@ test('should export tags', async () => { test('returns no features, when no feature was requested', async () => { await createProjects(); await createToggle({ - name: 'first_feature', + name: defaultFeatureName, description: 'the #1 feature', }); await createToggle({ @@ -518,8 +520,6 @@ test('returns no features, when no feature was requested', async () => { expect(body.features).toHaveLength(0); }); -const defaultFeature = 'first_feature'; - const variants: VariantsSchema = [ { name: 'variantA', @@ -546,7 +546,7 @@ const variants: VariantsSchema = [ ]; const exportedFeature: ImportTogglesSchema['data']['features'][0] = { project: 'old_project', - name: 'first_feature', + name: defaultFeatureName, type: 'release', }; const anotherExportedFeature: ImportTogglesSchema['data']['features'][0] = { @@ -564,7 +564,7 @@ const constraints: ImportTogglesSchema['data']['featureStrategies'][0]['constrai }, ]; const exportedStrategy: ImportTogglesSchema['data']['featureStrategies'][0] = { - featureName: defaultFeature, + featureName: defaultFeatureName, id: '798cb25a-2abd-47bd-8a95-40ec13472309', name: 'default', parameters: {}, @@ -573,17 +573,17 @@ const exportedStrategy: ImportTogglesSchema['data']['featureStrategies'][0] = { const tags = [ { - featureName: defaultFeature, + featureName: defaultFeatureName, tagType: 'simple', tagValue: 'tag1', }, { - featureName: defaultFeature, + featureName: defaultFeatureName, tagType: 'simple', tagValue: 'tag2', }, { - featureName: defaultFeature, + featureName: defaultFeatureName, tagType: 'special_tag', tagValue: 'feature_tagged', }, @@ -609,8 +609,8 @@ const defaultImportPayload: ImportTogglesSchema = { { enabled: true, environment: 'irrelevant', - featureName: defaultFeature, - name: defaultFeature, + featureName: defaultFeatureName, + name: defaultFeatureName, variants, }, ], @@ -675,23 +675,23 @@ test('import features to existing project and environment', async () => { await app.importToggles(defaultImportPayload); - const { body: importedFeature } = await getFeature(defaultFeature); + const { body: importedFeature } = await getFeature(defaultFeatureName); expect(importedFeature).toMatchObject({ - name: 'first_feature', + name: defaultFeatureName, project: DEFAULT_PROJECT, variants, }); const { body: importedFeatureEnvironment } = await getFeatureEnvironment( - defaultFeature, + defaultFeatureName, ); expect(importedFeatureEnvironment).toMatchObject({ - name: defaultFeature, + name: defaultFeatureName, environment: DEFAULT_ENV, enabled: true, strategies: [ { - featureName: defaultFeature, + featureName: defaultFeatureName, parameters: {}, constraints, sortOrder: 0, @@ -700,7 +700,7 @@ test('import features to existing project and environment', async () => { ], }); - const { body: importedTags } = await getTags(defaultFeature); + const { body: importedTags } = await getTags(defaultFeatureName); expect(importedTags).toMatchObject({ tags: resultTags, }); @@ -735,25 +735,25 @@ test('can update toggles on subsequent import', async () => { }, }); - const { body: importedFeature } = await getFeature(defaultFeature); + const { body: importedFeature } = await getFeature(defaultFeatureName); expect(importedFeature).toMatchObject({ - name: 'first_feature', + name: defaultFeatureName, project: DEFAULT_PROJECT, type: 'operational', variants, }); const { body: importedFeatureEnvironment } = await getFeatureEnvironment( - defaultFeature, + defaultFeatureName, ); expect(importedFeatureEnvironment).toMatchObject({ - name: defaultFeature, + name: defaultFeatureName, environment: DEFAULT_ENV, enabled: true, strategies: [ { - featureName: defaultFeature, + featureName: defaultFeatureName, parameters: {}, constraints, sortOrder: 0, @@ -843,14 +843,15 @@ test('validate import data', async () => { legalValues: [{ value: 'new_value' }], }; - await app.createFeature(defaultFeature); - await app.archiveFeature(defaultFeature); + await app.createFeature(defaultFeatureName); + await app.archiveFeature(defaultFeatureName); await app.createContextField(contextField); const importPayloadWithContextFields: ImportTogglesSchema = { ...defaultImportPayload, data: { ...defaultImportPayload.data, + features: [exportedFeature, exportedFeature], featureStrategies: [{ name: 'customStrategy' }], segments: [{ id: 1, name: 'customSegment' }], contextFields: [ @@ -877,12 +878,17 @@ 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], }, + { + message: + 'We detected the following features are duplicate in your import data:', + affectedItems: [defaultFeatureName], + }, ], warnings: [ { message: 'The following features will not be imported as they are currently archived. To import them, please unarchive them first:', - affectedItems: [defaultFeature], + affectedItems: [defaultFeatureName], }, ], permissions: [], @@ -913,7 +919,7 @@ test('should not import archived features tags', async () => { await createProjects(); await app.importToggles(defaultImportPayload); - await app.archiveFeature(defaultFeature); + await app.archiveFeature(defaultFeatureName); await app.importToggles({ ...defaultImportPayload, @@ -921,16 +927,16 @@ test('should not import archived features tags', async () => { ...defaultImportPayload.data, featureTags: [ { - featureName: defaultFeature, + featureName: defaultFeatureName, tagType: 'simple', tagValue: 'tag2', }, ], }, }); - await unArchiveFeature(defaultFeature); + await unArchiveFeature(defaultFeatureName); - const { body: importedTags } = await getTags(defaultFeature); + const { body: importedTags } = await getTags(defaultFeatureName); expect(importedTags).toMatchObject({ tags: resultTags, }); diff --git a/src/lib/features/export-import-toggles/import-permissions-service.ts b/src/lib/features/export-import-toggles/import-permissions-service.ts index 1bda360f8d..0f28fc3e20 100644 --- a/src/lib/features/export-import-toggles/import-permissions-service.ts +++ b/src/lib/features/export-import-toggles/import-permissions-service.ts @@ -14,7 +14,7 @@ import { } from '../../types'; import { PermissionError } from '../../error'; -type Mode = 'regular' | 'change_request'; +export type Mode = 'regular' | 'change_request'; export class ImportPermissionsService { private importTogglesStore: IImportTogglesStore; 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 376df05189..40308fd40f 100644 --- a/src/lib/features/export-import-toggles/import-validation-messages.ts +++ b/src/lib/features/export-import-toggles/import-validation-messages.ts @@ -4,6 +4,20 @@ import { } from '../../openapi'; import { IContextFieldDto } from '../../types/stores/context-field-store'; +export interface IErrorsParams { + projectName: string; + strategies: FeatureStrategySchema[]; + contextFields: IContextFieldDto[]; + otherProjectFeatures: string[]; + duplicateFeatures: string[]; +} + +export interface IWarningParams { + usedCustomStrategies: string[]; + archivedFeatures: string[]; + existingFeatures: string[]; +} + export class ImportValidationMessages { static compilePermissionErrors( missingPermissions: string[], @@ -20,14 +34,13 @@ export class ImportValidationMessages { return errors; } - static compileErrors( - projectName: string, - strategies: FeatureStrategySchema[], - contextFields: IContextFieldDto[], - segments: string[], - otherProjectFeatures: string[], - changeRequestExists: boolean, - ): ImportTogglesValidateItemSchema[] { + static compileErrors({ + projectName, + strategies, + contextFields, + otherProjectFeatures, + duplicateFeatures, + }: IErrorsParams): ImportTogglesValidateItemSchema[] { const errors: ImportTogglesValidateItemSchema[] = []; if (strategies.length > 0) { @@ -46,35 +59,28 @@ 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, - }); - } - if (changeRequestExists) { - errors.push({ - message: - 'Before importing any data, please resolve your pending change request in this project and environment as it is preventing you from importing at this time', - affectedItems: [], - }); - } if (otherProjectFeatures.length > 0) { errors.push({ message: `You cannot import a features that already exist in other projects. You already have the following features defined outside of project ${projectName}:`, affectedItems: otherProjectFeatures, }); } + if (duplicateFeatures.length > 0) { + errors.push({ + message: + 'We detected the following features are duplicate in your import data:', + affectedItems: duplicateFeatures, + }); + } return errors; } - static compileWarnings( - usedCustomStrategies: string[], - archivedFeatures: string[], - existingFeatures: string[], - ): ImportTogglesValidateItemSchema[] { + static compileWarnings({ + usedCustomStrategies, + existingFeatures, + archivedFeatures, + }: IWarningParams): ImportTogglesValidateItemSchema[] { const warnings: ImportTogglesValidateItemSchema[] = []; if (usedCustomStrategies.length > 0) { warnings.push({ diff --git a/src/lib/util/findDuplicates.test.ts b/src/lib/util/findDuplicates.test.ts new file mode 100644 index 0000000000..dc4093c908 --- /dev/null +++ b/src/lib/util/findDuplicates.test.ts @@ -0,0 +1,30 @@ +import { findDuplicates } from './findDuplicates'; + +test('should find single duplicates', () => { + expect(findDuplicates([1, 2, 3, 4, 1])).toEqual([1]); + expect(findDuplicates(['a', 'b', 'a', 'a'])).toEqual(['a']); +}); + +test('should return an empty array for unique elements', () => { + expect(findDuplicates(['a', 'b', 'c', 'd'])).toEqual([]); +}); + +test('should handle arrays with all identical elements', () => { + expect(findDuplicates([1, 1, 1, 1])).toEqual([1]); +}); + +test('should handle multiple duplicates', () => { + expect(findDuplicates([1, 2, 2, 1])).toEqual( + expect.arrayContaining([1, 2]), + ); +}); + +test('should handle an empty array', () => { + expect(findDuplicates([])).toEqual([]); +}); + +test('should handle arrays with boolean values', () => { + expect(findDuplicates([true, true, false, false, true])).toEqual( + expect.arrayContaining([true, false]), + ); +}); diff --git a/src/lib/util/findDuplicates.ts b/src/lib/util/findDuplicates.ts new file mode 100644 index 0000000000..a0c932fa5d --- /dev/null +++ b/src/lib/util/findDuplicates.ts @@ -0,0 +1,14 @@ +export const findDuplicates = (arr: T[]): T[] => { + const seen: Set = new Set(); + const duplicates: Set = new Set(); + + for (let item of arr) { + if (seen.has(item)) { + duplicates.add(item); + } else { + seen.add(item); + } + } + + return [...duplicates]; +};