fix: updateInbox claiming all rewards (@fehmer) (#5560)

!nuf

backend/__tests__/dal/user.spec.ts

@@ -892,7 +892,7 @@ describe("UserDal", () => {
     });
   });
   describe("updateInbox", () => {
-    it("claims rewards", async () => {
+    it("claims rewards on read", async () => {
       //GIVEN
       const rewardOne: SharedTypes.MonkeyMail = {
         id: "b5866d4c-0749-41b6-b101-3656249d39b9",
@@ -920,15 +920,24 @@ describe("UserDal", () => {
         subject: "reward three",
         timestamp: 3,
         read: true,
-        rewards: [{ type: "xp", item: 2000 }],
+        rewards: [{ type: "xp", item: 3000 }],
+      };
+      const rewardFour: SharedTypes.MonkeyMail = {
+        id: "d852d2cf-1802-4cd0-9fb4-336650fc470a",
+        body: "test",
+        subject: "reward four",
+        timestamp: 4,
+        read: false,
+        rewards: [{ type: "xp", item: 4000 }],
       };
 
       let user = await UserTestData.createUser({
         xp: 100,
-        inbox: [rewardOne, rewardTwo, rewardThree],
+        inbox: [rewardOne, rewardTwo, rewardThree, rewardFour],
       });
 
       //WNEN
+
       await UserDAL.updateInbox(
         user.uid,
         [rewardOne.id, rewardTwo.id, rewardThree.id],
@@ -940,41 +949,48 @@ describe("UserDal", () => {
       expect(read).not.toHaveProperty("tmp");
 
       const { xp, inbox } = read;
-      expect(xp).toEqual(3100);
+      expect(xp).toEqual(3100); //100 existing + 1000 from rewardOne, 2000 from rewardTwo
 
       //inbox is sorted by timestamp
       expect(inbox).toStrictEqual([
+        { ...rewardFour },
         { ...rewardThree },
         { ...rewardTwo, read: true, rewards: [] },
         { ...rewardOne, read: true, rewards: [] },
       ]);
     });
 
-    it("removes", async () => {
+    it("claims rewards on delete", async () => {
       //GIVEN
-      const rewardOne = {
+      //GIVEN
+      const rewardOne: SharedTypes.MonkeyMail = {
         id: "b5866d4c-0749-41b6-b101-3656249d39b9",
         body: "test",
         subject: "reward one",
-        timestamp: 0,
+        timestamp: 1,
         read: false,
-        rewards: [],
+        rewards: [
+          { type: "xp", item: 400 },
+          { type: "xp", item: 600 },
+          { type: "badge", item: { id: 4 } },
+        ],
       };
-      const rewardTwo = {
+      const rewardTwo: SharedTypes.MonkeyMail = {
         id: "3692b9f5-84fb-4d9b-bd39-9a3217b3a33a",
         body: "test",
         subject: "reward two",
-        timestamp: 0,
+        timestamp: 2,
         read: true,
-        rewards: [],
+        rewards: [{ type: "xp", item: 2000 }],
       };
-      const rewardThree = {
+
+      const rewardThree: SharedTypes.MonkeyMail = {
         id: "0d73b3e0-dc79-4abb-bcaf-66fa6b09a58a",
         body: "test",
         subject: "reward three",
-        timestamp: 0,
+        timestamp: 4,
         read: false,
-        rewards: [],
+        rewards: [{ type: "xp", item: 3000 }],
       };
 
       let user = await UserTestData.createUser({
@@ -986,7 +1002,8 @@ describe("UserDal", () => {
       await UserDAL.updateInbox(user.uid, [], [rewardOne.id, rewardTwo.id]);
 
       //THEN
-      const { inbox } = await UserDAL.getUser(user.uid, "");
+      const { xp, inbox } = await UserDAL.getUser(user.uid, "");
+      expect(xp).toBe(1100);
       expect(inbox).toStrictEqual([rewardThree]);
     });
 
@@ -1009,7 +1026,11 @@ describe("UserDal", () => {
         subject: "reward two",
         timestamp: 1,
         read: false,
-        rewards: [{ type: "badge", item: { id: 5 } }],
+        rewards: [
+          { type: "badge", item: { id: 3 } },
+          { type: "badge", item: { id: 4 } },
+          { type: "badge", item: { id: 5 } },
+        ],
       };
       const rewardThree: SharedTypes.MonkeyMail = {
         id: "0d73b3e0-dc79-4abb-bcaf-66fa6b09a58a",
@@ -1022,7 +1043,12 @@ describe("UserDal", () => {
 
       let user = await UserTestData.createUser({
         inbox: [rewardOne, rewardTwo, rewardThree],
-        inventory: { badges: [{ id: 1, selected: true }] },
+        inventory: {
+          badges: [
+            { id: 1, selected: true },
+            { id: 3, selected: false },
+          ],
+        },
       });
 
       //WNEN
@@ -1040,13 +1066,48 @@ describe("UserDal", () => {
         { ...rewardThree },
       ]);
       expect(inventory?.badges).toStrictEqual([
-        { id: 1, selected: true },
-        { id: 4 },
+        { id: 1, selected: true }, //previously owned
+        { id: 3, selected: false }, // previously owned, no duplicate
+        { id: 4 }, // gets only added once
         { id: 5 },
       ]);
     });
+    it("read and delete the same message does not claim reward twice", async () => {
+      //GIVEN
+      const rewardOne: SharedTypes.MonkeyMail = {
+        id: "b5866d4c-0749-41b6-b101-3656249d39b9",
+        body: "test",
+        subject: "reward one",
+        timestamp: 0,
+        read: false,
+        rewards: [{ type: "xp", item: 1000 }],
+      };
+      const rewardTwo: SharedTypes.MonkeyMail = {
+        id: "3692b9f5-84fb-4d9b-bd39-9a3217b3a33a",
+        body: "test",
+        subject: "reward two",
+        timestamp: 0,
+        read: false,
+        rewards: [{ type: "xp", item: 2000 }],
+      };
+      let user = await UserTestData.createUser({
+        xp: 100,
+        inbox: [rewardOne, rewardTwo],
+      });
 
-    it("does not claim reward multiple times", async () => {
+      await UserDAL.updateInbox(
+        user.uid,
+        [rewardOne.id, rewardTwo.id],
+        [rewardOne.id, rewardTwo.id]
+      );
+
+      //THEN
+
+      const { xp } = await UserDAL.getUser(user.uid, "");
+      expect(xp).toEqual(3100);
+    });
+
+    it("concurrent calls dont claim a reward multiple times", async () => {
       //GIVEN
       const rewardOne: SharedTypes.MonkeyMail = {
         id: "b5866d4c-0749-41b6-b101-3656249d39b9",
@@ -1088,7 +1149,7 @@ describe("UserDal", () => {
         .map(() =>
           UserDAL.updateInbox(
             user.uid,
-            [rewardOne.id, rewardTwo.id, rewardOne.id, rewardThree.id],
+            [rewardOne.id, rewardTwo.id, rewardThree.id],
             []
           )
         );

backend/src/dal/user.ts

@@ -981,9 +981,16 @@ export async function updateInbox(
   mailToRead: string[],
   mailToDelete: string[]
 ): Promise<void> {
-  const readSet = [...new Set(mailToRead)];
   const deleteSet = [...new Set(mailToDelete)];
 
+  //we don't need to read mails that are going to be deleted because
+  //Rewards will be claimed on unread mails on deletion
+  const readSet = [...new Set(mailToRead)].filter(
+    (it) => deleteSet.includes(it) === false
+  );
+
+  console.log({ deleteSet, readSet });
+
   const update = await getUsersCollection().updateOne({ uid }, [
     {
       $addFields: {
@@ -992,17 +999,46 @@ export async function updateInbox(
             lang: "js",
             args: ["$_id", "$inbox", "$xp", "$inventory"],
             body: `
-            function(_id, inbox, xp, inventory) {            
-              var rewards = inbox
+            function(_id, inbox, xp, inventory) {
+
+              var toBeDeleted = inbox.filter(it => ${JSON.stringify(
+                deleteSet
+              )}.includes(it.id) === true);
+
+              var toBeRead = inbox.filter(it => ${JSON.stringify(
+                readSet
+              )}.includes(it.id) === true && it.read === false);
+
+              //flatMap rewards
+              var rewards = [...toBeRead, ...toBeDeleted]
                   .filter(it => it.read === false)
                   .reduce((arr, current) => {
                       return arr.concat(current.rewards);
                   }, []);
-              
+
               var xpGain = rewards
                   .filter(it => it.type === "xp")
                   .map(it => it.item)
                   .reduce((s, a) => s + a, 0);
+
+              var badgesToClaim = rewards
+                  .filter(it => it.type === "badge")
+                  .map(it => it.item);
+
+              if (inventory === null) inventory = {
+                  badges: null
+              };
+              if (inventory.badges === null) inventory.badges = [];
+              
+              const uniqueBadgeIds = new Set();
+              const newBadges = [];
+
+              for(badge of [...inventory.badges, ...badgesToClaim]){
+                  if(uniqueBadgeIds.has(badge.id))continue;
+                  uniqueBadgeIds.add(badge.id);
+                  newBadges.push(badge);
+              }
+              inventory.badges = newBadges;
               
               //remove deleted mail from inbox, sort by timestamp descending
               var inboxUpdate = inbox
@@ -1012,21 +1048,13 @@ export async function updateInbox(
                   .sort((a, b) => b.timestamp - a.timestamp);
 
               //mark read mail as read, remove rewards
-              inboxUpdate.filter(it => it.read === false && ${JSON.stringify(
-                readSet
-              )}.includes(it.id)).forEach(it => {
+              toBeRead.forEach(it => {
                   it.read = true;
                   it.rewards = [];
               });
 
-              var badges = rewards
-                  .filter(it => it.type === "badge")
-                  .map(it => it.item);
 
-              if(inventory === null) inventory = { badges:null };
-              if(inventory.badges === null) inventory.badges = [];
-              inventory.badges.push(...badges);
-              
+
               return {
                   _id,
                   xp: xp + xpGain,