Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 Transmet correctement la reponse intitule #1377

Conversation

cprodhomme
Copy link
Contributor

No description provided.

@etienneCharignon
Copy link
Member

😑 Je m'aperçois que ça ne va pas du tout ce qu'on a fait. Ce n'est pas de la responsabilité de la class defi.vue de manipuler reponse.
Cette class ne doit pas connaitre reponse, mais simplement s'occuper de l'envoyer au serveur.
En ajoutant ici la responsabilité d'extraite scoreMax et reponseIntitule, on vient polluer toutes les situations avec des notions qui ne concernent qu'Evacob.
On a peut-être déjà introduit des régressions et pollué beaucoup d'événements.

@etienneCharignon
Copy link
Member

etienneCharignon commented Jun 29, 2024

…
       this.question.retranscription_audio,
        scoreMax: this.scoreMax(),
        metacompetence: this.question.metacompetence,
        reponseIntitule: this.reponseIntitule(),
        ...this.reponse
      });
},

En fait this.reponse contient retranscription_audio, du coup avec ce code, on envoie deux fois l'information, puisqu'elle est aussi envoyée dans reponseIntitule

@etienneCharignon
Copy link
Member

etienneCharignon commented Jun 29, 2024

🤔 Bon ben, on a quand même bien un nouveau besoin qui est de calculer des champs dynamiquement dans la réponse.

Je serais pour qu'on crée une nouvelle classe, peut-être "Reponse" ou "AdaptateurReponse"… qui, permettrait de calculer les champs dynamiques d'une réponse.
Pour éviter d'avoir le code dans defi.vue

Quelque chose comme ça :

    envoi () {
      this.envoyer = true;
      let reponse = new Reponse(this.reponse);
      this.$emit('reponse', {
        question: this.question.id,
        intitule: this.question.intitule ?? this.question.retranscription_audio,
        metacompetence: this.question.metacompetence,
        ...reponse.champs()
      });
   }

@cprodhomme cprodhomme force-pushed the EVA-46-le-conseiller-peut-consulter-le-detail-de-la-reponse-dans-lexport-detaille-5 branch 3 times, most recently from b56c217 to 8fed261 Compare July 5, 2024 08:16
@cprodhomme cprodhomme force-pushed the EVA-46-le-conseiller-peut-consulter-le-detail-de-la-reponse-dans-lexport-detaille-5 branch from 8fed261 to 3330d82 Compare July 5, 2024 08:16
Copy link
Member

@etienneCharignon etienneCharignon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

J'ai fait une relecture, mais je n'ai pas testé en local. Ça mériterait d'être fait avant de merger, je pense.
Je ne serais pas dispo avant jeudi prochain pour le faire par contre.

@@ -20,7 +20,8 @@ export default {
} else {
const succes = estSucces(reponse, this.question.reponse.bonne_reponse);
const score = succes ? this.question.score : 0;
this.$emit('reponse', { score, succes, reponse });
const scoreMax = this.question.score;
this.$emit('reponse', { score, succes, reponse, scoreMax });
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

est-ce que tu peux mettre les champs dans le même ordre que dans clic_sur_mots (reponse, succes, score, scoreMax) ?
Je sais que ce n'est pas important techniquement, mais c'est une bonne chose que le code dupliqué soit écrit de la même manière.

Par ailleurs, je vois une duplication :

     const score = succes ? this.question.score : 0;
      const scoreMax = this.question.score;
      this.$emit('reponse', { reponse, succes, score, scoreMax } )

Je te laisse voir si on ne veut pas déplacer cette duplication dans un helper ou pas. J'hésite.

src/situations/cafe_de_la_place/vues/components/puzzle.vue Outdated Show resolved Hide resolved
@@ -77,7 +77,7 @@ describe('La vue jauge', function () {
wrapper.find('.label-libelle').trigger('click');
wrapper.vm.$nextTick(() => {
expect(wrapper.emitted('reponse')[0])
.toEqual([{ reponse: '3c178015-a7c1-4ff8-a344-8553a61e754a' }]);
.toEqual([{ reponse: '3c178015-a7c1-4ff8-a344-8553a61e754a', reponseIntitule: '3 : facile' }]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 C'est chelou d'ajouter reponseIntitule sachant qu'on a déjà les infos côté backend.

Mais ok, pas souci de symétrie avec le reste.

@cprodhomme cprodhomme force-pushed the EVA-46-le-conseiller-peut-consulter-le-detail-de-la-reponse-dans-lexport-detaille-5 branch from 3330d82 to 8364e3f Compare July 5, 2024 11:01
@marouria marouria merged commit b546237 into develop Jul 5, 2024
2 checks passed
@marouria marouria deleted the EVA-46-le-conseiller-peut-consulter-le-detail-de-la-reponse-dans-lexport-detaille-5 branch July 5, 2024 14:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants