fix: import duplicate features (#4550)

frontend/src/hooks/api/actions/useApi/useApi.ts

@@ -108,6 +108,10 @@ const useAPI = ({
                 const response = await res.json();
                 if (response?.details?.length > 0 && propagateErrors) {
                     const error = response.details[0];
+                    setErrors(prev => ({
+                        ...prev,
+                        unknown: error,
+                    }));
                     if (propagateErrors) {
                         throw new Error(error.message || error.msg);
                     }

frontend/src/hooks/api/actions/useImportApi/useImportApi.ts

@@ -29,6 +29,11 @@ export const useImportApi = () => {
             });
             return res;
         } catch (e) {
+            trackEvent('export_import', {
+                props: {
+                    eventType: `features import failed`,
+                },
+            });
             throw e;
         }
     };

src/lib/db/feature-toggle-store.ts

@@ -8,6 +8,7 @@ import { FeatureToggle, FeatureToggleDTO, IVariant } from '../types/model';
 import { IFeatureToggleStore } from '../types/stores/feature-toggle-store';
 import { Db } from './db';
 import { LastSeenInput } from '../services/client-metrics/last-seen-service';
+import { NameExistsError } from '../error';
 
 export type EnvironmentFeatureNames = { [key: string]: string[] };
 
@@ -289,8 +290,16 @@ export default class FeatureToggleStore implements IFeatureToggleStore {
             return this.rowToFeature(row[0]);
         } catch (err) {
             this.logger.error('Could not insert feature, error: ', err);
+            if (
+                typeof err.detail === 'string' &&
+                err.detail.includes('already exists')
+            ) {
+                throw new NameExistsError(
+                    `Feature ${data.name} already exists`,
+                );
+            }
+            throw err;
         }
-        return undefined;
     }
 
     async update(

src/lib/features/export-import-toggles/export-import-service.ts

@@ -205,7 +205,7 @@ export default class ExportImportService {
             ),
             this.verifyFeatures(dto),
         ]);
-        await this.createToggles(cleanedDto, user);
+        await this.createOrUpdateToggles(cleanedDto, user);
         await this.importToggleVariants(dto, user);
         await this.importTagTypes(cleanedDto, user);
         await this.importTags(cleanedDto, user);
@@ -348,10 +348,21 @@ export default class ExportImportService {
         );
     }
 
-    private async createToggles(dto: ImportTogglesSchema, user: User) {
+    private async createOrUpdateToggles(dto: ImportTogglesSchema, user: User) {
+        const existingFeatures = await this.getExistingProjectFeatures(dto);
+        const username = extractUsernameFromUser(user);
         await Promise.all(
-            dto.data.features.map((feature) =>
+            dto.data.features.map((feature) => {
-                this.featureToggleService
+                if (existingFeatures.includes(feature.name)) {
+                    const { archivedAt, createdAt, ...rest } = feature;
+                    return this.featureToggleService.updateFeatureToggle(
+                        dto.project,
+                        rest as FeatureToggleDTO,
+                        username,
+                        feature.name,
+                    );
+                }
+                return this.featureToggleService
                     .validateName(feature.name)
                     .then(() => {
                         const { archivedAt, createdAt, ...rest } = feature;
@@ -360,9 +371,8 @@ export default class ExportImportService {
                             rest as FeatureToggleDTO,
                             extractUsernameFromUser(user),
                         );
-                    })
+                    });
-                    .catch(() => {}),
+            }),
-            ),
         );
     }
 
@@ -533,12 +543,10 @@ export default class ExportImportService {
     }
 
     private async getExistingProjectFeatures(dto: ImportTogglesSchema) {
-        const existingProjectsFeatures =
+        return this.importTogglesStore.getFeaturesInProject(
-            await this.importTogglesStore.getFeaturesInProject(
             dto.data.features.map((feature) => feature.name),
             dto.project,
         );
-        return existingProjectsFeatures;
     }
 
     private async getNewTagTypes(dto: ImportTogglesSchema) {

src/lib/features/export-import-toggles/export-import.e2e.test.ts

@@ -547,6 +547,7 @@ const variants: VariantsSchema = [
 const exportedFeature: ImportTogglesSchema['data']['features'][0] = {
     project: 'old_project',
     name: 'first_feature',
+    type: 'release',
 };
 const anotherExportedFeature: ImportTogglesSchema['data']['features'][0] = {
     project: 'old_project',
@@ -723,15 +724,22 @@ test('import multiple features with same tag', async () => {
     });
 });
 
-test('importing same JSON should work multiple times in a row', async () => {
+test('can update toggles on subsequent import', async () => {
     await createProjects();
     await app.importToggles(defaultImportPayload);
-    await app.importToggles(defaultImportPayload);
+    await app.importToggles({
+        ...defaultImportPayload,
+        data: {
+            ...defaultImportPayload.data,
+            features: [{ ...exportedFeature, type: 'operational' }],
+        },
+    });
 
     const { body: importedFeature } = await getFeature(defaultFeature);
     expect(importedFeature).toMatchObject({
         name: 'first_feature',
         project: DEFAULT_PROJECT,
+        type: 'operational',
         variants,
     });
 
@@ -803,6 +811,26 @@ test('reject import with unsupported strategies', async () => {
     expect(body.details[0].description).toMatch(/\bcustomStrategy\b/);
 });
 
+test('reject import with duplicate features', async () => {
+    await createProjects();
+    const importPayloadWithContextFields: ImportTogglesSchema = {
+        ...defaultImportPayload,
+        data: {
+            ...defaultImportPayload.data,
+            features: [exportedFeature, exportedFeature],
+        },
+    };
+
+    const { body } = await app.importToggles(
+        importPayloadWithContextFields,
+        409,
+    );
+
+    expect(body.details[0].description).toBe(
+        'Feature first_feature already exists',
+    );
+});
+
 test('validate import data', async () => {
     await createProjects();
     const contextField: IContextFieldDto = {

src/lib/features/export-import-toggles/import-context-validation.ts

@@ -10,7 +10,9 @@ export const isValidField = (
     if (!matchingExistingField) {
         return true;
     }
-    return importedField.legalValues.every((value) =>
+    return (importedField.legalValues || []).every((value) =>
-        matchingExistingField.legalValues.find((v) => v.value === value.value),
+        (matchingExistingField.legalValues || []).find(
+            (v) => v.value === value.value,
+        ),
     );
 };

src/lib/types/stores/context-field-store.ts

@@ -2,11 +2,11 @@ import { Store } from './store';
 
 export interface IContextFieldDto {
     name: string;
-    description?: string;
+    description?: string | null;
     stickiness?: boolean;
     sortOrder?: number;
-    usedInProjects?: number;
+    usedInProjects?: number | null;
-    usedInFeatures?: number;
+    usedInFeatures?: number | null;
     legalValues?: ILegalValue[];
 }