Fix incorrect query when multiple flowObject filter

UN-OCHA · Nov 21, 2023 · 3d788f5 · 3d788f5
1 parent 1d67924
commit 3d788f5
Show file tree

Hide file tree

Showing 3 changed files with 103 additions and 68 deletions.
diff --git a/src/domain-services/flows/flow-search-service.ts b/src/domain-services/flows/flow-search-service.ts
@@ -1,37 +1,37 @@
-import { Database } from '@unocha/hpc-api-core/src/db/type';
+import { type FlowId } from '@unocha/hpc-api-core/src/db/models/flow';
+import { type Database } from '@unocha/hpc-api-core/src/db/type';
 import { Op } from '@unocha/hpc-api-core/src/db/util/conditions';
 import { createBrandedValue } from '@unocha/hpc-api-core/src/util/types';
 import { Service } from 'typedi';
-import {
-  FlowObjectFilters,
-  SearchFlowsArgs,
-  SearchFlowsFilters,
-} from './graphql/args';
-import {
-  FlowPaged,
-  FlowParkedParentSource,
-  FlowSearchResult,
-} from './graphql/types';
-import { FlowSearchStrategy } from './strategy/flow-search-strategy';
-import { OnlyFlowFiltersStrategy } from './strategy/impl/only-flow-conditions-strategy';
-import { FlowObjectFiltersStrategy } from './strategy/impl/flow-object-conditions-strategy';
-import { FlowId } from '@unocha/hpc-api-core/src/db/models/flow';
 import { CategoryService } from '../categories/category-service';
+import { type Category } from '../categories/graphql/types';
 import { ExternalReferenceService } from '../external-reference/external-reference-service';
+import { FlowLinkService } from '../flow-link/flow-link-service';
+import { FlowObjectService } from '../flow-object/flow-object-service';
+import { type FlowObject } from '../flow-object/model';
+import { type BaseLocation } from '../location/graphql/types';
 import { LocationService } from '../location/location-service';
+import { type Organization } from '../organizations/graphql/types';
 import { OrganizationService } from '../organizations/organization-service';
+import { type BasePlan } from '../plans/graphql/types';
 import { PlanService } from '../plans/plan-service';
 import { ReportDetailService } from '../report-details/report-detail-service';
+import { type UsageYear } from '../usage-years/grpahql/types';
 import { UsageYearService } from '../usage-years/usage-year-service';
-import { FlowLinkService } from '../flow-link/flow-link-service';
-import { FlowObject } from '../flow-object/model';
-import { FlowObjectService } from '../flow-object/flow-object-service';
-import { FlowEntity } from './model';
-import { Category } from '../categories/graphql/types';
-import { Organization } from '../organizations/graphql/types';
-import { BaseLocation } from '../location/graphql/types';
-import { BasePlan } from '../plans/graphql/types';
-import { UsageYear } from '../usage-years/grpahql/types';
+import {
+  type FlowObjectFilters,
+  type SearchFlowsArgs,
+  type SearchFlowsFilters,
+} from './graphql/args';
+import {
+  type FlowPaged,
+  type FlowParkedParentSource,
+  type FlowSearchResult,
+} from './graphql/types';
+import { type FlowEntity } from './model';
+import { type FlowSearchStrategy } from './strategy/flow-search-strategy';
+import { FlowObjectFiltersStrategy } from './strategy/impl/flow-object-conditions-strategy';
+import { OnlyFlowFiltersStrategy } from './strategy/impl/only-flow-conditions-strategy';
 
 @Service()
 export class FlowSearchService {
@@ -144,14 +144,14 @@ export class FlowSearchService {
 
     const items = await Promise.all(
       flows.map(async (flow) => {
-        const flowLink = flowLinksMap.get(flow.id) || [];
-        const categories = categoriesMap.get(flow.id) || [];
-        const organizations = organizationsMap.get(flow.id) || [];
-        const locations = [...(locationsMap.get(flow.id) || [])];
-        const plans = plansMap.get(flow.id) || [];
-        const usageYears = usageYearsMap.get(flow.id) || [];
-        const externalReferences = externalReferencesMap.get(flow.id) || [];
-        const reportDetails = reportDetailsMap.get(flow.id) || [];
+        const flowLink = flowLinksMap.get(flow.id) ?? [];
+        const categories = categoriesMap.get(flow.id) ?? [];
+        const organizations = organizationsMap.get(flow.id) ?? [];
+        const locations = [...(locationsMap.get(flow.id) ?? [])];
+        const plans = plansMap.get(flow.id) ?? [];
+        const usageYears = usageYearsMap.get(flow.id) ?? [];
+        const externalReferences = externalReferencesMap.get(flow.id) ?? [];
+        const reportDetails = reportDetailsMap.get(flow.id) ?? [];
 
         let parkedParentSource: FlowParkedParentSource[] = [];
         if (flow.activeStatus && flowLink.length > 0) {
@@ -197,7 +197,7 @@ export class FlowSearchService {
       hasNextPage: limit <= flows.length,
       hasPreviousPage: afterCursor !== undefined,
       startCursor: flows.length ? flows[0].id.valueOf() : 0,
-      endCursor: flows.length ? flows[flows.length - 1].id.valueOf() : 0,
+      endCursor: flows.length ? flows.at(-1)?.id.valueOf() ?? 0 : 0,
       pageSize: flows.length,
       sortField: orderBy.column,
       sortOrder: orderBy.order,
@@ -209,11 +209,15 @@ export class FlowSearchService {
     let flowConditions = {};
 
     if (flowFilters) {
-      Object.entries(flowFilters).forEach(([key, value]) => {
+      for (const [key, value] of Object.entries(flowFilters)) {
         if (value !== undefined) {
-          flowConditions = { ...flowConditions, [key]: value };
+          if (Array.isArray(value) && value.length !== 0) {
+            flowConditions = { ...flowConditions, [key]: { [Op.IN]: value } };
+          } else {
+            flowConditions = { ...flowConditions, [key]: value };
+          }
         }
-      });
+      }
     }
 
     return flowConditions;
@@ -222,13 +226,16 @@ export class FlowSearchService {
   prepareFlowObjectConditions(
     flowObjectFilters: FlowObjectFilters[]
   ): Map<string, Map<string, number[]>> {
-    const flowObjectsConditions: Map<string, Map<string, number[]>> = new Map();
+    const flowObjectsConditions: Map<string, Map<string, number[]>> = new Map<
+      string,
+      Map<string, number[]>
+    >();
 
-    flowObjectFilters.forEach((flowObjectFilter) => {
+    for (const flowObjectFilter of flowObjectFilters) {
       const { objectType, direction, objectID } = flowObjectFilter;
 
       if (!flowObjectsConditions.has(objectType)) {
-        flowObjectsConditions.set(objectType, new Map());
+        flowObjectsConditions.set(objectType, new Map<string, number[]>());
       }
 
       const refDirectionMap = flowObjectsConditions.get(objectType);
@@ -245,7 +252,7 @@ export class FlowSearchService {
       }
 
       objectIDsArray!.push(objectID);
-    });
+    }
 
     return flowObjectsConditions;
   }
@@ -255,15 +262,22 @@ export class FlowSearchService {
     flowObjectFilters: FlowObjectFilters[]
   ): { strategy: FlowSearchStrategy; conditions: any } {
     let conditions = {};
+
+    const isFlowFilterDefined = flowFilters !== undefined;
+    const isFlowObjectFilterDefined = flowObjectFilters !== undefined;
+    const isFlowObjectFiltersNotEmpty =
+      isFlowObjectFilterDefined && flowObjectFilters.length !== 0;
+
     if (
-      (!flowFilters &&
-        (!flowObjectFilters || flowObjectFilters.length === 0)) ||
-      (flowFilters && (!flowObjectFilters || flowObjectFilters.length === 0))
+      (!isFlowFilterDefined &&
+        (!isFlowObjectFilterDefined || !isFlowObjectFiltersNotEmpty)) ||
+      (isFlowFilterDefined &&
+        (!isFlowObjectFilterDefined || !isFlowObjectFiltersNotEmpty))
     ) {
       const flowConditions = this.prepareFlowConditions(flowFilters);
       conditions = { ...conditions, ...flowConditions };
       return { strategy: this.onlyFlowFiltersStrategy, conditions };
-    } else if (!flowFilters && flowObjectFilters.length !== 0) {
+    } else if (!isFlowFilterDefined && isFlowObjectFiltersNotEmpty) {
       const flowObjectConditions =
         this.prepareFlowObjectConditions(flowObjectFilters);
       conditions = { ...conditions, ...flowObjectConditions };
@@ -272,7 +286,7 @@ export class FlowSearchService {
         strategy: this.flowObjectFiltersStrategy,
         conditions: this.buildConditionsMap(undefined, flowObjectConditions),
       };
-    } else if (flowFilters && flowObjectFilters.length !== 0) {
+    } else if (isFlowFilterDefined && isFlowObjectFiltersNotEmpty) {
       const flowConditions = this.prepareFlowConditions(flowFilters);
       const flowObjectConditions =
         this.prepareFlowObjectConditions(flowObjectFilters);
@@ -291,9 +305,7 @@ export class FlowSearchService {
       };
     }
 
-    throw new Error(
-      'Invalid combination of flowFilters and flowObjectFilters - temp: only provide flowFilters'
-    );
+    throw new Error('Invalid combination of flowFilters and flowObjectFilters');
   }
 
   private buildConditionsMap(flowConditions: any, flowObjectConditions: any) {
@@ -302,14 +314,15 @@ export class FlowSearchService {
     conditionsMap.set('flow', flowConditions);
     return conditionsMap;
   }
+
   private mapFlowObjects(
     flowObjects: FlowObject[],
     organizationsFO: any[],
     locationsFO: any[],
     plansFO: any[],
     usageYearsFO: any[]
   ) {
-    flowObjects.forEach((flowObject) => {
+    for (const flowObject of flowObjects) {
       if (flowObject.objectType === 'organization') {
         organizationsFO.push(flowObject);
       } else if (flowObject.objectType === 'location') {
@@ -319,7 +332,7 @@ export class FlowSearchService {
       } else if (flowObject.objectType === 'usageYear') {
         usageYearsFO.push(flowObject);
       }
-    });
+    }
   }
 
   private async getParketParents(

diff --git a/src/domain-services/flows/strategy/impl/flow-object-conditions-strategy.ts b/src/domain-services/flows/strategy/impl/flow-object-conditions-strategy.ts
@@ -1,11 +1,12 @@
-import { Database } from '@unocha/hpc-api-core/src/db';
+import { type Database } from '@unocha/hpc-api-core/src/db';
+import { type FlowId } from '@unocha/hpc-api-core/src/db/models/flow';
 import { Op } from '@unocha/hpc-api-core/src/db/util/conditions';
 import { Service } from 'typedi';
 import { FlowObjectService } from '../../../flow-object/flow-object-service';
-import { FlowService } from '../../flow-service';
+import { type FlowService } from '../../flow-service';
 import {
-  FlowSearchStrategy,
-  FlowSearchStrategyResponse,
+  type FlowSearchStrategy,
+  type FlowSearchStrategyResponse,
 } from '../flow-search-strategy';
 
 @Service()
@@ -22,22 +23,23 @@ export class FlowObjectFiltersStrategy implements FlowSearchStrategy {
     cursorCondition: any,
     models: Database
   ): Promise<FlowSearchStrategyResponse> {
+    // Obtain flowObjects conditions
     const flowObjectsConditions: Map<
       string,
       Map<string, number[]>
-    > = flowConditions.get('flowObjects');
-    const flowEntityConditions = flowConditions.get('flow');
+    > = flowConditions.get('flowObjects') ?? new Map();
 
+    // Obtain flow conditions
+    const flowEntityConditions = flowConditions.get('flow') ?? new Map();
+
+    // Obtain where clause for flowObjects
     const flowObjectWhere = this.mapFlowObjectConditionsToWhereClause(
       flowObjectsConditions
     );
 
     // Obtain flowIDs based on provided flowObject conditions
-    const flowIDsFromFilteredFlowObjects =
-      await this.flowObjectService.getFlowIdsFromFlowObjects(
-        models,
-        flowObjectWhere
-      );
+    const flowIDsFromFilteredFlowObjects: FlowId[] =
+      await this.getFlowIDsFromFilteredFlowObjects(models, flowObjectWhere);
 
     // Combine conditions from flowObjects FlowIDs and flow conditions
     const mergedFlowConditions = {
@@ -62,6 +64,25 @@ export class FlowObjectFiltersStrategy implements FlowSearchStrategy {
     return { flows, count: countObject.count };
   }
 
+  private async getFlowIDsFromFilteredFlowObjects(
+    models: Database,
+    flowObjectWhere: any[]
+  ): Promise<FlowId[]> {
+    const flowIDsFromFilteredFlowObjects: FlowId[] = [];
+    const tempFlowIDs: FlowId[][] = await Promise.all(
+      flowObjectWhere.map((whereClause) =>
+        this.flowObjectService.getFlowIdsFromFlowObjects(models, whereClause)
+      )
+    );
+    // Flatten array of arrays keeping only values present in all arrays
+    const flowIDs = tempFlowIDs.reduce((a, b) =>
+      a.filter((c) => b.includes(c))
+    );
+    flowIDsFromFilteredFlowObjects.push(...flowIDs);
+
+    return flowIDsFromFilteredFlowObjects;
+  }
+
   /*
    * Map structure:
    * {
@@ -74,12 +95,11 @@ export class FlowObjectFiltersStrategy implements FlowSearchStrategy {
    */
   private mapFlowObjectConditionsToWhereClause(
     flowObjectConditions: Map<string, Map<string, number[]>>
-  ): any {
-    let flowObjectWhere: any = {};
+  ): any[] {
+    const whereClauses: any = [];
     for (const [objectType, refDirectionMap] of flowObjectConditions) {
       for (const [refDirection, objectIDs] of refDirectionMap) {
-        flowObjectWhere = {
-          ...flowObjectWhere,
+        const whereClause = {
           objectID: {
             [Op.IN]: objectIDs,
           },
@@ -90,9 +110,11 @@ export class FlowObjectFiltersStrategy implements FlowSearchStrategy {
             [Op.LIKE]: objectType,
           },
         };
+
+        whereClauses.push(whereClause);
       }
     }
 
-    return flowObjectWhere;
+    return whereClauses;
   }
 }
diff --git a/src/utils/graphql/pagination.ts b/src/utils/graphql/pagination.ts
@@ -1,6 +1,6 @@
 import { Op } from '@unocha/hpc-api-core/src/db/util/conditions';
 import { createBrandedValue } from '@unocha/hpc-api-core/src/util/types';
-import { ObjectType, Field, ArgsType } from 'type-graphql';
+import { ArgsType, Field, ObjectType } from 'type-graphql';
 
 export type SortOrder = 'asc' | 'desc';
 
@@ -46,7 +46,7 @@ export function prepareConditionFromCursor(
 
   if (afterCursor || beforeCursor) {
     const isAscending = sortCondition.order === 'asc';
-    const cursorValue = afterCursor || beforeCursor;
+    const cursorValue = afterCursor ?? beforeCursor;
 
     let op;
     if (isAscending) {