From f29e6691a72c8cc8750ca33a34599a0f8e296d43 Mon Sep 17 00:00:00 2001 From: jamiesensei Date: Fri, 15 Aug 2008 09:59:55 +0000 Subject: [PATCH] MDL-15972 "Make ability to list regrades dependent on permission" fixed a problem showing some of the regrade interface to users who do not have permission to regrade. MDL-15960 "Overview report does not work correctly with groups" Fixed a problem that group mode was not working as the logic selecting the attempts mode was erroneously showing the all attempts mode. MDL-15958 "Error on selecting to hide marks for each question" problem with trying to sort on question grades when grades not displayed fixed. MDL-15953 "Summary chart to show in 5% ranges please" allowed the graph to show up to or equal to 20 bands, instead of just up to 20. MDL-14201 "Summary graph" showing two graphs now as it was felt that when there were few students in a group compared with all the students who took a quiz then it would be difficult to see the data for the group. --- lang/en_utf8/quiz_overview.php | 5 +- mod/quiz/report/overview/overview_table.php | 36 +++-- mod/quiz/report/overview/overviewgraph.php | 73 +++------- .../report/overview/overviewsettings_form.php | 9 +- mod/quiz/report/overview/report.php | 133 ++++++++++-------- 5 files changed, 127 insertions(+), 129 deletions(-) diff --git a/lang/en_utf8/quiz_overview.php b/lang/en_utf8/quiz_overview.php index d7c1bd1e04..7e06b16033 100644 --- a/lang/en_utf8/quiz_overview.php +++ b/lang/en_utf8/quiz_overview.php @@ -27,10 +27,11 @@ $string['optonlyregradedattempts'] = 'that have been regraded / are marked as ne $string['overview'] = 'Grades'; $string['overviewdownload'] = 'Overview download'; $string['overviewdownload'] = 'Overview download'; -$string['overviewreportgraph'] = 'Bar Graph of Number of Students Achieving Grade Ranges'; +$string['overviewreportgraph'] = 'Overall Number of Students Achieving Grade Ranges'; +$string['overviewreportgraphgroup'] = 'Number of Students in Group \'$a\' Achieving Grade Ranges'; $string['pagesize'] = 'Page size'; $string['preferencespage'] = 'Preferences just for this page'; -$string['preferencessave'] = 'Save preferences'; +$string['preferencessave'] = 'Show report'; $string['preferencesuser'] = 'Your preferences for this report'; $string['qprogress'] = 'Question $a->done of $a->todo'; $string['regrade'] = 'Regrade'; diff --git a/mod/quiz/report/overview/overview_table.php b/mod/quiz/report/overview/overview_table.php index 575d3a57cf..375aaa810c 100644 --- a/mod/quiz/report/overview/overview_table.php +++ b/mod/quiz/report/overview/overview_table.php @@ -7,6 +7,7 @@ class quiz_report_overview_table extends table_sql { var $candelete; var $reporturl; var $displayoptions; + var $regradedqs = array(); function quiz_report_overview_table($quiz , $qmsubselect, $groupstudents, $students, $detailedmarks, $questions, $candelete, $reporturl, $displayoptions, $context){ @@ -283,23 +284,34 @@ class quiz_report_overview_table extends table_sql { $this->sql->params['qid'.$qid] = $qid; } } + } else { + //unset any sort columns that sort on question grade as the + //grades are not being fetched as fields + $sess = &$this->sess; + foreach($sess->sortby as $column => $order) { + if (preg_match('/^qsgrade([0-9]+)/', trim($column))){ + unset($sess->sortby[$column]); + } + } } } parent::query_db($pagesize, $useinitialsbar); - if ($this->detailedmarks){ - //get all the attempt ids we want to display on this page - //or to export for download. - if (!$this->is_downloading()) { - $attemptids = array(); - foreach ($this->rawdata as $attempt){ - if ($attempt->attemptuniqueid > 0){ - $attemptids[] = $attempt->attemptuniqueid; - } + //get all the attempt ids we want to display on this page + //or to export for download. + if (!$this->is_downloading()) { + $attemptids = array(); + foreach ($this->rawdata as $attempt){ + if ($attempt->attemptuniqueid > 0){ + $attemptids[] = $attempt->attemptuniqueid; } - $this->gradedstatesbyattempt = quiz_get_newgraded_states($attemptids, true, 'qs.id, qs.grade, qs.event, qs.question, qs.attempt'); + } + $this->gradedstatesbyattempt = quiz_get_newgraded_states($attemptids, true, 'qs.id, qs.grade, qs.event, qs.question, qs.attempt'); + if (has_capability('mod/quiz:grade', $this->context)){ $this->regradedqs = quiz_get_regraded_qs($attemptids); - } else { - $this->gradedstatesbyattempt = quiz_get_newgraded_states($this->sql, true, 'qs.id, qs.grade, qs.event, qs.question, qs.attempt'); + } + } else { + $this->gradedstatesbyattempt = quiz_get_newgraded_states($this->sql, true, 'qs.id, qs.grade, qs.event, qs.question, qs.attempt'); + if (has_capability('mod/quiz:grade', $this->context)){ $this->regradedqs = quiz_get_regraded_qs($this->sql); } } diff --git a/mod/quiz/report/overview/overviewgraph.php b/mod/quiz/report/overview/overviewgraph.php index 1dd60bba7b..58809e1a3c 100644 --- a/mod/quiz/report/overview/overviewgraph.php +++ b/mod/quiz/report/overview/overviewgraph.php @@ -2,29 +2,31 @@ include '../../../../config.php'; include $CFG->dirroot."/lib/graphlib.php"; include $CFG->dirroot."/mod/quiz/report/reportlib.php"; -function graph_get_new_colour(){ - static $colourindex = 0; - $colours = array('red', 'green', 'yellow', 'orange', 'purple', 'black', 'maroon', 'blue', 'ltgreen', 'navy', 'ltred', 'ltltgreen', 'ltltorange', 'olive', 'gray', 'ltltred', 'ltorange', 'lime', 'ltblue', 'ltltblue'); - $colour = $colours[$colourindex]; - $colourindex++; - if ($colourindex > (count($colours)-1)){ - $colourindex =0; - } - return $colour; -} -define('QUIZ_REPORT_MAX_PARTICIPANTS_TO_SHOW_ALL_GROUPS', 500); $quizid = required_param('id', PARAM_INT); +$groupid = optional_param('groupid', 0, PARAM_INT); $quiz = $DB->get_record('quiz', array('id' => $quizid)); $course = $DB->get_record('course', array('id' => $quiz->course)); require_login($course); $cm = get_coursemodule_from_instance('quiz', $quizid); -if ($groupmode = groups_get_activity_groupmode($cm)) { // Groups are being used +$modcontext = get_context_instance(CONTEXT_MODULE, $cm->id); +if ($groupid && $groupmode = groups_get_activity_groupmode($cm)) { // Groups are being used $groups = groups_get_activity_allowed_groups($cm); + if (array_key_exists($groupid, $groups)){ + $group = $groups[$groupid]; + if (!$groupusers = get_users_by_capability($modcontext, 'mod/quiz:attempt','','','','',$group->id,'',false)){ + print_error('nostudentsingroup'); + } else { + $groupusers = array_keys($groupusers); + } + } else { + print_error('errorinvalidgroup', 'group', null, $groupid); + } } else { $groups = false; + $group = false; + $groupusers = array(); } -$modcontext = get_context_instance(CONTEXT_MODULE, $cm->id); require_capability('mod/quiz:viewreports', $modcontext); $line = new graph(800,600); @@ -44,15 +46,15 @@ $line->parameter['bar_spacing'] = 10; // don't forget to increase spacing so tha //pick a sensible number of bands depending on quiz maximum grade. $bands = $quiz->grade; -while ($bands >= 20 || $bands < 10){ - if ($bands >= 50){ +while ($bands > 20 || $bands <= 10){ + if ($bands > 50){ $bands = $bands /5; - } else if ($bands >= 20) { + } else if ($bands > 20) { $bands = $bands /2; } if ($bands < 4){ $bands = $bands * 5; - } else if ($bands < 10){ + } else if ($bands <= 10){ $bands = $bands * 2; } } @@ -72,41 +74,10 @@ for ($i=0;$i < $quiz->grade;$i += $bandwidth){ $line->x_data = $bandlabels; $line->y_format['allusers'] = - array('colour' => graph_get_new_colour(), 'bar' => 'fill', 'shadow_offset' => 1, 'legend' => get_string('allparticipants')); -$line->y_data['allusers'] = quiz_report_grade_bands($bandwidth, $bands, $quizid); -if (array_sum($line->y_data['allusers'])>QUIZ_REPORT_MAX_PARTICIPANTS_TO_SHOW_ALL_GROUPS || - count($groups)>4){ - if ($groups){ - if ($currentgroup = groups_get_activity_group($cm)){ - $groups = array($currentgroup=>''); - } else { - $groups = false;//all participants mode - } - } -} -$line->y_order = array('allusers'); -if ($groups){ - foreach (array_keys($groups) as $group){ - $useridingroup = get_users_by_capability($modcontext, 'mod/quiz:attempt','','','','',$group,'',false); - if ($useridingroup){ - $groupdata = quiz_report_grade_bands($bandwidth, $bands, $quizid, array_keys($useridingroup)); - if ($groupdata){ - $line->parameter['bar_size'] = 1.2; - $line->y_data['groupusers'.$group] = $groupdata; - //only turn on legends if there is more than one set of bars - $line->parameter['legend'] = 'outside-top'; - $line->parameter['legend_border'] = 'black'; - $line->parameter['legend_offset'] = 4; - $line->y_format['groupusers'.$group] = - array('colour' => graph_get_new_colour(), 'bar' => 'fill', 'shadow_offset' => 1, 'legend' => groups_get_group_name($group)); - $line->y_order[] ='groupusers'.$group; - } - } - } -} - - + array('colour' => 'red', 'bar' => 'fill', 'shadow_offset' => 1, 'legend' => get_string('allparticipants')); +$line->y_data['allusers'] = quiz_report_grade_bands($bandwidth, $bands, $quizid, $groupusers); +$line->y_order = array('allusers'); $line->parameter['y_min_left'] = 0; // start at 0 $line->parameter['y_max_left'] = max($line->y_data['allusers']); diff --git a/mod/quiz/report/overview/overviewsettings_form.php b/mod/quiz/report/overview/overviewsettings_form.php index 55e97198f5..8441cab9cc 100644 --- a/mod/quiz/report/overview/overviewsettings_form.php +++ b/mod/quiz/report/overview/overviewsettings_form.php @@ -33,9 +33,12 @@ class mod_quiz_report_overview_settings extends moodleform { $gm = ''.quiz_get_grading_option_name($this->_customdata['quiz']->grademethod).''; $showattemptsgrp[] =& $mform->createElement('advcheckbox', 'qmfilter', get_string('showattempts', 'quiz_overview'), get_string('optonlygradedattempts', 'quiz_overview', $gm), null, array(0,1)); } - - $showattemptsgrp[] =& $mform->createElement('advcheckbox', 'regradefilter', get_string('showattempts', 'quiz_overview'), get_string('optonlyregradedattempts', 'quiz_overview'), null, array(0,1)); - $mform->addGroup($showattemptsgrp, null, get_string('showattempts', 'quiz_overview'), '
', false); + if (has_capability('mod/quiz:grade', $this->_customdata['context'])){ + $showattemptsgrp[] =& $mform->createElement('advcheckbox', 'regradefilter', get_string('showattempts', 'quiz_overview'), get_string('optonlyregradedattempts', 'quiz_overview'), null, array(0,1)); + } + if ($showattemptsgrp){ + $mform->addGroup($showattemptsgrp, null, get_string('showattempts', 'quiz_overview'), '
', false); + } //------------------------------------------------------------------------------- $mform->addElement('header', 'preferencesuser', get_string('preferencesuser', 'quiz_overview')); diff --git a/mod/quiz/report/overview/report.php b/mod/quiz/report/overview/report.php index 2959ba9dec..a7fc84717f 100644 --- a/mod/quiz/report/overview/report.php +++ b/mod/quiz/report/overview/report.php @@ -82,8 +82,8 @@ class quiz_overview_report extends quiz_default_report { $reporturl = new moodle_url($CFG->wwwroot.'/mod/quiz/report.php', $pageoptions); $qmsubselect = quiz_report_qm_filter_select($quiz); - - $mform = new mod_quiz_report_overview_settings($reporturl, array('qmsubselect'=> $qmsubselect, 'quiz'=>$quiz, 'currentgroup'=>$currentgroup)); + $mform = new mod_quiz_report_overview_settings($reporturl, array('qmsubselect'=> $qmsubselect, 'quiz'=>$quiz, + 'currentgroup'=>$currentgroup, 'context'=>$this->context)); if ($fromform = $mform->get_data()){ $regradeall = false; $regradealldry = false; @@ -113,21 +113,22 @@ class quiz_overview_report extends quiz_default_report { $qmfilter = 0; } $regradefilter = optional_param('regradefilter', 0, PARAM_INT); - if ($attemptsmode === null){ - //default - $attemptsmode = QUIZ_REPORT_ATTEMPTS_ALL; - } else if ($currentgroup){ - //default for when a group is selected - if ($attemptsmode === null || $attemptsmode == QUIZ_REPORT_ATTEMPTS_ALL){ - $attemptsmode = QUIZ_REPORT_ATTEMPTS_STUDENTS_WITH; - } - } else if (!$currentgroup && $course->id == SITEID) { - //force report on front page to show all, unless a group is selected. - $attemptsmode = QUIZ_REPORT_ATTEMPTS_ALL; - } + $detailedmarks = get_user_preferences('quiz_report_overview_detailedmarks', 1); $pagesize = get_user_preferences('quiz_report_pagesize', 0); } + if ($currentgroup){ + //default for when a group is selected + if ($attemptsmode === null || $attemptsmode == QUIZ_REPORT_ATTEMPTS_ALL){ + $attemptsmode = QUIZ_REPORT_ATTEMPTS_STUDENTS_WITH; + } + } else if (!$currentgroup && $course->id == SITEID) { + //force report on front page to show all, unless a group is selected. + $attemptsmode = QUIZ_REPORT_ATTEMPTS_ALL; + } else if ($attemptsmode === null){ + //default + $attemptsmode = QUIZ_REPORT_ATTEMPTS_ALL; + } if (!$reviewoptions->scores) { $detailedmarks = 0; } @@ -201,45 +202,8 @@ class quiz_overview_report extends quiz_default_report { - if (!$table->is_downloading()) { //do not print notices when downloading - $countregradeneeded = $this->count_regrade_all_needed($quiz, $groupstudents); - //regrade buttons - if ($currentgroup){ - $a= new object(); - $a->groupname = groups_get_group_name($currentgroup); - $a->coursestudents = $COURSE->students; - $a->countregradeneeded = $countregradeneeded; - $regradealldrydolabel = get_string('regradealldrydogroup', 'quiz_overview', $a); - $regradealldrylabel = get_string('regradealldrygroup', 'quiz_overview', $a); - $regradealllabel = get_string('regradeallgroup', 'quiz_overview', $a); - } else { - $regradealldrydolabel = get_string('regradealldrydo', 'quiz_overview', $countregradeneeded); - $regradealldrylabel = get_string('regradealldry', 'quiz_overview'); - $regradealllabel = get_string('regradeall', 'quiz_overview'); - } - if (has_capability('mod/quiz:grade', $this->context)){ - echo '
'; - echo '
'; - echo '
'; - echo $reporturl->hidden_params_out(array(), 0, $displayoptions); - echo ''; - echo ''; - if ($countregradeneeded){ - echo ''; - } - echo '
'; - echo '
'; - echo '
'; - } - } + if (!$nostudents || ($attemptsmode == QUIZ_REPORT_ATTEMPTS_ALL)){ - // Print information on the grading method and whether we are displaying - // - if (!$table->is_downloading()) { //do not print notices when downloading - if ($strattempthighlight = quiz_report_highlighting_grading_method($quiz, $qmsubselect, $qmfilter)) { - echo '
' . $strattempthighlight . '
'; - } - } $showgrades = $quiz->grade && $quiz->sumgrades && $reviewoptions->scores; @@ -314,7 +278,41 @@ class quiz_overview_report extends quiz_default_report { // Define table columns $columns = array(); $headers = array(); - + if (!$table->is_downloading()) { //do not print notices when downloading + //regrade buttons + if (has_capability('mod/quiz:grade', $this->context)){ + $countregradeneeded = $this->count_regrade_all_needed($quiz, $groupstudents); + if ($currentgroup){ + $a= new object(); + $a->groupname = groups_get_group_name($currentgroup); + $a->coursestudents = $COURSE->students; + $a->countregradeneeded = $countregradeneeded; + $regradealldrydolabel = get_string('regradealldrydogroup', 'quiz_overview', $a); + $regradealldrylabel = get_string('regradealldrygroup', 'quiz_overview', $a); + $regradealllabel = get_string('regradeallgroup', 'quiz_overview', $a); + } else { + $regradealldrydolabel = get_string('regradealldrydo', 'quiz_overview', $countregradeneeded); + $regradealldrylabel = get_string('regradealldry', 'quiz_overview'); + $regradealllabel = get_string('regradeall', 'quiz_overview'); + } + echo '
'; + echo '
'; + echo '
'; + echo $reporturl->hidden_params_out(array(), 0, $displayoptions); + echo ''; + echo ''; + if ($countregradeneeded){ + echo ''; + } + echo '
'; + echo '
'; + echo '
'; + } + // Print information on the grading method + if ($strattempthighlight = quiz_report_highlighting_grading_method($quiz, $qmsubselect, $qmfilter)) { + echo '
' . $strattempthighlight . '
'; + } + } if (!$table->is_downloading() && $candelete) { $columns[]= 'checkbox'; @@ -363,7 +361,7 @@ class quiz_overview_report extends quiz_default_report { $headers[] = $header; } } - if ($regradedattempts){ + if (!$table->is_downloading() && has_capability('mod/quiz:grade', $this->context) && $regradedattempts){ $columns[] = 'regraded'; $headers[] = get_string('regrade', 'quiz_overview'); } @@ -403,11 +401,22 @@ class quiz_overview_report extends quiz_default_report { $table->out($pagesize, true); } if (!$table->is_downloading()) { - //should be quicker than a COUNT to test if there is at least one record : + if ($currentgroup && $groupstudents){ + list($usql, $params) = $DB->get_in_or_equal($groupstudents); + $params[] = $quiz->id; + //should be quicker than a COUNT to test if there is at least one record : + if ($DB->get_records_select('quiz_grades', "userid $usql AND quiz = ?", $params, '', '*', 0, 1)){ + $imageurl = "{$CFG->wwwroot}/mod/quiz/report/overview/overviewgraph.php?id={$quiz->id}&groupid=$currentgroup"; + $graphname = get_string('overviewreportgraphgroup', 'quiz_overview', groups_get_group_name($currentgroup)); + print_heading($graphname); + echo '
'.$graphname.'
'; + } + } if ($DB->get_records('quiz_grades', array('quiz'=> $quiz->id), '', '*', 0, 1)){ + $graphname = get_string('overviewreportgraph', 'quiz_overview'); $imageurl = $CFG->wwwroot.'/mod/quiz/report/overview/overviewgraph.php?id='.$quiz->id; - print_heading(get_string('overviewreportgraph', 'quiz_overview')); - echo '
'.get_string('overviewreportgraph', 'quiz_overview').'
'; + print_heading($graphname); + echo '
'.$graphname.'
'; } } return true; @@ -576,8 +585,8 @@ class quiz_overview_report extends quiz_default_report { //recalculate $attempt->sumgrade //already updated in regrade_question_in_attempt $sql = "UPDATE {quiz_attempts} SET sumgrades= " . - "(SELECT SUM(qs.grade) FROM {question_sessions} qns, {question_states} qs " . - "WHERE qns.newgraded = qs.id AND qns.attemptid = {quiz_attempts}.uniqueid ) WHERE "; + "COALESCE((SELECT SUM(qs.grade) FROM {question_sessions} qns, {question_states} qs " . + "WHERE qns.newgraded = qs.id AND qns.attemptid = {quiz_attempts}.uniqueid ), 0) WHERE "; $attemptsql=''; if (!$attemptids){ if ($userids){ @@ -601,9 +610,11 @@ class quiz_overview_report extends quiz_default_report { if ($attemptids){ //make sure we fetch all attempts for users to calculate grade. //not just those that have changed. - $sql = "SELECT qa2.* FROM {quiz_attempts} qa2 WHERE qa2.userid IN (SELECT DISTINCT userid FROM {quiz_attempts} WHERE $attemptsql)"; + $sql = "SELECT qa2.* FROM {quiz_attempts} qa2 WHERE " . + "qa2.userid IN (SELECT DISTINCT userid FROM {quiz_attempts} WHERE $attemptsql) " . + "AND qa2.timefinish > 0"; } else { - $sql = "SELECT * FROM {quiz_attempts} WHERE $attemptsql"; + $sql = "SELECT * FROM {quiz_attempts} WHERE $attemptsql AND timefinish > 0"; } if ($attempts = $DB->get_records_sql($sql, $params)) { $attemptsbyuser = quiz_report_index_by_keys($attempts, array('userid', 'id')); -- 2.39.5