MDL-74234 assign: Fix grading page for filtered out users

lang/en/error.php

@@ -599,6 +599,7 @@ $string['useradminodelete'] = 'Administrator accounts cannot be deleted.';
 $string['userautherror'] = 'Unknown auth plugin';
 $string['userauthunsupported'] = 'Auth plugin not supported here';
 $string['useremailduplicate'] = 'Duplicate address';
+$string['userisfilteredout'] = 'This user does not match filters and table preferences!';
 $string['usermustbemnet'] = 'Users in the MNET access control list must be remote MNET users';
 $string['usernamelowercase'] = 'The username must be in lower case';
 $string['usernotaddederror'] = 'User not added - error';

mod/assign/amd/src/grading_navigation.js

@@ -40,6 +40,14 @@ define(['jquery', 'core/notification', 'core/str', 'core/form-autocomplete',
         this._lastXofYUpdate = 0;
         this._firstLoadUsers = true;
 
+        let url = new URL(window.location);
+        if (parseInt(url.searchParams.get('treset')) > 0) {
+            // Remove 'treset' url parameter to make sure that
+            // table preferences won't be reset on page refresh.
+            url.searchParams.delete('treset');
+            window.history.replaceState({}, "", url);
+        }
+
         // Get the current user list from a webservice.
         this._loadAllUsers();
 
@@ -50,6 +58,7 @@ define(['jquery', 'core/notification', 'core/str', 'core/form-autocomplete',
         this._region.find('[data-action="next-user"]').on('click', this._handleNextUser.bind(this));
         this._region.find('[data-action="change-user"]').on('change', this._handleChangeUser.bind(this));
         this._region.find('[data-region="user-filters"]').on('click', this._toggleExpandFilters.bind(this));
+        this._region.find('[data-region="user-resettable"]').on('click', this._toggleResetTable.bind());
 
         $(document).on('user-changed', this._refreshSelector.bind(this));
         $(document).on('done-saving-show-next', this._handleNextUser.bind(this));
@@ -255,7 +264,7 @@ define(['jquery', 'core/notification', 'core/str', 'core/form-autocomplete',
             // Reload the list of users to apply the new filters.
             if (!this._loadAllUsers()) {
                 var userid = parseInt(select.attr('data-selected'));
-                var foundIndex = 0;
+                let foundIndex = null;
                 // Search the returned users for the current selection.
                 $.each(this._filteredUsers, function(index, user) {
                     if (userid == user.id) {
@@ -263,7 +272,7 @@ define(['jquery', 'core/notification', 'core/str', 'core/form-autocomplete',
                     }
                 });
 
-                if (this._filteredUsers.length) {
+                if (this._filteredUsers.length && foundIndex !== null) {
                     this._selectUserById(this._filteredUsers[foundIndex].id);
                 } else {
                     this._selectNoUser();
@@ -365,6 +374,18 @@ define(['jquery', 'core/notification', 'core/str', 'core/form-autocomplete',
         }
     };
 
+    /**
+     * Reset table preferences.
+     *
+     * @private
+     * @method _toggleResetTable
+     */
+    GradingNavigation.prototype._toggleResetTable = function() {
+        let url = new URL(window.location);
+        url.searchParams.set('treset', '1');
+        window.location.href = url;
+    };
+
     /**
      * Change to the previous user in the grading list.
      *

mod/assign/externallib.php

@@ -2846,6 +2846,12 @@ class mod_assign_external extends \mod_assign\external\external_api {
             throw new moodle_exception('usernotincourse');
         }
 
+        $filtered = $assign->is_userid_filtered($userid);
+        if (!$filtered) {
+            // User is filtered out by user filters or table preferences.
+            throw new moodle_exception('userisfilteredout');
+        }
+
         $return = array(
             'id' => $participant->id,
             'fullname' => $participant->fullname,

mod/assign/lib.php

@@ -1438,6 +1438,14 @@ function mod_assign_output_fragment_gradingpanel($args) {
     $assign = new assign($context, null, null);
 
     $userid = clean_param($args['userid'], PARAM_INT);
+
+    $participant = $assign->get_participant($userid);
+    $isfiltered = $assign->is_userid_filtered($userid);
+    if (!$participant || !$isfiltered) {
+        // User is not enrolled or filtered out by filters and table preferences.
+        return '';
+    }
+
     $attemptnumber = clean_param($args['attemptnumber'], PARAM_INT);
     $formdata = array();
     if (!empty($args['jsonformdata'])) {

mod/assign/locallib.php

@@ -2637,6 +2637,17 @@ class assign {
         return $useridlist;
     }
 
+    /**
+     * Is user id filtered by user filters and table preferences.
+     *
+     * @param int $userid User id that needs to be checked.
+     * @return bool
+     */
+    public function is_userid_filtered($userid) {
+        $users = $this->get_grading_userid_list();
+        return in_array($userid, $users);
+    }
+
     /**
      * Finds all assignment notifications that have yet to be mailed out, and mails them.
      *
@@ -4778,6 +4789,10 @@ class assign {
             $userid = $this->get_user_id_for_uniqueid($blindid);
         }
 
+        // Instantiate table object to apply table preferences.
+        $gradingtable = new assign_grading_table($this, 10, '', 0, false);
+        $gradingtable->setup();
+
         $currentgroup = groups_get_activity_group($this->get_course_module(), true);
         $framegrader = new grading_app($userid, $currentgroup, $this);
 

mod/assign/templates/grading_navigation_user_selector.mustache

@@ -29,6 +29,9 @@
     * see mod/assign/classes/output/grading_app.php
 
     This template uses ajax functionality, so it cannot be shown in the template library.
+
+    Example context (json):
+    { }
 }}
 <a href="#previous" data-action="previous-user" aria-label="{{#str}} previoususer, mod_assign {{/str}}" title="{{#str}} previoususer, mod_assign {{/str}}">{{{larrow}}}</a>
 <span data-region="input-field">
@@ -55,6 +58,9 @@
     </span>
     {{#pix}}i/filter{{/pix}}
 </a>
+<a href="#" data-region="user-resettable" title="{{#str}}resettable{{/str}}">
+    {{#str}}resettable{{/str}}
+</a>
 <div data-region="configure-filters" id="filter-configuration-{{uniqid}}" class="card card-large p-2">
     <form>
         <span class="row px-3 py-1">

mod/assign/tests/behat/assign_table_preferences.feature

+@mod @mod_assign
+Feature: In an assignment, teachers can use table preferences.
+  In order to improve grading process
+  As a teacher
+  I need to be able to filter students by first and last name.
+
+  Background:
+    Given the following "users" exist:
+      | username | firstname | lastname | email                |
+      | student1 | Student   | One      | student1@example.com |
+      | student2 | Student   | Two      | student2@example.com |
+      | teacher1 | Darrell   | Teacher1 | teacher1@example.com |
+    And the following "courses" exist:
+      | fullname | shortname | enablecompletion | showcompletionconditions |
+      | Course 1 | C1        | 1                | 1                        |
+    And the following "course enrolments" exist:
+      | user     | course | role           |
+      | student1 | C1     | student        |
+      | student2 | C1     | student        |
+      | teacher1 | C1     | editingteacher |
+    And the following "activity" exists:
+      | activity                            | assign             |
+      | course                              | C1                 |
+      | name                                | Test assignment    |
+      | assignsubmission_onlinetext_enabled | 1                  |
+    And I log out
+    And I log in as "student1"
+    And I am on the "Test assignment" Activity page
+    And I press "Add submission"
+    And I set the following fields to these values:
+      | Online text | This is a submission for Student One |
+    And I press "Save changes"
+    And I press "Submit assignment"
+    And I press "Continue"
+    And I log out
+    And I log in as "student2"
+    And I am on the "Test assignment" Activity page
+    And I press "Add submission"
+    And I set the following fields to these values:
+      | Online text | This is a submission for Student Two |
+    And I press "Save changes"
+    And I press "Submit assignment"
+    And I press "Continue"
+
+  @javascript
+  Scenario: As a teacher I can filter student submissions on the View all submission page
+    When I log in as "teacher1"
+    And I am on the "Test assignment" Activity page
+    And I follow "View all submissions"
+    And I click on "T" "link" in the ".lastinitial" "css_element"
+    And I click on "Grade" "link" in the "Student Two" "table_row"
+    And I should see "This is a submission for Student Two"
+    And I should see "1 of 1"
+    And I follow "Reset table preferences"
+    Then I should see "This is a submission for Student Two"
+    And I should see "2 of 2"
+    And I log out

mod/assign/tests/behat/filter_drafts.feature

@@ -56,6 +56,9 @@ Feature: In an assignment, teachers can filter displayed submissions and see dra
     When I click on "[data-region=user-filters]" "css_element"
     And I set the field "filter" to "Draft"
     Then I should see "1 of 1"
+    And I should see "No users selected"
+    And I click on "[data-region=user-selector]" "css_element"
+    And I type "Student"
     And I should see "Student 2"
     And I should not see "Student 1"
     And I should not see "Student 3"

mod/assign/tests/locallib_test.php

@@ -4528,4 +4528,71 @@ Anchor link 2:<a title=\"bananas\" href=\"../logo-240x60.gif\">Link text</a>
 
         return array($assign, $instance, $student);
     }
+
+    /**
+     * Test user filtering by First name, Last name and Submission status.
+     *
+     * @covers \assign::is_userid_filtered
+     */
+    public function test_is_userid_filtered() {
+        $this->resetAfterTest();
+
+        // Generate data and simulate student submissions.
+        $course = $this->getDataGenerator()->create_course();
+        $params1 = ['firstname' => 'Valentin', 'lastname' => 'Ivanov'];
+        $student1 = $this->getDataGenerator()->create_and_enrol($course, 'student', $params1);
+        $params2 = ['firstname' => 'Nikolay', 'lastname' => 'Petrov'];
+        $student2 = $this->getDataGenerator()->create_and_enrol($course, 'student', $params2);
+        $assign = $this->create_instance($course, ['assignsubmission_onlinetext_enabled' => 1]);
+        $teacher = $this->getDataGenerator()->create_and_enrol($course, 'teacher');
+        $this->setUser($student1);
+        $submission = $assign->get_user_submission($student1->id, true);
+        $submission->status = ASSIGN_SUBMISSION_STATUS_SUBMITTED;
+        $assign->testable_update_submission($submission, $student1->id, true, false);
+        $this->setUser($student2);
+        $submission = $assign->get_user_submission($student2->id, true);
+        $submission->status = ASSIGN_SUBMISSION_STATUS_DRAFT;
+        $assign->testable_update_submission($submission, $student2->id, true, false);
+        $this->setUser($teacher);
+
+        // By default, both users should match filters.
+        $this->AssertTrue($assign->is_userid_filtered($student1->id));
+        $this->AssertTrue($assign->is_userid_filtered($student2->id));
+
+        // Filter by First name starting with V.
+        $_GET['tifirst'] = 'V';
+        $this->AssertTrue($assign->is_userid_filtered($student1->id));
+        $this->AssertFalse($assign->is_userid_filtered($student2->id));
+
+        // Add Last name to filter out both users.
+        $_GET['tilast'] = 'G';
+        $this->AssertFalse($assign->is_userid_filtered($student1->id));
+        $this->AssertFalse($assign->is_userid_filtered($student2->id));
+
+        // Unsetting variables doesn't change behaviour because filters are stored in user preferences.
+        unset($_GET['tifirst']);
+        unset($_GET['tilast']);
+        $this->AssertFalse($assign->is_userid_filtered($student1->id));
+        $this->AssertFalse($assign->is_userid_filtered($student2->id));
+
+        // Reset table preferences.
+        $_GET['treset'] = '1';
+        $this->AssertTrue($assign->is_userid_filtered($student1->id));
+        $this->AssertTrue($assign->is_userid_filtered($student2->id));
+
+        // Display users with submitted submissions only.
+        set_user_preference('assign_filter', ASSIGN_SUBMISSION_STATUS_SUBMITTED);
+        $this->AssertTrue($assign->is_userid_filtered($student1->id));
+        $this->AssertFalse($assign->is_userid_filtered($student2->id));
+
+        // Display users with drafts.
+        set_user_preference('assign_filter', ASSIGN_SUBMISSION_STATUS_DRAFT);
+        $this->AssertFalse($assign->is_userid_filtered($student1->id));
+        $this->AssertTrue($assign->is_userid_filtered($student2->id));
+
+        // Reset the filter.
+        set_user_preference('assign_filter', '');
+        $this->AssertTrue($assign->is_userid_filtered($student1->id));
+        $this->AssertTrue($assign->is_userid_filtered($student2->id));
+    }
 }

mod/assign/upgrade.txt

 This files describes API changes in the assign code.
+=== 4.1 ===
+* New method \assign::is_userid_filtered() has been implemented. It returns false if user id is filtered out by either
+  user preferences for grading table or submission status filter. Otherwise, returns true.
+
 === 4.0 ===
 * The method \assign::grading_disabled() now has optional $gradinginfo parameter to improve performance
 * Renderer (renderer.php) has been moved from mod root to classes/output/ to be more PSR compliant.