Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-30018

Quiz Duplication With Carriage Return Results in a Fatal Error

    Details

    • Database:
      PostgreSQL
    • Testing Instructions:
      Hide

      NOTE where I say '^M' you must insert a carriage return character into the field, on my OS this is achieved when I type ctrl-m

      1/ Create a new quiz with default settings
      2/ Add a multiple choice question to the quiz

      • Question Name: 'Duplication Problems'
      • Choice 1:
        Answer: 'An answer with this: ^M newline'
        Grade: 100%
      • Choice 2:
        Answer: 'Another answer'
        Grade: 0%
        3/ Go to course page and press the x2 button
        4/ Continue with duplicate of quiz

      Expected result
      -----------------
      Quiz is Duplicated

      Actual Result
      -------------
      error/error_question_answers_missing_in_db

      Show
      NOTE where I say '^M' you must insert a carriage return character into the field, on my OS this is achieved when I type ctrl-m 1/ Create a new quiz with default settings 2/ Add a multiple choice question to the quiz Question Name: 'Duplication Problems' Choice 1: Answer: 'An answer with this: ^M newline' Grade: 100% Choice 2: Answer: 'Another answer' Grade: 0% 3/ Go to course page and press the x2 button 4/ Continue with duplicate of quiz Expected result ----------------- Quiz is Duplicated Actual Result ------------- error/error_question_answers_missing_in_db
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:

      Description

      When attempting to duplicate a quiz using the x2 button the duplication fails. I thought this might be MDL-29432, but it doesn't seem to match any of the rules in that bug.

      When I attempt to replicate the issue on my local machine by backing up and restoring the whole course this works fine and I can't replicate the issue. I have access to the live data which might be useful to replicate and fix this issue.

      error/error_question_answers_missing_in_db
       
      More information about this error
       
      Stack trace:
      line 159 of /backup/moodle2/restore_qtype_plugin.class.php: restore_step_exception thrown
      line 131 of /backup/util/plan/restore_structure_step.class.php: call to restore_qtype_plugin->process_question_answer()
      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 148 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 302 of /backup/controller/restore_controller.class.php: call to restore_plan->execute()
      line 90 of /course/modduplicate.php: call to restore_controller->execute_plan()

        Gliffy Diagrams

          Issue Links

            Activity

            poltawski Dan Poltawski created issue -
            Hide
            timhunt Tim Hunt added a comment -

            I would guess that this is due to bad data in your DB.

            Can you track down which question id this is? Try something like

            SELECT * FROM mdl_question q WHERE qtype IN ('shortanswer', 'multichoice', ...)
            AND NOT EXIST (SELECT * FROM mdl_question_answers qa WHERE qa.question = q.id)

            Show
            timhunt Tim Hunt added a comment - I would guess that this is due to bad data in your DB. Can you track down which question id this is? Try something like SELECT * FROM mdl_question q WHERE qtype IN ('shortanswer', 'multichoice', ...) AND NOT EXIST (SELECT * FROM mdl_question_answers qa WHERE qa.question = q.id)
            Hide
            poltawski Dan Poltawski added a comment - - edited

            I can't find anything the quiz without answer. The quiz which is causing the error has multiple multichoice questions and an essay question. The essay question doesn't have answer records (which I assume is correct behaviour).

            The site originated from 2.1 so shouldn't have suffered from bad data migration for previous versions.

            Show
            poltawski Dan Poltawski added a comment - - edited I can't find anything the quiz without answer. The quiz which is causing the error has multiple multichoice questions and an essay question. The essay question doesn't have answer records (which I assume is correct behaviour). The site originated from 2.1 so shouldn't have suffered from bad data migration for previous versions.
            Hide
            timhunt Tim Hunt added a comment -

            If we can't reproduce the issue, then it is very hard to know how to proceed. I can only suggest that you keep poking around in the live databsae. Or, clone the whole site to a test server, so you can add debugging to the code to diagnose what is going on.

            Show
            timhunt Tim Hunt added a comment - If we can't reproduce the issue, then it is very hard to know how to proceed. I can only suggest that you keep poking around in the live databsae. Or, clone the whole site to a test server, so you can add debugging to the code to diagnose what is going on.
            Hide
            poltawski Dan Poltawski added a comment -

            I have the site on the test server and can debug to my hearts content. Will need to get my head around the code first

            Show
            poltawski Dan Poltawski added a comment - I have the site on the test server and can debug to my hearts content. Will need to get my head around the code first
            Hide
            poltawski Dan Poltawski added a comment -

            So set $DB->set_debug(true) reveals the query is:

            SELECT id
                                  FROM mdl_question_answers
                                 WHERE question = $1
                                   AND answer = $2
            [array (
              0 => '104',
              1 => '<div>
            <p>asked late in the interview</p>
            </div>',
            )]
            <hr />

            Show
            poltawski Dan Poltawski added a comment - So set $DB->set_debug(true) reveals the query is: SELECT id FROM mdl_question_answers WHERE question = $1 AND answer = $2 [array ( 0 => '104', 1 => '<div> <p>asked late in the interview</p> </div>', )] <hr />
            Hide
            poltawski Dan Poltawski added a comment -

            lumoodle=# select * from mdl_question_answers where question =104;
             id  | question |                   answer                   | answerformat | fraction  | feedback | feedbackformat 
            -----+----------+--------------------------------------------+--------------+-----------+----------+----------------
             325 |      104 | <div>\r                                   +|            1 | 1.0000000 |          |              1
                 |          | <p>asked late in the interview</p>\r      +|              |           |          | 
                 |          | </div>                                     |              |           |          | 
             326 |      104 | <div>\r                                   +|            1 | 0.0000000 |          |              1
                 |          | <div>\r                                   +|              |           |          | 
                 |          | <p>asked early in the interview</p>\r     +|              |           |          | 
                 |          | </div>\r                                  +|              |           |          | 
                 |          | </div>                                     |              |           |          | 
             327 |      104 | <p>avoided</p>                             |            1 | 0.0000000 |          |              1
             328 |      104 | <p>interspersed with factual questions</p> |            1 | 0.0000000 |          |              1
            (4 rows)

            Show
            poltawski Dan Poltawski added a comment - lumoodle=# select * from mdl_question_answers where question =104; id | question | answer | answerformat | fraction | feedback | feedbackformat -----+----------+--------------------------------------------+--------------+-----------+----------+---------------- 325 | 104 | <div>\r +| 1 | 1.0000000 | | 1 | | <p>asked late in the interview</p>\r +| | | | | | </div> | | | | 326 | 104 | <div>\r +| 1 | 0.0000000 | | 1 | | <div>\r +| | | | | | <p>asked early in the interview</p>\r +| | | | | | </div>\r +| | | | | | </div> | | | | 327 | 104 | <p>avoided</p> | 1 | 0.0000000 | | 1 328 | 104 | <p>interspersed with factual questions</p> | 1 | 0.0000000 | | 1 (4 rows)
            Hide
            timhunt Tim Hunt added a comment -

            That is starting to look horribly suspicious.

            Show
            timhunt Tim Hunt added a comment - That is starting to look horribly suspicious.
            Hide
            poltawski Dan Poltawski added a comment -

            2011-11-01 12:06:24 GMT LOG:  duration: 0.010 ms  execute <unnamed>: SELECT id
                                          FROM mdl_question_answers
                                         WHERE question = $1
                                           AND answer = $2
            2011-11-01 12:06:24 GMT DETAIL:  parameters: $1 = '104', $2 = '<div>
                    <p>asked late in the interview</p>
                    </div>'

            Show
            poltawski Dan Poltawski added a comment - 2011-11-01 12:06:24 GMT LOG: duration: 0.010 ms execute <unnamed>: SELECT id FROM mdl_question_answers WHERE question = $1 AND answer = $2 2011-11-01 12:06:24 GMT DETAIL: parameters: $1 = '104', $2 = '<div> <p>asked late in the interview</p> </div>'
            Hide
            poltawski Dan Poltawski added a comment -

            Found the same in another question

            Show
            poltawski Dan Poltawski added a comment - Found the same in another question
            Hide
            poltawski Dan Poltawski added a comment -

            In fact most of the questions in that quiz had the problematic character returns. Once they were gone things worked fine

            Show
            poltawski Dan Poltawski added a comment - In fact most of the questions in that quiz had the problematic character returns. Once they were gone things worked fine
            Hide
            timhunt Tim Hunt added a comment -

            So, do you think some sort of "\r\n" -> "\n", or "\r" -> '' on import/restore is a good solution?

            Presumably this is a fairly generic backup/restore issue, not just Questions. Adding Eloy for his wisdom.

            Show
            timhunt Tim Hunt added a comment - So, do you think some sort of "\r\n" -> "\n", or "\r" -> '' on import/restore is a good solution? Presumably this is a fairly generic backup/restore issue, not just Questions. Adding Eloy for his wisdom.
            Hide
            poltawski Dan Poltawski added a comment -

            I can replicate this issue, updating test instructions

            Show
            poltawski Dan Poltawski added a comment - I can replicate this issue, updating test instructions
            poltawski Dan Poltawski made changes -
            Field Original Value New Value
            Testing Instructions NOTE where I say '^M' you must insert a carriage return character into the field, on my OS this is achieved when I type ctrl-m

            1/ Create a new quiz with default settings
            2/ Add a multiple choice question to the quiz
            - Question Name: 'Duplication Problems'
            - Choice 1:
              Answer: 'An answer with this: ^M newline'
              Grade: 100%
            - Choice 2:
              Answer: 'Another answer'
              Grade: 0%
            3/ Go to course page and press the x2 button
            4/ Continue with duplicate of quiz

            Expected result
            -----------------
            Quiz is Duplicated

            Actual Result
            -------------
            error/error_question_answers_missing_in_db
            poltawski Dan Poltawski made changes -
            Summary Quiz Duplication Results in a Fatal Error Quiz Duplication With Carriage Return Results in a Fatal Error
            Hide
            poltawski Dan Poltawski added a comment -

            Tim: Its the data within the exsiting question in the DB which has carriage returns in it. I think backup is stripping them as backup & then restore is not affected

            Show
            poltawski Dan Poltawski added a comment - Tim: Its the data within the exsiting question in the DB which has carriage returns in it. I think backup is stripping them as backup & then restore is not affected
            Hide
            timhunt Tim Hunt added a comment -

            OK, so perhaps it is a HTML editor or input cleaning issue. Anyway, hopefully Eloy can advise.

            Show
            timhunt Tim Hunt added a comment - OK, so perhaps it is a HTML editor or input cleaning issue. Anyway, hopefully Eloy can advise.
            timhunt Tim Hunt made changes -
            Fix Version/s STABLE backlog [ 10463 ]
            Labels triaged
            Hide
            tbannister Tyler Bannister added a comment -

            We've run into a similar problem with one of our customers, several questions with carriage returns are causing the restore process to fail for various courses.

            The HTML editor does allow the insertion of carriage returns into a question answer, in fact, it even automatically inserts the carriage returns when using a Linux client to access a Linux server.

            Show
            tbannister Tyler Bannister added a comment - We've run into a similar problem with one of our customers, several questions with carriage returns are causing the restore process to fail for various courses. The HTML editor does allow the insertion of carriage returns into a question answer, in fact, it even automatically inserts the carriage returns when using a Linux client to access a Linux server.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Would be great to get confirmation about how the same activity duplication behaves if you, simply, comment out, from backup/util/xml/xml_writer.class.php, line 264:

            $content = preg_replace("/\r\n|\r/", "\n", $content); // Normalize line&return=>line

            That's not a proper fix but can show us if that's the cause. Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Would be great to get confirmation about how the same activity duplication behaves if you, simply, comment out, from backup/util/xml/xml_writer.class.php, line 264: $content = preg_replace("/\r\n|\r/", "\n", $content); // Normalize line&return=>line That's not a proper fix but can show us if that's the cause. Ciao
            Hide
            poltawski Dan Poltawski added a comment -

            Attached backup file of test course with problem question

            Show
            poltawski Dan Poltawski added a comment - Attached backup file of test course with problem question
            poltawski Dan Poltawski made changes -
            Hide
            poltawski Dan Poltawski added a comment -

            Ah I didn't notice your comment above!

            Show
            poltawski Dan Poltawski added a comment - Ah I didn't notice your comment above!
            Hide
            poltawski Dan Poltawski added a comment -

            Seems to still be ok on backup/restore if I comment out that line.

            Show
            poltawski Dan Poltawski added a comment - Seems to still be ok on backup/restore if I comment out that line.
            Hide
            tbannister Tyler Bannister added a comment -

            An additional note: To replicate this problem with a backup, the question must be shared. So if you backup from a category and restore to the same category the question must be assigned to either the category pool or the site level pool. It's the question de-duplication code that's triggering the error.

            Commenting out line 264 in backup/util/xml/xml_writer.class.php doesn't seem to fix the problem. It looks like carriage return is dropped when the xml file is read in. I did confirm that carriage return was output into the file when that line was commented out, but was still unable to restore from the file.

            Show
            tbannister Tyler Bannister added a comment - An additional note: To replicate this problem with a backup, the question must be shared. So if you backup from a category and restore to the same category the question must be assigned to either the category pool or the site level pool. It's the question de-duplication code that's triggering the error. Commenting out line 264 in backup/util/xml/xml_writer.class.php doesn't seem to fix the problem. It looks like carriage return is dropped when the xml file is read in. I did confirm that carriage return was output into the file when that line was commented out, but was still unable to restore from the file.
            Hide
            tbannister Tyler Bannister added a comment -

            As far as I can tell, carriage returns are stripped out by the xml_parse function. I did a test by restoring an XML file with an answer with a carriage return in it. I visually confirmed the presence of the carriage return in the XML file, but during the restore process when the file is being parse, and the cdata is put into the data accumulator for the parser, the carriage return is no longer present.

            I inserted a print statement into progressive_parser.class.php and I printed out the line with both newlines and carriage returns replaced by different placeholders. The newline was kept, the carriage return was stripped.

            I think this means the bug has to be fixed in the restore_qtype_plugin.class.php class when the answers are compared. Fixing the html editor so that it doesn't insert carriage returns will only partially solve the problem since it could still allow carriage returns to be inserted by an import function and wouldn't do anything fix backups already made with carriage returns in them.

            This means instead of the current database query to find the answer that matches the data, all of the answers for the question will have to be fetched, cleaned, and then compared against the answer from the XML file.

            Show
            tbannister Tyler Bannister added a comment - As far as I can tell, carriage returns are stripped out by the xml_parse function. I did a test by restoring an XML file with an answer with a carriage return in it. I visually confirmed the presence of the carriage return in the XML file, but during the restore process when the file is being parse, and the cdata is put into the data accumulator for the parser, the carriage return is no longer present. I inserted a print statement into progressive_parser.class.php and I printed out the line with both newlines and carriage returns replaced by different placeholders. The newline was kept, the carriage return was stripped. I think this means the bug has to be fixed in the restore_qtype_plugin.class.php class when the answers are compared. Fixing the html editor so that it doesn't insert carriage returns will only partially solve the problem since it could still allow carriage returns to be inserted by an import function and wouldn't do anything fix backups already made with carriage returns in them. This means instead of the current database query to find the answer that matches the data, all of the answers for the question will have to be fetched, cleaned, and then compared against the answer from the XML file.
            Hide
            tbannister Tyler Bannister added a comment -

            I've attached a simple patch to fix the carriage return problem.

            Show
            tbannister Tyler Bannister added a comment - I've attached a simple patch to fix the carriage return problem.
            tbannister Tyler Bannister made changes -
            Hide
            timhunt Tim Hunt added a comment -

            Thanks for working on this, but: Code review comments:

            1. $DB->get_records always returns an array, so the if (is_array test should go.

            2. $newitemid = 0; should be closer to the loop that sets it.

            3. We are going to restore each answer one after another, therefore doing

            $params = array('question' => $newquestionid);
            $answers = $DB->get_records('question_answers', $params, '', 'id, answer'); 

            once for each answer is horribly inefficient. How ease is to to do that once per question?

            4. By the way, I would inline $params there. Fewer local variables are better (within reason).

            5. Where do the specific details of the cleaning come from? What is our guarantee that that is identical to what happens in the backup code (at some arbitrary date in the future).

            Show
            timhunt Tim Hunt added a comment - Thanks for working on this, but: Code review comments: 1. $DB->get_records always returns an array, so the if (is_array test should go. 2. $newitemid = 0; should be closer to the loop that sets it. 3. We are going to restore each answer one after another, therefore doing $params = array('question' => $newquestionid); $answers = $DB->get_records('question_answers', $params, '', 'id, answer'); once for each answer is horribly inefficient. How ease is to to do that once per question? 4. By the way, I would inline $params there. Fewer local variables are better (within reason). 5. Where do the specific details of the cleaning come from? What is our guarantee that that is identical to what happens in the backup code (at some arbitrary date in the future).
            Hide
            tbannister Tyler Bannister added a comment -

            On issue #3, it's not very easy. We'd either have to rewrite the question/answer handling functions so that questions are not handled individually or use static or instance variables to cache the results from the first lookup.

            On issue #5, the details of the cleaning come from the protected function xml_safe_utf8 in backup/util/xml/xml_writer.class.php.

            Show
            tbannister Tyler Bannister added a comment - On issue #3, it's not very easy. We'd either have to rewrite the question/answer handling functions so that questions are not handled individually or use static or instance variables to cache the results from the first lookup. On issue #5, the details of the cleaning come from the protected function xml_safe_utf8 in backup/util/xml/xml_writer.class.php.
            Hide
            tbannister Tyler Bannister added a comment -

            I've attached an updated fix with the is_array removed and the $newitemid holder moved closer to the loop.

            Show
            tbannister Tyler Bannister added a comment - I've attached an updated fix with the is_array removed and the $newitemid holder moved closer to the loop.
            tbannister Tyler Bannister made changes -
            Hide
            timhunt Tim Hunt added a comment -

            The code is now technically OK, I think. It really needs a backup and restore expert (hello Eloy) to comment on whether the approach is sound.

            Show
            timhunt Tim Hunt added a comment - The code is now technically OK, I think. It really needs a backup and restore expert (hello Eloy) to comment on whether the approach is sound.
            mblake Michael Blake made changes -
            Labels triaged partner triaged
            mchurch Mike Churchward made changes -
            Labels partner triaged partner remote-learner triaged
            Hide
            mchurch Mike Churchward added a comment -

            Hi Eloy... This has become a big issue with us. Can this be accelerated?

            Show
            mchurch Mike Churchward added a comment - Hi Eloy... This has become a big issue with us. Can this be accelerated?
            mchurch Mike Churchward made changes -
            Affects Version/s 2.3.1 [ 12253 ]
            Affects Version/s 2.2.4 [ 12162 ]
            Hide
            tbannister Tyler Bannister added a comment -

            Hi Eloy, I've run into this issue with a few different customers now and I think my patch may also resolve some other bugs like MDL-26442. So do you have an opinion to share on my fix for the issue?

            Show
            tbannister Tyler Bannister added a comment - Hi Eloy, I've run into this issue with a few different customers now and I think my patch may also resolve some other bugs like MDL-26442 . So do you have an opinion to share on my fix for the issue?
            mchurch Mike Churchward made changes -
            Priority Minor [ 4 ] Critical [ 2 ]
            Hide
            mchurch Mike Churchward added a comment -

            I upgraded this issue to critical. In all released versions of Moodle 2, when restores are done with questions in the defined state, the restores fail. This is preventing people from being able to migrate their courses.

            Show
            mchurch Mike Churchward added a comment - I upgraded this issue to critical. In all released versions of Moodle 2, when restores are done with questions in the defined state, the restores fail. This is preventing people from being able to migrate their courses.
            Hide
            mchurch Mike Churchward added a comment -

            I believe MDL-26442 is the same issue.

            Show
            mchurch Mike Churchward added a comment - I believe MDL-26442 is the same issue.
            mchurch Mike Churchward made changes -
            Link This issue has been marked as being related by MDL-26442 [ MDL-26442 ]
            salvetore Michael de Raadt made changes -
            Labels partner remote-learner triaged partner patch remote-learner triaged
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hi Tyler, Mike, Tim, Dan (and Michael, thanks for pointing me to this),

            While the solution is correct... I really think that it would be better to add Tyler's loop+normalization as a fallback alternative, if the direct/simpler/quicker get_field() fails. How does it sound?

            Here there is the candidate for my idea:

            https://github.com/stronk7/moodle/compare/MDL-30018

            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.

            TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hi Tyler, Mike, Tim, Dan (and Michael, thanks for pointing me to this), While the solution is correct... I really think that it would be better to add Tyler's loop+normalization as a fallback alternative, if the direct/simpler/quicker get_field() fails. How does it sound? Here there is the candidate for my idea: https://github.com/stronk7/moodle/compare/MDL-30018 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. TIA and ciao
            Hide
            rob13 Rob added a comment -

            Hi Eloy,

            I tested your patch, and it clears the error. That said, importing or restoring a quiz with system level questions still takes a great deal of time. I left for lunch after 20 minutes of waiting. The import completed successfully before I got back to the office.

            Thank you for the time and effort put in to fixing this and MDL-26442.

            Show
            rob13 Rob added a comment - Hi Eloy, I tested your patch, and it clears the error. That said, importing or restoring a quiz with system level questions still takes a great deal of time. I left for lunch after 20 minutes of waiting. The import completed successfully before I got back to the office. Thank you for the time and effort put in to fixing this and MDL-26442 .
            Hide
            timhunt Tim Hunt added a comment -

            I looked at Eloy's patch. I am not claiming to be qualified to comment, but it looks OK to me.

            Performance is a problem. The answer would seem to be some sort of caching.

            Show
            timhunt Tim Hunt added a comment - I looked at Eloy's patch. I am not claiming to be qualified to comment, but it looks OK to me. Performance is a problem. The answer would seem to be some sort of caching.
            Hide
            tbannister Tyler Bannister added a comment -

            Eloy, the candidate looks good to me.

            Show
            tbannister Tyler Bannister added a comment - Eloy, the candidate looks good to me.
            Hide
            dougiamas Martin Dougiamas added a comment -

            This seems to have a good patch with some peer review. I'm submitting it for integration.

            Please fix the git fields if required.

            Show
            dougiamas Martin Dougiamas added a comment - This seems to have a good patch with some peer review. I'm submitting it for integration. Please fix the git fields if required.
            dougiamas Martin Dougiamas made changes -
            dougiamas Martin Dougiamas made changes -
            Assignee Tim Hunt [ timhunt ] Eloy Lafuente (stronk7) [ stronk7 ]
            dougiamas Martin Dougiamas made changes -
            Status Open [ 1 ] Waiting for integration review [ 10010 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Waiting for integration review [ 10010 ] Reopened [ 4 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

            TIA and ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
            samhemelryk Sam Hemelryk made changes -
            Currently in integration Yes [ 10041 ]
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday.

            This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody.

            This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P

            Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - The integration of this issue has been delayed to next week because the integration period is over (Monday, Tuesday) and testing must happen on Wednesday. This change to a more rigid timeframe on each integration/testing cycle aims to produce a better and clear separation and organization of tasks for everybody. This is a bulk-automated message, so if you want to blame somebody/thing/where, don't do it here (use git instead) :-D :-P Apologizes for the inconvenient, this will be integrated next week. Thanks for your collaboration & ciao
            stronk7 Eloy Lafuente (stronk7) made changes -
            Currently in integration Yes [ 10041 ]
            stronk7 Eloy Lafuente (stronk7) made changes -
            Currently in integration Yes [ 10041 ]
            samhemelryk Sam Hemelryk made changes -
            Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
            Integrator samhemelryk
            Hide
            samhemelryk Sam Hemelryk added a comment -

            Thanks everyone, I've integrated Eloy's fix now.

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks everyone, I've integrated Eloy's fix now.
            samhemelryk Sam Hemelryk made changes -
            Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
            Fix Version/s 2.2.6 [ 12372 ]
            Fix Version/s 2.3.3 [ 12373 ]
            Fix Version/s STABLE backlog [ 10463 ]
            timb Tim Barker made changes -
            Tester davmon
            dmonllao David Monllaó made changes -
            Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
            Hide
            dmonllao David Monllaó added a comment -

            It passes; I've been able to replicate the problem with linux in 22 (mysql) and master (postgres) the activity duplication works as expected with the patch applied

            Show
            dmonllao David Monllaó added a comment - It passes; I've been able to replicate the problem with linux in 22 (mysql) and master (postgres) the activity duplication works as expected with the patch applied
            dmonllao David Monllaó made changes -
            Status Testing in progress [ 10011 ] Tested [ 10006 ]
            Hide
            rob13 Rob added a comment -

            I just ran into error_question_match_sub_missing_in_db while importing a set of quizzes with Eloy's patch applied. This only happens in two courses. I was able to import the same quizzes into several other courses with not problem (except it took over 20 minutes). I am able to comment out the line in the matching question code that throws the error and then the import works as expected (except the slowness).

            Show
            rob13 Rob added a comment - I just ran into error_question_match_sub_missing_in_db while importing a set of quizzes with Eloy's patch applied. This only happens in two courses. I was able to import the same quizzes into several other courses with not problem (except it took over 20 minutes). I am able to comment out the line in the matching question code that throws the error and then the import works as expected (except the slowness).
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio

            This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

            Thanks!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Gutta cavat lapidem, non vi sed saepe cadendo - Ovidio This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks!
            stronk7 Eloy Lafuente (stronk7) made changes -
            Status Tested [ 10006 ] Closed [ 6 ]
            Resolution Fixed [ 1 ]
            Currently in integration Yes [ 10041 ]
            Integration date 14/Sep/12
            jmvedrine Jean-Michel Vedrine made changes -
            Link This issue has a non-specific relationship to MDL-36683 [ MDL-36683 ]
            timhunt Tim Hunt made changes -
            Link This issue is duplicated by MDL-29432 [ MDL-29432 ]
            mr-russ Russell Smith made changes -
            Link This issue has been marked as being related by MDL-40579 [ MDL-40579 ]
            mr-russ Russell Smith made changes -
            Link This issue has been marked as being related by MDL-40893 [ MDL-40893 ]

              People

              • Votes:
                7 Vote for this issue
                Watchers:
                12 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  12/Nov/12