From 2569fb5d2cb61f765071246d9776bb48f14a45df Mon Sep 17 00:00:00 2001
From: Tim Hunt <T.J.Hunt@open.ac.uk>
Date: Thu, 30 Jan 2020 17:24:23 +0000
Subject: [PATCH] MDL-67842 questions: Cannot remove the idnumber from a
 question

---
 question/tests/behat/edit_questions.feature | 13 +++++++++--
 question/type/questiontypebase.php          | 24 +++++++++++++--------
 2 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/question/tests/behat/edit_questions.feature b/question/tests/behat/edit_questions.feature
index 183d0c10e07..58d020016e4 100644
--- a/question/tests/behat/edit_questions.feature
+++ b/question/tests/behat/edit_questions.feature
@@ -24,7 +24,6 @@ Feature: A teacher can edit questions in the question bank
     And I am on "Course 1" course homepage
     And I navigate to "Question bank > Questions" in current page administration
 
-  @javascript
   Scenario: Edit a previously created question
     When I choose "Edit question" action for "Test question to be edited" in the question bank
     And I set the following fields to these values:
@@ -36,7 +35,6 @@ Feature: A teacher can edit questions in the question bank
     And "Edited question name" row "Created by" column of "categoryquestions" table should contain "Admin User"
     And "Edited question name" row "Last modified by" column of "categoryquestions" table should contain "Teacher 1"
 
-  @javascript
   Scenario: Editing a question can be cancelled
     When I choose "Edit question" action for "Test question to be edited" in the question bank
     And I set the field "Question name" to "Edited question name"
@@ -44,3 +42,14 @@ Feature: A teacher can edit questions in the question bank
     Then I should see "Test question to be edited"
     And "Test question to be edited" row "Created by" column of "categoryquestions" table should contain "Admin User"
     And "Test question to be edited" row "Last modified by" column of "categoryquestions" table should contain "Admin User"
+
+  Scenario: A question can have its idnumber removed
+    Given the following "questions" exist:
+      | questioncategory | qtype | name                   | idnumber |
+      | Test questions   | essay | Question with idnumber | frog     |
+    And I reload the page
+    Then I should see "frog" in the "Question with idnumber" "table_row"
+    When I choose "Edit question" action for "Question with idnumber" in the question bank
+    And I set the field "ID number" to ""
+    And I press "id_submitbutton"
+    Then I should not see "frog" in the "Question with idnumber" "table_row"
diff --git a/question/type/questiontypebase.php b/question/type/questiontypebase.php
index 9b54465d055..5891d2ae4f5 100644
--- a/question/type/questiontypebase.php
+++ b/question/type/questiontypebase.php
@@ -371,16 +371,22 @@ class question_type {
             $question->defaultmark = $form->defaultmark;
         }
 
-        if (isset($form->idnumber) && ((string) $form->idnumber !== '')) {
-            // While this check already exists in the form validation, this is a backstop preventing unnecessary errors.
-            if (strpos($form->category, ',') !== false) {
-                list($category, $categorycontextid) = explode(',', $form->category);
+        if (isset($form->idnumber)) {
+            if ((string) $form->idnumber === '') {
+                $question->idnumber = null;
             } else {
-                $category = $form->category;
-            }
-            if (!$DB->record_exists('question',
-                    ['idnumber' => $form->idnumber, 'category' => $category])) {
-                $question->idnumber = $form->idnumber;
+                // While this check already exists in the form validation,
+                // this is a backstop preventing unnecessary errors.
+                // Only set the idnumber if it has changed and will not cause a unique index violation.
+                if (strpos($form->category, ',') !== false) {
+                    list($category, $categorycontextid) = explode(',', $form->category);
+                } else {
+                    $category = $form->category;
+                }
+                if (!$DB->record_exists('question',
+                        ['idnumber' => $form->idnumber, 'category' => $category])) {
+                    $question->idnumber = $form->idnumber;
+                }
             }
         }