Moodle
  1. Moodle
  2. MDL-36683

impossible to duplicate a quiz when Course question bank contains matching questions

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.2.6, 2.3.3, 2.4
    • Fix Version/s: 2.2.7, 2.3.4
    • Component/s: Questions, Quiz
    • Labels:
    • Testing Instructions:
      Hide

      Note: I suggest to first try to reproduce the problem without applying the patch because I think that as the problem may be OS dependant we first need to ensure that the problem is happening on the test configuration to be sure it is fixed after applying the patch !!
      When both Moodle server and client are on a windows machine, the problem always happens.

      In a course with some questions create a match question containing a subquestion with several paragraphs (You can as the original reporter did for instance type some text and hit return to create an empty paragraph, but you can try other variations as long as there is several paragraphs in the subquestion text).
      Create a quiz in this course. You don't need to include the offending question in the quiz.
      Try to duplicate the quiz.
      Without the patch
      You get an error_question_match_sub_missing_in_db exception
      With the patch applied
      Duplication should not display any error.

      Show
      Note: I suggest to first try to reproduce the problem without applying the patch because I think that as the problem may be OS dependant we first need to ensure that the problem is happening on the test configuration to be sure it is fixed after applying the patch !! When both Moodle server and client are on a windows machine, the problem always happens. In a course with some questions create a match question containing a subquestion with several paragraphs (You can as the original reporter did for instance type some text and hit return to create an empty paragraph, but you can try other variations as long as there is several paragraphs in the subquestion text). Create a quiz in this course. You don't need to include the offending question in the quiz. Try to duplicate the quiz. Without the patch You get an error_question_match_sub_missing_in_db exception With the patch applied Duplication should not display any error.
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
    • Rank:
      46184

      Description

      when trying to duplicate a quiz
      debug mode max DEVELOPER
      if the course's question bank contains Matching questions, error:
      even if the quiz being duplicated does NOT contain any matching questions!
      example with course containing just one matching question:
      // question: 43 name: match countries and capitals
      ::match countries and capitals::[html]<p>match countries and capitals</p>{
      =<p>France</p> -> Paris
      =<p>the US</p>\n<p> </p> -> Washington
      =<p>Italy</p>\n<p> </p> -> Roma
      }

      ------------------
      error/error_question_match_sub_missing_in_db

      More information about this error
      Debug info:
      Error code: error_question_match_sub_missing_in_db
      $a contents: stdClass Object
      (
      [id] => 8
      [code] => 407836914
      [questiontext] => <p>the US</p>
      <p> </p>
      [questiontextformat] => 1
      [answertext] => Washington
      )
      Stack trace:

      line 144 of \question\type\match\backup\moodle2\restore_qtype_match_plugin.class.php: restore_step_exception thrown
      line 131 of \backup\util\plan\restore_structure_step.class.php: call to restore_qtype_match_plugin->process_match()
      line 103 of \backup\util\helper\restore_structure_parser_processor.class.php: call to restore_structure_step->process()
      line 125 of \backup\util\xml\parser\processors\grouped_parser_processor.class.php: call to restore_structure_parser_processor->dispatch_chunk()
      line 91 of \backup\util\helper\restore_structure_parser_processor.class.php: call to grouped_parser_processor->postprocess_chunk()
      line 148 of \backup\util\xml\parser\processors\simplified_parser_processor.class.php: call to restore_structure_parser_processor->postprocess_chunk()
      line 92 of \backup\util\xml\parser\processors\progressive_parser_processor.class.php: call to simplified_parser_processor->process_chunk()
      line 169 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser_processor->receive_chunk()
      line 253 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser->publish()
      line ? of unknownfile: call to progressive_parser->end_tag()
      line 158 of \backup\util\xml\parser\progressive_parser.class.php: call to xml_parse()
      line 137 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser->parse()
      line 105 of \backup\util\plan\restore_structure_step.class.php: call to progressive_parser->process()
      line 153 of \backup\util\plan\base_task.class.php: call to restore_structure_step->execute()
      line 163 of \backup\util\plan\base_plan.class.php: call to base_task->execute()
      line 157 of \backup\util\plan\restore_plan.class.php: call to base_plan->execute()
      line 315 of \backup\controller\restore_controller.class.php: call to restore_plan->execute()
      line 101 of \course\modduplicate.php: call to restore_controller->execute_plan()

        Issue Links

          Activity

          Hide
          Jean-Michel Vedrine added a comment -

          Hello Joseph,
          I think this is because of the empty paragraph <p> </p> in question match sub questiontext.
          If I remember well people have already reported getting this error when restoring a backup (quite normal as the same code is used during backup/restore and duplication). I will try to find the tracker issue(s) and link them to this one.
          It would be interesting to find

          • Why your match sub question text contains an empty paragraph (If I remember, you recently created another issue where you had an empty paragraph too ?). Do you know what is the character in this empty paragraph ? (in the other issue it was a non-breaking space, by the way I don't really understand how you get a non breaking space in a Moodle question ??).
          • Why the backup/restore process seems to have trouble when there is such a paragraph in a match subquestion.

          If you look at the code in restore_qtype_match_plugin.class.php process_match function
          // Look for matching subquestion (by question, questiontext and answertext)
          $sub = $DB->get_record_select('question_match_sub', 'question = ? AND ' .
          $DB->sql_compare_text('questiontext') . ' = ' .
          $DB->sql_compare_text('?').' AND answertext = ?',
          array($newquestionid, $data->questiontext, $data->answertext),
          'id', IGNORE_MULTIPLE);
          you need to find why this query is not returning any record.
          So look in your database to see if there is a record in the question_match_sub table with questiontext equal <p>the US</p>
          <p> </p>
          and answertext equal Washington
          I will try do do some tests

          Show
          Jean-Michel Vedrine added a comment - Hello Joseph, I think this is because of the empty paragraph <p> </p> in question match sub questiontext. If I remember well people have already reported getting this error when restoring a backup (quite normal as the same code is used during backup/restore and duplication). I will try to find the tracker issue(s) and link them to this one. It would be interesting to find Why your match sub question text contains an empty paragraph (If I remember, you recently created another issue where you had an empty paragraph too ?). Do you know what is the character in this empty paragraph ? (in the other issue it was a non-breaking space, by the way I don't really understand how you get a non breaking space in a Moodle question ??). Why the backup/restore process seems to have trouble when there is such a paragraph in a match subquestion. If you look at the code in restore_qtype_match_plugin.class.php process_match function // Look for matching subquestion (by question, questiontext and answertext) $sub = $DB->get_record_select('question_match_sub', 'question = ? AND ' . $DB->sql_compare_text('questiontext') . ' = ' . $DB->sql_compare_text('?').' AND answertext = ?', array($newquestionid, $data->questiontext, $data->answertext), 'id', IGNORE_MULTIPLE); you need to find why this query is not returning any record. So look in your database to see if there is a record in the question_match_sub table with questiontext equal <p>the US</p> <p> </p> and answertext equal Washington I will try do do some tests
          Hide
          Joseph Rézeau added a comment - - edited

          Hello Jean-Michel and thanks for your prompt reply... Looks like French moodlers never take a rest, not even on Sundays!

          1.- Your diagnostic is correct, this bug is indeed related to MDL-36571. If I remove the empty paragraphs (with non-breaking space included) from my questions' answer fields, I can duplicate a quiz.

          2.- J.-M. :

          by the way I don't really understand how you get a non breaking space in a Moodle question


          Well, the HTML editor automatically inserts a non-breaking space inside empty paragraphs, like most HTML editors will do (e.g. Dreamweaver).

          3.- Now, the question is why do I sometimes get empty paragraphs in my question's Answer fields? Well, the answer is: this happens when - after I have entered some text in a field - I use the tab key on my keyboard to navigate from one field to another - rather than clicking in the next field with the mouse! I expect very few moodlers will do that, but whenever possible, to save time, I prefer using the keyboard to the mouse.

          4.- And finally how to fix this bug?
          4a. The fix I suggested in MDL-36571 will not help here, as it only works for the question display.
          4b. We must attack the very source of the problem, not its consequences. We must find a fix which will prevent empty paragraphs (with non-breaking space included) from being saved to the moodle database when questions are saved.

          Show
          Joseph Rézeau added a comment - - edited Hello Jean-Michel and thanks for your prompt reply... Looks like French moodlers never take a rest, not even on Sundays! 1.- Your diagnostic is correct, this bug is indeed related to MDL-36571 . If I remove the empty paragraphs (with non-breaking space included) from my questions' answer fields, I can duplicate a quiz. 2.- J.-M. : by the way I don't really understand how you get a non breaking space in a Moodle question Well, the HTML editor automatically inserts a non-breaking space inside empty paragraphs, like most HTML editors will do (e.g. Dreamweaver). 3.- Now, the question is why do I sometimes get empty paragraphs in my question's Answer fields? Well, the answer is: this happens when - after I have entered some text in a field - I use the tab key on my keyboard to navigate from one field to another - rather than clicking in the next field with the mouse! I expect very few moodlers will do that, but whenever possible, to save time, I prefer using the keyboard to the mouse. 4.- And finally how to fix this bug? 4a. The fix I suggested in MDL-36571 will not help here, as it only works for the question display. 4b. We must attack the very source of the problem, not its consequences. We must find a fix which will prevent empty paragraphs (with non-breaking space included) from being saved to the moodle database when questions are saved .
          Hide
          Jean-Michel Vedrine added a comment - - edited

          hello Joseph,
          You said :
          the HTML editor automatically inserts a non-breaking space inside empty paragraphs, like most HTML editors will do (e.g. Dreamweaver).
          I was totally unaware of this behaviour ! In fact I was even thinking that non breaking spaces had to be encoded as entities : & nbsp; or & #160; or & #xa0; in HTML (sorry I had to put spaces so that entitues codes are displayed).
          But if the sequence <p>non breaking space</p> is a valid HTML construct and is produced every time a Moodle user insert an empty paragraph in the HTML editor (so quite often !) then Moodle backup/restore process must be absolutely be able to manage such a sequence !!
          So I don't think the right way is to remove or prevent empty paragraph, we need to fix backup/restore !

          Show
          Jean-Michel Vedrine added a comment - - edited hello Joseph, You said : the HTML editor automatically inserts a non-breaking space inside empty paragraphs, like most HTML editors will do (e.g. Dreamweaver). I was totally unaware of this behaviour ! In fact I was even thinking that non breaking spaces had to be encoded as entities : & nbsp; or & #160; or & #xa0; in HTML (sorry I had to put spaces so that entitues codes are displayed). But if the sequence <p>non breaking space</p> is a valid HTML construct and is produced every time a Moodle user insert an empty paragraph in the HTML editor (so quite often !) then Moodle backup/restore process must be absolutely be able to manage such a sequence !! So I don't think the right way is to remove or prevent empty paragraph, we need to fix backup/restore !
          Hide
          Joseph Rézeau added a comment -

          @Jean-Michel,
          You got me wrong. I was certainly not suggesting to remove empty paragraphs (that include non-breaking space) for ALL uses of the Moodle HTML editor, but only when saving the Answer texts of questions (MCQ, Matching, maybe others too). I'm pretty sure users do not want/need to have empty paragraphs in their MCQ or Matching questions, do they? And anyway, as I mentioned in MDL-36571, such sequences of <p>nonbreakingspace</p> ruin the appearance of those questions, so they must be removed when the question is saved.

          The backup/restore is a different problem, which would only need to be solved if the <p>nonbreakingspace</p> sequence affects other parts of Moodle than questions.

          Show
          Joseph Rézeau added a comment - @Jean-Michel, You got me wrong. I was certainly not suggesting to remove empty paragraphs (that include non-breaking space) for ALL uses of the Moodle HTML editor, but only when saving the Answer texts of questions (MCQ, Matching, maybe others too). I'm pretty sure users do not want/need to have empty paragraphs in their MCQ or Matching questions, do they? And anyway, as I mentioned in MDL-36571 , such sequences of <p>nonbreakingspace</p> ruin the appearance of those questions, so they must be removed when the question is saved. The backup/restore is a different problem, which would only need to be solved if the <p>nonbreakingspace</p> sequence affects other parts of Moodle than questions.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Joseph,
          I was able to reproduce your problem.
          here is what I did (master branch)
          I created a new course
          I created a match question putting a valid paragraph + empty paragraph in one of the subquestions
          I created a quiz (I tested both including and not including the match question)
          When I try to duplicate the quiz I get the infamous error_question_match_sub_missing_in_db error message.
          So now we "just" need to find what exactly is going wrong because we have a reproducible pattern !

          Show
          Jean-Michel Vedrine added a comment - Hello Joseph, I was able to reproduce your problem. here is what I did (master branch) I created a new course I created a match question putting a valid paragraph + empty paragraph in one of the subquestions I created a quiz (I tested both including and not including the match question) When I try to duplicate the quiz I get the infamous error_question_match_sub_missing_in_db error message. So now we "just" need to find what exactly is going wrong because we have a reproducible pattern !
          Hide
          Jean-Michel Vedrine added a comment - - edited

          Well in fact quiz duplication fails in a lot more cases than when there is a subquestion with an empty paragraph at the end !
          I can reproduce the problem

          • when there is an empty paragraph anywhere in the subquestion
          • when there is 2 valid non empty paragraphs in the subquestion

          Also there is a similar problem with hints when I include an empty paragraph producing the error_question_hint_missing_in_db
          Maybe the problem is not with the fact there is a paragraph (empty or not) but that there is an end of line character ?

          Show
          Jean-Michel Vedrine added a comment - - edited Well in fact quiz duplication fails in a lot more cases than when there is a subquestion with an empty paragraph at the end ! I can reproduce the problem when there is an empty paragraph anywhere in the subquestion when there is 2 valid non empty paragraphs in the subquestion Also there is a similar problem with hints when I include an empty paragraph producing the error_question_hint_missing_in_db Maybe the problem is not with the fact there is a paragraph (empty or not) but that there is an end of line character ?
          Hide
          Jean-Michel Vedrine added a comment - - edited

          Bingo !
          the string coming from $data->questiontext contains a lf and the string in database contains a cr+lf
          so if I replace $data->questiontext with preg_replace('~\R~u', "\r\n", $data->questiontext) in the query $sub = $DB->get_record_select( ... the error message is not displayed and I am able to duplicate the quiz without any problem.
          Warning ! I am not suggesting this is the needed fix for this issue, it only demonstrates that the problem is an end of line problem.

          Show
          Jean-Michel Vedrine added a comment - - edited Bingo ! the string coming from $data->questiontext contains a lf and the string in database contains a cr+lf so if I replace $data->questiontext with preg_replace('~\R~u', "\r\n", $data->questiontext) in the query $sub = $DB->get_record_select( ... the error message is not displayed and I am able to duplicate the quiz without any problem. Warning ! I am not suggesting this is the needed fix for this issue, it only demonstrates that the problem is an end of line problem.
          Hide
          Joseph Rézeau added a comment -

          well-done, Jean-Michel!
          Let's wait and see what Tim thinks of it all...

          Show
          Joseph Rézeau added a comment - well-done, Jean-Michel! Let's wait and see what Tim thinks of it all...
          Hide
          Jean-Michel Vedrine added a comment -

          Yes we need to wait Tim comment.
          Just to be clear my code change above was just to demonstrate the problem.
          In fact I think the opposite : I think $data is right to have normalised end of lines (quite to be expected as duplication use the same code that backup/restore so must be xml safe) and that it's odd to have unnormalised end of lines allowed to go into the database.
          Anyway the current sql query can't find the subquestion as soon as there is more than a single paragraph in the subquestion and that it is stored with non unix EOL in the database.
          The problem is exactly the same with the sql query lines 3125-3128 of backup\moodle2\restore_stepslib.php for hints with multiple paragraphs.
          I wonder if this problem is dependent of the server or client OS (only tested on windows for both) ?

          Show
          Jean-Michel Vedrine added a comment - Yes we need to wait Tim comment. Just to be clear my code change above was just to demonstrate the problem. In fact I think the opposite : I think $data is right to have normalised end of lines (quite to be expected as duplication use the same code that backup/restore so must be xml safe) and that it's odd to have unnormalised end of lines allowed to go into the database. Anyway the current sql query can't find the subquestion as soon as there is more than a single paragraph in the subquestion and that it is stored with non unix EOL in the database. The problem is exactly the same with the sql query lines 3125-3128 of backup\moodle2\restore_stepslib.php for hints with multiple paragraphs. I wonder if this problem is dependent of the server or client OS (only tested on windows for both) ?
          Hide
          Jean-Michel Vedrine added a comment -

          Most probably the solution is to do something similar to what is done lines 162-172 of backup\moodle2\restore_qtype_plugin.class.php for answers : get all the relevant records and loop to find the good one, cleaning the content of the database field before doing the compare.

          Show
          Jean-Michel Vedrine added a comment - Most probably the solution is to do something similar to what is done lines 162-172 of backup\moodle2\restore_qtype_plugin.class.php for answers : get all the relevant records and loop to find the good one, cleaning the content of the database field before doing the compare.
          Hide
          Jean-Michel Vedrine added a comment -

          Linking to the issue where the same problem was solved for answers only a month ago !!

          Show
          Jean-Michel Vedrine added a comment - Linking to the issue where the same problem was solved for answers only a month ago !!
          Hide
          Jean-Michel Vedrine added a comment -

          I have added Eloy to this issue as he is MDL-30018 fix author and clearly when he said in MDL-30018 comments :

          IMPORTANT: Also, in order to get more and more matches and less &%$&%$ in the DB... couldn't we be performing that xml_safe_utf8() normalization. I'm a bit concerned that this exact problem is susceptible to be happening in a lot of places where we perform text matching. Perhaps something to discuss @ HQ, because that would be a global thing to add to all forms or so.

          he was obviously right !!

          Eloy, was this discussed at HQ ?
          Patching the code each time we run in a similar situation where text compare is failing seems as an awfull way to go ! Just for the quiz this issue reveal 2 places (subquestions and hints) where there is the same problem.

          Show
          Jean-Michel Vedrine added a comment - I have added Eloy to this issue as he is MDL-30018 fix author and clearly when he said in MDL-30018 comments : IMPORTANT: Also, in order to get more and more matches and less &%$&%$ in the DB... couldn't we be performing that xml_safe_utf8() normalization. I'm a bit concerned that this exact problem is susceptible to be happening in a lot of places where we perform text matching. Perhaps something to discuss @ HQ, because that would be a global thing to add to all forms or so. he was obviously right !! Eloy, was this discussed at HQ ? Patching the code each time we run in a similar situation where text compare is failing seems as an awfull way to go ! Just for the quiz this issue reveal 2 places (subquestions and hints) where there is the same problem.
          Hide
          Tim Hunt added a comment -

          MDL-26442 may also be similar.

          Show
          Tim Hunt added a comment - MDL-26442 may also be similar.
          Hide
          Tim Hunt added a comment -

          I think this fix is right. I have taken the fix for question_answers from MDL-30018, and done the same sort of thing for qtype_match subquestions.

          However, I don't have an easy way to verify this, so if anyone else could test, that would be great.

          Could someone write some testing instructions?

          Show
          Tim Hunt added a comment - I think this fix is right. I have taken the fix for question_answers from MDL-30018 , and done the same sort of thing for qtype_match subquestions. However, I don't have an easy way to verify this, so if anyone else could test, that would be great. Could someone write some testing instructions?
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          Looking at it your fix seems OK, I will test it.
          Do you think we should fix the same problem for hints (see backup/moodle2/restore_stepslib.php lines 3125-3139 in this issue or create another issue ?

          Show
          Jean-Michel Vedrine added a comment - Hello Tim, Looking at it your fix seems OK, I will test it. Do you think we should fix the same problem for hints (see backup/moodle2/restore_stepslib.php lines 3125-3139 in this issue or create another issue ?
          Hide
          Jean-Michel Vedrine added a comment -

          Tim,
          Your fix works.
          With your fix applied, in all the test courses I have created to reproduce this bug, quiz duplicating now succeed and don't display any error.
          but in the course where there is a question with a hint containing several paragraphs, as I was expecting I get
          error/error_question_hint_missing_in_db

          More information about this error
          Debug info:
          Error code: error_question_hint_missing_in_db
          $a contents: stdClass Object
          (
          [filequestionid] => 2222
          [dbquestionid] => 2222
          [hint] => <p>This is hint 1</p>
          <p> </p>
          )
          Stack trace:

          line 3138 of \backup\moodle2\restore_stepslib.php: restore_step_exception thrown
          line 131 of \backup\util\plan\restore_structure_step.class.php: call to restore_create_categories_and_questions->process_question_hint()
          line 103 of \backup\util\helper\restore_structure_parser_processor.class.php: call to restore_structure_step->process()
          line 125 of \backup\util\xml\parser\processors\grouped_parser_processor.class.php: call to restore_structure_parser_processor->dispatch_chunk()
          line 91 of \backup\util\helper\restore_structure_parser_processor.class.php: call to grouped_parser_processor->postprocess_chunk()
          line 148 of \backup\util\xml\parser\processors\simplified_parser_processor.class.php: call to restore_structure_parser_processor->postprocess_chunk()
          line 92 of \backup\util\xml\parser\processors\progressive_parser_processor.class.php: call to simplified_parser_processor->process_chunk()
          line 169 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser_processor->receive_chunk()
          line 253 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser->publish()
          line ? of unknownfile: call to progressive_parser->end_tag()
          line 158 of \backup\util\xml\parser\progressive_parser.class.php: call to xml_parse()
          line 137 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser->parse()
          line 105 of \backup\util\plan\restore_structure_step.class.php: call to progressive_parser->process()
          line 153 of \backup\util\plan\base_task.class.php: call to restore_structure_step->execute()
          line 163 of \backup\util\plan\base_plan.class.php: call to base_task->execute()
          line 157 of \backup\util\plan\restore_plan.class.php: call to base_plan->execute()
          line 315 of \backup\controller\restore_controller.class.php: call to restore_plan->execute()
          line 101 of \course\modduplicate.php: call to restore_controller->execute_plan()

          Show
          Jean-Michel Vedrine added a comment - Tim, Your fix works. With your fix applied, in all the test courses I have created to reproduce this bug, quiz duplicating now succeed and don't display any error. but in the course where there is a question with a hint containing several paragraphs, as I was expecting I get error/error_question_hint_missing_in_db More information about this error Debug info: Error code: error_question_hint_missing_in_db $a contents: stdClass Object ( [filequestionid] => 2222 [dbquestionid] => 2222 [hint] => <p>This is hint 1</p> <p> </p> ) Stack trace: line 3138 of \backup\moodle2\restore_stepslib.php: restore_step_exception thrown line 131 of \backup\util\plan\restore_structure_step.class.php: call to restore_create_categories_and_questions->process_question_hint() line 103 of \backup\util\helper\restore_structure_parser_processor.class.php: call to restore_structure_step->process() line 125 of \backup\util\xml\parser\processors\grouped_parser_processor.class.php: call to restore_structure_parser_processor->dispatch_chunk() line 91 of \backup\util\helper\restore_structure_parser_processor.class.php: call to grouped_parser_processor->postprocess_chunk() line 148 of \backup\util\xml\parser\processors\simplified_parser_processor.class.php: call to restore_structure_parser_processor->postprocess_chunk() line 92 of \backup\util\xml\parser\processors\progressive_parser_processor.class.php: call to simplified_parser_processor->process_chunk() line 169 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser_processor->receive_chunk() line 253 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser->publish() line ? of unknownfile: call to progressive_parser->end_tag() line 158 of \backup\util\xml\parser\progressive_parser.class.php: call to xml_parse() line 137 of \backup\util\xml\parser\progressive_parser.class.php: call to progressive_parser->parse() line 105 of \backup\util\plan\restore_structure_step.class.php: call to progressive_parser->process() line 153 of \backup\util\plan\base_task.class.php: call to restore_structure_step->execute() line 163 of \backup\util\plan\base_plan.class.php: call to base_task->execute() line 157 of \backup\util\plan\restore_plan.class.php: call to base_plan->execute() line 315 of \backup\controller\restore_controller.class.php: call to restore_plan->execute() line 101 of \course\modduplicate.php: call to restore_controller->execute_plan()
          Hide
          Tim Hunt added a comment -

          There is already a separate issue for hints, so I will do a separate patch for that. Thanks for testing. I will cherry-pick and submit for integration tomorrow.

          Show
          Tim Hunt added a comment - There is already a separate issue for hints, so I will do a separate patch for that. Thanks for testing. I will cherry-pick and submit for integration tomorrow.
          Hide
          Jean-Michel Vedrine added a comment -

          Is it MDL-33863 ?

          Show
          Jean-Michel Vedrine added a comment - Is it MDL-33863 ?
          Hide
          Tim Hunt added a comment -

          Yes. That's the one.

          Show
          Tim Hunt added a comment - Yes. That's the one.
          Hide
          Tim Hunt added a comment -

          Jean-Michel. If you can upload a test file that can be used to reproduce the but, and verify the fix, then it might be a good idea if you can do that, and update the testing instructions to use it. Don't worry if you can't. Thanks.

          Show
          Tim Hunt added a comment - Jean-Michel. If you can upload a test file that can be used to reproduce the but, and verify the fix, then it might be a good idea if you can do that, and update the testing instructions to use it. Don't worry if you can't. Thanks.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (22, 23 & master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (22, 23 & master), thanks!
          Hide
          Jean-Michel Vedrine added a comment -

          Hello,
          I am going to work so I will not be able to upload something before several hours. Sorry.
          Additionally, maybe I am wrong but I think this will be difficult to test with a file (either import/export or backup/restore) as it is only triggered when $questioncreated is false so for instance during duplication or backup/restore when there are shared question categories used.
          But maybe I am not seeing what you mean when you say to upload a file ?

          Show
          Jean-Michel Vedrine added a comment - Hello, I am going to work so I will not be able to upload something before several hours. Sorry. Additionally, maybe I am wrong but I think this will be difficult to test with a file (either import/export or backup/restore) as it is only triggered when $questioncreated is false so for instance during duplication or backup/restore when there are shared question categories used. But maybe I am not seeing what you mean when you say to upload a file ?
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim (but this comment may also be interesting for future tester of this issue),
          I have made additional test and I think the result is interesting.

          Test procedure

          • Create some questions at system level and among them a match question with several paragraphs in one of the subquestions
          • In a course create a test and include some questions from system level in that quiz (you don't need to include the problematic match question in the quiz)
          • Backup the course
          • Try to restore the course on the same Moodle server
          • Try to restore the course on another Moodle server

          Results of the test

          Without MDL-36683 fix applied
          Restore is successful on another server but fails on the same server with the error_question_match_sub_missing_in_db exception
          With MDL-36683 patch applied
          Restore is successful on the same server and on another server

          Conclusions

          As we were thinking this bug not only affected duplication with match questions with several paragraphs in the course but also backup/restore with match questions with several paragraphs at the system level in case of restore on the same server.

          There was no problem when restore was done on a different server, so I am totally unable to upload a file exhibiting the problem because restoring it on a different server will succeed.

          The proposed patch seems to fix the problem in both cases. So fixing this issue will make life a lot easier for people using shared questions. This is good news.

          To reproduce and test the problem you must create the question on the test configuration because this bug is about non cleaned data getting into the database from the question editing form and comparison of this data with cleaned data coming from a backup. You cannot test with only a backup file.

          This is easily understandable because as I said the code affected by this bug is only executed when the question was not created during restore.

          Show
          Jean-Michel Vedrine added a comment - Hello Tim (but this comment may also be interesting for future tester of this issue), I have made additional test and I think the result is interesting. Test procedure Create some questions at system level and among them a match question with several paragraphs in one of the subquestions In a course create a test and include some questions from system level in that quiz (you don't need to include the problematic match question in the quiz) Backup the course Try to restore the course on the same Moodle server Try to restore the course on another Moodle server Results of the test Without MDL-36683 fix applied Restore is successful on another server but fails on the same server with the error_question_match_sub_missing_in_db exception With MDL-36683 patch applied Restore is successful on the same server and on another server Conclusions As we were thinking this bug not only affected duplication with match questions with several paragraphs in the course but also backup/restore with match questions with several paragraphs at the system level in case of restore on the same server. There was no problem when restore was done on a different server, so I am totally unable to upload a file exhibiting the problem because restoring it on a different server will succeed. The proposed patch seems to fix the problem in both cases. So fixing this issue will make life a lot easier for people using shared questions. This is good news. To reproduce and test the problem you must create the question on the test configuration because this bug is about non cleaned data getting into the database from the question editing form and comparison of this data with cleaned data coming from a backup. You cannot test with only a backup file. This is easily understandable because as I said the code affected by this bug is only executed when the question was not created during restore.
          Hide
          Tim Hunt added a comment -

          Thanks for the detailed explanation Jean-Michel. You are right, and I was already aware of the "this bug not only affected duplication ... but also backup/restore with ... questions ... at the system level" thing. It is just that the duplication thing leads to simpler testing instructions.

          Show
          Tim Hunt added a comment - Thanks for the detailed explanation Jean-Michel. You are right, and I was already aware of the "this bug not only affected duplication ... but also backup/restore with ... questions ... at the system level" thing. It is just that the duplication thing leads to simpler testing instructions.
          Hide
          Rajesh Taneja added a comment -

          Thanks Tim,

          I tried replicating the issue first and it was easy to replicate error. With patch no error came and quiz was duplicated without problem.

          Show
          Rajesh Taneja added a comment - Thanks Tim, I tried replicating the issue first and it was easy to replicate error. With patch no error came and quiz was duplicated without problem.
          Hide
          Dan Poltawski added a comment -

          Congratulations! Another bug solved.. only another 7330 to go, thanks for contributing to contributing to 0.8% of all bugs being fixed this week!

          ciao
          Dan

          Show
          Dan Poltawski added a comment - Congratulations! Another bug solved.. only another 7330 to go, thanks for contributing to contributing to 0.8% of all bugs being fixed this week! ciao Dan

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: