Moodle

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
  • Pull Master Diff URL:

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

Ascending order - Click to sort in descending order
Hide
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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
Permalink
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

Vote (0)
Watch (2)

Dates

  • Created:
    Updated:
    Resolved: