Simplifie massivement les statistiques K-Fêt + étend la période de stats #715
No reviewers
Labels
No labels
devtype -- backend
devtype -- docs
devtype -- frontend
devtype -- user interface
difficulty -- easy
difficulty -- hard
difficulty -- normal
Doing
domain -- bda
domain -- bds
domain -- cof
domain -- core
domain -- kfet
Good first issue
priority -- high
priority -- low
priority -- medium
priority -- staff-wanted
status -- development
status -- discussion
status -- need review
status -- production
status -- ready to merge
status -- todo
To Do
type -- bug
type -- hygiene
type -- improvement
type -- new feature
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: DGNum/gestioCOF#715
Loading…
Reference in a new issue
No description provided.
Delete branch "Aufinal/simplify_stats"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Vire pas mal de code mort/rework le code redondant des statistiques K-Psul. En side effect, on peut avoir des stats depuis la création du compte/de l'article.
Fixes #257, #244
added 2 commits
Compare with previous version
C'est pas clair ce que ça veut dire pour moi.
Et puis sur toutes les subdivisions ou sur la première ?
Les subdivisions après la première sont générées par (en gros)
subdivision[k] = subdivision[0] + k * timedelta
; donc standardiser le début de la première subdivision se répercute sur toutes.C'est assez fragile tout ça. On récupère des données sans une url dans les nettoyer, c'est pas une bonne pratique.
Entre autres je peux passer
?n_steps=toto
dans l'url pour faire crasher la conversion enint
, ou pire :n_steps=-1
qui fait une boucle infinie dans_gen_datetimes
et donc un DOS facile.Évidemment on aurait dû voir tout ça quand on a mis en prod les stats la première fois…
C'est du même acabit que #255, et ça peut aller dans l'issue si tu veux, mais faudrait vraiment qu'on s'en occupe un jour…
Celui là je ne l'aime pas du tout non plus : je serais en faveur de hardcoder les noms des échelles valides quelque part dans un dictionnaire plutôt que faire cette résolution dynamique. En plus le
name
vient d'une url non nettoyéeÇa n'explique pas ce que veux dire "standardisé", même en ayant lu le code je ne vois pas.
Si ça veut dire "on appelle
get_chunk_start
pour définir la première subdivision" alors il faut le mettre et ça deviendra clair.J'ai réussi à avoir 3 erreurs 500 différentes (mais pas de DOS), donc je vais y jeter un oeil...
Modulo les remarques de sécurité sur les paramètres GET (qui posaient déjà problème avant) ça me va, et c'est beaucoup plus clair qu'avant ! Merci.
Si tu ne veux pas gérer le souci des urls maintenant on peut faire une issue et la réunir avec les autres issues du genre sous une milestone "sécu". À gérer rapidement
changed this line in version 3 of the diff
added 1 commit
Compare with previous version
Après check, ça a l'air plus complexe que prévu à régler, et pas si dangereux que ça (on peut provoquer des erreurs 500, mais pas grand chose de plus), donc je suis pour merge avec #255.
En passant, pourquoi ce truc est asynchrone ? Une solution simple pour nettoyer des paramètres GET est de ne pas faire de requête GET...
added 51 commits
master
6767ba8e
- Rajoute de la doc partout78ad4402
- Plus de timezones26bcd729
- Supprime le code mort ou redondantef35f45a
- Fusionne deux fonctions `chunkify`48ad5cd1
- Misc cleanupc66fb7eb
- Simplify statistic.js97cb9d1f
- Rework `stats_manifest`f10d6d1a
- Bugfixc9dad946
- Fix tests61e4ad97
- Better docstringc9136dbc
- CHANGELOGCompare with previous version
Tout à fait d'accord !
resolved all discussions
Allez hop
mentioned in commit
5a9ea4234e
merged