Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TECH] Mise à jour de la dépendance Joi dans l'API #704

Closed
wants to merge 1 commit into from

Conversation

laura-bergoens
Copy link
Member

🦄 Problème

La version de la dépendance de Joi incluse dans l'API est dépréciée. De plus, la dépendance a été déplacée vers @hapi/joi.

🤖 Solution

Modification de la dépendance de joi vers @hapi/joi
Mise à jour de 14.3.1 vers 16.0.0
Adaptation de la validation des competence-marks pour fonctionner avec la nouvelle version.

🌈 Remarques

Changelog :
de 14.3.1 vers 16.0.0 : hapijs/joi#2037

@pix-service
Copy link
Contributor

@@ -34,8 +34,8 @@ class CompetenceMark {
}

validate() {
const result = Joi.validate(this, schemaValidateCompetenceMark);
if (result.error === null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attention, cela peut changer le fonctionnel.

joi package was deprecated and now has moved to @hapi/joi.
Joi validation was used in CompetenceMark. Changes
happened to remain compliant.
const result = Joi.validate(this, schemaValidateCompetenceMark);
if (result.error === null) {
const result = schemaValidateCompetenceMark.validate(this);
if (result.error === undefined) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Qu'est-ce qui fait qu'on souhaite avoir un comportement différent pour null et pour undefined ?

@sroccaserra
Copy link
Contributor

Liste des modèles Joi que j'ai trouvés :

lib/domain/models/CompetenceMark.js
lib/domain/validators/campaign-validator.js
lib/domain/validators/session-validator.js
lib/domain/validators/user-validator.js
lib/domain/validators/organization-creation-validator.js
lib/application/passwords/index.js
lib/application/users/index.js
lib/application/authentication/index.js

@sroccaserra
Copy link
Contributor

sroccaserra commented Sep 16, 2019

Il reste des require('joi') à changer en require('@hapi/joi'). Attention, je pense que ça ne se voit pas car il nous reste des dépendances qui dépendent de 'joi' donc il se trouve toujours dans nos node_modules.
Ici :

lib/application/passwords/index.js
lib/application/authentication/index.js
lib/application/users/index.js
lib/domain/validators/campaign-validator.js
lib/domain/validators/user-validator.js
lib/domain/validators/session-validator.js
lib/domain/validators/organization-creation-validator.js

@sroccaserra
Copy link
Contributor

sroccaserra commented Sep 16, 2019

On peut renommer string().regex() en string().pattern() (pas obligé, il y a un alias, c'est listé dans les breaking changes cependant).
Ici :

lib/application/users/index.js
lib/domain/validators/session-validator.js

Copy link
Contributor

@sroccaserra sroccaserra left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Il y a des requires à corriger et des breaking changes à faire

@sroccaserra
Copy link
Contributor

sroccaserra commented Sep 16, 2019

Je pense qu'il vaut mieux passer à Hapi 18.3.3 ou 18.4.0 avant de faire ce changement. Attention : "hapi" est devenu "@hapi/hapi" sur npm.

@sroccaserra
Copy link
Contributor

sroccaserra commented Sep 16, 2019

Le truc le plus casse-pieds à faire sera probablement de mettre à jour l'extension qu'on utilise pour valider les passwords (Joi.extend() a bien changé).

Tentative non terminée (des tests échouent encore) :

diff --git a/api/lib/domain/validators/user-validator.js b/api/lib/domain/validators/user-validator.js
index 326767fbd..e079dccec 100644
--- a/api/lib/domain/validators/user-validator.js
+++ b/api/lib/domain/validators/user-validator.js
@@ -6,20 +6,18 @@ const validationConfiguration = { abortEarly: false, allowUnknown: true };

 // TODO this check on password format should be done in create user use case and not user validation
 const joiWithPasswordValidation = Joi.extend((joi) => ({
-  name: 'string',
+  type: 'string',
   base: joi.string(),
-  rules: [
-    {
-      name: 'password',
-      validate(params, value, state, options) {
+  rules: {
+    password: {
+      validate(value, helpers) {
         const isValid = validatePassword(value);
         if (!isValid) {
-          return this.createError('string.password', { v: value }, state, options);
+          return { value, errors: helpers.error('string.password') };
         }
-        return value;
       }
     }
-  ]
+  }
 }));

 const userValidationJoiSchema = Joi.object().keys({

@laura-bergoens
Copy link
Member Author

Implique d'abord une mise à jour de Hapi

@VincentHardouin VincentHardouin deleted the tech-update-joi branch October 13, 2022 07:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants