Skip to content

GitLab

Commit 8fbc41d8 authored by Jake Dallimore's avatar Jake Dallimore
Browse files

MDL-37361 completion: minor code fixes.

parent 194f1422
Hide whitespace changes
Inline Side-by-side
completion/classes/external.php
View file @ 8fbc41d8
......@@ -118,7 +118,7 @@ class core_completion_external extends external_api {
* Describes the parameters for override_activity_completion_status.
*
* @return external_external_function_parameters
* @since Moodle 3.1
* @since Moodle 3.4
*/
public static function override_activity_completion_status_parameters() {
return new external_function_parameters (
......@@ -136,7 +136,7 @@ class core_completion_external extends external_api {
* @param int $cmid Course module id
* @param int $newstate Activity completion
* @return array Result and possible warnings
* @since Moodle 3.1
* @since Moodle 3.4
* @throws moodle_exception
*/
public static function override_activity_completion_status($userid, $cmid, $newstate) {
......@@ -149,8 +149,6 @@ class core_completion_external extends external_api {
$cmid = $params['cmid'];
$newstate = $params['newstate'];
$warnings = array();
$context = context_module::instance($cmid);
self::validate_context($context);
......@@ -191,9 +189,8 @@ class core_completion_external extends external_api {
($cm->completion == COMPLETION_TRACKING_AUTOMATIC ? 'auto' : 'manual').
'-'.$completiontype;
$overridebyuser = $DB->get_record('user', array('id' => $USER->id), '*', MUST_EXIST);
$describe = get_string('completion-' . $completiontype, 'completion', fullname($overridebyuser));
$user = $DB->get_record('user', array('id' => $userid), '*', MUST_EXIST);
$describe = get_string('completion-' . $completiontype, 'completion', fullname($USER));
$user = \core_user::get_user($userid, '*', MUST_EXIST);
$a = new StdClass;
$a->state = $describe;
$a->date = $date;
......@@ -203,15 +200,11 @@ class core_completion_external extends external_api {
$img = $OUTPUT->pix_icon('i/' . $completionicon, s($fulldescribe));
// Set data values for next completion change.
$otherstate = ($state == COMPLETION_COMPLETE) ? COMPLETION_INCOMPLETE : COMPLETION_COMPLETE;
$changecompl = $userid . '-' . $cmid . '-' . $otherstate;
$result = array();
$result['status'] = true;
$result['warnings'] = $warnings;
$result['changecompl'] = $changecompl;
$result['img'] = $img;
// Return the current state of completion.
$result = [
'completionstate' => $state,
'img' => $img
];
return $result;
}
......@@ -219,15 +212,13 @@ class core_completion_external extends external_api {
* Describes the override_activity_completion_status return value.
*
* @return external_single_structure
* @since Moodle 3.1
* @since Moodle 3.4
*/
public static function override_activity_completion_status_returns() {
return new external_single_structure(
array(
'status' => new external_value(PARAM_BOOL, 'Status, true if success'),
'warnings' => new external_warnings(),
'changecompl' => new external_value(PARAM_ALPHANUMEXT, 'The new completion change data'),
'completionstate' => new external_value(PARAM_BOOL, 'The current completion state.'),
'img' => new external_value(PARAM_RAW, 'Image element to replace existing one'),
)
);
......
completion/tests/externallib_test.php
View file @ 8fbc41d8
......@@ -202,6 +202,7 @@ class core_completion_externallib_testcase extends externallib_advanced_testcase
$this->getDataGenerator()->enrol_user($student->id, $course->id, $studentrole->id);
$teacherrole = $DB->get_record('role', array('shortname' => 'teacher'));
$this->getDataGenerator()->enrol_user($teacher->id, $course->id, $teacherrole->id);
$coursecontext = context_course::instance($course->id);
// Create 2 activities, one with manual completion (data), one with automatic completion triggered by viewiung it (forum).
$data = $this->getDataGenerator()->create_module('data', ['course' => $course->id], ['completion' => 1]);
......@@ -219,28 +220,45 @@ class core_completion_externallib_testcase extends externallib_advanced_testcase
$this->setUser($teacher);
$result = core_completion_external::override_activity_completion_status($student->id, $data->cmid, COMPLETION_INCOMPLETE);
$result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
$this->assertTrue($result['status']);
$this->assertEquals($result['completionstate'], COMPLETION_INCOMPLETE);
$completiondata = $completion->get_data($cmdata, false, $student->id);
$this->assertEquals(COMPLETION_INCOMPLETE, $completiondata->completionstate);
// Test overriding the status of the manual-completion-activity back to 'complete'.
$result = core_completion_external::override_activity_completion_status($student->id, $data->cmid, COMPLETION_COMPLETE);
$result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
$this->assertTrue($result['status']);
$this->assertEquals($result['completionstate'], COMPLETION_COMPLETE);
$completiondata = $completion->get_data($cmdata, false, $student->id);
$this->assertEquals(COMPLETION_COMPLETE, $completiondata->completionstate);
// Test overriding the status of the auto-completion-activity to 'complete'.
$result = core_completion_external::override_activity_completion_status($student->id, $forum->cmid, COMPLETION_COMPLETE);
$result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
$this->assertTrue($result['status']);
$this->assertEquals($result['completionstate'], COMPLETION_COMPLETE);
$completionforum = $completion->get_data($cmforum, false, $student->id);
$this->assertEquals(COMPLETION_COMPLETE, $completionforum->completionstate);
// Test overriding the status of the auto-completion-activity to 'incomplete'.
$result = core_completion_external::override_activity_completion_status($student->id, $forum->cmid, COMPLETION_INCOMPLETE);
$result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
$this->assertTrue($result['status']);
$this->assertEquals($result['completionstate'], COMPLETION_INCOMPLETE);
$completionforum = $completion->get_data($cmforum, false, $student->id);
$this->assertEquals(COMPLETION_INCOMPLETE, $completionforum->completionstate);
// Test overriding the status of the auto-completion-activity to an invalid state. It should remain incomplete.
$this->expectException('moodle_exception');
$result = core_completion_external::override_activity_completion_status($student->id, $forum->cmid, 3);
$result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
$this->assertEquals($result['completionstate'], COMPLETION_INCOMPLETE);
$completionforum = $completion->get_data($cmforum, false, $student->id);
$this->assertEquals(COMPLETION_INCOMPLETE, $completionforum->completionstate);
// Test overriding the status of the auto-completion-activity for a user without capabilities. It should remain incomplete.
$this->expectException('moodle_exception');
unassign_capability('moodle/course:overridecompletion', $teacherrole->id, $coursecontext);
$result = core_completion_external::override_activity_completion_status($student->id, $forum->cmid, 1);
$result = external_api::clean_returnvalue(core_completion_external::override_activity_completion_status_returns(), $result);
$this->assertEquals($result['completionstate'], COMPLETION_INCOMPLETE);
$completionforum = $completion->get_data($cmforum, false, $student->id);
$this->assertEquals(COMPLETION_INCOMPLETE, $completionforum->completionstate);
}
......
course/renderer.php
View file @ 8fbc41d8
......@@ -510,7 +510,7 @@ class core_course_renderer extends plugin_renderer_base {
if ($completiondata->overrideby) {
$args = new stdClass();
$args->modname = $formattedname;
$overridebyuser = $DB->get_record('user', array('id' => $completiondata->overrideby), '*', MUST_EXIST);
$overridebyuser = \core_user::get_user($completiondata->overrideby, '*', MUST_EXIST);
$args->overrideuser = fullname($overridebyuser);
$imgalt = get_string('completion-alt-' . $completionicon, 'completion', $args);
} else {
......
lib/classes/event/course_module_completion_updated.php
View file @ 8fbc41d8
......@@ -127,6 +127,7 @@ class course_module_completion_updated extends base {
public static function get_other_mapping() {
$othermapped = array();
$othermapped['relateduserid'] = array('db' => 'user', 'restore' => 'user');
$othermapped['overrideby'] = array('db' => 'user', 'restore' => 'user');
return $othermapped;
}
......
lib/completionlib.php
View file @ 8fbc41d8
......@@ -519,6 +519,16 @@ class completion_info {
return $ccompletion->is_complete();
}
/**
* Check whether the supplied user can override the activity completion statuses within the current course.
*
* @param stdClass $user The user object.
* @return bool True if the user can override, false otherwise.
*/
public function user_can_override_completion($user) {
return has_capability('moodle/course:overridecompletion', context_course::instance($this->course_id), $user);
}
/**
* Updates (if necessary) the completion state of activity $cm for the given
* user.
......@@ -550,6 +560,7 @@ class completion_info {
* @param int $userid User ID to be updated. Default 0 = current user
* @param bool $override Whether manually overriding the existing completion state.
* @return void
* @throws moodle_exception if trying to override without permission.
*/
public function update_state($cm, $possibleresult=COMPLETION_UNKNOWN, $userid=0, $override = false) {
global $USER;
......@@ -559,6 +570,14 @@ class completion_info {
return;
}
// If we're processing an override and the current user isn't allowed to do so, then throw an exception.
if ($override) {
if (!$this->user_can_override_completion($USER)) {
throw new required_capability_exception(context_course::instance($this->course_id),
'moodle/course:overridecompletion', 'nopermission', '');
}
}
// Get current value of completion state and do nothing if it's same as
// the possible result of this change. If the change is to COMPLETE and the
// current value is one of the COMPLETE_xx subtypes, ignore that as well
......
lib/tests/completionlib_test.php
View file @ 8fbc41d8
......@@ -128,10 +128,10 @@ class core_completionlib_testcase extends advanced_testcase {
$this->mock_setup();
$mockbuilder = $this->getMockBuilder('completion_info');
$mockbuilder->setMethods(array('is_enabled', 'get_data', 'internal_get_state', 'internal_set_data'));
$mockbuilder->setMethods(array('is_enabled', 'get_data', 'internal_get_state', 'internal_set_data',
'user_can_override_completion'));
$mockbuilder->setConstructorArgs(array((object)array('id' => 42)));
$c = $mockbuilder->getMock();
$cm = (object)array('id'=>13, 'course'=>42);
// Not enabled, should do nothing.
......@@ -229,7 +229,10 @@ class core_completionlib_testcase extends advanced_testcase {
->method('is_enabled')
->with($cm)
->will($this->returnValue(true));
$c->expects($this->at(1))
$c->expects($this->at(1)) // Pretend the user has the required capability for overriding completion statuses.
->method('user_can_override_completion')
->will($this->returnValue(true));
$c->expects($this->at(2))
->method('get_data')
->with($cm, false, 100)
->will($this->returnValue($current));
......@@ -239,7 +242,7 @@ class core_completionlib_testcase extends advanced_testcase {
$changed->overrideby = 314159;
$comparewith = new phpunit_constraint_object_is_equal_with_exceptions($changed);
$comparewith->add_exception('timemodified', 'assertGreaterThanOrEqual');
$c->expects($this->at(2))
$c->expects($this->at(3))
->method('internal_set_data')
->with($cm, $comparewith);
$c->update_state($cm, COMPLETION_COMPLETE, 100, true);
......@@ -258,7 +261,10 @@ class core_completionlib_testcase extends advanced_testcase {
->method('is_enabled')
->with($cm)
->will($this->returnValue(true));
$c->expects($this->at(1))
$c->expects($this->at(1)) // Pretend the user has the required capability for overriding completion statuses.
->method('user_can_override_completion')
->will($this->returnValue(true));
$c->expects($this->at(2))
->method('get_data')
->with($cm, false, 100)
->will($this->returnValue($current));
......@@ -268,7 +274,7 @@ class core_completionlib_testcase extends advanced_testcase {
$changed->overrideby = 314159;
$comparewith = new phpunit_constraint_object_is_equal_with_exceptions($changed);
$comparewith->add_exception('timemodified', 'assertGreaterThanOrEqual');
$c->expects($this->at(2))
$c->expects($this->at(3))
->method('internal_set_data')
->with($cm, $comparewith);
$c->update_state($cm, COMPLETION_COMPLETE, 100, true);
......@@ -294,7 +300,10 @@ class core_completionlib_testcase extends advanced_testcase {
->method('is_enabled')
->with($cm)
->will($this->returnValue(true));
$c->expects($this->at(1))
$c->expects($this->at(1)) // Pretend the user has the required capability for overriding completion statuses.
->method('user_can_override_completion')
->will($this->returnValue(true));
$c->expects($this->at(2))
->method('get_data')
->with($cm, false, 100)
->will($this->returnValue($current));
......@@ -304,7 +313,7 @@ class core_completionlib_testcase extends advanced_testcase {
$changed->overrideby = 314159;
$comparewith = new phpunit_constraint_object_is_equal_with_exceptions($changed);
$comparewith->add_exception('timemodified', 'assertGreaterThanOrEqual');
$c->expects($this->at(2))
$c->expects($this->at(3))
->method('internal_set_data')
->with($cm, $comparewith);
$c->update_state($cm, COMPLETION_INCOMPLETE, 100, true);
......
report/progress/amd/build/completion_override.min.js
View file @ 8fbc41d8
File suppressed by a .gitattributes entry or the file's encoding is unsupported.
report/progress/amd/src/completion_override.js
View file @ 8fbc41d8
......@@ -51,9 +51,10 @@ define(['jquery', 'core/ajax', 'core/str', 'core/modal_factory', 'core/modal_eve
}])[0];
}).then(function(results) {
// Update the DOM accordingly.
var flipState = results.completionstate ? 0 : 1;
triggerElement.find('.loading-icon').remove();
triggerElement.data('changecompl', results.changecompl);
triggerElement.attr('data-changecompl', results.changecompl);
triggerElement.data('changecompl', override.userid + '-' + override.cmid + '-' + flipState);
triggerElement.attr('data-changecompl', override.userid + '-' + override.cmid + '-' + flipState);
triggerElement.children("img").replaceWith(results.img);
return;
}).catch(Notification.exception);
......@@ -68,7 +69,9 @@ define(['jquery', 'core/ajax', 'core/str', 'core/modal_factory', 'core/modal_eve
*/
var userConfirm = function(e, data) {
data.originalEvent.preventDefault();
data.originalEvent.stopPropagation();
e.preventDefault();
e.stopPropagation();
triggerElement = $(e.currentTarget);
var elemData = triggerElement.data('changecompl').split('-');
......
report/progress/index.php
View file @ 8fbc41d8
......@@ -51,9 +51,6 @@ $sifirst = optional_param('sifirst', 'all', PARAM_NOTAGS);
$silast = optional_param('silast', 'all', PARAM_NOTAGS);
$start = optional_param('start', 0, PARAM_INT);
// Action.
$changecompl = optional_param('changecompl', '', PARAM_ALPHANUMEXT);
// Whether to show extra user identity information
$extrafields = get_extra_user_fields($context);
$leftcols = 1 + count($extrafields);
......@@ -103,20 +100,6 @@ $reportsurl = $CFG->wwwroot.'/course/report.php?id='.$course->id;
$completion = new completion_info($course);
$activities = $completion->get_activities();
if ($changecompl) {
if ($changecompl) {
require_capability('moodle/course:overridecompletion', $context);
require_sesskey();
list($userid, $cmid, $newstate) = preg_split('/-/', $changecompl, 3);
// Make sure the activity and user are tracked.
if (isset($activities[$cmid]) &&
$completion->get_num_tracked_users('u.id = :userid', array('userid' => (int)$userid), $group)) {
$completion->update_state($activities[$cmid], $newstate, $userid, true);
}
redirect($PAGE->url);
}
}
if ($sifirst !== 'all') {
set_user_preference('ifirst', $sifirst);
}
......@@ -416,7 +399,7 @@ foreach($progress as $user) {
'-'.$completiontype;
if ($overrideby) {
$overridebyuser = $DB->get_record('user', array('id' => $overrideby), '*', MUST_EXIST);
$overridebyuser = \core_user::get_user($overrideby, '*', MUST_EXIST);
$describe = get_string('completion-' . $completiontype, 'completion', fullname($overridebyuser));
} else {
$describe = get_string('completion-' . $completiontype, 'completion');
......@@ -436,8 +419,7 @@ foreach($progress as $user) {
$state != COMPLETION_COMPLETE_PASS && $state != COMPLETION_COMPLETE_FAIL) {
$newstate = ($state == COMPLETION_COMPLETE) ? COMPLETION_INCOMPLETE : COMPLETION_COMPLETE;
$changecompl = $user->id . '-' . $activity->id . '-' . $newstate;
$url = new moodle_url($PAGE->url, array('sesskey' => sesskey(),
'changecompl' => $changecompl));
$url = new moodle_url($PAGE->url, ['sesskey' => sesskey()]);
$celltext = html_writer::link($url, $celltext, array('class' => 'changecompl', 'data-changecompl' => $changecompl,
'aria-role' => 'button'));
}
......
report/progress/styles.css
View file @ 8fbc41d8
......@@ -59,9 +59,4 @@
#page-report-progress-index .modicon {
padding-top: 5px;
}
#page-report-progress-index #completion-progress td a .ajaxworking {
height: 16px;
background: url([[pix:i/ajaxloader]]) no-repeat;
}
/*rtl:end:ignore*/
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment