From 65cafd36cc40bfeb4e6d45b445dd0371d66a2b42 Mon Sep 17 00:00:00 2001 From: Ilya Tregubov Date: Fri, 17 Mar 2023 11:29:23 +0800 Subject: [PATCH 1/3] MDL-77632 core_grades: Move select checkboxes to name column. --- grade/edit/tree/lib.php | 114 +++++++++++++++++++++++++++-- grade/upgrade.txt | 2 +- lang/en/grades.php | 2 +- theme/boost/scss/moodle/grade.scss | 2 - theme/boost/style/moodle.css | 3 - theme/classic/style/moodle.css | 3 - 6 files changed, 108 insertions(+), 18 deletions(-) diff --git a/grade/edit/tree/lib.php b/grade/edit/tree/lib.php index 7fe400db8de..267887a8f14 100644 --- a/grade/edit/tree/lib.php +++ b/grade/edit/tree/lib.php @@ -69,10 +69,6 @@ class grade_edit_tree { $this->columns[] = grade_edit_tree_column::factory('status'); $this->columns[] = grade_edit_tree_column::factory('actions'); - if ($this->deepest_level > 1) { - $this->columns[] = grade_edit_tree_column::factory('select'); - } - $this->table = new html_table(); $this->table->id = "grade_edit_tree_table"; $this->table->attributes['class'] = 'generaltable simple setup-grades'; @@ -818,16 +814,41 @@ class grade_edit_tree_column_name extends grade_edit_tree_column { 'category' => $params['eid'] ]); + $mastercheckbox = ''; + if ($this->deepest_level > 1) { + if (empty($params['eid'])) { + throw new Exception('Array key (eid) missing from 3rd param of ' . + 'grade_edit_tree_column_select::get_category_cell($category, $levelclass, $params)'); + } + + // Get toggle group for this master checkbox. + $togglegroup = $this->get_checkbox_togglegroup($category); + // Set label for this master checkbox. + $masterlabel = $params['level'] === 1 ? get_string('all') : $params['name']; + // Build the master checkbox. + $mastercheckbox = new \core\output\checkbox_toggleall($togglegroup, true, [ + 'id' => $togglegroup, + 'name' => $togglegroup, + 'value' => 1, + 'classes' => 'itemselect ignoredirty mr-2', + 'label' => $masterlabel, + // Consistent label to prevent the select column from resizing. + 'selectall' => $masterlabel, + 'deselectall' => $masterlabel, + 'labelclasses' => 'accesshide m-0', + ]); + + $mastercheckbox = $OUTPUT->render($mastercheckbox); + } + $moveaction = isset($params['moveaction']) ? $params['moveaction'] : ''; $categorycell = parent::get_category_cell($category, $levelclass, $params); $categorycell->colspan = ($this->deepest_level + 2) - $params['level']; - $categorycell->text = html_writer::div($visibilitytoggle . $moveaction . $params['name'], 'font-weight-bold'); + $categorycell->text = html_writer::div($mastercheckbox . $visibilitytoggle . $moveaction . $params['name'], 'font-weight-bold'); return $categorycell; } public function get_item_cell($item, $params) { - global $CFG; - if (empty($params['element']) || empty($params['name']) || empty($params['level'])) { throw new Exception('Array key (name, level or element) missing from 2nd param of grade_edit_tree_column_name::get_item_cell($item, $params)'); } @@ -849,9 +870,56 @@ class grade_edit_tree_column_name extends grade_edit_tree_column { $itemcell = parent::get_item_cell($item, $params); $itemcell->colspan = ($this->deepest_level + 1) - $params['level']; - $itemcell->text = \html_writer::div($moveaction . $itemicon . $content, "{$params['itemtype']} d-flex align-items-center"); + + $checkbox = ''; + if (($this->deepest_level > 1) && ($params['itemtype'] != 'course') && ($params['itemtype'] != 'category')) { + global $OUTPUT; + + $label = get_string('select', 'grades', $params['name']); + + if (empty($params['itemtype']) || empty($params['eid'])) { + throw new \moodle_exception('missingitemtypeoreid', 'core_grades'); + } + + // Fetch the grade item's category. + $category = $item->get_parent_category(); + $togglegroup = $this->get_checkbox_togglegroup($category); + + $checkboxid = 'select_' . $params['eid']; + $checkbox = new \core\output\checkbox_toggleall($togglegroup, false, [ + 'id' => $checkboxid, + 'name' => $checkboxid, + 'label' => $label, + 'labelclasses' => 'accesshide', + 'classes' => 'itemselect ignoredirty mr-2', + ]); + $checkbox = $OUTPUT->render($checkbox); + } + + $itemcell->text = \html_writer::div($checkbox . $moveaction . $itemicon . $content, + "{$params['itemtype']} d-flex align-items-center"); return $itemcell; } + + /** + * Generates a toggle group name for a bulk-action checkbox based on the given grade category. + * + * @param grade_category $category The grade category. + * @return string + */ + protected function get_checkbox_togglegroup(grade_category $category): string { + $levels = []; + $categories = explode('/', $category->path); + foreach ($categories as $categoryid) { + $level = 'category' . $categoryid; + if (!in_array($level, $levels)) { + $levels[] = 'category' . $categoryid; + } + } + $togglegroup = implode(' ', $levels); + + return $togglegroup; + } } /** @@ -1089,20 +1157,37 @@ class grade_edit_tree_column_actions extends grade_edit_tree_column { /** * Class grade_edit_tree_column_select * + * @deprecated Since Moodle 4.3. + * @todo Final deprecation on Moodle 4.7 MDL-77668 + * * @package core_grades * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class grade_edit_tree_column_select extends grade_edit_tree_column { + /** + * @deprecated Since Moodle 4.3. + * @todo Final deprecation on Moodle 4.7 MDL-77668 + */ public function get_header_cell() { + debugging('Method grade_edit_tree_column_select::get_header_cell() is deprecated, ' . + 'please do not use it anymore.', DEBUG_DEVELOPER); + $headercell = clone($this->headercell); $headercell->text = get_string('select'); return $headercell; } + /** + * @deprecated Since Moodle 4.3. + * @todo Final deprecation on Moodle 4.7 MDL-77668 + */ public function get_category_cell($category, $levelclass, $params) { global $OUTPUT; + debugging('Method grade_edit_tree_column_select::get_category_cell() is deprecated, ' . + 'please do not use it anymore.', DEBUG_DEVELOPER); + if (empty($params['eid'])) { throw new Exception('Array key (eid) missing from 3rd param of grade_edit_tree_column_select::get_category_cell($category, $levelclass, $params)'); } @@ -1137,7 +1222,14 @@ class grade_edit_tree_column_select extends grade_edit_tree_column { return $categorycell; } + /** + * @deprecated Since Moodle 4.3. + * @todo Final deprecation on Moodle 4.7 MDL-77668 + */ public function get_item_cell($item, $params) { + debugging('Method grade_edit_tree_column_select::get_item_cell() is deprecated, ' . + 'please do not use it anymore.', DEBUG_DEVELOPER); + if (empty($params['itemtype']) || empty($params['eid'])) { throw new \moodle_exception('missingitemtypeoreid', 'core_grades'); } @@ -1166,10 +1258,16 @@ class grade_edit_tree_column_select extends grade_edit_tree_column { /** * Generates a toggle group name for a bulk-action checkbox based on the given grade category. * + * @deprecated Since Moodle 4.3. + * @todo Final deprecation on Moodle 4.7 MDL-77668 + * * @param grade_category $category The grade category. * @return string */ protected function get_checkbox_togglegroup(grade_category $category): string { + debugging('Method grade_edit_tree_column_select::get_checkbox_togglegroup() is deprecated, ' . + 'please do not use it anymore.', DEBUG_DEVELOPER); + $levels = []; $categories = explode('/', $category->path); foreach ($categories as $categoryid) { diff --git a/grade/upgrade.txt b/grade/upgrade.txt index 37b7c0f23b5..74f47466277 100644 --- a/grade/upgrade.txt +++ b/grade/upgrade.txt @@ -2,10 +2,10 @@ This file describes API changes in /grade/* ; Information provided here is intended especially for developers. === 4.3 === - * The $showtitle parameter in the print_grade_page_head function located inside grade/lib.php has been deprecated and is not used anymore. * The deprecated `core_grades_create_gradecategory` external method has been removed, in addition to the accompanying `core_grades_external::create_gradecategory` methods +* The grade_edit_tree_column_select class has been deprecated. === 4.1 === * The $importactiveurl parameter in the constructor of the core_grades\output\import_action_bar class has been deprecated and is not used anymore. diff --git a/lang/en/grades.php b/lang/en/grades.php index 153153d2123..f11becc1932 100644 --- a/lang/en/grades.php +++ b/lang/en/grades.php @@ -507,7 +507,7 @@ $string['minmaxupgradefixbutton'] = 'Resolve inconsistencies'; $string['minmaxupgradewarning'] = 'Note: An inconsistency has been detected with some grades due to a change in the minimum and maximum grades used when calculating the grade displayed in the gradebook. It is recommended that the inconsistency is resolved by clicking the button below, though this will result in some grades being changed.'; $string['minimum_show'] = 'Show minimum grade'; $string['minimum_show_help'] = 'Minimum grade is used in calculating grades and weights. If not shown, minimum grade will default to zero and cannot be edited.'; -$string['missingitemtypeoreid'] = 'Array key (itemtype or eid) missing from 2nd param of grade_edit_tree_column_select::get_item_cell($item, $params)'; +$string['missingitemtypeoreid'] = 'Array key (itemtype or eid) missing'; $string['missingscale'] = 'Scale must be selected'; $string['mode'] = 'Mode'; $string['modgrade'] = 'Grade'; diff --git a/theme/boost/scss/moodle/grade.scss b/theme/boost/scss/moodle/grade.scss index 1d8325928f1..57b7036b36b 100644 --- a/theme/boost/scss/moodle/grade.scss +++ b/theme/boost/scss/moodle/grade.scss @@ -355,8 +355,6 @@ vertical-align: middle; &.column-name { - padding-left: 38px; - .small { font-size: 70%; } diff --git a/theme/boost/style/moodle.css b/theme/boost/style/moodle.css index 804021cb2e6..2a1abe87ab7 100644 --- a/theme/boost/style/moodle.css +++ b/theme/boost/style/moodle.css @@ -34951,9 +34951,6 @@ p.arrow_button { border: none; vertical-align: middle; } -.path-grade-edit-tree .gradetree-wrapper .setup-grades.generaltable tr td.column-name { - padding-left: 38px; -} .path-grade-edit-tree .gradetree-wrapper .setup-grades.generaltable tr td.column-name .small { font-size: 70%; } diff --git a/theme/classic/style/moodle.css b/theme/classic/style/moodle.css index 50d36935518..ce4987e1879 100644 --- a/theme/classic/style/moodle.css +++ b/theme/classic/style/moodle.css @@ -34951,9 +34951,6 @@ p.arrow_button { border: none; vertical-align: middle; } -.path-grade-edit-tree .gradetree-wrapper .setup-grades.generaltable tr td.column-name { - padding-left: 38px; -} .path-grade-edit-tree .gradetree-wrapper .setup-grades.generaltable tr td.column-name .small { font-size: 70%; } From 3643f48e0f402aa16724301a49a30a6a7de328b3 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Mon, 12 Jun 2023 13:06:13 +0800 Subject: [PATCH 2/3] MDL-77632 core_grades: Fix row_column_of_table_should_contain() --- grade/edit/tree/lib.php | 12 ++---------- lib/tests/behat/behat_general.php | 4 ++-- 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/grade/edit/tree/lib.php b/grade/edit/tree/lib.php index 267887a8f14..5cdc18ea358 100644 --- a/grade/edit/tree/lib.php +++ b/grade/edit/tree/lib.php @@ -855,16 +855,8 @@ class grade_edit_tree_column_name extends grade_edit_tree_column { $itemicon = \html_writer::div($params['icon'], 'mr-1'); $itemtype = \html_writer::span($params['type'], 'd-block text-uppercase small dimmed_text'); - - // Generate the content for a cell that represents a grade item. - // If a behat test site is running avoid outputting the information about the type of the grade item. - // This additional information causes issues in behat particularly with the existing xpath used to - // interact with table elements. - if (!defined('BEHAT_SITE_RUNNING')) { - $content = \html_writer::div($itemtype . $params['name']); - } else { - $content = \html_writer::div($params['name']); - } + $itemtitle = html_writer::div($params['name'], 'rowtitle'); + $content = \html_writer::div($itemtype . $itemtitle); $moveaction = isset($params['moveaction']) ? $params['moveaction'] : ''; diff --git a/lib/tests/behat/behat_general.php b/lib/tests/behat/behat_general.php index 9f8f292a857..c5f97ed3ce7 100644 --- a/lib/tests/behat/behat_general.php +++ b/lib/tests/behat/behat_general.php @@ -1456,8 +1456,8 @@ EOF; // Check if value exists in specific row/column. // Get row xpath. // GoutteDriver uses DomCrawler\Crawler and it is making XPath relative to the current context, so use descendant. - $rowxpath = $tablexpath."/tbody/tr[descendant::th[normalize-space(.)=" . $rowliteral . - "] | descendant::td[normalize-space(.)=" . $rowliteral . "]]"; + $rowxpath = $tablexpath . "/tbody/tr[descendant::*[@class='rowtitle'][normalize-space(.)=" . $rowliteral . "] | " . " + descendant::th[normalize-space(.)=" . $rowliteral . "] | descendant::td[normalize-space(.)=" . $rowliteral . "]]"; $columnvaluexpath = $rowxpath . $columnpositionxpath . "[contains(normalize-space(.)," . $valueliteral . ")]"; From b4cbad8b33a333130e0950e5eaef0dde66b83994 Mon Sep 17 00:00:00 2001 From: Mihail Geshoski Date: Mon, 12 Jun 2023 14:31:44 +0800 Subject: [PATCH 3/3] MDL-77632 core_grades: Add rowtitle class to grade category cell --- grade/edit/tree/lib.php | 3 ++- grade/tests/behat/toggle_grade_categories.feature | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/grade/edit/tree/lib.php b/grade/edit/tree/lib.php index 5cdc18ea358..5d939e36a8b 100644 --- a/grade/edit/tree/lib.php +++ b/grade/edit/tree/lib.php @@ -844,7 +844,8 @@ class grade_edit_tree_column_name extends grade_edit_tree_column { $moveaction = isset($params['moveaction']) ? $params['moveaction'] : ''; $categorycell = parent::get_category_cell($category, $levelclass, $params); $categorycell->colspan = ($this->deepest_level + 2) - $params['level']; - $categorycell->text = html_writer::div($mastercheckbox . $visibilitytoggle . $moveaction . $params['name'], 'font-weight-bold'); + $rowtitle = html_writer::div($params['name'], 'rowtitle'); + $categorycell->text = html_writer::div($mastercheckbox . $visibilitytoggle . $moveaction . $rowtitle, 'font-weight-bold'); return $categorycell; } diff --git a/grade/tests/behat/toggle_grade_categories.feature b/grade/tests/behat/toggle_grade_categories.feature index 64a26841816..7965fb915d6 100644 --- a/grade/tests/behat/toggle_grade_categories.feature +++ b/grade/tests/behat/toggle_grade_categories.feature @@ -126,10 +126,12 @@ Feature: Teachers can toggle the visibility of the grade categories in the Grade And I should not see "Course total" in the "setup-grades" "table" # Expand the grade category 'Course'. The aggregated max grade should not be displayed within the 'Course' row anymore. And I click on "Expand" "link" in the "Course" "table_row" + And the following should exist in the "setup-grades" table: | Name | Max grade | | Course | | | Category 1 | 140.00 | | Course total | 240.00 | + And I should not see "Category 1 total" in the "setup-grades" "table" Scenario: A teacher can collapse and expand grade categories in the Gradebook setup when moving grade items Given I navigate to "Setup > Gradebook setup" in the course gradebook