Browse Source

fix(core): Fix some issues with sorting/filtering calculated properties

Michael Bromley 4 years ago
parent
commit
2d89554e61

+ 41 - 3
packages/core/e2e/order.e2e-spec.ts

@@ -227,6 +227,7 @@ describe('Orders resolver', () => {
                         sort: {
                             total: SortOrder.DESC,
                         },
+                        take: 10,
                     },
                 },
             );
@@ -236,19 +237,21 @@ describe('Orders resolver', () => {
             ]);
         });
 
-        it('filter by totalWithTax', async () => {
+        it('sort by totalWithTax', async () => {
             const result = await adminClient.query<GetOrderList.Query, GetOrderList.Variables>(
                 GET_ORDERS_LIST,
                 {
                     options: {
-                        filter: {
-                            totalWithTax: { gt: 323760 },
+                        sort: {
+                            totalWithTax: SortOrder.DESC,
                         },
+                        take: 10,
                     },
                 },
             );
             expect(result.orders.items.map(o => pick(o, ['id', 'totalWithTax']))).toEqual([
                 { id: 'T_2', totalWithTax: 959520 },
+                { id: 'T_1', totalWithTax: 323760 },
             ]);
         });
 
@@ -260,6 +263,7 @@ describe('Orders resolver', () => {
                         sort: {
                             totalQuantity: SortOrder.DESC,
                         },
+                        take: 10,
                     },
                 },
             );
@@ -269,6 +273,40 @@ describe('Orders resolver', () => {
             ]);
         });
 
+        it('filter by total', async () => {
+            const result = await adminClient.query<GetOrderList.Query, GetOrderList.Variables>(
+                GET_ORDERS_LIST,
+                {
+                    options: {
+                        filter: {
+                            total: { gt: 323760 },
+                        },
+                        take: 10,
+                    },
+                },
+            );
+            expect(result.orders.items.map(o => pick(o, ['id', 'total']))).toEqual([
+                { id: 'T_2', total: 799600 },
+            ]);
+        });
+
+        it('filter by totalWithTax', async () => {
+            const result = await adminClient.query<GetOrderList.Query, GetOrderList.Variables>(
+                GET_ORDERS_LIST,
+                {
+                    options: {
+                        filter: {
+                            totalWithTax: { gt: 323760 },
+                        },
+                        take: 10,
+                    },
+                },
+            );
+            expect(result.orders.items.map(o => pick(o, ['id', 'totalWithTax']))).toEqual([
+                { id: 'T_2', totalWithTax: 959520 },
+            ]);
+        });
+
         it('filter by totalQuantity', async () => {
             const result = await adminClient.query<GetOrderList.Query, GetOrderList.Variables>(
                 GET_ORDERS_LIST,

+ 105 - 0
packages/core/e2e/product.e2e-spec.ts

@@ -356,18 +356,21 @@ describe('Product resolver', () => {
                     id: 'T_34',
                     name: 'Bonsai Tree',
                     price: 1999,
+                    priceWithTax: 2399,
                     sku: 'B01MXFLUSV',
                 },
                 {
                     id: 'T_24',
                     name: 'Boxing Gloves',
                     price: 3304,
+                    priceWithTax: 3965,
                     sku: 'B000ZYLPPU',
                 },
                 {
                     id: 'T_19',
                     name: 'Camera Lens',
                     price: 10400,
+                    priceWithTax: 12480,
                     sku: 'B0012UUP02',
                 },
             ]);
@@ -391,22 +394,123 @@ describe('Product resolver', () => {
                     id: 'T_23',
                     name: 'Skipping Rope',
                     price: 799,
+                    priceWithTax: 959,
                     sku: 'B07CNGXVXT',
                 },
                 {
                     id: 'T_20',
                     name: 'Tripod',
                     price: 1498,
+                    priceWithTax: 1798,
                     sku: 'B00XI87KV8',
                 },
                 {
                     id: 'T_32',
                     name: 'Spiky Cactus',
                     price: 1550,
+                    priceWithTax: 1860,
                     sku: 'SC011001',
                 },
             ]);
         });
+
+        it('sort by priceWithTax', async () => {
+            const { productVariants } = await adminClient.query<
+                GetProductVariantList.Query,
+                GetProductVariantList.Variables
+            >(GET_PRODUCT_VARIANT_LIST, {
+                options: {
+                    take: 3,
+                    sort: {
+                        priceWithTax: SortOrder.ASC,
+                    },
+                },
+            });
+
+            expect(productVariants.items).toEqual([
+                {
+                    id: 'T_23',
+                    name: 'Skipping Rope',
+                    price: 799,
+                    priceWithTax: 959,
+                    sku: 'B07CNGXVXT',
+                },
+                {
+                    id: 'T_20',
+                    name: 'Tripod',
+                    price: 1498,
+                    priceWithTax: 1798,
+                    sku: 'B00XI87KV8',
+                },
+                {
+                    id: 'T_32',
+                    name: 'Spiky Cactus',
+                    price: 1550,
+                    priceWithTax: 1860,
+                    sku: 'SC011001',
+                },
+            ]);
+        });
+
+        it('filter by price', async () => {
+            const { productVariants } = await adminClient.query<
+                GetProductVariantList.Query,
+                GetProductVariantList.Variables
+            >(GET_PRODUCT_VARIANT_LIST, {
+                options: {
+                    take: 3,
+                    filter: {
+                        price: {
+                            between: {
+                                start: 1400,
+                                end: 1500,
+                            },
+                        },
+                    },
+                },
+            });
+
+            expect(productVariants.items).toEqual([
+                {
+                    id: 'T_20',
+                    name: 'Tripod',
+                    price: 1498,
+                    priceWithTax: 1798,
+                    sku: 'B00XI87KV8',
+                },
+            ]);
+        });
+
+        it('filter by priceWithTax', async () => {
+            const { productVariants } = await adminClient.query<
+                GetProductVariantList.Query,
+                GetProductVariantList.Variables
+            >(GET_PRODUCT_VARIANT_LIST, {
+                options: {
+                    take: 3,
+                    filter: {
+                        priceWithTax: {
+                            between: {
+                                start: 1400,
+                                end: 1500,
+                            },
+                        },
+                    },
+                },
+            });
+
+            // Note the results are incorrect. This is a design trade-off. See the
+            // commend on the ProductVariant.priceWithTax annotation for explanation.
+            expect(productVariants.items).toEqual([
+                {
+                    id: 'T_20',
+                    name: 'Tripod',
+                    price: 1498,
+                    priceWithTax: 1798,
+                    sku: 'B00XI87KV8',
+                },
+            ]);
+        });
     });
 
     describe('productVariant query', () => {
@@ -1343,6 +1447,7 @@ export const GET_PRODUCT_VARIANT_LIST = gql`
                 name
                 sku
                 price
+                priceWithTax
             }
             totalItems
         }

+ 6 - 3
packages/core/src/entity/order/order.entity.ts

@@ -136,7 +136,10 @@ export class Order extends VendureEntity implements ChannelAware, HasCustomField
         return [...groupedAdjustments.values()];
     }
 
-    @Calculated({ expression: 'subTotal + shipping' })
+    @Calculated({
+        query: qb => qb.addSelect('shipping', 'shipping'),
+        expression: 'subTotal + shipping',
+    })
     get total(): number {
         return this.subTotal + (this.shipping || 0);
     }
@@ -160,9 +163,9 @@ export class Order extends VendureEntity implements ChannelAware, HasCustomField
                 },
                 't1',
                 't1.oid = order.id',
-            );
+            ).addSelect('t1.qty', 'qty');
         },
-        expression: 't1.qty',
+        expression: 'qty',
     })
     get totalQuantity(): number {
         return summate(this.lines, 'quantity');

+ 4 - 0
packages/core/src/entity/product-variant/product-variant.entity.ts

@@ -80,6 +80,10 @@ export class ProductVariant
     }
 
     @Calculated({
+        // Note: this works fine for sorting by priceWithTax, but filtering will return inaccurate
+        // results due to this expression not taking taxes into account. This is because the tax
+        // rate is calculated at run-time in the application layer based on the current context,
+        // and is unknown to the database.
         expression: 'productvariant_productVariantPrices.price',
     })
     get priceWithTax(): number {

+ 1 - 0
packages/core/src/service/helpers/list-query-builder/parse-sort-params.spec.ts

@@ -135,6 +135,7 @@ export class MockConnection {
     private columnsMap = new Map<Type<any>, Array<Partial<ColumnMetadata>>>();
     private relationsMap = new Map<Type<any>, Array<Partial<RelationMetadata>>>();
     setColumns(entity: Type<any>, value: Array<Partial<ColumnMetadata>>) {
+        value.forEach(v => (v.propertyPath = v.propertyName));
         this.columnsMap.set(entity, value);
     }
     setRelations(entity: Type<any>, value: Array<Partial<RelationMetadata>>) {

+ 3 - 2
packages/core/src/service/helpers/list-query-builder/parse-sort-params.ts

@@ -30,8 +30,9 @@ export function parseSortParams<T extends VendureEntity>(
     const output: OrderByCondition = {};
     for (const [key, order] of Object.entries(sortParams)) {
         const calculatedColumnDef = calculatedColumns.find(c => c.name === key);
-        if (columns.find(c => c.propertyName === key)) {
-            output[`${alias}.${key}`] = order as any;
+        const matchingColumn = columns.find(c => c.propertyName === key);
+        if (matchingColumn) {
+            output[`${alias}.${matchingColumn.propertyPath}`] = order as any;
         } else if (translationColumns.find(c => c.propertyName === key)) {
             const translationsAlias = connection.namingStrategy.eagerJoinRelationAlias(alias, 'translations');
             output[`${translationsAlias}.${key}`] = order as any;