Merge branch 'MDL-76149-master' of https://github.com/rezaies/moodle

grade/report/grader/db/upgrade.php

@@ -62,5 +62,10 @@ function xmldb_gradereport_grader_upgrade(int $oldversion): bool {
         upgrade_plugin_savepoint(true, 2023032100, 'grade', 'gradereport_grader');
     }
 
+    if ($oldversion < 2023032700) {
+        unset_config('grade_report_studentsperpage');
+        upgrade_plugin_savepoint(true, 2023032700, 'gradereport', 'grader');
+    }
+
     return true;
 }

grade/report/grader/index.php

@@ -43,6 +43,8 @@ $toggle_type   = optional_param('toggle_type', 0, PARAM_ALPHANUM);
 $graderreportsifirst  = optional_param('sifirst', null, PARAM_NOTAGS);
 $graderreportsilast   = optional_param('silast', null, PARAM_NOTAGS);
 
+$studentsperpage = optional_param('perpage', null, PARAM_INT);
+
 $PAGE->set_url(new moodle_url('/grade/report/grader/index.php', array('id'=>$courseid)));
 $PAGE->set_pagelayout('report');
 $PAGE->requires->js_call_amd('gradereport_grader/stickycolspan', 'init');
@@ -62,6 +64,10 @@ if (isset($graderreportsilast)) {
     $SESSION->gradereport["filtersurname-{$context->id}"] = $graderreportsilast;
 }
 
+if (isset($studentsperpage) && $studentsperpage >= 0) {
+    set_user_preference('grade_report_studentsperpage', $studentsperpage);
+}
+
 require_capability('gradereport/grader:view', $context);
 require_capability('moodle/grade:viewall', $context);
 
@@ -144,12 +150,6 @@ foreach ($warnings as $warning) {
     echo $OUTPUT->notification($warning);
 }
 
-$studentsperpage = $report->get_students_per_page();
-// Don't use paging if studentsperpage is empty or 0 at course AND site levels
-if (!empty($studentsperpage)) {
-    echo $OUTPUT->paging_bar($numusers, $report->page, $studentsperpage, $report->pbarurl);
-}
-
 $displayaverages = true;
 if ($numusers == 0) {
     $displayaverages = false;
@@ -157,6 +157,46 @@ if ($numusers == 0) {
 
 $reporthtml = $report->get_grade_table($displayaverages);
 
+$studentsperpage = $report->get_students_per_page();
+
+// Print per-page dropdown.
+$pagingoptions = grade_report_grader::PAGINATION_OPTIONS;
+if ($studentsperpage) {
+    $pagingoptions[] = $studentsperpage; // To make sure the current preference is within the options.
+}
+$pagingoptions = array_unique($pagingoptions);
+sort($pagingoptions);
+$pagingoptions = array_combine($pagingoptions, $pagingoptions);
+if ($numusers > grade_report_grader::MAX_STUDENTS_PER_PAGE) {
+    $pagingoptions['0'] = grade_report_grader::MAX_STUDENTS_PER_PAGE;
+} else {
+    $pagingoptions['0'] = get_string('all');
+}
+
+$perpagedata = [
+    'baseurl' => new moodle_url('/grade/report/grader/index.php', ['id' => s($courseid), 'report' => 'grader']),
+    'options' => []
+];
+foreach ($pagingoptions as $key => $name) {
+    $perpagedata['options'][] = [
+        'name' => $name,
+        'value' => $key,
+        'selected' => $key == $studentsperpage,
+    ];
+}
+
+$footercontent = html_writer::div(
+    $OUTPUT->render_from_template('gradereport_grader/perpage', $perpagedata)
+    , 'col-auto'
+);
+
+// The number of students per page is always limited even if it is claimed to be unlimited.
+$studentsperpage = $studentsperpage ?: grade_report_grader::MAX_STUDENTS_PER_PAGE;
+$footercontent .= html_writer::div(
+    $OUTPUT->paging_bar($numusers, $report->page, $studentsperpage, $report->pbarurl),
+    'col'
+);
+
 // print submit button
 if (!empty($USER->editing) && $report->get_pref('quickgrading')) {
     echo '<form action="index.php" enctype="application/x-www-form-urlencoded" method="post" id="gradereport_grader">'; // Enforce compatibility with our max_input_vars hack.
@@ -168,16 +208,21 @@ if (!empty($USER->editing) && $report->get_pref('quickgrading')) {
     echo '<input type="hidden" value="'.$page.'" name="page"/>';
     echo $gpr->get_form_fields();
     echo $reporthtml;
-    echo '<div class="submit"><input type="submit" id="gradersubmit" class="btn btn-primary"
-        value="'.s(get_string('savechanges')).'" /></div>';
+
+    $footercontent .= html_writer::div(
+        '<input type="submit" id="gradersubmit" class="btn btn-primary" value="'.s(get_string('savechanges')).'" />',
+        'col-auto'
+    );
+
+    $stickyfooter = new core\output\sticky_footer($footercontent);
+    echo $OUTPUT->render($stickyfooter);
+
     echo '</div></form>';
 } else {
     echo $reporthtml;
-}
 
-// prints paging bar at bottom for large pages
-if (!empty($studentsperpage) && $studentsperpage >= 20) {
-    echo $OUTPUT->paging_bar($numusers, $report->page, $studentsperpage, $report->pbarurl);
+    $stickyfooter = new core\output\sticky_footer($footercontent);
+    echo $OUTPUT->render($stickyfooter);
 }
 
 $event = \gradereport_grader\event\grade_report_viewed::create(

grade/report/grader/lib.php

@@ -102,6 +102,12 @@ class grade_report_grader extends grade_report {
      */
     public $canviewhidden;
 
+    /** @var int Maximum number of students that can be shown on one page */
+    public const MAX_STUDENTS_PER_PAGE = 5000;
+
+    /** @var int[] List of available options on the pagination dropdown */
+    public const PAGINATION_OPTIONS = [20, 100];
+
     /**
      * Allow category grade overriding
      * @var bool $overridecat
@@ -440,7 +446,8 @@ class grade_report_grader extends grade_report {
                    $this->userwheresql
                    $this->groupwheresql
               ORDER BY $sort";
-        $studentsperpage = $this->get_students_per_page();
+        // We never work with unlimited result. Limit the number of records by MAX_STUDENTS_PER_PAGE if no other limit is specified.
+        $studentsperpage = $this->get_students_per_page() ?: static::MAX_STUDENTS_PER_PAGE;
         $this->users = $DB->get_records_sql($sql, $params, $studentsperpage * $this->page, $studentsperpage);
 
         if (empty($this->users)) {
@@ -2001,6 +2008,7 @@ class grade_report_grader extends grade_report {
      * @return int The maximum number of students to display per page
      */
     public function get_students_per_page(): int {
-        return (int) $this->get_pref('studentsperpage');
+        // Default to the lowest available option.
+        return (int) get_user_preferences('grade_report_studentsperpage', min(static::PAGINATION_OPTIONS));
     }
 }

grade/report/grader/preferences_form.php

@@ -95,7 +95,6 @@ class grader_report_preferences_form extends moodleform {
 
         // View capability is the lowest permission. Users with grade:manage or grade:edit must also have grader:view
         if (has_capability('gradereport/grader:view', $context)) {
-            $preferences['prefgeneral']['studentsperpage'] = 'text';
             if (has_capability('moodle/course:viewsuspendedusers', $context)) {
                 $preferences['prefgeneral']['showonlyactiveenrol'] = $checkbox_default;
             }

grade/report/grader/settings.php

@@ -31,10 +31,6 @@ if ($ADMIN->fulltree) {
     $strreal                = get_string('real', 'grades');
     $strletter              = get_string('letter', 'grades');
 
-    /// Add settings for this module to the $settings object (it's already defined)
-    $settings->add(new admin_setting_configtext('grade_report_studentsperpage', get_string('studentsperpage', 'grades'),
-                                            get_string('studentsperpage_help', 'grades'), 100, PARAM_INT));
-
     $settings->add(new admin_setting_configcheckbox('grade_report_showonlyactiveenrol', get_string('showonlyactiveenrol', 'grades'),
                                                 get_string('showonlyactiveenrol_help', 'grades'), 1));
 

grade/report/grader/templates/perpage.mustache

@@ -0,0 +1,51 @@
+{{!
+    This file is part of Moodle - http://moodle.org/
+
+    Moodle is free software: you can redistribute it and/or modify
+    it under the terms of the GNU General Public License as published by
+    the Free Software Foundation, either version 3 of the License, or
+    (at your option) any later version.
+
+    Moodle is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+    GNU General Public License for more details.
+
+    You should have received a copy of the GNU General Public License
+    along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+}}
+{{!
+    @template gradereport_grader/perpage
+
+    The students per page dropdown element.
+
+    Context variables required for this template:
+    * baseurl - The base URL for the perpage action.
+    * options - Choices
+
+    Example context (json):
+    {
+        "baseurl": "http://example.com/grade/report/grader/index.php?id=2&report=grader",
+        "options": [
+            {"name": "20", "value": "20"},
+            {"name": "100", "value": "100", "selected": true},
+            {"name": "All", "value": "0"}
+        ]
+    }
+}}
+<label>
+    {{#str}}show{{/str}}
+    <select name="perpage" class="custom-select ignoredirty" id="{{uniqid}}">
+        {{#options}}
+            <option value="{{value}}" {{#selected}}selected{{/selected}}>{{{name}}}</option>
+        {{/options}}
+    </select>
+</label>
+{{#js}}
+    document.getElementById('{{uniqid}}').addEventListener('change', function(e) {
+        var url = new URL('{{baseurl}}');
+        url.searchParams.set('perpage', e.target.value);
+
+        window.location.href = url;
+    });
+{{/js}}

grade/report/grader/tests/behat/pagination.feature

@@ -0,0 +1,87 @@
+@gradereport @gradereport_grader
+Feature: grader report pagination
+  In order to consume the content of the report better
+  As a teacher
+  I need the report to be paginated
+
+  Background:
+    Given the following "courses" exist:
+      | fullname | shortname |
+      | Course 1 | C1        |
+    And the following "users" exist:
+      | username | firstname | lastname | email                |
+      | teacher1 | Teacher   | 1        | teacher1@example.com |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | teacher1 | C1     | editingteacher |
+
+  @javascript
+  Scenario: Default is used when teachers have no preference yet
+    Given "41" "users" exist with the following data:
+      | username  | student[count]             |
+      | firstname | Student                    |
+      | lastname  | [count]                    |
+      | email     | student[count]@example.com |
+    And "41" "course enrolments" exist with the following data:
+      | user   | student[count] |
+      | course | C1             |
+      | role   |student         |
+    When I am on the "Course 1" "Course" page logged in as "teacher1"
+    And I navigate to "View > Grader report" in the course gradebook
+    Then the field "perpage" matches value "20"
+    # Add 3 to the expected number because there are 2 header and 1 footer rows.
+    And I should see "23" node occurrences of type "tr" in the "user-grades" "table"
+    And I should see "3" in the ".stickyfooter .pagination" "css_element"
+    And I should not see "4" in the ".stickyfooter .pagination" "css_element"
+
+  @javascript
+  Scenario: Teachers can have their preference for the number of students
+    Given the following "courses" exist:
+      | fullname | shortname |
+      | Course 2 | C2        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | teacher1 | C2     | editingteacher |
+    When I am on the "Course 1" "Course" page logged in as "teacher1"
+    And I navigate to "View > Grader report" in the course gradebook
+    And I set the field "perpage" to "100"
+    And I am on the "Course 2" "Course" page
+    And I navigate to "View > Grader report" in the course gradebook
+    Then the field "perpage" matches value "100"
+
+  @javascript
+  Scenario: Teachers can change the number of students shown on the report
+    Given "101" "users" exist with the following data:
+      | username  | student[count]             |
+      | firstname | Student                    |
+      | lastname  | [count]                    |
+      | email     | student[count]@example.com |
+    And "101" "course enrolments" exist with the following data:
+      | user   | student[count] |
+      | course | C1             |
+      | role   |student         |
+    When I am on the "Course 1" "Course" page logged in as "teacher1"
+    And I navigate to "View > Grader report" in the course gradebook
+    And I set the field "perpage" to "100"
+    # Add 3 to the expected number because there are 2 header and 1 footer rows.
+    Then I should see "103" node occurrences of type "tr" in the "user-grades" "table"
+    And I should see "2" in the ".stickyfooter .pagination" "css_element"
+    And I should not see "3" in the ".stickyfooter .pagination" "css_element"
+
+  @javascript
+  Scenario: The pagination bar is only displayed when there is more than one page
+    Given "21" "users" exist with the following data:
+      | username  | student[count]             |
+      | firstname | Student                    |
+      | lastname  | [count]                    |
+      | email     | student[count]@example.com |
+    And "21" "course enrolments" exist with the following data:
+      | user   | student[count] |
+      | course | C1             |
+      | role   |student         |
+    When I am on the "Course 1" "Course" page logged in as "teacher1"
+    And I navigate to "View > Grader report" in the course gradebook
+    # By default, we have 20 students per page.
+    Then ".stickyfooter .pagination" "css_element" should exist
+    And I set the field "perpage" to "100"
+    Then ".stickyfooter .pagination" "css_element" should not exist

grade/report/grader/version.php

@@ -24,6 +24,6 @@
 
 defined('MOODLE_INTERNAL') || die();
 
-$plugin->version   = 2023032100;        // The current plugin version (Date: YYYYMMDDXX).
+$plugin->version   = 2023032700;        // The current plugin version (Date: YYYYMMDDXX).
 $plugin->requires  = 2022111800;        // Requires this Moodle version.
 $plugin->component = 'gradereport_grader'; // Full name of the plugin (used for diagnostics)

grade/report/upgrade.txt

@@ -14,6 +14,9 @@ information provided here is intended especially for developers.
 * A new method grade_report::get_report_links() is created to obtain links to other grade plugins report pages.
  It loops through all installed grade report plugins and checks if callback function gradereport_*_get_report_link
  is implemented in for given grade report plugin in the corresponding lib.php
+* The setting $CFG->grade_report_studentsperpage has been completely removed because it's not required anymore. This setting
+  was used to set the default number of students displayed per page in the grader report. Now the default is set to
+  20.
 
 === 3.6 ===
 * External function gradereport_user_external::get_grade_items now return the following information (only for course managers).

lang/en/deprecated.txt

@@ -86,3 +86,5 @@ showactivityicons,core_grades
 showactivityicons_help,core_grades
 showcalculations,core_grades
 showcalculations_help,core_grades
+studentsperpage,core_grades
+studentsperpage_help,core_grades

lang/en/grades.php

@@ -809,8 +809,6 @@ $string['standarddeviation'] = 'Standard deviation';
 $string['stats'] = 'Statistics';
 $string['statslink'] = 'Stats';
 $string['student'] = 'Student';
-$string['studentsperpage'] = 'Students per page';
-$string['studentsperpage_help'] = 'This setting determines the number of students displayed per page in the grader report.';
 $string['studentsperpagereduced'] = 'Reduced maximum students per page from {$a->originalstudentsperpage} to {$a->studentsperpage}. Consider increasing the PHP setting max_input_vars from {$a->maxinputvars}.';
 $string['subcategory'] = 'Normal category';
 $string['submissions'] = 'Submissions';
@@ -914,3 +912,5 @@ $string['showactivityicons'] = 'Show activity icons';
 $string['showactivityicons_help'] = 'If enabled, activity icons are shown next to activity names.';
 $string['showcalculations'] = 'Show calculations';
 $string['showcalculations_help'] = 'If enabled, when editing, a calculator icon is shown for each grade item and category with a visual indicator that a grade item is calculated.';
+$string['studentsperpage'] = 'Students per page';
+$string['studentsperpage_help'] = 'This setting determines the number of students displayed per page in the grader report.';

lib/form/amd/src/changechecker.js

@@ -253,7 +253,8 @@ export const isAnyWatchedFormDirty = () => {
         // Elements currently holding focus will not have triggered change detection.
         // Check whether the value matches the original value upon form load.
         if (document.activeElement && document.activeElement.dataset.propertyIsEnumerable('initialValue')) {
-            const isActiveElementWatched = isWatchingForm(document.activeElement);
+            const isActiveElementWatched = isWatchingForm(document.activeElement)
+                && !shouldIgnoreChangesForNode(document.activeElement);
             const hasValueChanged = document.activeElement.dataset.initialValue !== document.activeElement.value;
 
             if (isActiveElementWatched && hasValueChanged) {