Moodle
  1. Moodle
  2. MDL-34837

Restoring course throws an exception

    Details

    • Testing Instructions:
      Hide

      Test pre-requisites

      • Create 2 courses with the exact same name (not short name)
      • Backup one of them

      Test steps

      1. Go to the restore page of the backed up course.
      2. Restore it as a new course
      3. Make sure no errors or exceptions occur
      Show
      Test pre-requisites Create 2 courses with the exact same name (not short name) Backup one of them Test steps Go to the restore page of the backed up course. Restore it as a new course Make sure no errors or exceptions occur
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-34837-master
    • Rank:
      43339

      Description

      There is an error in method for generating new temporary name for a restored course. Method is located in
      <moodle>/backup/util/dbops/restore_dbops.class.php

      restore_dbops::calculate_course_names
      

      That method uses

      $DB->get_record_select
      

      Which throws exception if query returns more than one record and debugging is turned on. It does not break anything but it annoys suspicious users(clients).

      Quick fix would be to change these lines:

      $coursefull  = $DB->get_record_select('course', 'fullname = ? AND id != ?', array($currentfullname, $courseid));
      $courseshort = $DB->get_record_select('course', 'shortname = ? AND id != ?', array($currentshortname, $courseid));
      

      To:

      $coursefull  = $DB->get_record_select('course', 'fullname = ? AND id != ?', array($currentfullname, $courseid), IGNORE_MULTIPLE);
      $courseshort = $DB->get_record_select('course', 'shortname = ? AND id != ?', array($currentshortname, $courseid), IGNORE_MULTIPLE);
      

        Activity

        Hide
        A G Morris added a comment -

        Is this related to: Default exception handler: A required parameter (id) was missing Debug: \n* line 435 of /lib/setuplib.php: moodle_exception thrown\n* line 504 of /lib/moodlelib.php: call to print_error()\n* line 30 of /mod/quiz/index.php: call to required_param()\n

        Show
        A G Morris added a comment - Is this related to: Default exception handler: A required parameter (id) was missing Debug: \n* line 435 of /lib/setuplib.php: moodle_exception thrown\n* line 504 of /lib/moodlelib.php: call to print_error()\n* line 30 of /mod/quiz/index.php: call to required_param()\n
        Hide
        Darko Miletic added a comment -

        No. The error you are mentioning occurs when somebody call's a quiz index.php without passing a required parameter id.

        Show
        Darko Miletic added a comment - No. The error you are mentioning occurs when somebody call's a quiz index.php without passing a required parameter id.
        Hide
        Mark Nielsen added a comment -

        Attaching a patch with a proposed solution. Darko, changing the DB call on courseshort shouldn't be necessary since it should be unique for all courses. Please correct me if you found that not to be the case.

        Show
        Mark Nielsen added a comment - Attaching a patch with a proposed solution. Darko, changing the DB call on courseshort shouldn't be necessary since it should be unique for all courses. Please correct me if you found that not to be the case.
        Hide
        Darko Miletic added a comment -

        Although Moodle enforces uniquenes of course short name in the code in the database there is no such constraint, hence duplicates are possible. I even asked about that in Moodle dev. forum
        http://moodle.org/mod/forum/discuss.php?d=208684

        Show
        Darko Miletic added a comment - Although Moodle enforces uniquenes of course short name in the code in the database there is no such constraint, hence duplicates are possible. I even asked about that in Moodle dev. forum http://moodle.org/mod/forum/discuss.php?d=208684
        Hide
        Michael de Raadt added a comment -

        Thanks for reporting this, guys. There has been some recent work in the area. Hopefully we can get this sorted out soon.

        Show
        Michael de Raadt added a comment - Thanks for reporting this, guys. There has been some recent work in the area. Hopefully we can get this sorted out soon.
        Hide
        Frédéric Massart added a comment -

        Thanks for providing a patch for this. I have given credit as author to you Mark. Cheers!

        Show
        Frédéric Massart added a comment - Thanks for providing a patch for this. I have given credit as author to you Mark. Cheers!
        Hide
        Ankit Agarwal added a comment -

        Hi Fred,
        This looks good +1 for integration
        Reviewed list:
        [y] Syntax
        [na] Output
        [y] Whitespace
        [na] Language
        [y] Databases
        [y] Testing
        [na] Security
        [na] Documentation
        [y] Git
        [y] Sanity check
        Thanks

        Show
        Ankit Agarwal added a comment - Hi Fred, This looks good +1 for integration Reviewed list: [y] Syntax [na] Output [y] Whitespace [na] Language [y] Databases [y] Testing [na] Security [na] Documentation [y] Git [y] Sanity check Thanks
        Hide
        Frédéric Massart added a comment -

        Thanks Ankit! Pushing for integration.

        Show
        Frédéric Massart added a comment - Thanks Ankit! Pushing for integration.
        Hide
        Aparup Banerjee 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
        Aparup Banerjee 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
        Hide
        Dan Poltawski added a comment -

        Integrated to 22, 23 and master.

        Thanks guys!

        Show
        Dan Poltawski added a comment - Integrated to 22, 23 and master. Thanks guys!
        Hide
        Adrian Greeve added a comment -

        Tested on master first to replicate the error and then tested on the 2.2, 2.3 and master integration branches.
        No errors are being displayed.
        Test passed.

        Show
        Adrian Greeve added a comment - Tested on master first to replicate the error and then tested on the 2.2, 2.3 and master integration branches. No errors are being displayed. Test passed.
        Hide
        Dan Poltawski added a comment -

        Hurray!

        You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

        Show
        Dan Poltawski added a comment - Hurray! You did it, congratulations! You have on Mojito credit to redeem after the release of Moodle 2.4

          People

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

            Dates

            • Created:
              Updated:
              Resolved: