diff --git a/config-dist.php b/config-dist.php index 5227584d044..ba04c387c3d 100644 --- a/config-dist.php +++ b/config-dist.php @@ -626,6 +626,23 @@ $CFG->admin = 'admin'; // // $CFG->uninstallclionly = true; // +// +// Customise question bank display +// +// The display of Moodle's question bank is made up of a number of columns. +// You can customise this display by giving a comma-separated list of column class +// names here. Each class must be a subclass of \core_question\bank\column_base. +// For example you might define a class like +// class \local_qbank_extensions\my_column extends \core_question\bank\column_base +// in a local plugin, then add it to the list here. At the time of writing, +// the default question bank display is equivalent to the following, but you might like +// to check the latest default in question/classes/bank/view.php before setting this. +// +// $CFG->questionbankcolumns = 'checkbox_column,question_type_column,' +// . 'question_name_idnumber_tags_column,tags_action_column,edit_action_column,' +// . 'copy_action_column,preview_action_column,delete_action_column,' +// . 'creator_name_column,modifier_name_column'; +// //========================================================================= // 7. SETTINGS FOR DEVELOPMENT SERVERS - not intended for production use!!! //========================================================================= diff --git a/lang/en/question.php b/lang/en/question.php index 7dc290e5ef7..8da8aa4e891 100644 --- a/lang/en/question.php +++ b/lang/en/question.php @@ -70,6 +70,9 @@ $string['categoryinfo'] = 'Category info'; $string['categorymove'] = 'The category \'{$a->name}\' contains {$a->count} questions (some of which may be hidden questions or random questions that are still in use in a quiz). Please choose another category to move them to.'; $string['categorymoveto'] = 'Save in category'; $string['categorynamecantbeblank'] = 'The category name cannot be blank.'; +$string['categorynamewithcount'] = '{$a->name} ({$a->questioncount})'; +$string['categorynamewithidnumber'] = '{$a->name} [{$a->idnumber}]'; +$string['categorynamewithidnumberandcount'] = '{$a->name} [{$a->idnumber}] ({$a->questioncount})'; $string['clickflag'] = 'Flag question'; $string['clicktoflag'] = 'Flag this question for future reference'; $string['clicktounflag'] = 'Remove flag'; diff --git a/lib/outputrenderers.php b/lib/outputrenderers.php index e0570e428d2..10124c0c3b3 100644 --- a/lib/outputrenderers.php +++ b/lib/outputrenderers.php @@ -4511,10 +4511,12 @@ EOD; * @param int $limit limit the number of tags to display, if size of $tags is more than this limit the "more" link * will be appended to the end, JS will toggle the rest of the tags * @param context $pagecontext specify if needed to overwrite the current page context for the view tag link + * @param bool $accesshidelabel if true, the label should have class="accesshide" added. * @return string */ - public function tag_list($tags, $label = null, $classes = '', $limit = 10, $pagecontext = null) { - $list = new \core_tag\output\taglist($tags, $label, $classes, $limit, $pagecontext); + public function tag_list($tags, $label = null, $classes = '', $limit = 10, + $pagecontext = null, $accesshidelabel = false) { + $list = new \core_tag\output\taglist($tags, $label, $classes, $limit, $pagecontext, $accesshidelabel); return $this->render_from_template('core_tag/taglist', $list->export_for_template($this)); } diff --git a/lib/questionlib.php b/lib/questionlib.php index 8ab9a38a355..7aee57fb94e 100644 --- a/lib/questionlib.php +++ b/lib/questionlib.php @@ -1440,11 +1440,25 @@ function question_category_options($contexts, $top = false, $currentcat = 0, if ($category->contextid == $contextid) { $cid = $category->id; if ($currentcat != $cid || $currentcat == 0) { - $countstring = !empty($category->questioncount) ? - " ($category->questioncount)" : ''; - $categoriesarray[$contextstring][$cid] = - format_string($category->indentedname, true, - array('context' => $context)) . $countstring; + $a = new stdClass; + $a->name = format_string($category->indentedname, true, + array('context' => $context)); + if ($category->idnumber !== null && $category->idnumber !== '') { + $a->idnumber = s($category->idnumber); + } + if (!empty($category->questioncount)) { + $a->questioncount = $category->questioncount; + } + if (isset($a->idnumber) && isset($a->questioncount)) { + $formattedname = get_string('categorynamewithidnumberandcount', 'question', $a); + } else if (isset($a->idnumber)) { + $formattedname = get_string('categorynamewithidnumber', 'question', $a); + } else if (isset($a->questioncount)) { + $formattedname = get_string('categorynamewithcount', 'question', $a); + } else { + $formattedname = $a->name; + } + $categoriesarray[$contextstring][$cid] = $formattedname; } } } @@ -1875,14 +1889,14 @@ class question_edit_contexts { } /** - * @return array all parent contexts + * @return context[] all parent contexts */ public function all() { return $this->allcontexts; } /** - * @return object lowest context which must be either the module or course context + * @return context lowest context which must be either the module or course context */ public function lowest() { return $this->allcontexts[0]; @@ -1890,7 +1904,7 @@ class question_edit_contexts { /** * @param string $cap capability - * @return array parent contexts having capability, zero based index + * @return context[] parent contexts having capability, zero based index */ public function having_cap($cap) { $contextswithcap = array(); @@ -1904,7 +1918,7 @@ class question_edit_contexts { /** * @param array $caps capabilities - * @return array parent contexts having at least one of $caps, zero based index + * @return context[] parent contexts having at least one of $caps, zero based index */ public function having_one_cap($caps) { $contextswithacap = array(); @@ -1921,14 +1935,14 @@ class question_edit_contexts { /** * @param string $tabname edit tab name - * @return array parent contexts having at least one of $caps, zero based index + * @return context[] parent contexts having at least one of $caps, zero based index */ public function having_one_edit_tab_cap($tabname) { return $this->having_one_cap(self::$caps[$tabname]); } /** - * @return those contexts where a user can add a question and then use it. + * @return context[] those contexts where a user can add a question and then use it. */ public function having_add_and_use() { $contextswithcap = array(); @@ -1993,7 +2007,7 @@ class question_edit_contexts { /** * Throw error if at least one parent context hasn't got one of the caps $caps * - * @param array $cap capabilities + * @param array $caps capabilities */ public function require_one_cap($caps) { if (!$this->have_one_cap($caps)) { diff --git a/mod/quiz/classes/question/bank/custom_view.php b/mod/quiz/classes/question/bank/custom_view.php index e4d52694e6b..8eef0ef1358 100644 --- a/mod/quiz/classes/question/bank/custom_view.php +++ b/mod/quiz/classes/question/bank/custom_view.php @@ -207,7 +207,7 @@ class custom_view extends \core_question\bank\view { } protected function print_category_info($category) { - $formatoptions = new stdClass(); + $formatoptions = new \stdClass(); $formatoptions->noclean = true; $strcategory = get_string('category', 'quiz'); echo '
' . diff --git a/mod/quiz/classes/question/bank/question_name_text_column.php b/mod/quiz/classes/question/bank/question_name_text_column.php index fa15f54097d..ecde32767f0 100644 --- a/mod/quiz/classes/question/bank/question_name_text_column.php +++ b/mod/quiz/classes/question/bank/question_name_text_column.php @@ -44,7 +44,7 @@ class question_name_text_column extends \core_question\bank\question_name_column if ($labelfor) { echo ''; } @@ -55,6 +55,12 @@ class question_name_text_column extends \core_question\bank\question_name_column $fields = parent::get_required_fields(); $fields[] = 'q.questiontext'; $fields[] = 'q.questiontextformat'; + $fields[] = 'q.idnumber'; return $fields; } + + public function load_additional_data(array $questions) { + parent::load_additional_data($questions); + parent::load_question_tags($questions); + } } diff --git a/mod/quiz/locallib.php b/mod/quiz/locallib.php index 8db703b5ce7..ad16a5ad8a6 100644 --- a/mod/quiz/locallib.php +++ b/mod/quiz/locallib.php @@ -2048,17 +2048,43 @@ class qubaids_for_quiz_user extends qubaid_join { * @param bool $showicon If true, show the question's icon with the question. False by default. * @param bool $showquestiontext If true (default), show question text after question name. * If false, show only question name. - * @return string + * @param bool $showidnumber If true, show the question's idnumber, if any. False by default. + * @param core_tag_tag[]|bool $showtags if array passed, show those tags. Else, if true, get and show tags, + * else, don't show tags (which is the default). + * @return string HTML fragment. */ -function quiz_question_tostring($question, $showicon = false, $showquestiontext = true) { +function quiz_question_tostring($question, $showicon = false, $showquestiontext = true, + $showidnumber = false, $showtags = false) { + global $OUTPUT; $result = ''; + // Question name. $name = shorten_text(format_string($question->name), 200); if ($showicon) { $name .= print_question_icon($question) . ' ' . $name; } $result .= html_writer::span($name, 'questionname'); + // Question idnumber. + if ($showidnumber && $question->idnumber !== null && $question->idnumber !== '') { + $result .= ' ' . html_writer::span( + html_writer::span(get_string('idnumber', 'question'), 'accesshide') . + ' ' . $question->idnumber, 'badge badge-primary'); + } + + // Question tags. + if (is_array($showtags)) { + $tags = $showtags; + } else if ($showtags) { + $tags = core_tag_tag::get_item_tags('core_question', 'question', $question->id); + } else { + $tags = []; + } + if ($tags) { + $result .= $OUTPUT->tag_list($tags, null, 'd-inline', 0, null, true); + } + + // Question text. if ($showquestiontext) { $questiontext = question_utils::to_plain_text($question->questiontext, $question->questiontextformat, array('noclean' => true, 'para' => false)); diff --git a/mod/quiz/styles.css b/mod/quiz/styles.css index eae1b2878b6..ca307702967 100644 --- a/mod/quiz/styles.css +++ b/mod/quiz/styles.css @@ -968,8 +968,7 @@ table.quizreviewsummary td.cell { border: 0 none; } -#categoryquestions th.modifiername .sorters, -#categoryquestions th.creatorname .sorters { +#categoryquestions th .sorters { font-weight: normal; font-size: 0.8em; } diff --git a/mod/quiz/tests/behat/editing_add_from_question_bank.feature b/mod/quiz/tests/behat/editing_add_from_question_bank.feature index 56e983d5f4c..2e8708e82c5 100644 --- a/mod/quiz/tests/behat/editing_add_from_question_bank.feature +++ b/mod/quiz/tests/behat/editing_add_from_question_bank.feature @@ -21,9 +21,9 @@ Feature: Adding questions to a quiz from the question bank | contextlevel | reference | name | | Course | C1 | Test questions | And the following "questions" exist: - | questioncategory | qtype | name | user | questiontext | - | Test questions | essay | question 01 name | admin | Question 01 text | - | Test questions | essay | question 02 name | teacher1 | Question 02 text | + | questioncategory | qtype | name | user | questiontext | idnumber | + | Test questions | essay | question 01 name | admin | Question 01 text | | + | Test questions | essay | question 02 name | teacher1 | Question 02 text | qidnum | Scenario: The questions can be filtered by tag Given I log in as "teacher1" @@ -42,9 +42,12 @@ Feature: Adding questions to a quiz from the question bank And I navigate to "Edit quiz" in current page administration And I open the "last" add to quiz menu And I follow "from question bank" + Then I should see "foo" in the "question 01 name" "table_row" + And I should see "bar" in the "question 02 name" "table_row" + And I should see "qidnum" in the "question 02 name" "table_row" And I set the field "Filter by tags..." to "foo" And I press key "13" in the field "Filter by tags..." - Then I should see "question 01 name" in the "categoryquestions" "table" + And I should see "question 01 name" in the "categoryquestions" "table" And I should not see "question 02 name" in the "categoryquestions" "table" Scenario: The question modal can be paginated diff --git a/mod/quiz/tests/behat/editing_add_random.feature b/mod/quiz/tests/behat/editing_add_random.feature index 2249d113d0e..3e3a53b9384 100644 --- a/mod/quiz/tests/behat/editing_add_random.feature +++ b/mod/quiz/tests/behat/editing_add_random.feature @@ -35,7 +35,7 @@ Feature: Adding random questions to a quiz based on category and tags | Tags | foo | And I press "id_submitbutton" And I click on "Manage tags" "link" in the "question 2 name" "table_row" - And I set the following fields to these values: + And I set the following fields in the "Question tags" "dialogue" to these values: | Tags | bar | And I press "Save changes" And I am on "Course 1" course homepage diff --git a/question/category_class.php b/question/category_class.php index 6711a833b8d..d267230ae5e 100644 --- a/question/category_class.php +++ b/question/category_class.php @@ -143,8 +143,13 @@ class question_category_list_item extends list_item { $questionbankurl = new moodle_url('/question/edit.php', $this->parentlist->pageurl->params()); $questionbankurl->param('cat', $category->id . ',' . $category->contextid); $item = ''; - $text = format_string($category->name, true, ['context' => $this->parentlist->context]) - . ' (' . $category->questioncount . ')'; + $text = format_string($category->name, true, ['context' => $this->parentlist->context]); + if ($category->idnumber !== null && $category->idnumber !== '') { + $text .= ' ' . html_writer::span( + html_writer::span(get_string('idnumber', 'question'), 'accesshide') . + ' ' . $category->idnumber, 'badge badge-primary'); + } + $text .= ' (' . $category->questioncount . ')'; $item .= html_writer::tag('b', html_writer::link($questionbankurl, $text, ['title' => $editqestions]) . ' '); $item .= format_text($category->info, $category->infoformat, diff --git a/question/classes/bank/column_base.php b/question/classes/bank/column_base.php index 9994b9f1813..58f892f8829 100644 --- a/question/classes/bank/column_base.php +++ b/question/classes/bank/column_base.php @@ -34,7 +34,7 @@ namespace core_question\bank; abstract class column_base { /** - * @var question_bank_view + * @var view $qbank the question bank view we are helping to render. */ protected $qbank; @@ -43,7 +43,7 @@ abstract class column_base { /** * Constructor. - * @param $qbank the question_bank_view we are helping to render. + * @param view $qbank the question bank view we are helping to render. */ public function __construct(view $qbank) { $this->qbank = $qbank; @@ -103,8 +103,6 @@ abstract class column_base { /** * Title for this column. Not used if is_sortable returns an array. - * @param object $question the row from the $question table, augmented with extra information. - * @param string $rowclasses CSS class names that should be applied to this row of output. */ protected abstract function get_title(); @@ -118,10 +116,10 @@ abstract class column_base { /** * Get a link that changes the sort order, and indicates the current sort state. - * @param $name internal name used for this type of sorting. - * @param $currentsort the current sort order -1, 0, 1 for descending, none, ascending. - * @param $title the link text. - * @param $defaultreverse whether the default sort order for this column is descending, rather than ascending. + * @param string $sort the column to sort on. + * @param string $title the link text. + * @param string $tip the link tool-tip text. If empty, defaults to title. + * @param bool $defaultreverse whether the default sort order for this column is descending, rather than ascending. * @return string HTML fragment. */ protected function make_sort_link($sort, $title, $tip, $defaultreverse = false) { @@ -149,7 +147,7 @@ abstract class column_base { /** * Get an icon representing the corrent sort state. - * @param $reverse sort is descending, not ascending. + * @param bool $reverse sort is descending, not ascending. * @return string HTML image tag. */ protected function get_sort_icon($reverse) { @@ -175,8 +173,8 @@ abstract class column_base { /** * Output the opening column tag. If it is set as heading, it will use tag instead of * - * @param stdClass $question - * @param array $rowclasses + * @param \stdClass $question + * @param string $rowclasses */ protected function display_start($question, $rowclasses) { $tag = 'td'; @@ -198,9 +196,10 @@ abstract class column_base { } /** - * @param object $question the row from the $question table, augmented with extra information. - * @return string internal name for this column. Used as a CSS class name, - * and to store information about the current sort. Must match PARAM_ALPHA. + * 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. + * + * @return string column name. */ public abstract function get_name(); @@ -258,6 +257,42 @@ abstract class column_base { return array(); } + /** + * If this column needs extra data (e.g. tags) then load that here. + * + * The extra data should be added to the question object in the array. + * Probably a good idea to check that another column has not already + * loaded the data you want. + * + * @param \stdClass[] $questions the questions that will be displayed. + */ + public function load_additional_data(array $questions) { + } + + /** + * Load the tags for each question. + * + * Helper that can be used from {@link load_additional_data()}; + * + * @param array $questions + */ + public function load_question_tags(array $questions) { + $firstquestion = reset($questions); + if (isset($firstquestion->tags)) { + // Looks like tags are already loaded, so don't do it again. + return; + } + + // Load the tags. + $tagdata = \core_tag_tag::get_items_tags('core_question', 'question', + array_keys($questions)); + + // Add them to the question objects. + foreach ($tagdata as $questionid => $tags) { + $questions[$questionid]->tags = $tags; + } + } + /** * Can this column be sorted on? You can return either: * + false for no (the default), @@ -265,7 +300,7 @@ abstract class column_base { * + an array of subnames to sort on as follows * return array( * 'firstname' => array('field' => 'uc.firstname', 'title' => get_string('firstname')), - * 'lastname' => array('field' => 'uc.lastname', 'field' => get_string('lastname')), + * 'lastname' => array('field' => 'uc.lastname', 'title' => get_string('lastname')), * ); * As well as field, and field, you can also add 'revers' => 1 if you want the default sort * order to be DESC. @@ -278,7 +313,6 @@ abstract class column_base { /** * Helper method for building sort clauses. * @param bool $reverse whether the normal direction should be reversed. - * @param string $normaldir 'ASC' or 'DESC' * @return string 'ASC' or 'DESC' */ protected function sortorder($reverse) { @@ -290,8 +324,8 @@ abstract class column_base { } /** - * @param $reverse Whether to sort in the reverse of the default sort order. - * @param $subsort if is_sortable returns an array of subnames, then this will be + * @param bool $reverse Whether to sort in the reverse of the default sort order. + * @param string $subsort if is_sortable returns an array of subnames, then this will be * one of those. Otherwise will be empty. * @return string some SQL to go in the order by clause. */ @@ -299,14 +333,14 @@ abstract class column_base { $sortable = $this->is_sortable(); if (is_array($sortable)) { if (array_key_exists($subsort, $sortable)) { - return $sortable[$subsort]['field'] . $this->sortorder($reverse, !empty($sortable[$subsort]['reverse'])); + return $sortable[$subsort]['field'] . $this->sortorder($reverse); } else { - throw new coding_exception('Unexpected $subsort type: ' . $subsort); + throw new \coding_exception('Unexpected $subsort type: ' . $subsort); } } else if ($sortable) { return $sortable . $this->sortorder($reverse); } else { - throw new coding_exception('sort_expression called on a non-sortable column.'); + throw new \coding_exception('sort_expression called on a non-sortable column.'); } } } diff --git a/question/classes/bank/question_name_idnumber_tags_column.php b/question/classes/bank/question_name_idnumber_tags_column.php new file mode 100644 index 00000000000..b9dc3f8a499 --- /dev/null +++ b/question/classes/bank/question_name_idnumber_tags_column.php @@ -0,0 +1,89 @@ +. + +/** + * A question bank column showing the question name with idnumber and tags. + * + * @package core_question + * @copyright 2019 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ + +namespace core_question\bank; +defined('MOODLE_INTERNAL') || die(); + + +/** + * A question bank column showing the question name with idnumber and tags. + * + * @copyright 2019 The Open University + * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later + */ +class question_name_idnumber_tags_column extends question_name_column { + public function get_name() { + return 'qnameidnumbertags'; + } + + protected function display_content($question, $rowclasses) { + global $OUTPUT; + + $layoutclasses = 'd-inline-flex flex-nowrap overflow-hidden w-100'; + $labelfor = $this->label_for($question); + if ($labelfor) { + echo ''; + } else { + echo ''; + $closetag = ''; + } + + // Question name. + echo \html_writer::span(format_string($question->name), 'questionname flex-grow-1 flex-shrink-1 text-truncate'); + + // Question idnumber. + if ($question->idnumber !== null && $question->idnumber !== '') { + echo ' ' . \html_writer::span( + \html_writer::span(get_string('idnumber', 'question'), 'accesshide') . ' ' . + \html_writer::span($question->idnumber, 'badge badge-primary'), 'ml-1'); + } + + // Question tags. + if (!empty($question->tags)) { + $tags = \core_tag_tag::get_item_tags('core_question', 'question', $question->id); + echo $OUTPUT->tag_list($tags, null, 'd-inline flex-shrink-1 text-truncate ml-1', 0, null, true); + } + + echo $closetag; // Computed above to ensure it matches. + } + + public function get_required_fields() { + $fields = parent::get_required_fields(); + $fields[] = 'q.idnumber'; + return $fields; + } + + public function is_sortable() { + return [ + 'name' => ['field' => 'q.name', 'title' => get_string('questionname', 'question')], + 'lastname' => ['field' => 'q.idnumber', 'title' => get_string('idnumber', 'question')], + ]; + } + + public function load_additional_data(array $questions) { + parent::load_additional_data($questions); + parent::load_question_tags($questions); + } +} diff --git a/question/classes/bank/view.php b/question/classes/bank/view.php index 12b781f207b..850ce2b55ad 100644 --- a/question/classes/bank/view.php +++ b/question/classes/bank/view.php @@ -17,6 +17,8 @@ namespace core_question\bank; +use core_question\bank\search\condition; + /** * Functions used to show question editing interface * @@ -50,21 +52,80 @@ namespace core_question\bank; class view { const MAX_SORTS = 3; + /** + * @var \moodle_url base URL for the current page. Used as the + * basis for making URLs for actions that reload the page. + */ protected $baseurl; + + /** + * @var \moodle_url used as a basis for URLs that edit a question. + */ protected $editquestionurl; - protected $quizorcourseid; + + /** + * @var \question_edit_contexts + */ protected $contexts; + + /** + * @var object|\cm_info|null if we are in a module context, the cm. + */ protected $cm; + + /** + * @var object the course we are within. + */ protected $course; - protected $visiblecolumns; - protected $extrarows; + + /** + * @var \question_bank_column_base[] these are all the 'columns' that are + * part of the display. Array keys are the class name. + */ protected $requiredcolumns; + + /** + * @var \question_bank_column_base[] these are the 'columns' that are + * actually displayed as a column, in order. Array keys are the class name. + */ + protected $visiblecolumns; + + /** + * @var \question_bank_column_base[] these are the 'columns' that are + * actually displayed as an additional row (e.g. question text), in order. + * Array keys are the class name. + */ + protected $extrarows; + + /** + * @var array list of column class names for which columns to sort on. + */ protected $sort; + + /** + * @var int|null id of the a question to highlight in the list (if present). + */ protected $lastchangedid; + + /** + * @var string SQL to count the number of questions matching the current + * search conditions. + */ protected $countsql; + + /** + * @var string SQL to actually load the question data to display. + */ protected $loadsql; + + /** + * @var array params used by $countsql and $loadsql (which currently must be the same). + */ protected $sqlparams; - /** @var array of \core_question\bank\search\condition objects. */ + + /** + * @var condition[] search conditions. + */ protected $searchconditions = array(); /** @@ -75,19 +136,11 @@ class view { * @param object $cm (optional) activity settings. */ public function __construct($contexts, $pageurl, $course, $cm = null) { - global $CFG, $PAGE; - $this->contexts = $contexts; $this->baseurl = $pageurl; $this->course = $course; $this->cm = $cm; - if (!empty($cm) && $cm->modname == 'quiz') { - $this->quizorcourseid = '&quizid=' . $cm->instance; - } else { - $this->quizorcourseid = '&courseid=' .$this->course->id; - } - // Create the url of the new question page to forward to. $returnurl = $pageurl->out_as_local_url(false); $this->editquestionurl = new \moodle_url('/question/question.php', @@ -102,7 +155,7 @@ class view { $this->init_columns($this->wanted_columns(), $this->heading_column()); $this->init_sort(); - $this->init_search_conditions($this->contexts, $this->course, $this->cm); + $this->init_search_conditions(); } /** @@ -124,9 +177,9 @@ class view { if (empty($CFG->questionbankcolumns)) { $questionbankcolumns = array('checkbox_column', 'question_type_column', - 'question_name_column', 'tags_action_column', 'edit_action_column', - 'copy_action_column', 'preview_action_column', 'delete_action_column', - 'creator_name_column', 'modifier_name_column'); + 'question_name_idnumber_tags_column', 'tags_action_column', 'edit_action_column', + 'copy_action_column', 'preview_action_column', 'delete_action_column', + 'creator_name_column', 'modifier_name_column'); } else { $questionbankcolumns = explode(',', $CFG->questionbankcolumns); } @@ -226,8 +279,10 @@ class view { /** * Deal with a sort name of the form columnname, or colname_subsort by - * breaking it up, validating the bits that are presend, and returning them. + * breaking it up, validating the bits that are present, and returning them. * If there is no subsort, then $subsort is returned as ''. + * + * @param string $sort the sort parameter to process. * @return array array($colname, $subsort). */ protected function parse_subsort($sort) { @@ -272,7 +327,7 @@ class view { } } // Deal with subsorts. - list($colname, $subsort) = $this->parse_subsort($sort); + list($colname) = $this->parse_subsort($sort); $this->requiredcolumns[$colname] = $this->get_column_type($colname); $this->sort[$sort] = $order; } @@ -296,7 +351,7 @@ class view { } /** - * @param $sort a column or column_subsort name. + * @param string $sort a column or column_subsort name. * @return int the current sort order for this column -1, 0, 1 */ public function get_primary_sort_order($sort) { @@ -311,6 +366,7 @@ class view { /** * Get a URL to redisplay the page with a new sort for the question bank. + * * @param string $sort the column, or column_subsort to sort on. * @param bool $newsortreverse whether to sort in reverse order. * @return string The new URL. @@ -336,7 +392,8 @@ class view { /** * Create the SQL query to retrieve the indicated questions - * @param stdClass $category no longer used. + * + * @param \stdClass $category no longer used. * @param bool $recurse no longer used. * @param bool $showhidden no longer used. * @deprecated since Moodle 2.7 MDL-40313. @@ -355,8 +412,6 @@ class view { * \core_question\bank\search\condition filters. */ protected function build_query() { - global $DB; - // Get the required tables and fields. $joins = array(); $fields = array('q.hidden', 'q.category'); @@ -402,12 +457,19 @@ class view { return $DB->count_records_sql($this->countsql, $this->sqlparams); } + /** + * Load the questions we need to display. + * + * @param int $page page to display. + * @param int $perpage number of questions per page. + * @return \moodle_recordset questionid => data about each question. + */ protected function load_page_questions($page, $perpage) { global $DB; $questions = $DB->get_recordset_sql($this->loadsql, $this->sqlparams, $page * $perpage, $perpage); - if (!$questions->valid()) { - // No questions on this page. Reset to page 0. + if (empty($questions)) { $questions->close(); + // No questions on this page. Reset to page 0. $questions = $DB->get_recordset_sql($this->loadsql, $this->sqlparams, 0, $perpage); } return $questions; @@ -424,7 +486,7 @@ class view { /** * Get the URL for duplicating a given question. * @param int $questionid the question id. - * @return moodle_url the URL. + * @return string the URL, HTML-escaped. */ public function copy_question_url($questionid) { return $this->editquestionurl->out(true, array('id' => $questionid, 'makecopy' => 1)); @@ -432,7 +494,7 @@ class view { /** * Get the context we are displaying the question bank for. - * @return context context object. + * @return \context context object. */ public function get_most_specific_context() { return $this->contexts->lowest(); @@ -440,8 +502,8 @@ class view { /** * Get the URL to preview a question. - * @param stdClass $questiondata the data defining the question. - * @return moodle_url the URL. + * @param \stdClass $questiondata the data defining the question. + * @return \moodle_url the URL. */ public function preview_question_url($questiondata) { return question_preview_url($questiondata->id, null, null, null, null, @@ -458,7 +520,15 @@ class view { * deleteselected Deletes the selected questions from the category * Other actions: * category Chooses the category - * displayoptions Sets display options + * + * @param string $tabname question bank edit tab name, for permission checking. + * @param int $page the page number to show. + * @param int $perpage the number of questions per page to show. + * @param string $cat 'categoryid,contextid'. + * @param int $recurse Whether to include subcategories. + * @param bool $showhidden whether deleted questions should be displayed. + * @param bool $showquestiontext whether the text of each question should be shown in the list. Deprecated. + * @param array $tagids current list of selected tags. */ public function display($tabname, $page, $perpage, $cat, $recurse, $showhidden, $showquestiontext, $tagids = []) { @@ -468,7 +538,7 @@ class view { return; } $editcontexts = $this->contexts->having_one_edit_tab_cap($tabname); - list($categoryid, $contextid) = explode(',', $cat); + list(, $contextid) = explode(',', $cat); $catcontext = \context::instance_by_id($contextid); $thiscontext = $this->get_most_specific_context(); // Category selection form. @@ -521,7 +591,7 @@ class view { /** * prints category information - * @param stdClass $category the category row from the database. + * @param \stdClass $category the category row from the database. * @deprecated since Moodle 2.7 MDL-40313. * @see \core_question\bank\search\condition * @todo MDL-41978 This will be deleted in Moodle 2.8 @@ -568,7 +638,7 @@ class view { */ protected function display_options($recurse, $showhidden, $showquestiontext) { debugging('display_options() is deprecated, please use display_options_form instead.', DEBUG_DEVELOPER); - return $this->display_options_form($showquestiontext); + $this->display_options_form($showquestiontext); } /** @@ -594,7 +664,7 @@ class view { /** * Display the form with options for which questions are displayed and how they are displayed. * @param bool $showquestiontext Display the text of the question within the list. - * @param string $path path to the script displaying this page. + * @param string $scriptpath path to the script displaying this page. * @param bool $showtextoption whether to include the 'Show question text' checkbox. */ protected function display_options_form($showquestiontext, $scriptpath = '/question/edit.php', @@ -619,7 +689,7 @@ class view { echo \html_writer::input_hidden_params($this->baseurl, $excludes); foreach ($this->searchconditions as $searchcondition) { - echo $searchcondition->display_options($this); + echo $searchcondition->display_options(); } if ($showtextoption) { $this->display_showtext_checkbox($showquestiontext); @@ -639,7 +709,7 @@ class view { print_collapsible_region_start('', 'advancedsearch', get_string('advancedsearchoptions', 'question'), 'question_bank_advanced_search'); foreach ($this->searchconditions as $searchcondition) { - echo $searchcondition->display_options_adv($this); + echo $searchcondition->display_options_adv(); } print_collapsible_region_end(); } @@ -666,7 +736,6 @@ class view { } protected function create_new_question_form($category, $canadd) { - global $CFG; echo '
'; if ($canadd) { create_new_question_button($category->id, $this->editquestionurl->params(), @@ -681,20 +750,20 @@ class view { * Prints the table of questions in a category with interactions * * @param array $contexts Not used! - * @param moodle_url $pageurl The URL to reload this page. + * @param \moodle_url $pageurl The URL to reload this page. * @param string $categoryandcontext 'categoryID,contextID'. - * @param stdClass $cm Not used! - * @param bool $recurse Whether to include subcategories. + * @param \stdClass $cm Not used! + * @param int $recurse Whether to include subcategories. * @param int $page The number of the page to be displayed * @param int $perpage Number of questions to show per page - * @param bool $showhidden whether deleted questions should be displayed. - * @param bool $showquestiontext whether the text of each question should be shown in the list. Deprecated. + * @param bool $showhidden Not used! This is now controlled in a different way. + * @param bool $showquestiontext Not used! This is now controlled in a different way. * @param array $addcontexts contexts where the user is allowed to add new questions. */ protected function display_question_list($contexts, $pageurl, $categoryandcontext, $cm = null, $recurse=1, $page=0, $perpage=100, $showhidden=false, $showquestiontext = false, $addcontexts = array()) { - global $CFG, $DB, $OUTPUT, $PAGE; + global $OUTPUT; // This function can be moderately slow with large question counts and may time out. // We probably do not want to raise it to unlimited, so randomly picking 5 minutes. @@ -715,11 +784,18 @@ class view { if ($totalnumber == 0) { return; } - $questions = $this->load_page_questions($page, $perpage); + $questionsrs = $this->load_page_questions($page, $perpage); + $questions = []; + foreach ($questionsrs as $question) { + $questions[$question->id] = $question; + } + $questionsrs->close(); + foreach ($this->requiredcolumns as $name => $column) { + $column->load_additional_data($questions); + } echo '
'; - $pageingurl = new \moodle_url('edit.php'); - $r = $pageingurl->params($pageurl->params()); + $pageingurl = new \moodle_url('edit.php', $pageurl->params()); $pagingbar = new \paging_bar($totalnumber, $page, $perpage, $pageingurl); $pagingbar->pagevar = 'qpage'; echo $OUTPUT->render($pagingbar); @@ -737,7 +813,6 @@ class view { $this->print_table_row($question, $rowcount); $rowcount += 1; } - $questions->close(); $this->end_table(); echo "
\n"; @@ -771,8 +846,8 @@ class view { * Display the controls at the bottom of the list of questions. * @param int $totalnumber Total number of questions that might be shown (if it was not for paging). * @param bool $recurse Whether to include subcategories. - * @param stdClass $category The question_category row from the database. - * @param context $catcontext The context of the category being displayed. + * @param \stdClass $category The question_category row from the database. + * @param \context $catcontext The context of the category being displayed. * @param array $addcontexts contexts where the user is allowed to add new questions. */ protected function display_bottom_controls($totalnumber, $recurse, $category, \context $catcontext, array $addcontexts) { @@ -865,7 +940,7 @@ class view { } public function process_actions() { - global $CFG, $DB; + global $DB; // Now, check for commands on this page and modify variables as necessary. if (optional_param('move', false, PARAM_BOOL) and confirm_sesskey()) { // Move selected questions to new category. @@ -886,7 +961,6 @@ class view { } if ($questionids) { list($usql, $params) = $DB->get_in_or_equal($questionids); - $sql = ""; $questions = $DB->get_records_sql(" SELECT q.*, c.contextid FROM {question} q @@ -976,11 +1050,13 @@ class view { return true; } + + return false; } /** * Add another search control to this view. - * @param \core_question\bank\search\condition $searchcondition the condition to add. + * @param condition $searchcondition the condition to add. */ public function add_searchcondition($searchcondition) { $this->searchconditions[] = $searchcondition; diff --git a/question/tests/behat/copy_questions.feature b/question/tests/behat/copy_questions.feature index e6e60967137..612e4fb576f 100644 --- a/question/tests/behat/copy_questions.feature +++ b/question/tests/behat/copy_questions.feature @@ -34,7 +34,7 @@ Feature: A teacher can duplicate questions in the question bank Then I should see "Duplicated question name" And I should see "Test question to be copied" And "Duplicated question name" row "Last modified by" column of "categoryquestions" table should contain "Teacher 1" - And "Test question to be copied" row "Created by" column of "categoryquestions" table should contain "Admin User" + And "Test question to be copied ID number qid" row "Created by" column of "categoryquestions" table should contain "Admin User" @javascript Scenario: Duplicated questions automatically get a new name suggested diff --git a/question/tests/behat/question_categories.feature b/question/tests/behat/question_categories.feature index 6d7edf25f96..b0786e07c7c 100644 --- a/question/tests/behat/question_categories.feature +++ b/question/tests/behat/question_categories.feature @@ -32,9 +32,11 @@ Feature: A teacher can put questions in categories in the question bank | Name | New Category 1 | | Parent category | Top | | Category info | Created as a test | + | ID number | newcatidnumber | And I press "submitbutton" - Then I should see "New Category 1 (0)" + Then I should see "New Category 1 ID number newcatidnumber (0)" And I should see "Created as a test" in the "New Category 1" "list_item" + And "New Category 1 [newcatidnumber]" "option" should exist in the "Parent category" "select" Scenario: A question category can be edited When I navigate to "Question bank > Categories" in current page administration diff --git a/question/tests/behat/question_categories_idnumber.feature b/question/tests/behat/question_categories_idnumber.feature index 695a5a45d5c..021fc14a288 100644 --- a/question/tests/behat/question_categories_idnumber.feature +++ b/question/tests/behat/question_categories_idnumber.feature @@ -35,7 +35,7 @@ Feature: A teacher can put questions with idnumbers in categories with idnumbers # Correction to a unique idnumber for the context. And I set the field "ID number" to "c1unused" And I press "Add category" - Then I should see "Sub used category (0)" + Then I should see "Sub used category ID number c1unused (0)" And I should see "Created as a test" in the "Sub used category" "list_item" Scenario: A question category can be edited and saved without changing the idnumber diff --git a/question/tests/behat/sort_questions.feature b/question/tests/behat/sort_questions.feature index 92ba9b531a6..7a5c84d1bd9 100644 --- a/question/tests/behat/sort_questions.feature +++ b/question/tests/behat/sort_questions.feature @@ -18,10 +18,10 @@ Feature: The questions in the question bank can be sorted in various ways | contextlevel | reference | name | | Course | C1 | Test questions | And the following "questions" exist: - | questioncategory | qtype | name | user | questiontext | - | Test questions | essay | A question 1 name | admin | Question 1 text | - | Test questions | essay | B question 2 name | teacher1 | Question 2 text | - | Test questions | numerical | C question 3 name | teacher1 | Question 3 text | + | questioncategory | qtype | name | user | questiontext | idnumber | + | Test questions | essay | A question 1 name | admin | Question 1 text | | + | Test questions | essay | B question 2 name | teacher1 | Question 2 text | | + | Test questions | numerical | C question 3 name | teacher1 | Question 3 text | numidnum | And I log in as "teacher1" And I am on "Course 1" course homepage And I navigate to "Question bank > Questions" in current page administration @@ -30,6 +30,12 @@ Feature: The questions in the question bank can be sorted in various ways Scenario: The questions are sorted by type by default Then "A question 1 name" "checkbox" should appear before "C question 3 name" "checkbox" + @javascript + Scenario: The questions can be sorted by idnumber + When I follow "Sort by ID number ascending" + Then "C question 3 name" "checkbox" should appear before "A question 1 name" "checkbox" + And I should see "numidnum" in the "C question 3 name" "table_row" + @javascript Scenario: The questions can be sorted in reverse order by type When I follow "Sort by Question type descending" @@ -37,14 +43,14 @@ Feature: The questions in the question bank can be sorted in various ways @javascript Scenario: The questions can be sorted by name - When I follow "Sort by Question ascending" + When I follow "Sort by Question name ascending" Then "A question 1 name" "checkbox" should appear before "B question 2 name" "checkbox" And "B question 2 name" "checkbox" should appear before "C question 3 name" "checkbox" @javascript Scenario: The questions can be sorted in reverse order by name - When I follow "Sort by Question ascending" - And I follow "Sort by Question descending" + When I follow "Sort by Question name ascending" + And I follow "Sort by Question name descending" Then "C question 3 name" "checkbox" should appear before "B question 2 name" "checkbox" And "B question 2 name" "checkbox" should appear before "A question 1 name" "checkbox" diff --git a/question/upgrade.txt b/question/upgrade.txt index 94f5e589a3c..cb8daf9eccd 100644 --- a/question/upgrade.txt +++ b/question/upgrade.txt @@ -1,5 +1,13 @@ This files describes API changes for code that uses the question API. +=== 3.8 === + +If you have customised the display of the question bank (using $CFG->questionbankcolumns) +then be aware that the default configuration has changed, and you may wish to make +equivalent changes in your customised version. The old column question_name_column +has been replaced by question_name_idnumber_tags_column. The old question_name_column +still exists, so it is safe to continue using it. + === 3.7 === The code for the is_valid_number function that was duplicated in the diff --git a/tag/classes/output/taglist.php b/tag/classes/output/taglist.php index df821f45664..dda618d7d19 100644 --- a/tag/classes/output/taglist.php +++ b/tag/classes/output/taglist.php @@ -45,6 +45,9 @@ class taglist implements templatable { /** @var string */ protected $label; + /** @var bool $accesshidelabel if true, the label should have class="accesshide" added. */ + protected $accesshidelabel; + /** @var string */ protected $classes; @@ -59,14 +62,17 @@ class taglist implements templatable { * to use default, set to '' (empty string) to omit the label completely * @param string $classes additional classes for the enclosing div element * @param int $limit limit the number of tags to display, if size of $tags is more than this limit the "more" link - * will be appended to the end, JS will toggle the rest of the tags + * will be appended to the end, JS will toggle the rest of the tags. 0 means no limit. * @param context $pagecontext specify if needed to overwrite the current page context for the view tag link + * @param bool $accesshidelabel if true, the label should have class="accesshide" added. */ - public function __construct($tags, $label = null, $classes = '', $limit = 10, $pagecontext = null) { + public function __construct($tags, $label = null, $classes = '', + $limit = 10, $pagecontext = null, $accesshidelabel = false) { global $PAGE; $canmanagetags = has_capability('moodle/tag:manage', \context_system::instance()); $this->label = ($label === null) ? get_string('tags') : $label; + $this->accesshidelabel = $accesshidelabel; $this->classes = $classes; $fromctx = $pagecontext ? $pagecontext->id : (($PAGE->context->contextlevel == CONTEXT_SYSTEM) ? 0 : $PAGE->context->id); @@ -106,6 +112,7 @@ class taglist implements templatable { return (object)array( 'tags' => array_values($this->tags), 'label' => $this->label, + 'accesshidelabel' => $this->accesshidelabel, 'tagscount' => $cnt, 'overflow' => ($this->limit && $cnt > $this->limit) ? 1 : 0, 'classes' => $this->classes, diff --git a/tag/classes/tag.php b/tag/classes/tag.php index 5047197da86..ba04be8dc5f 100644 --- a/tag/classes/tag.php +++ b/tag/classes/tag.php @@ -648,7 +648,8 @@ class core_tag_tag { * @param int[] $itemids * @param int $standardonly wether to return only standard tags or any * @param int $tiuserid tag instance user id, only needed for tag areas with user tagging - * @return core_tag_tag[] each object contains additional fields taginstanceid, taginstancecontextid and ordering + * @return core_tag_tag[][] first array key is itemid. For each itemid, + * an array tagid => tag object with additional fields taginstanceid, taginstancecontextid and ordering */ public static function get_items_tags($component, $itemtype, $itemids, $standardonly = self::BOTH_STANDARD_AND_NOT, $tiuserid = 0) { diff --git a/tag/templates/taglist.mustache b/tag/templates/taglist.mustache index 72975fd8068..7dc30a386c3 100644 --- a/tag/templates/taglist.mustache +++ b/tag/templates/taglist.mustache @@ -38,6 +38,7 @@ {"id":1,"name":"Mice","viewurl":"http://moodle.org/tag/index.php?tag=Mice","isstandard":"0","flag":0} ], "label": "Tags", + "accesshidelabel": false, "tagscount": 3, "overflow": 1, "classes": "someadditionalclass" @@ -47,7 +48,7 @@ {{#tagscount}}
{{#label}} - {{label}}: + {{label}}: {{/label}}
    {{#tags}}