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

Supprimer la saisie de date d'arrêté #988

Open
johanricher opened this issue Oct 2, 2024 · 9 comments · May be fixed by #1011
Open

Supprimer la saisie de date d'arrêté #988

johanricher opened this issue Oct 2, 2024 · 9 comments · May be fixed by #1011
Assignees
Labels
Impact : Agents Indicateur "Utilisateurs actifs"

Comments

@johanricher
Copy link
Collaborator

johanricher commented Oct 2, 2024

Description

Suite au problème identifié et décrit sur #701, une réunion a eu lieu aujourd'hui pour se mettre d'accord sur l'implémentation de la solution. Celle-ci reprend la proposition discutée il y a quelques mois : ne plus demander la saisie des dates de début et fin d'un arrêté lors de la création d'un arrêté dans l'UI de DiaLog.

Les dates de début et fin d'un arrêté continueront cependant à être affichées quand le besoin se présente (par exemple valeurs de "période" sur la page qui liste les arrêtés) et que cela n'entraine pas de confusion (voir discussion sur #843 pour un exemple). Les valeurs en question seront inférées à partir des dates de début/fin des restrictions ("mesures") qui composent l'arrêté (début d'un arrêté = date la plus tôt de début d'une restriction ; fin d'un arrêté = date la plus tard de fin d'une restriction).

Dans le cadre de cette évolution des adaptations du formulaire sont nécessaire pour simplifier la saisie.

Implémentation

  • Sur la première page du formulaire de création d'un arrêté ("Informations générales") : supprimer la saisie des dates sur l'arrêté
  • Enregistrer en base les dates de début et fin d'un arrêté en les inférant à partir de celles des restrictions
  • Sur la deuxième page du formulaire de de création d'un arrêté : la saisie des dates de début et de fin d'une restriction est toujours affichée
  • Remplacer "plage", "jours et horaires" par "Périodes d'application" + autres reformulation (voir maquette)

Design

image

Maquette

@florimondmanca
Copy link
Collaborator

florimondmanca commented Oct 16, 2024

On a un tout petit sujet supplémentaire posé par ce ticket :

Actuellement pour savoir si un arrêté est "permanent" ou "temporaire", on regarde si une date de fin a été définie au niveau de l'arrêté

En fonction de ça on affiche ou pas les champs de date de fin dans le formulaire des périodes pour les mesures

Ici on veut inverser la logique, et faire en sorte que les dates de l'arrêté "remontent" des périodes des mesures

Par conséquent, il faut un moyen plus explicite de configurer le fait qu'un arrêté soit permanent ou temporaire, non ?

cc @aureliebaton

On pourrait dire : si aucune période n'a de date de fin, alors c'est du permanent, sinon c'est du temporaire... Mais est-ce que c'est satisfaisant en termes d'UX ?

@aureliebaton
Copy link
Collaborator

C'est une bonne question en effet.
Le plus explicite serait peut-être d'ajouter une liste dans le premier formulaire pour sélectionner l'un ou l'autre (temporaire ou permanent). Par exemple à la place de la liste "Nature de l'arrêté" dont on ne se sert pas.

@florimondmanca
Copy link
Collaborator

florimondmanca commented Oct 16, 2024

@mmarchois Est-ce que tu aurais un avis technique sur le calcul et le stockage / synchronisation des dates globales ? (On peut réutiliser la terminologie datex de la overallPeriod)

Au niveau du calcul ces dates sont un MIN(period.startDate) et MAX(period.endDate), donc avec double jointure (regulationOrder -> measure -> period)

J'ai envisagé ces options...

  1. Créer une VIEW en relation one-to-one avec RegulationOrder
    • La VIEW est exécutée à chaque fois qu'on y accède, potentiellement de façon "incontrôlée" (requêtes inutiles, cache la complexité, source de problèmes de perf ?)
  2. Créer une MATERIALIZED VIEW pour éviter les recalculs
    • Mais il faut gérer son rafraîchissement en appelant REFRESH MATERIALIZED VIEW, donc ça ne semble pas appropriées (les vues matérialisées c'est plutôt un cas d'usage d'analytics j'ai l'impression)
  3. Faire "manuellement" et explicitement le calcul au niveau des repository chaque fois qu'on a besoin de ces dates globales
    • => ❤️ C'est l'option vers laquelle je tends pour l'instant... Car c'est la plus explicite donc la moins sujette à introduire des bugs
    • On pourrait combiner avec une VIEW pour "factoriser" la requête, mais on peut peut-être être + efficace au cas par cas (parfois on fait déjà les jointures sur measure et period de toute façon)
  4. Créer des champs overallStartDate, overallEndDate (renommage des champs actuels) mis à jour par des TRIGGER lors d'un INSERT / UPDATE
    • Les triggers rendent le code moins maintenable car il faut savoir qu'ils existent
    • Il faudrait s'assurer que le trigger se déclenche quand une période est modifiée sur une des mesures de l'arrêté : la condition de déclenchement peut-être prendre en compte des jointures ? Ça a l'air compliqué
  5. Créer ces champs et les peupler manuellement dans le code applicatif
    • Lors d'un update() sur une Period, remonter l'info pour aller modifier le RegulationOrder
    • Soit en synchrone (balader une référence à l'arrêté jusque dans le code des périodes), soit par un "signal" / événement
    • Semble un peu compliqué aussi, mais moins pire que l'option 4)

@johanricher
Copy link
Collaborator Author

johanricher commented Oct 16, 2024

Ici on veut inverser la logique, et faire en sorte que les dates de l'arrêté "remontent" des périodes des mesures

Par conséquent, il faut un moyen plus explicite de configurer le fait qu'un arrêté soit permanent ou temporaire, non ?

Si la définition d'arrêté permanent est "un arrêté contenant au moins une restriction sans date de fin", alors on n'a pas besoin de demander au préalable à la personne de choisir "permanent" ou "temporaire", c'est une information dont la saisie est redondante.

Je ne vois pas de cas où le fait que la saisie ne soit pas explicite poserait un problème.

L'information devrait néanmoins restée affichée (comme pour les valeurs de périodes de l'arrêté) : inférée et non pas demandée.

A l'inverse, si on veut une UX où la personne doit choisir explicitement entre "permanent" et "temporaire", il risque d'y avoir une confusion.

Par exemple : je choisis "arrêté permanent" sans comprendre exactement ce que ça veut dire, et ensuite le formulaire rend impossible de choisir une date de fin pour la restriction, et je ne comprends pas pourquoi.

Autre exemple : je choisis "arrêté permanent" mais je veux définir 2 restrictions dont seulement une n'a pas de date de fin.

@mmarchois
Copy link
Collaborator

mmarchois commented Oct 17, 2024

@florimondmanca je dirais que la solution se jouera entre la 3 et la 4, et cela dépendra également de la fréquence d'utilisation des dates de début/fin.

Si c'est une donnée qui sera souvent requêtée, dans un souci de performance, je préférerais la stocker dans la table RegulationOrder pour éviter un recalcul à la volée. Concernant l'implémentation technique de cette option, je n'opterais pas pour des TRIGGER, mais plutôt pour des opérations effectuées directement dans les handlers, afin de conserver la logique applicative.
En revanche, si cette donnée est utilisée de manière occasionnelle, la solution 4 me semble la plus adaptée.

Sauf erreur de ma part, il ne me semble pas que ces données soient souvent récupérées, et donc je pense que la solution 3 sera plus adaptée.

@florimondmanca
Copy link
Collaborator

florimondmanca commented Oct 17, 2024

Merci @mmarchois ça confirme plutôt ce que je pensais aussi !

@aureliebaton @MathieuFV

Il est ressorti d'une discussion ce matin en daily (où étaient présent-es Johan et Léa) qu'il y avait des questions UX / métier à trancher

L'idée actuelle du ticket est de supprimer la saisie des dates de début et de fin car elles sont considérées comme "redondantes" vis à vis des dates qu'on saisit dans les périodes d'application.

En effet intuitivement on a ces définitions :

  • Date de début de l'arrêté = Date de début de la période qui commence le plus tôt
  • Date de fin de l'arrêté = Date de fin de la période qui termine le plus tard

Le problème est qu'on se base actuellement sur l'existence d'une date de fin de l'arrêté pour déterminer s'il est permanent ou temporaire, pour ensuite répercuter cela sur les périodes (on cache les champs de date de fin sur le formulaire période si l'arrêté n'en a pas)

Donc ça nous amène à nous demander : veut-on garder cette cohérence entre les mesures (soit toutes permanentes, soit toutes temporaires) ?

Soit on la garde, mais en l'absence d'info "au niveau de l'arrêté" il alors faut implémenter une vérification qui dit : "attention, vous avez saisi un mélange mesures temporaires et de mesures permanentes". L'UX n'est pas forcément géniale... ?

Soit on lève cette contrainte, et on permet de mélanger des mesures temporaires et des mesures permanentes au sein d'un même arrêté.

Mais

  1. Est-ce que ça a du sens métier ? Est-ce que les utilisateurs seront "perdus" voire feront des erreurs (ex : arrêté calculé comme permanent juste parce qu'on a oublié une date de fin)
  2. Dans ce cas comment définir un arrêté temporaire ou permanent (pour faire ressortir l'info quand on présente l'arrêté) ? Johan proposait ça :

Arrêté permanent = arrêté dont au moins une mesure n'a pas de limite de temps, car une de ses périodes d'application n'en a pas.
Arrêté temporaire = arrêté dont toutes les mesures sont limitées dans le temps.

La dernière option est celle proposée par Aurélie ci-dessus (Johan n'y est pas favorable si j'ai bien compris ?), à savoir garder la contrainte et demander l'info permanent / temporaire en amont de façon explicite (en remplacement de la saisie de date de fin) et garder la logique actuelle (continuer à cacher les dates de fin sur les périodes sur l'arrêté est indiqué comme permanent).

^C'est l'option la moins coûteuse au niveau dev. Elle répond quand même à ce ticket car on supprime la saisie de dates globales, que l'on calculera à partir des périodes d'application. Et au niveau UX j'ai moins d'interrogation sur l'impact car on ne dévie que très peu de l'actuel. Et ça n'interdit pas dans le futur de faire "encore plus smart" en inférant le statut permanent/temporaire à partir des périodes (ce qui en fait n'est pas l'objet de ce ticket à la base). @johanricher Qu'en penses-tu ?

@MathieuFV
Copy link
Collaborator

Merci pour le compte rendu très clair déjà :)

De ma compréhension, l'option que proposait Johan ne me dérange pas. Il s'agit finalement de poser la question à un utilisateur pour nous assurer que cette catégorisation implicite en arrêté permanent ou temporaire leur semble souhaitable.

D'un autre côté, on peut imaginer que (plus tard) les formulaires pour les arrêtés temporaires et permanents ne soient pas tout à fait les mêmes, ça plaiderait pour l'option d'Aurélie.

Je vais poser la question à la personne de Lambersart qui m'a dit avoir testé l'outil récemment. Au demeurant choisir l'option la moins complexe me semblerait souhaitable ?

Je reviens vers vous quand j'en sais plus !

@florimondmanca florimondmanca linked a pull request Oct 17, 2024 that will close this issue
5 tasks
@florimondmanca
Copy link
Collaborator

Vu les mails : un utilisateur a confirmé qu'il ne s'attendait pas à devoir indiquer explicitement en amont si un arrêté est temporaire ou permanent, car il considère que l'info est présente dans les périodes d'application

Ça confirme l'intérêt de pousser l'inférence jusqu'à la qualification "temporaire vs permanent"

Par contre pour éviter toute introduction de bug (toucher aux périodes est un peu casse-tête), je vais procéder en deux étapes

  1. Supprimer les champs date de début / fin globales sur l'arrêté (l'objet de ce ticket) ; le calcul du critère temporaire / permanent est reporté sur la "catégorie" : un arrêté est permanent si et seulement si sa catégorie vaut "Réglementation permanente"
  2. Inférer le type temporaire / permanent en fonction des périodes d'application

@aureliebaton
Copy link
Collaborator

Suite à la discussion sur Mattermost et à la proposition d'ajouter un bloc spécifique pour les périodes, voici une nouvelle proposition de maquette https://www.figma.com/design/YuT6Uh4wxe90U8LaPLlDzO/Dialog?node-id=7230-24296&t=UoPI31I6VQTOoG2x-1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Impact : Agents Indicateur "Utilisateurs actifs"
Projects
Status: En revue de code
Development

Successfully merging a pull request may close this issue.

5 participants