L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-74054 core_question: Define unique question bank column IDs

Browse Source 
This also resolves MDL-78829.
Some question bank plugins use a separate class for each plugin they
define. However, qbank_customfields (and potentially others in the
future) uses a single class to define multiple fields. Using the class
name as an ID for the column doesn't give us a way of reliable
instantiating an object for the column. Previously, qbank_customfields
appended the field name as though it was a namespaced class, but this
had to be manually constructed and deconstructed by detecting this
particular column class.

This change introduces a standard way of constructing a unique ID for
each question bank column, in the form
pluginname\columnclass-columnname. This ensures that the ID will be
unique for each column, and the ID can be used to instatiate the
column's object.
This commit is contained in:
Mark Johnson
2023-09-22 10:53:51 +08:00
committed by Andrew Nicols
parent 2be0e10a80
commit 6001ee3dfd
6 changed files with 101 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files
question/bank/columnsortorder/classes/column_manager.php
+18 -43
View File
@@ -21,7 +21,6 @@ defined('MOODLE_INTERNAL') || die();
require_once($CFG->libdir . '/questionlib.php');
use context_system;
use core_question\local\bank\column_action_base;
use core_question\local\bank\column_base;
use core_question\local\bank\column_manager_base;
use core_question\local\bank\question_edit_contexts;
@@ -29,6 +28,7 @@ use core_question\local\bank\view;
use qbank_columnsortorder\local\bank\column_action_move;
use qbank_columnsortorder\local\bank\column_action_remove;
use qbank_columnsortorder\local\bank\column_action_resize;
use qbank_columnsortorder\local\bank\preview_view;
use moodle_url;
/**
@@ -158,7 +158,8 @@ class column_manager extends column_manager_base {
$context = context_system::instance();
$contexts = new question_edit_contexts($context);
// Dummy call to get the objects without error.
$questionbank = new view($contexts, new moodle_url('/question/bank/columnsortorder/sortcolumns.php'), $course, null);
$questionbank = new preview_view($contexts, new moodle_url('/question/bank/columnsortorder/sortcolumns.php'), $course,
null);
return $questionbank;
}
@@ -178,6 +179,7 @@ class column_manager extends column_manager_base {
'class' => get_class($column),
'name' => $column->get_title(),
'colname' => end($classelements),
'id' => implode(self::ID_SEPARATOR, [$column::class, $column->get_column_name()]),
];
}
return $columns;
@@ -191,20 +193,12 @@ class column_manager extends column_manager_base {
public function get_disabled_columns(): array {
$disabled = [];
if ($this->disabledcolumns) {
foreach ($this->disabledcolumns as $class => $value) {
if (strpos($class, 'qbank_customfields\custom_field_column') !== false) {
$class = explode('\\', $class);
$disabledname = array_pop($class);
$class = implode('\\', $class);
$disabled[] = (object) [
'disabledname' => $disabledname,
];
} else {
$columnobject = new $class($this->get_questionbank());
$disabled[] = (object) [
'disabledname' => $columnobject->get_title(),
];
}
foreach (array_keys($this->disabledcolumns) as $column) {
[$classname, $columnname] = explode(self::ID_SEPARATOR, $column);
$columnobject = $classname::from_column_name($this->get_questionbank(), $columnname);
$disabled[] = (object) [
'disabledname' => $columnobject->get_title(),
];
}
}
return $disabled;
@@ -271,17 +265,10 @@ class column_manager extends column_manager_base {
}
foreach ($allcolumns as $column) {
if (strpos($column->class, $plugin) !== false) {
if ($column->class === 'qbank_customfields\custom_field_column') {
$disabledcolumns[$column->class . '\\' . $column->colname] = $column->class . '\\' . $column->colname;
if (isset($enabledcolumns[$column->class . '\\' . $column->colname])) {
unset($enabledcolumns[$column->class. '\\' . $column->colname]);
}
} else {
$disabledcolumns[$column->class] = $column->class;
if (isset($enabledcolumns[$column->class])) {
unset($enabledcolumns[$column->class]);
}
if (str_contains($column->class, $plugin)) {
$disabledcolumns[$column->id] = $column->id;
if (isset($enabledcolumns[$column->id])) {
unset($enabledcolumns[$column->id]);
}
}
}
@@ -301,21 +288,9 @@ class column_manager extends column_manager_base {
$columnsortorder = $this->columnorder;
asort($columnsortorder);
$columnorder = [];
foreach ($columnsortorder as $classname => $colposition) {
$colname = explode('\\', $classname);
if (strpos($classname, 'qbank_customfields\custom_field_column') !== false) {
unset($colname[0]);
$classname = implode('\\', $colname);
// Checks if custom column still exists.
if (array_key_exists($classname, $ordertosort)) {
$columnorder[$classname] = $colposition;
} else {
$configtounset = str_replace('\\', '\\\\', $classname);
// Cleans config db.
unset_config($configtounset, 'column_sortorder');
}
} else {
$columnorder[end($colname)] = $colposition;
foreach ($columnsortorder as $columnid => $colposition) {
if (array_key_exists($columnid, $ordertosort)) {
$columnorder[$columnid] = $colposition;
}
}
$properorder = array_merge($columnorder, $ordertosort);
@@ -375,7 +350,7 @@ class column_manager extends column_manager_base {
$colsizemap = $this->get_colsize_map();
$columnclass = get_class($column);
if (array_key_exists($columnclass, $colsizemap)) {
return $colsizemap[$columnclass];
return $colsizemap[$columnclass] . 'px';
}
return parent::get_column_width($column);
}
question/bank/customfields/classes/custom_field_column.php
+13 -1
View File
@@ -17,6 +17,8 @@
namespace qbank_customfields;
use core_question\local\bank\column_base;
use core_question\local\bank\view;
use qbank_customfields\customfield\question_handler;
/**
* A column type for the name of the question creator.
@@ -42,6 +44,16 @@ class custom_field_column extends column_base {
$this->field = $field;
}
public static function from_column_name(view $view, string $columnname): 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.');
}
/**
* Get the internal name for this column. Used as a CSS class name,
* and to store information about the current sort. Must match PARAM_ALPHA.
@@ -60,7 +72,7 @@ class custom_field_column extends column_base {
* @return string The unique name;
*/
public function get_column_name(): string {
return 'custom_field_column\\' . $this->field->get('shortname');
return $this->field->get('shortname');
}
/**
question/classes/local/bank/checkbox_column.php
+1
View File
@@ -70,6 +70,7 @@ class checkbox_column extends column_base {
$data['tip'] = $this->get_title_tip();
$data['colname'] = $this->get_column_name();
$data['columnid'] = $this->get_column_id();
$data['name'] = get_string('selectall');
$data['class'] = $name;
$data['width'] = $width;
question/classes/local/bank/column_base.php
+38
View File
@@ -33,6 +33,11 @@ namespace core_question\local\bank;
*/
abstract class column_base extends view_component {
/**
* @const string A separator for joining column attributes together into a unique ID string.
*/
const ID_SEPARATOR = '-';
/**
* @var view $qbank the question bank view we are helping to render.
*/
@@ -44,6 +49,20 @@ abstract class column_base extends view_component {
/** @var bool determine whether the column is visible */
public $isvisible = true;
/**
* Return an instance of this column, based on the column name.
*
* In the case of the base class, we don't actually use the column name since the class represents one specific column.
* However, sub-classes may use the column name as an additional constructor to the parameter.
*
* @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.
*/
public static function from_column_name(view $view, string $columnname): column_base {
return new static($view);
}
/**
* Set the column as heading
*/
@@ -121,6 +140,7 @@ abstract class column_base extends view_component {
}
$data['colname'] = $this->get_column_name();
$data['columnid'] = $this->get_column_id();
$data['name'] = $title;
$data['class'] = $name;
$data['width'] = $width;
@@ -272,6 +292,24 @@ abstract class column_base extends view_component {
return (new \ReflectionClass($this))->getShortName();
}
/**
* Return a unique ID for this column object.
*
* This is constructed using the class name and get_column_name(), which must be unique.
*
* The combination of these attributes allows the object to be reconstructed, by splitting the ID into its constituent
* parts then calling {@see from_column_name()}, like this:
* [$class, $columnname] = explode(column_base::ID_SEPARATOR, $columnid, 2);
* $column = $class::from_column_name($qbank, $columnname);
* Including 2 as the $limit parameter for explode() is a good idea for safely, in case a plugin defines a column with the
* ID_SEPARATOR in the column name.
*
* @return string The column ID.
*/
final public function get_column_id(): string {
return implode(self::ID_SEPARATOR, [static::class, $this->get_column_name()]);
}
/**
* Any extra class names you would like applied to every cell in this column.
*
question/classes/local/bank/view.php
+25 -42
View File
@@ -383,66 +383,49 @@ class view {
*
* @return array
*/
protected function get_class_for_columns(): array {
$this->corequestionbankcolumns = [
'checkbox_column',
'question_type_column',
'question_name_idnumber_tags_column',
'edit_menu_column',
'export_xml_action_column',
'question_status_column',
'version_number_column',
'creator_name_column',
'comment_count_column',
protected function get_question_bank_plugins(): array {
$questionbankclasscolumns = [];
$newpluginclasscolumns = [];
$corequestionbankcolumns = [
'core_question\local\bank\checkbox_column' . column_base::ID_SEPARATOR . 'checkbox_column',
'qbank_viewquestiontype\question_type_column' . column_base::ID_SEPARATOR . 'question_type_column',
'qbank_viewquestionname\question_name_idnumber_tags_column' . column_base::ID_SEPARATOR .
'question_name_idnumber_tags_column',
'core_question\local\bank\edit_menu_column' . column_base::ID_SEPARATOR . 'edit_menu_column',
'qbank_editquestion\question_status_column' . column_base::ID_SEPARATOR . 'question_status_column',
'qbank_history\version_number_column' . column_base::ID_SEPARATOR . 'version_number_column',
'qbank_viewcreator\creator_name_column' . column_base::ID_SEPARATOR . 'creator_name_column',
'qbank_comment\comment_count_column' . column_base::ID_SEPARATOR . 'comment_count_column',
];
if (question_get_display_preference('qbshowtext', 0, PARAM_INT, new \moodle_url(''))) {
$this->corequestionbankcolumns[] = 'question_text_row';
$corequestionbankcolumns[] = 'qbank_viewquestiontext\question_text_row' . column_base::ID_SEPARATOR .
'question_text_row';
}
$questionbankclasscolumns = [];
foreach ($this->corequestionbankcolumns as $fullname) {
$shortname = $fullname;
if (class_exists('core_question\\local\\bank\\' . $fullname)) {
$fullname = 'core_question\\local\\bank\\' . $fullname;
$questionbankclasscolumns[$shortname] = new $fullname($this);
} else {
$questionbankclasscolumns[$shortname] = '';
foreach ($corequestionbankcolumns as $columnid) {
[$columnclass, $columnname] = explode(column_base::ID_SEPARATOR, $columnid, 2);
if (class_exists($columnclass)) {
$questionbankclasscolumns[$columnid] = $columnclass::from_column_name($this, $columnname);
}
}
return $questionbankclasscolumns;
}
/**
* Get the list of qbank plugins with available objects for features.
*
* @return array
*/
protected function get_question_bank_plugins(): array {
$newpluginclasscolumns = [];
$questionbankclasscolumns = $this->get_class_for_columns();
$plugins = $this->plugins;
foreach ($this->plugins as $componentname => $plugin) {
foreach ($this->plugins as $plugin) {
$plugincolumnobjects = $plugin->get_question_columns($this);
foreach ($plugincolumnobjects as $columnobject) {
$columnname = $columnobject->get_column_name();
foreach ($this->corequestionbankcolumns as $key => $corequestionbankcolumn) {
if (!\core\plugininfo\qbank::is_plugin_enabled($componentname)) {
unset($questionbankclasscolumns[$columnname]);
continue;
}
$columnid = $columnobject->get_column_id();
foreach ($corequestionbankcolumns as $corequestionbankcolumn) {
// Check if it has custom preference selector to view/hide.
if ($columnobject->has_preference()) {
if (!$columnobject->get_preference()) {
continue;
}
}
if ($corequestionbankcolumn === $columnname) {
$questionbankclasscolumns[$columnname] = $columnobject;
if ($corequestionbankcolumn === $columnid) {
$questionbankclasscolumns[$columnid] = $columnobject;
} else {
// Any community plugin for column/action.
$newpluginclasscolumns[$columnname] = $columnobject;
$newpluginclasscolumns[$columnid] = $columnobject;
}
}
}
question/templates/column_header.mustache
+6 -1
View File
@@ -51,7 +51,12 @@
"name": "plugin_name"
}
}}
<th class="header align-top {{extraclasses}}" scope="col" data-pluginname="{{class}}" data-name="{{name}}" {{#width}}style="width: {{width}};"{{/width}}>
<th class="header align-top {{extraclasses}}"
scope="col"
data-pluginname="{{class}}"
data-name="{{name}}"
data-columnid="{{columnid}}"
{{#width}}style="width: {{width}};"{{/width}}>
<div class="header-container">
<div class="header-text">
{{#title}}