Skip to content

Commit

Permalink
Merge branch 'MDL-65634_master' of git://github.com/dmonllao/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
@abgreeve
abgreeve committed May 23, 2019
2 parents aec03b8 + ed1a2e8 commit c37a446
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 10 deletions.
5 changes: 5 additions & 0 deletions course/classes/analytics/target/course_competencies.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ public function is_valid_analysable(\core_analytics\analysable $course, $fortrai
*/
protected function calculate_sample($sampleid, \core_analytics\analysable $course, $starttime = false, $endtime = false) {

if ($this->enrolment_starts_after_calculation_start($sampleid, $starttime)) {
// Discard user enrolments whose start date is after $starttime.
return null;
}

$userenrol = $this->retrieve('user_enrolments', $sampleid);

$key = $course->get_id();
Expand Down
4 changes: 4 additions & 0 deletions course/classes/analytics/target/course_completion.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,10 @@ public function is_valid_analysable(\core_analytics\analysable $course, $fortrai
*/
protected function calculate_sample($sampleid, \core_analytics\analysable $course, $starttime = false, $endtime = false) {

if ($this->enrolment_starts_after_calculation_start($sampleid, $starttime)) {
// Discard user enrolments whose start date is after $starttime.
return null;
}
$userenrol = $this->retrieve('user_enrolments', $sampleid);

// We use completion as a success metric.
Expand Down
5 changes: 5 additions & 0 deletions course/classes/analytics/target/course_dropout.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,11 @@ public function is_valid_analysable(\core_analytics\analysable $course, $fortrai
*/
protected function calculate_sample($sampleid, \core_analytics\analysable $course, $starttime = false, $endtime = false) {

if ($this->enrolment_starts_after_calculation_start($sampleid, $starttime)) {
// Discard user enrolments whose start date is after $starttime.
return null;
}

$userenrol = $this->retrieve('user_enrolments', $sampleid);

// We use completion as a success metric only when it is enabled.
Expand Down
44 changes: 41 additions & 3 deletions course/classes/analytics/target/course_enrolments.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ public function is_valid_analysable(\core_analytics\analysable $course, $fortrai
*/
public function is_valid_sample($sampleid, \core_analytics\analysable $course, $fortraining = true) {

$now = time();

$userenrol = $this->retrieve('user_enrolments', $sampleid);
if ($userenrol->timeend && $course->get_start() > $userenrol->timeend) {
// Discard enrolments which time end is prior to the course start. This should get rid of
Expand All @@ -139,9 +141,22 @@ public function is_valid_sample($sampleid, \core_analytics\analysable $course, $
return false;
}

if (($userenrol->timestart && $userenrol->timestart > $course->get_end()) ||
(!$userenrol->timestart && $userenrol->timecreated > $course->get_end())) {
// Discard user enrolments that starts after the analysable official end.
if ($course->get_end()) {
if (($userenrol->timestart && $userenrol->timestart > $course->get_end()) ||
(!$userenrol->timestart && $userenrol->timecreated > $course->get_end())) {
// Discard user enrolments that starts after the analysable official end.
return false;
}

}

if ($now < $userenrol->timestart && $userenrol->timestart) {
// Discard enrolments whose start date is after now (no need to check timecreated > $now :P).
return false;
}

if (!$fortraining && $userenrol->timeend && $userenrol->timeend < $now) {
// We don't want to generate predictions for finished enrolments.
return false;
}

Expand Down Expand Up @@ -182,4 +197,27 @@ public function prediction_actions(\core_analytics\prediction $prediction, $incl

return array_merge($actions, parent::prediction_actions($prediction, $includedetailsaction));
}

/**
* Does the user enrolment created after this time range start time or starts after it?
*
* We need to identify these enrolments because the indicators can not be calculated properly
* if the student enrolment started half way through this time range.
*
* User enrolments whose end date is before time() have already been discarded in
* course_enrolments::is_valid_sample.
*
* @param int $sampleid
* @param int $starttime
* @return bool
*/
protected function enrolment_starts_after_calculation_start(int $sampleid, int $starttime) {

$userenrol = $this->retrieve('user_enrolments', $sampleid);
if ($userenrol->timestart && $userenrol->timestart > $starttime) {
return true;
}

return false;
}
}
5 changes: 5 additions & 0 deletions course/classes/analytics/target/course_gradetopass.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,11 @@ public function is_valid_analysable(\core_analytics\analysable $course, $fortrai
*/
protected function calculate_sample($sampleid, \core_analytics\analysable $course, $starttime = false, $endtime = false) {

if ($this->enrolment_starts_after_calculation_start($sampleid, $starttime)) {
// Discard user enrolments whose start date is after $starttime.
return null;
}

$userenrol = $this->retrieve('user_enrolments', $sampleid);

// Get course grade to pass.
Expand Down
46 changes: 39 additions & 7 deletions lib/tests/targets_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -148,21 +148,40 @@ public function sample_provider() {
'courseend' => $now + (WEEKSECS * 8),
'timestart' => $now,
'timeend' => $now - DAYSECS,
'isvalid' => false
'isvalidfortraining' => false,
'isvalidforprediction' => false
],
'enrolmenttoolong' => [
'coursestart' => $now,
'courseend' => $now + (WEEKSECS * 8),
'timestart' => $now - (YEARSECS + (WEEKSECS * 8)),
'timeend' => $now + (WEEKSECS * 8),
'isvalid' => false
'isvalidfortraining' => false,
'isvalidforprediction' => false
],
'enrolmentstartaftercourse' => [
'coursestart' => $now,
'courseend' => $now + (WEEKSECS * 8),
'timestart' => $now + (WEEKSECS * 9),
'timeend' => $now + (WEEKSECS * 10),
'isvalid' => false
'isvalidfortraining' => false,
'isvalidforprediction' => false
],
'enrolmentstartsafternow' => [
'coursestart' => $now,
'courseend' => $now + (WEEKSECS * 8),
'timestart' => $now + (WEEKSECS * 2),
'timeend' => $now + (WEEKSECS * 7),
'isvalidfortraining' => false,
'isvalidforprediction' => false
],
'enrolmentfinishedbeforenow' => [
'coursestart' => $now - (WEEKSECS * 4),
'courseend' => $now - (WEEKSECS * 1),
'timestart' => $now - (WEEKSECS * 3),
'timeend' => $now - (WEEKSECS * 2),
'isvalidfortraining' => true,
'isvalidforprediction' => false
],
];
}
Expand Down Expand Up @@ -228,9 +247,11 @@ public function test_core_target_course_completion_analysable($courseparams, $is
* @param int $courseend Course end date
* @param int $timestart Enrol start date
* @param int $timeend Enrol end date
* @param boolean $isvalid True when sample is valid, false when it is not
* @param boolean $isvalidfortraining True when sample is valid for training, false when it is not
* @param boolean $isvalidforprediction True when sample is valid for prediction, false when it is not
*/
public function test_core_target_course_completion_samples($coursestart, $courseend, $timestart, $timeend, $isvalid) {
public function test_core_target_course_completion_samples($coursestart, $courseend, $timestart, $timeend,
$isvalidfortraining, $isvalidforprediction) {

$this->resetAfterTest(true);

Expand All @@ -254,7 +275,8 @@ public function test_core_target_course_completion_samples($coursestart, $course
$target->add_sample_data($samplesdata);
$sampleid = reset($sampleids);

$this->assertEquals($isvalid, $target->is_valid_sample($sampleid, $analysable));
$this->assertEquals($isvalidfortraining, $target->is_valid_sample($sampleid, $analysable, true));
$this->assertEquals($isvalidforprediction, $target->is_valid_sample($sampleid, $analysable, false));
}

/**
Expand Down Expand Up @@ -391,11 +413,15 @@ public function test_core_target_course_gradetopass_calculate() {
$student1 = $dg->create_user();
$student2 = $dg->create_user();
$student3 = $dg->create_user();
$student4 = $dg->create_user();
$studentrole = $DB->get_record('role', array('shortname' => 'student'));
$dg->enrol_user($student1->id, $course1->id, $studentrole->id);
$dg->enrol_user($student2->id, $course1->id, $studentrole->id);
$dg->enrol_user($student3->id, $course1->id, $studentrole->id);

$enrolstart = mktime(0, 0, 0, 10, 25, 2015);
$dg->enrol_user($student4->id, $course1->id, $studentrole->id, 'manual', $enrolstart);

// get_all_samples() does not guarantee any order, so let's
// explicitly define the expectations here for later comparing.
// Expectations format being array($userid => expectation, ...)
Expand All @@ -413,6 +439,9 @@ public function test_core_target_course_gradetopass_calculate() {
// Student 3 (has no grade) fails, so it's non achieved sample.
$expectations[$student3->id] = 1;

// Student 4 should be null as its enrolment timestart is after the this range.
$expectations[$student4->id] = null;

$courseitem->gradepass = 50;
$DB->update_record('grade_items', $courseitem);

Expand All @@ -431,9 +460,12 @@ public function test_core_target_course_gradetopass_calculate() {
$method = $class->getMethod('calculate_sample');
$method->setAccessible(true);

$starttime = mktime(0, 0, 0, 10, 24, 2015);

// Verify all the expectations are fulfilled.
foreach ($sampleids as $sampleid => $key) {
$this->assertEquals($expectations[$samplesdata[$key]['user']->id], $method->invoke($target, $sampleid, $analysable));
$this->assertEquals($expectations[$samplesdata[$key]['user']->id], $method->invoke($target, $sampleid,
$analysable, $starttime));
}
}
}

0 comments on commit c37a446

Please sign in to comment.