Récupérer l'attribut "correct" d'une proposition

LouisePietropaoli commented 4 years ago

Bonjour, J'essaie de récupérer l'attribut "correct" d'une proposition mais, d'après l'API serveur, aucun moyen de le récupérer. A la création ou la mise à jour d'une question, on DOIT renseigner cet attribut, sinon, erreur 500 : Error: Internal Server Error { "appname": "lifap5-backend-2019-2020", "version": "1.0.5", "name": "error", "message": "null value in column \"correct\" violates not-null constraint", "status": 500 }

Mais, quand on récupère une question, l'attribut "correct" n'apparaît pas : { "quiz_id": 2508, "question_id": 0, "sentence": "Cette question a t'elle une réponse ?", "propositions_number": 2, "correct_propositions_number": 1, "propositions": [ { "content": "oui", "proposition_id": 0 }, { "content": "non", "proposition_id": 1 } ] }

Comment faire pour y avoir accès ? Merci !

Lifeismana commented 4 years ago

Je l'ai fait moi-même perso, tu as "correct_propositions_number" et "proposition_id" plus qu'à les match ~~PS: si tu ne l'as pas fait , tu aurais pu demandé à un chargé de td sur le chat/en mp, c'est plus à privilégier avant de poster une question ici je pense~~

LouisePietropaoli commented 4 years ago

Bonjour,

l'attribut "correct_propositions_number" est le nombre de propositions justes, donc il ne donne pas d'indication sur LES propositions qui sont justes ! (même en ayant les id des propositions...)

Webcretaire commented 4 years ago

Bonjour,

Je vous ai répondu à l'un et l'autre en MP, mais voici la réponse si quelqu'un d'autre se pose la question :

Il est impossible de récupérer les réponses justes via ce point d'API, car ce serait une grosse faille dans le système du quizz : n'importe quel utilisateur curieux pourrait regarder l'onglet réseau de son navigateur et avoir la correction au quizz qu'il est en train de faire

Il n'y a que le propriétaire qui a accès à la "correction" d'un quizz, via ce point d'API spécifique : https://lifap5.univ-lyon1.fr/api-docs/#/answers/getOneQuestionAnswersHandler

romulusFR commented 4 years ago

Oui, l'idée était que seul le propriétaire puisse savoir quelle proposition est correcte, sans cela, un répondant (non propriétaire) pourrait tricher. Donc /quizzes/{quiz_id}/questions/ ne donne pas ce champs. Par contre, le PB, est peut être que UNE fois crée, même le proprio ne sait plus laquelle est correcte.

En effet, sur https://lifap5.univ-lyon1.fr/api-docs/#/answers/getOneQuestionAnswersHandler on a l'info. La question c'est peut -être où le mettre de mieux ?

Webcretaire commented 4 years ago

Je vois principalement 4 options :

Faire des points d'API spécifiques pour le propriétaire, qui fonctionnent exactement comme les versions publiques (style /quizzes/{quiz_id}/questions), mais qui ont ce champ en plus (style /quizzes/{quiz_id}/questions/admin ou je ne sais quoi) :arrow_right: Ça fait beaucoup de duplication d'information / code dans le backend pas forcément nécessaire
Réutiliser le point d'API mais en ajoutant une option (style /quizzes/{quiz_id}/questions?owner=1), qui rend le point d'API plus restrictif (403 si pas owner) mais qui ajoute le champ si l'utilisateur est propriétaire :arrow_right: Ça va faire des conditions peut être pas belles dans le code du backend, et c'est peut être pas trivial à expliquer dans la doc swagger
Faire en sorte que le point d'API détecte automatiquement si l'utilisateur est propriétaire ou pas, pour ajouter (ou non) le champ dans les résultats du point d'API public qui existe déjà :arrow_right: C'est peut être moins facile à implémenter / exprimer clairement dans la doc
Ajouter l'info dans un des points d'API qui est déjà restreint au propriétaire (un peu comme /answers/getOneQuestionAnswersHandler, mais peut être le mettre à d'autre(s) endroit(s)) : par exemple dans /quizzes/getUserQuizzes :
```
// Au lieu de l'attribut JSON :
"questions_ids": [
  0,
  1
],
```

// Quelque chose comme : "questions": [ {"id": 0, "correct_answers": [0]}, {"id": 1, "correct_answers": [0,1]}, ],


:arrow_right: *C'est un peu tordu, comme dans `/answers/getOneQuestionAnswersHandler` mais peut être moins mal placé, je ne sais pas*

Clairement le plus simple pour ne pas se prendre la tête c'est la première option, mais d'un autre côté c'est pas beau d'ajouter un point d'API pour accéder à juste un attribut d'une entité

romulusFR commented 4 years ago

Globalement. Il faudrait sans doute refactor l'API, car les routes ne sont pas toutes très intuitives (notament, s'il fallait refaire, je choisirai la 3ème option de partout : traitement différencié)

De plus toutes les features qu'on aimerait ne sont pas proposées (même s'il est fait du mieu que je peux, ca reste un serveur jouet).

Donc là la question, c'est est-ce que ca empèche de réaliser une des features demandées ? Là j'ai l'impression que ça gène pour la feature "calcul des notes des quiz" https://github.com/romulusFR/lifap5-backend-2019-2020/blob/master/client/README.md

L'idée que j'avais c'était d'aller une fois sur </users/quizzes> puis pour chaque question une fois sur </quizzes/{quiz_id}/questions/{question_id}/answers/>. Ca ne suffit pas ?

Alors, ca risque de faire péter le rate_limiter, je pourrai le monter

Webcretaire commented 4 years ago

La feature "calcul des notes des quiz" est faisable (et à déjà été faite, coucou @LouisePietropaoli ). Après c'est vrai qu'elle est assez difficile vu le format des données renvoyées par N appels à /quizzes/{quiz_id}/questions/{question_id}/answers/ : on récupère un tableau d'utilisateur pour chaque question, or dans l'affichage on veut plutôt un tableau de questions pour chaque utilisateur. C'est surtout pas facile à faire de manière 100% fonctionnelle, mais bon dans la mesure où c'est une des fonctionnalités optionnelles je ne trouve pas ça dérangeant que ça pose un peu de challenge.

La question du rate limiter se pose par contre en effet. La solution simple est de dire que s'il y a trop de questions dans un quizz, et donc trop de requêtes, on met une message d'erreur dans le client (style "cette feature n'est pas supportée pour ce quizz"). Sinon peut être faire une route du style /quizzes/{quiz_id}/answers/, qui fait exactement la même chose que N appels à /quizzes/{quiz_id}/questions/{question_id}/answers/ regroupés (ça renverra potentiellement un très gros JSON, mais c'est toujours mieux un JSON N fois plus gros que N appels au serveur). Ou sinon monter le rate limiter, mais bon ça ne fait que repousser le problème à des quizz plus gros

Lifeismana commented 4 years ago

Pour passer à travers le rate limiter je "me suis fais" une ptite fonction fetch qui réessaye n fois si y'a une erreur dans la réponse (je filtre pas en fonction des erreurs je pourrais le faire en soit mais ça veut dire parser ici et changer des trucs autre part)

// "Bypass" du rate limiter
const delay = (t) => new Promise((resolve) => setTimeout(resolve, t));
// eslint-disable-next-line no-unused-vars
const fetchLimiter = (url, body, n) =>
  fetch(url, body).catch((error) => {
    if (n === 1) throw error;
    // on met un delai de 0 à 2.5s pour pas trop spam le serveur
    // eslint-disable-next-line promise/no-nesting
    return delay(Math.floor(Math.random() * 2500)).then(() => fetchLimiter(url, body, n - 1));
  });

ça marche bien, bon ça spam un poil la console mais ça marche j'ai pas essayé mais sinon mettre du délai avant d’envoyer une requête après un des problème c'est qu'on attend d'avoir toutes les données avant de faire l'affichage mais c'est potentiellement gérable

Webcretaire commented 4 years ago

@Lifeismana je comprends que tu ais voulu bypass ce problème en mettant un délai, mais :

C'est un mauvais contournement, dans la vraie vie c'est avec des bricolages comme ça qu'on se retrouve avec des systèmes inutilement lourds, peu maintenables, etc. Encourager ce genre de choses dans un projet scolaire me semble une très mauvaise idée, donc je maintiens qu'il vaut mieux gérer l'erreur si elle arrive (afficher un message d'erreur par exemple), sans faire de bricolage pour que ça passe quand même (et si c'était une fonctionnalité critique d'un vrai système on refactorerait le backend)
Même si on admettait que la solution du délai est acceptable, pourquoi mettre de l'aléatoire là-dedans ? Et surtout de l'aléatoire qui peut donner une très petite valeur ? Si le tirage te donne 4 puis 27 et enfin 12 ça fait 3 requêtes au serveur dans un intervalle de temps ridiculement court, c'est du spam qu'on pourrait facilement éviter soit en mettant un délai fixe raisonnable, soit en mettant une borne inférieure qui fait sens à ton aléatoire (si pour une raison obscure on tient à ce que le délai soit aléatoire)

Lifeismana commented 4 years ago

Pour moi l'intérêt de l'aléatoire c'est vraiment d'étaler les requêtes pour limiter un peu le spam et puis la première requête est pas instantané donc se retrouve avec encore du délai et si #21 est implémenter on a plus du tout besoin d'aléatoire

romulusFR / lifap5-backend-2019-2020

Récupérer l'attribut "correct" d'une proposition #17