MDL-67934 questions: give a sensible default idnumber when duplicating

lib/questionlib.php

@@ -2372,3 +2372,38 @@ function question_module_uses_questions($modname) {
 
     return false;
 }
+
+/**
+ * If $oldidnumber ends in some digits then return the next available idnumber of the same form.
+ *
+ * So idnum -> null (no digits at the end) idnum0099 -> idnum0100 (if that is unused,
+ * else whichever of idnum0101, idnume0102, ... is unused. idnum9 -> idnum10.
+ *
+ * @param string $oldidnumber a question idnumber.
+ * @param int $categoryid a question category id.
+ * @return string|null suggested new idnumber for a question in that category, or null if one cannot be found.
+ */
+function core_question_find_next_unused_idnumber(string $oldidnumber, int $categoryid):? string {
+    global $DB;
+
+    // The the old idnumber is not of the right form, bail now.
+    if (!preg_match('~\d+$~', $oldidnumber, $matches)) {
+        return null;
+    }
+
+    // Find all used idnumbers in one DB query.
+    $usedidnumbers = $DB->get_records_select_menu('question', 'category = ? AND idnumber IS NOT NULL',
+            [$categoryid], '', 'idnumber, 1');
+
+    // Find the next unused idnumber.
+    $newidnumber = $oldidnumber;
+    do {
+        // If we have got to something9999, insert an extra digit before incrementing.
+        if (preg_match('~^(.*[^0-9])(9+)$~', $newidnumber, $matches)) {
+            $newidnumber = $matches[1] . '0' . $matches[2];
+        }
+        $newidnumber++;
+    } while (isset($usedidnumbers[$newidnumber]));
+
+    return (string) $newidnumber;
+}

lib/tests/questionlib_test.php

@@ -2092,4 +2092,47 @@ class core_questionlib_testcase extends advanced_testcase {
                 ['id' => $systemq->id, 'courseid' => SITEID, 'sesskey' => sesskey()]),
                 question_get_export_single_question_url(question_bank::load_question($systemq->id)));
     }
+
+    /**
+     * Get test cases for test_core_question_find_next_unused_idnumber.
+     *
+     * @return array test cases.
+     */
+    public function find_next_unused_idnumber_cases(): array {
+        return [
+            ['id', null],
+            ['id1a', null],
+            ['id001', 'id002'],
+            ['id9', 'id10'],
+            ['id009', 'id010'],
+            ['id999', 'id1000'],
+        ];
+    }
+
+    /**
+     * Test core_question_find_next_unused_idnumber in the case when there are no other questions.
+     *
+     * @dataProvider find_next_unused_idnumber_cases
+     * @param string $oldidnumber value to pass to core_question_find_next_unused_idnumber.
+     * @param string|null $expectednewidnumber expected result.
+     */
+    public function test_core_question_find_next_unused_idnumber(string $oldidnumber, ?string $expectednewidnumber) {
+        $this->assertSame($expectednewidnumber, core_question_find_next_unused_idnumber($oldidnumber, 0));
+    }
+
+    public function test_core_question_find_next_unused_idnumber_skips_used() {
+        $this->resetAfterTest();
+
+        /** @var core_question_generator $generator */
+        $generator = $this->getDataGenerator()->get_plugin_generator('core_question');
+        $category = $generator->create_question_category();
+        $othercategory = $generator->create_question_category();
+        $generator->create_question('truefalse', null, ['category' => $category->id, 'idnumber' => 'id9']);
+        $generator->create_question('truefalse', null, ['category' => $category->id, 'idnumber' => 'id10']);
+        // Next one to make sure only idnumbers from the right category are ruled out.
+        $generator->create_question('truefalse', null, ['category' => $othercategory->id, 'idnumber' => 'id11']);
+
+        $this->assertSame('id11', core_question_find_next_unused_idnumber('id9', $category->id));
+        $this->assertSame('id11', core_question_find_next_unused_idnumber('id8', $category->id));
+    }
 }

question/question.php

@@ -178,7 +178,7 @@ if ($id) {
     if ($makecopy) {
         // If we are duplicating a question, add some indication to the question name.
         $question->name = get_string('questionnamecopy', 'question', $question->name);
-        $question->idnumber = null;
+        $question->idnumber = core_question_find_next_unused_idnumber($question->idnumber, $category->id);
         $question->beingcopied = true;
     }