From 3722000d85ba1c901c99e003560634d24aae5f19 Mon Sep 17 00:00:00 2001 From: Christian Fehmer Date: Fri, 5 Jul 2024 13:47:54 +0200 Subject: [PATCH] fix: updateInbox claiming all rewards (@fehmer) (#5560) !nuf --- backend/__tests__/dal/user.spec.ts | 103 +++++++++++++++++++++++------ backend/src/dal/user.ts | 56 ++++++++++++---- 2 files changed, 124 insertions(+), 35 deletions(-) diff --git a/backend/__tests__/dal/user.spec.ts b/backend/__tests__/dal/user.spec.ts index 07aa9d087..21243c056 100644 --- a/backend/__tests__/dal/user.spec.ts +++ b/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], [] ) ); diff --git a/backend/src/dal/user.ts b/backend/src/dal/user.ts index df6128ad0..e3dc452e0 100644 --- a/backend/src/dal/user.ts +++ b/backend/src/dal/user.ts @@ -981,9 +981,16 @@ export async function updateInbox( mailToRead: string[], mailToDelete: string[] ): Promise { - 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,