Moodle
  1. Moodle
  2. MDL-6340

force unique/unseen questions in quiz retakes

    Details

    • Type: Improvement Improvement
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.8.2
    • Fix Version/s: STABLE backlog
    • Component/s: Quiz
    • Labels:
      None
    • Environment:
      not sure, but probably all environments
    • Database:
      Any
    • Affected Branches:
      MOODLE_18_STABLE
    • Rank:
      988

      Description

      When retaking a quiz, there is no way to force retakes to pull ONLY unseen questions from the pool, so that EVERY retake has completely unique/unseen questions in it. For example, I have a pool of 100 questions, and want to create a quiz with 25 questions in it. I want to give the student 4 chances at the quiz, and every time the student takes the quiz, he gets completely new questions, such that if he takes it 4 times, he will have seen all 100 questions. This would work well in conjunction with randomizing questions, which currently just pulls a random set for each retake, but allows for the same questions to randomly be pulled into each retake.

      This would be useful in the flashcard type of lesson too.

        Issue Links

          Activity

          Hide
          Oleg Sychev added a comment -

          I think the correct way to implement this would be to add a new field to $cmoptions passed to create_session_and_responses like $cmoptions->excludedquestions

          Than a module (guided by it's logic and current settings) can deside which questions it would like to exclude from random question, while random question just follows it's hints.

          If finding suitable question is impossible, should it return an error or silently fallback on using excluded questions?

          Show
          Oleg Sychev added a comment - I think the correct way to implement this would be to add a new field to $cmoptions passed to create_session_and_responses like $cmoptions->excludedquestions Than a module (guided by it's logic and current settings) can deside which questions it would like to exclude from random question, while random question just follows it's hints. If finding suitable question is impossible, should it return an error or silently fallback on using excluded questions?
          Hide
          Shkarupa Alex added a comment -

          This patch for moodle 1.9.
          I'll make patch for moodle 2.0 in around 2 days

          Show
          Shkarupa Alex added a comment - This patch for moodle 1.9. I'll make patch for moodle 2.0 in around 2 days
          Hide
          Tim Hunt added a comment - - edited

          That looks pretty good. I have a bunch of minor complaints, and one serious one, number 6.

          > $oldattempts = array_keys(get_records_select( ...

          1. can we avoid doing this potentially expensive database query unless it is really necessary?

          2. The logic is wrong. The student may have attempted other quizzes, we are only interested in their attempts at this quiz, aren't we?

          3. The logic is wrong in another way. You need the quiz_attempt.uniqueid, not quiz_attempt.id in $oldattempts.

          > function get_usable_questions_from_category(...)

          4. I don't really like passing the whole of $cmoptions in here. I would rather leave $questionsinuse, and add an extra parameter, perhaps $avoidquestionsfromattempts

          5. Please use preg, not ereg.

          6. You are possibly using two or three database queries where previously there was one. This is bad for performance. Hmm. I see, it is more complex than that because of the $this->catrandoms caching. I guess the code now does the right thing, it is just not very clear.

          7. Good idea that get_usable_questions_from_category returns just the question ids, rather than objects with just an ->id field.

          So, what to do about 6?

          It would be nice to do everything in one database query. The following is my idea. I have not tested it, so it probably does not work. It also needs to be tested for performance, but the idea is worth pursuing.

          SELECT q.id, COUNT(qst.id) AS numprevioususes
          
          FROM mdl_question q
          LEFT JOIN mdl_question_states qst ON qst.answer LIKE CONCAT('random', q.id, '-%')
          
          WHERE q.category IN ($categorylist)
          AND q.parent = 0
          AND q.hidden = 0
          AND q.id NOT IN ($questionsinuse)
          AND qtype NOT IN ($this->excludedqtypes)
          AND qst.attempt IN ($avoidquestionsfromattempts)
          AND qst.seq_number = 0
          
          GROUP BY q.id
          
          ORDER BY numprevioususes ASC
          

          Do that query with get_records_sql_menu. That will give you an array of questionid => number of times that question has been used in previous attempts, in order of number of attempts.

          That way, you can use the first part of the array to get questions that have never been used before, then use the next bit to get questions that have been used once before, and then questions that have been used twice before, and so on.

          I hope all that makes sense. Thank you for working on this.

          Show
          Tim Hunt added a comment - - edited That looks pretty good. I have a bunch of minor complaints, and one serious one, number 6. > $oldattempts = array_keys(get_records_select( ... 1. can we avoid doing this potentially expensive database query unless it is really necessary? 2. The logic is wrong. The student may have attempted other quizzes, we are only interested in their attempts at this quiz, aren't we? 3. The logic is wrong in another way. You need the quiz_attempt.uniqueid, not quiz_attempt.id in $oldattempts. > function get_usable_questions_from_category(...) 4. I don't really like passing the whole of $cmoptions in here. I would rather leave $questionsinuse, and add an extra parameter, perhaps $avoidquestionsfromattempts 5. Please use preg, not ereg. 6. You are possibly using two or three database queries where previously there was one. This is bad for performance. Hmm. I see, it is more complex than that because of the $this->catrandoms caching. I guess the code now does the right thing, it is just not very clear. 7. Good idea that get_usable_questions_from_category returns just the question ids, rather than objects with just an ->id field. So, what to do about 6? It would be nice to do everything in one database query. The following is my idea. I have not tested it, so it probably does not work. It also needs to be tested for performance, but the idea is worth pursuing. SELECT q.id, COUNT(qst.id) AS numprevioususes FROM mdl_question q LEFT JOIN mdl_question_states qst ON qst.answer LIKE CONCAT('random', q.id, '-%') WHERE q.category IN ($categorylist) AND q.parent = 0 AND q.hidden = 0 AND q.id NOT IN ($questionsinuse) AND qtype NOT IN ($this->excludedqtypes) AND qst.attempt IN ($avoidquestionsfromattempts) AND qst.seq_number = 0 GROUP BY q.id ORDER BY numprevioususes ASC Do that query with get_records_sql_menu. That will give you an array of questionid => number of times that question has been used in previous attempts, in order of number of attempts. That way, you can use the first part of the array to get questions that have never been used before, then use the next bit to get questions that have been used once before, and then questions that have been used twice before, and so on. I hope all that makes sense. Thank you for working on this.
          Hide
          Shkarupa Alex added a comment -

          So, this is the next version of my patch.
          To Tim: i modified your SQL query a bit, because it caused an error when there were no attempts

          Show
          Shkarupa Alex added a comment - So, this is the next version of my patch. To Tim: i modified your SQL query a bit, because it caused an error when there were no attempts
          Hide
          Oleg Sychev added a comment -

          Alex, some questions about you patch:
          1. Are you sure about using isset($avoidquestionsfromattempts) ? This is a function argument and it will probably always be set. Did you test it?
          2. You are probably forget to shuffle questions in case of no previous attempts exists.

          Tim, could you test new SQL query please? It works fine in MySQL. The changes where caused by NULLs returning by join in case no matching row exists, which caused MySQL to not return questions with 0 uses. So case with 0 uses require more attentions in testing.

          Also, would you like to have to returns in get_usable_questions.... (like in the last patch), or prefer setting $catrandoms and one return in the end?

          Show
          Oleg Sychev added a comment - Alex, some questions about you patch: 1. Are you sure about using isset($avoidquestionsfromattempts) ? This is a function argument and it will probably always be set. Did you test it? 2. You are probably forget to shuffle questions in case of no previous attempts exists. Tim, could you test new SQL query please? It works fine in MySQL. The changes where caused by NULLs returning by join in case no matching row exists, which caused MySQL to not return questions with 0 uses. So case with 0 uses require more attentions in testing. Also, would you like to have to returns in get_usable_questions.... (like in the last patch), or prefer setting $catrandoms and one return in the end?
          Hide
          Shkarupa Alex added a comment -

          1) Yes, i'm sure. It works properly.
          2) You're right, i forgot. but now i corrected that

          Show
          Shkarupa Alex added a comment - 1) Yes, i'm sure. It works properly. 2) You're right, i forgot. but now i corrected that
          Hide
          Oleg Sychev added a comment -

          Patch for Moodle 2.0 uploaded. Credits for patch upgrading goes to Sergei Bastrykin.

          Let's see how many more Moodle versions will go without that functionality...

          Show
          Oleg Sychev added a comment - Patch for Moodle 2.0 uploaded. Credits for patch upgrading goes to Sergei Bastrykin. Let's see how many more Moodle versions will go without that functionality...
          Hide
          Oleg Sychev added a comment -

          Well, does question_variant_pseudorandom_no_repeats_strategy in Moodle 2.1 fixes this?

          Show
          Oleg Sychev added a comment - Well, does question_variant_pseudorandom_no_repeats_strategy in Moodle 2.1 fixes this?
          Hide
          Tim Hunt added a comment -

          No, I don't think it does. It solves a different problem (synchronised datasets and similar functionality in our new 'varnumeric' question types.

          However, when I was doing the changes in Moodle 2.1, I did consider this issue, even though I did not solve it. This was one of the reasons I tried to un-tangle the way the random question type works.

          I think that it is now possible to fix this issue while only changing mod/quiz/startattempt.php (around lines 163-170).

          (Well, I don't know if we need to introduce an option for this, or whether we just always do it. If we need to introduce an option, then you would have to change other files, but actually, I see not need to make this optional. I think that we should always avoid repeats with previous attempts, if possible. If that is not possible, we should fall back to only avoiding repeats with the current attempt.)

          Show
          Tim Hunt added a comment - No, I don't think it does. It solves a different problem (synchronised datasets and similar functionality in our new 'varnumeric' question types. However, when I was doing the changes in Moodle 2.1, I did consider this issue, even though I did not solve it. This was one of the reasons I tried to un-tangle the way the random question type works. I think that it is now possible to fix this issue while only changing mod/quiz/startattempt.php (around lines 163-170). (Well, I don't know if we need to introduce an option for this, or whether we just always do it. If we need to introduce an option, then you would have to change other files, but actually, I see not need to make this optional. I think that we should always avoid repeats with previous attempts, if possible. If that is not possible, we should fall back to only avoiding repeats with the current attempt.)
          Hide
          Tim Hunt added a comment -

          Just removing the patch label. None of the patches here are for recent versions of Moodle.

          Show
          Tim Hunt added a comment - Just removing the patch label. None of the patches here are for recent versions of Moodle.
          Hide
          Oleg Sychev added a comment -

          And what chances there are if even they would be updated for more recent versions of Moodle? Judging by extra_question_fields code not much. I'm not going to update it many times (it's already was done from 1.9 to 2.0 with this code, and to no avail), so please note times when you really could look at results.

          Show
          Oleg Sychev added a comment - And what chances there are if even they would be updated for more recent versions of Moodle? Judging by extra_question_fields code not much. I'm not going to update it many times (it's already was done from 1.9 to 2.0 with this code, and to no avail), so please note times when you really could look at results.
          Hide
          Tim Hunt added a comment -

          This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.

          For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          Tim Hunt added a comment - This issue was assigned to me automatically, however I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

            People

            • Votes:
              4 Vote for this issue
              Watchers:
              10 Start watching this issue

              Dates

              • Created:
                Updated: