From 682aea9b84ecf4c339558855f7ea9585bbf6d4ad Mon Sep 17 00:00:00 2001 From: Juan Leyva Date: Fri, 31 Mar 2017 10:34:50 +0200 Subject: [PATCH] MDL-58453 mod_feedback: Refactor get_non_respondents WS We should return more information like if the user started or not the feedback and the total number of non respondents (to be able to paginate). --- mod/feedback/classes/external.php | 40 ++++++++++++++++++++++++++---------- mod/feedback/lib.php | 35 +++++++++++++++++++++---------- mod/feedback/show_nonrespondents.php | 22 +++++++++++--------- mod/feedback/tests/external_test.php | 2 +- 4 files changed, 66 insertions(+), 33 deletions(-) diff --git a/mod/feedback/classes/external.php b/mod/feedback/classes/external.php index d3cca99cce8..9a1a166ede7 100644 --- a/mod/feedback/classes/external.php +++ b/mod/feedback/classes/external.php @@ -992,10 +992,12 @@ class mod_feedback_external extends external_api { * @since Moodle 3.3 */ public static function get_non_respondents($feedbackid, $groupid = 0, $sort = 'lastaccess', $page = 0, $perpage = 0) { + global $CFG; + require_once($CFG->dirroot . '/mod/feedback/lib.php'); $params = array('feedbackid' => $feedbackid, 'groupid' => $groupid, 'sort' => $sort, 'page' => $page, 'perpage' => $perpage); $params = self::validate_parameters(self::get_non_respondents_parameters(), $params); - $warnings = $itemsdata = array(); + $warnings = $nonrespondents = array(); list($feedback, $course, $cm, $context) = self::validate_feedback($params['feedbackid']); @@ -1024,20 +1026,28 @@ class mod_feedback_external extends external_api { if ($params['sort'] !== 'firstname' && $params['sort'] !== 'lastname' && $params['sort'] !== 'lastaccess') { throw new invalid_parameter_exception('Invalid sort param, must be firstname, lastname or lastaccess.'); } - $params['sort'] = 'u.' . $params['sort']; // Check if we are page filtering. - if ($params['page'] == 0 && $params['perpage'] == 0) { - $perpage = false; - $page = false; + if ($params['perpage'] == 0) { + $page = $params['page']; + $perpage = FEEDBACK_DEFAULT_PAGE_COUNT; } else { $perpage = $params['perpage']; $page = $perpage * $params['page']; } - $users = feedback_get_incomplete_users($cm, $groupid, $params['sort'], $page, $perpage); + $users = feedback_get_incomplete_users($cm, $groupid, $params['sort'], $page, $perpage, true); + foreach ($users as $user) { + $nonrespondents[] = [ + 'courseid' => $course->id, + 'userid' => $user->id, + 'fullname' => fullname($user), + 'started' => $user->feedbackstarted + ]; + } $result = array( - 'users' => $users, + 'users' => $nonrespondents, + 'total' => feedback_count_incomplete_users($cm, $groupid), 'warnings' => $warnings ); return $result; @@ -1052,10 +1062,18 @@ class mod_feedback_external extends external_api { public static function get_non_respondents_returns() { return new external_single_structure( array( - 'users' => new external_multiple_structure( - new external_value(PARAM_INT, 'The user id') - ), - 'warnings' => new external_warnings(), + 'users' => new external_multiple_structure( + new external_single_structure( + array( + 'courseid' => new external_value(PARAM_INT, 'Course id'), + 'userid' => new external_value(PARAM_INT, 'The user id'), + 'fullname' => new external_value(PARAM_TEXT, 'User full name'), + 'started' => new external_value(PARAM_BOOL, 'If the user has started the attempt'), + ) + ) + ), + 'total' => new external_value(PARAM_INT, 'Total number of non respondents'), + 'warnings' => new external_warnings(), ) ); } diff --git a/mod/feedback/lib.php b/mod/feedback/lib.php index 8ae3d32826a..3fe661a8ef3 100644 --- a/mod/feedback/lib.php +++ b/mod/feedback/lib.php @@ -971,13 +971,15 @@ function feedback_check_is_switchrole() { * @param string $sort * @param int $startpage * @param int $pagecount - * @return object the userrecords + * @param bool $includestatus to return if the user started or not the feedback among the complete user record + * @return array array of user ids or user objects when $includestatus set to true */ function feedback_get_incomplete_users(cm_info $cm, $group = false, $sort = '', $startpage = false, - $pagecount = false) { + $pagecount = false, + $includestatus = false) { global $DB; @@ -985,7 +987,8 @@ function feedback_get_incomplete_users(cm_info $cm, //first get all user who can complete this feedback $cap = 'mod/feedback:complete'; - $fields = 'u.id, u.username'; + $allnames = get_all_user_name_fields(true, 'u'); + $fields = 'u.id, ' . $allnames . ', u.picture, u.email, u.imagealt'; if (!$allusers = get_users_by_capability($context, $cap, $fields, @@ -999,25 +1002,35 @@ function feedback_get_incomplete_users(cm_info $cm, } // Filter users that are not in the correct group/grouping. $info = new \core_availability\info_module($cm); - $allusers = $info->filter_user_list($allusers); + $allusersrecords = $info->filter_user_list($allusers); - $allusers = array_keys($allusers); + $allusers = array_keys($allusersrecords); //now get all completeds $params = array('feedback'=>$cm->instance); - if (!$completedusers = $DB->get_records_menu('feedback_completed', $params, '', 'id, userid')) { - return $allusers; + if ($completedusers = $DB->get_records_menu('feedback_completed', $params, '', 'id, userid')) { + // Now strike all completedusers from allusers. + $allusers = array_diff($allusers, $completedusers); } - //now strike all completedusers from allusers - $allusers = array_diff($allusers, $completedusers); - //for paging I use array_slice() if ($startpage !== false AND $pagecount !== false) { $allusers = array_slice($allusers, $startpage, $pagecount); } - return $allusers; + // Check if we should return the full users objects. + if ($includestatus) { + $userrecords = []; + $startedusers = $DB->get_records_menu('feedback_completedtmp', ['feedback' => $cm->instance], '', 'id, userid'); + $startedusers = array_flip($startedusers); + foreach ($allusers as $userid) { + $allusersrecords[$userid]->feedbackstarted = isset($startedusers[$userid]); + $userrecords[] = $allusersrecords[$userid]; + } + return $userrecords; + } else { // Return just user ids. + return $allusers; + } } /** diff --git a/mod/feedback/show_nonrespondents.php b/mod/feedback/show_nonrespondents.php index 340380b436d..5339e9d0310 100644 --- a/mod/feedback/show_nonrespondents.php +++ b/mod/feedback/show_nonrespondents.php @@ -216,36 +216,38 @@ if ($showall) { $pagecount = $table->get_page_size(); } -$students = feedback_get_incomplete_users($cm, $usedgroupid, $sort, $startpage, $pagecount); +// Return students record including if they started or not the feedback. +$students = feedback_get_incomplete_users($cm, $usedgroupid, $sort, $startpage, $pagecount, true); //####### viewreports-start //print the list of students echo $OUTPUT->heading(get_string('non_respondents_students', 'feedback', $matchcount), 4); echo isset($groupselect) ? $groupselect : ''; echo '
'; -if (!$students) { +if (empty($students)) { echo $OUTPUT->notification(get_string('noexistingparticipants', 'enrol')); } else { - if (has_capability('moodle/course:bulkmessaging', $coursecontext)) { + $canbulkmessaging = has_capability('moodle/course:bulkmessaging', $coursecontext); + if ($canbulkmessaging) { echo '
'; } + foreach ($students as $student) { - $user = $DB->get_record('user', array('id'=>$student)); //userpicture and link to the profilepage - $profile_url = $CFG->wwwroot.'/user/view.php?id='.$user->id.'&course='.$course->id; - $profilelink = ''.fullname($user).''; - $data = array ($OUTPUT->user_picture($user, array('courseid'=>$course->id)), $profilelink); + $profileurl = $CFG->wwwroot.'/user/view.php?id='.$student->id.'&course='.$course->id; + $profilelink = ''.fullname($student).''; + $data = array($OUTPUT->user_picture($student, array('courseid' => $course->id)), $profilelink); - if ($DB->record_exists('feedback_completedtmp', array('userid' => $user->id, 'feedback' => $feedback->id))) { + if ($student->feedbackstarted) { $data[] = get_string('started', 'feedback'); } else { $data[] = get_string('not_started', 'feedback'); } //selections to bulk messaging - if (has_capability('moodle/course:bulkmessaging', $coursecontext)) { - $data[] = ''; + if ($canbulkmessaging) { + $data[] = ''; } $table->add_data($data); } diff --git a/mod/feedback/tests/external_test.php b/mod/feedback/tests/external_test.php index da2f72059d7..1d8bae1323a 100644 --- a/mod/feedback/tests/external_test.php +++ b/mod/feedback/tests/external_test.php @@ -705,7 +705,7 @@ class mod_feedback_external_testcase extends externallib_advanced_testcase { $result = external_api::clean_returnvalue(mod_feedback_external::get_non_respondents_returns(), $result); $this->assertCount(0, $result['warnings']); $this->assertCount(1, $result['users']); - $this->assertEquals($anotherstudent->id, $result['users'][0]); + $this->assertEquals($anotherstudent->id, $result['users'][0]['userid']); // Create another student. $anotherstudent2 = self::getDataGenerator()->create_user(); -- 2.11.4.GIT