From cd0277cfd11f816526e472fcef1eb0e56cd5aa20 Mon Sep 17 00:00:00 2001
From: Tirth Patel <102514909+tirth1356@users.noreply.github.com>
Date: Sun, 17 May 2026 18:10:20 +0530
Subject: [PATCH 1/2] Fix mail template sanitization, test coverage, validation
 type mismatch, and test console noise

---
 .../src/controllers/project.controller.js     | 30 +++++--
 .../src/middlewares/authMiddleware.js         |  5 +-
 .../src/__tests__/mail.controller.test.js     | 87 ++++++++++++++++++-
 .../src/controllers/data.controller.js        | 28 ++++--
 .../common/src/queues/publicEmailQueue.js     |  4 +-
 packages/common/src/utils/input.validation.js |  5 +-
 6 files changed, 141 insertions(+), 18 deletions(-)

diff --git a/apps/dashboard-api/src/controllers/project.controller.js b/apps/dashboard-api/src/controllers/project.controller.js
index ebf430671..8792651c8 100644
--- a/apps/dashboard-api/src/controllers/project.controller.js
+++ b/apps/dashboard-api/src/controllers/project.controller.js
@@ -1604,7 +1604,10 @@ module.exports.listMailTemplates = async (req, res) => {
       message: "Mail templates fetched.",
     });
   } catch (err) {
-    return res.status(500).json({ success: false, data: {}, message: "Failed to fetch mail templates." });
+    return res.status(500).json({
+      error: "Internal server error",
+      details: process.env.NODE_ENV === "development" ? err.message : undefined,
+    });
   }
 };
 
@@ -1639,7 +1642,10 @@ module.exports.listGlobalMailTemplates = async (req, res) => {
       message: "Global mail templates fetched.",
     });
   } catch (err) {
-    return res.status(500).json({ success: false, data: {}, message: err.message });
+    return res.status(500).json({
+      error: "Internal server error",
+      details: process.env.NODE_ENV === "development" ? err.message : undefined,
+    });
   }
 };
 
@@ -1702,7 +1708,10 @@ module.exports.getMailTemplate = async (req, res) => {
       message: "Mail template fetched.",
     });
   } catch (err) {
-    return res.status(500).json({ success: false, data: {}, message: "Failed to fetch template." });
+    return res.status(500).json({
+      error: "Internal server error",
+      details: process.env.NODE_ENV === "development" ? err.message : undefined,
+    });
   }
 };
 
@@ -1767,7 +1776,10 @@ module.exports.createMailTemplate = async (req, res) => {
       return res.status(409).json({ success: false, data: {}, message: "Template name/key already exists." });
     }
 
-    return res.status(500).json({ success: false, data: {}, message: "Failed to create template." });
+    return res.status(500).json({
+      error: "Internal server error",
+      details: process.env.NODE_ENV === "development" ? err.message : undefined,
+    });
   }
 };
 
@@ -1853,7 +1865,10 @@ module.exports.updateMailTemplate = async (req, res) => {
       return res.status(409).json({ success: false, data: {}, message: "Template name/key already exists." });
     }
 
-    return res.status(500).json({ success: false, data: {}, message: "Failed to update template." });
+    return res.status(500).json({
+      error: "Internal server error",
+      details: process.env.NODE_ENV === "development" ? err.message : undefined,
+    });
   }
 };
 
@@ -1880,7 +1895,10 @@ module.exports.deleteMailTemplate = async (req, res) => {
 
     return res.json({ success: true, data: {}, message: "Mail template deleted." });
   } catch (err) {
-    return res.status(500).json({ success: false, data: {}, message: "Failed to delete template." });
+    return res.status(500).json({
+      error: "Internal server error",
+      details: process.env.NODE_ENV === "development" ? err.message : undefined,
+    });
   }
 };
 
diff --git a/apps/dashboard-api/src/middlewares/authMiddleware.js b/apps/dashboard-api/src/middlewares/authMiddleware.js
index 424740bd6..01a9343cb 100644
--- a/apps/dashboard-api/src/middlewares/authMiddleware.js
+++ b/apps/dashboard-api/src/middlewares/authMiddleware.js
@@ -31,8 +31,9 @@ module.exports = function (req, res, next) {
         // Proceed to the next middleware or route handler
         next();
     } catch (err) {
-        console.log("err---------------2")
-        console.error(err);
+        if (process.env.NODE_ENV !== 'test') {
+            console.error(err);
+        }
 
         res.status(400).json({ error: 'Invalid Token' });
     }
diff --git a/apps/public-api/src/__tests__/mail.controller.test.js b/apps/public-api/src/__tests__/mail.controller.test.js
index bbac71354..6c148cc4b 100644
--- a/apps/public-api/src/__tests__/mail.controller.test.js
+++ b/apps/public-api/src/__tests__/mail.controller.test.js
@@ -70,7 +70,7 @@ jest.mock('@urbackend/common', () => {
     };
 });
 
-const { Project, decrypt, redis, publicEmailQueue, MailLog } = require('@urbackend/common');
+const { Project, decrypt, redis, publicEmailQueue, MailTemplate, MailLog } = require('@urbackend/common');
 const mailController = require('../controllers/mail.controller');
 const originalResendApiKey2 = process.env.RESEND_API_KEY_2;
 
@@ -208,6 +208,91 @@ describe('mail.controller', () => {
         }), expect.objectContaining({ attempts: expect.any(Number) }));
     });
 
+    test('renders and sends a project-scoped mail template from DB', async () => {
+        const req = makeReq();
+        req.body = {
+            to: 'user@example.com',
+            templateName: 'welcome',
+            variables: { name: 'Yash' },
+        };
+        const res = makeRes();
+
+        mockProjectConfig({
+            _id: 'proj_1',
+            resendApiKey: null,
+        });
+
+        MailTemplate.findOne.mockReturnValueOnce({
+            lean: jest.fn().mockResolvedValue({
+                _id: 'tpl_db_1',
+                name: 'welcome',
+                subject: 'Hello {{name}}',
+                text: 'Welcome to project!',
+                html: '<p>Welcome to project!</p>',
+                projectId: 'proj_1'
+            })
+        });
+
+        decrypt.mockReturnValue(null);
+        redis.eval.mockResolvedValue(1);
+
+        await mailController.sendMail(req, res);
+
+        expect(res.status).toHaveBeenCalledWith(200);
+        expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
+            success: true,
+            data: expect.objectContaining({
+                templateUsed: expect.objectContaining({ name: 'welcome', id: 'tpl_db_1', scope: 'project' }),
+            }),
+        }));
+    });
+
+    test('renders and sends a global mail template from DB when no project template exists', async () => {
+        const req = makeReq();
+        req.body = {
+            to: 'user@example.com',
+            templateName: 'welcome',
+            variables: { name: 'Yash' },
+        };
+        const res = makeRes();
+
+        mockProjectConfig({
+            _id: 'proj_1',
+            resendApiKey: null,
+        });
+
+        // First call: project scope (returns null)
+        MailTemplate.findOne.mockReturnValueOnce({
+            lean: jest.fn().mockResolvedValue(null)
+        });
+
+        // Second call: global scope
+        MailTemplate.findOne.mockReturnValueOnce({
+            lean: jest.fn().mockResolvedValue({
+                _id: 'tpl_global_1',
+                name: 'welcome',
+                subject: 'Global Hello {{name}}',
+                text: 'Global welcome!',
+                html: '<p>Global welcome!</p>',
+                projectId: null,
+                isSystem: true
+            })
+        });
+
+        decrypt.mockReturnValue(null);
+        redis.eval.mockResolvedValue(1);
+
+        await mailController.sendMail(req, res);
+
+        expect(res.status).toHaveBeenCalledWith(200);
+        expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
+            success: true,
+            data: expect.objectContaining({
+                templateUsed: expect.objectContaining({ name: 'welcome', id: 'tpl_global_1', scope: 'global' }),
+            }),
+        }));
+    });
+
     test('refunds quota on terminal async worker failure', async () => {
         let failedHandler;
         jest.resetModules();
diff --git a/apps/public-api/src/controllers/data.controller.js b/apps/public-api/src/controllers/data.controller.js
index cf961bcb5..1971ab49e 100644
--- a/apps/public-api/src/controllers/data.controller.js
+++ b/apps/public-api/src/controllers/data.controller.js
@@ -84,7 +84,9 @@ module.exports.insertData = async (req, res) => {
     if (isDebug) console.log(`[DEBUG] insert data took ${(performance.now() - start).toFixed(2)}ms`);
     res.status(201).json(result);
   } catch (err) {
-    console.error(err);
+    if (process.env.NODE_ENV !== 'test') {
+      console.error(err);
+    }
 
     if (isDuplicateKeyError(err)) {
       return res.status(409).json({
@@ -173,7 +175,9 @@ module.exports.insertData = async (req, res) => {
       message: "Bulk insert successful",
     });
   } catch (err) {
-  console.error(err);
+    if (process.env.NODE_ENV !== 'test') {
+      console.error(err);
+    }
 
   if (isDuplicateKeyError(err)) {
     return next(
@@ -285,7 +289,9 @@ module.exports.getAllData = async (req, res) => {
       message: "Data fetched successfully",
     });
   } catch (err) {
-    console.error(err);
+    if (process.env.NODE_ENV !== 'test') {
+      console.error(err);
+    }
 
     if (err && (err.statusCode === 400 || err.name === 'QueryFilterError')) {
       return res.status(400).json({
@@ -358,7 +364,9 @@ module.exports.getSingleDoc = async (req, res) => {
 
     res.json(doc);
   } catch (err) {
-    console.error(err);
+    if (process.env.NODE_ENV !== 'test') {
+      console.error(err);
+    }
     res.status(500).json({ error: err.message });
   }
 };
@@ -418,7 +426,9 @@ module.exports.aggregateData = async (req, res) => {
       message: "Aggregation executed successfully.",
     });
   } catch (err) {
-    console.error(err);
+    if (process.env.NODE_ENV !== 'test') {
+      console.error(err);
+    }
 
     if (err instanceof z.ZodError) {
       return res.status(400).json({
@@ -489,7 +499,9 @@ module.exports.updateSingleData = async (req, res) => {
 
     res.json({ message: "Updated", data: result });
   } catch (err) {
-    console.error(err);
+    if (process.env.NODE_ENV !== 'test') {
+      console.error(err);
+    }
 
     if (isDuplicateKeyError(err)) {
       return res.status(409).json({
@@ -557,7 +569,9 @@ module.exports.deleteSingleDoc = async (req, res) => {
 
     res.json({ message: "Document deleted", id });
   } catch (err) {
-    console.error(err);
+    if (process.env.NODE_ENV !== 'test') {
+      console.error(err);
+    }
     res.status(500).json({ error: err.message });
   }
 };
\ No newline at end of file
diff --git a/packages/common/src/queues/publicEmailQueue.js b/packages/common/src/queues/publicEmailQueue.js
index da2dcb4f3..496463503 100644
--- a/packages/common/src/queues/publicEmailQueue.js
+++ b/packages/common/src/queues/publicEmailQueue.js
@@ -103,7 +103,9 @@ const initPublicEmailWorker = () => {
     });
 
     worker.on('failed', async (job, err) => {
-        console.error(`[Queue] Job ${job?.id} (public email) failed:`, err);
+        if (process.env.NODE_ENV !== 'test') {
+            console.error(`[Queue] Job ${job?.id} (public email) failed:`, err);
+        }
         if (job && job.data && job.data.consumedQuotaKey) {
             const maxAttempts = job.opts?.attempts || 1;
             if (job.attemptsMade >= maxAttempts) {
diff --git a/packages/common/src/utils/input.validation.js b/packages/common/src/utils/input.validation.js
index c39b0079c..9a9643141 100755
--- a/packages/common/src/utils/input.validation.js
+++ b/packages/common/src/utils/input.validation.js
@@ -553,7 +553,10 @@ module.exports.updateWebhookSchema = z.object({
 
 module.exports.sendMailSchema = z
   .object({
-    to: z.string().email("Invalid recipient email format"),
+    to: z.union([
+      z.string().email("Invalid recipient email format"),
+      z.array(z.string().email("Invalid recipient email format")).nonempty("Recipient list cannot be empty")
+    ]),
 
     // Direct-send fields (backward compatible)
     subject: z.preprocess(

From 3b780634ef36755e5d29028e1278230cb84bcf20 Mon Sep 17 00:00:00 2001
From: Tirth Patel <102514909+tirth1356@users.noreply.github.com>
Date: Tue, 19 May 2026 15:56:12 +0530
Subject: [PATCH 2/2] fix: address coderabbitai review issues from PR #124

---
 .../src/controllers/project.controller.js     | 46 +++++--------
 .../src/__tests__/mail.controller.test.js     | 64 +++++++++++++++++++
 2 files changed, 80 insertions(+), 30 deletions(-)

diff --git a/apps/dashboard-api/src/controllers/project.controller.js b/apps/dashboard-api/src/controllers/project.controller.js
index 8792651c8..8357b155c 100644
--- a/apps/dashboard-api/src/controllers/project.controller.js
+++ b/apps/dashboard-api/src/controllers/project.controller.js
@@ -1489,7 +1489,7 @@ const toSlug = (value) => {
 };
 
 
-module.exports.listMailTemplates = async (req, res) => {
+module.exports.listMailTemplates = async (req, res, next) => {
   try {
     const { projectId } = req.params;
 
@@ -1604,14 +1604,12 @@ module.exports.listMailTemplates = async (req, res) => {
       message: "Mail templates fetched.",
     });
   } catch (err) {
-    return res.status(500).json({
-      error: "Internal server error",
-      details: process.env.NODE_ENV === "development" ? err.message : undefined,
-    });
+    if (err instanceof AppError) return next(err);
+    return next(new AppError(500, "Internal server error"));
   }
 };
 
-module.exports.listGlobalMailTemplates = async (req, res) => {
+module.exports.listGlobalMailTemplates = async (req, res, next) => {
   try {
     const { projectId } = req.params;
 
@@ -1642,14 +1640,12 @@ module.exports.listGlobalMailTemplates = async (req, res) => {
       message: "Global mail templates fetched.",
     });
   } catch (err) {
-    return res.status(500).json({
-      error: "Internal server error",
-      details: process.env.NODE_ENV === "development" ? err.message : undefined,
-    });
+    if (err instanceof AppError) return next(err);
+    return next(new AppError(500, "Internal server error"));
   }
 };
 
-module.exports.getMailTemplate = async (req, res) => {
+module.exports.getMailTemplate = async (req, res, next) => {
   try {
     const { projectId, templateId } = req.params;
     if (!mongoose.isValidObjectId(templateId)) {
@@ -1708,14 +1704,12 @@ module.exports.getMailTemplate = async (req, res) => {
       message: "Mail template fetched.",
     });
   } catch (err) {
-    return res.status(500).json({
-      error: "Internal server error",
-      details: process.env.NODE_ENV === "development" ? err.message : undefined,
-    });
+    if (err instanceof AppError) return next(err);
+    return next(new AppError(500, "Internal server error"));
   }
 };
 
-module.exports.createMailTemplate = async (req, res) => {
+module.exports.createMailTemplate = async (req, res, next) => {
   try {
     const { projectId } = req.params;
 
@@ -1776,14 +1770,11 @@ module.exports.createMailTemplate = async (req, res) => {
       return res.status(409).json({ success: false, data: {}, message: "Template name/key already exists." });
     }
 
-    return res.status(500).json({
-      error: "Internal server error",
-      details: process.env.NODE_ENV === "development" ? err.message : undefined,
-    });
+    return next(new AppError(500, "Internal server error"));
   }
 };
 
-module.exports.updateMailTemplate = async (req, res) => {
+module.exports.updateMailTemplate = async (req, res, next) => {
   try {
     const { projectId, templateId } = req.params;
     if (!mongoose.isValidObjectId(templateId)) {
@@ -1865,14 +1856,11 @@ module.exports.updateMailTemplate = async (req, res) => {
       return res.status(409).json({ success: false, data: {}, message: "Template name/key already exists." });
     }
 
-    return res.status(500).json({
-      error: "Internal server error",
-      details: process.env.NODE_ENV === "development" ? err.message : undefined,
-    });
+    return next(new AppError(500, "Internal server error"));
   }
 };
 
-module.exports.deleteMailTemplate = async (req, res) => {
+module.exports.deleteMailTemplate = async (req, res, next) => {
   try {
     const { projectId, templateId } = req.params;
     if (!mongoose.isValidObjectId(templateId)) {
@@ -1895,10 +1883,8 @@ module.exports.deleteMailTemplate = async (req, res) => {
 
     return res.json({ success: true, data: {}, message: "Mail template deleted." });
   } catch (err) {
-    return res.status(500).json({
-      error: "Internal server error",
-      details: process.env.NODE_ENV === "development" ? err.message : undefined,
-    });
+    if (err instanceof AppError) return next(err);
+    return next(new AppError(500, "Internal server error"));
   }
 };
 
diff --git a/apps/public-api/src/__tests__/mail.controller.test.js b/apps/public-api/src/__tests__/mail.controller.test.js
index 6c148cc4b..c409d69d5 100644
--- a/apps/public-api/src/__tests__/mail.controller.test.js
+++ b/apps/public-api/src/__tests__/mail.controller.test.js
@@ -129,6 +129,18 @@ describe('mail.controller', () => {
             success: true,
             data: expect.objectContaining({ provider: 'byok', monthlyUsage: 1 }),
         }));
+        expect(publicEmailQueue.add).toHaveBeenCalledWith("send-public-email", expect.objectContaining({
+            projectId: 'proj_1',
+            usingByok: true,
+            payload: expect.objectContaining({
+                to: 'user@example.com',
+                subject: 'Hello',
+                text: 'This is a message.'
+            })
+        }), expect.objectContaining({
+            attempts: 3,
+            backoff: expect.objectContaining({ type: 'exponential', delay: 5000 })
+        }));
     });
 
     test('falls back to default key when BYOK missing', async () => {
@@ -145,6 +157,18 @@ describe('mail.controller', () => {
         expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
             data: expect.objectContaining({ provider: 'default', monthlyUsage: 2 }),
         }));
+        expect(publicEmailQueue.add).toHaveBeenCalledWith("send-public-email", expect.objectContaining({
+            projectId: 'proj_1',
+            usingByok: false,
+            payload: expect.objectContaining({
+                to: 'user@example.com',
+                subject: 'Hello',
+                text: 'This is a message.'
+            })
+        }), expect.objectContaining({
+            attempts: 3,
+            backoff: expect.objectContaining({ type: 'exponential', delay: 5000 })
+        }));
     });
 
     test('enforces monthly limit', async () => {
@@ -238,6 +262,11 @@ describe('mail.controller', () => {
 
         await mailController.sendMail(req, res);
 
+        // Assert project-scope query was made first
+        expect(MailTemplate.findOne).toHaveBeenCalledWith(expect.objectContaining({
+            projectId: 'proj_1',
+        }));
+
         expect(res.status).toHaveBeenCalledWith(200);
         expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
             success: true,
@@ -245,6 +274,19 @@ describe('mail.controller', () => {
                 templateUsed: expect.objectContaining({ name: 'welcome', id: 'tpl_db_1', scope: 'project' }),
             }),
         }));
+        expect(publicEmailQueue.add).toHaveBeenCalledWith("send-public-email", expect.objectContaining({
+            projectId: 'proj_1',
+            usingByok: false,
+            payload: expect.objectContaining({
+                to: 'user@example.com',
+                subject: 'Hello Yash',
+                text: 'Welcome to project!',
+                html: '<p>Welcome to project!</p>'
+            })
+        }), expect.objectContaining({
+            attempts: 3,
+            backoff: expect.objectContaining({ type: 'exponential', delay: 5000 })
+        }));
     });
 
     test('renders and sends a global mail template from DB when no project template exists', async () => {
@@ -284,6 +326,15 @@ describe('mail.controller', () => {
 
         await mailController.sendMail(req, res);
 
+        // Assert project-scope was queried first, then global fallback
+        expect(MailTemplate.findOne).toHaveBeenNthCalledWith(1, expect.objectContaining({
+            projectId: 'proj_1',
+        }));
+        expect(MailTemplate.findOne).toHaveBeenNthCalledWith(2, expect.objectContaining({
+            projectId: null,
+            isSystem: true,
+        }));
+
         expect(res.status).toHaveBeenCalledWith(200);
         expect(res.json).toHaveBeenCalledWith(expect.objectContaining({
             success: true,
@@ -291,6 +342,19 @@ describe('mail.controller', () => {
                 templateUsed: expect.objectContaining({ name: 'welcome', id: 'tpl_global_1', scope: 'global' }),
             }),
         }));
+        expect(publicEmailQueue.add).toHaveBeenCalledWith("send-public-email", expect.objectContaining({
+            projectId: 'proj_1',
+            usingByok: false,
+            payload: expect.objectContaining({
+                to: 'user@example.com',
+                subject: 'Global Hello Yash',
+                text: 'Global welcome!',
+                html: '<p>Global welcome!</p>'
+            })
+        }), expect.objectContaining({
+            attempts: 3,
+            backoff: expect.objectContaining({ type: 'exponential', delay: 5000 })
+        }));
     });
 
     test('refunds quota on terminal async worker failure', async () => {