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

La règle "sauvegarde des données" est rendue indispensable pour le profil MSS+ #436

Merged

Conversation

ColinGarrigaSalaun
Copy link
Contributor

Le moteur de règles peut maintenant rendre indispensable une liste de mesures pour un profil donné.

La pastille "indispensable" est mise à jour dans la page des mesures.
Les statistiques sont mises à jour dans la page de synthèse du PDF.

@JacquesRogueOne
Copy link
Contributor

😇 Les messages des 2 derniers commits commencent par des minuscules, pourrais-tu si tu le souhaites les faire commencer par des majuscules comme les autres messages

Copy link
Contributor

@JacquesRogueOne JacquesRogueOne left a comment

Choose a reason for hiding this comment

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

Je pense que l'on peut séparer le dernier commit du reste qui est un gros refacto

return idMesures.reduce(
(resultat, id) => Object.assign(resultat, { [id]: this.referentiel.mesure(id) }),
{},
const mesurePossiblementRendueIndispensable = (idsMesuresReference, idMesure) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

ce nom de fonction ne me parle pas directement, je pencherai pour mesureAvecImportanceAjustee
On peut en débattre

Copy link
Contributor

Choose a reason for hiding this comment

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

Allez hop, pourquoi pas.

expect(moteur.mesures(service)).to.eql({ supervision: { indispensable: true } });
});

it("change l'importance de la mesure lorsque la description du service change", () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Je ne comprends pas bien la pertinence de ce test. Est ce que tu pourrais m'éclairer stp

Copy link
Contributor

Choose a reason for hiding this comment

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

Je comprends, c'est pour tester le deep clone qui se trouve dans referentiel.js
Est ce qu'il ne serait pas préférable de tester le deep clone dans referentiel.spec.js

Copy link
Contributor

@egaillot egaillot Oct 19, 2022

Choose a reason for hiding this comment

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

Oui, on a posé ce test-là pour mettre en évidence un bug qu'on a vu… mais peut-être que c'est aussi bien de bouger le test plus près du code. J'aimerais quand même conserver ce test, car si quelqu'un se demande à quoi bon faire un deep clone… ben là raison est pile ici 😉

Copy link
Contributor

Choose a reason for hiding this comment

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

J'ai ajouté le test ✅

@egaillot
Copy link
Contributor

Je pense que l'on peut séparer le dernier commit du reste qui est un gros refacto

Pas sûr - ce n'est pas juste un gros remaniement, c'est surtout l'introduction d'une nouvelle fonctionnalité, et le dernier commit permet de tester que ça marche. Je serais pour garder cette PR avec les 4 commits. Est-ce que ça te convient ?

@egaillot egaillot force-pushed the moteur-de-regles--rends-mesure-indispensable branch from 0b0ec33 to a7637c5 Compare October 19, 2022 20:29
Colin Garriga-Salaün and others added 2 commits October 19, 2022 22:42
@egaillot egaillot force-pushed the moteur-de-regles--rends-mesure-indispensable branch from a7637c5 to e456e5d Compare October 19, 2022 20:42
@egaillot egaillot force-pushed the moteur-de-regles--rends-mesure-indispensable branch from e456e5d to 88736f0 Compare October 19, 2022 20:43
@egaillot
Copy link
Contributor

😇 Les messages des 2 derniers commits commencent par des minuscules, pourrais-tu si tu le souhaites les faire commencer par des majuscules comme les autres messages

C'est corrigé ✅

@egaillot egaillot added urgent This should be merged ASAP and removed urgent This should be merged ASAP labels Oct 21, 2022
@@ -21,7 +21,7 @@ const creeReferentiel = (donneesReferentiel = donneesParDefaut) => {
const localisationsDonnees = () => donnees.localisationsDonnees;
const identifiantsLocalisationsDonnees = () => Object.keys(localisationsDonnees());
const mesureIndispensable = (idMesure) => !!donnees.mesures[idMesure].indispensable;
const mesures = () => donnees.mesures;
const mesures = () => JSON.parse(JSON.stringify(donnees.mesures));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Il faudra étendre ce mécanisme aux autres objets provenant des donnees dans une PR à part. Le ticket #438 a été créer pour ça.

@ColinGarrigaSalaun ColinGarrigaSalaun merged commit a6886c5 into master Oct 21, 2022
@ColinGarrigaSalaun ColinGarrigaSalaun deleted the moteur-de-regles--rends-mesure-indispensable branch October 21, 2022 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants