feat(front): #3043 add 'from' fields in user registration form #3119
Conversation
cspriet
changed the title
…-asso into 3043-add-field-register-form
| @@ -208,4 +208,31 @@ describe("StructureFormStepController", () => { | |||
| expect(values.centralSomething).toBeUndefined(); | |||
| }); | |||
| }); | |||
|
|
|||
| describe("onUpdateRegistrationSrc", () => { | |||
There was a problem hiding this comment.
Il manque un mock sur ctrl.onUpdate non ?
There was a problem hiding this comment.
@mxmeunier je ne pense pas, ici je teste la fonction d'update sur le champ registrationSrc qui appelle elle même directement onUpdate.
Mais j'ai peut-être raté un truc dans la façon de tester.
There was a problem hiding this comment.
Par habitude, même si dans ce cas ça ne doit rien changer, on mock toutes les méthodes appelées par la méthode testée. Donc ici on devrait mock onUpdate (par exemple, imagine si ça changeait la valeur de values ça biaiserai le test)
There was a problem hiding this comment.
Ok oui c'est plus clair
| @@ -102,6 +112,26 @@ export default class StructureFormStepController { | |||
| this.dispatch(shouldBlockStep ? "error" : "valid"); | |||
| } | |||
|
|
|||
| onUpdateRegistrationSrc(values: Record<string, unknown>) { | |||
There was a problem hiding this comment.
On peut avoir du contexte / commentaire sur pourquoi on remplace registrationSrcEmail et registrationSrcDetail par une chaîne vide pour COLLEAGUES_HIERARCHY et OTHER ?
There was a problem hiding this comment.
Parce que sinon la donnée est sauvegardée même si la valeur de registrationSrc n'est pas la bonne.
J'ai hésité à le faire juste avant la sauvegarde effective ou ici. Tu en penses quoi ?
There was a problem hiding this comment.
Yes au final j'ai compris en lisant tout le code, mais un petit commentaire serait cool pour gagner du temps de compréhension si on revient dessus dans 3 mois
| bind:value={values.registrationSrc} /> | ||
| </div> | ||
|
|
||
| {#if values?.registrationSrc && values?.registrationSrc.includes(RegistrationSrcTypeEnum.COLLEAGUES_HIERARCHY)} |
There was a problem hiding this comment.
Normalement c'est au controller d'avoir la logique; il faut une méthode ctrl.isRegisteredFromColleaguesOrHierarchy + ctrl.isRegisteredFromOther ou équivalent pour enlever la logique des fichiers svelte
There was a problem hiding this comment.
@mxmeunier ok, oui ce sera plus propre effectivement.
alice-telescoop
changed the title
| if (registerSrcValue && registerSrcValue.includes(RegistrationSrcTypeEnum.COLLEAGUES_HIERARCHY)) { | ||
| return true; | ||
| } | ||
| return false; |
There was a problem hiding this comment.
| if (registerSrcValue && registerSrcValue.includes(RegistrationSrcTypeEnum.COLLEAGUES_HIERARCHY)) { | |
| return true; | |
| } | |
| return false; | |
| return registerSrcValue && registerSrcValue.includes(RegistrationSrcTypeEnum.COLLEAGUES_HIERARCHY) |
| isRegistrationSrcDetailsVisible(registerSrcValue: RegistrationSrcTypeEnum[]) { | ||
| if (registerSrcValue && registerSrcValue.includes(RegistrationSrcTypeEnum.OTHER)) { | ||
| return true; | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
| isRegistrationSrcDetailsVisible(registerSrcValue: RegistrationSrcTypeEnum[]) { | |
| if (registerSrcValue && registerSrcValue.includes(RegistrationSrcTypeEnum.OTHER)) { | |
| return true; | |
| } | |
| return false; | |
| } | |
| isRegistrationSrcDetailsVisible(registerSrcValue: RegistrationSrcTypeEnum[]) { | |
| return registerSrcValue && registerSrcValue.includes(RegistrationSrcTypeEnum.OTHER) | |
| } |
Ça c'est bon, ça passe dedans, par contre il y a un problème (au moins) à la sauvegarde. |
No description provided.