Improve support for trips that span more than one date

- Remove the where clause on schedule times, as it is not possible to compute a sensible driver date within the SQL query.
- Simplify the date clause comparisons
- Replace the compared date range with one adjusted with the driver duration, to catch ads that depart on a different day

src/modules/ad/core/application/queries/match/selector/passenger-oriented.selector.ts

@@ -1,16 +1,19 @@
 import { AdEntity } from '@modules/ad/core/domain/ad.entity';
 import { Frequency, Role } from '@modules/ad/core/domain/ad.types';
 import { CandidateEntity } from '@modules/ad/core/domain/candidate.entity';
-import { ScheduleItem } from '@modules/ad/core/domain/value-objects/schedule-item.value-object';
 import { Point } from '../../../types/point.type';
 import { Waypoint } from '../../../types/waypoint.type';
 import { Selector } from '../algorithm.abstract';
+import { ScheduleItem } from '../match.query';
 
 /**
  * This class complements the AdRepository prisma service by turning a match query object into a SQL query,
- * with the assumption that the query is passenger-oriented (i.e. it is up to the driver to go out of his way to pick up the passenger)
+ * with the assumption that the query is passenger-oriented (i.e. it is up to the driver to go out of his way to pick up the passenger).
- * TODO: Converting the query object into a SQL query is a job for the prisma service / repository implementation,
+ * The idea is to make a rough filter of the ads in DB to limit the number of ads to be processed more precisely by the application code.
- *       any logic related to being passenger-oriented should be in the domain layer
+ * TODO: Converting the query object into a SQL query is a job for the repository implementation
+ *       (or anything behind the repository interface),
+ *       any logic related to being passenger-oriented should be in the domain layer.
+ *       (though it might be difficult to describe generically the search criteria with a query object)
  */
 export class PassengerOrientedSelector extends Selector {
   select = async (): Promise<CandidateEntity[]> => {
@@ -25,6 +28,7 @@ export class PassengerOrientedSelector extends Selector {
         query: this._createQueryString(Role.PASSENGER),
         role: Role.PASSENGER,
       });
+
     return (
       await Promise.all(
         queryStringRoles.map<Promise<AdsRole>>(
@@ -140,8 +144,7 @@ export class PassengerOrientedSelector extends Selector {
     [
       this._whereRole(role),
       this._whereStrict(),
-      this._whereDate(),
+      this._whereDate(role),
-      this._whereSchedule(role),
       this._whereExcludedAd(),
       this._whereAzimuth(),
       this._whereProportion(role),
@@ -160,110 +163,71 @@ export class PassengerOrientedSelector extends Selector {
         : `frequency='${Frequency.RECURRENT}'`
       : '';
 
-  private _whereDate = (): string =>
+  /**
-    this.query.frequency == Frequency.PUNCTUAL
+   * Generates the WHERE clause checking that the date range of the query intersects with the range of the ad.
-      ? `("fromDate" <= '${this.query.fromDate}' AND "toDate" >= '${this.query.fromDate}')`
+   * Note that driver dates might not be comparable with passenger dates when the trip is by night or very long.
-      : `(\
+   * For this reason, the pickup date is adjusted with the driver duration,
-      (\
+   * so as to compare with the maximum / minimum driver date that could make sense for the passenger.
-        "fromDate" <= '${this.query.fromDate}' AND "fromDate" <= '${this.query.toDate}' AND\
+   * This may return more ads than necessary, but they will be filtered out in further processing.
-        "toDate" >= '${this.query.toDate}' AND "toDate" >= '${this.query.fromDate}'\
+   */
-      ) OR (\
+  private _whereDate = (role: Role): string => {
-        "fromDate" >= '${this.query.fromDate}' AND "fromDate" <= '${this.query.toDate}' AND\
+    const maxFromDate = this._maxFromDate(role);
-        "toDate" <= '${this.query.toDate}' AND "toDate" >= '${this.query.fromDate}'\
+    const minToDate = this._minToDate(role);
-      ) OR (\
+    return `("fromDate" <= ${maxFromDate} AND "toDate" >= ${minToDate})`;
-        "fromDate" <= '${this.query.fromDate}' AND "fromDate" <= '${this.query.toDate}' AND\
+  };
-        "toDate" <= '${this.query.toDate}' AND "toDate" >= '${this.query.fromDate}'\
-      ) OR (\
-        "fromDate" >= '${this.query.fromDate}' AND "fromDate" <= '${this.query.toDate}' AND\
-        "toDate" >= '${this.query.toDate}' AND "toDate" >= '${this.query.fromDate}'\
-      )\
-    )`;
 
-  private _whereSchedule = (role: Role): string => {
+  private _maxFromDate = (role: Role): string => {
-    // no schedule filtering if schedule is not set
+    if (role == Role.DRIVER) {
-    if (this.query.schedule === undefined) return '';
+      const querySchedule = this.query.schedule;
-    const schedule: string[] = [];
+      // When there is no schedule (search whole day), we consider the driver accepts to depart until 23:59
-    // we need full dates to compare times, because margins can lead to compare on previous or next day
+      const maxScheduleTime =
-    // - first we establish a base calendar (up to a week)
+        querySchedule === undefined
-    const scheduleDates: Date[] = this._datesBetweenBoundaries(
+          ? '23:59'
-      this.query.fromDate,
+          : querySchedule.reduce(
-      this.query.toDate,
+              (max, s) => (s.time > max ? s.time : max),
-    );
+              '00:00',
-    // - then we compare each resulting day of the schedule with each day of calendar,
+            );
-    //  adding / removing margin depending on the role
+      const [h, m] = maxScheduleTime.split(':');
-    scheduleDates.map((date: Date) => {
+      const maxFromDate = new Date(this.query.toDate);
-      (this.query.schedule as ScheduleItem[])
+      maxFromDate.setHours(parseInt(h));
-        .filter(
+      maxFromDate.setMinutes(parseInt(m));
-          (scheduleItem: ScheduleItem) => date.getUTCDay() == scheduleItem.day,
+      maxFromDate.setSeconds(this.query.driverRoute!.duration);
-        )
+      return `'${maxFromDate.getUTCFullYear()}-${maxFromDate.getUTCMonth() + 1}-${maxFromDate.getUTCDate()}'`;
-        .map((scheduleItem: ScheduleItem) => {
+    } else {
-          switch (role) {
+      return `'${this.query.toDate}'`;
-            case Role.PASSENGER:
+    }
-              schedule.push(this._wherePassengerSchedule(date, scheduleItem));
+  };
-              break;
+
-            case Role.DRIVER:
+  private _minToDate = (role: Role): string => {
-              schedule.push(this._whereDriverSchedule(date, scheduleItem));
+    if (role == Role.PASSENGER) {
-              break;
+      const querySchedule = this.query.schedule;
-          }
+      // When there is no schedule (search whole day), we consider the passenger accepts to depart from 00:00
-        });
+      const minScheduleTime =
-    });
+        querySchedule === undefined
-    if (schedule.length > 0) {
+          ? '00:00'
-      return ['(', schedule.join(' OR '), ')'].join('');
+          : querySchedule.reduce(
+              (min, s) => (s.time < min ? s.time : min),
+              '23:59',
+            );
+      const [h, m] = minScheduleTime.split(':');
+      const minToDate = new Date(this.query.fromDate);
+      minToDate.setHours(parseInt(h));
+      minToDate.setMinutes(parseInt(m));
+      return `(make_timestamp(\
+        ${minToDate.getUTCFullYear()},\
+        ${minToDate.getUTCMonth() + 1},\
+        ${minToDate.getUTCDate()},\
+        ${minToDate.getUTCHours()},\
+        ${minToDate.getUTCMinutes()},0)\
+        - concat("driverDuration", ' second')::interval)::date`;
+    } else {
+      return `'${this.query.fromDate}'`;
     }
-    return '';
   };
 
   private _whereExcludedAd = (): string =>
     this.query.excludedAdId ? `ad.uuid <> '${this.query.excludedAdId}'` : '';
 
-  private _wherePassengerSchedule = (
-    date: Date,
-    scheduleItem: ScheduleItem,
-  ): string => {
-    let maxDepartureDatetime: Date = new Date(date);
-    maxDepartureDatetime.setHours(parseInt(scheduleItem.time.split(':')[0]));
-    maxDepartureDatetime.setMinutes(parseInt(scheduleItem.time.split(':')[1]));
-    maxDepartureDatetime = this._addMargin(
-      maxDepartureDatetime,
-      scheduleItem.margin as number,
-    );
-    // we want the min departure time of the driver to be before the max departure time of the passenger
-    return `make_timestamp(\
-                ${maxDepartureDatetime.getUTCFullYear()},\
-                ${maxDepartureDatetime.getUTCMonth() + 1},\
-                ${maxDepartureDatetime.getUTCDate()},\
-                CAST(EXTRACT(hour from time) as integer),\
-                CAST(EXTRACT(minute from time) as integer),0) - interval '1 second' * margin <=\
-                make_timestamp(\
-                  ${maxDepartureDatetime.getUTCFullYear()},\
-                  ${maxDepartureDatetime.getUTCMonth() + 1},\
-                  ${maxDepartureDatetime.getUTCDate()},${maxDepartureDatetime.getUTCHours()},${maxDepartureDatetime.getUTCMinutes()},0)`;
-  };
-
-  private _whereDriverSchedule = (
-    date: Date,
-    scheduleItem: ScheduleItem,
-  ): string => {
-    let minDepartureDatetime: Date = new Date(date);
-    minDepartureDatetime.setHours(parseInt(scheduleItem.time.split(':')[0]));
-    minDepartureDatetime.setMinutes(parseInt(scheduleItem.time.split(':')[1]));
-    minDepartureDatetime = this._addMargin(
-      minDepartureDatetime,
-      -(scheduleItem.margin as number),
-    );
-    // we want the max departure time of the passenger to be after the min departure time of the driver
-    return `make_timestamp(\
-                ${minDepartureDatetime.getUTCFullYear()},
-                ${minDepartureDatetime.getUTCMonth() + 1},
-                ${minDepartureDatetime.getUTCDate()},\
-                CAST(EXTRACT(hour from time) as integer),\
-                CAST(EXTRACT(minute from time) as integer),0) + interval '1 second' * margin >=\
-                make_timestamp(\
-                  ${minDepartureDatetime.getUTCFullYear()},
-                  ${minDepartureDatetime.getUTCMonth() + 1},
-                  ${minDepartureDatetime.getUTCDate()},${minDepartureDatetime.getUTCHours()},${minDepartureDatetime.getUTCMinutes()},0)`;
-  };
-
   private _whereAzimuth = (): string => {
     if (!this.query.useAzimuth) return '';
     const { minAzimuth, maxAzimuth } = this._azimuthRange(
@@ -323,37 +287,6 @@ export class PassengerOrientedSelector extends Selector {
     }
   };
 
-  /**
-   * Returns an array of dates containing all the dates (limited to 7 by default) between 2 boundary dates.
-   *
-   * The array length can be limited to a _max_ number of dates (default: 7)
-   */
-  private _datesBetweenBoundaries = (
-    firstDate: string,
-    lastDate: string,
-    max = 7,
-  ): Date[] => {
-    const fromDate: Date = new Date(firstDate);
-    const toDate: Date = new Date(lastDate);
-    const dates: Date[] = [];
-    let count = 0;
-    for (
-      let date = fromDate;
-      date <= toDate;
-      date.setUTCDate(date.getUTCDate() + 1)
-    ) {
-      dates.push(new Date(date));
-      count++;
-      if (count == max) break;
-    }
-    return dates;
-  };
-
-  private _addMargin = (date: Date, marginInSeconds: number): Date => {
-    date.setUTCSeconds(marginInSeconds);
-    return date;
-  };
-
   private _azimuthRange = (
     azimuth: number,
     margin: number,

src/modules/ad/core/domain/candidate.entity.ts

@@ -353,7 +353,7 @@ class Schedule extends ValueObject<{
           duration,
         );
         acc.push({
-          day: itemDate.getUTCDay(),
+          day: driverStartDatetime.getUTCDay(),
           margin: scheduleItemProps.margin,
           time: this._formatTime(driverStartDatetime),
         });

src/modules/ad/tests/unit/core/match.query-handler.spec.ts

@@ -1,8 +1,8 @@
 import {
-  Domain,
-  KeyType,
   Configurator,
+  Domain,
   GetConfigurationRepositoryPort,
+  KeyType,
 } from '@mobicoop/configuration-module';
 import {
   CARPOOL_CONFIG_DEPARTURE_TIME_MARGIN,
@@ -16,8 +16,9 @@ import {
   AD_REPOSITORY,
   INPUT_DATETIME_TRANSFORMER,
   MATCHING_REPOSITORY,
+  TIMEZONE_FINDER,
+  TIME_CONVERTER,
 } from '@modules/ad/ad.di-tokens';
-import { DateTimeTransformerPort } from '@modules/ad/core/application/ports/datetime-transformer.port';
 import { MatchingRepositoryPort } from '@modules/ad/core/application/ports/matching.repository.port';
 import { MatchQuery } from '@modules/ad/core/application/queries/match/match.query';
 import {
@@ -30,6 +31,9 @@ import { Frequency, Role } from '@modules/ad/core/domain/ad.types';
 import { Target } from '@modules/ad/core/domain/candidate.types';
 import { MatchEntity } from '@modules/ad/core/domain/match.entity';
 import { MatchingEntity } from '@modules/ad/core/domain/matching.entity';
+import { InputDateTimeTransformer } from '@modules/ad/infrastructure/input-datetime-transformer';
+import { TimeConverter } from '@modules/ad/infrastructure/time-converter';
+import { TimezoneFinder } from '@modules/ad/infrastructure/timezone-finder';
 import {
   MATCH_CONFIG_ALGORITHM,
   MATCH_CONFIG_AZIMUTH_MARGIN,
@@ -344,13 +348,6 @@ const mockConfigurationRepository: GetConfigurationRepositoryPort = {
   ),
 };
 
-const mockInputDateTimeTransformer: DateTimeTransformerPort = {
-  fromDate: jest.fn(),
-  toDate: jest.fn(),
-  day: jest.fn(),
-  time: jest.fn(),
-};
-
 const mockRouteProvider = simpleMockGeorouter;
 
 describe('Match Query Handler', () => {
@@ -372,9 +369,17 @@ describe('Match Query Handler', () => {
           provide: AD_CONFIGURATION_REPOSITORY,
           useValue: mockConfigurationRepository,
         },
+        {
+          provide: TIMEZONE_FINDER,
+          useClass: TimezoneFinder,
+        },
+        {
+          provide: TIME_CONVERTER,
+          useClass: TimeConverter,
+        },
         {
           provide: INPUT_DATETIME_TRANSFORMER,
-          useValue: mockInputDateTimeTransformer,
+          useClass: InputDateTimeTransformer,
         },
       ],
     }).compile();