fix(core): Fix facet value CollectionFilter

Fixes #158

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

@@ -131,7 +131,7 @@ describe('Collection resolver', () => {
             expect(electronicsCollection.parent!.name).toBe(ROOT_COLLECTION_NAME);
         });
 
-        it('creates a nested category', async () => {
+        it('creates a nested collection', async () => {
             const result = await client.query<CreateCollection.Mutation, CreateCollection.Variables>(
                 CREATE_COLLECTION,
                 {
@@ -162,7 +162,7 @@ describe('Collection resolver', () => {
             expect(computersCollection.parent!.name).toBe(electronicsCollection.name);
         });
 
-        it('creates a 2nd level nested category', async () => {
+        it('creates a 2nd level nested collection', async () => {
             const result = await client.query<CreateCollection.Mutation, CreateCollection.Variables>(
                 CREATE_COLLECTION,
                 {
@@ -689,6 +689,59 @@ describe('Collection resolver', () => {
                     'Camera Lens',
                     'Tripod',
                     'SLR Camera',
+                    'Hat',
+                ]);
+            });
+
+            it('bell OR pear in computers', async () => {
+                const result = await client.query<
+                    CreateCollectionSelectVariants.Mutation,
+                    CreateCollectionSelectVariants.Variables
+                >(CREATE_COLLECTION_SELECT_VARIANTS, {
+                    input: {
+                        parentId: computersCollection.id,
+                        translations: [
+                            {
+                                languageCode: LanguageCode.en,
+                                name: 'Bell OR Pear Computers',
+                                description: '',
+                            },
+                        ],
+                        filters: [
+                            {
+                                code: facetValueCollectionFilter.code,
+                                arguments: [
+                                    {
+                                        name: 'facetValueIds',
+                                        value: `["${getFacetValueId('pear')}", "${getFacetValueId('bell')}"]`,
+                                        type: 'facetValueIds',
+                                    },
+                                    {
+                                        name: 'containsAny',
+                                        value: `true`,
+                                        type: 'boolean',
+                                    },
+                                ],
+                            },
+                        ],
+                    } as CreateCollectionInput,
+                });
+
+                await awaitRunningJobs(client);
+                const { collection } = await client.query<GetCollection.Query, GetCollection.Variables>(
+                    GET_COLLECTION,
+                    {
+                        id: result.createCollection.id,
+                    },
+                );
+
+                expect(collection!.productVariants.items.map(i => i.name)).toEqual([
+                    'Laptop 13 inch 8GB',
+                    'Laptop 15 inch 8GB',
+                    'Laptop 13 inch 16GB',
+                    'Laptop 15 inch 16GB',
+                    'Curvy Monitor 24 inch',
+                    'Curvy Monitor 27 inch',
                 ]);
             });
         });
@@ -799,6 +852,7 @@ describe('Collection resolver', () => {
                     'Clacky Keyboard',
                     'USB Cable',
                     'Tripod',
+                    'Hat',
                 ]);
             });
         });
@@ -913,6 +967,55 @@ describe('Collection resolver', () => {
                 ]);
             });
         });
+
+        it('filter inheritance of nested collections (issue #158)', async () => {
+            const { createCollection: pearElectronics } = await client.query<
+                CreateCollectionSelectVariants.Mutation,
+                CreateCollectionSelectVariants.Variables
+            >(CREATE_COLLECTION_SELECT_VARIANTS, {
+                input: {
+                    parentId: electronicsCollection.id,
+                    translations: [
+                        { languageCode: LanguageCode.en, name: 'pear electronics', description: '' },
+                    ],
+                    filters: [
+                        {
+                            code: facetValueCollectionFilter.code,
+                            arguments: [
+                                {
+                                    name: 'facetValueIds',
+                                    value: `["${getFacetValueId('pear')}"]`,
+                                    type: 'facetValueIds',
+                                },
+                                {
+                                    name: 'containsAny',
+                                    value: `false`,
+                                    type: 'boolean',
+                                },
+                            ],
+                        },
+                    ],
+                } as CreateCollectionInput,
+            });
+
+            await awaitRunningJobs(client);
+
+            const result = await client.query<GetCollectionProducts.Query, GetCollectionProducts.Variables>(
+                GET_COLLECTION_PRODUCT_VARIANTS,
+                { id: pearElectronics.id },
+            );
+            expect(result.collection!.productVariants.items.map(i => i.name)).toEqual([
+                'Laptop 13 inch 8GB',
+                'Laptop 15 inch 8GB',
+                'Laptop 13 inch 16GB',
+                'Laptop 15 inch 16GB',
+                'Curvy Monitor 24 inch',
+                'Curvy Monitor 27 inch',
+                'Gaming PC i7-8700 240GB SSD',
+                'Instant Camera',
+                // no "Hat"
+            ]);
+        });
     });
 
     describe('Product collections property', () => {
@@ -926,8 +1029,9 @@ describe('Collection resolver', () => {
                 { id: 'T_5', name: 'Pear' },
                 { id: 'T_8', name: 'Photo AND Pear' },
                 { id: 'T_9', name: 'Photo OR Pear' },
-                { id: 'T_10', name: 'contains camera' },
-                { id: 'T_12', name: 'endsWith camera' },
+                { id: 'T_11', name: 'contains camera' },
+                { id: 'T_13', name: 'endsWith camera' },
+                { id: 'T_15', name: 'pear electronics' },
             ]);
         });
     });

packages/core/e2e/fixtures/e2e-products-collections.csv

@@ -20,3 +20,4 @@ Instant Camera  , instant-camera  , "With its nostalgic design and simple point-
 Camera Lens     , camera-lens     , "This lens is a Di type lens using an optical system with improved multi-coating designed to function with digital SLR cameras as well as film cameras."                                                                                                                                                                                                                                                                                                                                      ,                                    , category:electronics|category:photo                ,                   ,                     , B0012UUP02   , 104.00  , standard    , 0           , false          ,               ,
 Tripod          , tripod          , "Capture vivid, professional-style photographs with help from this lightweight tripod. The adjustable-height tripod makes it easy to achieve reliable stability and score just the right angle when going after that award-winning shot."                                                                                                                                                                                                                                                     ,                                    , category:electronics|category:photo|brand:bell     ,                   ,                     , B00XI87KV8   , 14.98   , standard    , 0           , false          ,               ,
 SLR Camera      , slr-camera      , "Retro styled, portable in size and built around a powerful 24-megapixel APS-C CMOS sensor, this digital camera is the ideal companion for creative everyday photography. Packed full of high spec features such as an advanced hybrid autofocus system able to keep pace with even the most active subjects, a speedy 6fps continuous-shooting mode, high-resolution electronic viewfinder and intuitive swivelling touchscreen, it brings professional image making into everyone’s grasp." ,                                    , category:electronics|category:photo|brand:bell     ,                   ,                     , B07D75V44S   , 521.00  , standard    , 0           , false          ,               ,
+Hat             , hat             , "A fetching hat."                                                                                                                                                                                                                                                                                                                                                                                                                                                                             ,                                    , category:clothing|style:casual                     ,                   ,                     , B07D75V44S   , 23.99   , standard    , 0           , false          ,               , brand:pear

packages/core/src/config/collection/default-collection-filters.ts

@@ -1,7 +1,7 @@
 import { LanguageCode } from '@vendure/common/lib/generated-types';
-import { Brackets } from 'typeorm';
 
 import { UserInputError } from '../../common/error/errors';
+import { ProductVariant } from '../../entity/product-variant/product-variant.entity';
 
 import { CollectionFilter } from './collection-filter';
 
@@ -16,20 +16,45 @@ export const facetValueCollectionFilter = new CollectionFilter({
     code: 'facet-value-filter',
     description: [{ languageCode: LanguageCode.en, value: 'Filter by FacetValues' }],
     apply: (qb, args) => {
-        if (args.facetValueIds.length) {
-            qb.leftJoin('productVariant.product', 'product')
-                .leftJoin('product.facetValues', 'productFacetValues')
-                .leftJoin('productVariant.facetValues', 'variantFacetValues')
-                .andWhere(
-                    new Brackets(qb1 => {
-                        const ids = args.facetValueIds;
-                        return qb1
-                            .where(`productFacetValues.id IN (:...ids)`, { ids })
-                            .orWhere(`variantFacetValues.id IN (:...ids)`, { ids });
-                    }),
+        const ids = args.facetValueIds;
+
+        if (ids.length) {
+            const idsName = `ids_${ids.join('_')}`;
+            const countName = `count_${ids.join('_')}`;
+            const productFacetValues = qb.connection
+                .createQueryBuilder(ProductVariant, 'product_variant')
+                .select('product_variant.id', 'variant_id')
+                .addSelect('facet_value.id', 'facet_value_id')
+                .leftJoin('product_variant.facetValues', 'facet_value')
+                .where(`facet_value.id IN (:...${idsName})`);
+
+            const variantFacetValues = qb.connection
+                .createQueryBuilder(ProductVariant, 'product_variant')
+                .select('product_variant.id', 'variant_id')
+                .addSelect('facet_value.id', 'facet_value_id')
+                .leftJoin('product_variant.product', 'product')
+                .leftJoin('product.facetValues', 'facet_value')
+                .where(`facet_value.id IN (:...${idsName})`);
+
+            const union = qb.connection
+                .createQueryBuilder()
+                .select('union_table.variant_id')
+                .from(
+                    `(${productFacetValues.getQuery()} UNION ${variantFacetValues.getQuery()})`,
+                    'union_table',
                 )
-                .groupBy('productVariant.id')
-                .having(`COUNT(1) >= :count`, { count: args.containsAny ? 1 : args.facetValueIds.length });
+                .groupBy('variant_id')
+                .having(`COUNT(*) >= :${countName}`);
+
+            const variantIds = qb.connection
+                .createQueryBuilder()
+                .select('variant_ids_table.variant_id')
+                .from(`(${union.getQuery()})`, 'variant_ids_table');
+
+            qb.andWhere(`productVariant.id IN (${variantIds.getQuery()})`).setParameters({
+                [idsName]: ids,
+                [countName]: args.containsAny ? 1 : ids.length,
+            });
         } else {
             // If no facetValueIds are specified, no ProductVariants will be matched.
             qb.andWhere('1 = 0');

packages/core/src/service/controllers/collection.controller.ts

@@ -100,21 +100,9 @@ export class CollectionController {
 
         // Apply any facetValue-based filters
         if (facetFilters.length) {
-            const [idsArg, containsAnyArg] = facetFilters[0].args;
-            const mergedArgs = facetFilters
-                .map(f => f.args[0].value)
-                .filter(notNullOrUndefined)
-                .map(value => JSON.parse(value))
-                .reduce((all, ids) => [...all, ...ids]);
-
-            qb = facetValueCollectionFilter.apply(qb, [
-                {
-                    name: idsArg.name,
-                    type: idsArg.type,
-                    value: JSON.stringify(Array.from(new Set(mergedArgs))),
-                },
-                containsAnyArg,
-            ]);
+            for (const filter of facetFilters) {
+                qb = facetValueCollectionFilter.apply(qb, filter.args);
+            }
         }
 
         // Apply any variant name-based filters