From e20b44c6f9d6c907c76cc4fd7f69f069e7f729f9 Mon Sep 17 00:00:00 2001 From: nmrgt Date: Mon, 21 Oct 2024 14:49:40 +0000 Subject: [PATCH] Fix: error terms selector by operator_id (#2667) * wip * fix: use a scoped by operator_id detection * remove console.log * fix: integration test --------- Co-authored-by: Jonathan Fallon --- ...poolAcquisitionService.integration.spec.ts | 38 +++++----- .../providers/CarpoolAcquisitionService.ts | 45 +++++------- ...arpoolLookupRepository.integration.spec.ts | 39 +++++----- .../repositories/CarpoolLookupRepository.ts | 72 ++++++++++--------- 4 files changed, 101 insertions(+), 93 deletions(-) diff --git a/api/src/pdc/providers/carpool/providers/CarpoolAcquisitionService.integration.spec.ts b/api/src/pdc/providers/carpool/providers/CarpoolAcquisitionService.integration.spec.ts index 36f609c767..f96721253b 100644 --- a/api/src/pdc/providers/carpool/providers/CarpoolAcquisitionService.integration.spec.ts +++ b/api/src/pdc/providers/carpool/providers/CarpoolAcquisitionService.integration.spec.ts @@ -15,10 +15,7 @@ import { import sql, { raw } from "@/lib/pg/sql.ts"; import { GeoProvider } from "@/pdc/providers/geo/index.ts"; import { DbContext, makeDbBeforeAfter } from "@/pdc/providers/test/index.ts"; -import { - insertableCarpool, - updatableCarpool, -} from "../mocks/database/carpool.ts"; +import { insertableCarpool, updatableCarpool } from "../mocks/database/carpool.ts"; import { CarpoolGeoRepository } from "../repositories/CarpoolGeoRepository.ts"; import { CarpoolLookupRepository } from "../repositories/CarpoolLookupRepository.ts"; import { CarpoolRepository } from "../repositories/CarpoolRepository.ts"; @@ -200,9 +197,7 @@ describe("CarpoolAcquistionService", () => { ...insertableCarpool, operator_journey_id: "operator_journey_id_2", }; - await assertRejects(async () => - await service.registerRequest({ ...data, api_version: 3 }) - ); + await assertRejects(async () => await service.registerRequest({ ...data, api_version: 3 })); assert(carpoolRepositoryL.register.calledOnce); assert(requestRepositoryL.save.calledOnce); @@ -224,6 +219,7 @@ describe("CarpoolAcquistionService", () => { }); const data = { + operator_id: 1, created_at: new Date("2024-01-01T05:00:00.000Z"), distance: 4_000, driver_identity_key: "key_driver", @@ -242,24 +238,30 @@ describe("CarpoolAcquistionService", () => { carpoolL.countJourneyBy.getCalls().map((c: any) => c.args), [ [ - ["key_driver", "key_passenger"], { - max: new Date("2024-01-01T22:59:59.999Z"), - min: new Date("2023-12-31T23:00:00.000Z"), + identity_key: ["key_driver", "key_passenger"], + start_date: { + max: new Date("2024-01-01T22:59:59.999Z"), + min: new Date("2023-12-31T23:00:00.000Z"), + }, + operator_id: 1, }, undefined, - undefined, - undefined, ], [ - ["key_driver", "key_passenger"], - { - max: new Date("2024-01-01T04:30:00.000Z"), - }, { - min: new Date("2024-01-01T01:30:00.000Z"), + identity_key: ["key_driver", "key_passenger"], + start_date: { + min: new Date("2024-01-01T02:00:00.000Z"), + max: new Date("2024-01-01T04:30:00.000Z"), + }, + end_date: { + min: new Date("2024-01-01T01:30:00.000Z"), + max: new Date("2024-01-01T04:00:00.000Z"), + }, + operator_trip_id: "operator_trip_id", + operator_id: 1, }, - "operator_trip_id", undefined, ], ], diff --git a/api/src/pdc/providers/carpool/providers/CarpoolAcquisitionService.ts b/api/src/pdc/providers/carpool/providers/CarpoolAcquisitionService.ts index 8442443cce..42c6609ff3 100644 --- a/api/src/pdc/providers/carpool/providers/CarpoolAcquisitionService.ts +++ b/api/src/pdc/providers/carpool/providers/CarpoolAcquisitionService.ts @@ -1,19 +1,11 @@ import { NotFoundException, provider } from "@/ilos/common/index.ts"; -import { - PoolClient, - PostgresConnection, -} from "@/ilos/connection-postgres/index.ts"; +import { PoolClient, PostgresConnection } from "@/ilos/connection-postgres/index.ts"; import { addMinutes, differenceInHours } from "@/lib/date/index.ts"; import { env_or_false } from "@/lib/env/index.ts"; import { logger } from "@/lib/logger/index.ts"; import { endOfDay, startOfDay } from "@/pdc/helpers/dates.helper.ts"; import { GeoProvider } from "@/pdc/providers/geo/index.ts"; -import { - CancelRequest, - CarpoolAcquisitionStatusEnum, - RegisterRequest, - UpdateRequest, -} from "../interfaces/index.ts"; +import { CancelRequest, CarpoolAcquisitionStatusEnum, RegisterRequest, UpdateRequest } from "../interfaces/index.ts"; import { CarpoolGeoRepository } from "../repositories/CarpoolGeoRepository.ts"; import { CarpoolLookupRepository } from "../repositories/CarpoolLookupRepository.ts"; import { CarpoolRepository } from "../repositories/CarpoolRepository.ts"; @@ -34,6 +26,7 @@ export class CarpoolAcquisitionService { ) {} public async verifyTermsViolation(data: { + operator_id: number; created_at: Date; distance: number; driver_identity_key: string; @@ -51,31 +44,31 @@ export class CarpoolAcquisitionService { if (data.distance < 2_000) { result.push("distance_too_short"); } + // This select all distinct operator_trip_id that started in the same day // with the same identity key as any role - const journeyCount = await this.lookupRepository.countJourneyBy( - [data.driver_identity_key, data.passenger_identity_key], - { + const journeyCount = await this.lookupRepository.countJourneyBy({ + operator_id: data.operator_id, + identity_key: [data.driver_identity_key, data.passenger_identity_key], + start_date: { min: startOfDay(data.start_datetime), max: endOfDay(data.start_datetime), }, - undefined, - undefined, - client, - ); + }, client); if (journeyCount >= 4) { result.push("too_many_trips_by_day"); } + // This select all distinct operator_trip_id that started before // 30 minutes after the end of the current trip OR ended after // 30 minutes before the start of the current trip - const journeyCloseCount = await this.lookupRepository.countJourneyBy( - [data.driver_identity_key, data.passenger_identity_key], - { max: addMinutes(data.end_datetime, 30) }, - { min: addMinutes(data.start_datetime, -30) }, - data.operator_trip_id, - client, - ); + const journeyCloseCount = await this.lookupRepository.countJourneyBy({ + operator_id: data.operator_id, + identity_key: [data.driver_identity_key, data.passenger_identity_key], + start_date: { min: data.start_datetime, max: addMinutes(data.end_datetime, 30) }, + end_date: { min: addMinutes(data.start_datetime, -30), max: data.end_datetime }, + operator_trip_id: data.operator_trip_id, + }, client); if (journeyCloseCount >= 1) { result.push("too_close_trips"); } @@ -115,6 +108,7 @@ export class CarpoolAcquisitionService { ); } else { terms_violation_error_labels = await this.verifyTermsViolation({ + operator_id: data.operator_id, created_at: carpool.created_at, distance: data.distance, driver_identity_key: data.driver_identity_key, @@ -161,8 +155,7 @@ export class CarpoolAcquisitionService { const conn = await this.connection.getClient().connect(); await conn.query("BEGIN"); try { - const { api_version, operator_id, operator_journey_id, ...carpoolData } = - data; + const { api_version, operator_id, operator_journey_id, ...carpoolData } = data; const carpool = await this.carpoolRepository.update( operator_id, operator_journey_id, diff --git a/api/src/pdc/providers/carpool/repositories/CarpoolLookupRepository.integration.spec.ts b/api/src/pdc/providers/carpool/repositories/CarpoolLookupRepository.integration.spec.ts index a46bc67179..aa3ad65393 100644 --- a/api/src/pdc/providers/carpool/repositories/CarpoolLookupRepository.integration.spec.ts +++ b/api/src/pdc/providers/carpool/repositories/CarpoolLookupRepository.integration.spec.ts @@ -1,11 +1,4 @@ -import { - afterAll, - assertEquals, - assertObjectMatch, - beforeAll, - describe, - it, -} from "@/dev_deps.ts"; +import { afterAll, assertEquals, assertObjectMatch, beforeAll, describe, it } from "@/dev_deps.ts"; import { DbContext, makeDbBeforeAfter } from "@/pdc/providers/test/index.ts"; import { Id } from "../interfaces/index.ts"; import { insertableCarpool } from "../mocks/database/carpool.ts"; @@ -72,17 +65,29 @@ describe("CarpoolGeoRepository", () => { it("should count the carpool by start/end date", async () => { const data = { ...insertableCarpool }; - const count = await repository.countJourneyBy([data.driver_identity_key], { - min: new Date(data.start_datetime.valueOf() - 1000), - max: new Date(data.start_datetime.valueOf() + 1000), - }, {}); + const count = await repository.countJourneyBy({ + operator_id: data.operator_id, + identity_key: [data.driver_identity_key], + start_date: { + min: new Date(data.start_datetime.valueOf() - 1000), + max: new Date(data.start_datetime.valueOf() + 1000), + }, + }); assertEquals(count, 1); - const count2 = await repository.countJourneyBy([data.driver_identity_key], { - min: new Date(data.start_datetime.valueOf() - 1000), - }, { - max: new Date(data.end_datetime.valueOf() + 1000), - }, `${data.operator_trip_id}-diff`); + const count2 = await repository.countJourneyBy({ + operator_id: data.operator_id, + identity_key: [data.driver_identity_key], + start_date: { + min: new Date(data.start_datetime.valueOf() - 1000), + max: new Date(data.start_datetime.valueOf()), + }, + end_date: { + min: new Date(data.end_datetime.valueOf()), + max: new Date(data.end_datetime.valueOf() + 1000), + }, + operator_trip_id: `${data.operator_trip_id}-diff`, + }); assertEquals(count2, 1); }); }); diff --git a/api/src/pdc/providers/carpool/repositories/CarpoolLookupRepository.ts b/api/src/pdc/providers/carpool/repositories/CarpoolLookupRepository.ts index cd73d8f8ba..68f8461f1d 100644 --- a/api/src/pdc/providers/carpool/repositories/CarpoolLookupRepository.ts +++ b/api/src/pdc/providers/carpool/repositories/CarpoolLookupRepository.ts @@ -1,13 +1,7 @@ import { provider } from "@/ilos/common/index.ts"; -import { - PoolClient, - PostgresConnection, -} from "@/ilos/connection-postgres/index.ts"; +import { PoolClient, PostgresConnection } from "@/ilos/connection-postgres/index.ts"; import sql, { join, raw } from "@/lib/pg/sql.ts"; -import { - SelectableCarpool, - SelectableCarpoolStatus, -} from "../interfaces/database/lookup.ts"; +import { SelectableCarpool, SelectableCarpoolStatus } from "../interfaces/database/lookup.ts"; import { Id, Uuid } from "../interfaces/index.ts"; @provider() @@ -18,55 +12,69 @@ export class CarpoolLookupRepository { constructor(protected connection: PostgresConnection) {} - public async countJourneyBy( - identity_key: string[], + public async countJourneyBy(selectors: { + identity_key: string[]; start_date?: { min?: Date; max?: Date; - }, + }; end_date?: { min?: Date; max?: Date; - }, - operator_trip_id?: string, - client?: PoolClient, - ): Promise { + }; + operator_trip_id?: string; + operator_id?: number; + }, client?: PoolClient): Promise { const cl = client ?? this.connection.getClient(); const filters = [ sql`(${ join([ - sql`cc.driver_identity_key = ANY(${identity_key})`, - sql`cc.passenger_identity_key = ANY(${identity_key})`, + sql`cc.driver_identity_key = ANY(${selectors.identity_key})`, + sql`cc.passenger_identity_key = ANY(${selectors.identity_key})`, ], " OR ") })`, sql`cs.acquisition_status <> ANY('{canceled,expired,terms_violation_error}'::carpool_v2.carpool_acquisition_status_enum[]) `, ]; + if (selectors.operator_id) { + filters.push(sql`cc.operator_id = ${selectors.operator_id}`); + } + + const start_date_filters = []; + const end_date_filters = []; - if (start_date) { - if (start_date.max) { - filters.push(sql`cc.start_datetime < ${start_date.max}`); + if (selectors.start_date) { + if (selectors.start_date.max) { + start_date_filters.push(sql`cc.start_datetime < ${selectors.start_date.max}`); } - if (start_date.min) { - filters.push(sql`cc.start_datetime >= ${start_date.min}`); + if (selectors.start_date.min) { + start_date_filters.push(sql`cc.start_datetime >= ${selectors.start_date.min}`); } } - if (end_date) { - if (end_date.max) { - filters.push(sql`cc.end_datetime < ${end_date.max}`); + if (selectors.end_date) { + if (selectors.end_date.max) { + end_date_filters.push(sql`cc.end_datetime < ${selectors.end_date.max}`); } - if (end_date.min) { - filters.push(sql`cc.end_datetime >= ${end_date.min}`); + if (selectors.end_date.min) { + end_date_filters.push(sql`cc.end_datetime >= ${selectors.end_date.min}`); } } - if (operator_trip_id) { - filters.push(sql`cc.operator_trip_id <> ${operator_trip_id}`); + if (start_date_filters.length && end_date_filters.length) { + filters.push(sql`(${ + join([ + sql`(${join(start_date_filters, " AND ")})`, + sql`(${join(end_date_filters, " AND ")})`, + ], " OR ") + })`); + } else if (start_date_filters.length) { + filters.push(...start_date_filters); + } + + if (selectors.operator_trip_id) { + filters.push(sql`cc.operator_trip_id <> ${selectors.operator_trip_id}`); } - // operator_trip_id should be unique - // it may occurs some conflict between operators - // maybe use a composite count distinct const sqlQuery = sql` SELECT count(distinct cc.operator_trip_id)