EW-777 working on review account module Refactoring part #5406
Conversation
|
|
||
| class AccountRepoSpec extends AccountRepo { |
There was a problem hiding this comment.
Rename to "AccountTestRepo" to make its use better visible
| export class AccountRepo { | ||
| constructor(private readonly em: EntityManager) {} | ||
| export class AccountRepo extends BaseDomainObjectRepo<Account, AccountEntity> { | ||
| protected mapDOToEntityProperties(entityDO: Account): EntityData<AccountEntity> { |
There was a problem hiding this comment.
Place this method after the constructor and getter.
|
|
||
| return entityProps; | ||
| } | ||
|
|
||
| public async save(account: Account): Promise<Account> { |
There was a problem hiding this comment.
Some methods are no longer needed now that this class derives from BaseDomainObjectRepo.
Check if there are methods already in BaseDomainObjectRepo like save and saveAll that can be removed here
| mapDOToEntityPropertiesSpec(entityDO: Account): EntityData<AccountEntity> { | ||
| return super.mapDOToEntityProperties(entityDO); | ||
| } | ||
| } | ||
| describe('account repo', () => { |
There was a problem hiding this comment.
Use real name of class: AccountRepo
apps/server/src/modules/account/domain/services/account.service.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/account/domain/services/account.service.ts
Outdated
Show resolved
Hide resolved
apps/server/src/modules/account/domain/services/account.service.ts
Outdated
Show resolved
Hide resolved
| @@ -13,7 +13,7 @@ export class AccountByIdBodyParams { | |||
| required: false, | |||
| nullable: true, | |||
| }) | |||
| username?: string; | |||
There was a problem hiding this comment.
Use @ApiPropertyOptional for optional values (required: false, nullable: true) in this file
| @@ -21,7 +21,7 @@ export class PatchMyAccountParams { | |||
| required: false, | |||
| nullable: true, | |||
| }) | |||
| passwordNew?: string; | |||
| public passwordNew?: string; | |||
There was a problem hiding this comment.
Use @ApiPropertyOptional for optional values (required: false, nullable: true) in this file
|
|
||
| export class ResolvedAccountDto { | ||
| @IsOptional() | ||
| @ApiPropertyOptional() |
There was a problem hiding this comment.
Not sure if we need ApiPropertyOptional here
|
Description
Links to Tickets or other pull requests
https://ticketsystem.dbildungscloud.de/browse/EW-777
Approval for review
generate-client:serverwas executed in vue frontend and changes were tested and put in a PR with the same branch name.