Move mapping in controller and uc, create dto.

hpi-schul-cloud · Jan 12, 2024 · 4bfd818 · 4bfd818
1 parent 3891d5f
commit 4bfd818
Show file tree

Hide file tree

Showing 8 changed files with 289 additions and 36 deletions.
diff --git a/apps/server/src/modules/account/controller/account.controller.ts b/apps/server/src/modules/account/controller/account.controller.ts
@@ -13,6 +13,7 @@ import {
 	PatchMyAccountParams,
 	PatchMyPasswordParams,
 } from './dto';
+import { AccountResponseMapper } from './mapper/account-response.mapper';
 
 @ApiTags('Account')
 @Authenticate('jwt')
@@ -32,9 +33,9 @@ export class AccountController {
 		@CurrentUser() currentUser: ICurrentUser,
 		@Query() query: AccountSearchQueryParams
 	): Promise<AccountSearchListResponse> {
-		return this.accountUc.searchAccounts(currentUser, query);
+		const searchResult = await this.accountUc.searchAccounts(currentUser, query);
 
-		// TODO: mapping from domain to api dto should be a responsability of the controller (also every other function here)
+		return AccountResponseMapper.mapToAccountSearchListResponse(searchResult);
 	}
 
 	@Get(':id')
@@ -47,7 +48,8 @@ export class AccountController {
 		@CurrentUser() currentUser: ICurrentUser,
 		@Param() params: AccountByIdParams
 	): Promise<AccountResponse> {
-		return this.accountUc.findAccountById(currentUser, params);
+		const dto = await this.accountUc.findAccountById(currentUser, params);
+		return AccountResponseMapper.mapToAccountResponse(dto);
 	}
 
 	// IMPORTANT!!!
@@ -74,7 +76,8 @@ export class AccountController {
 		@Param() params: AccountByIdParams,
 		@Body() body: AccountByIdBodyParams
 	): Promise<AccountResponse> {
-		return this.accountUc.updateAccountById(currentUser, params, body);
+		const dto = await this.accountUc.updateAccountById(currentUser, params, body);
+		return AccountResponseMapper.mapToAccountResponse(dto);
 	}
 
 	@Delete(':id')
@@ -87,7 +90,8 @@ export class AccountController {
 		@CurrentUser() currentUser: ICurrentUser,
 		@Param() params: AccountByIdParams
 	): Promise<AccountResponse> {
-		return this.accountUc.deleteAccountById(currentUser, params);
+		const dto = await this.accountUc.deleteAccountById(currentUser, params);
+		return AccountResponseMapper.mapToAccountResponse(dto);
 	}
 
 	@Patch('me/password')

diff --git a/apps/server/src/modules/account/controller/mapper/account-response.mapper.spec.ts b/apps/server/src/modules/account/controller/mapper/account-response.mapper.spec.ts
@@ -1,9 +1,10 @@
 import { accountDtoFactory } from '@shared/testing';
 import { Account } from '../../domain';
 import { AccountResponseMapper } from './account-response.mapper';
+import { ResolvedSearchListAccountDto } from '../../uc/dto/resolved-account.dto';
 
 describe('AccountResponseMapper', () => {
-	describe('mapToResponse', () => {
+	describe('mapToAccountResponse', () => {
 		describe('When mapping Account to AccountResponse', () => {
 			const setup = () => {
 				const testDto: Account = accountDtoFactory.buildWithId();
@@ -13,7 +14,7 @@ describe('AccountResponseMapper', () => {
 			it('should map all fields', () => {
 				const testDto = setup();
 
-				const ret = AccountResponseMapper.mapToResponse(testDto);
+				const ret = AccountResponseMapper.mapToAccountResponse(testDto);
 
 				expect(ret.id).toBe(testDto.id);
 				expect(ret.userId).toBe(testDto.userId?.toString());
@@ -22,4 +23,50 @@ describe('AccountResponseMapper', () => {
 			});
 		});
 	});
+
+	describe('mapToAccountResponses', () => {
+		describe('When mapping Account[] to AccountResponse[]', () => {
+			const setup = () => {
+				const testDto: Account[] = accountDtoFactory.buildListWithId(3);
+				return testDto;
+			};
+
+			it('should map all fields', () => {
+				const testDto = setup();
+
+				const ret = AccountResponseMapper.mapToAccountResponses(testDto);
+
+				expect(ret.length).toBe(testDto.length);
+				expect(ret[0].id).toBe(testDto[0].id);
+				expect(ret[0].userId).toBe(testDto[0].userId?.toString());
+				expect(ret[0].activated).toBe(testDto[0].activated);
+				expect(ret[0].username).toBe(testDto[0].username);
+			});
+		});
+	});
+
+	describe('mapToAccountSearchListResponse', () => {
+		describe('When mapping ResolvedSearchListAccountDto to AccountSearchListResponse', () => {
+			const setup = () => {
+				const testDto = accountDtoFactory.buildWithId();
+				const searchListDto = new ResolvedSearchListAccountDto([testDto], 1, 0, 1);
+				return searchListDto;
+			};
+
+			it('should map all fields', () => {
+				const searchListDto = setup();
+
+				const ret = AccountResponseMapper.mapToAccountSearchListResponse(searchListDto);
+
+				expect(ret.data.length).toBe(searchListDto.data.length);
+				expect(ret.data[0].id).toBe(searchListDto.data[0].id);
+				expect(ret.data[0].userId).toBe(searchListDto.data[0].userId?.toString());
+				expect(ret.data[0].activated).toBe(searchListDto.data[0].activated);
+				expect(ret.data[0].username).toBe(searchListDto.data[0].username);
+				expect(ret.total).toBe(searchListDto.total);
+				expect(ret.skip).toBe(searchListDto.skip);
+				expect(ret.limit).toBe(searchListDto.limit);
+			});
+		});
+	});
 });
diff --git a/apps/server/src/modules/account/controller/mapper/account-response.mapper.ts b/apps/server/src/modules/account/controller/mapper/account-response.mapper.ts
@@ -1,14 +1,29 @@
-import { Account } from '@src/modules/account/domain/account';
-import { AccountResponse } from '../dto';
+import { AccountResponse, AccountSearchListResponse } from '../dto';
+import { ResolvedAccountDto, ResolvedSearchListAccountDto } from '../../uc/dto/resolved-account.dto';
 
 export class AccountResponseMapper {
-	static mapToResponse(account: Account): AccountResponse {
+	static mapToAccountResponse(resolvedAccount: ResolvedAccountDto): AccountResponse {
 		return new AccountResponse({
-			id: account.id ?? '',
-			userId: account.userId,
-			activated: account.activated,
-			username: account.username,
-			updatedAt: account.updatedAt,
+			id: resolvedAccount.id as string,
+			userId: resolvedAccount.userId,
+			activated: resolvedAccount.activated,
+			username: resolvedAccount.username ?? '',
+			updatedAt: resolvedAccount.updatedAt,
 		});
 	}
+
+	static mapToAccountResponses(resolvedAccounts: ResolvedAccountDto[]): AccountResponse[] {
+		return resolvedAccounts.map((resolvedAccount) => AccountResponseMapper.mapToAccountResponse(resolvedAccount));
+	}
+
+	static mapToAccountSearchListResponse(
+		resolvedSearchListAccountDto: ResolvedSearchListAccountDto
+	): AccountSearchListResponse {
+		return new AccountSearchListResponse(
+			AccountResponseMapper.mapToAccountResponses(resolvedSearchListAccountDto.data),
+			resolvedSearchListAccountDto.total,
+			resolvedSearchListAccountDto.skip,
+			resolvedSearchListAccountDto.limit
+		);
+	}
 }
diff --git a/apps/server/src/modules/account/uc/account.uc.spec.ts b/apps/server/src/modules/account/uc/account.uc.spec.ts
@@ -16,14 +16,14 @@ import {
 	AccountByIdBodyParams,
 	AccountByIdParams,
 	AccountResponse,
-	AccountSearchListResponse,
 	AccountSearchQueryParams,
 	AccountSearchType,
 } from '../controller/dto';
 import { Account } from '../domain';
 import { AccountEntityToDoMapper } from '../repo/mapper';
 import { AccountValidationService } from '../services/account.validation.service';
 import { AccountUc } from './account.uc';
+import { ResolvedSearchListAccountDto } from './dto/resolved-account.dto';
 
 describe('AccountUc', () => {
 	let module: TestingModule;
@@ -1059,7 +1059,7 @@ describe('AccountUc', () => {
 					{ userId: mockSuperheroUser.id } as ICurrentUser,
 					{ type: AccountSearchType.USER_ID, value: mockStudentUser.id } as AccountSearchQueryParams
 				);
-				const expected = new AccountSearchListResponse(
+				const expected = new ResolvedSearchListAccountDto(
 					[
 						new AccountResponse({
 							id: mockStudentAccount.id ?? '',
@@ -1073,7 +1073,7 @@ describe('AccountUc', () => {
 					0,
 					1
 				);
-				expect(accounts).toStrictEqual<AccountSearchListResponse>(expected);
+				expect(accounts).toStrictEqual<ResolvedSearchListAccountDto>(expected);
 			});
 		});
 
@@ -1110,8 +1110,8 @@ describe('AccountUc', () => {
 					{ userId: mockSuperheroUser.id } as ICurrentUser,
 					{ type: AccountSearchType.USER_ID, value: mockUserWithoutAccount.id } as AccountSearchQueryParams
 				);
-				const expected = new AccountSearchListResponse([], 0, 0, 0);
-				expect(accounts).toStrictEqual<AccountSearchListResponse>(expected);
+				const expected = new ResolvedSearchListAccountDto([], 0, 0, 0);
+				expect(accounts).toStrictEqual<ResolvedSearchListAccountDto>(expected);
 			});
 		});
 		describe('When search type is username', () => {

diff --git a/apps/server/src/modules/account/uc/account.uc.ts b/apps/server/src/modules/account/uc/account.uc.ts
@@ -11,23 +11,23 @@ import { Permission, RoleName } from '@shared/domain/interface';
 import { PermissionService } from '@shared/domain/service';
 import { EntityId } from '@shared/domain/types';
 import { UserRepo } from '@shared/repo';
-// TODO: module internals should be imported with relative paths
+// TODO: module internals should be imported with relative paths, AccountEntity should be moved to this module
 
 import { ICurrentUser } from '@modules/authentication';
 import { AccountService } from '..';
 import { AccountConfig } from '../account-config';
 import {
 	AccountByIdBodyParams,
 	AccountByIdParams,
-	AccountResponse,
-	AccountSearchListResponse,
 	AccountSearchQueryParams,
 	AccountSearchType,
 	PatchMyAccountParams,
 } from '../controller/dto';
 import { AccountResponseMapper } from '../controller/mapper/account-response.mapper';
 import { Account } from '../domain';
 import { AccountValidationService } from '../services/account.validation.service';
+import { ResolvedAccountDto, ResolvedSearchListAccountDto } from './dto/resolved-account.dto';
+import { AccountUcMapper } from './mapper/account-uc.mapper';
 
 type UserPreferences = {
 	// first login completed
@@ -59,7 +59,10 @@ export class AccountUc {
 	 * @throws {ValidationError}
 	 * @throws {ForbiddenOperationError}
 	 */
-	async searchAccounts(currentUser: ICurrentUser, query: AccountSearchQueryParams): Promise<AccountSearchListResponse> {
+	async searchAccounts(
+		currentUser: ICurrentUser,
+		query: AccountSearchQueryParams
+	): Promise<ResolvedSearchListAccountDto> {
 		const skip = query.skip ?? 0;
 		const limit = query.limit ?? 10;
 		const executingUser = await this.userRepo.findById(currentUser.userId, true);
@@ -70,9 +73,9 @@ export class AccountUc {
 			if (!(await this.isSuperhero(currentUser))) {
 				throw new ForbiddenOperationError('Current user is not authorized to search for accounts.');
 			}
-			const [accounts, total] = await this.accountService.searchByUsernamePartialMatch(query.value, skip, limit);
-			const accountList = accounts.map((tempAccount) => AccountResponseMapper.mapToResponse(tempAccount));
-			return new AccountSearchListResponse(accountList, total, skip, limit);
+			const searchDoCounted = await this.accountService.searchByUsernamePartialMatch(query.value, skip, limit);
+			const [searchResult, total] = AccountUcMapper.mapSearchResult(searchDoCounted);
+			return new ResolvedSearchListAccountDto(searchResult, total, skip, limit);
 		}
 		if (query.type === AccountSearchType.USER_ID) {
 			const targetUser = await this.userRepo.findById(query.value, true);
@@ -84,10 +87,10 @@ export class AccountUc {
 			const account = await this.accountService.findByUserId(query.value);
 			if (account) {
 				// HINT: skip and limit should be from the query
-				return new AccountSearchListResponse([AccountResponseMapper.mapToResponse(account)], 1, 0, 1);
+				return new ResolvedSearchListAccountDto([AccountResponseMapper.mapToAccountResponse(account)], 1, 0, 1);
 			}
 			// HINT: skip and limit should be from the query
-			return new AccountSearchListResponse([], 0, 0, 0);
+			return new ResolvedSearchListAccountDto([], 0, 0, 0);
 		}
 
 		throw new ValidationError('Invalid search type.');
@@ -101,12 +104,12 @@ export class AccountUc {
 	 * @throws {ForbiddenOperationError}
 	 * @throws {EntityNotFoundError}
 	 */
-	async findAccountById(currentUser: ICurrentUser, params: AccountByIdParams): Promise<AccountResponse> {
+	async findAccountById(currentUser: ICurrentUser, params: AccountByIdParams): Promise<ResolvedAccountDto> {
 		if (!(await this.isSuperhero(currentUser))) {
 			throw new ForbiddenOperationError('Current user is not authorized to search for accounts.');
 		}
 		const account = await this.accountService.findById(params.id);
-		return AccountResponseMapper.mapToResponse(account); // TODO: mapping should be done in controller
+		return AccountUcMapper.mapToResolvedAccountDto(account);
 	}
 
 	async saveAccount(dto: Account): Promise<void> {
@@ -126,7 +129,7 @@ export class AccountUc {
 		currentUser: ICurrentUser,
 		params: AccountByIdParams,
 		body: AccountByIdBodyParams
-	): Promise<AccountResponse> {
+	): Promise<ResolvedAccountDto> {
 		const executingUser = await this.userRepo.findById(currentUser.userId, true);
 		const targetAccount = await this.accountService.findById(params.id);
 
@@ -175,9 +178,8 @@ export class AccountUc {
 				throw new EntityNotFoundError(AccountEntity.name);
 			}
 		}
-		// TODO: mapping from domain to api dto should be a responsability of the controller
 
-		return AccountResponseMapper.mapToResponse(targetAccount);
+		return AccountUcMapper.mapToResolvedAccountDto(targetAccount);
 	}
 
 	/**
@@ -188,13 +190,13 @@ export class AccountUc {
 	 * @throws {ForbiddenOperationError}
 	 * @throws {EntityNotFoundError}
 	 */
-	async deleteAccountById(currentUser: ICurrentUser, params: AccountByIdParams): Promise<AccountResponse> {
+	async deleteAccountById(currentUser: ICurrentUser, params: AccountByIdParams): Promise<ResolvedAccountDto> {
 		if (!(await this.isSuperhero(currentUser))) {
 			throw new ForbiddenOperationError('Current user is not authorized to delete an account.');
 		}
 		const account: Account = await this.accountService.findById(params.id);
 		await this.accountService.delete(account.id ?? '');
-		return AccountResponseMapper.mapToResponse(account);
+		return AccountUcMapper.mapToResolvedAccountDto(account);
 	}
 
 	/**