Bug #31222
Lien invalide sur la liste des rôles d'un utilisateur
0%
Description
Par exemple sur /manage/users/<user_id>/roles/ si on clique sur une ligne (vraiment la ligne, pas le nom du rôle, ie le <tr>, pas le <td>) on arrive sur /manage/users/<user_id>/roles/<role_id> qui n'existe pas. On devrait tomber sur /manage/roles/<role_id>.
En fait il y a juste une typo dans le code.
Fichiers
Historique
Mis à jour par Valentin Deniaud il y a environ 5 ans
- Fichier 0001-fix-invalid-url-on-clickable-row.patch ajouté
- Tracker changé de Support à Bug
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
Mis à jour par Paul Marillonnet il y a environ 5 ans
Je crois que tu peux t'inspirer de #31092 pour ajouter une ligne de test, aussi.
Mis à jour par Benjamin Dauvergne il y a environ 5 ans
Il faut peut-être faire le tour de tous les templates de tableau à l'occasion puisque c'est le même souci que #31092 et pas une typo.
Mis à jour par Valentin Deniaud il y a environ 5 ans
- Fichier
0001-fix-invalid-url-on-clickable-row.patchsupprimé
Mis à jour par Valentin Deniaud il y a environ 5 ans
- Fichier 0001-manager-access-url_name-through-table-object-31222.patch 0001-manager-access-url_name-through-table-object-31222.patch ajouté
J'ai ajouté un test et modifié le message de commit, puisque je n'avais pas compris d'où venait le bug initialement.
Mis à jour par Benjamin Dauvergne il y a environ 5 ans
Je vois active_role dans src/authentic2/manager/templates/authentic2/manager/role_members_table.html qui a le même problème je pense, liste des templates à regarder :
$ git grep -l block\ table -- \*.html src/authentic2/manager/templates/authentic2/manager/ous_table.html src/authentic2/manager/templates/authentic2/manager/role_children_table.html src/authentic2/manager/templates/authentic2/manager/role_members_table.html src/authentic2/manager/templates/authentic2/manager/role_permissions_table.html src/authentic2/manager/templates/authentic2/manager/service_roles_table.html src/authentic2/manager/templates/authentic2/manager/table.html src/authentic2/manager/templates/authentic2/manager/user_roles_table.html
Mis à jour par Valentin Deniaud il y a environ 5 ans
- Fichier 0001-manager-access-context-through-table-object-bis-3122.patch 0001-manager-access-context-through-table-object-bis-3122.patch ajouté
- Statut changé de Solution proposée à En cours
Bon j'ai passé un certain temps à comprendre comment était structuré le code et à découvrir la magie de django au niveau du contexte des templates.
Sauf si il y a une couche de sorcellerie que j'ai raté, le paramètre active_role que tu mentionnes n'est jamais utilisé (git grep active_role ne renvoie rien hors des templates, et j'ai l'impression qu'il a été retiré en 2015 par 25ad9166a1a0367c659e858346db7f9b8b7fa3f3).
À chaque fois, il est utilisé comme suit :
{% blocktrans with name=row.record.name role=active_role.name %}Do you really want to remove role "{{ name }}" from role "{{ object }}" ?{% endblocktrans %}Donc j'ai l'impression qu'il y a un genre de typo, par ex {{ object }} devrait être {{ role }} (mais à ce moment là ça ne marche plus, parce que active_role n'est jamais passé).
Bref, c'est cassé de toute façon avec la version de django-tables2 qui nous préoccupe, parce que object n'existe plus et il faut aller le chercher dans table.context.object.
Et pour une raison mystérieuse, table.context.object n'est pas accessible à l'intérieur du bloc blocktrans.
La solution que je propose est donc de faire :
{% blocktrans with name=row.record.name role=table.context.object.name %}Do you really want to remove role "{{ name }}" from role "{{ role }}" ?{% endblocktrans %}Pour l'instant j'ai tout mis dans un commit, je suis conscient qu'il faudrait en faire deux (suppression de active_role et correctif avec table.context) mais c'est assez laborieux à rebase comme tout est sur la même ligne. Dis moi si ça serait vraiment mieux, je le ferai.
Il y a le même genre d'incohérence dans service_roles_table.html.
À part ça, deux des fichiers que tu as listé ne sont plus utilisés, j'ai l'impression, et n'ont pas été touchés depuis 2015. Donc pour l'instant je n'y ai pas touché. Ces fichiers sont :
src/authentic2/manager/templates/authentic2/manager/ous_table.html src/authentic2/manager/templates/authentic2/manager/role_children_table.html
Leur abandon a l'air de bien dater, je peux m'occuper d'ouvrir un autre ticket et de les supprimer si besoin. Il y a potentiellement plus que ça à nettoyer, par exemple la vue inutilisée RoleChildrenView référence une template qui n'existe plus depuis 2016 (voir 76b070f482712e46ef711c1187766838a6240e6d par ex).
Dernier truc, est-ce qu'il faut ajouter des tests pour chaque modif dans les templates ?
Mis à jour par Valentin Deniaud il y a environ 5 ans
- Statut changé de En cours à Solution proposée
Mis à jour par Benjamin Dauvergne il y a environ 5 ans
- Statut changé de Solution proposée à En cours
Oui blocktrans ne permet pas d'utiliser des chemins {{ x.y.z }} parce qu'il transforme ça xxx {{ uuu }} en ça "xxx %(uuu)s" et l'interpolation classique ne supporte pas le déréférencement.
Ok avec la modification plutôt réécrire comme cela (et en un seul commit pas de soucis) :
{% blocktrans with name=row.record.name role=table.context.object.name %}Do you really want to remove role "{{ name }}" from role "{{ role }}" ?{% endblocktrans %}
car les rôles ont une méthode unicode et donc savent s'afficher dans un template, pas besoin de désigner leur attribut
name.
Tu peux virer les deux templates inusités dans un deuxième commit.
Mis à jour par Valentin Deniaud il y a environ 5 ans
- Statut changé de En cours à Solution proposée
Mis à jour par Valentin Deniaud il y a environ 5 ans
Mis à jour par Frédéric Péters il y a environ 5 ans
Je déconseille de mêler les traductions au code, ça rend les rebase/merge trop difficiles.
Mis à jour par Paul Marillonnet il y a environ 5 ans
Quitte à virer la vue de table, autant sortir la table associée aussi (RoleChildrenTable).
Mis à jour par Valentin Deniaud il y a environ 5 ans
- Fichier 0003-manager-remove-some-unused-code-31222.patch 0003-manager-remove-some-unused-code-31222.patch ajouté
- Fichier 0001-manager-access-context-through-table-object-bis-3122.patch 0001-manager-access-context-through-table-object-bis-3122.patch ajouté
- Fichier 0002-manager-update-translations-31222.patch 0002-manager-update-translations-31222.patch ajouté
Mis à jour par Benjamin Dauvergne il y a environ 5 ans
- Statut changé de Solution proposée à Solution validée
Ok.
Mis à jour par Valentin Deniaud il y a environ 5 ans
- Statut changé de Solution validée à Résolu (à déployer)
commit 14361e000bb8660498969eb00d9ddc8fccb82882
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu Mar 14 11:44:32 2019 +0100
manager: remove some unused code (#31222)
commit 26dd7e67b60d84026b09d8fd471937219e1ffb0f
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Thu Mar 14 12:02:57 2019 +0100
manager: update translations (#31222)
commit df8cc6dff595a634996c19da3230b1795fc018d5
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date: Fri Mar 8 13:56:25 2019 +0100
manager: access context through table object bis (#31222)
Related to #31092.
Mis à jour par Frédéric Péters il y a environ 5 ans
- Statut changé de Résolu (à déployer) à Solution déployée
manager: access context through table object bis (#31222)
Related to #31092.