From 03b6d6313b060bb6823e8aa2242741bdfd384a1f Mon Sep 17 00:00:00 2001 From: Anupama Sarjoshi Date: Mon, 22 Apr 2024 18:10:23 +0100 Subject: [PATCH] MDL-81407 qbank_columnsortorder: Fix error displaying hiddencols When the question custom fields were deleted, if there were any hidden columns in qbank Column sort order referring those fields they were throwing 'Custom field does not exist' exception. Changes done to ignore such fields and display valid hiddencols, as these references were breaking the qbank Column sort order and Question bank pages. --- .../classes/column_manager.php | 44 +++++++++++++------ .../tests/behat/admin_settings.feature | 34 ++++++++++++++ .../tests/behat/question_bank.feature | 35 +++++++++++++++ .../classes/custom_field_column.php | 8 +++- question/bank/upgrade.txt | 5 +++ question/classes/local/bank/column_base.php | 5 ++- 6 files changed, 114 insertions(+), 17 deletions(-) diff --git a/question/bank/columnsortorder/classes/column_manager.php b/question/bank/columnsortorder/classes/column_manager.php index c731f9820fa32..a6cd3d81df2f5 100644 --- a/question/bank/columnsortorder/classes/column_manager.php +++ b/question/bank/columnsortorder/classes/column_manager.php @@ -214,15 +214,12 @@ public function get_columns(): array { * @return array */ public function get_disabled_columns(): array { + $result = $this->create_column_objects(array_keys($this->disabledcolumns)); $disabled = []; - if ($this->disabledcolumns) { - foreach (array_keys($this->disabledcolumns) as $column) { - [$classname, $columnname] = explode(column_base::ID_SEPARATOR, $column, 2); - $columnobject = $classname::from_column_name($this->get_questionbank(), $columnname); - $disabled[$column] = (object) [ - 'disabledname' => $columnobject->get_title(), - ]; - } + foreach ($result as $column => $columnobject) { + $disabled[$column] = (object) [ + 'disabledname' => $columnobject->get_title(), + ]; } return $disabled; } @@ -368,11 +365,32 @@ public function get_colsize_map(): array { * @return array */ public function get_hidden_columns(): array { - return array_reduce($this->hiddencolumns, function($result, $hiddencolumn) { - [$columnclass, $columnname] = explode(column_base::ID_SEPARATOR, $hiddencolumn, 2); - $result[$hiddencolumn] = $columnclass::from_column_name($this->get_questionbank(), $columnname)->get_title(); - return $result; - }, []); + $result = $this->create_column_objects($this->hiddencolumns); + $hidden = []; + foreach ($result as $column => $columnobject) { + $hidden[$column] = $columnobject->get_title(); + } + return $hidden; + } + + /** + * Returns an array of column objects. + * + * @param array $columnsnames Array of columns. + * @return column_base[] Array of $columnsname => $columnobject + */ + public function create_column_objects(array $columnsnames): array { + $result = []; + foreach ($columnsnames as $column) { + [$columnclass, $columnname] = explode(column_base::ID_SEPARATOR, $column, 2); + if (class_exists($columnclass)) { + $columnobject = $columnclass::from_column_name($this->get_questionbank(), $columnname, true); + if ($columnobject != null) { + $result[$column] = $columnobject; + } + } + } + return $result; } public function get_column_width(column_base $column): string { diff --git a/question/bank/columnsortorder/tests/behat/admin_settings.feature b/question/bank/columnsortorder/tests/behat/admin_settings.feature index affcb047c324f..0be07b3494743 100644 --- a/question/bank/columnsortorder/tests/behat/admin_settings.feature +++ b/question/bank/columnsortorder/tests/behat/admin_settings.feature @@ -116,3 +116,37 @@ Feature: Set default question bank column order and size Then "Last used" "table_row" should exist And "Question" "table_row" should appear before "Comments" "table_row" And the field "Width of 'Question' in pixels" matches value "" + + Scenario: Deleting a custom field which is removed from the Column sort order + Given the following "custom field categories" exist: + | name | component | area | itemid | + | Category for test | qbank_customfields | question | 0 | + And the following "custom fields" exist: + | name | category | type | shortname | configdata | + | Field 1 | Category for test | text | f1 | {"visibility":"2"} | + | Field 2 | Category for test | text | f2 | {"visibility":"2"} | + And I change the window size to "large" + When I log in as "admin" + And I navigate to "Plugins > Question bank plugins > Column sort order" in site administration + And "Field 1" "table_row" should exist + And "Field 2" "table_row" should exist + And I click on "Actions menu" "link" in the "Field 1" "table_row" + And I choose "Remove" in the open action menu + + # Delete a question custom field. + And I navigate to "Plugins > Question bank plugins > Question custom fields" in site administration + And I click on "Delete" "link" in the "Field 1" "table_row" + And I click on "Yes" "button" in the "Confirm" "dialogue" + And I navigate to "Plugins > Question bank plugins > Column sort order" in site administration + Then I should see "Column sort order" + And "Field 2" "table_row" should exist + And I click on "Actions menu" "link" in the "Field 2" "table_row" + And I choose "Remove" in the open action menu + + # Delete the question custom category. + And I navigate to "Plugins > Question bank plugins > Question custom fields" in site administration + And I click on "[data-role='deletecategory']" "css_element" + And I click on "Yes" "button" in the "Confirm" "dialogue" + And I should not see "Category for test" in the "#customfield_catlist" "css_element" + And I navigate to "Plugins > Question bank plugins > Column sort order" in site administration + And I should see "Column sort order" diff --git a/question/bank/columnsortorder/tests/behat/question_bank.feature b/question/bank/columnsortorder/tests/behat/question_bank.feature index 93269279b146a..fed6ab56be434 100644 --- a/question/bank/columnsortorder/tests/behat/question_bank.feature +++ b/question/bank/columnsortorder/tests/behat/question_bank.feature @@ -141,3 +141,38 @@ Feature: Set question bank column order and size Then I should see "Question bank" And "Create a new question" "button" should exist # Really, we are just checking the question bank displayed without errors. + + Scenario: Deleting a custom field which a user had removed from his preferences + Given the following "custom field categories" exist: + | name | component | area | itemid | + | Category for test | qbank_customfields | question | 0 | + And the following "custom fields" exist: + | name | category | type | shortname | configdata | + | Field 1 | Category for test | text | f1 | {"visibility":"2"} | + | Field 2 | Category for test | text | f2 | {"visibility":"2"} | + And I am on the "Test quiz Q001" "mod_quiz > question bank" page logged in as "teacher1" + And I apply question bank filter "Category" with value "Question category 1" + And I click on "Actions menu" "link" in the "Field 1" "qbank_columnsortorder > column header" + And I choose "Remove" in the open action menu + And I click on "Actions menu" "link" in the "Field 2" "qbank_columnsortorder > column header" + And I choose "Remove" in the open action menu + And "Field 2" "qbank_columnsortorder > column header" should not exist + + # Delete a question custom field. + And I log in as "admin" + And I navigate to "Plugins > Question bank plugins > Question custom fields" in site administration + And I click on "Delete" "link" in the "Field 1" "table_row" + And I click on "Yes" "button" in the "Confirm" "dialogue" + And I am on the "Test quiz Q001" "mod_quiz > question bank" page logged in as "teacher1" + And I apply question bank filter "Category" with value "Question category 1" + Then I should see "Question bank" + + # Delete the question custom category. + And I log in as "admin" + And I navigate to "Plugins > Question bank plugins > Question custom fields" in site administration + And I click on "[data-role='deletecategory']" "css_element" + And I click on "Yes" "button" in the "Confirm" "dialogue" + And I should not see "Category for test" in the "#customfield_catlist" "css_element" + And I am on the "Test quiz Q001" "mod_quiz > question bank" page logged in as "teacher1" + And I apply question bank filter "Category" with value "Question category 1" + And I should see "Question bank" diff --git a/question/bank/customfields/classes/custom_field_column.php b/question/bank/customfields/classes/custom_field_column.php index 6a21e10b77e3d..3785018b8417b 100644 --- a/question/bank/customfields/classes/custom_field_column.php +++ b/question/bank/customfields/classes/custom_field_column.php @@ -44,14 +44,18 @@ public function __construct(\core_question\local\bank\view $qbank, \core_customf $this->field = $field; } - public static function from_column_name(view $view, string $columnname): custom_field_column { + public static function from_column_name(view $view, string $columnname, bool $ingoremissing = false): ?custom_field_column { $handler = question_handler::create(); foreach ($handler->get_fields() as $field) { if ($field->get('shortname') == $columnname) { return new static($view, $field); } } - throw new \coding_exception('Custom field ' . $columnname . ' does not exist.'); + if ($ingoremissing) { + return null; + } else { + throw new \coding_exception('Custom field ' . $columnname . ' does not exist.'); + } } /** diff --git a/question/bank/upgrade.txt b/question/bank/upgrade.txt index 0b5955d64ffff..7a105eb8210b8 100644 --- a/question/bank/upgrade.txt +++ b/question/bank/upgrade.txt @@ -1,6 +1,11 @@ This file describes core qbank plugin changes in /question/bank/*, information provided here is intended especially for developers. +=== 4.3.5 === + +* column_base::from_column_name now has an ignoremissing field, which can be used to ignore + if the class does not exist, instead of throwing an exception. + === 4.3.4 === * Question bank actions (anything that subclasses question_action_base) should implement the diff --git a/question/classes/local/bank/column_base.php b/question/classes/local/bank/column_base.php index cc122865de3b3..c4abe2dc8a0e3 100644 --- a/question/classes/local/bank/column_base.php +++ b/question/classes/local/bank/column_base.php @@ -57,9 +57,10 @@ abstract class column_base extends view_component { * * @param view $view Question bank view * @param string $columnname The column name for this instance, as returned by {@see get_column_name()} - * @return column_base An instance of this class. + * @param bool $ingoremissing Whether to ignore if the class does not exist. + * @return column_base|null An instance of this class. */ - public static function from_column_name(view $view, string $columnname): column_base { + public static function from_column_name(view $view, string $columnname, bool $ingoremissing = false): ?column_base { return new static($view); }