OpenWF/SpaceNinjaServer
28
29
Fork 35
Code Issues 23 Pull Requests 1 Actions Packages Projects Releases Activity

feat: Inbox #876

Merged
OrdisPrime merged 11 commits from inbox into main 2025-01-31 05:15:36 -08:00
Conversation 4 Commits 11 Files Changed 14 +12 -17
1 changed files with 12 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 1326fb9092 - Show all commits

29
src/services/inboxService.ts
View File

@ -32,7 +32,7 @@ export const createNewEventMessages = async (req: Request) => {
const account = await getAccountForRequest(req);
const latestEventMessageDate = account.LatestEventMessageDate;
//TODO: is baroo there? send message
//TODO: is baroo there? create these kind of messages too (periodical messages)
const newEventMessages = messages.Messages.filter(m => new Date(m.eventMessageDate) > latestEventMessageDate);
if (newEventMessages.length === 0) {
@ -40,30 +40,25 @@ export const createNewEventMessages = async (req: Request) => {
return;
}
const newEventMessagesSorted = newEventMessages.sort(
(a, b) => new Date(a.eventMessageDate).getTime() - new Date(b.eventMessageDate).getTime()
);
const savedEventMessages = await createMessage(account._id.toString(), newEventMessages);
logger.debug("created event messages", savedEventMessages);
const latestEventMessage = savedEventMessages.reduce((prev, current) =>
prev.date > current.date ? prev : current
const latestEventMessage = newEventMessages.reduce((prev, current) =>
prev.eventMessageDate > current.eventMessageDate ? prev : current
);
coderabbitai[bot] commented 2025-01-31 05:15:16 -08:00 (Migrated from github.com)
Review
Copy Link

🛠️ Refactor suggestion

Remove console.log statement.

Production code should use the logger instead of console.log.

-    console.log("latestEventMessage", latestEventMessage);
+    logger.debug("Latest event message:", latestEventMessage);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

    logger.debug("Latest event message:", latestEventMessage);
_:hammer_and_wrench: Refactor suggestion_ **Remove console.log statement.** Production code should use the logger instead of console.log. ```diff - console.log("latestEventMessage", latestEventMessage); + logger.debug("Latest event message:", latestEventMessage); ``` <!-- suggestion_start --> <details> <summary>📝 Committable suggestion</summary> > ‼️ **IMPORTANT** > Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements. `````suggestion logger.debug("Latest event message:", latestEventMessage); ````` </details> <!-- suggestion_end --> <!-- This is an auto-generated comment by CodeRabbit -->
account.LatestEventMessageDate = latestEventMessage.date;
console.log("latestEventMessage", latestEventMessage);
account.LatestEventMessageDate = new Date(latestEventMessage.eventMessageDate);
await account.save();
coderabbitai[bot] commented 2025-01-31 04:36:22 -08:00 (Migrated from github.com)
Review
Copy Link

🛠️ Refactor suggestion

Consider potential race conditions in updating the latest event message date.

Multiple concurrent requests could lead to a race condition when updating LatestEventMessageDate. Consider using a transaction to ensure atomicity.

 export const createNewEventMessages = async (req: Request) => {
-    const account = await getAccountForRequest(req);
-    const latestEventMessageDate = account.LatestEventMessageDate;
+    const session = await mongoose.startSession();
+    try {
+        await session.withTransaction(async () => {
+            const account = await getAccountForRequest(req);
+            const latestEventMessageDate = account.LatestEventMessageDate;

-    //TODO: is baroo there? create these kind of messages too (periodical messages)
-    const newEventMessages = messages.Messages.filter(m => new Date(m.eventMessageDate) > latestEventMessageDate);
+            const newEventMessages = messages.Messages.filter(m => new Date(m.eventMessageDate) > latestEventMessageDate);

-    if (newEventMessages.length === 0) {
-        logger.debug(`No new event messages. Latest event message date: ${latestEventMessageDate.toISOString()}`);
-        return;
-    }
+            if (newEventMessages.length === 0) {
+                logger.debug(`No new event messages. Latest event message date: ${latestEventMessageDate.toISOString()}`);
+                return;
+            }

-    const savedEventMessages = await createMessage(account._id.toString(), newEventMessages);
-    logger.debug("created event messages", savedEventMessages);
+            const savedEventMessages = await createMessage(account._id.toString(), newEventMessages);
+            logger.debug("created event messages", savedEventMessages);

-    const latestEventMessage = newEventMessages.reduce((prev, current) =>
-        prev.eventMessageDate > current.eventMessageDate ? prev : current
-    );
+            const latestEventMessage = newEventMessages.reduce((prev, current) =>
+                prev.eventMessageDate > current.eventMessageDate ? prev : current
+            );

-    console.log("latestEventMessage", latestEventMessage);
-    account.LatestEventMessageDate = new Date(latestEventMessage.eventMessageDate);
-    await account.save();
+            account.LatestEventMessageDate = new Date(latestEventMessage.eventMessageDate);
+            await account.save({ session });
+        });
+    } finally {
+        await session.endSession();
+    }
 };

Committable suggestion skipped: line range outside the PR's diff.

_:hammer_and_wrench: Refactor suggestion_ **Consider potential race conditions in updating the latest event message date.** Multiple concurrent requests could lead to a race condition when updating `LatestEventMessageDate`. Consider using a transaction to ensure atomicity. ```diff export const createNewEventMessages = async (req: Request) => { - const account = await getAccountForRequest(req); - const latestEventMessageDate = account.LatestEventMessageDate; + const session = await mongoose.startSession(); + try { + await session.withTransaction(async () => { + const account = await getAccountForRequest(req); + const latestEventMessageDate = account.LatestEventMessageDate; - //TODO: is baroo there? create these kind of messages too (periodical messages) - const newEventMessages = messages.Messages.filter(m => new Date(m.eventMessageDate) > latestEventMessageDate); + const newEventMessages = messages.Messages.filter(m => new Date(m.eventMessageDate) > latestEventMessageDate); - if (newEventMessages.length === 0) { - logger.debug(`No new event messages. Latest event message date: ${latestEventMessageDate.toISOString()}`); - return; - } + if (newEventMessages.length === 0) { + logger.debug(`No new event messages. Latest event message date: ${latestEventMessageDate.toISOString()}`); + return; + } - const savedEventMessages = await createMessage(account._id.toString(), newEventMessages); - logger.debug("created event messages", savedEventMessages); + const savedEventMessages = await createMessage(account._id.toString(), newEventMessages); + logger.debug("created event messages", savedEventMessages); - const latestEventMessage = newEventMessages.reduce((prev, current) => - prev.eventMessageDate > current.eventMessageDate ? prev : current - ); + const latestEventMessage = newEventMessages.reduce((prev, current) => + prev.eventMessageDate > current.eventMessageDate ? prev : current + ); - console.log("latestEventMessage", latestEventMessage); - account.LatestEventMessageDate = new Date(latestEventMessage.eventMessageDate); - await account.save(); + account.LatestEventMessageDate = new Date(latestEventMessage.eventMessageDate); + await account.save({ session }); + }); + } finally { + await session.endSession(); + } }; ``` > Committable suggestion skipped: line range outside the PR's diff. <!-- This is an auto-generated comment by CodeRabbit -->
};
export const createMessage = async (accountId: string, message: IMessageCreationTemplate[]) => {
//TODO: createmany
const savedMessages = [];
for (const msg of message) {
const savedMessage = await Inbox.create({
...msg,
ownerId: accountId
});
savedMessages.push(savedMessage);
}
export const createMessage = async (accountId: string, messages: IMessageCreationTemplate[]) => {
const ownerIdMessages = messages.map(m => ({
...m,
ownerId: accountId
}));
const savedMessages = await Inbox.insertMany(ownerIdMessages);
return savedMessages;
};