refactor(match): address previous PR comments

package.json

@@ -16,7 +16,7 @@
   "license": "ISC",
   "dependencies": {
     "@boklisten/bl-email": "^1.8.1",
-    "@boklisten/bl-model": "^0.25.41",
+    "@boklisten/bl-model": "^0.25.42",
     "@boklisten/bl-post-office": "^0.5.56",
     "@napi-rs/image": "^1.8.0",
     "@sendgrid/mail": "^7.7.0",

src/collections/match/helpers/match-finder-2/match-finder.spec.ts

@@ -22,7 +22,7 @@ import {
   groupMatchesByUser,
   seededRandom,
   shuffler,
-  createMatchableUsersWithIdPrefix,
+  createMatchableUsersWithIdSuffix,
 } from "./match-testing-utils";
 
 chai.use(chaiAsPromised);
@@ -311,8 +311,8 @@ describe("Large User Groups", () => {
   it("can sufficiently match realistic user data with itself", () => {
     const shuffle = shuffler(seededRandom(12345));
     const rawData = ullern_test_users;
-    const test_senders = createMatchableUsersWithIdPrefix(rawData, "_sender");
-    const test_receivers = createMatchableUsersWithIdPrefix(
+    const test_senders = createMatchableUsersWithIdSuffix(rawData, "_sender");
+    const test_receivers = createMatchableUsersWithIdSuffix(
       rawData,
       "_receiver",
     );
@@ -337,8 +337,8 @@ describe("Large User Groups", () => {
   it("can sufficiently match realistic user data with a modified version of itself", () => {
     const shuffle = shuffler(seededRandom(123454332));
     const rawData = ullern_test_users;
-    const test_senders = createMatchableUsersWithIdPrefix(rawData, "_sender");
-    const test_receivers = createMatchableUsersWithIdPrefix(
+    const test_senders = createMatchableUsersWithIdSuffix(rawData, "_sender");
+    const test_receivers = createMatchableUsersWithIdSuffix(
       rawData,
       "_receiver",
     );

src/collections/match/helpers/match-finder-2/match-finder.ts

@@ -1,3 +1,5 @@
+import { BlError } from "@boklisten/bl-model";
+
 import {
   MatchableUser,
   CandidateMatch,
@@ -43,22 +45,16 @@ export class MatchFinder {
    */
   public generateMatches() {
     // First remove the perfect matches
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
     this.createMatches(tryFindTwoWayMatch, this.senders);
 
     // Fulfill the largest possible senders with the best receivers
     sortUsersNumberOfItemsDescending(this.senders);
     sortUsersNumberOfItemsDescending(this.receivers);
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
     this.createMatches(tryFindOneWayMatch, this.senders);
 
     // Remove all unmatchable items
     this.standMatchUnmatchableItems();
     // We might have opened up for some TwoWay matches after purging the unmatchable items
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
     this.createMatches(tryFindTwoWayMatch, this.senders);
     // Edge case, but removing TwoWay matches might make some more items unmatchable
     this.standMatchUnmatchableItems();
@@ -76,8 +72,6 @@ export class MatchFinder {
 
     for (const sortedSenderGroup of sortedSenderGroups) {
       this.createMatches(
-        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-        // @ts-ignore
         tryFindPartialMatch,
         sortedSenderGroup,
         sortedSenderGroups,
@@ -101,7 +95,7 @@ export class MatchFinder {
     this.receivers = removeFullyMatchedUsers(this.receivers);
 
     if (this.senders.length > 0 || this.receivers.length > 0) {
-      throw new Error("Some senders or receivers did not receive a match!");
+      throw new BlError("Some senders or receivers did not receive a match!");
     }
 
     let originalSenders = copyUsers(this._senders);
@@ -116,14 +110,12 @@ export class MatchFinder {
         match.variant === CandidateMatchVariant.StandMatch
           ? match.handoffItems
           : match.items;
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore
-      const sender: MatchableUser = originalSenders.find(
-        (sender) => sender.id === senderId,
-      );
+      const sender = originalSenders.find((sender) => sender.id === senderId);
       if (sender) {
         if (hasDifference(intersect(sentItems, sender.items), sentItems)) {
-          throw new Error("Sender cannot give away more books than they have!");
+          throw new BlError(
+            "Sender cannot give away more books than they have!",
+          );
         }
         sender.items = difference(sender.items, sentItems);
         originalSenders = removeFullyMatchedUsers(originalSenders);
@@ -137,31 +129,31 @@ export class MatchFinder {
         match.variant === CandidateMatchVariant.StandMatch
           ? match.pickupItems
           : match.items;
-      // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-      // @ts-ignore
-      const receiver: MatchableUser = originalReceivers.find(
+      const receiver = originalReceivers.find(
         (receiver) => receiver.id === receiverId,
       );
       if (receiver) {
         if (
           hasDifference(intersect(receivedItems, receiver.items), receivedItems)
         ) {
-          throw new Error("Receiver cannot receive more books than they want!");
+          throw new BlError(
+            "Receiver cannot receive more books than they want!",
+          );
         }
         receiver.items = difference(receiver.items, receivedItems);
         originalReceivers = removeFullyMatchedUsers(originalReceivers);
       }
 
       if (
-        senderId === receiverId &&
-        match.variant === CandidateMatchVariant.UserMatch
+        match.variant === CandidateMatchVariant.UserMatch &&
+        senderId === receiverId
       ) {
-        throw new Error("Receiver and sender cannot be the same person");
+        throw new BlError("Receiver and sender cannot be the same person");
       }
     }
 
     if (originalSenders.length > 0 || originalReceivers.length > 0) {
-      throw new Error("Some senders or receivers did not get fulfilled");
+      throw new BlError("Some senders or receivers did not get fulfilled");
     }
   }
 
@@ -274,7 +266,7 @@ export class MatchFinder {
     matchFinder: (
       sender: MatchableUser,
       receivers: MatchableUser[],
-    ) => MatchableUser,
+    ) => MatchableUser | null,
     senders: MatchableUser[],
     sortedSenderGroups?: MatchableUser[][],
   ) {

src/collections/match/helpers/match-finder-2/match-testing-utils.ts

@@ -67,12 +67,12 @@ export function createFakeMatchableUser(
 }
 
 export function createUserGroup(
-  idPrefix: string,
+  idSuffix: string,
   size: number,
   ...items: string[]
 ): MatchableUser[] {
   return [...Array(size)].map((_, id) =>
-    createFakeMatchableUser(idPrefix + id, ...items),
+    createFakeMatchableUser(id + idSuffix, ...items),
   );
 }
 
@@ -115,12 +115,12 @@ export const shuffler =
     return list;
   };
 
-export function createMatchableUsersWithIdPrefix(
+export function createMatchableUsersWithIdSuffix(
   rawData: { id: string; items: { $numberLong: string }[] }[],
-  idPrefix: string,
+  idSuffix: string,
 ): MatchableUser[] {
   return rawData.map(({ id, items }) => ({
-    id: id + idPrefix,
+    id: id + idSuffix,
     items: new Set(items.map((item) => item["$numberLong"])),
   }));
 }

src/collections/match/helpers/match-finder-2/match-utils.ts

@@ -187,11 +187,9 @@ export function countItemOccurrences(users: MatchableUser[]): {
   return users
     .flatMap((user) => Array.from(user.items))
     .reduce(
-      (acc, next) => ({
+      (acc: { [item: string]: number }, next) => ({
         ...acc,
-        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-        // @ts-ignore
-        [next]: next in acc ? acc[next] + 1 : 1,
+        [next]: acc[next] ? acc[next] + 1 : 1,
       }),
       {},
     );

src/collections/match/match.schema.ts

@@ -62,17 +62,26 @@ const userMatchSchema = {
   },
   // unique items owned by sender which have been given to anyone. May differ from receivedBlIds
   // when a book is borrowed and handed over to someone other than the technical owner's match
-  deliveredBlIds: [String],
+  deliveredBlIds: {
+    type: [String],
+    default: undefined,
+  },
   // unique items which have been received by the receiver from anyone
-  receivedBlIds: [String],
+  receivedBlIds: {
+    type: [String],
+    default: undefined,
+  },
   // if true, disallow handing the items out or in at a stand, only allow match exchange
   itemsLockedToMatch: {
     type: Boolean,
     default: undefined,
   },
   // if receiver items have overrides, the generated customer items will
   // get the deadline specified in the override instead of using the branch defined deadline
-  deadlineOverrides: [{ item: String, deadline: Date }],
+  deadlineOverrides: {
+    type: Map,
+    of: String,
+  },
 };
 
 /** @see StandMatch */

src/collections/match/operations/match-generate-operation-helper.ts

@@ -7,7 +7,7 @@ import {
 } from "@boklisten/bl-model";
 import { ObjectId } from "mongodb";
 
-import { isBoolean } from "../../../helper/typescript-helpers";
+import { isBoolean, isNotNullish } from "../../../helper/typescript-helpers";
 import { BlDocumentStorage } from "../../../storage/blDocumentStorage";
 import {
   CandidateMatchVariant,
@@ -28,13 +28,13 @@ export interface MatcherSpec {
   deadlineBefore: string;
   matchMeetingDurationInMS: number;
   includeSenderItemsFromOtherBranches: boolean;
-  additionalReceiverItems: { branch: string; items: string[] }[];
-  deadlineOverrides: { item: string; deadline: string }[];
+  additionalReceiverItems: { [branch: string]: string[] };
+  deadlineOverrides: { [item: string]: string };
 }
 
 export function candidateMatchToMatch(
   candidate: MatchWithMeetingInfo,
-  deadlineOverrides: { item: string; deadline: string }[],
+  deadlineOverrides: { [item: string]: string },
 ): Match {
   switch (candidate.variant) {
     case CandidateMatchVariant.StandMatch:
@@ -121,12 +121,12 @@ export async function getMatchableSenders(
  *
  * @param branchIds The IDs of branches to search for users and items
  * @param orderStorage
- * @param additionalReceiverItems items that all receivers in the predefined branches want
+ * @param additionalReceiverItems items that should get even without an order (known required courses)
  */
 export async function getMatchableReceivers(
   branchIds: string[],
   orderStorage: BlDocumentStorage<Order>,
-  additionalReceiverItems: { branch: string; items: string[] }[],
+  additionalReceiverItems: { [branch: string]: string[] },
 ): Promise<MatchableUser[]> {
   const aggregatedReceivers = (await orderStorage.aggregate([
     {
@@ -178,10 +178,12 @@ export async function getMatchableReceivers(
     },
   ])) as { id: string; items: string[]; branches: string[] }[];
 
-  for (const branchReceiverItems of additionalReceiverItems) {
+  for (const [branchId, receiverItems] of Object.entries(
+    additionalReceiverItems,
+  )) {
     for (const receiver of aggregatedReceivers) {
-      if (receiver.branches.includes(branchReceiverItems.branch)) {
-        receiver.items = [...receiver.items, ...branchReceiverItems.items];
+      if (receiver.branches.includes(branchId)) {
+        receiver.items = [...receiver.items, ...receiverItems];
       }
     }
   }
@@ -225,23 +227,25 @@ export function verifyMatcherSpec(
     typeof m["matchMeetingDurationInMS"] === "number" &&
     !isNaN(m["matchMeetingDurationInMS"]) &&
     isBoolean(m["includeSenderItemsFromOtherBranches"]) &&
-    Array.isArray(m["additionalReceiverItems"]) &&
-    m["additionalReceiverItems"].every(
-      (entry) =>
-        typeof entry["branch"] === "string" &&
-        ObjectId.isValid(entry["branch"]) &&
-        Array.isArray(entry["items"]) &&
-        entry["items"].every(
+    typeof m["additionalReceiverItems"] === "object" &&
+    isNotNullish(m["additionalReceiverItems"]) &&
+    Object.entries(m["additionalReceiverItems"]).every(
+      ([branchId, itemIds]) =>
+        typeof branchId === "string" &&
+        ObjectId.isValid(branchId) &&
+        Array.isArray(itemIds) &&
+        itemIds.every(
           (itemId) => typeof itemId === "string" && ObjectId.isValid(itemId),
         ),
     ) &&
-    Array.isArray(m["deadlineOverrides"]) &&
-    m["deadlineOverrides"].every(
-      (override) =>
-        typeof override["item"] === "string" &&
-        ObjectId.isValid(override["item"]) &&
-        typeof override["deadline"] === "string" &&
-        !isNaN(new Date(override["deadline"]).getTime()),
+    typeof m["deadlineOverrides"] === "object" &&
+    isNotNullish(m["deadlineOverrides"]) &&
+    Object.entries(m["deadlineOverrides"]).every(
+      ([itemId, newDeadline]) =>
+        typeof itemId === "string" &&
+        ObjectId.isValid(itemId) &&
+        typeof newDeadline === "string" &&
+        !isNaN(new Date(newDeadline).getTime()),
     )
   );
 }

src/collections/match/operations/match-generate.operation.ts

@@ -23,21 +23,25 @@ import assignMeetingInfoToMatches from "../helpers/match-finder-2/match-meeting-
 import { matchSchema } from "../match.schema";
 
 export class MatchGenerateOperation implements Operation {
+  private readonly _customerItemStorage: BlDocumentStorage<CustomerItem>;
+  private readonly _matchStorage: BlDocumentStorage<Match>;
+  private readonly _orderStorage: BlDocumentStorage<Order>;
+
   constructor(
-    private customerItemStorage?: BlDocumentStorage<CustomerItem>,
-    private matchStorage?: BlDocumentStorage<Match>,
-    private orderStorage?: BlDocumentStorage<Order>,
+    customerItemStorage?: BlDocumentStorage<CustomerItem>,
+    matchStorage?: BlDocumentStorage<Match>,
+    orderStorage?: BlDocumentStorage<Order>,
   ) {
-    this.customerItemStorage = customerItemStorage
+    this._customerItemStorage = customerItemStorage
       ? customerItemStorage
       : new BlDocumentStorage(
           BlCollectionName.CustomerItems,
           customerItemSchema,
         );
-    this.matchStorage = matchStorage
+    this._matchStorage = matchStorage
       ? matchStorage
       : new BlDocumentStorage(BlCollectionName.Matches, matchSchema);
-    this.orderStorage = orderStorage
+    this._orderStorage = orderStorage
       ? orderStorage
       : new BlDocumentStorage(BlCollectionName.Orders, orderSchema);
   }
@@ -52,15 +56,11 @@ export class MatchGenerateOperation implements Operation {
         matcherSpec.senderBranches,
         matcherSpec.deadlineBefore,
         matcherSpec.includeSenderItemsFromOtherBranches,
-        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-        // @ts-ignore
-        this.customerItemStorage,
+        this._customerItemStorage,
       ),
       getMatchableReceivers(
         matcherSpec.receiverBranches,
-        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-        // @ts-ignore
-        this.orderStorage,
+        this._orderStorage,
         matcherSpec.additionalReceiverItems,
       ),
     ]);
@@ -80,9 +80,7 @@ export class MatchGenerateOperation implements Operation {
       throw new BlError("No matches generated");
     }
 
-    // eslint-disable-next-line @typescript-eslint/ban-ts-comment
-    // @ts-ignore
-    const res = await this.matchStorage.addMany(matches);
+    const res = await this._matchStorage.addMany(matches);
     return new BlapiResponse(res.map((r) => r.id));
   }
 }