Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 103 additions & 0 deletions apps/dashboard-api/src/__tests__/project.controller.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
'use strict';

const mongoose = require('mongoose');

// Mock @urbackend/common first to mock Project and cache utilities
const mockProjectSave = jest.fn();
const mockProjectFindOne = jest.fn();

jest.mock('@urbackend/common', () => ({
Project: {
findOne: mockProjectFindOne
},
deleteProjectById: jest.fn().mockResolvedValue(true)
}));

const { updateNotificationSettings } = require('../controllers/project.controller');

describe('project.controller - updateNotificationSettings', () => {
let mockReq;
let mockRes;

beforeEach(() => {
jest.clearAllMocks();

mockReq = {
params: { projectId: '60c72b2f9b1d8b001c8e4b52' },
user: { _id: 'ownerId123' },
body: {}
};

mockRes = {
status: jest.fn().mockReturnThis(),
json: jest.fn()
};
});

test('should return 400 if email payload is missing', async () => {
mockReq.body = {}; // no 'email' property
await updateNotificationSettings(mockReq, mockRes);

expect(mockRes.status).toHaveBeenCalledWith(400);
expect(mockRes.json).toHaveBeenCalledWith({ error: "Missing 'email' settings in body" });
});

test('should return 404 if project is not found or not owned by user', async () => {
mockReq.body = { email: { enabled: true } };
mockProjectFindOne.mockResolvedValueOnce(null);

await updateNotificationSettings(mockReq, mockRes);

expect(mockRes.status).toHaveBeenCalledWith(404);
expect(mockRes.json).toHaveBeenCalledWith({ error: "Project not found or access denied." });
});

test('should successfully update notification settings and save the project', async () => {
const incomingEmailSettings = {
enabled: true,
storage: { type: 'absolute', absoluteLimit: 1000 },
database: { type: 'percentage', thresholds: [50, 75] }
};

mockReq.body = { email: incomingEmailSettings };

const mockProjectDoc = {
_id: '60c72b2f9b1d8b001c8e4b52',
notificationSettings: {
email: {
enabled: false
}
},
markModified: jest.fn(),
save: mockProjectSave
};

mockProjectFindOne.mockResolvedValueOnce(mockProjectDoc);
mockProjectSave.mockResolvedValueOnce(mockProjectDoc);

await updateNotificationSettings(mockReq, mockRes);

// Expect the object to be updated with merged settings
expect(mockProjectDoc.notificationSettings.email).toMatchObject(incomingEmailSettings);

// Since we provided storage/database, markModified should be called
expect(mockProjectDoc.markModified).toHaveBeenCalledWith('notificationSettings');
expect(mockProjectSave).toHaveBeenCalled();

expect(mockRes.json).toHaveBeenCalledWith({
success: true,
settings: mockProjectDoc.notificationSettings
});
});

test('should handle validation errors or save failures gracefully', async () => {
mockReq.body = { email: { enabled: true } };

mockProjectFindOne.mockRejectedValueOnce(new Error('DB Connection Failed'));

await updateNotificationSettings(mockReq, mockRes);

expect(mockRes.status).toHaveBeenCalledWith(500);
expect(mockRes.json).toHaveBeenCalledWith({ error: 'DB Connection Failed' });
});
});
34 changes: 34 additions & 0 deletions apps/dashboard-api/src/controllers/project.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -1159,3 +1159,37 @@ module.exports.updateCollectionRls = async (req, res) => {
res.status(500).json({ error: err.message });
}
}

// PATCH REQ FOR NOTIFICATION SETTINGS
module.exports.updateNotificationSettings = async (req, res) => {
try {
const { projectId } = req.params;
const { email } = req.body; // { enabled: true, storage: {...}, database: {...} }

if (!email) {
return res.status(400).json({ error: "Missing 'email' settings in body" });
}

const project = await Project.findOne({ _id: projectId, owner: req.user._id });
if (!project) return res.status(404).json({ error: "Project not found or access denied." });

project.notificationSettings = {
...project.notificationSettings,
email: {
...project.notificationSettings?.email,
...email,
}
};

if (email.storage || email.database) {
project.markModified('notificationSettings');
}

await project.save();

await deleteProjectById(projectId);
res.json({ success: true, settings: project.notificationSettings });
} catch (err) {
Comment on lines +1164 to +1192
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Validate and deep-merge notification settings payload.

Line 1176-1182 performs a shallow merge. Partial updates like email.storage = { thresholds: [...] } can overwrite sibling storage fields (type, absoluteLimit) unintentionally, and there’s no schema validation to prevent invalid payload shapes.

🧩 Proposed fix
 module.exports.updateNotificationSettings = async (req, res) => {
     try {
         const { projectId } = req.params;
-        const { email } = req.body; // { enabled: true, storage: {...}, database: {...} }
+        const { email } = req.body;

         if (!email) {
             return res.status(400).json({ error: "Missing 'email' settings in body" });
         }

         const project = await Project.findOne({ _id: projectId, owner: req.user._id });
         if (!project) return res.status(404).json({ error: "Project not found or access denied." });

+        const currentEmail = project.notificationSettings?.email || {};
         project.notificationSettings = {
             ...project.notificationSettings,
             email: {
-                ...project.notificationSettings?.email,
+                ...currentEmail,
                 ...email,
+                storage: email.storage
+                    ? { ...(currentEmail.storage || {}), ...email.storage }
+                    : currentEmail.storage,
+                database: email.database
+                    ? { ...(currentEmail.database || {}), ...email.database }
+                    : currentEmail.database,
             }
         };

-        if (email.storage || email.database) {
-             project.markModified('notificationSettings');
-        }
+        project.markModified('notificationSettings');

         await project.save();

         await deleteProjectById(projectId);
         res.json({ success: true, settings: project.notificationSettings });
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/dashboard-api/src/controllers/project.controller.js` around lines 1164 -
1192, The updateNotificationSettings handler currently does a shallow merge of
req.body.email into project.notificationSettings.email which can overwrite
sibling fields; replace the shallow merge in updateNotificationSettings so you
perform a deep/recursive merge of project.notificationSettings.email and the
incoming email payload (preserving unspecified nested fields like storage.type
and storage.absoluteLimit), validate the resulting settings against a schema
(e.g., required shapes for email.storage and email.database, allowed keys and
types) before assigning, only call project.markModified('notificationSettings')
when nested objects actually changed, and retain the
deleteProjectById(projectId) and save flow after successful validation/merge.

res.status(500).json({ error: err.message });
}
};
6 changes: 5 additions & 1 deletion apps/dashboard-api/src/routes/projects.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ const {
analytics,
updateAllowedDomains,
toggleAuth,
updateCollectionRls
updateCollectionRls,
updateNotificationSettings
} = require("../controllers/project.controller")

const { createAdminUser, resetPassword, getUserDetails, updateAdminUser, listUserSessions, revokeUserSession } = require('../controllers/userAuth.controller');
Expand Down Expand Up @@ -105,6 +106,9 @@ router.patch('/:projectId/auth/toggle', authMiddleware, verifyEmail, toggleAuth)
// PATCH REQ FOR COLLECTION RLS SETTINGS
router.patch('/:projectId/collections/:collectionName/rls', authMiddleware, verifyEmail, updateCollectionRls);

// PATCH REQ FOR NOTIFICATION SETTINGS
router.patch('/:projectId/notification-settings', authMiddleware, verifyEmail, updateNotificationSettings);

// ADMIN AUTH ROUTES
const {checkAuthEnabled} = require('@urbackend/common');
const {loadProjectForAdmin} = require('@urbackend/common');
Expand Down
Loading