Development #21602
Versionning des pages
0%
Description
Sans passer par django-reversion qui est trop attaché aux modèles, qui serait galère avec la structure qu'on a ici. Plutôt lors d'une modif (de cellule ou de page) commencer par une sérialisation json de celle-ci et taper cette sérialisation dans un modèle PageRevision (timestamp/author/serialization).
À côté de ça, pour l'UI, dans un premier temps je dirais juste une page historique listant les versions (cf ce qu'on a dans welco) et à l'affichage d'une version, afficher les différences : pour les métadonnées directes c'est facile, pour les cellules juste dire "ajoutée/modifiée/supprimée" sans plus de détails (dans un second temps permettre aux cellules d'afficher une info détaillée sur les changements). Comme le rendu d'une page est compliqué par des appels asynchrones je n'incluerais pas de possibilité pour afficher la page telle qu'elle était à cette version. Mais une action pour restaurer la version, oui.
Fichiers
Révisions associées
general: add possibility to restore snapshots from history (#21602)
Historique
Mis à jour par Frédéric Péters il y a environ 6 ans
- Fichier 0001-general-add-page-versionning-21602.patch 0001-general-add-page-versionning-21602.patch ajouté
- Fichier versionning.png versionning.png ajouté
- Statut changé de Nouveau à En cours
- Patch proposed changé de Non à Oui
Un peu différent en terme de fonctionnalités que ce que je notais, j'ai trouvé plus important de pouvoir voir pour de vrai ce qu'était une version passée, cellules asynchrones etc. et plutôt qu'un système de diffs compliqué(s). Pour le moment j'accompagne simplement les modifications d'un commentaire (posé automatiquement).
Bref, on a :
- sur les modifs de page/cellules enregistrement dans un modèle PageSnapshot de la sérialisation page/cellules
- une page affichant l'historique des modifs (cf capture)
- la possibilité depuis celle-ci d'ouvrir une version ancienne, avec dans celle-ci également les cellules asynchrones dans leur état passé
Les évolutions à venir rapidement derrière : la possibilité de restaurer une ancienne version et une commande de gestion pour nettoyer les objets créés pour la visualisation.
Mis à jour par Frédéric Péters il y a environ 6 ans
- Fichier 0002-general-add-possibility-to-restore-snapshots-from-hi.patch 0002-general-add-possibility-to-restore-snapshots-from-hi.patch ajouté
- Fichier 0001-general-add-page-versionning-21602.patch 0001-general-add-page-versionning-21602.patch ajouté
Voilà avec un second patch permettant de restaurer une ancienne version.
Mis à jour par Frédéric Péters il y a environ 6 ans
- Fichier 0001-general-add-page-versionning-21602.patch 0001-general-add-page-versionning-21602.patch ajouté
Mise à jour du premier patch suite à #23262.
Mis à jour par Frédéric Péters il y a presque 6 ans
- Fichier 0002-general-add-possibility-to-restore-snapshots-from-hi.patch 0002-general-add-possibility-to-restore-snapshots-from-hi.patch ajouté
- Fichier 0001-general-add-page-versionning-21602.patch 0001-general-add-page-versionning-21602.patch ajouté
Rebase des deux patchs.
Mis à jour par Emmanuel Cazenave il y a presque 6 ans
La fonctionnalité est bien sympa, usage facile et compréhensible.
J'avoue ne pas comprendre la nécessité d'avoir une foreign key snapshot sur Page, je trouve
son utilisation un peu dure à suivre dans le code.
Sémantiquement ça semble indiquer qu'une page résulte de la restauration d'un snapshot, so what ?
Je crois que l'attribut de classe snaphosts sur la classe PageManager ne sert à rien.
Manque les traductions.
Mis à jour par Frédéric Péters il y a presque 6 ans
J'avoue ne pas comprendre la nécessité d'avoir une foreign key snapshot sur Page, je trouve son utilisation un peu dure à suivre dans le code.
Il y a possibilité de visualiser un snapshot passé, ça passe par une restauration temporaire de la page et de son contenu. Mais on ne voudrait pas que cette page temporaire se trouve dans la liste des pages, etc.
Je crois que l'attribut de classe snaphosts sur la classe PageManager ne sert à rien.
Il sert à obtenir une page qui est une restauration temporaire, alors que le manager par défaut zappe celles-ci.
Manque les traductions.
Yep dans un commit à part au moment de la release.
Mis à jour par Emmanuel Cazenave il y a presque 6 ans
Frédéric Péters a écrit :
Il y a possibilité de visualiser un snapshot passé, ça passe par une restauration temporaire de la page et de son contenu. Mais on ne voudrait pas que cette page temporaire se trouve dans la liste des pages, etc.
Oui d'accord d'où le "une commande de gestion pour nettoyer les objets créés pour la visualisation." dans les étapes à venir.
Je m'attendais à ce qu'on ait pas besoin de persistance sur ces objets, et que ça puisse fonctionner à la volée.
Pour ma compréhension (et/ou pour celle de tout le monde, avec un commentaire dans le code, c'est pas clair pour Thomas non plus) je veux bien deux mots là dessus (j'imagine que le code de rendu d'une page, d'un snapshot en l’occurrence, a besoin d'objets déjà persistés)
Je crois que l'attribut de classe snaphosts sur la classe PageManager ne sert à rien.
Il sert à obtenir une page qui est une restauration temporaire, alors que le manager par défaut zappe celles-ci.
Tu parles de Page.snapshots, je parle de PageManager.snapshots qui vraiment semble ne servir à rien : en le supprimant aucune erreur dans
les tests unitaires et dans les tests manuels dans l'UI.
class PageManager(models.Manager):
snapshots = False
Mis à jour par Frédéric Péters il y a presque 6 ans
Pour ma compréhension (et/ou pour celle de tout le monde, avec un commentaire dans le code, c'est pas clair pour Thomas non plus) je veux bien deux mots là dessus (j'imagine que le code de rendu d'une page, d'un snapshot en l’occurrence, a besoin d'objets déjà persistés)
# mark temporarily restored snapshots, it is required to save objects
# (pages and cells) for real for viewing past snapshots as many cells are
# asynchronously loaded and must refer to a real Page object.
Tu parles de Page.snapshots, je parle de PageManager.snapshots qui vraiment semble ne servir à rien : en le supprimant aucune erreur dans les tests unitaires et dans les tests manuels dans l'UI.
L'attribut est nécessaire, utilisé dans le get_queryset (explication dans le précédent commentaire); mais peut-être que tu parles juste de retirer la déclaration, étant donné qu'il est positionné lors du __init__ ? Perso j'aime bien avoir une vue des attributs de la classe directement au niveau de celle-ci.
Mis à jour par Emmanuel Cazenave il y a presque 6 ans
L'attribut est nécessaire, utilisé dans le get_queryset (explication dans le précédent commentaire); mais peut-être que tu parles juste de retirer la déclaration, étant donné qu'il est positionné lors du
__init__? Perso j'aime bien avoir une vue des attributs de la classe directement au niveau de celle-ci.
Oui c'est ça, mais donc amen pour tes préférences (ça me fait penser à ça https://attrs.readthedocs.io/en/stable/) et ack.
Mis à jour par Frédéric Péters il y a presque 6 ans
- Statut changé de En cours à Résolu (à déployer)
commit 0a610fd2a2c006c79949aec99df14570c9fb9b83
Author: Frédéric Péters <fpeters@entrouvert.com>
Date: Mon Apr 2 12:07:34 2018 +0200
general: add possibility to restore snapshots from history (#21602)
commit 43da526e699cb1509e821559db0971fd7d8f2802
Author: Frédéric Péters <fpeters@entrouvert.com>
Date: Sun Apr 1 15:23:01 2018 +0200
general: add page versionning (#21602)
Mis à jour par Frédéric Péters il y a plus de 5 ans
- Statut changé de Résolu (à déployer) à Solution déployée
general: add page versionning (#21602)