Simplifie massivement les statistiques K-Fêt + étend la période de stats

lstephan commented

2020-03-09 16:24:02 +01:00

(Migrated from git.eleves.ens.fr)

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

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

lstephan commented

2020-03-09 17:01:17 +01:00

(Migrated from git.eleves.ens.fr)

added 2 commits

b16a96ea - Bugfix
05a5fec6 - Fix tests

Compare with previous version

added 2 commits <ul><li>b16a96ea - Bugfix</li><li>05a5fec6 - Fix tests</li></ul> [Compare with previous version](/klub-dev-ens/gestioCOF/merge_requests/411/diffs?diff_id=1527&start_sha=5d0f21399ccccac6f9ed9bcb0f362a6cd52a5565)

mpepin commented

2020-05-07 18:19:28 +02:00

(Migrated from git.eleves.ens.fr)

C'est pas clair ce que ça veut dire pour moi.

Et puis sur toutes les subdivisions ou sur la première ?

C'est pas clair ce que ça veut dire pour moi. Et puis sur toutes les subdivisions ou sur la première ?

lstephan commented

2020-05-07 18:29:25 +02:00

(Migrated from git.eleves.ens.fr)

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.

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.

mpepin commented

2020-05-07 18:33:33 +02:00

(Migrated from git.eleves.ens.fr)

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 en int, 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…

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 en `int`, 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…

mpepin commented

2020-05-07 18:35:28 +02:00

(Migrated from git.eleves.ens.fr)

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

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

mpepin commented

2020-05-07 18:44:03 +02:00

(Migrated from git.eleves.ens.fr)

Ç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.

Ç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.

lstephan commented

2020-05-07 18:46:29 +02:00

(Migrated from git.eleves.ens.fr)

J'ai réussi à avoir 3 erreurs 500 différentes (mais pas de DOS), donc je vais y jeter un oeil...

mpepin commented

2020-05-07 18:46:45 +02:00

(Migrated from git.eleves.ens.fr)

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

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

lstephan commented

2020-05-08 11:10:07 +02:00

(Migrated from git.eleves.ens.fr)

changed this line in version 3 of the diff

changed this line in [version 3 of the diff](/klub-dev-ens/gestioCOF/merge_requests/411/diffs?diff_id=1589&start_sha=05a5fec664c38e0ef7a58aad51e7c4075f79346b#aa1a330c13e6e0a16675cd54bc580336b47cbdeb_35_34)

lstephan commented

2020-05-08 11:10:10 +02:00

(Migrated from git.eleves.ens.fr)

added 1 commit

f781ae38 - Better docstring

Compare with previous version

added 1 commit <ul><li>f781ae38 - Better docstring</li></ul> [Compare with previous version](/klub-dev-ens/gestioCOF/merge_requests/411/diffs?diff_id=1589&start_sha=05a5fec664c38e0ef7a58aad51e7c4075f79346b)

lstephan commented

2020-05-08 11:13:16 +02:00

(Migrated from git.eleves.ens.fr)

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...

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...

lstephan commented

2020-05-08 11:15:57 +02:00

(Migrated from git.eleves.ens.fr)

added 51 commits

f781ae38...4f15bb96 - 40 commits from branch master
6767ba8e - Rajoute de la doc partout
78ad4402 - Plus de timezones
26bcd729 - Supprime le code mort ou redondant
ef35f45a - Fusionne deux fonctions `chunkify`
48ad5cd1 - Misc cleanup
c66fb7eb - Simplify statistic.js
97cb9d1f - Rework `stats_manifest`
f10d6d1a - Bugfix
c9dad946 - Fix tests
61e4ad97 - Better docstring
c9136dbc - CHANGELOG

Compare with previous version

added 51 commits <ul><li>f781ae38...4f15bb96 - 40 commits from branch <code>master</code></li><li>6767ba8e - Rajoute de la doc partout</li><li>78ad4402 - Plus de timezones</li><li>26bcd729 - Supprime le code mort ou redondant</li><li>ef35f45a - Fusionne deux fonctions `chunkify`</li><li>48ad5cd1 - Misc cleanup</li><li>c66fb7eb - Simplify statistic.js</li><li>97cb9d1f - Rework `stats_manifest`</li><li>f10d6d1a - Bugfix</li><li>c9dad946 - Fix tests</li><li>61e4ad97 - Better docstring</li><li>c9136dbc - CHANGELOG</li></ul> [Compare with previous version](/klub-dev-ens/gestioCOF/merge_requests/411/diffs?diff_id=1590&start_sha=f781ae38f9aa3d72d33c82e36bb261facec29b75)

mpepin commented

2020-05-08 12:40:52 +02:00

(Migrated from git.eleves.ens.fr)

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

Tout à fait d'accord !

> 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 Tout à fait d'accord !

mpepin commented

2020-05-08 12:40:57 +02:00

(Migrated from git.eleves.ens.fr)

resolved all discussions

mpepin commented

2020-05-08 12:41:13 +02:00

(Migrated from git.eleves.ens.fr)

Allez hop

mpepin commented

2020-05-08 12:41:20 +02:00

(Migrated from git.eleves.ens.fr)

mentioned in commit 5a9ea4234e