feat(core): Enforce unique slugs for Products

Closes #103

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

@@ -121,7 +121,7 @@ describe('Product resolver', () => {
             const { product } = await client.query<
                 GetProductWithVariants.Query,
                 GetProductWithVariants.Variables
-            >(GET_PRODUCT_WITH_VARIANTS, {
+                >(GET_PRODUCT_WITH_VARIANTS, {
                 languageCode: LanguageCode.en,
                 id: 'T_2',
             });
@@ -260,14 +260,71 @@ describe('Product resolver', () => {
                             {
                                 languageCode: LanguageCode.en,
                                 name: 'en Mashed Potato',
-                                slug: 'A (very) nice potato!!1',
+                                slug: 'A (very) nice potato!!',
                                 description: 'A blob of mashed potato',
                             },
                         ],
                     },
                 },
             );
-            expect(result.updateProduct.slug).toBe('a-very-nice-potato1');
+            expect(result.updateProduct.slug).toBe('a-very-nice-potato');
+        });
+
+        it('create with duplicate slug is renamed to be unique', async () => {
+            const result = await client.query<CreateProduct.Mutation, CreateProduct.Variables>(
+                CREATE_PRODUCT,
+                {
+                    input: {
+                        translations: [
+                            {
+                                languageCode: LanguageCode.en,
+                                name: 'Another baked potato',
+                                slug: 'a-very-nice-potato',
+                                description: 'Another baked potato but a bit different',
+                            },
+                        ],
+                    },
+                },
+            );
+            expect(result.createProduct.slug).toBe('a-very-nice-potato-2');
+        });
+
+        it('update with duplicate slug is renamed to be unique', async () => {
+            const result = await client.query<UpdateProduct.Mutation, UpdateProduct.Variables>(
+                UPDATE_PRODUCT,
+                {
+                    input: {
+                        id: newProduct.id,
+                        translations: [
+                            {
+                                languageCode: LanguageCode.en,
+                                name: 'Yet another baked potato',
+                                slug: 'a-very-nice-potato-2',
+                                description: 'Possibly the final baked potato',
+                            },
+                        ],
+                    },
+                },
+            );
+            expect(result.updateProduct.slug).toBe('a-very-nice-potato-3');
+        });
+
+        it('slug duplicate check does not include self', async () => {
+            const result = await client.query<UpdateProduct.Mutation, UpdateProduct.Variables>(
+                UPDATE_PRODUCT,
+                {
+                    input: {
+                        id: newProduct.id,
+                        translations: [
+                            {
+                                languageCode: LanguageCode.en,
+                                slug: 'a-very-nice-potato-3',
+                            },
+                        ],
+                    },
+                },
+            );
+            expect(result.updateProduct.slug).toBe('a-very-nice-potato-3');
         });
 
         it('updateProduct accepts partial input', async () => {
@@ -287,7 +344,7 @@ describe('Product resolver', () => {
             );
             expect(result.updateProduct.translations.length).toBe(2);
             expect(result.updateProduct.translations[0].name).toBe('en Very Mashed Potato');
-            expect(result.updateProduct.translations[0].description).toBe('A blob of mashed potato');
+            expect(result.updateProduct.translations[0].description).toBe('Possibly the final baked potato');
             expect(result.updateProduct.translations[1].name).toBe('de Mashed Potato');
         });
 
@@ -316,7 +373,7 @@ describe('Product resolver', () => {
             const productResult = await client.query<
                 GetProductWithVariants.Query,
                 GetProductWithVariants.Variables
-            >(GET_PRODUCT_WITH_VARIANTS, {
+                >(GET_PRODUCT_WITH_VARIANTS, {
                 id: newProduct.id,
                 languageCode: LanguageCode.en,
             });
@@ -378,7 +435,7 @@ describe('Product resolver', () => {
             const result = await client.query<
                 AddOptionGroupToProduct.Mutation,
                 AddOptionGroupToProduct.Variables
-            >(ADD_OPTION_GROUP_TO_PRODUCT, {
+                >(ADD_OPTION_GROUP_TO_PRODUCT, {
                 optionGroupId: 'T_2',
                 productId: newProduct.id,
             });
@@ -420,7 +477,7 @@ describe('Product resolver', () => {
             const result = await client.query<
                 RemoveOptionGroupFromProduct.Mutation,
                 RemoveOptionGroupFromProduct.Variables
-            >(REMOVE_OPTION_GROUP_FROM_PRODUCT, {
+                >(REMOVE_OPTION_GROUP_FROM_PRODUCT, {
                 optionGroupId: 'T_1',
                 productId: 'T_1',
             });
@@ -435,7 +492,7 @@ describe('Product resolver', () => {
                     client.query<
                         RemoveOptionGroupFromProduct.Mutation,
                         RemoveOptionGroupFromProduct.Variables
-                    >(REMOVE_OPTION_GROUP_FROM_PRODUCT, {
+                        >(REMOVE_OPTION_GROUP_FROM_PRODUCT, {
                         optionGroupId: '1',
                         productId: '999',
                     }),
@@ -479,7 +536,7 @@ describe('Product resolver', () => {
                 const result = await client.query<
                     GenerateProductVariants.Mutation,
                     GenerateProductVariants.Variables
-                >(GENERATE_PRODUCT_VARIANTS, {
+                    >(GENERATE_PRODUCT_VARIANTS, {
                     productId: newProduct.id,
                     defaultPrice: 123,
                     defaultSku: 'ABC',
@@ -495,7 +552,7 @@ describe('Product resolver', () => {
                 const result = await client.query<
                     UpdateProductVariants.Mutation,
                     UpdateProductVariants.Variables
-                >(UPDATE_PRODUCT_VARIANTS, {
+                    >(UPDATE_PRODUCT_VARIANTS, {
                     input: [
                         {
                             id: firstVariant.id,
@@ -519,7 +576,7 @@ describe('Product resolver', () => {
                 const result = await client.query<
                     UpdateProductVariants.Mutation,
                     UpdateProductVariants.Variables
-                >(UPDATE_PRODUCT_VARIANTS, {
+                    >(UPDATE_PRODUCT_VARIANTS, {
                     input: [
                         {
                             id: firstVariant.id,
@@ -542,7 +599,7 @@ describe('Product resolver', () => {
                 const result = await client.query<
                     UpdateProductVariants.Mutation,
                     UpdateProductVariants.Variables
-                >(UPDATE_PRODUCT_VARIANTS, {
+                    >(UPDATE_PRODUCT_VARIANTS, {
                     input: [
                         {
                             id: firstVariant.id,
@@ -565,7 +622,7 @@ describe('Product resolver', () => {
                 const result = await client.query<
                     UpdateProductVariants.Mutation,
                     UpdateProductVariants.Variables
-                >(UPDATE_PRODUCT_VARIANTS, {
+                    >(UPDATE_PRODUCT_VARIANTS, {
                     input: [
                         {
                             id: firstVariant.id,
@@ -675,7 +732,7 @@ describe('Product resolver', () => {
                     client.query<
                         RemoveOptionGroupFromProduct.Mutation,
                         RemoveOptionGroupFromProduct.Variables
-                    >(REMOVE_OPTION_GROUP_FROM_PRODUCT, {
+                        >(REMOVE_OPTION_GROUP_FROM_PRODUCT, {
                         optionGroupId: 'T_1',
                         productId: productToDelete.id,
                     }),

packages/core/src/entity/product/product-translation.entity.ts

@@ -1,6 +1,6 @@
 import { LanguageCode } from '@vendure/common/lib/generated-types';
 import { DeepPartial, HasCustomFields } from '@vendure/common/lib/shared-types';
-import { Column, Entity, ManyToOne } from 'typeorm';
+import { Column, Entity, Index, ManyToOne } from 'typeorm';
 
 import { Translation } from '../../common/types/locale-types';
 import { VendureEntity } from '../base/base.entity';
@@ -9,6 +9,7 @@ import { CustomProductFieldsTranslation } from '../custom-entity-fields';
 import { Product } from './product.entity';
 
 @Entity()
+@Index(['languageCode', 'slug'], { unique: true })
 export class ProductTranslation extends VendureEntity implements Translation<Product>, HasCustomFields {
     constructor(input?: DeepPartial<Translation<Product>>) {
         super(input);

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

@@ -3,7 +3,7 @@ import { InjectConnection } from '@nestjs/typeorm';
 import {
     CreateProductInput,
     DeletionResponse,
-    DeletionResult,
+    DeletionResult, LanguageCode,
     UpdateProductInput,
 } from '@vendure/common/lib/generated-types';
 import { normalizeString } from '@vendure/common/lib/normalize-string';
@@ -100,7 +100,7 @@ export class ProductService {
     }
 
     async create(ctx: RequestContext, input: CreateProductInput): Promise<Translated<Product>> {
-        this.normalizeSlugs(input);
+        await this.validateSlugs(input);
         const product = await this.translatableSaver.create({
             input,
             entityType: Product,
@@ -119,7 +119,7 @@ export class ProductService {
 
     async update(ctx: RequestContext, input: UpdateProductInput): Promise<Translated<Product>> {
         await getEntityOrThrow(this.connection, Product, input.id);
-        this.normalizeSlugs(input);
+        await this.validateSlugs(input);
         const product = await this.translatableSaver.update({
             input,
             entityType: Product,
@@ -187,13 +187,36 @@ export class ProductService {
         return product;
     }
 
-    private normalizeSlugs<T extends CreateProductInput | UpdateProductInput>(input: T): T {
+    /**
+     * Normalizes the slug to be URL-safe, and ensures it is unique for the given languageCode.
+     */
+    private async validateSlugs<T extends CreateProductInput | UpdateProductInput>(input: T): Promise<T> {
         if (input.translations) {
-            input.translations.forEach(t => {
+            for (const t of input.translations) {
                 if (t.slug) {
                     t.slug = normalizeString(t.slug, '-');
+                    let match: ProductTranslation | undefined;
+                    let suffix = 1;
+                    const alreadySuffixed = /-\d+$/;
+                    do {
+                        const qb = this.connection.getRepository(ProductTranslation).createQueryBuilder('translation')
+                            .where(`translation.slug = :slug`, { slug: t.slug })
+                            .andWhere(`translation.languageCode = :languageCode`, { languageCode: t.languageCode });
+                        if ((input as UpdateProductInput).id) {
+                            qb.andWhere(`translation.base != :id`, { id: (input as UpdateProductInput).id });
+                        }
+                        match = await qb.getOne();
+                        if (match) {
+                            suffix++;
+                            if (alreadySuffixed.test(t.slug)) {
+                                t.slug = t.slug.replace(alreadySuffixed, `-${suffix}`);
+                            } else {
+                                t.slug = `${t.slug}-${suffix}`;
+                            }
+                        }
+                    } while (match);
                 }
-            });
+            }
         }
         return input;
     }