Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Distinguer les modèles enregistrés et les modèles rattachés aux imports #234

Closed
DonovanMaillard opened this issue Sep 8, 2021 · 15 comments
Labels
BDD About database enhancement New feature or request question Further information is requested

Comments

@DonovanMaillard
Copy link
Collaborator

Actuellement, les imports sont rattachés à un id_mapping (pour les champs, puis pour les nomenclatures), qui font référence à la "banque de modèles d'imports" qui peuvent être réutilisés.

Le soucis avec ce fonctionnement, c'est qu'un import terminé peut être rattaché à un modèle qui continue d'évoluer et d'être mis à jour.

Si c'était déjà problématique, c'était peu visible jusqu'à maintenant. Avec #146 et #158 ça va devenir bien plus visible et problématique. Concrètement :

  • Le lundi, j'utilise mon Modèle A pour faire un premier import
  • Le mardi, j'utilise mon Modèle A mais je le modifie, pou faire mon 2eme import
  • Le mercredi, je consulte le rapport de mon premier import, et je télécharge son modèle -> il n'est plus valide

C'est également vrai avec la notion d'imports "temporaires" qu'on n'enregistre pas dans la "banque de modèles d'imports".

Ce que je propose :

  • Dissocier cette "banque de modèle d'imports" des imports eux-même, et limiter son rôle à une banque de modèles réutilisables
  • Supprimer les id_mappings de la t_imports, et sauvegarder à la place les mappings "champs" et "nomenclatures" dans deux champs jsonb dédiés, en sauvant par conséquent leur version modifiée/temporaire à l'instant T de l'import sans que ces versions soient stockées dans la banque de modèles
  • Baser l'affichage et l'export des mappings depuis le rapport d'import sur cette version enregistrée dans la t_imports

Par conséquent, le rapport d'imports n'a plus d'interactions avec la banque de modèles. Et modifier ou mettre à jour la banque de modèle n'a plus d'impacts sur l'historique des imports terminés.

Se poserait aussi la question d'utiliser à terme des champs jsonb pour le stockage des mappings dans la banque de modèles, plutôt que la table de correspondance actuelle conçue avant l'existence de ce format dans pgsql. Pour ce type de données, ça me semblerait pertinent et tout autant lisible.

@DonovanMaillard DonovanMaillard added enhancement New feature or request question Further information is requested BDD About database labels Sep 8, 2021
@DonovanMaillard
Copy link
Collaborator Author

Maxime relève d'autres soucis, là aussi déjà présents mais qui se retrouvent mis en exergue maintenant avec ces fonctionnalités. Le fait d'avoir des modèles "mouvants", non versionnés, limite leur potentielle réutilisation à venir.

Le fait de versionner des modèles ou de dupliquer des modèles proches, compliquera les choses au moment du modèle d'imports à sélectionner.

Après échange avec Camille, ressort alors le principe de "machine learning", pas si compliqué à mettre en place, qui consisterait pour chaque champs de la dict_fields à stocker l'ensemble des noms de champs sources possible : si l'un de ces noms est trouvé dans le fichier source alors il est pré-rempli intelligemment.

La logique serait alors :

  • j'upload mon fichier de données à importer
  • J'arrive au mapping des champs : soit je rentre un modèle en json, soit je fais un "import sans modèle", auquel cas le module renseigne intelligemment le formulaire à partir de ce qu'il a appris, et on complète manuellement
  • Le module enregistre alors les nouvelles correspondances pour pouvoir les proposer plus tard lors des imports suivants

Ca impliquerait au niveau de l'update :

  • de transformer les mappings actuels en listes de synonymes pour les pré-remplissages
  • le stocker les mappings actuels sur les imports déjà faits, pour permettre leur affichage dans le rapport + leur export (même s'il y a eu des changements de mappings entre deux, fonctionnement actuel oblige
  • La suppression des tables de mappings et des id_mappings dans la t_imports

@camillemonchicourt
Copy link
Member

Oui en effet le fonctionnement actuel des modèles d'import peut poser les soucis que tu évoques.

Pour moi, ça va même plus loin, et ça me conforte dans l'idée que notre logique globale de modèles d'imports n'est pas forcément pertinente, et qu'on devrait plutôt avoir une logique de mappings de champs, de synonymes de champs, avec une bibliothèque qui se complète pour chaque champs au fur et à mesure où on mappe des imports.

Toujours de manière assez globale, je pense qu'on a conçu ce module d'imports, sans regarder et analyser ce qui se fait par ailleurs dans d'autres outils d'imports en terme technique et ergonomique.
J'en avais fait un petit pour avoir des idées, que je vais essayer de retrouver et partager.

@DonovanMaillard
Copy link
Collaborator Author

Sur la 2eme partie, je le vois pas tout a fait comme toi. On avait fait le choix d'un outil destiné à aider l'administrateur de base pour ses imports. Et on le fait évoluer pour répondre à des besoins d'utilisateurs novices.

@mvergez
Copy link
Contributor

mvergez commented Sep 8, 2021

C'est une bonne idée la fonctionnalité de synonymes. Sans aller jusqu'au machine learning au début, mais déjà juste vérifier si les colonnes du fichier uploadé sont disponibles dans les synonymes permettrait de tester et voir ce que ça donne.
Ensuite pour aller plus loin, peut-être que je me trompe mais j'ai l'impression que ça s'appelle le "Schema matching" (https://en.wikipedia.org/wiki/Schema_matching)

@camillemonchicourt
Copy link
Member

Sur la 2eme partie, je le vois pas tout a fait comme toi. On avait fait le choix d'un outil destiné à aider l'administrateur de base pour ses imports. Et on le fait évoluer pour répondre à des besoins d'utilisateurs novices.

Peu importe la cible, faire une étude des outils existants avec des besoins similaires est toujours utile. Et il y a cet article super intéressant sur comment faire un outil d'import ergonomique : https://www.smashingmagazine.com/2019/11/flatfile-seamless-spreadsheet-import-experience/

Pour la cible utilisateur, selon moi ajouter des outils pour des novices ne veut pas dire qu'il ne réponde plus au besoin initial d'un administrateur, au contraire ça doit juste lui faciliter les choses en plus.
Si on a perdu des fonctionnalités importantes pour les administrateurs, c'est une erreur à laquelle il faut remédier.

Oui @mvergez, en effet l'idée est plus du "schema matching" que du "machine learning".

@DonovanMaillard
Copy link
Collaborator Author

DonovanMaillard commented Oct 4, 2021

Ce qui est retenu pour l'instant sur ce point :

  • On "met de coté" la réflexion sur le fait de conserver ou non la logique de "mappings", d'auto remplissage etc, le temps de mûrir ces idées

  • On stocke d'ores et déjà dans un champs jsonb (table t_imports) les correspondances utilisées pour chacun des imports : c'est ce champs qui alimentera le rapport d'import, et les exports associés

  • On arrête de stocker pour chaque import l'identifiant du mapping utilisé, puisque celui-ci est susceptible d'être modifié voire suppimé...

@bouttier , est-ce que ca te semble OK comme manière de faire à ce stade ?

@camillemonchicourt
Copy link
Member

Pour moi il y a un truc.
Si on supprime le lien entre un import et son mapping, quand on revient sur un import, on ne peut plus charger le mapping qui a été utilisé, avec son nom et compagnie.

@bouttier
Copy link
Contributor

bouttier commented Oct 5, 2021

Bonjour,

La modification d’un import qui est utilisé par un mapping précédent est effectivement un problème que j’avais identifié lorsque j’avais commencé à refactorer le module d’import. J’avais corrigé ce problème en dupliquant le mapping dès lors qu’il est modifié. C’est-à-dire que si un nouvel import utilise un mapping d’un import précédent, et que le mapping est modifiié, deux possibilité :

  • l’utilisateur décide de sauvegarder ses modifications, alors l’ancien mapping est dupliqué, marqué temporaire, et le modèle est effectivement modifié sans que cela n’ait d’impact sur l’ancien import
  • l’utilisateur décide de ne pas sauvegarder ses modifications, alors un nouveau mapping temporaire est créé pour le nouvel import là encore de manière à ne pas influer l’ancien import

En ce qui concerne le schéma matching etc., à chaque fois que je modifie un modèle, je viens l’enrichir avec une nouvelle connaissance d’associations champs source => destination sans perdre la connaissance précédente. Ainsi, mon modèle se perfectionne avec le temps et devient capable de traiter tous les fichiers auquel je suis confronté. De ce fait, je trouve inutile de mettre en place des procédés d’automatisations qui peuvent s’avérer hasardeux.

@DonovanMaillard
Copy link
Collaborator Author

Merci @bouttier Elie pour ton retour.

A ton sens aujourd'hui, est-ce qu'il vaut mieux partir sur ton principe -> A chaque import, on duplique un mapping qui ne devient pas utilisable mais qui est archivé pour chaque import passé, au risque de dupliquer pas mal de lignes dans les tables de mappings ? Auquel cas, on attendrait plutôt le refact à priori, ou alors il faudrait pousser ton travail dans notre prochaine release en attendant que le refact soit terminé ?

Ou est-ce qu'il vaut mieux stocker le mapping dans un champ json dans chaque ligne de la t_imports, et dissocier totalement le stokage de la "banque de mappings réutilisables" et le stockage des mappings utilisés pour chaque import ? Ce qu'on proposait de faire via ce projet, mais qui ne doit pas s'imposer face à la solution que tu prévoyais.

@camillemonchicourt
Copy link
Member

Ça me semble discutable de dupliquer complètement un mapping dès qu'on modifie un des champs.

@DonovanMaillard
Copy link
Collaborator Author

En soit, qiz ca soit la proposition d'elie ou la mienne, on duplique l'information.cest juste fait sous deux formats différents... et on a pas mieux, sinon il faudrait historiser les modifications et chèche la date d'un import puis de son mapping à un instant donné....

@bouttier
Copy link
Contributor

bouttier commented Oct 7, 2021

Je suis d’accord avec Donovan pour sauvegarder le mapping avec chaque import, comme ça on est sûr de pouvoir re-générer la synthèse ! Et on a pas ses problèmes de modifications de modèle qui casse un import existant. Mon approche visant à dupliquer le mapping lors d’une modification était peut-être un peu plus optimisé (si pas de modif, les imports partage le même mapping) mais peut-être aussi plus complexe : je ne suis pas contre simplifié en enregistrant le mapping à chaque fois.
Par contre je suis pour garder la structure en BDD plutôt que d’avoir un champs JSON. C’est d’autant plus dommage d’utiliser du JSON qu’on a déjà toute une API pour manipuler les modèles stoqués en BDD.

@DonovanMaillard
Copy link
Collaborator Author

Ca marche, merci pour ton retour.

Je propose donc qu'on conserve la logique d'Elie :

  • On stocke des modèles comme c'est actuellement le cas
  • A chaque utilisation, l'import est "dupliqué" dans les tables de correspondances
  • On gade les id_mappings tels qu'ils sont aujourd'hui dans la table t_mappings pour faire le lien.

Quand tu parles d'optimisation, tu veux dire que la duplication n'est pas fait systématiqument ? Dans quel cas est-ce que tu ne dupliques pas ?
Ou alors j'ai mal compris : ca veut dire que quand tu modifies un mapping et que tu veux l'enegistrer, ça conserver l'ancienne version en tant que "mapping temporaire qu'on ne peut pas réutiliser par la suite" ?

@DonovanMaillard
Copy link
Collaborator Author

@mvergez , est-ce que tu penses que c'est faisable de récupérer dans la branch refact ce qu'a fait Elie ? Ca permettrait de faire converger encore un peu les deux travaux, et d'en bénéficier dès la prochaine release pour régler le soucis actuel - avant la refonte qui devrait encore demander un certain temps non planifié

@DonovanMaillard
Copy link
Collaborator Author

Je cloture ce ticket, on conserve donc la logique de Elie, et on attendra le refact pour bénéficier de cette amélioration afin de ne pas faire/refaire le travail plusieurs fois.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
BDD About database enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants