Moodle
  1. Moodle
  2. MDL-37161

Import/Restore : Database error caused by course grade book settings

    Details

    • Testing Instructions:
      Hide
      1. Create 2 new courses source and destination for the importation
      2. In each course, go in the course grade settings and modify the "Grade display type" element to something different than the default. Click to save changes.
      3. In the source course, create an assignment activity.
      4. Process the importation from source course to destination course.
        You should see that the importation is successful and that the "Grade display type" of the destination course is the same as before (Before the patch you would see a database error)
      1. Create 2 new courses source and destination for another importation
      2. Do the same operations as before but without modifying the "Grade display type" of the destination course.
        You should see that the importation is successful and that the "Grade display type" of the destination course is the one from the source course
      Show
      Create 2 new courses source and destination for the importation In each course, go in the course grade settings and modify the "Grade display type" element to something different than the default. Click to save changes. In the source course, create an assignment activity. Process the importation from source course to destination course. You should see that the importation is successful and that the "Grade display type" of the destination course is the same as before (Before the patch you would see a database error) Create 2 new courses source and destination for another importation Do the same operations as before but without modifying the "Grade display type" of the destination course. You should see that the importation is successful and that the "Grade display type" of the destination course is the one from the source course
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull 2.4 Branch:
      MDL-37161-moodle24
    • Pull Master Branch:
      MDL-37161-master
    • Rank:
      46734

      Description

      A Database error is displayed when importating or restorating into a course if both source and destination have informations (Display type, Range decimal points...) in grade book settings.

      This error breaks the content of the targeted course :

      • Imported or restored activities and resources not visible but exists in course.
      • Grade book displays "error" message in users grades related to the activities imported/restored.

        Activity

        Hide
        Michael de Raadt added a comment -

        Thanks for reporting that and providing a patch. Hopefully we can get that reviewed soon.

        Show
        Michael de Raadt added a comment - Thanks for reporting that and providing a patch. Hopefully we can get that reviewed soon.
        Hide
        Andrew Davis added a comment -

        [Y] Syntax
        [NA] Output
        [Y] Whitespace
        [NA] Language
        [Y] Databases
        [Y] Testing
        [Y] Security
        [NA] Documentation
        [Y] Git
        [Y] Sanity check

        Looks good. The only minor point is I would amend the testing instructions to say that you need a new course for the 2nd part of the testing (the second 1.) If I'm understanding this correctly, if you dont start with a new course the grade_settings record will already exist and the test will fail.

        Otherwise, feel free to submit this for integration whenever you are ready.

        Show
        Andrew Davis added a comment - [Y] Syntax [NA] Output [Y] Whitespace [NA] Language [Y] Databases [Y] Testing [Y] Security [NA] Documentation [Y] Git [Y] Sanity check Looks good. The only minor point is I would amend the testing instructions to say that you need a new course for the 2nd part of the testing (the second 1.) If I'm understanding this correctly, if you dont start with a new course the grade_settings record will already exist and the test will fail. Otherwise, feel free to submit this for integration whenever you are ready.
        Hide
        Andrew Davis added a comment -

        Sorry, one other minor thing. In the array please add a space either side of the arrow thingy ie this 'courseid'=>$data->courseid should be 'courseid' => $data->courseid

        Show
        Andrew Davis added a comment - Sorry, one other minor thing. In the array please add a space either side of the arrow thingy ie this 'courseid'=>$data->courseid should be 'courseid' => $data->courseid
        Hide
        Jean-Philippe Gaudreau added a comment -

        Hi Andrew,

        I've modified the minor points in code and testing instructions. Submitting for integration review.

        Thx!

        Show
        Jean-Philippe Gaudreau added a comment - Hi Andrew, I've modified the minor points in code and testing instructions. Submitting for integration review. Thx!
        Hide
        Jean-Philippe Gaudreau added a comment -

        Sorry for the newbie question but how can I put the task in "integration review" state ? Thx

        Show
        Jean-Philippe Gaudreau added a comment - Sorry for the newbie question but how can I put the task in "integration review" state ? Thx
        Hide
        Andrew Davis added a comment -

        Its possible you don't have the required capability. If you do there will be a "submit for integration" button next to the button to submit an issue for peer review. I have now submitted it for you

        Show
        Andrew Davis added a comment - Its possible you don't have the required capability. If you do there will be a "submit for integration" button next to the button to submit an issue for peer review. I have now submitted it for you
        Hide
        Sam Hemelryk added a comment -

        Thanks guys, this has been integrated now

        Show
        Sam Hemelryk added a comment - Thanks guys, this has been integrated now
        Hide
        Adrian Greeve added a comment -

        No errors displayed.
        (Checked with a non-integration branch to view the error)
        Test passed.

        Show
        Adrian Greeve added a comment - No errors displayed. (Checked with a non-integration branch to view the error) Test passed.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        And your fantastic code has met core, hope they become good friends for a long period.

        Closing, thanks!

        Show
        Eloy Lafuente (stronk7) added a comment - And your fantastic code has met core, hope they become good friends for a long period. Closing, thanks!

          People

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

            Dates

            • Created:
              Updated:
              Resolved: