Projet

Général

Profil

Development #47155

Avoir sur les objets un journal des modifications et sur les utilisateurs, en plus, un journal des actions

Ajouté par Benjamin Dauvergne il y a plus de 3 ans. Mis à jour il y a plus de 3 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
30 septembre 2020
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Suite de #20695.


Fichiers


Demandes liées

Lié à Authentic 2 - Development #20695: Avoir sur les objets un journal des modifications et sur les utilisateurs, en plus, un journal des actionsFermé14 décembre 2017

Actions
Bloque Authentic 2 - Development #47467: Produire des statistiques simplesFermé08 octobre 2020

Actions

Révisions associées

Révision 9a1631b1 (diff)
Ajouté par Benjamin Dauvergne il y a plus de 3 ans

misc: add journal application (#47155)

Révision 1cc04e3a (diff)
Ajouté par Benjamin Dauvergne il y a plus de 3 ans

misc: integration of journal authentic views (#47155)

Révision 8487d33c (diff)
Ajouté par Benjamin Dauvergne il y a plus de 3 ans

misc: integration of journal in manager (#47155)

Historique

#1

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Lié à Development #20695: Avoir sur les objets un journal des modifications et sur les utilisateurs, en plus, un journal des actions ajouté
#2

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Statut changé de Nouveau à Solution proposée
  • Patch proposed changé de Non à Oui
Le ticket peut commencer à être relu, mais je rajouterai bien avant de pousser :
  • un setting pour désactiver ça le temps que ça se stabilise au niveau IHM
  • une icône (là j'ai mis class=icon-journal qui ne correspond à rien pour l'instant) pour l'icône en page d'accueil du BO
  • éventuellement une vue de journal pour les rôles en général (sur /manage/roles/journal/) qui montrerait l'ensemble des actions sur les rôles
#3

Mis à jour par Frédéric Péters il y a plus de 3 ans

Pourquoi src/authentic2/apps/quelquechose/ plutôt que les applications directement sous src/ comme c'était le cas ?

L'introduction du typage, ça va sans doute casser Hobo qui continue à tourner ses tests en Python 2, il faudrait vraisemblablement retirer les tests py2/agent authentic d'abord.

L'introduction du typage, sans discussion, dans une situation de délais tendue, bravo.

Il y a un style gadjo pour la pagination. (qui n'a pas l'affichage de raccourcis clavier de vi dans les libellés de boutons, mais c'est une bonne chose).

Il y une petite incohérence dans la forme des libellés d'événements, le "authorize single sign on" sonne bizarre avec son verbe là, plutôt que par exemple "authorization of single sign on".

Au niveau des identifiants c'est l'absence d'underscores pour les premiers mots de celui-ci qui sonne bizarre :

+class DoNotForcePasswordChange(UserEvents):
+    name = 'donotforce_password_change'

Avoir des noms très génériques façon "edit"

+class EditUser(UserEvents):
+    name = 'edit'

ça ne va pas tenir quand il s'agira d'éditer autre chose.

D'ailleurs déjà, pour d'autres événements l'edit est qualifié :

+class ProfileEdit(UserEvents):
+    name = 'profile_edit'

Il n'y aurait pas eu un fichier modifié juste pour l'année du copyright je n'aurais pas fait la remarque sur les nouveaux fichiers pas tous introduits avec 2010-2019.

Ça n'enregistre pas les SSO SAML comme événements. (ça me va très bien d'avoir des tickets par la suite pour les événements à suivre)

Dans le fil d'ariane quand je fais "éditer" sur un utilisateur j'ai "Utilisateurs: entité > Frédéric" mais quand je fais "journal" j'ai "Utilisateurs > Frédéric".

Une modification de l'usager ne touchant à rien s'enregistre/s'affiche avec "edit ()".

L'affichage est chronologique et comme en plus la pagination n'a pas de lien vers la page finale, ce n'est pas pratique. (la pagination gadjo a un lien vers la page finale).

En fait c'est plus bizarre que ça il est chronologique sur la page mais par défaut sur la dernière page et il faut faire "page précédente" pour voir ce qui arrivait avant.

Ça fait que dans le fil d'ariane "Utilisateurs > Frédéric > Journal" quand je suis sur une page avancée je clique sur "Journal" avec l'idée de revenir sur la première^Wdernière page et non ça m'amène sur /manage/journal/.

Il y a une tentative de quelque chose avec

+        $('textarea.js-autoresize').each(function () {

mais je ne vois pas quoi, je ne vois pas de textarea.

Pareil perdu js, il y a un

+        function FitToContent(id, maxHeight)

et rien qui appelle ça. (?)

Un échec de connexion est enregistré avec username:fred mais pas attaché à l'utilisateur concerné, du coup on ne peut pas avoir l'info d'échec depuis le journal de l'usager il faut aller sur le journal global et chercher username:whatever et ça va retourner les événements pour tous les comptes commençant par whatever et on va dire que souvent c'est ok parce que l'username ça sera l'email; reste que pour exploiter ensuite les données du journal pour afficher à l'usager une liste des (tentatives de) connexions à son compte, il me semble qu'un truc manque.

Côté code coverage on est à 79% ce qui est pile le coverage général, donc on ne peut pas dire que ça fait baisser la moyenne, mais c'est quand même pas terrible.

Voilà pour mon vrac de commentaires; je laisse la suite à d'autres.

#4

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

Pourquoi src/authentic2/apps/quelquechose/ plutôt que les applications directement sous src/ comme c'était le cas ?

Parce que ça fait des tables avec des noms trop long, là ça donne journal_event c'est mieux.

L'introduction du typage, ça va sans doute casser Hobo qui continue à tourner ses tests en Python 2, il faudrait vraisemblablement retirer les tests py2/agent authentic d'abord.

Ok je vire ça, mais nos codes ne marchent qu'en python3 désormais de toute façon, plus personne ne fait attention à la compatibilité py2, il faut désactiver py2 dans hobo c'est tout.

Il y a un style gadjo pour la pagination. (qui n'a pas l'affichage de raccourcis clavier de vi dans les libellés de boutons, mais c'est une bonne chose).

Ok, mais ça ne rend pas super bien avec previous/next page (on ne peut pas utiliser de la pagination par offset/limit).

Il y une petite incohérence dans la forme des libellés d'événements, le "authorize single sign on" sonne bizarre avec son verbe là, plutôt que par exemple "authorization of single sign on".

Ok je vais utiliser une forme nominale pour tous.

Au niveau des identifiants c'est l'absence d'underscores pour les premiers mots de celui-ci qui sonne bizarre :

[...]

Parce que le nom complet dépend de la classe parent, c'est 'manager.user.edit' le vrai nom et c'est comme ça qu'il est appelé dans le code.

Avoir des noms très génériques façon "edit"

[...]

ça ne va pas tenir quand il s'agira d'éditer autre chose.

D'ailleurs déjà, pour d'autres événements l'edit est qualifié :

[...]

Pour moi c'est différent d'un edit dans le manager pour cela que j'ai qualifié ainsi mais je peux mettre 'edit' comme pour le manager.

Il n'y aurait pas eu un fichier modifié juste pour l'année du copyright je n'aurais pas fait la remarque sur les nouveaux fichiers pas tous introduits avec 2010-2019.

Ça n'enregistre pas les SSO SAML comme événements. (ça me va très bien d'avoir des tickets par la suite pour les événements à suivre)

C'est voulu, je ne pense pas qu'on veuille voir les SSOs vers nos briques ça va noyer l'information, mon idée pour un prochain ticket c'est d'avoir un booléen sur les servics 'log SSO' qu'on pourra mettre à faux lors du (re)déploiement via hobo-deploy.

Dans le fil d'ariane quand je fais "éditer" sur un utilisateur j'ai "Utilisateurs: entité > Frédéric" mais quand je fais "journal" j'ai "Utilisateurs > Frédéric".

Ok corrigé.

Une modification de l'usager ne touchant à rien s'enregistre/s'affiche avec "edit ()".

Ok, le journal ne se remplit que si form.has_changed().

L'affichage est chronologique et comme en plus la pagination n'a pas de lien vers la page finale, ce n'est pas pratique. (la pagination gadjo a un lien vers la page finale).

Je me suis dit que pour des noms informaticiens afficher les informations en sens inverse de la lecture serait perturbant. Mais je vais voir pour ajouter un lien pour dernière et première page.

En fait c'est plus bizarre que ça il est chronologique sur la page mais par défaut sur la dernière page et il faut faire "page précédente" pour voir ce qui arrivait avant.

Oui c'est voulu, antichronologique sur les pages mais chronologique sur la page, mais je vais inverse l'ordre sur la page.

Ça fait que dans le fil d'ariane "Utilisateurs > Frédéric > Journal" quand je suis sur une page avancée je clique sur "Journal" avec l'idée de revenir sur la première^Wdernière page et non ça m'amène sur /manage/journal/.

Je pense que c'est mieux d'arriver sur la dernière page mais peut-être est-ce plus cohérent d'avoir un ordre antichronologique dans les lignes aussi.

Il y a une tentative de quelque chose avec

[...]

mais je ne vois pas quoi, je ne vois pas de textarea.

Supprimé.

Pareil perdu js, il y a un

[...]

et rien qui appelle ça. (?)

Supprimé.

Un échec de connexion est enregistré avec username:fred mais pas attaché à l'utilisateur concerné, du coup on ne peut pas avoir l'info d'échec depuis le journal de l'usager il faut aller sur le journal global et chercher username:whatever et ça va retourner les événements pour tous les comptes commençant par whatever et on va dire que souvent c'est ok parce que l'username ça sera l'email; reste que pour exploiter ensuite les données du journal pour afficher à l'usager une liste des (tentatives de) connexions à son compte, il me semble qu'un truc manque.

On aimerait avoir les deux types d'échec je crois, le cas où le username est inconnu et le cas où c'est le mot de passe qui ne correspond pas. Pour l'instant je ne gère que le premier. Je vais voir pour gérer le second sans générer deux lignes dans le journal à chaque échec.

Correction des choses plus hautes poussées sur la branche.

#5

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Benjamin Dauvergne a écrit :

Il y une petite incohérence dans la forme des libellés d'événements, le "authorize single sign on" sonne bizarre avec son verbe là, plutôt que par exemple "authorization of single sign on".

Ok je vais utiliser une forme nominale pour tous.

C'est pas évident de trouver une forme nominale à chaque fois j'ai finalement adopté une forme verbale pour tous, sauf pour les échecs.

Au niveau des identifiants c'est l'absence d'underscores pour les premiers mots de celui-ci qui sonne bizarre :

[...]

Parce que le nom complet dépend de la classe parent, c'est 'manager.user.edit' le vrai nom et c'est comme ça qu'il est appelé dans le code.

Je vais virer les request.journal.record.truc.muche.machin(...) pour un plus évident request.journal.record('truc.muche.machin', ....)

#6

Mis à jour par Frédéric Péters il y a plus de 3 ans

C'est pas évident de trouver une forme nominale à chaque fois j'ai finalement adopté une forme verbale pour tous, sauf pour les échecs.

Je ne vois pas trop comment les traductions français vont être; tu imagines quoi ?

(aussi ça fait un peu bizarre d'avoir cet écart par rapport aux noms des classes.)

#7

Mis à jour par Frédéric Péters il y a plus de 3 ans

plus personne ne fait attention à la compatibilité py2, il faut désactiver py2 dans hobo c'est tout.

Il faut abandonner mandaye ou #46485.

#8

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

Il faut abandonner mandaye ou #46485.

Oui je regarderai dès que j'aurai poussé ça.

plus personne ne fait attention à la compatibilité py2, il faut désactiver py2 dans hobo c'est tout.

Séparons les choses: faire tourner les cibles authentic, passerelle, combo en python2 dans le tox.ini de hobo ne sert pas à maintenir mandaye en vie, on peut déjà commencer là.

#9

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

(aussi ça fait un peu bizarre d'avoir cet écart par rapport aux noms des classes.)

Je vais faire converger les noms des classes, elles n'ont pas d'importance.

#10

Mis à jour par Frédéric Péters il y a plus de 3 ans

Séparons les choses: faire tourner les cibles authentic, passerelle, combo en python2 dans le tox.ini de hobo ne sert pas à maintenir mandaye en vie, on peut déjà commencer là.

J'ai tout à fait inclus "il faudrait vraisemblablement retirer les tests py2/agent authentic d'abord" dans mon premier commentaire.

#11

Mis à jour par Frédéric Péters il y a plus de 3 ans

Voilà pour mon vrac de commentaires; je laisse la suite à d'autres.

Et je reviens à ça; je ne souhaite pas porter la relecture.

#12

Mis à jour par Paul Marillonnet il y a plus de 3 ans

Je vois l'idée, dans les modèles, d'une durée de rétention des événements liée à la définition du type d'événement (EventTypeDefinition.retention_days) mais en pratique utilisée nulle part dans les classes concrètes qui héritent de EventTypeDefinition.
Vu que ces définitions sont en dur dans le code, est-ce qu'on ne peut pas tout simplement laisser tomber cette idée de durée par type et laisser un setting global ?
D'ailleurs, en l'état on s'attendrait à ce que ce setting soit la durée maximale, i.e. une sorte de garde-fou contre la conservation indéfinie de ces infos, mais ce n'est pas le cas, c'est juste la durée par défaut quand rien n'est précisé.

Rien à voir, mais en relisant pour m'imprégner de l'esprit du patch, je pousse des petits trucs au dessus dans une branche à part.

#13

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Paul Marillonnet a écrit :

Je vois l'idée, dans les modèles, d'une durée de rétention des événements liée à la définition du type d'événement (EventTypeDefinition.retention_days) mais en pratique utilisée nulle part dans les classes concrètes qui héritent de EventTypeDefinition.

Je vais laisser ça non défini pour l'instant (mais la méthode est appelée toute seule par la commande cleanupauthentic lancé par cron, ça doit déjà marcher), j'ai l'idée bien en tête de ce que je veux faire à ce sujet mais pas eu le temps de finir en me concentrant sur définir tous les évènements important. L'idée final ce sera d'avoir des règles de rétention différentes selon les évènements, login_failure 1 semaine, login 1 mois, sso 1 mois, toutes les modifications au compte ou aux autorisations, tant que le compte existe ou qu'un objet DeletedUser pour cet user_id existe, etc...

Vu que ces définitions sont en dur dans le code, est-ce qu'on ne peut pas tout simplement laisser tomber cette idée de durée par type et laisser un setting global ?

Je ne sais pas encore quel quantité de log tout ça va générer mais j'imagine pas mal alors je vais peut-être laisser au moins la suppression des évènements login, login_failure, sso et logout qui seront les plus fréquents.

D'ailleurs, en l'état on s'attendrait à ce que ce setting soit la durée maximale, i.e. une sorte de garde-fou contre la conservation indéfinie de ces infos, mais ce n'est pas le cas, c'est juste la durée par défaut quand rien n'est précisé.

La règle pour moi c'est tant qu'un compte existe on doit garder tous les évènements concernant ces données (qui a changé le nom de madame michu en madame trucmuche l'année dernière), pour le reste oui. Pour les rôles, idem, tant qu'un rôle existe on gardera tout.

Rien à voir, mais en relisant pour m'imprégner de l'esprit du patch, je pousse des petits trucs au dessus dans une branche à part.

Ok je jette un oeuil.

PS: je viens d'ajouter un test du cleanup sur la branche, ça fonctionne.

#14

Mis à jour par Paul Marillonnet il y a plus de 3 ans

Benjamin Dauvergne a écrit :

Je vais laisser ça non défini pour l'instant (mais la méthode est appelée toute seule par la commande cleanupauthentic lancé par cron, ça doit déjà marcher), j'ai l'idée bien en tête de ce que je veux faire à ce sujet mais pas eu le temps de finir en me concentrant sur définir tous les évènements important. L'idée final ce sera d'avoir des règles de rétention différentes selon les évènements, login_failure 1 semaine, login 1 mois, sso 1 mois, toutes les modifications au compte ou aux autorisations, tant que le compte existe ou qu'un objet DeletedUser pour cet user_id existe, etc...

Oui je comprends l'idée mais je trouve ça étrange que ces règles soient en dur dans le code. J'imagine des déploiements où ces règles auraient intérêt à varier, pas possible en l'état.

Je ne sais pas encore quel quantité de log tout ça va générer mais j'imagine pas mal alors je vais peut-être laisser au moins la suppression des évènements login, login_failure, sso et logout qui seront les plus fréquents.

Ok.

Ok je jette un oeuil.

PS: je viens d'ajouter un test du cleanup sur la branche, ça fonctionne.

Ok, vu tes modifs passer. J'ai re-relu et me suis un peu perdu dans le code de pagination. Je vais tester en live mais c'est quand même un gros bout de code et si un·e troisième relecteur-rice veut/peut donner son avis ça serait bien.

#15

Mis à jour par Frédéric Péters il y a plus de 3 ans

Au cas où passée inaperçue, je reviens avec :

Je ne vois pas trop comment les traductions français vont être; tu imagines quoi ?

Et j'interroge le "mais je rajouterais bien avant de pousser" du début, s'agit-il de points à attendre ?

#16

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

Au cas où passée inaperçue, je reviens avec :

Je ne vois pas trop comment les traductions français vont être; tu imagines quoi ?

Aucune idée, j'ai jamais vu d'application avec genre de logs.

Et j'interroge le "mais je rajouterais bien avant de pousser" du début, s'agit-il de points à attendre ?

Les deux premiers sont en fait des questions à l'équipe :
  • veut-on un setting pour désactiver ça le temps que ça se stabilise au niveau IHM ?
  • a-t-on une icône ? (là j'ai mis class=icon-journal qui ne correspond à rien pour l'instant) pour l'icône en page d'accueil du BO
Ça finalement je ne ferai pas là, je vais ouvrir un ticket.
  • éventuellement une vue de journal pour les rôles en général (sur /manage/roles/journal/) qui montrerait l'ensemble des actions sur les rôles

Il me reste à obtenir un login_failure qui fonctionne aussi pour les erreurs de mot de passe et ce sera bon pour moi (en dehors de la définition des messages et de leur traduction).

#17

Mis à jour par Frédéric Péters il y a plus de 3 ans

Aucune idée, j'ai jamais vu d'application avec genre de logs.

J'insisterai pour que ça ne soit pas poussé avant que quelqu'un n'ait fait le pas de s'engager pour la traduction.

#18

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

Aucune idée, j'ai jamais vu d'application avec genre de logs.

J'insisterai pour que ça ne soit pas poussé avant que quelqu'un n'ait fait le pas de s'engager pour la traduction.

Si c'est faire les traductions des chaînes actuelles, je peux le faire, ou en tout cas proposer quelque chose dans la foulée, si c'est déterminé si les chaînes actuelles corresponde à quelque chose d'intelligible et ergonomique en anglais et en français, je veux bien des conseils.

#19

Mis à jour par Frédéric Péters il y a plus de 3 ans

proposer quelque chose

Ça serait déjà une réponse à mon "Je ne vois pas trop comment les traductions français vont être; tu imagines quoi ?".

#20

Mis à jour par Emmanuel Cazenave il y a plus de 3 ans

Pour essayer de faire avancer le schmilblick (et suite à un message de de Benjamin sur jabber), une proposition avec des formes nominales, avec les trads posées dans le patch à titre indicatif pour faciliter la discussion.

  • force_password_change et donotforce_password_change , je sèche, en anglais comme en français
  • pas particulièrement content des formulation sur les rôles, genre administration user removal : suppression d'un utilisateur administrateur
  • il y a des formulations qui se retrouvent coté manager et utilisateur, genre "changement de mot de passe", peut-être à différencier ? ou pas parce que la différence se verra par autre chose ? (je n'ai pas fait tourner tout ça en local)
#21

Mis à jour par Mikaël Ates (de retour le 29 avril) il y a plus de 3 ans

Plusieurs occurrences de "adminstrateur".

force_password_change : force password change at next login / demande de changement de mot de passe à la prochaine connexion

donotforce_password_change : do not force password change at next login / suppression de la demande de changement de mot de passe à la prochaine connexion

#22

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Je vais commencer par faire un modification à l'IHM, le type de l'évènement était prévu pour rentrer dans une colonne, avec des libellés aussi longs ça ne rentrera jamais, je vais plutôt mettre ça en message, avec éventuellement la méthode get_message() surchargée si la création du message est plus compliquée.

#23

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

branche à jour et finalisé (des tests, les labels sont bons pour moi).

#24

Mis à jour par Frédéric Péters il y a plus de 3 ans

À quel moment sert class Array(Expression): ? (c'est une classe sur laquelle les tests ne passent pas d'après le coverage)

#25

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Frédéric Péters a écrit :

À quel moment sert class Array(Expression): ? (c'est une classe sur laquelle les tests ne passent pas d'après le coverage)

Je vais le virer; la raison de son existence est un peu longur à expliquer, je préviens :
  • j'ai voulu intégrer find_duplicates() (qui renvoie un QuerySet d'User) à la recherche sur le journal (parce que vu que ça rentre pas cette semaine je continue sur ma lancée, et puis je voulais si j'arrivais à obtenir le journal d'une liste d'objets)
  • j'avais implémenté le champ "reference_ids" des références entre une ligne du journal et des objets comme un tableau (postgres) de tableaux à deux entrée (content_type.pk, instance.pk)
  • pour requêter ça il faut arriver à passer un truc genre WHERE references_ids && ARRRAY(SELECT array[5,user.id] FROM custom_user_user WHERE....), la fonction ARRAY n'étant pas définie dans l'ORM Django je l'ai ajouté (ça s'utilise comme ça ArraySubquery(qs))...) et pour constuire la valeur array[5, user.id] pareil ça n'existait pas non plus d'où Array(Expression) qui permet de construire qs.values_list(Array(Value(5), F('id')), flat=True);
  • et là je me suis aperçu que overlap et contains sur les champs ArrayField ne fonctionnent pas du tout comme je l'espérais. En fait les tableaux sont aplatis, postgres ne sait pas vraiment gérer les tableaux multi-dimensionels, il fait semblant, {{{1,x}}}} && {1} est toujours vrai quelque soit x quelque soit la profondeur du nesting, du point de vue de postgres tout est aplati pour les opérateurs que soit en contenance ou en intersection;
  • donc retour en arrière, c'est désormais un tableau de bigint (64bits) et je construis les références via content_type.id << 32 + instance.pk si on a moins de 2 milliards d'objets ça devrait tenir (parce que instance.pk est int32_t pas un uint32_t par défaut), et donc plus besoin de Array uniquement de ArraySubquery.

Suite à ça j'ai oublié de nettoyer Array(Expression) je m'empresse de le faire.

PS: j'ai mélangé ArraySubquery/Array, j'ai corrigé.

#26

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Statut changé de Solution proposée à En cours

Je remet à en cours mais ça me fait toujours plaisir si des gens jettent un oeuil ou font tourner la branche.

#27

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

#28

Mis à jour par Frédéric Péters il y a plus de 3 ans

Le retrait d'un rôle à un utilisateur ne fonctionne plus. (j'ai fait un bisect pour voir mais ça m'a amené au commit initial).

#30

Mis à jour par Valentin Deniaud il y a plus de 3 ans

Mon train de remarques :
  • J'ai un « ProgrammingError at /manage/journal/ more than one row returned by a subquery used as an expression » quand je recherche « nom prénom » et que deux utilisateurs existent avec le même nom et prénom.
  • La pagination me perd complètement :
    • Les boutons « First page » et « Last page » semblent tout bonnement inversés.
    • Si j'ai 21 évènements, que je vais sur le journal, j'en vois 20. Si je clique sur « Previous page » (qui devrait aussi certainement s'appeler « Next page »), j'en vois toujours 20 (le 21ème étant bien là et le premier non), mais je devrais n'en voir qu'un.
  • La suppression d'un rôle n'est pas loggée.

Frédéric Péters a écrit :

Le retrait d'un rôle à un utilisateur ne fonctionne plus. (j'ai fait un bisect pour voir mais ça m'a amené au commit initial).

Je constate ça aussi sur /manage/users/xxx/roles/, le retrait fonctionne mais la case se recoche, chelou.

Dans le genre moins important :
  • Quand on crée un utilisateur on a une ligne « password reset request with email » dans le journal, en plus de celle qui indique la création, c'est moyen.
  • Vraiment pas fan des clean_year = clean_integer_value('year') dans JournalForm, le ratio économie de ligne/lisibilité/ésotérisme n'est pas favorable par rapport à un
      def clean_year(value):
           return self._clean_integer_value(value)
    
  • Faire un tour de pylint sur les nouveaux fichiers, il y a des imports inutilisés, des erreurs d'indentation et des espaces en trop autour d'opérateurs, notamment.
  • Sur l'aide de JournalForm, la phrase « to find all events related to users whose email address starts with » doit être revue.
  • Il y a un FIXME dans get_attributes_label, on sait comment ça finit les FIXME, je pense qu'il faut soit le fixer soit ne pas le mettre (et faire un ticket).
  • Ça rendrait la relecture nettement moins indigeste d'avoir un truc un peu découpé genre 0001: journal 0002: pagination 0003: recherche.
#31

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Rebasage et découpage en 3 commits :
  • application de base authentic2.apps.journal
  • intégration dans a2
  • intégration dans le backoffice
#32

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Branche à jour.

Valentin Deniaud a écrit :

Mon train de remarques :
  • J'ai un « ProgrammingError at /manage/journal/ more than one row returned by a subquery used as an expression » quand je recherche « nom prénom » et que deux utilisateurs existent avec le même nom et prénom.

Corrigé.

  • La pagination me perd complètement :
    • Les boutons « First page » et « Last page » semblent tout bonnement inversés.
    • Si j'ai 21 évènements, que je vais sur le journal, j'en vois 20. Si je clique sur « Previous page » (qui devrait aussi certainement s'appeler « Next page »), j'en vois toujours 20 (le 21ème étant bien là et le premier non), mais je devrais n'en voir qu'un.

Le but c'est de ne jamais avoir de page quasi vide; si je parle d'évènements plutôt que de page ça t'irait ("Previous events / Next events / First events / Last events") cr là ça devient logique.

  • La suppression d'un rôle n'est pas loggée.

Corrigé, il y avait un bug plus profond sur la vue (form_valid() / delete()) corrigé aussi.

Frédéric Péters a écrit :

Le retrait d'un rôle à un utilisateur ne fonctionne plus. (j'ai fait un bisect pour voir mais ça m'a amené au commit initial).

Je constate ça aussi sur /manage/users/xxx/roles/, le retrait fonctionne mais la case se recoche, chelou.

Je regarde juste ça maintenant.

Dans le genre moins important :
  • Quand on crée un utilisateur on a une ligne « password reset request with email » dans le journal, en plus de celle qui indique la création, c'est moyen.

Si tu ne décoches pas "Envoyer un courriel à l’utilisateur pour qu’il choisisse son mot de passe : (optional)" ça parait normal.

  • Vraiment pas fan des clean_year = clean_integer_value('year') dans JournalForm, le ratio économie de ligne/lisibilité/ésotérisme n'est pas favorable par rapport à un

Ok simplifié.

  • Faire un tour de pylint sur les nouveaux fichiers, il y a des imports inutilisés, des erreurs d'indentation et des espaces en trop autour d'opérateurs, notamment.

J'air viré 3 imports inutilisés et repassé un coup de black.

C'est à dire ?

  • Il y a un FIXME dans get_attributes_label, on sait comment ça finit les FIXME, je pense qu'il faut soit le fixer soit ne pas le mettre (et faire un ticket).

Viré, c'est pas un bug, juste un souci de performance non mesuré.

#33

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Benjamin Dauvergne a écrit :

Frédéric Péters a écrit :

Le retrait d'un rôle à un utilisateur ne fonctionne plus. (j'ai fait un bisect pour voir mais ça m'a amené au commit initial).

Je constate ça aussi sur /manage/users/xxx/roles/, le retrait fonctionne mais la case se recoche, chelou.

Je regarde juste ça maintenant.

Copier/coller un peu rapide, c'est corrigé.

#34

Mis à jour par Valentin Deniaud il y a plus de 3 ans

Benjamin Dauvergne a écrit :

Valentin Deniaud a écrit :

  • La pagination me perd complètement :
    • Les boutons « First page » et « Last page » semblent tout bonnement inversés.
    • Si j'ai 21 évènements, que je vais sur le journal, j'en vois 20. Si je clique sur « Previous page » (qui devrait aussi certainement s'appeler « Next page »), j'en vois toujours 20 (le 21ème étant bien là et le premier non), mais je devrais n'en voir qu'un.

Le but c'est de ne jamais avoir de page quasi vide;

Pourquoi ? Tout le monde est habitué au fait que la dernière page soit quasi vide, c'est un peu ce qui la caractérise.

si je parle d'évènements plutôt que de page ça t'irait ("Previous events / Next events / First events / Last events") cr là ça devient logique.

L'ordre des boutons c'est plutôt ("First events / Previous events / Next events / Last events"), correspondant en ascii art à << < > >>, ça me semble être un standard hégémonique.

Bref gros bémol du bouzin pour moi cette histoire de pagination, le code est imbitable comme l'a noté Paul au dessus, je ne vois pas pourquoi il ne serait pas le même que celui de l'affichage des journaux/jobs d'un connecteur dans passerelle. Quel avantage a cette pagination par curseur ?

C'est à dire ?

Ben, si tu tapes « » c'est pour chercher l'adresse exacte, pas espérer trouver « », donc la phrase ne correspond pas à un cas d'usage réel, elle m'avait l'air d'être un copier/coller rapide de la ligne pour username, qui elle a du sens.

#35

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

Valentin Deniaud a écrit :

Bref gros bémol du bouzin pour moi cette histoire de pagination, le code est imbitable comme l'a noté Paul au dessus, je ne vois pas pourquoi il ne serait pas le même que celui de l'affichage des journaux/jobs d'un connecteur dans passerelle. Quel avantage a cette pagination par curseur ?

La stabilité de la navigation alors que des évènements arrivent en permanence et la possibilité de pointer une page de façon unique pour les rapports de bug en copiant l'URL. On a juste pas autant de logs dans passerelle.

Ben, si tu tapes « » c'est pour chercher l'adresse exacte, pas espérer trouver « », donc la phrase ne correspond pas à un cas d'usage réel, elle m'avait l'air d'être un copier/coller rapide de la ligne pour username, qui elle a du sens.

C'est mieux quand tu expliques, je vais changer.

Si il n'y pas de point bloquant j'aimerai bien pousser demain, histoire d'avoir le temps de traduire.

#36

Mis à jour par Valentin Deniaud il y a plus de 3 ans

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

Mis à jour par Benjamin Dauvergne il y a plus de 3 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 8487d33cff4c18211056f56dbdc58b67daa27691
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue Oct 13 19:08:43 2020 +0200

    misc: integration of journal in manager (#47155)

commit 1cc04e3ad7ad62b07861e9f992b84a86ef88a839
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue Oct 13 19:08:00 2020 +0200

    misc: integration of journal authentic views (#47155)

commit 9a1631b18a7511e909ff6802e2d321e37e652b02
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Sun Aug 23 02:28:25 2020 +0200

    misc: add journal application (#47155)
#39

Mis à jour par Frédéric Péters il y a plus de 3 ans

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

Formats disponibles : Atom PDF