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

Update qtype_match tables to match the coding guidelines

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 2.4.1
    • Fix Version/s: 2.5
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      You need to test all aspects of match questions.

      1. Create, edit and delete match questions.

      2. Preview a match question.

      3. Export match questions as Moodle XML, and re-import and check that all the options are correctly preserved.

      4. Backup and restore a course with match questions, and check that all the settings are correctly preserved.

      5. In a Moodle 2.0 install, create a quiz including some match questions. Attempt that quiz with some student accounts. Backup the course including user data, and restore it to your master install. Verify the question definitions and attempt data is transferred correctly.

      6. In am Moodle 1.9 install, create a quiz including some match questions. Back up that course, and restore it into your master install. Verify that the question definitions are transferred correctly.

      Show
      You need to test all aspects of match questions. 1. Create, edit and delete match questions. 2. Preview a match question. 3. Export match questions as Moodle XML, and re-import and check that all the options are correctly preserved. 4. Backup and restore a course with match questions, and check that all the settings are correctly preserved. 5. In a Moodle 2.0 install, create a quiz including some match questions. Attempt that quiz with some student accounts. Backup the course including user data, and restore it to your master install. Verify the question definitions and attempt data is transferred correctly. 6. In am Moodle 1.9 install, create a quiz including some match questions. Back up that course, and restore it into your master install. Verify that the question definitions are transferred correctly.
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull Master Branch:

      Description

      • Rename table question_match -> qtype_match_options.
      • Rename table question_match_sub -> qtype_match_subquestions.
      • Rename column question -> questionid in both tables.
      • Since we have to drop and re-create the foreign key, make qtype_match_options.questionid a foreign-unique key.
      • Drop the unnecessary column qtype_match_options.subquestions.
      • Drop the unnecessary column qtype_match_subquestions.code.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              timhunt Tim Hunt added a comment -

              MDL-17812 is the equivalent issue for shortanswer.

              Show
              timhunt Tim Hunt added a comment - MDL-17812 is the equivalent issue for shortanswer.
              Hide
              timhunt Tim Hunt added a comment -

              Yay! hideous testing instructions complete, and it works for me. Now for peer review.

              Show
              timhunt Tim Hunt added a comment - Yay! hideous testing instructions complete, and it works for me. Now for peer review.
              Hide
              timhunt Tim Hunt added a comment -

              Also, separate commit added to fix all the code-checker issues.

              Show
              timhunt Tim Hunt added a comment - Also, separate commit added to fix all the code-checker issues.
              Hide
              fred Frédéric Massart added a comment -

              Hi Tim, I had a look through and the code looks good. For some reason I decided to run the Unit Tests, and they failed, here is the error:

              There was 1 failure:
               
              1) qtype_match_attempt_upgrader_test::test_match_deferredfeedback_history622220
              Failed asserting that two objects are equal.
              --- Expected
              +++ Actual
              @@ @@
               stdClass Object (
                   'behaviour' => 'deferredfeedback'
              -    'questionid' => 11135
              +    'questionid' => '11135'
                   'variant' => 1
              -    'maxmark' => 1
              +    'maxmark' => '1'
                   'minfraction' => 0
                   'flagged' => 0
                   'questionsummary' => 'Whichofthefollowingstatements...False}'
                   'rightanswer' => 'Subject gateways provide link...> True'
              -    'responsesummary' => 'Subjectgatewaysoffermorevarietythansearchengines->False;Subjectgatewayscanprovideamoredirectroutetowebsitescontainingacademiccontent->True;Subjectgatewaysindexwebsitesautomatically->True;Subjectgatewaysprovidelinkstositesthathavebeenqualitychecked->True'
              -    'timemodified' => 1200507467
              +    'responsesummary' => 'Subjectgatewaysoffermorevarietythansearchengines->True;Subjectgatewayscanprovideamoredirectroutetowebsitescontainingacademiccontent->True;Subjectgatewaysindexwebsitesautomatically->True;Subjectgatewaysprovidelinkstositesthathavebeenqualitychecked->True'
              +    'timemodified' => '1200507467'
               
              @@ @@
                           'fraction' => 0.75
              -            'timecreated' => 1200507467
              -            'userid' => 6584
              +            'timecreated' => '1200507467'
              +            'userid' => '6584'
                           'data' => Array (
              -                'sub0' => 1
              +                'sub0' => 2
                               'sub1' => 2
                               'sub2' => 2
                               'sub3' => 2
              -                '-finish' => 1
              +                '-finish' => '1'
                           )
                       )
                   )
               )
               
              /home/fred/www/repositories/sm/moodle/question/engine/upgrade/tests/helper.php:139
              /home/fred/www/repositories/sm/moodle/question/type/match/tests/upgradelibnewqe_test.php:818
              /home/fred/www/repositories/sm/moodle/lib/phpunit/classes/advanced_testcase.php:76
              /usr/bin/phpunit:46
              

              Would that have some value to try to upgrade from 2.4 to 2.5 during testing?

              Show
              fred Frédéric Massart added a comment - Hi Tim, I had a look through and the code looks good. For some reason I decided to run the Unit Tests, and they failed, here is the error: There was 1 failure:   1) qtype_match_attempt_upgrader_test::test_match_deferredfeedback_history622220 Failed asserting that two objects are equal. --- Expected +++ Actual @@ @@ stdClass Object ( 'behaviour' => 'deferredfeedback' - 'questionid' => 11135 + 'questionid' => '11135' 'variant' => 1 - 'maxmark' => 1 + 'maxmark' => '1' 'minfraction' => 0 'flagged' => 0 'questionsummary' => 'Whichofthefollowingstatements...False}' 'rightanswer' => 'Subject gateways provide link...> True' - 'responsesummary' => 'Subjectgatewaysoffermorevarietythansearchengines->False;Subjectgatewayscanprovideamoredirectroutetowebsitescontainingacademiccontent->True;Subjectgatewaysindexwebsitesautomatically->True;Subjectgatewaysprovidelinkstositesthathavebeenqualitychecked->True' - 'timemodified' => 1200507467 + 'responsesummary' => 'Subjectgatewaysoffermorevarietythansearchengines->True;Subjectgatewayscanprovideamoredirectroutetowebsitescontainingacademiccontent->True;Subjectgatewaysindexwebsitesautomatically->True;Subjectgatewaysprovidelinkstositesthathavebeenqualitychecked->True' + 'timemodified' => '1200507467'   @@ @@ 'fraction' => 0.75 - 'timecreated' => 1200507467 - 'userid' => 6584 + 'timecreated' => '1200507467' + 'userid' => '6584' 'data' => Array ( - 'sub0' => 1 + 'sub0' => 2 'sub1' => 2 'sub2' => 2 'sub3' => 2 - '-finish' => 1 + '-finish' => '1' ) ) ) )   /home/fred/www/repositories/sm/moodle/question/engine/upgrade/tests/helper.php:139 /home/fred/www/repositories/sm/moodle/question/type/match/tests/upgradelibnewqe_test.php:818 /home/fred/www/repositories/sm/moodle/lib/phpunit/classes/advanced_testcase.php:76 /usr/bin/phpunit:46 Would that have some value to try to upgrade from 2.4 to 2.5 during testing?
              Hide
              timhunt Tim Hunt added a comment -

              Thanks for the review Fred.

              That is an odd unit test failure. I will look into it.

              The strange thing is, I have not seen that when I run the tests. Weird. It looks like PHPunit has started considering strings and ints different.

              I don't think it is worth testing 2.4 -> 2.5 upgrade separately. Testing master last week -> master this week will be just the same as far as the qtype_match plugin is concerned, and many other people will test 2.4 -> 2.5 before the 2.5 release, so no need to waste tester time now.

              Show
              timhunt Tim Hunt added a comment - Thanks for the review Fred. That is an odd unit test failure. I will look into it. The strange thing is, I have not seen that when I run the tests. Weird. It looks like PHPunit has started considering strings and ints different. I don't think it is worth testing 2.4 -> 2.5 upgrade separately. Testing master last week -> master this week will be just the same as far as the qtype_match plugin is concerned, and many other people will test 2.4 -> 2.5 before the 2.5 release, so no need to waste tester time now.
              Hide
              timhunt Tim Hunt added a comment -

              Acutally, there was one value in the unit test that was wrong. Strangely, when one int did not match, then PHPunit showed lots of other irrelevant int/string differences, but once the test passed, those were not shown.

              Test fixed. Submitting for integration now.

              Show
              timhunt Tim Hunt added a comment - Acutally, there was one value in the unit test that was wrong. Strangely, when one int did not match, then PHPunit showed lots of other irrelevant int/string differences, but once the test passed, those were not shown. Test fixed. Submitting for integration now.
              Hide
              poltawski Dan Poltawski added a comment -

              Integrated to master, thanks Tim!

              Show
              poltawski Dan Poltawski added a comment - Integrated to master, thanks Tim!
              Hide
              poltawski Dan Poltawski added a comment -

              CI server currently says:

              Table question_match only available in first DB
              Table question_match_sub only available in first DB
              Table qtype_match_options only available in second DB
              Table qtype_match_subquestions only available in second DB

              Show
              poltawski Dan Poltawski added a comment - CI server currently says: Table question_match only available in first DB Table question_match_sub only available in first DB Table qtype_match_options only available in second DB Table qtype_match_subquestions only available in second DB
              Hide
              poltawski Dan Poltawski added a comment -

              It resolved itself. I've created an issue to work out whats going wrong with the CI server: MDLSITE-2100

              Show
              poltawski Dan Poltawski added a comment - It resolved itself. I've created an issue to work out whats going wrong with the CI server: MDLSITE-2100
              Hide
              timhunt Tim Hunt added a comment -

              Remember that exactly the same transient failure happened last week with MDL-17812.

              Show
              timhunt Tim Hunt added a comment - Remember that exactly the same transient failure happened last week with MDL-17812 .
              Hide
              poltawski Dan Poltawski added a comment -

              Tim, yep - I know, thats why I created MDLSITE-2100

              Show
              poltawski Dan Poltawski added a comment - Tim, yep - I know, thats why I created MDLSITE-2100
              Hide
              dmonllao David Monllaó added a comment -

              It passes, tested only in master, all the values of the match questions are still there

              Show
              dmonllao David Monllaó added a comment - It passes, tested only in master, all the values of the match questions are still there
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed.

              (and won't be revisiting it unless some regression is found)

              Thanks and ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - A brilliant future is awaiting us out there, better with your code. Let's look towards the future together, this is now closed. (and won't be revisiting it unless some regression is found) Thanks and ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    14/May/13