Skip to content

Commit

Permalink
fix(core): Fix facet value CollectionFilter
Browse files Browse the repository at this point in the history 
Fixes #158
  • Loading branch information
@michaelbromley
michaelbromley committed Sep 11, 2019
1 parent 5667062 commit 7b6fe6c
Show file tree
Hide file tree
Showing 4 changed files with 151 additions and 33 deletions.
112 changes: 108 additions & 4 deletions packages/core/e2e/collection.e2e-spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
{
Expand Down Expand Up @@ -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,
{
Expand Down Expand Up @@ -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',
]);
});
});
Expand Down Expand Up @@ -799,6 +852,7 @@ describe('Collection resolver', () => {
'Clacky Keyboard',
'USB Cable',
'Tripod',
'Hat',
]);
});
});
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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' },
]);
});
});
Expand Down
1 change: 1 addition & 0 deletions packages/core/e2e/fixtures/e2e-products-collections.csv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
53 changes: 39 additions & 14 deletions packages/core/src/config/collection/default-collection-filters.ts
View file
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -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');
Expand Down
18 changes: 3 additions & 15 deletions packages/core/src/service/controllers/collection.controller.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7b6fe6c

Please sign in to comment.