diff --git a/question/classes/local/bank/column_base.php b/question/classes/local/bank/column_base.php index 3a4898ddae0..66832ed337e 100644 --- a/question/classes/local/bank/column_base.php +++ b/question/classes/local/bank/column_base.php @@ -31,7 +31,7 @@ namespace core_question\local\bank; * @author 2021 Safat Shahin * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ -abstract class column_base { +abstract class column_base extends view_component { /** * @var view $qbank the question bank view we are helping to render. @@ -41,22 +41,6 @@ abstract class column_base { /** @var bool determine whether the column is td or th. */ protected $isheading = false; - /** - * Constructor. - * @param view $qbank the question bank view we are helping to render. - */ - public function __construct(view $qbank) { - $this->qbank = $qbank; - $this->init(); - } - - /** - * A chance for subclasses to initialise themselves, for example to load lang strings, - * without having to override the constructor. - */ - protected function init(): void { - } - /** * Set the column as heading */ @@ -297,30 +281,10 @@ abstract class column_base { echo \html_writer::end_tag($tag); } - /** - * Return an array 'table_alias' => 'JOIN clause' to bring in any data that - * this column required. - * - * The return values for all the columns will be checked. It is OK if two - * columns join in the same table with the same alias and identical JOIN clauses. - * If to columns try to use the same alias with different joins, you get an error. - * The only table included by default is the question table, which is aliased to 'q'. - * - * It is importnat that your join simply adds additional data (or NULLs) to the - * existing rows of the query. It must not cause additional rows. - * - * @return array 'table_alias' => 'JOIN clause' - */ public function get_extra_joins(): array { return []; } - /** - * Use table alias 'q' for the question table, or one of the - * ones from get_extra_joins. Every field requested must specify a table prefix. - * - * @return array fields required. - */ public function get_required_fields(): array { return []; } diff --git a/question/classes/local/bank/edit_menu_column.php b/question/classes/local/bank/edit_menu_column.php index 0c2cd32c430..796a12eb0c5 100644 --- a/question/classes/local/bank/edit_menu_column.php +++ b/question/classes/local/bank/edit_menu_column.php @@ -24,12 +24,14 @@ namespace core_question\local\bank; +use \core\plugininfo\qbank; + /** * A question bank column which gathers together all the actions into a menu. * * This question bank column, if added to the question bank, will * replace all of the other columns which implement the - * {@see menuable_action} interface and replace them with a single + * {@see menu_action_column_base} interface and replace them with a single * column containing an Edit menu. * * @copyright 2019 The Open University @@ -37,33 +39,6 @@ namespace core_question\local\bank; * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class edit_menu_column extends column_base { - /** - * @var menuable_action[] - */ - protected $actions; - - /** - * Set up the list of actions that should be shown in the menu. - * - * This takes a list of column object (the list from a question - * bank view). It extracts all the ones that should go in the menu - * and stores them for later use. Then it returns the remaining columns. - * - * @param column_base[] $allcolumns a set of columns. - * @return column_base[] the non-action columns from the set. - */ - public function claim_menuable_columns($allcolumns): array { - $remainingcolumns = []; - foreach ($allcolumns as $key => $column) { - if ($column instanceof menuable_action) { - $this->actions[$key] = $column; - } else { - $remainingcolumns[$key] = $column; - } - } - return $remainingcolumns; - } - public function get_title() { return get_string('actions'); } @@ -74,11 +49,12 @@ class edit_menu_column extends column_base { protected function display_content($question, $rowclasses): void { global $OUTPUT; + $actions = $this->qbank->get_question_actions(); $menu = new \action_menu(); $menu->set_menu_trigger(get_string('edit')); - foreach ($this->actions as $actioncolumn) { - $action = $actioncolumn->get_action_menu_link($question); + foreach ($actions as $action) { + $action = $action->get_action_menu_link($question); if ($action) { $menu->add($action); } @@ -100,7 +76,7 @@ class edit_menu_column extends column_base { /** * Get menuable actions. * - * @return menuable_action Menuable actions. + * @return menu_action_column_base Menuable actions. */ public function get_actions(): array { return $this->actions; diff --git a/question/classes/local/bank/plugin_features_base.php b/question/classes/local/bank/plugin_features_base.php index f12729b9dcb..de42d3cf8eb 100644 --- a/question/classes/local/bank/plugin_features_base.php +++ b/question/classes/local/bank/plugin_features_base.php @@ -38,7 +38,7 @@ namespace core_question\local\bank; class plugin_features_base { /** - * This method will return the array of objects to be rendered as a part of question bank columns/actions. + * This method will return the array of objects to be rendered as a part of question bank columns. * * @param view $qbank * @return array @@ -47,6 +47,16 @@ class plugin_features_base { return []; } + /** + * This method will return the array of objects to be rendered as a part of question bank actions. + * + * @param view $qbank + * @return question_action_base[] + */ + public function get_question_actions(view $qbank): array { + return []; + } + /** * This method will return the object for the navigation node. * diff --git a/question/classes/local/bank/question_action_base.php b/question/classes/local/bank/question_action_base.php new file mode 100644 index 00000000000..bf745ce1ec9 --- /dev/null +++ b/question/classes/local/bank/question_action_base.php @@ -0,0 +1,77 @@ +. + +/** + * Base class to make it easier to implement actions that are menuable_actions. + * + * @package core_question + * @copyright 2019 Tim Hunt + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core_question\local\bank; + +/** + * Base class to make it easier to implement actions that are menuable_actions. + * + * Use this class if your action is simple (defined by just a URL, label and icon). + * If your action is not simple enough to fit into the pattern that this + * class implements, then you will have to implement the menuable_action + * interface yourself. + * + * @copyright 2019 Tim Hunt + * @author 2021 Safat Shahin + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +abstract class question_action_base extends view_component { + + /** + * Get the information required to display this action either as a menu item or a separate action column. + * + * For most actions, it should be sufficient to override just this method. {@see get_action_menu_link()} is the public interface + * and handles building a renderable menu link object from this data. + * + * If this action cannot apply to this question (e.g. because the user does not have + * permission, then return [null, null, null]. + * + * @param \stdClass $question the row from the $question table, augmented with extra information. + * @return array with three elements. + * $url - the URL to perform the action. + * $icon - the icon for this action. E.g. 't/delete'. + * $label - text label to display in the UI (either in the menu, or as a tool-tip on the icon) + */ + protected function get_url_icon_and_label(\stdClass $question): array { + return [null, null, null]; + } + + /** + * Return the action menu link for this action on the supplied question. + * + * For most actions, you will just need to override {@see get_url_icon_and_label()}. You only need to override + * this method if you need to pass additional attributes to {@see action_menu_link_secondary}, or use a different class to + * render the link. + * + * @param \stdClass $question + * @return \action_menu_link|null + */ + public function get_action_menu_link(\stdClass $question): ?\action_menu_link { + [$url, $icon, $label] = $this->get_url_icon_and_label($question); + if (!$url) { + return null; + } + return new \action_menu_link_secondary($url, new \pix_icon($icon, ''), $label); + } +} diff --git a/question/classes/local/bank/view.php b/question/classes/local/bank/view.php index 722c8a90226..b8b41218ff7 100644 --- a/question/classes/local/bank/view.php +++ b/question/classes/local/bank/view.php @@ -24,6 +24,7 @@ namespace core_question\local\bank; +use core\plugininfo\qbank; use core_plugin_manager; use core_question\bank\search\condition; use core_question\local\statistics\statistics_bulk_loader; @@ -96,6 +97,13 @@ class view { */ protected $requiredcolumns; + /** + * @var menu_action_base[] these are all the menu actions that can be displayed in a row's action menu. + * + * Array keys are the class name. + */ + protected $menuactions; + /** * @var column_base[] these are the 'columns' that are * actually displayed as a column, in order. Array keys are the class name. @@ -201,6 +209,7 @@ class view { // Possibly the heading part can be removed. $this->init_columns($this->wanted_columns(), $this->heading_column()); + $this->init_menu_actions(); $this->init_sort(); $this->init_search_conditions(); $this->init_bulk_actions(); @@ -251,6 +260,28 @@ class view { } } + /** + * Initialise list of menu actions for enabled question bank plugins. + * + * Menu action objects are stored in $this->menuactions, keyed by class name. + * + * @return void + */ + protected function init_menu_actions(): void { + $plugins = \core_component::get_plugin_list_with_class('qbank', 'plugin_feature', 'plugin_feature.php'); + $this->menuactions = []; + foreach ($plugins as $component => $plugin) { + if (!qbank::is_plugin_enabled($component)) { + continue; + } + $pluginentrypointobject = new $plugin(); + $menuactions = $pluginentrypointobject->get_question_actions($this); + foreach ($menuactions as $menuaction) { + $this->menuactions[$menuaction::class] = $menuaction; + } + } + } + /** * Get the list of qbank plugins with available objects for features. * @@ -264,13 +295,6 @@ class view { 'question_type_column', 'question_name_idnumber_tags_column', 'edit_menu_column', - 'edit_action_column', - 'copy_action_column', - 'tags_action_column', - 'preview_action_column', - 'history_action_column', - 'delete_action_column', - 'export_xml_action_column', 'question_status_column', 'version_number_column', 'creator_name_column', @@ -388,14 +412,6 @@ class view { * @param string $heading The name of column that is set as heading */ protected function init_columns($wanted, $heading = ''): void { - // If we are using the edit menu column, allow it to absorb all the actions. - foreach ($wanted as $column) { - if ($column instanceof edit_menu_column) { - $wanted = $column->claim_menuable_columns($wanted); - break; - } - } - // Now split columns into real columns and rows. $this->visiblecolumns = []; $this->extrarows = []; @@ -589,26 +605,77 @@ class view { } /** - * Create the SQL query to retrieve the indicated questions, based on - * \core_question\bank\search\condition filters. + * Return an array 'table_alias' => 'JOIN clause' to bring in any data that + * the core view requires. + * + * @return string[] 'table_alias' => 'JOIN clause' */ - protected function build_query(): void { - // Get the required tables and fields. - $joins = []; - $fields = ['qv.status', 'qc.id as categoryid', 'qv.version', 'qv.id as versionid', 'qbe.id as questionbankentryid']; - if (!empty($this->requiredcolumns)) { - foreach ($this->requiredcolumns as $column) { - $extrajoins = $column->get_extra_joins(); + protected function get_required_joins(): array { + return [ + 'qv' => 'JOIN {question_versions} qv ON qv.questionid = q.id', + 'qbe' => 'JOIN {question_bank_entries} qbe on qbe.id = qv.questionbankentryid', + 'qc' => 'JOIN {question_categories} qc ON qc.id = qbe.questioncategoryid', + ]; + } + + /** + * Return an array of fields for any data that the core view requires. + * + * Use table alias 'q' for the question table, or one of the ones from get_required_joins. + * Every field requested must specify a table prefix. + * + * @return string[] fields required. + */ + protected function get_required_fields(): array { + return [ + 'q.id', + 'q.qtype', + 'q.createdby', + 'qc.id as categoryid', + 'qc.contextid', + 'qv.status', + 'qv.version', + 'qv.id as versionid', + 'qbe.id as questionbankentryid', + ]; + } + + /** + * Gather query requirements from view component objects. + * + * This will take the required fields and joins for this view, and combine them with those for all active view components. + * Fields will be de-duplicated in multiple components require the same field. + * Joins will be de-duplicated if the alias and join clause match exactly. + * + * @throws \coding_exception If two components attempt to use the same alias for different joins. + * @param view_component[] $viewcomponents List of component objects included in the current view + * @return array [$fields, $joins] SQL fields and joins to add to the query. + */ + protected function get_component_requirements(array $viewcomponents): array { + $fields = $this->get_required_fields(); + $joins = $this->get_required_joins(); + if (!empty($viewcomponents)) { + foreach ($viewcomponents as $viewcomponent) { + $extrajoins = $viewcomponent->get_extra_joins(); foreach ($extrajoins as $prefix => $join) { if (isset($joins[$prefix]) && $joins[$prefix] != $join) { throw new \coding_exception('Join ' . $join . ' conflicts with previous join ' . $joins[$prefix]); } $joins[$prefix] = $join; } - $fields = array_merge($fields, $column->get_required_fields()); + $fields = array_merge($fields, $viewcomponent->get_required_fields()); } } - $fields = array_unique($fields); + return [array_unique($fields), $joins]; + } + + /** + * Create the SQL query to retrieve the indicated questions, based on + * \core_question\bank\search\condition filters. + */ + protected function build_query(): void { + // Get the required tables and fields. + [$fields, $joins] = $this->get_component_requirements(array_merge($this->requiredcolumns, $this->questionactions)); // Build the order by clause. $sorts = []; @@ -1335,4 +1402,13 @@ class view { public function is_listing_specific_versions(): bool { return false; } + + /** + * Return array of menu actions. + * + * @return menu_action_base[] + */ + public function get_menuactions(): array { + return $this->menuactions; + } } diff --git a/question/classes/local/bank/view_component.php b/question/classes/local/bank/view_component.php new file mode 100644 index 00000000000..3932e9647a4 --- /dev/null +++ b/question/classes/local/bank/view_component.php @@ -0,0 +1,72 @@ +. + +namespace core_question\local\bank; + +/** + * Abstract class to define functionality shared by all pluggable components used in the question bank view. + * + * @package core_question + * @copyright 2023 onwards Catalyst IT EU {@link https://catalyst-eu.net} + * @author Mark Johnson + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +abstract class view_component { + + /** + * Constructor. + * @param view $qbank the question bank view we are helping to render. + */ + public function __construct(view $qbank) { + $this->qbank = $qbank; + $this->init(); + } + + /** + * A chance for subclasses to initialise themselves, for example to load lang strings, + * without having to override the constructor. + */ + protected function init(): void { + } + + /** + * Return an array 'table_alias' => 'JOIN clause' to bring in any data that + * this feature requires. + * + * The return values for all the features will be checked. It is OK if two + * features join in the same table with the same alias and identical JOIN clauses. + * If two features try to use the same alias with different joins, you get an error. + * Tables included by default are question (alias q) and those defined in {@see view::get_required_joins()} + * + * It is importnat that your join simply adds additional data (or NULLs) to the + * existing rows of the query. It must not cause additional rows. + * + * @return string[] 'table_alias' => 'JOIN clause' + */ + public function get_extra_joins(): array { + return []; + } + + /** + * Use table alias 'q' for the question table, or one of the + * ones from get_extra_joins. Every field requested must specify a table prefix. + * + * @return string[] fields required. + */ + public function get_required_fields(): array { + return []; + } +} diff --git a/question/type/edit_question_form.php b/question/type/edit_question_form.php index 4fe7ed56fac..18daca2c286 100644 --- a/question/type/edit_question_form.php +++ b/question/type/edit_question_form.php @@ -256,7 +256,6 @@ abstract class question_edit_form extends question_wizard_form { $this->definition_inner($mform); if (core_tag_tag::is_enabled('core_question', 'question') - && class_exists('qbank_tagquestion\\tags_action_column') && \core\plugininfo\qbank::is_plugin_enabled('qbank_tagquestion')) { $this->add_tag_fields($mform); }