-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat(front): #3043 add 'from' fields in user registration form #3119
base: develop
Are you sure you want to change the base?
Changes from 3 commits
352c8a2
c46e064
f75cbab
e9957e4
6e2b094
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { AgentTypeEnum } from "dto"; | ||
import { AgentTypeEnum, RegistrationSrcTypeEnum } from "dto"; | ||
import type { MockInstance } from "vitest"; | ||
import { beforeEach } from "vitest"; | ||
import StructureFormStepController from "./StructureFormStep.controller"; | ||
|
@@ -208,4 +208,31 @@ describe("StructureFormStepController", () => { | |
expect(values.centralSomething).toBeUndefined(); | ||
}); | ||
}); | ||
|
||
describe("onUpdateRegistrationSrc", () => { | ||
it("registrationSrcEmail and registrationSrcDetails are set to empty strings", () => { | ||
const values = { | ||
registrationSrc: [RegistrationSrcTypeEnum.DEMO], | ||
registrationSrcEmail: "[email protected]", | ||
registrationSrcDetails: "Other", | ||
}; | ||
const expected = { | ||
registrationSrc: [RegistrationSrcTypeEnum.DEMO], | ||
registrationSrcEmail: "", | ||
registrationSrcDetails: "", | ||
}; | ||
ctrl.onUpdateRegistrationSrc(values); | ||
expect(values).toStrictEqual(expected); | ||
}); | ||
it("registrationSrcEmail and registrationSrcDetails are not set to empty strings", () => { | ||
const values = { | ||
registrationSrc: [RegistrationSrcTypeEnum.COLLEAGUES_HIERARCHY, RegistrationSrcTypeEnum.OTHER], | ||
registrationSrcEmail: "[email protected]", | ||
registrationSrcDetails: "Other", | ||
}; | ||
const expected = { ...values }; | ||
ctrl.onUpdateRegistrationSrc(values); | ||
expect(values).toStrictEqual(expected); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
import { AgentTypeEnum, AgentJobTypeEnum } from "dto"; | ||
import { AgentTypeEnum, AgentJobTypeEnum, RegistrationSrcTypeEnum } from "dto"; | ||
import type { ComponentType, SvelteComponent } from "svelte"; | ||
import OperatorSubStep from "./OperatorSubStep/OperatorSubStep.svelte"; | ||
import CentralSubStep from "./CentralSubStep/CentralSubStep.svelte"; | ||
|
@@ -48,6 +48,16 @@ export default class StructureFormStepController { | |
{ value: AgentJobTypeEnum.CONTROLLER, label: "Contrôleur / Inspecteur" }, | ||
{ value: AgentJobTypeEnum.OTHER, label: "Autre" }, | ||
]; | ||
public readonly registrationSrcOptions: Option<RegistrationSrcTypeEnum>[] = [ | ||
{ value: RegistrationSrcTypeEnum.DEMO, label: "Lors d’une présentation avec une personne de Data.Subvention" }, | ||
{ value: RegistrationSrcTypeEnum.SEARCH_ENGINE, label: "Via des recherches sur internet" }, | ||
{ | ||
value: RegistrationSrcTypeEnum.COLLEAGUES_HIERARCHY, | ||
label: "Un de mes collègues ou de ma hiérarchie m’en a parlé", | ||
}, | ||
{ value: RegistrationSrcTypeEnum.SOCIALS, label: "Via une publication sur les réseaux sociaux" }, | ||
{ value: RegistrationSrcTypeEnum.OTHER, label: "Autre" }, | ||
]; | ||
|
||
private static subStepByAgentType: Record<AgentTypeEnum, ComponentType> = { | ||
[AgentTypeEnum.CENTRAL_ADMIN]: CentralSubStep, | ||
|
@@ -102,6 +112,26 @@ export default class StructureFormStepController { | |
this.dispatch(shouldBlockStep ? "error" : "valid"); | ||
} | ||
|
||
onUpdateRegistrationSrc(values: Record<string, unknown>) { | ||
this.onUpdate(values, "registrationSrc"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On peut avoir du contexte / commentaire sur pourquoi on remplace There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Parce que sinon la donnée est sauvegardée même si la valeur de There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
if (values["registrationSrc"] && Array.isArray(values["registrationSrc"])) { | ||
if ( | ||
!values["registrationSrc"].includes(RegistrationSrcTypeEnum.COLLEAGUES_HIERARCHY) && | ||
values["registrationSrcEmail"] !== "" | ||
) { | ||
values["registrationSrcEmail"] = ""; | ||
this.onUpdate(values, "registrationSrcEmail"); | ||
} | ||
if ( | ||
!values["registrationSrc"].includes(RegistrationSrcTypeEnum.OTHER) && | ||
values["registrationSrcDetails"] !== "" | ||
) { | ||
values["registrationSrcDetails"] = ""; | ||
this.onUpdate(values, "registrationSrcDetails"); | ||
} | ||
} | ||
} | ||
|
||
cleanSubStepValues(values: Record<string, any>, contextAgentType: AgentTypeEnum) { | ||
const prefixes: string[] = []; | ||
for (const [agentType, prefix] of Object.entries(StructureFormStepController.subFieldsPrefixByAgentType)) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
<script lang="ts"> | ||
import { RegistrationSrcTypeEnum } from "dto"; | ||
import StructureFormStepController from "./StructureFormStep.controller"; | ||
import Checkbox from "$lib/dsfr/Checkbox.svelte"; | ||
import Input from "$lib/dsfr/Input.svelte"; | ||
|
@@ -7,6 +8,9 @@ | |
service: "", | ||
jobType: [], | ||
phoneNumber: "", | ||
registrationSrc: [] as RegistrationSrcTypeEnum[], | ||
registrationSrcEmail: "", | ||
registrationSrcDetails: "", | ||
}; | ||
export let context = {}; | ||
|
||
|
@@ -55,4 +59,39 @@ | |
on:change | ||
on:blur={() => ctrl.onUpdate(values, "phoneNumber")} /> | ||
</div> | ||
|
||
<div class="fr-fieldset__element fr-mb-0 fr-mt-4v"> | ||
<Checkbox | ||
options={ctrl.registrationSrcOptions} | ||
label="Comment avez-vous connu Data.Subvention ?" | ||
errorMsg={$errors.registrationSrc} | ||
on:change={() => ctrl.onUpdateRegistrationSrc(values)} | ||
bind:value={values.registrationSrc} /> | ||
</div> | ||
|
||
{#if values?.registrationSrc && values?.registrationSrc.includes(RegistrationSrcTypeEnum.COLLEAGUES_HIERARCHY)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Normalement c'est au controller d'avoir la logique; il faut une méthode There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @mxmeunier ok, oui ce sera plus propre effectivement. |
||
<div class="fr-fieldset__element"> | ||
<Input | ||
id="registrationSrcEmail-input" | ||
type="email" | ||
label="Pouvez-vous nous indiquer son email ?" | ||
autocomplete="email" | ||
bind:value={values.registrationSrcEmail} | ||
errorMsg={$errors.registrationSrcEmail} | ||
error={$errors.registrationSrcEmail} | ||
on:change | ||
on:blur={() => ctrl.onUpdate(values, "registrationSrcEmail")} /> | ||
</div> | ||
{/if} | ||
{#if values?.registrationSrc && values?.registrationSrc.includes(RegistrationSrcTypeEnum.OTHER)} | ||
<div class="fr-fieldset__element"> | ||
<Input | ||
id="registrationSrcDetails-input" | ||
type="text" | ||
label="Précisez" | ||
bind:value={values.registrationSrcDetails} | ||
on:change | ||
on:blur={() => ctrl.onUpdate(values, "registrationSrcDetails")} /> | ||
</div> | ||
{/if} | ||
</fieldset> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Il manque un mock sur
ctrl.onUpdate
non ?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mxmeunier je ne pense pas, ici je teste la fonction d'update sur le champ
registrationSrc
qui appelle elle même directementonUpdate
.Mais j'ai peut-être raté un truc dans la façon de tester.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok oui c'est plus clair