fix(core): Prevent Facet code conflicts

Relates to #715

packages/core/e2e/facet.e2e-spec.ts

@@ -4,7 +4,7 @@ import gql from 'graphql-tag';
 import path from 'path';
 
 import { initialData } from '../../../e2e-common/e2e-initial-data';
-import { TEST_SETUP_TIMEOUT_MS, testConfig } from '../../../e2e-common/test-config';
+import { testConfig, TEST_SETUP_TIMEOUT_MS } from '../../../e2e-common/test-config';
 
 import { FACET_VALUE_FRAGMENT, FACET_WITH_VALUES_FRAGMENT } from './graphql/fragments';
 import {
@@ -363,6 +363,45 @@ describe('Facet resolver', () => {
             expect(result.deleteFacet.result).toBe(DeletionResult.DELETED);
         });
     });
+
+    // https://github.com/vendure-ecommerce/vendure/issues/715
+    describe('code conflicts', () => {
+        function createFacetWithCode(code: string) {
+            return adminClient.query<CreateFacet.Mutation, CreateFacet.Variables>(CREATE_FACET, {
+                input: {
+                    isPrivate: false,
+                    code,
+                    translations: [{ languageCode: LanguageCode.en, name: `Test Facet (${code})` }],
+                    values: [],
+                },
+            });
+        }
+
+        it('createFacet with conflicting slug gets renamed', async () => {
+            const { createFacet: result1 } = await createFacetWithCode('test');
+            expect(result1.code).toBe('test');
+
+            const { createFacet: result2 } = await createFacetWithCode('test');
+            expect(result2.code).toBe('test-2');
+        });
+
+        it('updateFacet with conflicting slug gets renamed', async () => {
+            const { createFacet } = await createFacetWithCode('foo');
+            expect(createFacet.code).toBe('foo');
+
+            const { updateFacet } = await adminClient.query<UpdateFacet.Mutation, UpdateFacet.Variables>(
+                UPDATE_FACET,
+                {
+                    input: {
+                        id: createFacet.id,
+                        code: 'test-2',
+                    },
+                },
+            );
+
+            expect(updateFacet.code).toBe('test-3');
+        });
+    });
 });
 
 export const GET_FACET_WITH_VALUES = gql`

packages/core/src/service/services/facet.service.ts

@@ -6,12 +6,13 @@ import {
     LanguageCode,
     UpdateFacetInput,
 } from '@vendure/common/lib/generated-types';
+import { normalizeString } from '@vendure/common/lib/normalize-string';
 import { ID, PaginatedList } from '@vendure/common/lib/shared-types';
 
 import { RequestContext } from '../../api/common/request-context';
 import { ListQueryOptions } from '../../common/types/common-types';
 import { Translated } from '../../common/types/locale-types';
-import { assertFound } from '../../common/utils';
+import { assertFound, idsAreEqual } from '../../common/utils';
 import { ConfigService } from '../../config/config.service';
 import { FacetTranslation } from '../../entity/facet/facet-translation.entity';
 import { Facet } from '../../entity/facet/facet.entity';
@@ -93,6 +94,9 @@ export class FacetService {
             input,
             entityType: Facet,
             translationType: FacetTranslation,
+            beforeSave: async f => {
+                f.code = await this.ensureUniqueCode(ctx, f.code);
+            },
         });
         return assertFound(this.findOne(ctx, facet.id));
     }
@@ -103,6 +107,9 @@ export class FacetService {
             input,
             entityType: Facet,
             translationType: FacetTranslation,
+            beforeSave: async f => {
+                f.code = await this.ensureUniqueCode(ctx, f.code, f.id);
+            },
         });
         return assertFound(this.findOne(ctx, facet.id));
     }
@@ -143,4 +150,30 @@ export class FacetService {
             message,
         };
     }
+
+    /**
+     * Checks to ensure the Facet code is unique. If there is a conflict, then the code is suffixed
+     * with an incrementing integer.
+     */
+    private async ensureUniqueCode(ctx: RequestContext, code: string, id?: ID) {
+        let candidate = code;
+        let suffix = 1;
+        let match: Facet | undefined;
+        const alreadySuffixed = /-\d+$/;
+        do {
+            match = await this.connection.getRepository(ctx, Facet).findOne({ where: { code: candidate } });
+
+            const conflict = !!match && ((id != null && !idsAreEqual(match.id, id)) || id == null);
+            if (conflict) {
+                suffix++;
+                if (alreadySuffixed.test(candidate)) {
+                    candidate = candidate.replace(alreadySuffixed, `-${suffix}`);
+                } else {
+                    candidate = `${candidate}-${suffix}`;
+                }
+            }
+        } while (match);
+
+        return candidate;
+    }
 }