Bug #68390
statistiques : plus possible de filtrer par service
0%
Description
Du moins depuis février.
#64853 a corrigé seulement un bout du problème (et ce ticket est pas sympa parce que pas de lien vers le ticket qui introduit le bug et patch qui mélange mille choses).
Fichiers
Révisions associées
journal: remove unused service parameter (#68390)
journal: log references to the base Service instance (#68390)
Historique
Mis à jour par Benjamin Dauvergne il y a plus d'un an
Valentin Deniaud a écrit :
Du moins depuis février.
#64853 a corrigé seulement un bout du problème (et ce ticket est pas sympa parce que pas de lien vers le ticket qui introduit le bug et patch qui mélange mille choses).
Je ne sais pas quel ticket introduit le bug, tu peux le rajouter si tu sais. Par ailleurs si tu sais quel bout du problème il reste à corriger tu peux l'écrire aussi ici. Ce ticket est encore plus cryptique pour l'instant.
Mis à jour par Valentin Deniaud il y a plus d'un an
Benjamin Dauvergne a écrit :
Ce ticket est encore plus cryptique pour l'instant.
Effectivement la description du bug ici est succinte mais le ticket lié est limpide.
Par ailleurs si tu sais quel bout du problème il reste à corriger tu peux l'écrire aussi ici.
Le problème c'est que « content_type_id peut parfois être l'id du modèle ContentType pour OIDCClient ou LibertyProvider », le ticket que je pointe a corrigé l'endroit où on groupe par service, il reste à corriger l'endroit où on filtre par service.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
Valentin Deniaud a écrit :
Par ailleurs si tu sais quel bout du problème il reste à corriger tu peux l'écrire aussi ici.
Le problème c'est que « content_type_id peut parfois être l'id du modèle ContentType pour OIDCClient ou LibertyProvider », le ticket que je pointe a corrigé l'endroit où on groupe par service, il reste à corriger l'endroit où on filtre par service.
C'est normal il est ouvert suite à un problème concernant l'agrégation par service pas le filtrage, il n'a effectivement résolu que ça et je ne vois pas en quoi il a cassé le filtrage ou alors le filtrage était déjà cassé ou bien je ne comprends pas.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Fichier 0001-statistics-lookup-service-also-with-their-subclass-r.patch 0001-statistics-lookup-service-also-with-their-subclass-r.patch ajouté
- Tracker changé de Bug à Development
- Statut changé de Nouveau à Solution proposée
- Patch proposed changé de Non à Oui
J'ai trouvé la raison du changement en février, la façon dont l'objet service est journalisé a changé à ce moment, de la classe authentic2.models.Service on est passé au sous-classes des différents modules (OIDCClient, LibertyProvider) ce qui a provoqué cette cassure. Comme on a désormais un mélange de deux types de référence j'ai préféré ajouté la recherche des deux ids; j'aurai pu faire une migration mais ça fait beaucoup de lignes à réécrire pour un web-service peu utilisé.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Fichier 0001-statistics-lookup-service-also-with-their-subclass-r.patch 0001-statistics-lookup-service-also-with-their-subclass-r.patch ajouté
Prise en compte du cas où l'OU ne contient aucun service.
Mis à jour par Valentin Deniaud il y a plus d'un an
(c'est rouge)
Benjamin Dauvergne a écrit :
j'aurai pu faire une migration mais ça fait beaucoup de lignes à réécrire pour un web-service peu utilisé
Dommage, là ça complexifie encore un peu plus un code qui est déjà illisible (je peux parler c'est moi qui l'ait écrit).
Comme on a désormais un mélange de deux types de référence j'ai préféré ajouté la recherche des deux ids
Du coup ça mérite peut-être l'ajout d'un commentaire à l'endroit où on récupère ces deux ids ?
return functools.reduce(
operator.or_,
(cls._which_references_query(ref) for ref in instance_or_model_class_or_queryset),
)
C'est pas équivalent à
return any(cls._which_references_query(ref) for ref in instance_or_model_class_or_queryset) ?
Je découvre aussi la dépendance à django-model-utils, si j'avais su je me serais pas embêté à réinventer tout ça dans apps/authenticators/query.py.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Statut changé de Solution proposée à En cours
Valentin Deniaud a écrit :
(c'est rouge)
Oui, j'étais perdu sur lasso.
Benjamin Dauvergne a écrit :
j'aurai pu faire une migration mais ça fait beaucoup de lignes à réécrire pour un web-service peu utilisé
Dommage, là ça complexifie encore un peu plus un code qui est déjà illisible (je peux parler c'est moi qui l'ait écrit).
Ouais, je vais mettre des comentaires mais dans l'immédiat je préfère ne pas toucher aux données.
Comme on a désormais un mélange de deux types de référence j'ai préféré ajouté la recherche des deux ids
Du coup ça mérite peut-être l'ajout d'un commentaire à l'endroit où on récupère ces deux ids ?
Ouaip.
[...]
C'est pas équivalent àreturn any(cls._which_references_query(ref) for ref in instance_or_model_class_or_queryset)?
C'est pas le même opérateur (https://docs.python.org/3/library/operator.html#operator.__or__, "or" != "|").
Je découvre aussi la dépendance à django-model-utils, si j'avais su je me serais pas embêté à réinventer tout ça dans apps/authenticators/query.py.
Je découvre ce query.py :)
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Statut changé de En cours à Solution validée
Voilà, j'ai quand même restauré le comportement précédent (j'enregistre la référence vers le ContentType de authentic2.models.Service); on pourra voir pour ne pas aggréger via service_name dans un autre ticket.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Statut changé de Solution validée à Solution proposée
Mis à jour par Valentin Deniaud il y a plus d'un an
- Statut changé de Solution proposée à Solution validée
Looks good
Benjamin Dauvergne a écrit :
Je découvre aussi la dépendance à django-model-utils, si j'avais su je me serais pas embêté à réinventer tout ça dans apps/authenticators/query.py.
Je découvre ce query.py :)
Je pense que tu avais aussi oublié cette dépendance quand tu as écrit #53902#note-11.
Mis à jour par Benjamin Dauvergne il y a plus d'un an
- Statut changé de Solution validée à Résolu (à déployer)
commit 467bb3c7dfac7f109c63ee243598faf9481f1f84
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Tue Sep 13 11:20:48 2022 +0200
journal: log references to the base Service instance (#68390)
commit 1016ca0647ae0aec796cd575ae0288f0e0aff0bd
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Tue Sep 13 11:19:00 2022 +0200
journal: remove unused service parameter (#68390)
commit 21249f84bb8b5e6190060570d690a12194f6cd26
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date: Mon Sep 12 22:36:54 2022 +0200
statistics: lookup service also with their subclass reference_id (#68390)Mis à jour par Transition automatique il y a plus d'un an
- Statut changé de Résolu (à déployer) à Solution déployée
statistics: lookup service also with their subclass reference_id (#68390)