Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Questionnaire: make Summary page display more intuitive #589

Open
wants to merge 2 commits into
base: MOODLE_401_STABLE
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,14 @@ jobs:
matrix:
include:
- php: '8.3'
moodle-branch: 'MOODLE_404_STABLE'
moodle-branch: 'main'
database: 'pgsql'
- php: '8.2'
moodle-branch: 'MOODLE_403_STABLE'
database: 'mariadb'
- php: '8.1'
moodle-branch: 'MOODLE_403_STABLE'
moodle-branch: 'MOODLE_404_STABLE'
database: 'pgsql'
- php: '8.0'
moodle-branch: 'MOODLE_402_STABLE'
- php: '8.1'
moodle-branch: 'MOODLE_404_STABLE'
database: 'mariadb'
- php: '7.4'
moodle-branch: 'MOODLE_401_STABLE'
database: 'pgsql'

steps:
- name: Checkout
Expand Down
19 changes: 19 additions & 0 deletions classes/output/renderer.php
Original file line number Diff line number Diff line change
Expand Up @@ -564,4 +564,23 @@ public function download_dataformat_selector($label, $base, $name = 'dataformat'

return $this->render_from_template('mod_questionnaire/dataformat_selector', $data);
}

/**
* @param $urlroot
* @param $options
* @param $userview
* @return string
* @throws \coding_exception
* @throws \moodle_exception
*/
public function viewresponse_print_menu($urlroot, $options, $userview) {
if (!$urlroot instanceof \moodle_url) {
$urlroot = new \moodle_url($urlroot);
}
$select = new \single_select($urlroot, 'responsestats', $options, $userview, null);
$select->label = get_string('view');
$select->class = 'm-1';
$output = $this->render($select);
return $output;
}
}
2 changes: 1 addition & 1 deletion classes/responsetype/date.php
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ public function get_results_tags($weights, $participants, $respondents, $showtot

if ($showtotals == 1) {
$pagetags->total = new \stdClass();
$pagetags->total->total = "$numresps/$participants";
$pagetags->total->total = "($respondents)";
}
}

Expand Down
2 changes: 1 addition & 1 deletion classes/responsetype/rank.php
Original file line number Diff line number Diff line change
Expand Up @@ -888,7 +888,7 @@ private function mkrescount($rids, $rows, $sort) {
$nbna = $this->counts[$content]->nbna;
// TOTAL number of responses for this possible answer.
$total = $this->counts[$content]->num;
$nbresp = '<strong>'.$total.'</strong>';
$nbresp = '<strong>('.$total.')</strong>';
if ($osgood) {
// Ensure there are two bits of content.
list($content, $contentright) = array_merge(preg_split('/[|]/', $content), array(' '));
Expand Down
2 changes: 1 addition & 1 deletion classes/responsetype/responsetype.php
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,7 @@ public function get_results_tags($weights, $participants, $respondents, $showtot
$pagetags->total->width2 = $percent * 1.4;
$pagetags->total->image2 = $imageurl . 'thbar.gif';
$pagetags->total->percent = sprintf('&nbsp;%.'.$precision.'f%%', $percent);
$pagetags->total->total = "$respondents/$participants";
$pagetags->total->total = "($respondents)";
$pagetags->total->evencolor = $evencolor;
}
}
Expand Down
14 changes: 11 additions & 3 deletions classes/responsetype/single.php
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,18 @@ public function display_results($rids=false, $sort='', $anonymous=false) {
}
$numresps = count($rids);

$rsql = '';
if (!empty($rids)) {
list($rsql, $params) = $DB->get_in_or_equal($rids);
$rsql = ' AND response_id ' . $rsql;
}

$responsecountsql = 'SELECT COUNT(DISTINCT r.response_id) ' .
'FROM {' . $this->response_table() . '} r ' .
'WHERE r.question_id = ? ';
$numrespondents = $DB->count_records_sql($responsecountsql, [$this->question->id]);
'FROM {' . $this->response_table() . '} r, ' .
'{questionnaire_response} qr ' .
'WHERE question_id = ' . $this->question->id . $rsql .
' AND r.response_id = qr.id ';
$numrespondents = $DB->count_records_sql($responsecountsql, $params);

if ($rows) {
$counts = [];
Expand Down
4 changes: 2 additions & 2 deletions classes/responsetype/text.php
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ public function get_results_tags($weights, $participants, $respondents, $showtot

if ($showtotals == 1) {
$pagetags->total = new \stdClass();
$pagetags->total->total = "$respondents/$participants";
$pagetags->total->total = "($respondents)";
}
} else {
$nbresponses = 0;
Expand Down Expand Up @@ -250,7 +250,7 @@ public function get_results_tags($weights, $participants, $respondents, $showtot

if ($showtotals == 1) {
$pagetags->total = new \stdClass();
$pagetags->total->total = "$respondents/$participants";
$pagetags->total->total = "($respondents)";
$pagetags->total->evencolor = $evencolor;
}
}
Expand Down
4 changes: 4 additions & 0 deletions lang/en/questionnaire.php
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
$string['allnameddegrees'] = 'Named degrees';
$string['allnameddegrees_help'] = 'Specify text to display for rate values instead of the number. Leave a value blank to not use.';
$string['alreadyfilled'] = 'You have already filled out this questionnaire for us{$a}. Thank you.';
$string['allresponses'] = 'All responses';
$string['andaveragevalues'] = 'and average values';
$string['anonymous'] = 'Anonymous';
$string['answergiven'] = 'This answer given';
Expand Down Expand Up @@ -250,6 +251,7 @@
$string['firstrespondent'] = 'First Respondent';
$string['formateditor'] = 'HTML editor';
$string['formatplain'] = 'Plain text';
$string['fullsubmissions'] = 'Full submissions';
$string['grade'] = 'Submission grade';
$string['gradesdeleted'] = 'Questionnaire grades deleted';
$string['headingtext'] = 'Heading text';
Expand Down Expand Up @@ -527,6 +529,7 @@
$string['remove'] = 'Delete';
$string['removenotinuse'] = 'This questionnaire used to depend on a Public questionnaire which has been deleted.
It can no longer be used and should be deleted.';
$string['responsesnotsubmitted'] = 'Responses not submitted';
$string['required'] = 'Response is required';
$string['required_help'] = 'If you select ***Yes***, response to this question will be required, i.e.
the respondent will not be able to submit the questionnaire
Expand Down Expand Up @@ -608,6 +611,7 @@
$string['submitpreview'] = 'Submit preview';
$string['submitpreviewcorrect'] = 'This submission would be accepted as correctly filled in.';
$string['submitsurvey'] = 'Submit questionnaire';
$string['submissions'] = 'Submissions';
$string['submitted'] = 'Submitted on:';
$string['subtitle'] = 'Subtitle';
$string['subtitle_help'] = 'Subtitle of this questionnaire. Appears below the title on the first page only.';
Expand Down
22 changes: 17 additions & 5 deletions questionnaire.class.php
Original file line number Diff line number Diff line change
Expand Up @@ -709,10 +709,12 @@ public function can_view_response($rid) {

/**
* True if the user can view the responses to this questionnaire, and there are valid responses.
*
* @param null|int $usernumresp
* @param bool $isviewreport
* @return bool
*/
public function can_view_all_responses($usernumresp = null) {
public function can_view_all_responses($usernumresp = null, $isviewreport = false) {
global $USER, $SESSION;

$owner = $this->is_survey_owner();
Expand All @@ -738,7 +740,7 @@ public function can_view_all_responses($usernumresp = null) {
}

$grouplogic = $canviewgroups || $canviewallgroups;
$respslogic = ($numresp > 0) && ($numselectedresps > 0);
$respslogic = ($numresp > 0) && ($numselectedresps > 0) || $isviewreport;
return $this->can_view_all_responses_anytime($grouplogic, $respslogic) ||
$this->can_view_all_responses_with_restrictions($usernumresp, $grouplogic, $respslogic);
}
Expand Down Expand Up @@ -851,15 +853,20 @@ public function get_responses($userid=false, $groupid=0) {
$sql = 'SELECT r.* ' .
'FROM {questionnaire_response} r ' .
$groupsql .
'WHERE r.questionnaireid = :questionnaireid AND r.complete = :status' . $groupcnd;
'WHERE r.questionnaireid = :questionnaireid' . $groupcnd;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you remove the 'complete' check?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to the new requirement of separating the 3 response viewing modes, we have been updating this SQL query and adding some logic to check the status correctly for each view.

image

$params['questionnaireid'] = $this->id;
$params['status'] = 'y';
}
if ($userid) {
$sql .= ' AND r.userid = :userid';
$params['userid'] = $userid;
}

$status = optional_param('responsestats', '0', PARAM_ALPHANUM);
if ($status) {
$status = ($status === 'n') ? 'n' : 'y';
$sql .= ' AND r.complete = :status';
$params['status'] = $status;
}
$sql .= ' ORDER BY r.id';
return $DB->get_records_sql($sql, $params) ?? [];
}
Expand Down Expand Up @@ -2858,6 +2865,7 @@ public function survey_results($rid = '', $uid=false, $pdf = false, $currentgrou
$numresps = 1;
} else {
$navbar = false;
$userview = optional_param('responsestats', 0, PARAM_ALPHA);
if ($uid !== false) { // One participant only.
$rows = $this->get_responses($uid);
// All participants or all members of a group.
Expand All @@ -2874,8 +2882,12 @@ public function survey_results($rid = '', $uid=false, $pdf = false, $currentgrou
return;
}
$numresps = count($rows);
$respondentstring = get_string('responses', 'questionnaire');
if ($userview === 'y') {
$respondentstring = get_string('submissions', 'questionnaire');
}
$this->page->add_to_page('respondentinfo',
' '.get_string('responses', 'questionnaire').': <strong>'.$numresps.'</strong>');
' ' . $respondentstring . ': <strong>' . $numresps . '</strong>');
if (empty($rows)) {
$errmsg = get_string('erroropening', 'questionnaire') .' '. get_string('noresponsedata', 'questionnaire');
return($errmsg);
Expand Down
29 changes: 20 additions & 9 deletions report.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
$currentgroupid = optional_param('group', 0, PARAM_INT); // Groupid.
$user = optional_param('user', '', PARAM_INT);
$outputtarget = optional_param('target', 'html', PARAM_ALPHA); // Default 'html'. Could be 'pdf'.
$userview = optional_param('responsestats', '0', PARAM_ALPHANUM);

$userid = $USER->id;
switch ($action) {
Expand Down Expand Up @@ -87,7 +88,7 @@

// If you can't view the questionnaire, or can't view a specified response, error out.
$context = context_module::instance($cm->id);
if (!$questionnaire->can_view_all_responses() && !$individualresponse) {
if (!$questionnaire->can_view_all_responses(null, true) && !$individualresponse) {
// Should never happen, unless called directly by a snoop...
throw new \moodle_exception('nopermissions', 'mod_questionnaire');
}
Expand Down Expand Up @@ -205,6 +206,14 @@
}
}

$responsestatus = [
'y' => get_string('fullsubmissions', 'questionnaire'),
'0' => get_string('allresponses', 'questionnaire'),
'n' => get_string('responsesnotsubmitted', 'questionnaire'),
];
// Set default userview to all responses.
$userview = array_key_exists($userview, $responsestatus) ? $userview : '0';

switch ($action) {

case 'dresp': // Delete individual response? Ask for confirmation.
Expand Down Expand Up @@ -582,7 +591,7 @@
if ($currentgroupid > 0) {
$groupname = get_string('group').': <strong>'.groups_get_group_name($currentgroupid).'</strong>';
} else {
$groupname = '<strong>'.get_string('allparticipants').'</strong>';
$groupname = '<strong>'.$responsestatus[$userview].'</strong>';
}

// Available group modes (0 = no groups; 1 = separate groups; 2 = visible groups).
Expand Down Expand Up @@ -627,9 +636,9 @@
if ($currentgroupid > 0) {
$groupname = get_string('group') . ': <strong>' . groups_get_group_name($currentgroupid) . '</strong>';
} else {
$groupname = '<strong>' . get_string('allparticipants') . '</strong>';
$groupname = '<strong>' . $responsestatus[$userview] . '</strong>';
}
$respinfo = get_string('viewallresponses', 'questionnaire') . '. ' . $groupname . '. ';
$respinfo = get_string('view') . ' ' . $groupname;
$strsort = get_string('order_' . $sort, 'questionnaire');
$respinfo .= $strsort;
$questionnaire->page->add_to_page('respondentinfo', $respinfo);
Expand All @@ -650,13 +659,15 @@
if ($outputtarget != 'print') {
$linkname = get_string('downloadpdf', 'mod_questionnaire');
$link = new moodle_url('/mod/questionnaire/report.php',
['action' => 'vall', 'instance' => $instance, 'group' => $currentgroupid, 'target' => 'pdf']);
['action' => 'vall', 'instance' => $instance, 'group' => $currentgroupid, 'target' => 'pdf',
'responsestats' => $userview]);
$downpdficon = new pix_icon('f/pdf', $linkname);
$respinfo .= $questionnaire->renderer->action_link($link, null, null, null, $downpdficon);

$linkname = get_string('print', 'mod_questionnaire');
$link = new \moodle_url('/mod/questionnaire/report.php',
['action' => 'vall', 'instance' => $instance, 'group' => $currentgroupid, 'target' => 'print']);
['action' => 'vall', 'instance' => $instance, 'group' => $currentgroupid, 'target' => 'print',
'responsestats' => $userview]);
$htmlicon = new pix_icon('t/print', $linkname);
$options = ['menubar' => true, 'location' => false, 'scrollbars' => true, 'resizable' => true,
'height' => 600, 'width' => 800, 'title' => $linkname];
Expand All @@ -666,7 +677,7 @@
$respinfo .= $questionnaire->renderer->action_link($link, null, $action,
['class' => $class, 'title' => $linkname], $htmlicon) . '&nbsp;';

$respinfo .= get_string('viewallresponses', 'questionnaire') . '. ' . $groupname . '. ';
$respinfo .= $questionnaire->renderer->viewresponse_print_menu($url->out(), $responsestatus, $userview);
$strsort = get_string('order_' . $sort, 'questionnaire');
$respinfo .= $strsort;
$respinfo .= $questionnaire->renderer->help_icon('orderresponses', 'questionnaire');
Expand Down Expand Up @@ -763,7 +774,7 @@
if ($currentgroupid > 0) {
$groupname = get_string('group') . ': <strong>' . groups_get_group_name($currentgroupid) . '</strong>';
} else {
$groupname = '<strong>' . get_string('allparticipants') . '</strong>';
$groupname = '<strong>' . $responsestatus[$userview] . '</strong>';
}
if (!$byresponse) { // Show respondents individual responses.
$questionnaire->view_response($rid, '', $resps, true, true, false, $currentgroupid, $outputtarget);
Expand Down Expand Up @@ -797,7 +808,7 @@

$groupname = get_string('group').': <strong>'.groups_get_group_name($currentgroupid).'</strong>';
if ($currentgroupid == 0 ) {
$groupname = get_string('allparticipants');
$groupname = $responsestatus[$userview];
}
if ($byresponse) {
$respinfo = '';
Expand Down
32 changes: 14 additions & 18 deletions tests/behat/check_responses.feature
Original file line number Diff line number Diff line change
Expand Up @@ -34,47 +34,43 @@ Feature: Review responses
And I follow "Test questionnaire"
Then I should see "View all responses"
And I navigate to "View all responses" in current page administration
Then I should see "View all responses."
And I should see "All participants."
And I should see "View Default order"
And I should see "Responses: 6"
And I should see "Responses: 7"
And I set the field "View" to "Full submissions"
And I should see "Submissions: 6"
And I set the field "View" to "Responses not submitted"
And I should see "Responses: 1"
And I follow "Ascending order"
Then I should see "View all responses."
And I should see "All participants."
And I should see "Ascending order"
And I should see "Responses: 6"
And I should see "Responses: 7"
And I follow "Descending order"
Then I should see "View all responses."
And I should see "All participants."
And I should see "Descending order"
And I should see "Responses: 6"
And I should see "Responses: 7"
And I follow "List of responses"
Then I should see "Individual responses : All participants"
Then I should see "Individual responses : All responses"
And I follow "Admin User"
Then I should see "1 / 6"
Then I should see "1 / 7"
And I should see "Respondent:"
And I should see "Admin User"
And I should see "Submitted on:"
# And I should see "Thursday, 14 January 2016, 9:22 pm"
And I should see "Test questionnaire"
And I follow "Next"
Then I should see "2 / 6"
Then I should see "2 / 7"
# And I should see "Thursday, 14 January 2016, 8:53 pm"
And I follow "Last Respondent"
Then I should see "6 / 6"
Then I should see "7 / 7"
# And I should see "Friday, 19 December 2014, 5:58 pm"
And I follow "Delete this Response"
Then I should see "Are you sure you want to delete the response"
# And I should see "Friday, 19 December 2014, 5:58 pm"
And I press "Delete"
Then I should see "Individual responses : All participants"
Then I should see "Individual responses : All responses"
And I follow "Admin User"
Then I should see "1 / 5"
Then I should see "1 / 6"
And I follow "Summary"
Then I should see "View all responses."
And I should see "All participants."
And I should see "View Default order"
And I should see "Responses: 5"
And I should see "Responses: 6"
And I follow "Delete ALL Responses"
Then I should see "Are you sure you want to delete ALL the responses in this questionnaire?"
And I press "Delete"
Expand Down
5 changes: 2 additions & 3 deletions tests/behat/check_responses_capabilities.feature
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,9 @@ Feature: Review responses with different capabilities
And I follow "Test questionnaire"
Then I should see "View all responses"
And I navigate to "View all responses" in current page administration
Then I should see "View all responses."
And I should see "All participants."
Then I should see "All responses"
And I should see "View Default order"
And I should see "Responses: 6"
And I should see "Responses: 7"
And I log out

@javascript
Expand Down
Loading