ajout de l'import d'une archive de tutoriel

[firm1]

Q	R
Correction de bugs ?	non
Nouvelle Fonctionnalité ?	oui
Tickets concernés	#949

Cette PR a pour objectif de donner la possibilité aux membres d'importer leurs tutoriels en ligne à partir d'une archive après avoir modifié les fichiers markdown en hors ligne.

Il y'a pas mal de contrôles qui sont fait pour s'assurer que le manifest.json est bien formé, et qu'on ne se retrouve pas avec des données incohérentes.

Note pour QA

• Créer un tutoriel (big et mini) en ligne, avec un minimum de contenu
• Téléchargez l'archive en hors ligne.
• Modifier certains fichiers markdown de votre contenu, et zippez (le format zip est le seul supporté) à nouveau le répertoire de votre tutoriel.
• Aller sur l'interface d'import (/tutoriels/importer/) et importez le.

Quelques exceptions à vérifier.

• On ne doit pas pouvoir importer un mini tutoriel dans un big tutoriel, et vice versa
• On ne doit pas pouvoir importer un tutoriel avec des chapitres/parties/extraits dont la clé primaire n'existe pas en base de donnée : cette PR ne gère pas les nouveaux chapitres/parties/extraits.
• Et d'autres trucs ...

[coveralls]

Coverage decreased (-0.88%) when pulling cb0b51a on firm1:feature-import-archive into 42a31d9 on zestedesavoir:dev.

[Eskimon]

On est toujours en freeze non ? (même si j'approuve fortement cette feature)

Ca serait bien d'avoir des tests, car une feature qui fait perdre 1% de coverage ca laisse presager un gros bout de code tout de meme...

[firm1]

On est toujours en freeze non ?

ouaip, c'est pour ça que la PR va dans dev et pas dans la branche master de release.

[firm1]

Ca serait bien d'avoir des tests

C'est vachement compliqué d'écrire un test là dessus. C'est un coup à tester le test

[Eskimon]

ouaip, c'est pour ça que la PR va dans dev et pas dans la branche master de release.

Arf excuse moi, j'ai pas pris l'habitude de verifier la branche de destination a cause du freeze

[coveralls]

Coverage decreased (-0.5%) when pulling 9e7a8c8 on firm1:feature-import-archive into 42a31d9 on zestedesavoir:dev.

[pierre-24]

Super PR, j'attendais ça avec impatience :) (par contre, la QA de ce machin va pas être drôle à faire).

Ceci dit, j'ai tout de même deux questions,

• Comme ton code est écrit, on dirait qu'il n'est possible d'importer (donc de re-importer) que de tutoriels qui existent déjà, au moins un minimum. Pourquoi pas partir du principe que le tutoriel pourrais ne pas exister à la base et avoir été créé from scratch via un éditeur externe ?
• Question un peu liée à la précédente ... Comme je lis ta fonction check_json(), je vois qu'a priori, il n'est pas possible de modifier l'archive à la main et de rajouter de sois-même une partie/chapitre/extrait de soi-même (pas possible de deviner le pk). Si c'est le cas, qu'est ce qui justifie ce choix ? Pourquoi ne pourrait-on pas modifier son archive puis la ré-importer ? D'autant qu'il me semble que c'est ce vers quoi la ZEP-12 tend à terme ...

[firm1]

Comme ton code est écrit, on dirait qu'il n'est possible d'importer (donc de re-importer) que de tutoriels qui existent déjà, au moins un minimum.

Comme tu l'as bien dit et comme je l'ai expliqué dans la PR, le champ d'action de cette PR se limite au fait que le tutoriel doit au préalable avoir été crée sur le site, afin d'être certain que les structures sont respectées.

Pourquoi pas partir du principe que le tutoriel pourrais ne pas exister à la base et avoir été créé from scratch via un éditeur externe ?

Parce que cette partie mérite une bonne réflexion collective autour du sujet. Il y'a pas mal de points qui peuvent faire débats, tel que :

• est-ce qu'on autorise les utilisateurs a modifier le json à la main ? (cette PR ne l'autorise pas)
• si l'utilisateur peut modifier le json à la main ou via un editeur codé maison (necessaire pour rajouter des partie/chapitres/extraits) :
  • Quels sont les données obligatoires pour créer une partie/chapitre/extrait ?
  • Si l'utilisateur supprimer des lignes du manifest.json, devons nous considérer ça comme une suppression de partie/chapitre/extraits ?
  • L'utilisateur peut-il réordonner ses parties/chapitres/extraits via le manifest.json ?
  • et plein d'autres questions assez techniques au final

Donc pour l'instant, je me suis contenté de coder la partie unanime, logique et qui fonctionne correctement. Rajouter par la suite la possibilité de créer from scratch ne sera qu'une simple formalité une fois qu'on saura ce qu'on veut.

Je pense que même avec les limitations de cette PR, elle sera déjà largement utile pour nos auteurs qui ne demandent qu'a pouvoir éditer en ligne, et ouvrirait des portes a ceux qui veulent coder un client lourd pour éditer en hors ligne.

Question un peu liée à la précédente ... Comme je lis ta fonction check_json(), je vois qu'a priori, il n'est pas possible de modifier l'archive à la main et de rajouter de sois-même une partie/chapitre/extrait de soi-même (pas possible de deviner le pk). Si c'est le cas, qu'est ce qui justifie ce choix ? Pourquoi ne pourrait-on pas modifier son archive puis la ré-importer ?

Comme on peut le lire dans la QA Note, ce qui est permis ici, c'est la modification du contenu des fichiers markdown. en gros, tu viens sur le site, tu crée ta structure (parties/chapitres/extraits), sans forcément remplir le contenu, tu télécharges l'archive, et tu peux tranquillement travailler en hors ligne en te contenant de modifier les fichiers markdown.

Donc encore une fois, ce n'est qu'on scope de départ, qui pourra évoluer de manière certaine, mais qui aura le mérite de poser des bases.

[pierre-24]

Ok, c'était juste pour être bien sur. Bon, je ferais la QA tantôt en gardant bien ça en tête (c'est mon genre d'aller poser ce genre de questions dans le rapport de QA si j'oublie, donc comme ça c'est bon). Je rajouterais quand même une mini-note quelque part pour préciser que non, pas de changement de structure dans l'absolu.

(pour ce mettre d'accord sur ce qui doit être dans le manifest.json, c'est via la ZEP-12 et ZEP-08, qui avancent petit à petit ^^ )

[pierre-24]

Rapport de QA :

• le code donne effectivement le fait qu'on importe du zip ... Sauf que quand on importe du tar (par erreur), on a aucune erreur (ça ne marche pas, par contre).
• Par contre, quand on importe du zip, on se mange une ValueError, ~/utils/tutorials.py in import_archive, line 367. Donc en fait, je suis un peu coincé sur la QA, parce que cette erreur ce produit quand on prend l'archive tar originale, qu'on la converti en zip et qu'on la réimporte (donc même sans modifications).

[pierre-24]

EDIT:

• L'erreur se produit que ce soit avec un big ou un mini tutoriel
• Au fait, dans la template d'import de tutoriel, le fil d'ariane fait Accueil > Tutoriels > Importer un tutoriel, je pense que ça serait logique que ce soit Accueil > Mes tutoriels > Importer un tutoriel

[Alex-D]

Au fait, dans la template d'import de tutoriel, le fil d'ariane fait Accueil > Tutoriels > Importer un tutoriel, je pense que ça serait logique que ce soit Accueil > Mes tutoriels > Importer un tutoriel

Hors scope de la PR.

[firm1]

Merci pour la QA, je viens de push les corrections et ajouter des tests unitaires pour confirmer tout ça.

QA CAN CONTINUE

[coveralls]

Coverage increased (+1.05%) when pulling 15d4f57 on firm1:feature-import-archive into 42a31d9 on zestedesavoir:dev.

[coveralls]

Coverage increased (+1.04%) when pulling 15d4f57 on firm1:feature-import-archive into 42a31d9 on zestedesavoir:dev.

[coveralls]

Changes Unknown when pulling 1ee0d98 on firm1:feature-import-archive into * on zestedesavoir:dev*.

[pierre-24]

Rapport de QA :

• Le fil d'ariane de la page d'import devrait être "accueil > mes tutoriels > importer un tutoriel" et non pas "accueil > tutoriels > importer un tutoriel", non ?
• J'ai crée un mini-tuto contenant un extrait contenant "A". J'ai téléchargé l'archive tar, ai modifié l'extrait en "B", ai refait un zip, l'ai réimporté ... Mais le contenu de l'extrait était toujours "A", pas OK
• Quand on modifie l'intro et la conclusion, ça ne marche pas non plus : pas OK
• Quand on modifie le titre et la description, ça fonctionne ... Presque. Je pense que la BDD n'est pas mise à jour, manque un update(); parce que quand on édite, on vois les modifs, mais pas quand on regarde le tuto : pas OK.
• Quand tente d'importer sans préciser de fichier, on a une erreur python "MultiValueDictKeyError at /tutoriels/importer/" sur "file"; pas OK
• Quand on importe le mini-tuto dans un big-tuto, on recoit un message d'erreur : OK
• Quand on importe le mini-tuto dans le mauvais mini-tuto, on a aussi droit à une erreur : OK
• Quand on édite le manifest.json pour mettre le mauvais pk à l'extrait, on a un message d'erreur : OK
• Quand on édite le manifest.json pour indiquer un fichier md qui n'existe pas comme extrait, on a pas de message d'erreur : pas OK
• Quand on édite le manifest.json pour indiquer un fichier md qui n'existe pas, comme extrait, on a pas de message d'erreur : pas OK

Je testerai avec un big-tuto plus tard.

Et puis, sérieusement, à ce sujet pourquoi du zip alors qu'on télécharge du tar ?? Ceci dit, cette fois-ci, quand on importe une archive tar, on recoit un message d'erreur.

EDIT: à force de tester, maintenant, quand je demande de re-télécharger l'archive du tuto, il mouline dans le vide sans rien renvoyer. Je vois pas pourquoi o_O

[Alex-D]

Et puis, sérieusement, à ce sujet pourquoi du zip alors qu'on télécharge du tar ??

On devrait proposer les deux : zip et tgz.

[pierre-24]

On devrait proposer les deux : zip et tgz.

Pourquoi pas, effectivement. Par contre, coté code, ça, ça serait plus tendu, je pense.

[firm1]

Le fil d'ariane de la page d'import devrait être "accueil > mes tutoriels > importer un tutoriel" et non pas "accueil > tutoriels > importer un tutoriel", non ?

Oui, mais ce n'est clairement pas le scope de la PR

Et puis, sérieusement, à ce sujet pourquoi du zip alors qu'on télécharge du tar ?? Ceci dit, cette fois-ci, quand on importe une archive tar, on recoit un message d'erreur.

Le zip est accessible a tous tout simplement. Proposer du tar serait un bonus, mais pour l'instant on commence déjà par du zip car il touche tout le monde. Parce que sinon, chacun va demander son format (pourquoi pas du 7zip aussi etc.)

EDIT: à force de tester, maintenant, quand je demande de re-télécharger l'archive du tuto, il mouline dans le vide sans rien renvoyer. Je vois pas pourquoi o_O

Repars sur un nouveau tutoriel.

Sinon, pour le reste c'est corrigé donc :

QA CAN CONTINUE

[Eskimon]

C'est PEP qui bug apparemment

[pierre-24]

Vous avez mis PEP dans les tests ?!? o_O

[Eskimon]

Apparemment @firm1 oui : #1489

[firm1]

C'est PEP qui bug apparemment

Ah, en effet, il manque un commit je pense :)

Vous avez mis PEP dans les tests ?!? o_O

Oui, comme ça tout reste propre.

[coveralls]

Changes Unknown when pulling 4cff14e on firm1:feature-import-archive into * on zestedesavoir:dev*.

[firm1]

Au cas ou mes modifs sont passés inaperçu :(

QA CAN CONTINUE

[pierre-24]

Juste, je l'avais laissée de coté avec la PR sur les archives ZIP. Maintenant qu'on sort du zip, il est plus logique qu'on rentre du zip à nouveau. Donc ...

Rapport de QA

Sur mini-tuto:

• Changer le titre, la description, l'intro, la conclusion ou le texte fonctionne, donc c'est cool, OK
• On a une "erreur" quand on oublie de préciser le fichier, OK
• On a une erreur quand on indique exprès un mauvais fichier markdown pour texte, OK
• On a bien une erreur quand on fait exprès de mettre un mauvais fichier markdown pour intro ou conclu. Par contre, le message d'erreur est bizarre : Le fichier « Mini-tuto en draft (seul) » renseigné dans vos métadonnées pour le tutoriel « introduction1.md » ne se trouve pas dans votre zip. Ce serait pas l'inverse ? :)

J'ai malheureusement pas le temps de tester convenablement sur big-tuto pour l'instant, mais les erreurs on bien été corrigé, donc ça avance !

[pierre-24]

Petite réflexion qui me vient à l'esprit : ça dépent à quel point on est strict sur notre propre norme, mais logiquement, je pourrait pas apeller le fichier d'intro autrement que introduction.md, si ?

[firm1]

Petite réflexion qui me vient à l'esprit : ça dépent à quel point on est strict sur notre propre norme, mais logiquement, je pourrait pas apeller le fichier d'intro autrement que introduction.md, si ?

Dans la version actuelle ce n'est pas possible d'appeler l'introduction autrement. Mais le but de la ZEP12 est justement de faire évoluer ça.

[firm1]

Ce serait pas l'inverse ? :)

Si, j'ai corrigé du coup.

QA CAN CONTINUE

[Eskimon]

Sinon faudra aussi rajouter ... de la doc ... (yeah haut les coeurs !!) :D

[firm1]

Sinon faudra aussi rajouter ... de la doc

Ah bah oui. Mais ça n'empêche pas de QA déjà :)

[coveralls]

Changes Unknown when pulling 4fef0e8 on firm1:feature-import-archive into * on zestedesavoir:dev*.

[coveralls]

Changes Unknown when pulling bfef889 on firm1:feature-import-archive into * on zestedesavoir:dev*.

[firm1]

Les conflits vont redébarquer ici c'est possible de QA/review par ici ?

[pierre-24]

Je reviens, je reviens. Ce soir :)

[pierre-24]

Rapport de QA :

• Import de big tuto avec modifs : OK
• Import de tutos avec erreurs dans les fichiers, message d'erreur obtenu, OK
• Import de tuto avec pk incorrects, message d'erreur obtenu, OK

Donc ... À merger :)

[Eskimon]

Je viens de tester en pprod', j'ai eu le message suivant en important mon archive:

L'archive n'a pas pu être importée car le fichier manifest.json (fichier de métadonnées est introuvable).

Je confirme pourtant la présence de mon manifest dans le zip
(et la parenthèse se ferme au mauvais endroit)

------- EDIT ------------

En fait j'ai compris pourquoi... J'ai zippe mon dossier de travail plutôt que les fichiers. Du coup l'arborescence était pas celle attendu je suppose. C'est dommage cependant... Moyen de régler ca ?

[firm1]

En fait j'ai compris pourquoi... J'ai zippe mon dossier de travail plutôt que les fichiers. Du coup l'arborescence était pas celle attendu je suppose. C'est dommage cependant... Moyen de régler ca ?

possible oui, mais je ne pourrais pas m'en charger.