feat: No class average if it is not known

Bulgus commented 2 weeks ago

Checklist d'avant pull request

Veuillez cocher toutes les cases applicables en remplaçant [ ] par [x].

[X] Vous avez testé de build le projet avec vos modifications et ce build a réussi
[X] Vous respectez les conventions de codage et de nommage du projet
[X] Vous utilisez la tabulation pour l'indentation afin de maintenir un code lisible
[X] Cette pull request n'est pas un duplicata d'une autre
[X] Cette pull request est prête à être revue (review) et fusionnée (merge)
[X] Il n'y a pas de TODO (aka des annotations pour du code manquant) dans vos modifications
[X] Il n'y a pas d'erreurs de langue dans votre code (grammaire, vocabulaire, conjugaison, orthographe)
[X] Les détails des changements ont été décrits ci-dessous
[X] Cette pull-request n'est pas une "breaking-change" (des modifications qui vont entraîner la modification du fonctionnement de certaines fonctionnalités déjà existantes)

Changelogs proposés

Si Papillon n'arrive pas à récupérer la moyenne de classe, celle ci est égale à 0.

La moyenne de classe est désormais affichée comme "Inconnue" si elle n'est pas récupérable par Papillon (= 0).
Il n'est pas non plus possible d'afficher les détails de celle ci en cliquant sur le graphique (Moyenne minimum et maximum, qui ne sont pas connues même si affichées).

Informations supplémentaires

Avant

https://github.com/user-attachments/assets/2eb089e2-7e52-4e74-87ea-a0f32ea5aa9c

https://github.com/user-attachments/assets/9215b788-612b-4c5b-8051-e63be3f43275

Kgeek33 commented 2 weeks ago

c'est dans quel cas que Papillon n'arrive pas à récupérer la moyenne de la classe ? Car normalement, Papillon ne récupère pas la moyenne exacte, mais la calcule en fonction de tes matières (en lien avec l'issue : #356 )

Bulgus commented 2 weeks ago

Merci des infos. J'ai regardé l'issue, et du coup : Comment la moyenne est calculée ? C'est basé sur toutes les moyennes de matières les plus hautes / basses ?

Si oui, je pense qu'il vaut mieux ne pas l'afficher, car des moyennes à 19,5 c'est très peu probable... donc pour moi si pas de moyenne retournée par Pronote (c'est le cas dans pas mal d'établissements dont le miens), ne pas l'afficher. ("Inconnue" et graph non cliquable)

Je peux tenter de regarder si je peux faire ça si ça vous semble bon..?

Et pour répondre à la première question, il semble qu'avec mon Pronote, Papillon échoue à trouver une moyenne générale de classe, et la met à 0. Et pensant que cette valeur était retournée lorsque que Pawnote ne pouvait pas récupérer la moyenne, j'ai fais cette PR.

Dites moi ce que vous en pensez, mais en tout cas pour moi pas de moyenne sur Pronote = pas de moyenne de classe calculée

Ou alors que la moyenne générale, et pas la moyenne plus haute / basse car celles ci sont vraiment improbables et à mon avis peu utiles.

Kgeek33 commented 2 weeks ago

Alors :

Je pense que tu devrais désactiver dans le cas général désactivé l'affichage des meilleures et pires moyennes. C'est qqch de calculé et donc improbable
Au niveau de ton problème, je pense que c'est un bug en rapport avec l'issue et ton problème. En effet, quand je lis tous vos comportements, on dirait que Pawnote récupère la moyenne quand elle n'est pas disponible (du coup, une moyenne simulée quand disponible). A voir dans le code

Mets tes modifs en commentaire et analyse le 2ème point que j'ai écrit et dis-moi quand tu as des nouvelles ;)

Kgeek33 commented 2 weeks ago

Et tiens, j'ai pensé à une Feature. Quand c'est une moyenne de classe simulée, afficher une icône, ou qqch pour qu'on comprenne que ce n'est pas la vraie moyenne de classe Comme sur la v6

Bulgus commented 2 weeks ago

@Kgeek33

Je suis d'accord avec toi. Pour moi la moyenne max et minimum ne servent à rien... Même si avec #363 cela évite la confusion, je pense que le mieux c'est de tout simplement les enlever, car ça sera toujours calculé, et improbable (donc inutile).
Oui, et cela devrait être patch avec #362
Et je suis complètement pour l'apparition d'un icon signalant que la moyenne est calculée quand celle ci n'est pas retourné par Pawnote

Kgeek33 commented 2 weeks ago

Mdrr t'aurait pu faire tout ça dans une seule pr mais Vince t'a devancé en faisant plusieurs pr...

Bon ben tout ce que tu peux faire, c'est ajouter une icône pour la moyenne de la classe (et encore faut que la pr sur la moyenne de la classe soit accepté)

Kgeek33 commented 2 weeks ago

Si tu veux une occupation pour garder cette pr essayé d'intégrer comme sur la v6 une simulation de notes Si tu n'as pas connu, c'est un bouton qui te permettait de saisir une note dans une matière, le coeff... Et le graphique affichait la nouvelle moyenne générale simulée du coup

Bulgus commented 2 weeks ago

Oui mdrr

Yep... mais je peux close celle là puisqu'à priori elle ne sera jamais inconnue la moyenne.. Et faut voir pour supprimer la moyenne max et minimum C'est qui qui décide de ça ?..

Kgeek33 commented 2 weeks ago

Oui du coup, ta pr ne sert plus. Décider sur quoi ?

Bulgus commented 2 weeks ago

Qui décide de si on garde la moyenne max et minimum ? Je peux refaire une PR en supprimant la moyenne max et minimum, et en ajoutant un icon si la moyenne est calculée

Kgeek33 commented 2 weeks ago

Ben en fait, c'est une fonctionnalité qui date (j'pense avant même la v6), donc c'est personne qui décide en soi, juste personne n'a eu l'idée de la remettre en question (perso, j'pense que ça sert à rien mais bon)

Moi, je te conseille de refaire une pr quand toutes les pr de Vince seront merged et aussi de le pas supprimer l'affiche des moyennes bonnes/pires, mais les désactiver (comme t'as fait sur ta pr, rien ne se passe quand on clique sur ta pr)

Bulgus commented 2 weeks ago

Okay... donc faut que quelqu'un la remette en question alors...

Nickel merci. Mais pourquoi masquer ?.. Ça fait du code en trop dont on ne se sert pas dans ces cas là nan ?..

Kgeek33 commented 2 weeks ago

Okay... donc faut que quelqu'un la remette en question alors...

Nickel merci. Mais pourquoi masquer ?.. Ça fait du code en trop dont on ne se sert pas dans ces cas là nan ?..

Oui, tu sais déjà que je suis contre 😁 (et surtout, entre nous, quelle serait la probabilité que les gens sachent qu'il faut cliquer sur le graphique pour afficher des moyennes bonnes/pires ?). C'est tellement mal indiqué !

Ah ça c'est à toi de décider sur ta future pr ! Mais t'as raison, du code en moins et plus d'efficacité :)

Bulgus commented 2 weeks ago

Oui 😁 (Oui, c'est vrai que je le savais, mais encore faut-il le savoir.. je vais faire une PR en supprimant tout, je verrais bien, mais si c'est pas merge, faudra me dire à quoi ça sert de laisser..)

Yep :)

Kgeek33 commented 2 weeks ago

Oui ben tkt pas sur le merged, regarde ma pr #325, c'est quand même pas rien de résoudre tous les bugs mais depuis que j'ai créé ma pr, 10 ont été merged entre temps............

Kgeek33 commented 2 weeks ago

Si on voit mon msg, j'espère que les façons de review les pr vont changer...

Bulgus commented 2 weeks ago

Oui j'ai vu ça... ça fait 2 semaines.. et ça devrait être merge en priorité pour continuer sur un code clean (si je dis pas de bêtise ?)...

Kgeek33 commented 2 weeks ago

Oui exactement, un code clean et une application stable mais bon... Par contre, a partir de demain, si tjrs pas de review, je vais commencer à spammer car ça commence à être énervant de prioriser certaines prs qui sont (beaucoup) moins importantes que d'autres (c'est pas pour me mettre à l'avant, loin de là mais comme exemple bien parfait : #281)

Bulgus commented 2 weeks ago

Wow 9 octobres... plus d'un mois... Ouais.. sachant que certaines de mes PR bien moins importantes et moins bien faites sont passées en 1 semaine...

Kgeek33 commented 2 weeks ago

Allez, on va rien dire 🙄 J'espère que notre workflow (que je travaille avec @Louis-htmlcss ) permettra de faire changer les choses Si ça t'intéresse, voici le repo (une pr sera tjrs ouverte pour tester) : https://github.com/Louis-htmlcss/test-du-workflows

Bulgus commented 2 weeks ago

Expo go intégré dans chaque PR ?? C'est génial.. Oui j'espère que ça aidera...

Kgeek33 commented 2 weeks ago

Et oui ! https://github.com/PapillonApp/Papillon/pull/364

Louis-htmlcss commented 2 weeks ago

Expo go intégré dans chaque PR ?? C'est génial.. Oui j'espère que ça aidera...

Oui j'ai fait ma pr si tu veux la voir ♥️

Bulgus commented 2 weeks ago

J'ai vu ça.. et ça peut être très très pratique (voire indispensable pour tester et rapidement review donc...) GG !

Kgeek33 commented 1 week ago

Mdrr faut closed la pr @Bulgus Et la refaire plus tard

PapillonApp / Papillon