pixilib / gaelo_flow_backend Goto Github PK

pour les test end2end il faut démarrer tout le backend, il devraient tout démarer avec l'app module,
Par contre on inject de env prédéfini pour que la db soit sqlite

` async update(
@param('name') name: string,
@Body() roleDto: RoleDto,
): Promise {
const role = await this.roleService.findOneByOrFail(name);

const requiredKeys = [
  'import',
  'anonymize',
  'export',
  'query',
  'autoQuery',
  'delete',
  'admin',
  'modify',
  'cdBurner',
  'autoRouting',
];`

A faire via un DTO + validator plutot

` async findAll(@query() withLabels: WithLabels): Promise<Role[]> {
const allRoles = await this.roleService.findAll();
if (withLabels.WithLabels) {
const allRoleLabels = await this.roleService.getAllRoleLabels();
allRoles.forEach((role) => {
role['labels'] = allRoleLabels
.filter((roleLabel) => roleLabel.Role.Name === role.Name)
.map((roleLabel) => roleLabel.Label.Name);
});
}

return allRoles;

}`

C'est pas fou comme factorisation, si tu a la propriété withLabel le mieux est d'avoir dans le roleService une méthode "findAllWithLabel" qui integrera la jointure directement
ou alors dans findAll ajouter un boolean withLabel pour avoir ou non la jointure

Ca permet de faire une seule requette sql au lieu de 2 et de ne pas avoir à refill la jointure dans ton controlleur

GaelO-Flow/src/auth/dto/

@UseInterceptors(NotFoundInterceptor) @Post('/:roleName/label') async addLabelToRole( @Param('roleName') roleName: string, @Body() labelDto: LabelDto, ): Promise<void> { const role = await this.roleService.findOneByOrFail(roleName);

Pas fou cette const role non utilisée, mieux de faire roleService.IsExiste(role) qui renvoit un boolean et le controlleur se charge de renvoyer 404 si le retour est false

user.entity -> Remove property SuperAdmin and Username (use email for validation)

update-user.dto -> add Email, RoleName

A faire ensemble une journée

Ca a l'air d'etre la convention NestJS
Par exemple dans leurs sources : https://github.com/nestjs/nest/tree/master/packages/platform-fastify

` async getOauthConfig() {
const oauthConfig = await this.oauthConfigService.getOauthConfig();
const oauthConfigDto = {};
for (const config of oauthConfig) {
oauthConfigDto[config.Provider] = {
AuthorizationUrl: config.AuthorizationUrl,
ClientId: config.ClientId,
};
}

return oauthConfigDto;

}`

Pas sur qu'on ait besoin d'indexer les auth config par provider, on peut simplement rendre un array d'object avec Provider, AuthorizationUrl, ClientId

• Refac OrGuards
• Guard par ressource Orthanc
• Guard DicomWeb

Dans cette class on a deux cron job qui ne font pas la meme chose avec des injection de dépendances différentes

Je pense qu'il faut mieux les dissocier dans deux fichiers, task-monitoring.service et task-autoquery.service
et referencer ces ceux dans le meme tasks.module qu'on a

async addJob(
@Req() request: Request,
@Body() processingJobDto: ProcessingJobDto,
): Promise {

Deplacer tous les client HTTP dans utils, vu qu'ils peuvent etre appelés partout

// check if all the keys are present if ( !userDto.Firstname == undefined || !userDto.Lastname == undefined || !userDto.Username == undefined || !userDto.Email == undefined || !userDto.Password == undefined || !userDto.SuperAdmin == undefined || !userDto.RoleName == undefined ) { throw new BadRequestException('All the keys are required'); }

Ca devrait pas plutot etre verifié par le DTO ?

@Delete('/:roleName/label/:label') async removeLabelFromRole( @Param('roleName') roleName: string, @Param('label') label: string, ): Promise<void> { console.log(roleName, label); }

Dans Userservice la methode create utilise un insert alors que les autres service utilisent save.

Le create de userservice renvoit un id et pas toute l'entité user crée,

ca serait mieux de tout utiliser en save et retourner l'entitée crée complete sur cette méthode

` const requiredKeys = [
'import',
'anonymize',
'export',
'query',
'autoQuery',
'delete',
'admin',
'modify',
'cdBurner',
'autoRouting',
];

requiredKeys.forEach((key) => {
  if (roleDto[key] === undefined) {
    throw new BadRequestException(`Missing key '${key}' in roleDto`);
  } else {
    role[key] = roleDto[key];
  }
});`

Pareil ca peut pas etre checké par un DTO ?

A renommer pour suivre la convention

`describe('UsersController (e2e)', () => {
let app: INestApplication;
let userRepository: Repository;

let adminToken: string = '';
let adminId: number = 0;

beforeEach(async () => {
const moduleFixture: TestingModule = await Test.createTestingModule({
imports: [AppModule],
}).compile();

app = moduleFixture.createNestApplication();
await app.init();

userRepository = moduleFixture.get<Repository<User>>(
  getRepositoryToken(User),
);

const hash: string = await bcryptjs.hash(
  'passwordadmin',
  await bcryptjs.genSalt(),
);`

const hash: string = await bcryptjs.hash(
  'passwordadmin',
  await bcryptjs.genSalt(),
)

=> ici tu peux passer par les utils

tu a besoin de lancer ca dans le constructeur ?
(async () => { this.lastChanges = (await this.orthancClient.getLastChanges()).data.Last; })();

Le every 10 sec il va pas le faire 10 secondes apres avoir construit le service de toute facon ? (car si ca met 10 secondes a faire le 1er appel c'est pas bien grave)

il y a ces lignes duplliquées plusieurs fois

const salt = await bcryptjs.genSalt(); const hash = await bcryptjs.hash(userDto.Password, salt);

Je pense qu'il vaut mieux encapsuter ca dans une fonction hashPassword, soit dans UserService soit dans les utils, et dans tes tests tu mock cette fonction pour renvoyer une string de ton choix pour tes tests, ca simplifiera le code et les tetss

async removeJob(jobData: object): Promise<void> { console.log(jobData); }

On aura besoin d'une route pour enlever un job processing (si il n'est pas démarré), faut implementer ce remove et ouvrir un controlleur pour delete un job processing (via son id)

On a des guards eclaté dans role, dans authorization,

Mieux vaut rassembler les guards dans un meme repertoire, ca simplifira la compréhension du projet et ca a l'air d'etre une bonne pratique en Nest

"We now will create a folder named guards under src folder" https://medium.com/@shkim04/implement-guards-in-nestjs-8c654173bbe9

` async validateUser(username: string, pass: string): Promise {
const user = await this.usersService.findOneByUsername(username);

if (user && (await bcryptjs.compare(pass, user.Password))) {
  const { Password, ...result } = user;
  return result;
}
return null;

}`

Ici aussi plutot que de passer par bcrypt pour valider le password le faire dans un wrapper dans le utils qui renvoi un boolean si le password et le bon et comme ca tu pourra le mocker facilement pour les tests

• Maybe repair of nodeMailer
• Response is 500 for api

@ApiProperty() @Column({ name: 'value' }) // TODO: VALIDATOR ? @IsString() Value: string | number;

async getOptions(): Promise<Option> { const options = await this.optionService.getOptions(); delete options.Id;

Pourquoi ne pas maquer le id comme on fait avec le password plutot que de le delete à la main ?

@Get('/:id') @UseInterceptors(NotFoundInterceptor) async getUsersId(@Param('id') id: number): Promise<GetUserDto> { const user = await this.UserService.findOne(id); return { ...user, Password: undefined }; }

Pareil ici je pense que soit le password est protégé via le Exclude de userDTO soit y doit y avoir un moyen de dire que GetUserDto est un UserDto moins le password (pour eviter de vider manuellement le password dans le controller)

On va avoir besoin d'un service de monitoring d'orthanc,

Le but de ce service va etre de surveiller Orthanc via son API change pour emmetre des event quand il y a un nouveau patient, study, series (en utilisant l'evenement onStable)

Differents autres services vont écouter ces evenements dans le but de :

• Automatiquement faire des inferences suivant certaines regles
• Faire de l'auto routing
• (lancer une gravure)

https://abrialstha.medium.com/file-naming-conventions-in-nestjs-88ae309aa299

C'est normal de pas avoir la jointure, c'est swagger qu'il faut corriger

A renommer

y a l'air d'avoir le repertoire constants qui est la de base pour définir toute les contants, y mettre nos enums (QueryAnswerType, TagPoliciesn ProcessingMaskEnum, ProcessingMaskEnum)

Les harmoniser je suis pas sur qu'il faille les faire finir par enum, regarder dans nest comment sont nommées leurs enums

On a des appels a bcrypt et crypto un peu partout dans le code

J'ai fait un utils password pour isoler ces appels en une fonction de hashage et generation de token

Il faut updater le code pour utiliser ces fonction, ca permet de limiter les import de crypto et bcrypt et pour les test mocker ce utils plutot que de mocker crypto/bcrypt

et les probleme de circular deps