Development #39091
nouvelle cellule "Pages récemment modifiées"
0%
Description
Cellule qu'on configurerait avec la hiérarchie de pages à suivre (en gros sélection de "tout le site" ou d'une page précise) + le nombre de pages à reprendre; et le rendu ce serait une liste de lien vers les pages + date/heure (de la dernière modification) +
un petit mot "nouvelle page" pour distinguer les créations de pages des modifications.
Fichiers
Révisions associées
data: deprecate get_last_update_time using signals (#39091)
data: get descendants of a page (#39091)
kb: add cell to display last page updates (#39091)
Historique
Mis à jour par Frédéric Péters il y a environ 4 ans
- Echéance mis à 17 février 2020
- Assigné à mis à Nicolas Roche (absent jusqu'au 3 avril)
Mis à jour par Frédéric Péters il y a environ 4 ans
Faire ça dans un nouveau module combo.apps.kb, qui pourra plus tard prendre d'autres trucs en rapport base de connaissances.
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0001-kb-add-cell-to-display-last-updated-pages-39091.patch 0001-kb-add-cell-to-display-last-updated-pages-39091.patch ajouté
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
nickel les specs, merci.
Mis à jour par Frédéric Péters il y a environ 4 ans
class LastUpdatedCell(CellBase):
Appelons plutôt ça LastPageUpdatesCell.
follow_page = models.ForeignKey(... verbose_name=_('Followed parent page')))
J'ai du mal à capter la traduction que tu imagines ici. (je suggérerais de faire comme MenuCell et d'appeler ça root_page).
def get_descendants(page): descendants = [page] for item in page.get_children(): descendants.extend(get_descendants(item)) return descendants
Ça me semble pouvoir être descendu au niveau de l'objet Page.
last_update_times = [(x.get_last_update_time(), x) for x in followed_pages]
Directement .ordered_by('-last_update_timestamp') plutôt qu'un tri manuel après coup.
Ailleurs il faudrait noter get_last_update_time comme deprecated et juste retourner .last_update_timestamp, qui va toujours être bon via le cache dans l'attribut related_cells.
item['is_new'] = not PageSnapshot.objects.filter(page=page)
Ça semble bricolage bizarre mais je me rends compte qu'il n'y a pas de creation_timestamp sur l'objet Page. L'ajouter et simplement utiliser ça pour déterminer si c'est une nouvelle page. (définir nouvelle page comme page créée il y a moins d'une semaine, genre, arbitrairement).
urlpatterns = [ ]
Ça, et tout le fichier, sont inutiles.
from bs4 import BeautifulSoup
Ça m'irait de ne pas ajouter cette dépendance, que simplement ça se passe ainsi :
- html = BeautifulSoup(cell.render(ctx)) - assert len(html.find_all('li')) == 3 + assert cell.render(ctx).count('<li') == 3
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0004-kb-add-cell-to-display-last-updated-pages-39091.patch 0004-kb-add-cell-to-display-last-updated-pages-39091.patch ajouté
- Fichier 0003-data-get-descendants-of-a-page-39091.patch 0003-data-get-descendants-of-a-page-39091.patch ajouté
- Fichier 0002-data-deprecate-get_last_update_time-using-signals-39.patch 0002-data-deprecate-get_last_update_time-using-signals-39.patch ajouté
- Fichier 0001-data-add-creation-timestamp-on-Page-object-39091.patch 0001-data-add-creation-timestamp-on-Page-object-39091.patch ajouté
- il n'y a pas de creation_timestamp sur l'objet Page. L'ajouter et simplement utiliser ça pour déterminer si c'est une nouvelle page.
- noter get_last_update_time comme deprecated et juste retourner .last_update_timestamp, qui va toujours être bon via le cache dans l'attribut related_cells.
- Ça me semble pouvoir être descendu au niveau de l'objet Page.
Puis le dernier correctif prend en compte les autres remarques, propres à ce ticket.
Avec en plus une modification sur la suppression de la root_page d'une cellule "page récemment modifiées" :
plutôt de de supprimer la cellule (on_delete=CASCADE), la cellule pointe à présent sur toutes les pages (on delete=SET_NULL).
(ce comportement me semblait plus approprié)
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
Ci-dessous deux explications supplémentaires.
Dans le premier patch,
creation_timestamp = models.DateTimeField(auto_now_add=True, null=True)
met la date de la migration pour les pages pré-existantes, ce qui peu sembler pas terrible.
Si je met
creation_timestamp = models.DateTimeField(auto_now_add=True)
alors la migration demande une valeur :
You are trying to add the field 'creation_timestamp' with 'auto_now_add=True' to page without a default; the database needs something to populate existing rows. 1) Provide a one-off default now (will be set on all existing rows) 2) Quit, and let me add a default in models.py
L'option 1 done au moment du migrate_schema :
django.db.utils.IntegrityError: ERREUR: la colonne « creation_timestamp » contient des valeurs NULL
L'option 2 ne fonctionne pas non plus :
creation_timestamp = models.DateTimeField(auto_now_add=True, null=True, default=None)
done au moment du
makemigrations
:ERRORS: data.Page.creation_timestamp: (fields.E160) The options auto_now, auto_now_add, and default are mutually exclusive. Only one of these options may be present.
Donc soit je fait une migration de donnée comme ici : https://stackoverflow.com/questions/56992825/how-to-let-existing-entries-empty-when-using-auto-now-add
soit je je laisse en l'état ce premier patch et pendant la première semaines, les pages pré-existantes seront toutes vues comme nouvelles.
Le second patch fait en sorte que chaque modification sur les cellules met à jour le champs last_update_timestamp
de la page qui la contient.
Il se base sur le travail fait dans #24239.
Mis à jour par Frédéric Péters il y a environ 4 ans
1) Provide a one-off default now (will be set on all existing rows)
À ça, répondre last_update_timestamp, ne fonctionne pas ?
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0001-data-add-creation-timestamp-on-Page-object-39091.patch 0001-data-add-creation-timestamp-on-Page-object-39091.patch ajouté
Non, répondre last_update_timestamp
, ne fonctionne pas.
Mais, c'est effectivement un bon candidat comme valeur initiale pour les lignes pré-existantes (cf 0001).
Encore autre chose, mais là je tourne en rond :
comme la migration (patch ci-dessus) fait un save()
pour renseigner la valeur du nouveau champ, alors toutes les anciennes pages vont être "rafraîchies" et apparaître comme ayant été modifiées dans la cellule "pages récemment modifiées" (ce qui est déjà un peu mieux, malgré tout).
Mis à jour par Frédéric Péters il y a environ 4 ans
Je trouve curieux que ça puisse fonctionner, parce que ça va laisser des lignes sans valeur entre le migrations.AddField et le migrations.RunPython, non ? Tu as joué ça avec postgresql ? Mais sinon on s'en fout on met timezone.now et hop, plutôt que s'ajouter des nouvelles questions.
~~
def get_last_update_time(self): + '''deprecated: + page.last_update_timestamp is now updated by + cell_maintain_page_cell_cache signal, triggered when saving cells. + ''' cells = CellBase.get_cells(page_id=self.id) return max([self.last_update_timestamp] + [x.last_update_timestamp for x in cells])
Ça sert à rien de laisser tous les calculs alors qu'on met un commentaire comme quoi page.last_update_timestamp suffit. Virer le commentaire/docstring et retourner page.last_update_timestamp.
~~
followed_pages
Je ne comprends toujours pas ce qui peut être compris par "followed"; tape juste pages comme nom de variable.
~~
+ {{ page.title }} + <em> + + {% trans "on"%} {{ page.last_update_timestamp }} + {% if page.is_new %} + {% trans "(new page)"%} + {% endif %} + </em>
il n'est sans doute pas nécessaire de taper d'espace insécable.
Taper aussi bien le titre que la date que le (new page) dans des <span> indépendants, avec une classe avec un nom approprité, pas taper de <em>.
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0001-data-add-creation-timestamp-on-Page-object-39091.patch 0001-data-add-creation-timestamp-on-Page-object-39091.patch ajouté
- Fichier 0002-data-deprecate-get_last_update_time-using-signals-39.patch 0002-data-deprecate-get_last_update_time-using-signals-39.patch ajouté
- Fichier 0003-data-get-descendants-of-a-page-39091.patch 0003-data-get-descendants-of-a-page-39091.patch ajouté
- Fichier 0004-kb-add-cell-to-display-last-updated-pages-39091.patch 0004-kb-add-cell-to-display-last-updated-pages-39091.patch ajouté
Tu as joué ça avec postgresql ?
oui
on s'en fout on met timezone.now
fait dans 0001
Virer le commentaire/docstring et retourner page.last_update_timestamp.
fait dans 0002 (j'ai du retirer l'ancien test qui ne fonctionnait plus)
J'ai traité les remarques suivantes dans 0004 (merci ThomasJ).
Remarque :
Les espaces entre le texte des balises <span>
sont présents sur le portail agent (display: block
),
mais pas sur le portail utilisateur, avec les thèmes de base (display: flex
).
Mis à jour par Frédéric Péters il y a environ 4 ans
lastpagesupdatescellcestvraimenttropcompliquéàlire, je trouve.
Tape plutôt le contenu contre les balises, pour éviter des espaces qu'on ne veut pas nécessairement.
<span class="lastpagesupdatescell--item-isnew">{% trans "(new page)" %}</span>
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0004-kb-add-cell-to-display-last-updated-pages-39091.patch 0004-kb-add-cell-to-display-last-updated-pages-39091.patch ajouté
<time class="lastpagesupdatescell--item-date" datetime={{ page.last_update_timestamp|datetime|date:"c"}} >{% trans "on"%} {{ page.last_update_timestamp }}</time> {% if page.is_new %} <span class="lastpagesupdatescell--item-isnew">{% trans "(new page)"%}</span>
Sinon ThomasJ devrais aussi bientôt passer par ici...
Il en veut aussi à la longueur du nom de ma classe CSS je crois : 'lastpagesupdatescell'.
Mis à jour par Frédéric Péters il y a environ 4 ans
Et ce bout vient de moi, désolé c'est seulement mainenant que je vois le truc mauvais,
Appelons plutôt ça LastPageUpdatesCell.
C'est plutôt Latest que Last qu'il faut; LatestPageUpdatedCell, désolé.
lastpagesupdatescell
Et le truc facile, comme c'est fait pour les noms de fichiers etc., c'est les tirets.
Le gabarit s'appelle last-pages-updates-cell.html (et mettons sera latest-page-updates-cell.html), la version "classe CSS" da la même cellule devrait être la même chose.
(alors oui, il y a une classe css automatiquement posée à partir du nom de la classe et c'est sans tiret, mais c'est l'occasion de se faire la remarque et de décider de changer ça, j'en fais un ticket dédié, #39314).
Mis à jour par Thomas Jund il y a environ 4 ans
Je me demandais même si latest n'est pas implicite et pourrais être supprimé.
Que `updated-pages-cell` pourrait suffire ?
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
Mis à jour par Frédéric Péters il y a environ 4 ans
Dites-moi, je peux aussi couper la poire en deux et surcharger class_name()
Non, il est important de garder une uniformité.
Si on trouve "Latest" redondant/inutile, il l'est alors également dans le nom de la classe Python.
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0004-kb-add-cell-to-display-last-updated-pages-39091.patch 0004-kb-add-cell-to-display-last-updated-pages-39091.patch ajouté
Ce dernier patch économise 150 octets et contribue donc à sauver des ours du réchauffement climatique.
Mis à jour par Frédéric Péters il y a environ 4 ans
- Statut changé de Solution proposée à Solution validée
Si on trouve "Latest" redondant/inutile (...)
Et dans le titre de la cellule etc. i.e. si on ne veut pas latest, qu'il n'apparaisse pas du tout dans le patch. (perso je trouve plus clair d'avoir le latest partout mais tu fais comme tu veux).
À part ça, ok mais datetime={{ page.last_update_timestamp|datetime|date:"c"}} et {% trans "on"%} , ajoute un espace avant la fermeture du }} ou %}. Aussi, pour l'attribut datetime, ajoute des guillemets autour.
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
perso je trouve plus clair d'avoir le latest
moi aussi, mais dans le précédent patch j'avais déjà une incohérence avec le titre de la cellule.
Et du coup, pour plus de cohérence,
plutôt que de modifier le titre en "Latest Page Updated",
est-ce que s/LatestPageUpdatedCell/LatestUpdatedPagesCell/
conviendrait ?
Mis à jour par Frédéric Péters il y a environ 4 ans
Je serais pour LatestPageUpdate*S*Cell; et Latest Page Updates comme titre, etc.
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0004-kb-add-cell-to-display-last-page-updates-39091.patch 0004-kb-add-cell-to-display-last-page-updates-39091.patch ajouté
- Statut changé de Solution validée à Solution proposée
ok, à présent ça me semble calé.
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0004-kb-add-cell-to-display-last-page-updates-39091.patch 0004-kb-add-cell-to-display-last-page-updates-39091.patch ajouté
oups, j'avai encore oublié un espace dans le template :
<<< {% trans "(new page)"%} --- {% trans "(new page)" %} >>>
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Fichier 0004-kb-add-cell-to-display-last-page-updates-39091.patch 0004-kb-add-cell-to-display-last-page-updates-39091.patch ajouté
- Fichier 0003-data-get-descendants-of-a-page-39091.patch 0003-data-get-descendants-of-a-page-39091.patch ajouté
- Fichier 0002-data-deprecate-get_last_update_time-using-signals-39.patch 0002-data-deprecate-get_last_update_time-using-signals-39.patch ajouté
- Fichier 0001-data-add-creation-timestamp-on-Page-object-39091.patch 0001-data-add-creation-timestamp-on-Page-object-39091.patch ajouté
Rebasage sur les patchs de Lauréline
Mis à jour par Nicolas Roche (absent jusqu'au 3 avril) il y a environ 4 ans
- Statut changé de Solution proposée à Résolu (à déployer)
(Thomas m'a suggéré que c'était déjà validé en l'état)
commit e451097beff65a41d4a39fada9efd1e84fce9e35 Author: Nicolas ROCHE <nroche@entrouvert.com> Date: Fri Jan 31 16:45:27 2020 +0100 kb: add cell to display last page updates (#39091) commit bd558163a85526f34f2a79ecd4d35b2669869ad6 Author: Nicolas ROCHE <nroche@entrouvert.com> Date: Thu Jan 23 15:30:57 2020 +0100 data: get descendants of a page (#39091) commit 3e5e80f6e6daec96914f75345639a5b3b284b149 Author: Nicolas ROCHE <nroche@entrouvert.com> Date: Thu Jan 23 16:25:32 2020 +0100 data: deprecate get_last_update_time using signals (#39091) commit c84c4751e2933de28dc2718f1469330fcba82dc1 Author: Nicolas ROCHE <nroche@entrouvert.com> Date: Thu Jan 23 16:24:45 2020 +0100 data: add creation timestamp on Page object (#39091)
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
data: add creation timestamp on Page object (#39091)