Verrouillage de la simulation au moment du dépôt de dossier (GUH)

[pyDez]

Ce ticket

1. Modifier la saisie des haies met à jour l’URL de la page résultat
2. Créer une version de la page de résultats en lecture seule

[pyDez]

@thibault
Pour qu'une url de simulation soit immuable, j'ai fait en sorte que chaque nouvelle validation d'un linéaire de haie crée un nouvel object plutôt que de mettre à jour un objet existant.
Cela risque de créer beaucoup plus d'objet. A voir, s'il faut trouver une manière de réduire ce nombre.

[thibault]

Hello,

J'avoue qu'à première lecture, je vois des points négatifs à l'implémentation actuelle, que je vais essayer de détailler.

En général, je suis plutôt partisan de réutiliser des fonctionnalités existantes (ici les mappings d'urls) mais ici, il me semble qu'il y a un assez grand écart entre ce qui se passe d'un point de vue métier et le vocabulaire utilisé dans le code, ce qui fait que :

• on se retrouve avec une solution fragile techniquement, et un potentiel de bug assez important
• le code n'est plus auto-documentant, on ne peut pas vraiment deviner de quoi il retourne en le lisant.

Pour préciser ce que je veux dire, si je devais décrire ce qui se passe d'un point de vue métier :

1. l'utilisateur effectue une simulation
2. il entame une démarche
3. il ne peut plus modifier la simulation actuelle

les mêmes étapes décrites du point de vue du code :

1. on récupère une url de simulation
2. on créé un objet « url mapping » avec l'url
3. on créé un nouveau type d'url de résultat qui tire ses paramètres de l'url enregistrée dans le mapping
4. on hacke le formulaire de saisie de haies pour empêcher de modifier les saisies existantes.

Ici, d'un point de vue de l'API, les choses ne sont pas explicites pour l'utilisateur :

• Ce n'est pas explicite qu'une url de type /simulateur/resultat/erqqzt/ correspond à la simulation d'une démarche simplifée

Pour les devs, voici quelques questions qui me viendraient à l'esprit si j'étais nouveau dev et que je bossais sur le code sans connaître l'historique, et qui n'auraient pas de réponse immédiate :

Dans moulinette/urls.py, à quoi correspond la référence de la moulinette, et à quoi peut correspondre un résultat de simulation en lecture seule ?

Puisque les données de la simulation sont tirées de l'url, que se passe-t-il si j'ajoute des paramètres à une url read-only, e.g /simulateur/resultat/erqqzt/?lineaire_detruit=200… ?

Il y a un lien entre les référence de moulinette et les démarches simplifiées, mais ça n'est vraiment explicité nulle part ?

Du coup, tous les objets UrlMapping correspondent à des démarches simplifiées ?

Dans les avis, on passe les paramètres de la simulation dans un champ « Url moulinette ». Qu'est-ce qui se passe si on passe une valeur avec une url mapping qui contient des paramètres ?

Tiens, chaque fois qu'on modifie une haie, ça créé une nouvelle entrée. Ça me semble un bug, je vais corriger ça.

Etc.

Enfin d'un point de vue technique, ça me semble assez fragile parce que concrètement, il n'y a que des contraintes d'interface mais pas de contraintes techniques qui font que la simulation est réellement read-only.

Par exemple, il n'y a pas de contraintes de clés entre une référence moulinette, un mapping d'url, un objet HedgeData, etc. En gros, ça veut dire que si on supprime un mapping, ou un hedge data, ou que l'un ou l'autre est modifié côté admin, on casse la fonctionnalité sans qu'il n'y ait d'alerte ou de moyen de vérifier. Ça créé donc une sacrée surface de problèmes potentiels bien vicieux.

Il me semble qu'il aurait été plus clair, explicite et robuste d'essayer de coller à ce qui se passe du point de vue métier, par exemple :

1. on récupère une url de simulation
2. on créé un objet, i.e « Démarche » avec des champs « url moulinette » et « données de haies » (en json)
3. on créé une nouvelle url « /démarche// »

Ainsi :

• toutes les données concernant une démarche sont à un seul endroit et on ne peut pas se retrouver dans un état incohérent
• on ne hacke pas des fonctionnalités existantes pour les intégrer à une fonctionnalité tierce
• on ne peut pas casser la fonctionnalité sans s'en apercevoir en bossant sur d'autres parties du code.

Enfin, j'avais cru comprendre qu'il fallait aussi faire en sorte que le résultat de la simulation devienne immuable, comme pour les avis. En l'état actuel, si des critères ou des cartes changent, je n'ai rien vu qui empêche l'url de simulation de renvoyer un résultat différent ?

Désolé pour le gros pavé. Je n'ai pas eu toutes les infos concernant les discussions sur cette carte, ni les étapes par lesquelles tu es passé pour parvenir à la solution actuelle. On en discute si tu en as envie / besoin.

[pyDez]

@thibault

Merci pour cette review !
Je vais ajouter un peu de contexte, et prendre tes arguments en compte, on verra ce qu'il en sort à la fin de ma réponse. En écrivant cette introduction, je ne sais pas du tout, ce que j'écrirais en conclusion...

Suite à nos questions lors du "grooming asynchrone", voici les conclusions de Mathieu et Nicolas (cf ticket Trello):

Trois tâches sont identifiées pour ce ticket :

1. Modifier la saisie des haies met à jour l’URL de la page résultat
2. Créer une version de la page de résultats en lecture seule
3. Mettre en place un système de versioning pour la page de résultats en lecture seule

Concernant le 3eme point : on commence sans versioning – on a l'expérience de la bascule en "statique" pour les AR, qui s'est faite récemment, sans trop de difficulté.

L'idée n'est donc pas vraiment de figer une simulation statique pour le moment, mais simplement de pouvoir afficher une simulation à un instructeur sans lui donner envie de cliquer partout pour la modifier ; et à un petitionaire pour qu'il comprenne que la suite ne se passe plus sur Envergo, mais sur Démarches Simplifiées.

Je pense avoir pris le chemin le plus court pour répondre aux deux premiers points. Malheureusement ton message me rappel que facile ne veut pas dire simple.

Parmis les points bloquants que tu mentionnes, je retiens ceux-ci :

Les linéaires de haies

Le comportement actuel (avant cette PR) me semble déjà problématique. Une URL de simulation avec un UUID de linéaire de haie, peut être enregistré ou partagé. Or n'importe quel utilisateur disposant de cet URL, s'il modifie son linéaire de haie, va impacter (sans forcement le savoir), toutes les autres personnes susceptibles de le connaitre.

Je pense donc qu'il faudrait créer un nouvel objet HedgeData à chaque soumission du formulaire de simulation (en envoyant le json des tracés) et non à chaque modification du linéaire sur la page du formulaire. Ces objets ne seraient, a priori, jamais modifiés.

l'affichage des résultats read only

J'ai eu froid dans le dos rien qu'en lisant la liste des problèmes que tu présentais à cause d'UrlMapping.
Ce choix n'est sans doute pas pertinent.

Cependant une version statique n'étant pas nécessaire pour le moment, je ne vois pas d'interet à imaginer des choses lourdes, avec de nouveaux objets.
Ce que l'on souhaite c'est simplement afficher une simulation, mais dans une UI légèrement modifié. Peut etre que l'on pourrait reprendre le fonctionnement du debug : se contenter d'ajouter un parametre dans l'url : &read_only=true

le lien avec Démarches Simplifiées

Il n'est matérialisé nul part dans le code, car je n'avais identifié aucun besoin de le matérialiser pour le moment.
Ton argument sur les contraintes techniques est excellent, il me convainc presque 😁

Voici les choses que l'on voudrait pouvoir contraindre ensemble afin de ne pas casser un objet en en modifiant un autre :

• les parametres de simulation
• le linéaire de haie
• le dossier côté Démarche Simplifiée
• la vue read only

OR,
A ce jour, on ne sait RIEN de ce qui se passe dans Démarche Simplifiée. Nous n'avons pas même l'id du dossier que nous venons de pré-remplir.

Le linaire de haie, est déjà présent dans l'url du simulateur, et il sera plus robuste s'il n'est créé que lors de la soumission de la simulation.

La vue read only peut devenir également partie prenante de l'url du simulateur en devenant un simple parametre.

DONC
Finalement le seul interet de cet objet serait de faire un lien entre une Url de simulation est un linéaire de haie au format JSON. Je ne suis pas convaincu de son utilité.

Conclusion

J'ai établi ma réflexion en écrivant le message. Je crains que ce ne soit pas bon signe pour la qualité literraire du dit message. J'espère que tu y comprendras quand même quelque chose, n'hésite pas à me le notifier si ce n'est pas le cas.

Je suis d'avis de :

• soumettre le linéaire de haie, au format json, en même temps que le formulaire et de ne créer les objet hedgeData que côté back avant l'affichage des résultats.
• faire une vue "read_only" simplement basé sur un parametre dans l'url et completement supprimer la notion de "reference moulinette" et d'UrlMapping

J'attends ton retour avant de me lancer :)

[thibault]

@pyDez Merci pour ta réponse. C'est très clair et très pertinent :)

Le comportement actuel (avant cette PR) me semble déjà problématique. Une URL de simulation avec un UUID de linéaire de haie, peut être enregistré ou partagé. Or n'importe quel utilisateur disposant de cet URL, s'il modifie son linéaire de haie, va impacter (sans forcement le savoir), toutes les autres personnes susceptibles de le connaitre.

Là dessus je suis d'accord. Le fait de devoir stocker les données de saisie de haies en base est de toutes façons un hack. Tu as raison ce sera moins problématique de rendre une saisie immuable.

soumettre le linéaire de haie, au format json, en même temps que le formulaire et de ne créer les objet hedgeData que côté back avant l'affichage des résultats.

Oui ça serait effectivement plus logique. Par contre, ça va nécessiter de refondre toute l'interaction entre le formulaire et la saisie de haie, et aussi une partie de la gestion des données par la moulinette. Je ne suis pas super certain que le jeu en vaille la chandelle.

J'attends ton retour avant de me lancer :)

Comme on l'a vu ce matin, on réfléchit un peu dans le vent parce qu'on n'a pas assez de contexte :D Attendons d'avoir une vision moins floue pour discuter ?

[pyDez]

Suite à la présentation des ambitions futures sur le GUH, je propose les modifications suivantes :

1. Création d'un objet "Dossier" (File) avec la structure suivante:

moulinette_url : str
linéaire_de_haie : OneToOneField( HedgeData )
reference : 6 length alphanumeric str

Il sera également utile plus tard pour suivre le dossier DS.

2. On ajoute une route dossier/{reference} donnant accés à la vue résultat actuelle mais sans les CTA.

3. soumettre le linéaire de haie, au format json, en même temps que le formulaire et de ne créer les objet hedgeData que côté back avant l'affichage des résultats.

@thibault qu'en penses tu ?

[thibault]

@pyDez

Création d'un objet "Dossier" (File) avec la structure suivante:

Le terme « file » me paraît un peu trop connoté « fichier » pour être autosuffisant, peut-être juste trouver un nom un peu plus descriptif ?

linéaire_de_haie : OneToOneField( HedgeData )

Je me demande si ça ne serait pas plus robuste de copier directement tout le json représentant la saisie, plutôt qu'un lien vers l'objet HedgeData (qui à la base est un objet un peu hack qui ne fait que compenser l'impossibilité de passer les données de haies dans l'url) ?

soumettre le linéaire de haie, au format json, en même temps que le formulaire et de ne créer les objet hedgeData que côté back avant l'affichage des résultats.

Ça va impliquer de refondre tout l'échange de données entre l'ui de saisie des données et le formulaire du simulateur. Je ne suis pas certain que ça fasse partie du scope de la demande en cours, mais si tu te sens motivé, je n'y vois pas d'inconvénient :)

[pyDez]

@thibault
J'utilise désormais un objet "PetitionProject" pour stocker les informations du dossier en cours. (Je n'utilise donc plus les UrlMapping)

Je n'ai finalement pas modifier l'envoie des hedges data, la PR me semblait déjà suffisament grosse comme ça.

[thibault]

Pas mal de petites questions et commentaires, notamment sur les gestions d'erreurs. Sinon, c'est vraiment super 👍