Error after upgrading to Moodle 3.11.4+ 'Too few arguments to function restore_dbops::restore_get_question_categories()'

[dcadiou]

Hello.

After upgrading our moodle from 3.11.3+ to 3.11.4+ (Build: 20211210, $version = 2021051704.09), an error message is displayed during copies using the sharing cart.

In blocks/sharing_cart/backup/moodle2/restore_fix_missing_questions.php, one statement is

            $categories = restore_dbops::restore_get_question_categories($restoreid, $contextid);

It calls the function with 2 arguments ($restoreid and$contextid).

But in backup/util/dbops/restore_dbops.class.php, the called function expects 3 arguments ($restoreid, $contextid and $contextlevel), as shown below :

    /**
     * Return one array of question_category records for
     * a given restore operation and one restore context (question bank)
     *
     * @param string $restoreid Unique identifier of the restore operation being performed.
     * @param int $contextid Context id we want question categories to be returned.
     * @param int $contextlevel Context level we want to restrict the returned categories.
     * @return array Question categories for the given context id and level.
     */
    public static function restore_get_question_categories($restoreid, $contextid, $contextlevel) {
        global $DB;

The change seems to come from backup/util/dbops/restore_dbops.class.php which changed in very last versions of 3.11 branch, as visible in the github repository with :

git diff v3.11.4 master -- backup/util/dbops/restore_dbops.class.php

Especially this :

@@ -765,8 +766,13 @@ abstract class restore_dbops {
     /**
      * Return one array of question_category records for
      * a given restore operation and one restore context (question bank)
+     *
+     * @param string $restoreid Unique identifier of the restore operation being performed.
+     * @param int $contextid Context id we want question categories to be returned.
+     * @param int $contextlevel Context level we want to restrict the returned categories.
+     * @return array Question categories for the given context id and level.
      */
-    public static function restore_get_question_categories($restoreid, $contextid) {
+    public static function restore_get_question_categories($restoreid, $contextid, $contextlevel) {
         global $DB;

[dcadiou]

When block_sharing_cart | workaround_qtypes configurations points are all unchecked, no error message is displayed during test module copy.

[dcadiou]

The following patch solve this issue : issue_118-1.zip

[donhinkelman]

Hi dcadiou, and thank you for making the patch. My apologies for a low skill level in handling patches. If you could make a pull request, I would like to merge it. That would be easiest for me.

[dcadiou]

Hi donhinkelman.

I pushed a new commit in a fork of your repository : https://github.com/dcadiou/moodle-block_sharing_cart/commits/issue_118

The first commit in it (https://github.com/dcadiou/moodle-block_sharing_cart/commit/bf78c43af7194cde6ad001c07fdab2d556665429) contains the mentioned fix.

However, I suspect this patch to fails with Moodle versions prior to 3.11.4+ (https://github.com/moodle/moodle/commit/0de172734c1). Before, restore_get_question_categories function has 2 arguments, and since this commit it has 3 !

So with my patch, since this Moodle commit, if should avoid the 'too few arguments' error and work better, but with older Moodle, it could leads to a 'too many arguments' error !

I don't know if it exists a solution to dynamically test Moodle version to make a distinct call to restore_get_question_categories function with 2 or 3 arguments depending on the version...

Note : I made some confusion in my commit log, the mentioned Moodle commit is a Moodle 4.0 commit. For Moodle 3.11, the change appears in https://github.com/moodle/moodle/commit/0de172734c1, just after https://github.com/moodle/moodle/commit/e1ad263f906 (which is the fist with $version =' 2021051704.00'). The change has been merged in MOODLE_311_STABLE on commit https://github.com/moodle/moodle/commit/5cb47da3f1f (still $version =' 2021051704.00').

[dcadiou]

For conditional call depending on the Moodle version, maybe something like this :

if ($CFG->version >= '2021051704.00') {
   // Call with 3 arguments...
} else {
   // Call with 2 arguments...
}

The only problem would be with https://github.com/moodle/moodle/commit/e1ad263f906 which is already with $version =' 2021051704.00' but with restore_get_question_categories function still waiting 2 arguments. Next commits in the release wait 3, if I'm not wrong.

Sorry not to have a perfect solution. I tried to reproduce the problem on a local Moodle to test this last proposal, but I didn't succeed to get the involved part of the plugin get called.

[nrosenquist]

This seems to me like a good case for just creating a new MOODLE_311_STABLE branch with the three-argument function and ditching version check logic.

[acquaalta]

Hi dcadiou, and thank you for making the patch. My apologies for a low skill level in handling patches. If you could make a pull request, I would like to merge it. That would be easiest for me.

Hi @donhinkelman , Do you intend to include this fix in the official release of the plugin? At least locally (by manually changing the file on line 52) this patch/fix seems to work.

And on that note, is an official release/update is planned for this week? I wonder because I see some commits for this plugin on GitHub but nothing yet on Moodle plugins website.

Thanks a lot for this plugin :-)

[donhinkelman]

The pull request into Github has been done and titled "Sharing Cart 3.11, release 1". The plugins database release takes time to prepare but I hope to do that by Monday. Thanks for checking, and if my plugins database release is incorrectly done, please let me know.

[donhinkelman]

To enrosenquist: I am afraid I do not have sufficient knowledge to do your suggestion. I thought I have already made the official release by approving the pull request by dcadiou. If you think this would be very useful to go beyond just a merge, could you post specific instructions to teach me how to create a new stable branch. Thanks!

[frederikmillingpytlick]

I've created a task on our task board for someone at Praxis to take a look at this issue in the near future.

[ematilla]

I've created a task on our task board for someone at Praxis to take a look at this issue in the near future.

Thanks. We've tried the patch and it worked (I mean, it solved this issue). Hope it will be integrated in a update soon :)

[frederikmillingpytlick]

The fix should now be on the MOODLE_311_STABLE and MOODLE_40_STABLE branches.

I will merge it into the master branch tomorrow

[frederikmillingpytlick]

The fix has now been merged into the master branch as well, closing issue.

Awaiting @donhinkelman for a new release on Moodle.org :)