Produits Entr'ouvert » Combo

Premier truc qui apparaît bloquant :

215         for wcs_slug, wcs_site in wcs_sites.items():

ie la pagination doit être faite entre plusieurs wcs. Pour faire propre on veut avoir le total des formulaires, ce qui permet d'afficher les boutons « 1, 2, 3, 4, etc » et « last page ».

1. Pour tous les sites, demander le total des formulaires (ajouter un endpoint/un paramètre à /api/user/forms
2. regarder quelle page est demandée, calculer, et faire une requête de la bonne taille au bon wcs, potentiellement à plusieurs pour remplir une page

Et en fait c'est 1. qui bloque : dans wcs la récupération des formulaires n'est pas une simple requête sql, où on aurait pu transformer un select en select count et garder des perfs correctes. Le select est suivi d'une cinquantaine de lignes de python pour déterminer la liste effective des formulaires à retourner, en fonction des permissions de l'utilisateur. Donc pour obtenir le total il faudrait select tout, filtrer tout, et faire len() du résultat, ce qui a des chances de nous faire retomber sur le problème qu'on souhaitait résoudre, rencontré dans #35023 (requête trop longue/trop coûteuse, quoique c'était peut-être juste la transmission des données par le réseau qui posait problème).

Peut-être que je passe à côté d'un truc, en attendant je continue sur le ticket en visant un semblant de pagination avec des boutons précédent/suivant/revenir au début, seuls possibles sans toucher à wcs.

Dans #35023 on parlait de plus de 4000 demandes en cours, on peut ignorer ça, rester à la limite qui a été posée de 100 demandes (par w.c.s.).

Évidemment c'était il y a trois ans et j'aurais du être plus complet sur ce que je pouvais avoir comme idée en tête à l'époque mais je dirais aujourd'hui qu'il s'agit ici davantage d'une question de présentation, qu'on peut très bien charger les 100 demandes dans l'html et avoir une pagination js.

une pagination js

Voilà qui est tout de suite moins sympa à coder. Dans #35403 d'il y a 2 mois tu semblais toujours avoir en tête de faire de la pagination côté serveur, j'ai quand même bien envie d'essayer de faire ça.

Attention, à prendre les choses par le côté serveur, tu vois les choses sur les API, ça peut amener à travailler dans w.c.s., pour remonter max de choses avant l'appel SQL, qu'il puisse arriver à être transformé en COUNT, ainsi il y aura moyen dans la cellule de définir de manière précise les appels à effectuer, puis il faudra caler ça sur des paramètres passés à la cellule, il n'y a actuellement pas de passage de paramètres, donc ajouter d'une manière ou d'une autre certains éléments de request.GET dans le contexte de rendu créé dans ajax_page_cell, puis faudra appeler ça et là ça voudra de toute façon dire javascript, limite davantage que dans l'option simple.

Mes 2 cents : il faudrait paginer par curseur (i.e. sans pouvoir dire le nombre de pages) et pas par page, c'est toujours plus simple et plus stable, le curseur serait la date, le slug wcs et l'id de la dernière demande de la page courante, tout ça trié lexicographiquement.

Étant donné un curseur on demande à chaque w.c.s les 100 dernières demandes plus vieille que ce curseur et on les trie lexicographiquement par date, slug, id. Si tout le monde en renvoie moins de 100 on donne un nombre de demandes, sinon on dit "beaucoup trop". Pour pouvoir paginer dans les deux sens il faut gérer un curseur before et after.

PS: remarque à ne prendre en compte que si on décide vraiment de paginer la totalité des demandes; dans le cas de la récupération de maximum n*100 demandes sur les n wcs puis pagination dans la page on peut l'ignorer.

Merci pour l'idée Benj. J'ai implémenté un truc incomplet par rapport à ce que tu as écrit, qui a l'air de marcher sur un wcs, pas encore testé sur plusieurs. Avec n le nombre de résultats à afficher par page, pour une page au pif, processus pour passer à la page suivante :

1. Faire le tour des wcs et demander n + 1 résultats plus vieux que le dernier résultat de la page courante
2. Agréger tous les résultats des wcs dans une liste, la trier sur la date
3. Tronquer à n résultats et les afficher

Le curseur pour passer à la page d'après est la date du dernier résultat après tronquage. Si on a pas eu n résultats, on sait qu'on est arrivé à la fin.

Ce qui me laisse avec les questions :

le curseur serait la date, le slug wcs et l'id

Pourquoi sur le slug et l'id ? Si l'idée c'est de dédupliquer des demandes qui auraient le même timestamp, le problème se pose en amont dans wcs au moment de la récupération des données... Mais de manière générale ajouter un timestamp précis à la milliseconde ça pourrait permettre d'ignorer le pb.

Si tout le monde en renvoie moins de 100 on donne un nombre de demandes, sinon on dit "beaucoup trop".

Pas compris ça.

Valentin Deniaud a écrit :

Ce qui me laisse avec les questions :

le curseur serait la date, le slug wcs et l'id

Pourquoi sur le slug et l'id ? Si l'idée c'est de dédupliquer des demandes qui auraient le même timestamp, le problème se pose en amont dans wcs au moment de la récupération des données... Mais de manière générale ajouter un timestamp précis à la milliseconde ça pourrait permettre d'ignorer le pb.

Il faut un ordre absolu entre les demandes pour pas en rater d'une page à l'autre, à la milliseconde ça limite le problème mais ça ne le supprime pas, mais oui ça peut être "good enough", ta solution m'a l'air suffisante on verra à l'usage.

Si tout le monde en renvoie moins de 100 on donne un nombre de demandes, sinon on dit "beaucoup trop".

Pas compris ça.

Je partais du postulat qu'on n'avait pas de requête pour faire un count(), mais si on en demande 100 à tout le monde et que tout le monde répond moins en fait on sait.

(sérieusement pour moi ce ticket ne vaut pas un patch de plus de vingt lignes) (je devrais être plus clair dès le début et dire qu'au-delà, ça sera très certainement rejeté).

Je ne vois pas trop comment ça marche; en local j'obtiens une cellule vide, il y a :

            wcs_sites = {'paginated': {'data': all_forms}}

mais j'imagine que WcsCurrentFormsCell::get_cell_extra_context devrait être modifié pour utiliser ça, ce qui n'est pas le cas.

Aussi, avant ça, pour l'affichage initial, quand il n'y a ni before ni after, toutes les demandes seront récupérées, ce qu'on voudrait justement éviter (ça va ramener les soucis qu'on avait avant le limit=100 de #35407).

De manière plus lointaine, quand j'écrivais ça :

puis il faudra caler ça sur des paramètres passés à la cellule, il n'y a actuellement pas de passage de paramètres, donc ajouter d'une manière ou d'une autre certains éléments de request.GET dans le contexte de rendu créé dans ajax_page_cell

C'était parce que je ne suis pas vraiment pour un accès à request.GET dans le code des cellules. (mais en réfléchissant davantage peut-être j'arriverais à me convaincre que ce n'est pas un vrai problème).

_~~

Mais même si je pense que la direction prise ici peut aboutir, je reste sur l'idée d'une version courte avec juste du javascript.

Je ne vois pas trop comment ça marche; en local j'obtiens une cellule vide

Au lieu d'avoir wcs_sites = {'slug1': {'data': data, 'extra_key': ...}, 'slug2': {'data': data, 'extra_key': ...}}, wcs_sites = {'paginated': {'data': all_forms}} écrabouille tout pour créer un wcs fictif au slug 'paginated' qui contient uniquement les données qu'on veut afficher. C'est sûrement pas terrible et ça n'a pas vocation à rester, mais pour mon PoC c'était le plus rapide. Je ne sais pas pourquoi ça ne marche pas chez toi mais des tests étant rouges pour l'instant ça ne m'étonne pas (je teste avec une cellule « Demandes de l'usager » qui inclut les demandes en cours et les demandes terminées).

quand il n'y a ni before ni after, toutes les demandes seront récupérées

Bien vu, effectivement il y a une indentation en trop.

je ne suis pas vraiment pour un accès à request.GET dans le code des cellules

Dac, si c'est ça le problème je vois moins comment m'en sortir.

je reste sur l'idée d'une version courte avec juste du javascript

Cette solution ne m'attire pas parce qu'avoir une limite arbitraire en dur dans le code ne me semble pas très joli, mais surtout parce que l'approche actuelle me permet de me familiariser avec le fonctionnement de combo/wcs, alors que clairement je n'apprendrais rien d'utile en pondant 20 lignes de js.

Dac, si c'est ça le problème je vois moins comment m'en sortir.

Mon idée était que la cellule déclare les paramètres qui l'intéressent (dans un attribut de la classe) et que cette info soit utilisée dans ajax_page_cell pour alimenter le contexte.

Cette solution ne m'attire pas parce qu'avoir une limite arbitraire en dur dans le code ne me semble pas très joli, mais surtout parce que l'approche actuelle me permet de me familiariser avec le fonctionnement de combo/wcs, alors que clairement je n'apprendrais rien d'utile en pondant 20 lignes de js.

En rester aux solutions simples est un apprentissage utile. Avoir quelque chose d'isolé et compréhensible facilitera la maintenance, pourra s'appliquer à d'autres listes affichées par Combo, qu'elles viennent d'autres endpoints de w.c.s. ou d'autres applications (comme la liste des documents récents tirée de Fargo).

Il manque dans le patch attaché le fichier combo/pagination.html, et tu peux envoyer ça vers une branche ? (j'étais perturbé par le jenkins rouge mais il ne correspond pas à ce dernier code).