Projet

Général

Profil

Development #39473

import des données d'une fiche depuis un CSV

Ajouté par Serghei Mihai il y a environ 4 ans. Mis à jour il y a environ 4 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Version cible:
-
Début:
03 février 2020
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Cf #37533


Fichiers


Demandes liées

Lié à Publik - Development #37533: Importer un CSV dans un modèle de ficheFermé07 novembre 201906 février 2020

Actions

Révisions associées

Révision 5519f488 (diff)
Ajouté par Serghei Mihai il y a environ 4 ans

cards: add import data from CSV (#39473)

Historique

#1

Mis à jour par Serghei Mihai il y a environ 4 ans

#2

Mis à jour par Frédéric Péters il y a environ 4 ans

Ça doit se faire depuis l'interface des agents, pas des admins (wcs/backoffice/data_management.py pas wcs/backoffice/cards.py).

#3

Mis à jour par Frédéric Péters il y a environ 4 ans

+        get_response().breadcrumb.append( ('import_csv', _('Import CSV')) )
+        self.html_top(title = _('Import CSV'))

On devrait, à l'occasion de copié/collés, corriger l'utilisation des espaces. (passage exemple)

#4

Mis à jour par Serghei Mihai il y a environ 4 ans

#5

Mis à jour par Frédéric Péters il y a environ 4 ans

Rapidement,

Il n'est rien noté concernant l'encodage du CSV, ça va donner des surprises, plutôt détecter comme chrono l'encodage opportun (utf-8 ou iso-8859-15).

Proposer un fichier d'exemple.

Comme ça va allonger la partie présentation, éventuellement basculer sur un gabarit pour le rendu.

Ne pas déplacer des bouts de message hors des méthodes s'ils ne vont être utilisés qu'à un seul endroit (import_csv_message/import_csv_success_message).

#6

Mis à jour par Serghei Mihai il y a environ 4 ans

  • Fichier 0001-cards-add-import-data-from-CSV-39473.patch ajouté

Ok. Avec, dans le fichier d'exemple, les données adaptées pour les champs comme BoolField, EmailField et DateField qui, je pense, couvre la plupart de cas.

#7

Mis à jour par Serghei Mihai il y a environ 4 ans

  • Fichier 0001-cards-add-import-data-from-CSV-39473.patch supprimé
#9

Mis à jour par Frédéric Péters il y a environ 4 ans

Je ne suis pas fan d'imposer aux champs qu'on voudrait importer d'avoir un identifiant; pourquoi ne pas avoir juste pris tous les champs ? Ou, plus précisément, tous les champs qui peuvent être importés, parce que là un champ fichier va juste faire planter les choses, et pareil pour les champs qui n'ont simplement pas de convert_value_from_anything. Ce qui me fait penser qu'il existe également convert_value_from_str, dont le nom semble dédié aux chaines.

#10

Mis à jour par Serghei Mihai il y a environ 4 ans

Du cas d'usage donné dans #37533 je déduis qu'il est souhaité de pouvoir spécifier les colonnes des champs à importer car le CSV ne contiendra pas toutes les données du schema d'une fiche.
Et je me rends compte que j'exige que tous les champs ayant un nom de variable soient définis, alors qu'il faut pas.

#11

Mis à jour par Frédéric Péters il y a environ 4 ans

Du cas d'usage donné dans #37533 je déduis qu'il est souhaité de pouvoir spécifier les colonnes des champs à importer car le CSV ne contiendra pas toutes les données du schema d'une fiche.

Il y a juste les dix mots de la description de ce ticket qui parlent d'identifiants et c'est pour moi une erreur. On produit un modèle de document, avec tous les champs possibles et c'est ce format qui sera à respecter, ça répondra tout à fait au besoin.

#12

Mis à jour par Serghei Mihai il y a environ 4 ans

Ok.
Et passage à convert_value_from_str.

#13

Mis à jour par Serghei Mihai il y a environ 4 ans

Un oeil est bienvenu.

#14

Mis à jour par Frédéric Péters il y a environ 4 ans

Taper dans le test des champs avec des types qui marcheront pas.

#15

Mis à jour par Frédéric Péters il y a environ 4 ans

Et taper dans le test des champs qui n'ont pas de varname.

+        if get_publisher().has_site_option('studio') and ('import-csv', 'import_csv') in self._q_exports:

La partie has_site_option n'est pas utile.

+<p>{% trans "You can add data to this card by uploading a file containing var names in columns captions." %}</p>

On ne parle pas de var names.

#16

Mis à jour par Frédéric Péters il y a environ 4 ans

Aussi, après avoir créé un carddata, il faut y jouer le workflow, ce qui fera rapidement dire que tout l'import devrait être dans un afterjob.

#17

Mis à jour par Serghei Mihai il y a environ 4 ans

Yep, store et perform_workflow faits via un job, avec une vérification préalable des données du CSV.
Dans les types qui ne peuvent pas marcher via un import je vois FileField et MapField.

#18

Mis à jour par Frédéric Péters il y a environ 4 ans

Dans les types qui ne peuvent pas marcher via un import je vois FileField et MapField.

Et les tableaux et les commentaires etc.

#19

Mis à jour par Serghei Mihai il y a environ 4 ans

Yep. Donc plutôt lister les champs pour lesquels l'import à partir d'une donnée texte est possible.

#20

Mis à jour par Frédéric Péters il y a environ 4 ans

Je ne vois pas de champs titre/etc. dans le test. Penser aussi à mettre ces champs qui ne figureront pas dans le CSV au milieu des autres, pas tous à la fin.

#23

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

  • Statut changé de Solution proposée à En cours
  • Dans tes tests les assert 'import-csv' in resp me semble inutile tant que tu cliques ensuite sur quelque chose, ça raccourcit le test et le rend plus lisible
  • en fin de test, il manque l'attente puis la validation que l'import a eu lieu (que les cartes avec les données correspondantes sont là)
  • pourquoi ça passe de mettre map dans un MapField ? (vu)
  • pourquoi le champ fichier du formulaire n'est pas requis ? ça éviterait un check ensuite dans submit je pense
  • pourquoi produire un fichier d'exemple pour des colonnes qui ne peuvent être importées comme MapField ? il faudrait les retirer
  • si un champ non supporté est un champs requis de la fiche je dirai qu'il ne faudrait pas permettre l'import, juste afficher "import impossible parce que : 1, 2, 3..."
  • si une colonne ne correspond pas à un champ connu ou est d'un type non supporté, afficher au niveau du job qu'on ne l'importera pas ("column "xxx" will not be imported as field of type "map" are not supported")
  • ça ça ne sert à rien :
    str(N_('Importing data into \'%s\'') % self.formdef.name
    le str() est de trop, il faudrait utiliser ugettext_lazy sinon la chaîne référencé pour la traduction ne correspondra jamais, mais je ne sais pas si ça marche dans w.c.s., en attenant juste mettre N_('Importing CSV file into cards'), s'il faut d'autres détails les stocker sur le job et les afficher dans le template (les jobs sont toujours des pickles il me semble donc tu peux stocker tout ce que tu veux sur l'objet retourné par add_after_job,
  • dans le template job.status doit être traduit ou passer traduit dans le contexte (dans les autres vues il est toujours fait _(job.status) avant usage, idem pour le label qui est généralement utilisé ainsi _(job.label)).
#24

Mis à jour par Serghei Mihai il y a environ 4 ans

Benjamin Dauvergne a écrit :

  • Dans tes tests les assert 'import-csv' in resp me semble inutile tant que tu cliques ensuite sur quelque chose, ça raccourcit le test et le rend plus lisible
  • en fin de test, il manque l'attente puis la validation que l'import a eu lieu (que les cartes avec les données correspondantes sont là)
  • pourquoi ça passe de mettre map dans un MapField ? (vu)

Ok.

  • pourquoi le champ fichier du formulaire n'est pas requis ? ça éviterait un check ensuite dans submit je pense
  • pourquoi produire un fichier d'exemple pour des colonnes qui ne peuvent être importées comme MapField ? il faudrait les retirer

A la base mon idée est d'exposer les champs du formulaire, avec la colonne vide pour un champ qu'on ne peut pas remplir à partir d'un CSV.
Cela me semble plus clair pour l'usager qui souhaite utiliser le modèle que de ne pas retrouver un champ de sa fiche dans le CSV.

Je préfère afficher un warning comme quoi le champ ne sera pas importé, comme tu le suggères plus bas.

  • si un champ non supporté est un champs requis de la fiche je dirai qu'il ne faudrait pas permettre l'import, juste afficher "import impossible parce que : 1, 2, 3..."

Tu as raison.

  • dans le template job.status doit être traduit ou passer traduit dans le contexte (dans les autres vues il est toujours fait _(job.status) avant usage, idem pour le label qui est généralement utilisé ainsi _(job.label)).

alors {% trans job.status %} et le libellé est déjà internationalisé.

#25

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

Serghei Mihai a écrit :

A la base mon idée est d'exposer les champs du formulaire, avec la colonne vide pour un champ qu'on ne peut pas remplir à partir d'un CSV.
Cela me semble plus clair pour l'usager qui souhaite utiliser le modèle que de ne pas retrouver un champ de sa fiche dans le CSV.

Je comprends, alors plus immédiat pour l'utilisateur, mettre dans la valeur d'exemple « ne sera pas importé - type "xxx" non supporté » au lieu d'une chaîne vide.

#26

Mis à jour par Serghei Mihai il y a environ 4 ans

Yes. Et warnings sur les champs ignorés lors de l'execution du job.

#27

Mis à jour par Benjamin Dauvergne il y a environ 4 ans

  • Statut changé de Solution proposée à Solution validée

Ça m'a l'air ok.

#28

Mis à jour par Serghei Mihai il y a environ 4 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 5519f4887d9043c2bbb09c5ef33ed813e4506293 (origin/master, origin/HEAD)
Author: Serghei Mihai <smihai@entrouvert.com>
Date:   Mon Feb 3 15:59:51 2020 +0100

    cards: add import data from CSV (#39473)
#29

Mis à jour par Frédéric Péters il y a environ 4 ans

Pousser les trucs le jeudi après-midi, c'est s'obliger à gérer par hotfix l'avant mise à jour, il faut éviter.

#30

Mis à jour par Frédéric Péters il y a environ 4 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF