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

feat(front): #3043 add 'from' fields in user registration form #3119

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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";
Expand Down Expand Up @@ -208,4 +208,31 @@ describe("StructureFormStepController", () => {
expect(values.centralSomething).toBeUndefined();
});
});

describe("onUpdateRegistrationSrc", () => {
it("registrationSrcEmail and registrationSrcDetails are set to empty strings", () => {
Copy link
Collaborator

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 ?

Copy link
Collaborator Author

@cspriet cspriet Jan 15, 2025

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 directement onUpdate.
Mais j'ai peut-être raté un truc dans la façon de tester.

Copy link
Collaborator

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)

Copy link
Collaborator Author

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

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";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -102,6 +112,26 @@ export default class StructureFormStepController {
this.dispatch(shouldBlockStep ? "error" : "valid");
}

onUpdateRegistrationSrc(values: Record<string, unknown>) {
this.onUpdate(values, "registrationSrc");
Copy link
Collaborator

Choose a reason for hiding this comment

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

On peut avoir du contexte / commentaire sur pourquoi on remplace registrationSrcEmail et registrationSrcDetail par une chaîne vide pour COLLEAGUES_HIERARCHY et OTHER ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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 registrationSrc n'est pas la bonne.
J'ai hésité à le faire juste avant la sauvegarde effective ou ici. Tu en penses quoi ?

Copy link
Collaborator

Choose a reason for hiding this comment

The 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)) {
Expand Down
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";
Expand All @@ -7,6 +8,9 @@
service: "",
jobType: [],
phoneNumber: "",
registrationSrc: [] as RegistrationSrcTypeEnum[],
registrationSrcEmail: "",
registrationSrcDetails: "",
};
export let context = {};

Expand Down Expand Up @@ -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)}
Copy link
Collaborator

Choose a reason for hiding this comment

The 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 ctrl.isRegisteredFromColleaguesOrHierarchy + ctrl.isRegisteredFromOther ou équivalent pour enlever la logique des fichiers svelte

Copy link
Collaborator Author

Choose a reason for hiding this comment

The 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>
3 changes: 3 additions & 0 deletions packages/front/src/lib/resources/users/user.port.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ export class UserPort {
decentralizedLevel: data.decentralizedLevel,
decentralizedTerritory: data.decentralizedTerritory,
territorialScope: data.territorialScope,
registrationSrc: data.from,
registrationSrcEmail: data.fromEmail,
registrationSrcDetails: data.fromOther,
};
return requestsService.patch(`${this.BASE_PATH}/`, updateProfile);
}
Expand Down
Loading