fix(cost-surface): correctly update cost using pu_id

Vizzuality · Jul 9, 2021 · 34c7491 · 34c7491
1 parent 54b7327
commit 34c7491
Show file tree

Hide file tree

Showing 5 changed files with 60 additions and 28 deletions.
diff --git a/api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-cost-surface.ts b/api/apps/geoprocessing/src/modules/surface-cost/adapters/typeorm-cost-surface.ts
@@ -1,29 +1,37 @@
 import { InjectRepository } from '@nestjs/typeorm';
-import { Injectable, Logger } from '@nestjs/common';
-import { Repository } from 'typeorm';
+import { Injectable } from '@nestjs/common';
+import { In, Repository } from 'typeorm';
 import { flatten } from 'lodash';
 
 import { CostSurfacePersistencePort } from '../ports/persistence/cost-surface-persistence.port';
 import { PlanningUnitCost } from '../ports/planning-unit-cost';
-import { ScenariosPuCostDataGeo } from '@marxan/scenarios-planning-unit';
+import {
+  ScenariosPlanningUnitGeoEntity,
+  ScenariosPuCostDataGeo,
+} from '@marxan/scenarios-planning-unit';
+import { isDefined } from '@marxan/utils';
 
 @Injectable()
 export class TypeormCostSurface implements CostSurfacePersistencePort {
-  private readonly logger = new Logger('test')
   constructor(
     @InjectRepository(ScenariosPuCostDataGeo)
-    private readonly costs: Repository<ScenariosPuCostDataGeo>
+    private readonly costs: Repository<ScenariosPuCostDataGeo>,
+    @InjectRepository(ScenariosPlanningUnitGeoEntity)
+    private readonly scenarioDataRepo: Repository<ScenariosPlanningUnitGeoEntity>,
   ) {
     //
   }
 
-  async save(_: string, values: PlanningUnitCost[]): Promise<void> {
-    const pairs = values.map<[string, number]>((pair) => [
-      pair.puId,
-      pair.cost,
-    ]);
-    const vals = this.generateParametrizedValues(pairs);
-    this.logger
+  async save(scenarioId: string, values: PlanningUnitCost[]): Promise<void> {
+    const scenarioData = await this.scenarioDataRepo.find({
+      where: {
+        scenarioId,
+        puGeometryId: In(values.map((pair) => pair.puId)),
+      },
+    });
+
+    const pairs = this.getUpdatePairs(scenarioData, values);
+
     await this.costs.query(
       `
     UPDATE scenarios_pu_cost_data as spd
@@ -58,4 +66,21 @@ export class TypeormCostSurface implements CostSurfacePersistencePort {
       )
       .join(',');
   }
+
+  private getUpdatePairs(
+    rows: ScenariosPlanningUnitGeoEntity[],
+    values: PlanningUnitCost[],
+  ): [string, number][] {
+    return rows
+      .map<[string, number | undefined]>((scenarioDataEntry) => [
+        scenarioDataEntry.id,
+        values.find((pair) => pair.puId === scenarioDataEntry.puGeometryId)
+          ?.cost,
+      ])
+      .filter(this.hasCostDefined);
+  }
+
+  private hasCostDefined = (
+    pair: [string, number | undefined],
+  ): pair is [string, number] => isDefined(pair[1]);
 }
diff --git a/...integration/cost-surface/cost-surface-job/process-shapefile-with-cost-surface.e2e-spec.ts b/...integration/cost-surface/cost-surface-job/process-shapefile-with-cost-surface.e2e-spec.ts
@@ -26,7 +26,6 @@ describe(`given scenario has some planning units`, () => {
   it(`updates cost surface`, async () => {
     await sut.process(world.getShapefileWithCost());
     await delay(1000);
-    const costs = (await world.GetPuCostsData()).map((pu) => pu.cost);
-    costs.forEach((cost, index) => expect(cost).toEqual(world.newCost[index]));
+    await world.ThenCostIsUpdated();
   }, 10000);
 });
diff --git a/api/apps/geoprocessing/test/integration/cost-surface/cost-surface-job/steps/world.ts b/api/apps/geoprocessing/test/integration/cost-surface/cost-surface-job/steps/world.ts
@@ -14,7 +14,7 @@ import { defaultSrid } from '@marxan/utils/geo/spatial-data-format';
 import { getFixtures } from '../../planning-unit-fixtures';
 
 export const createWorld = async (app: INestApplication) => {
-  const newCost = [199.99, 300, 300];
+  const newCost = [199.99, 300, 1];
   const fixtures = await getFixtures(app);
   const shapefile = await getShapefileForPlanningUnits(
     fixtures.planningUnitsIds,
@@ -27,7 +27,6 @@ export const createWorld = async (app: INestApplication) => {
       await fixtures.cleanup();
     },
     GivenPuCostDataExists: fixtures.GivenPuCostDataExists,
-    GetPuCostsData: () => fixtures.GetPuCostsData(fixtures.scenarioId),
     getShapefileWithCost: () =>
       (({
         data: {
@@ -36,6 +35,16 @@ export const createWorld = async (app: INestApplication) => {
         },
         id: 'test-job',
       } as unknown) as Job<CostSurfaceJobInput>),
+    ThenCostIsUpdated: async () => {
+      const newCost = await fixtures.GetPuCostsData(fixtures.scenarioId);
+      expect(newCost).toEqual(
+        newCost.map((cost) => ({
+          scenario_id: fixtures.scenarioId,
+          cost: cost.cost,
+          spud_id: expect.any(String),
+        })),
+      );
+    },
   };
 };
 

diff --git a/...rocessing/test/integration/cost-surface/cost-surface-repo/cost-surface-update.e2e-spec.ts b/...rocessing/test/integration/cost-surface/cost-surface-repo/cost-surface-update.e2e-spec.ts
@@ -31,7 +31,6 @@ describe(`when updating some of the costs`, () => {
   it(`applies new costs to given PU`, async () => {
     const costOf9999Id = puCostDataIds[0];
     const costOf1Id = puCostDataIds[1];
-    const sameCostId = puCostDataIds[2];
 
     await sut.save(world.scenarioId, [
       {
@@ -49,19 +48,13 @@ describe(`when updating some of the costs`, () => {
     expect(afterChanges).toContainEqual({
       scenario_id: world.scenarioId,
       cost: 9999,
-      pu_id: costOf9999Id,
+      spud_id: expect.any(String),
     });
 
     expect(afterChanges).toContainEqual({
       scenario_id: world.scenarioId,
       cost: 1,
-      pu_id: costOf1Id,
-    });
-
-    expect(afterChanges).toContainEqual({
-      scenario_id: world.scenarioId,
-      cost: expect.any(Number),
-      pu_id: sameCostId,
+      spud_id: expect.any(String),
     });
   });
 });
diff --git a/api/apps/geoprocessing/test/integration/cost-surface/planning-unit-fixtures.ts b/api/apps/geoprocessing/test/integration/cost-surface/planning-unit-fixtures.ts
@@ -9,6 +9,7 @@ import {
 } from '@marxan/scenarios-planning-unit';
 
 import { GivenScenarioPuDataExists } from '../../steps/given-scenario-pu-data-exists';
+import { isDefined } from '@marxan/utils';
 
 export const getFixtures = async (app: INestApplication) => {
   const scenarioId = v4();
@@ -35,9 +36,9 @@ export const getFixtures = async (app: INestApplication) => {
     scenarioPlanningUnitsGeometry: scenarioPuData,
     GetPuCostsData: async (
       scenarioId: string,
-    ): Promise<{ scenario_id: string; cost: number; pu_id: string }[]> =>
+    ): Promise<{ scenario_id: string; cost: number; spud_id: string }[]> =>
       puCostDataRepo.query(`
-      select spud.scenario_id, spucd."cost", spud.id as pu_id
+      select spud.scenario_id, spucd."cost", spud.id as spud_id
       from scenarios_pu_data as spud
       join scenarios_pu_cost_data as spucd on (spud."id" = spucd.scenarios_pu_data_id)
       where spud.scenario_id = '${scenarioId}'
@@ -53,7 +54,12 @@ export const getFixtures = async (app: INestApplication) => {
             }),
           ),
         )
-        .then((rows) => rows.map((row) => row.scenariosPlanningUnit)),
+        .then((rows) => rows.map((row) => row.scenariosPlanningUnit))
+        .then((scenarioPlanningUnits) =>
+          scenarioPlanningUnits
+            .map((spu) => spu?.puGeometryId)
+            .filter(isDefined),
+        ),
     cleanup: async () => {
       await puDataRepo.delete({
         scenarioId,