BC-11265 - hide deleted users

.github/workflows/test.yml

@@ -47,9 +47,9 @@ jobs:
           - 5672:5672
     steps:
       - name: checkout
-        uses: actions/checkout@v5
+        uses: actions/checkout@v6
       - name: Setup node
-        uses: actions/setup-node@v5
+        uses: actions/setup-node@v6
         with:
           node-version: ${{ env.NODE_VERSION }}
       - name: Start MongoDB

apps/server/src/modules/deletion/api/controller/api-test/deletion-request-create.api.spec.ts

@@ -131,6 +131,14 @@ describe(`deletionRequest create (api)`, () => {
 			expect(account?.deactivatedAt).toBeDefined();
 		});
 
+		it('should flag user as deleted until actual deletion is performed', async () => {
+			const { deletionRequestToCreate } = await setup();
+
+			await testApiClient.post('', deletionRequestToCreate);
+			const account = await em.findOne(AccountEntity, { userId: new ObjectId(deletionRequestToCreate.targetRef.id) });
+			expect(account?.deactivatedAt).toBeDefined();
+		});
+
 		describe('when the "delete after minutes" param has not been provided', () => {
 			it('should set the "deleteAfter" date to the date after the default DELETION_DELETE_AFTER_MINUTES ', async () => {
 				const { deletionRequestToCreate, defaultDeleteAfterMinutes, operationalTimeToleranceInSeconds } = await setup();

apps/server/src/modules/deletion/api/controller/api-test/deletion-request-public-create.api.spec.ts

@@ -7,6 +7,7 @@ import { TestApiClient } from '@testing/test-api-client';
 import { AccountEntity } from '@modules/account/repo';
 import { UserAndAccountTestFactory } from '@testing/factory/user-and-account.test.factory';
 import { DeletionRequestParams } from '../dto';
+import { User } from '@modules/user/repo';
 
 const baseRouteName = '/deletionRequestsPublic';
 
@@ -85,6 +86,15 @@ describe(`deletionRequest public create (api)`, () => {
 			expect(account?.deactivatedAt).toBeDefined();
 		});
 
+		it('should flag user as deleted until actual deletion is performed', async () => {
+			const { studentUser, queryString, loggedInClient } = await setup();
+
+			await loggedInClient.delete(`?${queryString}`);
+
+			const user = await em.findOne(User, { id: studentUser.id });
+			expect(user?.deletedAt).toBeDefined();
+		});
+
 		it('should not fail if the target user has no account', async () => {
 			const { studentUser, loggedInClient } = await setup();
 			const { studentUser: studentUser2 } = UserAndAccountTestFactory.buildStudent({ school: studentUser.school });

apps/server/src/modules/deletion/api/uc/deletion-request-public.uc.ts

@@ -72,8 +72,10 @@ export class DeletionRequestPublicUc {
 			deleteAfter
 		);
 
+		const deleteAt = new Date();
+		await this.userService.flagAsDeleted(targetRefId, deleteAt);
 		try {
-			await this.accountService.deactivateAccount(targetRefId, new Date());
+			await this.accountService.deactivateAccount(targetRefId, deleteAt);
 		} catch (error) {
 			// it can be the user has no account (either deleted or never finished the registration process)
 			this.logger.error({

apps/server/src/modules/deletion/api/uc/deletion-request.uc.spec.ts

@@ -1,4 +1,5 @@
 import { LegacyLogger } from '@core/logger';
+import { NotFoundException } from '@nestjs/common';
 import { createMock, DeepMocked } from '@golevelup/ts-jest';
 import { ObjectId } from '@mikro-orm/mongodb';
 import { AccountService } from '@modules/account';
@@ -23,6 +24,7 @@ describe(DeletionRequestUc.name, () => {
 	let deletionRequestService: DeepMocked<DeletionRequestService>;
 	let deletionLogService: DeepMocked<DeletionLogService>;
 	let config: DeletionConfig;
+	let legacyLogger: DeepMocked<LegacyLogger>;
 	let deletionExecutionService: DeepMocked<DeletionExecutionService>;
 	let accountService: DeepMocked<AccountService>;
 	let userService: DeepMocked<UserService>;
@@ -68,6 +70,7 @@ describe(DeletionRequestUc.name, () => {
 		deletionRequestService = module.get(DeletionRequestService);
 		deletionLogService = module.get(DeletionLogService);
 		config = module.get(DELETION_CONFIG_TOKEN);
+		legacyLogger = module.get(LegacyLogger);
 		deletionExecutionService = module.get(DeletionExecutionService);
 		accountService = module.get(AccountService);
 		userService = module.get(UserService);
@@ -139,6 +142,14 @@ describe(DeletionRequestUc.name, () => {
 				});
 			});
 
+			it('should call userService.flagAsDeleted if domain is DomainName.User', async () => {
+				const { deletionRequestToCreate } = setup();
+
+				await uc.createDeletionRequest(deletionRequestToCreate);
+
+				expect(userService.flagAsDeleted).toHaveBeenCalledWith(deletionRequestToCreate.targetRef.id, expect.any(Date));
+			});
+
 			it('should call accountService.deactivateAccount if domain is DomainName.User', async () => {
 				const { deletionRequestToCreate } = setup();
 
@@ -149,6 +160,15 @@ describe(DeletionRequestUc.name, () => {
 					expect.any(Date)
 				);
 			});
+
+			it('should log a warning if account is not found', async () => {
+				const { deletionRequestToCreate } = setup();
+				accountService.deactivateAccount.mockRejectedValueOnce(new NotFoundException());
+
+				await uc.createDeletionRequest(deletionRequestToCreate);
+
+				expect(legacyLogger.warn).toHaveBeenCalled();
+			});
 		});
 	});
 

apps/server/src/modules/deletion/api/uc/deletion-request.uc.ts

@@ -1,7 +1,7 @@
 import { LegacyLogger } from '@core/logger';
 import { AccountService } from '@modules/account';
 import { UserService } from '@modules/user';
-import { Inject, Injectable } from '@nestjs/common';
+import { Inject, Injectable, NotFoundException } from '@nestjs/common';
 import { EntityId } from '@shared/domain/types';
 import { DELETION_CONFIG_TOKEN, DeletionConfig } from '../../deletion.config';
 import { DomainDeletionReportBuilder } from '../../domain/builder';
@@ -45,7 +45,19 @@ export class DeletionRequestUc {
 		);
 
 		if (deletionRequest.targetRef.domain === DomainName.USER) {
-			await this.accountService.deactivateAccount(deletionRequest.targetRef.id, new Date());
+			const deleteAt = new Date();
+			await this.userService.flagAsDeleted(deletionRequest.targetRef.id, deleteAt);
+			try {
+				await this.accountService.deactivateAccount(deletionRequest.targetRef.id, deleteAt);
+			} catch (error) {
+				if (error instanceof NotFoundException) {
+					this.logger.warn({
+						action: 'createDeletionRequest',
+						message: error.message,
+						deletionRequest,
+					});
+				}
+			}
 		}
 
 		return result;

apps/server/src/modules/user/domain/service/user.service.ts

@@ -210,6 +210,10 @@ export class UserService {
 		return userDeleted;
 	}
 
+	public async flagAsDeleted(userId: EntityId, deletedAt?: Date): Promise<void> {
+		await this.userRepo.flagAsDeleted(userId, deletedAt);
+	}
+
 	public async getParentEmailsFromUser(userId: EntityId): Promise<string[]> {
 		const parentEmails = await this.userRepo.getParentEmailsFromUser(userId);
 

apps/server/src/modules/user/legacy/repo/helper/search-query-helper.ts

@@ -21,4 +21,8 @@ export class SearchQueryHelper {
 			}
 		}
 	}
+
+	public static setDeletedFilter(query: UserSearchQuery, deletionDate?: Date): void {
+		query['$or'] = [{ deletedAt: { $exists: false } }, { deletedAt: null }, { deletedAt: { $gte: deletionDate } }];
+	}
 }

apps/server/src/modules/user/legacy/repo/users-admin.repo.ts

@@ -43,6 +43,8 @@ export class UsersAdminRepo extends BaseRepo<User> {
 			],
 		};
 
+		SearchQueryHelper.setDeletedFilter(query, new Date());
+
 		const aggregation = createMultiDocumentAggregation(query);
 
 		return this._em.aggregate(User, aggregation);
@@ -87,6 +89,8 @@ export class UsersAdminRepo extends BaseRepo<User> {
 		SearchQueryHelper.setSearchParametersIfExist(query, params);
 		SearchQueryHelper.setDateParametersIfExists(query, params);
 
+		SearchQueryHelper.setDeletedFilter(query, new Date());
+
 		const aggregation = createMultiDocumentAggregation(query);
 
 		return this._em.aggregate(User, aggregation);

apps/server/src/modules/user/repo/scope/user.scope.spec.ts

@@ -325,7 +325,9 @@ describe('UserScope', () => {
 			it('should add a query that removes deleted users', () => {
 				scope1.withDeleted(false);
 
-				expect(scope1.query).toEqual({ $or: [{ deletedAt: { $exists: false } }, { deletedAt: null }] });
+				expect(scope1.query).toEqual({
+					$or: [{ deletedAt: { $exists: false } }, { deletedAt: null }, { deletedAt: { $gte: expect.any(Date) } }],
+				});
 			});
 		});
 	});

apps/server/src/modules/user/repo/scope/user.scope.ts

@@ -95,7 +95,9 @@ export class UserScope extends Scope<User> {
 
 	public withDeleted(deleted?: boolean): UserScope {
 		if (!deleted) {
-			this.addQuery({ $or: [{ deletedAt: { $exists: false } }, { deletedAt: null }] });
+			this.addQuery({
+				$or: [{ deletedAt: { $exists: false } }, { deletedAt: null }, { deletedAt: { $gte: new Date() } }],
+			});
 		}
 
 		return this;

apps/server/src/modules/user/repo/user-do.repo.integration.spec.ts

@@ -609,14 +609,19 @@ describe('UserRepo', () => {
 		});
 
 		const setupFind = async () => {
-			const query = {
+			let query = {
 				schoolId: undefined,
 				isOutdated: undefined,
 				lastLoginSystemChangeSmallerThan: undefined,
 				outdatedSince: undefined,
 				lastLoginSystemChangeBetweenEnd: undefined,
 				lastLoginSystemChangeBetweenStart: undefined,
 			};
+			const deletedFilter = {
+				$or: [{ deletedAt: { $exists: false } }, { deletedAt: null }, { deletedAt: { $gte: expect.any(Date) } }],
+			};
+
+			query = { ...query, ...deletedFilter };
 
 			const options: IFindOptions<UserDo> = {};
 
@@ -631,12 +636,12 @@ describe('UserRepo', () => {
 
 			const emFindAndCountSpy = jest.spyOn(em, 'findAndCount');
 
-			return { query, options, users, emFindAndCountSpy };
+			return { query, options, users, emFindAndCountSpy, deletedFilter };
 		};
 
 		describe('sorting', () => {
 			it('should create queryOrderMap with options.order', async () => {
-				const { query, options, emFindAndCountSpy } = await setupFind();
+				const { query, options, emFindAndCountSpy, deletedFilter } = await setupFind();
 				options.order = {
 					id: SortOrder.asc,
 				};
@@ -645,22 +650,22 @@ describe('UserRepo', () => {
 
 				expect(emFindAndCountSpy).toHaveBeenCalledWith(
 					User,
-					{},
+					deletedFilter,
 					expect.objectContaining<FindOptions<User>>({
 						orderBy: expect.objectContaining<QueryOrderMap<User>>({ _id: options.order.id }) as QueryOrderMap<User>,
 					})
 				);
 			});
 
 			it('should create queryOrderMap with an empty object', async () => {
-				const { query, options, emFindAndCountSpy } = await setupFind();
+				const { query, options, emFindAndCountSpy, deletedFilter } = await setupFind();
 				options.order = undefined;
 
 				await repo.find(query, options);
 
 				expect(emFindAndCountSpy).toHaveBeenCalledWith(
 					User,
-					{},
+					deletedFilter,
 					expect.objectContaining<FindOptions<User>>({
 						orderBy: expect.objectContaining<QueryOrderMap<User>>({}) as QueryOrderMap<User>,
 					})
@@ -741,6 +746,9 @@ describe('UserRepo', () => {
 					lastLoginSystemChangeBetweenStart: new Date(),
 					lastLoginSystemChangeBetweenEnd: new Date(),
 				};
+				const deletedFilter = {
+					$or: [{ deletedAt: { $exists: false } }, { deletedAt: null }, { deletedAt: { $gte: expect.any(Date) } }],
+				};
 
 				const options: IFindOptions<UserDo> = {};
 
@@ -755,11 +763,11 @@ describe('UserRepo', () => {
 
 				const emFindAndCountSpy = jest.spyOn(em, 'findAndCount');
 
-				return { query, options, users, emFindAndCountSpy };
+				return { query, options, users, emFindAndCountSpy, deletedFilter };
 			};
 
 			it('should add query to scope', async () => {
-				const { query, options, emFindAndCountSpy } = await setup();
+				const { query, options, emFindAndCountSpy, deletedFilter } = await setup();
 
 				await repo.find(query, options);
 
@@ -800,6 +808,7 @@ describe('UserRepo', () => {
 									$eq: query.outdatedSince,
 								},
 							},
+							deletedFilter,
 						],
 					},
 					expect.objectContaining<FindOptions<User>>({

apps/server/src/modules/user/repo/user-do.repo.ts

@@ -38,6 +38,7 @@ export class UserDoMikroOrmRepo extends BaseDORepo<UserDo, User> implements User
 				query.lastLoginSystemChangeBetweenEnd
 			)
 			.withOutdatedSince(query.outdatedSince)
+			.withDeleted(false)
 			.allowEmptyQuery(true);
 
 		order._id = order._id ?? SortOrder.asc;

apps/server/src/modules/user/repo/user.repo.ts

@@ -88,6 +88,11 @@ export class UserMikroOrmRepo extends BaseRepo<User> {
 		return deletedUserNumber;
 	}
 
+	public async flagAsDeleted(userId: EntityId, deletedDate?: Date): Promise<void> {
+		const deletedAt = deletedDate ?? new Date();
+		await this._em.nativeUpdate(this.entityName, { id: userId }, { deletedAt });
+	}
+
 	public async getParentEmailsFromUser(userId: EntityId): Promise<string[]> {
 		const user: User | null = await this._em.findOne(User, { id: userId });
 		let parentsEmails: string[] = [];

src/services/user/services/userService.js

@@ -92,6 +92,13 @@ const populateWhitelist = {
 	schoolId: ['_id', 'name'],
 };
 
+const notDeletedFilter = (context) => {
+	context.params.query.$and = context.params.query.$and || [];
+	context.params.query.$and.push({
+		$or: [{ deletedAt: { $exists: false } }, { deletedAt: null }, { deletedAt: { $gte: new Date() } }]
+	});
+}
+
 const userHooks = {
 	before: {
 		all: [],
@@ -103,6 +110,7 @@ const userHooks = {
 			iff(isProvider('external'), restrictToCurrentSchool),
 			iff(isProvider('external'), getRestrictPopulatesHook(populateWhitelist)),
 			mapRoleFilterQuery,
+			notDeletedFilter,
 			addCollation,
 			iff(isProvider('external'), includeOnlySchoolRoles),
 		],