drachels/moodle-mod_hotquestion

Vote and unvote actions undo / toggle on page reload

Kemmotar83 opened this issue · 3 comments

Hi @drachels ,

when a user vote a question everything is fine but, if you immediately reload the page the vote is removed. You have the same behaviour with the unvote action.

To test the vote issue (given a hotquestion activity with questions made by different users):

  • access as a student and go to hotquestion activity
  • vote a question
  • check if the vote is correctly saved
  • reload the page (ie. Ctrl+R, F5, ...)
  • check that the vote is no longer there

To test the unvote issue (given a hotquestion activity with questions made by different users):

  • access as a student and go to hotquestion activity
  • vote a question
  • check if the vote is correctly saved
  • unvote the same question
  • check if the vote is correctly removed
  • reload the page (ie. Ctrl+R, F5, ...)
  • check that the vote is there again

The reason of these behaviours can be found in the locallib.php file.

For the vote action, the function vote_on (row 113) has the following code lines (rows 128-134) that toggle the vote depending on if the user has already voted.

 if (!$this->has_voted($question->id)) {
    $votes->question = $question->id;
    $votes->voter = $USER->id;
    $DB->insert_record('hotquestion_votes', $votes);
} else {
    $DB->delete_records('hotquestion_votes', ['question' => $question->id, 'voter' => $USER->id]);
}

For the unvote action, function remove_vote (row 149) has the same code lines (rows 164-170) that toggle the vote depending on if the user has already voted.

if (!$this->has_voted($question->id)) {
    $votes->question = $question->id;
    $votes->voter = $USER->id;
    $DB->insert_record('hotquestion_votes', $votes);
} else {
    $DB->delete_records('hotquestion_votes', ['question' => $question->id, 'voter' => $USER->id]);
}

For me, in both functions the else block should be removed and the if conditions should be merged with the external ones. If a user reload the page, the action should be simply ignored.

If needed, I can provide a pull request.

Thanks,
Giorgio

Hi Giorgio,
Thanks for the information. I can confirm this behavior and will see about a fix as soon as I can as I do have a couple of other bug tickets I am currently working on for HotQuestion. I will shoot for a new release, next week.

I would be happy if you do provide a pull request as due to previous experience I know it is so easy to misunderstand what someone else is trying to describe as a possible fix for an error. I remember once, it took me three days to finally understand an error that someone was describing in another plugin I maintain. In that particular case the problem was due to not speaking the same language, but it is so easy to have a misunderstanding even when both users are fluent in a given language.

No need for a pull request now as I just pushed changes to the master branch that fixes this, and a couple of other items. I will try to do a new release in a couple of days as I have a couple more fixes I want to include.

Hi @drachels ,

thank you for solving this issue.

Regards,
Giorgio