search

moodleou / moodle-quiz_answersheets

A Moodle quiz report to allow quiz attempts to be exported
8 stars 15 forks source link

Fix create an attempt with unlimited attempts and update deprecated ModalEvents.yes #10

Closed lamtranb closed 4 years ago

lamtranb commented 4 years ago

Fix bugs:

HuongNV13 commented 4 years ago

This will fix #9

timhunt commented 4 years ago

I was already looking at the patch when Huong's request came throught :-)

Just one general point: When you find your code has a bug like "Cannot create attempt properly if quiz is unlimited", ... well that mistake happened once, so it is clearly the sort of case that can break, and we did not have an automated test to detect it. Therefore, it would be good practice to add a test now, to make sure it never breaks again.

In fact the best practice (if you have time) is:

  1. Write a test that reproduced the bug.
  2. Run the test and verify that it fails.
  3. Fix the bug.
  4. Run the test and verify that it now passes.

Anyway, I did not hold up merging this, but something to think about next time.

lamtranb commented 4 years ago

Hi @timhunt, Thanks for your advice. I will write a behat test to cover this case and make sure it's working as expected in the next commit.

timhunt commented 4 years ago

:-) Great thanks.