Project

General

Profile

Bug #16474

/manage/: tri asciibétique des rôles dans les tableaux

Added by Frédéric Péters almost 7 years ago. Updated about 2 months ago.

Status:
Solution déployée
Priority:
Normal
Category:
-
Target version:
-
Start date:
24 May 2017
Due date:
% Done:

0%

Estimated time:
Patch proposed:
No
Planning:
No

Description

  • Gestionnaire emploi
  • Gestionnaire espace vert
  • Gestionnaire voirie
  • Gestionnaire état civil

Le rôle "gestionnaire état civil" devrait être listé plus haut.


Files


Related issues

Related to Authentic 2 - Bug #36672: Tri des rôlesFermé03 October 2019

Actions

Associated revisions

Revision f45440e0 (diff)
Added by Benjamin Dauvergne 3 months ago

manager: fix ordering in user's roles table (#16474)

A list was used instead of a queryset to allow django-tables2 to order
the table by the boolean value of the prefetched member attribute,
django-tables2 could not do it if the list was a queryset.
The side effect is that the sorting was done by Python instead of
PostgreSQL but Python does not respect unicode collation when doing so.

Member is now replaced by a subquery using the pair Exists/OuterRef and
sorting is now done by PostgreSQL directly.

History

#1

Updated by Paul Marillonnet almost 7 years ago

  • Status changed from Nouveau to En cours
#2

Updated by Paul Marillonnet almost 7 years ago

Avec le patch c'est mieux, pardon.

#3

Updated by Thomas Noël almost 7 years ago

Nan. Pas avec les slugs, qui sont invariables et ne suivent pas les noms des rôles s'ils changent.

#4

Updated by Paul Marillonnet almost 7 years ago

Ok oui je réalise que je n'avais pas compris la nature du problème.

J'ai l'impression que la fonction de base de données Cast aurait été utile ici, mais c'est du Django>=1.10.

#5

Updated by Benjamin Dauvergne almost 7 years ago

Ce n'est pas juste un problème de locale sur la base de donnée parce que normalement le tri est délégué tout bêtement à postgres.

#6

Updated by Thomas Noël almost 7 years ago

Sur le saas (on a regardé prod, test et dev) tout semble ok côté PostgreSQL :

postgres=# \l
                                             Liste des bases de données
          Nom           |     Propriétaire      | Encodage | Collationnement | Type caract. |    Droits d'accès     
------------------------+-----------------------+----------+-----------------+--------------+-----------------------
 authentic2_multitenant | authentic-multitenant | UTF8     | fr_FR.UTF-8     | fr_FR.UTF-8  | 
#7

Updated by Benjamin Dauvergne almost 7 years ago

On peut déjà me donner l'URL de l'authentic qui a ces rôles ?

#8

Updated by Frédéric Péters almost 7 years ago

  • Status changed from En cours to Rejeté

J'aurais misé deux euros sur la recette GNM et c'est bien là qu'il y a ces rôles mais le /manage/roles/ y affiche les rôles triés correctement. Je rejette et si je retombe dessus je réouvrirai sans zapper d'infos.

#9

Updated by Frédéric Péters over 6 years ago

  • Status changed from Rejeté to Nouveau

Retrouvé ça sur la page des rôles d'un utilisateur (a2-manager-user-roles) sur Tournai (sur le même site, tri correct sur a2-manager-roles).

J'ai créé un rôle "État civil" sur la plateforme de démo de dev et il est trié en dernier : https://authentic-demo.dev.entrouvert.org/manage/users/53/roles/

#10

Updated by Benjamin Dauvergne over 6 years ago

  • Patch proposed changed from Yes to No

Il semblerait que ce patch solutionne l'ordre mais je pense que j'avais une bonne raison quand j'ai forcé la conversion en liste.

diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py
index bedb52c..3bb0c50 100644
--- a/src/authentic2/manager/user_views.py
+++ b/src/authentic2/manager/user_views.py
@@ -379,12 +379,6 @@ class UserRolesView(HideOUColumnMixin, BaseSubTableView):
         else:
             return self.object.roles_and_parents()

-    def get_table_data(self):
-        qs = super(UserRolesView, self).get_table_data()
-        if self.is_ou_specified():
-            qs = list(qs)
-        return qs
-
     def dispatch(self, request, *args, **kwargs):
         return super(UserRolesView, self).dispatch(request, *args, **kwargs)

Il faut que je retrouve cette raison.

#11

Updated by Frédéric Péters over 4 years ago

#12

Updated by Benjamin Dauvergne over 4 years ago

  • Status changed from Nouveau to Résolu (à déployer)

J'ai retrouvé la raison du list(qs):

commit d6b2a1806874b5404c88600b0532c35882d23c28
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Tue Jan 19 10:24:31 2016 +0100

    allow ordering user's roles table by the member column (fixes #9662)

diff --git a/src/authentic2/manager/tables.py b/src/authentic2/manager/tables.py
index 46224161..17bb3200 100644
--- a/src/authentic2/manager/tables.py
+++ b/src/authentic2/manager/tables.py
@@ -101,7 +101,7 @@ class OuUserRolesTable(tables.Table):
         '''{% for rel in row.record.via %}{{ rel.child }} {% if not forloop.last %}, {% endif %}{% endfor %}''',
         verbose_name=_('Inherited from'), orderable=False)
     member = tables.TemplateColumn('''{% load i18n %}<input class="role-member{% if not row.record.member and row.record.via %} indeterminate{% endif %}" name='role-{{ row.
record.pk }}' type='checkbox' {% if row.record.member %}checked{% endif %} {% if not row.record.has_perm %}disabled title="{% trans "You are not authorized to manage this ro
le" %}"{% endif %}/>''',
-                                  verbose_name=_('Member'), orderable=False)
+                                  verbose_name=_('Member'), order_by=('member', 'via', 'name'))

     class Meta:
diff --git a/src/authentic2/manager/user_views.py b/src/authentic2/manager/user_views.py
index 6cd75e93..84efa357 100644
--- a/src/authentic2/manager/user_views.py
+++ b/src/authentic2/manager/user_views.py
@@ -291,6 +291,12 @@ class UserRolesView(HideOUColumnMixin, BaseSubTableView):
         else:
             return self.object.roles_and_parents()

+    def get_table_data(self):
+        qs = super(UserRolesView, self).get_table_data()
+        if self.is_ou_specified():
+            qs = list(qs)
+        return qs
+
     def dispatch(self, request, *args, **kwargs):
         return super(UserRolesView, self).dispatch(request, *args, **kwargs)

sachant que member est une colonne calculée :

    def get_table_queryset(self):
        if self.is_ou_specified():
            roles = self.object.roles.all()
            User = get_user_model()
            Role = get_role_model()
            RoleParenting = get_role_parenting_model()
            rp_qs = RoleParenting.objects.filter(child__in=roles)
            qs = Role.objects.all()
            qs = qs.prefetch_related(models.Prefetch(
                'child_relation', queryset=rp_qs, to_attr='via'))
            qs = qs.prefetch_related(models.Prefetch(
                'members', queryset=User.objects.filter(pk=self.object.pk),
                to_attr='member')) <- ICI

Si je ne convertis pas en liste, et donc qu'on passe un QuerySet à django-tables2 il tente systématiquement de faire un .order_by() et ça ne marche pas sur member parce que c'est une jointure.
1. vérifier que c'est toujours le cas (ça plante si on enlève liste(qs) et qu'on trie sur member
2. si oui, adapter le code, détecter si on trie sur member et alors utiliser list() sinon on laisse le queryset et/ou trouver un moyen compatible avec l'ORM de générer une colonne member triable dans le SELECT SQL.

#13

Updated by Frédéric Péters over 4 years ago

  • Status changed from Résolu (à déployer) to Nouveau
#14

Updated by Benjamin Dauvergne about 3 years ago

  • Assignee set to Paul Marillonnet
#15

Updated by Robot Gitea 3 months ago

  • Status changed from Nouveau to Solution proposée
  • Assignee changed from Paul Marillonnet to Benjamin Dauvergne

Benjamin Dauvergne (bdauvergne) a ouvert une pull request sur Gitea concernant cette demande :

#16

Updated by Robot Gitea 3 months ago

  • Status changed from Solution proposée to Solution validée

Paul Marillonnet (pmarillonnet) a approuvé une pull request sur Gitea concernant cette demande :

#17

Updated by Robot Gitea 2 months ago

  • Status changed from Solution validée to Résolu (à déployer)

Benjamin Dauvergne (bdauvergne) a mergé une pull request sur Gitea concernant cette demande :

#18

Updated by Transition automatique about 2 months ago

  • Status changed from Résolu (à déployer) to Solution déployée

Also available in: Atom PDF