Browse Source

fix(core): Improved error handling for malformed collection filters

Michael Bromley 3 years ago
parent
commit
cab520b4fc

+ 5 - 22
packages/core/src/api/resolvers/admin/collection.resolver.ts

@@ -54,10 +54,7 @@ export class CollectionResolver {
         })
         })
         relations: RelationPaths<Collection>,
         relations: RelationPaths<Collection>,
     ): Promise<PaginatedList<Translated<Collection>>> {
     ): Promise<PaginatedList<Translated<Collection>>> {
-        return this.collectionService.findAll(ctx, args.options || undefined, relations).then(res => {
-            res.items.forEach(this.encodeFilters);
-            return res;
-        });
+        return this.collectionService.findAll(ctx, args.options || undefined, relations);
     }
     }
 
 
     @Query()
     @Query()
@@ -82,8 +79,7 @@ export class CollectionResolver {
         } else {
         } else {
             throw new UserInputError(`error.collection-id-or-slug-must-be-provided`);
             throw new UserInputError(`error.collection-id-or-slug-must-be-provided`);
         }
         }
-
-        return this.encodeFilters(collection);
+        return collection;
     }
     }
 
 
     @Query()
     @Query()
@@ -102,7 +98,7 @@ export class CollectionResolver {
     ): Promise<Translated<Collection>> {
     ): Promise<Translated<Collection>> {
         const { input } = args;
         const { input } = args;
         this.configurableOperationCodec.decodeConfigurableOperationIds(CollectionFilter, input.filters);
         this.configurableOperationCodec.decodeConfigurableOperationIds(CollectionFilter, input.filters);
-        return this.collectionService.create(ctx, input).then(this.encodeFilters);
+        return this.collectionService.create(ctx, input);
     }
     }
 
 
     @Transaction()
     @Transaction()
@@ -114,7 +110,7 @@ export class CollectionResolver {
     ): Promise<Translated<Collection>> {
     ): Promise<Translated<Collection>> {
         const { input } = args;
         const { input } = args;
         this.configurableOperationCodec.decodeConfigurableOperationIds(CollectionFilter, input.filters || []);
         this.configurableOperationCodec.decodeConfigurableOperationIds(CollectionFilter, input.filters || []);
-        return this.collectionService.update(ctx, input).then(this.encodeFilters);
+        return this.collectionService.update(ctx, input);
     }
     }
 
 
     @Transaction()
     @Transaction()
@@ -125,7 +121,7 @@ export class CollectionResolver {
         @Args() args: MutationMoveCollectionArgs,
         @Args() args: MutationMoveCollectionArgs,
     ): Promise<Translated<Collection>> {
     ): Promise<Translated<Collection>> {
         const { input } = args;
         const { input } = args;
-        return this.collectionService.move(ctx, input).then(this.encodeFilters);
+        return this.collectionService.move(ctx, input);
     }
     }
 
 
     @Transaction()
     @Transaction()
@@ -137,17 +133,4 @@ export class CollectionResolver {
     ): Promise<DeletionResponse> {
     ): Promise<DeletionResponse> {
         return this.collectionService.delete(ctx, args.id);
         return this.collectionService.delete(ctx, args.id);
     }
     }
-
-    /**
-     * Encodes any entity IDs used in the filter arguments.
-     */
-    private encodeFilters = <T extends Collection | undefined>(collection: T): T => {
-        if (collection) {
-            this.configurableOperationCodec.encodeConfigurableOperationIds(
-                CollectionFilter,
-                collection.filters,
-            );
-        }
-        return collection;
-    };
 }
 }

+ 24 - 1
packages/core/src/api/resolvers/entity/collection-entity.resolver.ts

@@ -1,14 +1,21 @@
+import { Logger } from '@nestjs/common';
 import { Args, Parent, ResolveField, Resolver } from '@nestjs/graphql';
 import { Args, Parent, ResolveField, Resolver } from '@nestjs/graphql';
-import { CollectionBreadcrumb, ProductVariantListOptions } from '@vendure/common/lib/generated-types';
+import {
+    CollectionBreadcrumb,
+    ConfigurableOperation,
+    ProductVariantListOptions,
+} from '@vendure/common/lib/generated-types';
 import { PaginatedList } from '@vendure/common/lib/shared-types';
 import { PaginatedList } from '@vendure/common/lib/shared-types';
 
 
 import { ListQueryOptions } from '../../../common/types/common-types';
 import { ListQueryOptions } from '../../../common/types/common-types';
 import { Translated } from '../../../common/types/locale-types';
 import { Translated } from '../../../common/types/locale-types';
+import { CollectionFilter } from '../../../config/index';
 import { Asset, Collection, Product, ProductVariant } from '../../../entity';
 import { Asset, Collection, Product, ProductVariant } from '../../../entity';
 import { LocaleStringHydrator } from '../../../service/helpers/locale-string-hydrator/locale-string-hydrator';
 import { LocaleStringHydrator } from '../../../service/helpers/locale-string-hydrator/locale-string-hydrator';
 import { AssetService } from '../../../service/services/asset.service';
 import { AssetService } from '../../../service/services/asset.service';
 import { CollectionService } from '../../../service/services/collection.service';
 import { CollectionService } from '../../../service/services/collection.service';
 import { ProductVariantService } from '../../../service/services/product-variant.service';
 import { ProductVariantService } from '../../../service/services/product-variant.service';
+import { ConfigurableOperationCodec } from '../../common/configurable-operation-codec';
 import { ApiType } from '../../common/get-api-type';
 import { ApiType } from '../../common/get-api-type';
 import { RequestContext } from '../../common/request-context';
 import { RequestContext } from '../../common/request-context';
 import { Api } from '../../decorators/api.decorator';
 import { Api } from '../../decorators/api.decorator';
@@ -22,6 +29,7 @@ export class CollectionEntityResolver {
         private collectionService: CollectionService,
         private collectionService: CollectionService,
         private assetService: AssetService,
         private assetService: AssetService,
         private localeStringHydrator: LocaleStringHydrator,
         private localeStringHydrator: LocaleStringHydrator,
+        private configurableOperationCodec: ConfigurableOperationCodec,
     ) {}
     ) {}
 
 
     @ResolveField()
     @ResolveField()
@@ -118,4 +126,19 @@ export class CollectionEntityResolver {
     async assets(@Ctx() ctx: RequestContext, @Parent() collection: Collection): Promise<Asset[] | undefined> {
     async assets(@Ctx() ctx: RequestContext, @Parent() collection: Collection): Promise<Asset[] | undefined> {
         return this.assetService.getEntityAssets(ctx, collection);
         return this.assetService.getEntityAssets(ctx, collection);
     }
     }
+
+    @ResolveField()
+    filters(@Ctx() ctx: RequestContext, @Parent() collection: Collection): ConfigurableOperation[] {
+        try {
+            return this.configurableOperationCodec.encodeConfigurableOperationIds(
+                CollectionFilter,
+                collection.filters,
+            );
+        } catch (e: any) {
+            Logger.error(
+                `Could not decode the collection filter arguments for "${collection.name}" (id: ${collection.id}). Error message: ${e.message}`,
+            );
+            return [];
+        }
+    }
 }
 }

+ 1 - 1
packages/core/src/common/configurable-operation.ts

@@ -440,7 +440,7 @@ function coerceValueToType<T extends ConfigArgs>(
         try {
         try {
             return (JSON.parse(value) as string[]).map(v => coerceValueToType(v, type, false)) as any;
             return (JSON.parse(value) as string[]).map(v => coerceValueToType(v, type, false)) as any;
         } catch (err) {
         } catch (err) {
-            throw new InternalServerError(err.message);
+            throw new InternalServerError(`Could not parse list value "${value}": ` + err.message);
         }
         }
     }
     }
     switch (type) {
     switch (type) {

+ 14 - 4
packages/core/src/service/services/collection.service.ts

@@ -119,10 +119,20 @@ export class CollectionService implements OnModuleInit {
                     }
                     }
                     completed++;
                     completed++;
                     if (collection) {
                     if (collection) {
-                        const affectedVariantIds = await this.applyCollectionFiltersInternal(
-                            collection,
-                            job.data.applyToChangedVariantsOnly,
-                        );
+                        let affectedVariantIds: ID[] = [];
+                        try {
+                            affectedVariantIds = await this.applyCollectionFiltersInternal(
+                                collection,
+                                job.data.applyToChangedVariantsOnly,
+                            );
+                        } catch (e) {
+                            const translatedCollection = await this.translator.translate(collection, ctx);
+                            Logger.error(
+                                `An error occurred when processing the filters for the collection "${translatedCollection.name}" (id: ${collection.id})`,
+                            );
+                            Logger.error(e.message);
+                            continue;
+                        }
                         job.setProgress(Math.ceil((completed / job.data.collectionIds.length) * 100));
                         job.setProgress(Math.ceil((completed / job.data.collectionIds.length) * 100));
                         this.eventBus.publish(
                         this.eventBus.publish(
                             new CollectionModificationEvent(ctx, collection, affectedVariantIds),
                             new CollectionModificationEvent(ctx, collection, affectedVariantIds),