Moodle

force unique/unseen questions in quiz retakes

Details

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

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

This issue is blocked by:
MDL-20764 In quiz attempt.php, call to $accessmanager->prevent_access() shoud pass arguments Major Open

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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.)

People

Vote (3)
Watch (8)

Dates

  • Created:
    Updated:
Atlassian JIRA (v4.4.3#663-r165197) | Report a problem
Powered by a free Atlassian JIRA open source license for Moodle. Try JIRA - bug tracking software for your team.