From 3413487e5fa109a8050ee64cbac765802b2e30c6 Mon Sep 17 00:00:00 2001 From: PipKcK Date: Tue, 23 Sep 2025 20:44:30 -0700 Subject: [PATCH 1/4] fix: removed userId of deleted users from task->resources and teams-members --- src/controllers/userProfileController.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/controllers/userProfileController.js b/src/controllers/userProfileController.js index 412b0a42d..1f052a6f9 100644 --- a/src/controllers/userProfileController.js +++ b/src/controllers/userProfileController.js @@ -21,6 +21,8 @@ const Badge = require('../models/badge'); const yearMonthDayDateValidator = require('../utilities/yearMonthDayDateValidator'); const cacheClosure = require('../utilities/nodeCache'); const followUp = require('../models/followUp'); +const task = require('../models/task'); +const team = require('../models/team'); const HGNFormResponses = require('../models/hgnFormResponse'); const userService = require('../services/userService'); const { hasPermission, canRequestorUpdateUser } = require('../utilities/permissions'); @@ -1427,6 +1429,13 @@ const createControllerMethods = function (UserProfile, Project, cache) { await UserProfile.deleteOne({ _id: userId }); // delete followUp for deleted user await followUp.findOneAndDelete({ userId }); + // delete user from task-resources + await task.updateMany( + { 'resources.userID': userId }, + { $pull: { resources: { userID: userId } } }, + ); + // delete user from teams-members + await team.updateMany({ 'members.userId': userId }, { $pull: { members: { userId } } }); res.status(200).send({ message: 'Executed Successfully' }); auditIfProtectedAccountUpdated({ requestorId: req.body.requestor.requestorId, From 59669f56e20667cced3a3f063ea6b615d5d59097 Mon Sep 17 00:00:00 2001 From: Yu Yan Date: Thu, 2 Apr 2026 20:06:03 -0700 Subject: [PATCH 2/4] fix: convert userId to ObjectId for proper MongoDB matching when deleting user from tasks and teams --- src/controllers/userProfileController.js | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/controllers/userProfileController.js b/src/controllers/userProfileController.js index 1f052a6f9..8cd43ce52 100644 --- a/src/controllers/userProfileController.js +++ b/src/controllers/userProfileController.js @@ -1429,13 +1429,18 @@ const createControllerMethods = function (UserProfile, Project, cache) { await UserProfile.deleteOne({ _id: userId }); // delete followUp for deleted user await followUp.findOneAndDelete({ userId }); + // Convert userId to ObjectId for proper matching in MongoDB + const userIdObject = new mongoose.Types.ObjectId(userId); // delete user from task-resources await task.updateMany( - { 'resources.userID': userId }, - { $pull: { resources: { userID: userId } } }, + { 'resources.userID': userIdObject }, + { $pull: { resources: { userID: userIdObject } } }, ); // delete user from teams-members - await team.updateMany({ 'members.userId': userId }, { $pull: { members: { userId } } }); + await team.updateMany( + { 'members.userId': userIdObject }, + { $pull: { members: { userId: userIdObject } } }, + ); res.status(200).send({ message: 'Executed Successfully' }); auditIfProtectedAccountUpdated({ requestorId: req.body.requestor.requestorId, From e225073674d562e3429dadc314ab1f6ab699c70b Mon Sep 17 00:00:00 2001 From: Yu Yan Date: Thu, 2 Apr 2026 20:07:57 -0700 Subject: [PATCH 3/4] fix: add error handling for ObjectId conversion to prevent 500 errors --- src/controllers/userProfileController.js | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/controllers/userProfileController.js b/src/controllers/userProfileController.js index 8cd43ce52..bffbc73ff 100644 --- a/src/controllers/userProfileController.js +++ b/src/controllers/userProfileController.js @@ -1429,8 +1429,15 @@ const createControllerMethods = function (UserProfile, Project, cache) { await UserProfile.deleteOne({ _id: userId }); // delete followUp for deleted user await followUp.findOneAndDelete({ userId }); - // Convert userId to ObjectId for proper matching in MongoDB - const userIdObject = new mongoose.Types.ObjectId(userId); + // Validate and convert userId to ObjectId for proper matching in MongoDB + let userIdObject; + try { + userIdObject = new mongoose.Types.ObjectId(userId); + } catch (idError) { + // If conversion fails, userId might already be an ObjectId or invalid + logger.logInfo(`Invalid userId format for ObjectId conversion: ${userId}`); + userIdObject = userId; + } // delete user from task-resources await task.updateMany( { 'resources.userID': userIdObject }, From 2667e9d8f9dcf62eb708a37c6bab0b0eb9c14ce1 Mon Sep 17 00:00:00 2001 From: Yu Yan Date: Sat, 11 Apr 2026 23:50:29 -0700 Subject: [PATCH 4/4] fix(user-profile): clean deleted users from tasks and teams --- ...rofileController.deleteUserProfile.spec.js | 100 ++++++++++++++++++ src/controllers/userProfileController.js | 26 ++--- 2 files changed, 110 insertions(+), 16 deletions(-) create mode 100644 src/controllers/userProfileController.deleteUserProfile.spec.js diff --git a/src/controllers/userProfileController.deleteUserProfile.spec.js b/src/controllers/userProfileController.deleteUserProfile.spec.js new file mode 100644 index 000000000..bdd82565c --- /dev/null +++ b/src/controllers/userProfileController.deleteUserProfile.spec.js @@ -0,0 +1,100 @@ +jest.mock('../helpers/userHelper', () => () => ({})); +jest.mock('../models/timeentry', () => ({})); +jest.mock('../models/team', () => ({ + updateMany: jest.fn(), +})); +jest.mock('../models/badge', () => ({})); +jest.mock('../utilities/nodeCache', () => + jest.fn(() => ({ + removeCache: jest.fn(), + getCache: jest.fn(() => null), + setCache: jest.fn(), + })), +); +jest.mock('../models/followUp', () => ({ + findOneAndDelete: jest.fn(), +})); +jest.mock('../models/task', () => ({ + updateMany: jest.fn(), +})); +jest.mock('../models/hgnFormResponse', () => ({})); +jest.mock('../services/userService', () => ({})); +jest.mock('../utilities/permissions', () => ({ + hasPermission: jest.fn(), + canRequestorUpdateUser: jest.fn(), +})); +jest.mock('../utilities/emailSender', () => jest.fn()); +jest.mock('../utilities/objectUtils', () => ({ + deepCopyMongooseObjectWithLodash: jest.fn((value) => value), +})); +jest.mock('../startup/logger', () => ({ + logInfo: jest.fn(), + logException: jest.fn(), +})); +jest.mock('./reportsController', () => () => ({})); + +const mongoose = require('mongoose'); +const Team = require('../models/team'); +const Task = require('../models/task'); +const followUp = require('../models/followUp'); +const { hasPermission, canRequestorUpdateUser } = require('../utilities/permissions'); +const { mockReq, mockRes } = require('../test'); +const userProfileController = require('./userProfileController'); + +describe('userProfileController deleteUserProfile', () => { + const userId = '65cf6c3706d8ac105827bb2e'; + const mockUserProfileModel = { + findById: jest.fn(), + deleteOne: jest.fn(), + }; + + const makeSut = () => userProfileController(mockUserProfileModel, {}); + + beforeEach(() => { + jest.clearAllMocks(); + hasPermission.mockResolvedValue(true); + canRequestorUpdateUser.mockResolvedValue(true); + mockUserProfileModel.findById.mockResolvedValue({ + _id: userId, + email: 'volunteer@example.org', + }); + mockUserProfileModel.deleteOne.mockResolvedValue({ deletedCount: 1 }); + followUp.findOneAndDelete.mockResolvedValue({}); + Task.updateMany.mockResolvedValue({ modifiedCount: 1 }); + Team.updateMany.mockResolvedValue({ modifiedCount: 1 }); + mockRes.status.mockReturnThis(); + + mockReq.body = { + ...mockReq.body, + userId, + option: 'delete', + role: 'Volunteer', + requestor: { + ...mockReq.body.requestor, + requestorId: '507f1f77bcf86cd799439011', + }, + }; + }); + + test('removes deleted users from tasks and teams using both string and ObjectId matches', async () => { + const { deleteUserProfile } = makeSut(); + + await deleteUserProfile(mockReq, mockRes); + + const expectedObjectId = new mongoose.Types.ObjectId(userId); + const expectedMatchedUserIds = [userId, expectedObjectId]; + + expect(mockUserProfileModel.deleteOne).toHaveBeenCalledWith({ _id: userId }); + expect(followUp.findOneAndDelete).toHaveBeenCalledWith({ userId }); + expect(Task.updateMany).toHaveBeenCalledWith( + { 'resources.userID': { $in: expectedMatchedUserIds } }, + { $pull: { resources: { userID: { $in: expectedMatchedUserIds } } } }, + ); + expect(Team.updateMany).toHaveBeenCalledWith( + { 'members.userId': { $in: expectedMatchedUserIds } }, + { $pull: { members: { userId: { $in: expectedMatchedUserIds } } } }, + ); + expect(mockRes.status).toHaveBeenCalledWith(200); + expect(mockRes.send).toHaveBeenCalledWith({ message: 'Executed Successfully' }); + }); +}); diff --git a/src/controllers/userProfileController.js b/src/controllers/userProfileController.js index bffbc73ff..cc9de5f5b 100644 --- a/src/controllers/userProfileController.js +++ b/src/controllers/userProfileController.js @@ -21,8 +21,7 @@ const Badge = require('../models/badge'); const yearMonthDayDateValidator = require('../utilities/yearMonthDayDateValidator'); const cacheClosure = require('../utilities/nodeCache'); const followUp = require('../models/followUp'); -const task = require('../models/task'); -const team = require('../models/team'); +const Task = require('../models/task'); const HGNFormResponses = require('../models/hgnFormResponse'); const userService = require('../services/userService'); const { hasPermission, canRequestorUpdateUser } = require('../utilities/permissions'); @@ -1429,24 +1428,19 @@ const createControllerMethods = function (UserProfile, Project, cache) { await UserProfile.deleteOne({ _id: userId }); // delete followUp for deleted user await followUp.findOneAndDelete({ userId }); - // Validate and convert userId to ObjectId for proper matching in MongoDB - let userIdObject; - try { - userIdObject = new mongoose.Types.ObjectId(userId); - } catch (idError) { - // If conversion fails, userId might already be an ObjectId or invalid - logger.logInfo(`Invalid userId format for ObjectId conversion: ${userId}`); - userIdObject = userId; + const matchedUserIds = [userId]; + if (mongoose.Types.ObjectId.isValid(userId)) { + matchedUserIds.push(new mongoose.Types.ObjectId(userId)); } // delete user from task-resources - await task.updateMany( - { 'resources.userID': userIdObject }, - { $pull: { resources: { userID: userIdObject } } }, + await Task.updateMany( + { 'resources.userID': { $in: matchedUserIds } }, + { $pull: { resources: { userID: { $in: matchedUserIds } } } }, ); // delete user from teams-members - await team.updateMany( - { 'members.userId': userIdObject }, - { $pull: { members: { userId: userIdObject } } }, + await Team.updateMany( + { 'members.userId': { $in: matchedUserIds } }, + { $pull: { members: { userId: { $in: matchedUserIds } } } }, ); res.status(200).send({ message: 'Executed Successfully' }); auditIfProtectedAccountUpdated({