Skip to content

Commit

Permalink
Merge branch 'MDL-60434_master' of git://github.com/dmonllao/moodle
Browse files Browse the repository at this point in the history
  • Loading branch information
@snake
snake committed Oct 26, 2017
2 parents 7dfc9ea + 3a89d0b commit 1e67827
Show file tree
Hide file tree
Showing 7 changed files with 92 additions and 71 deletions.
4 changes: 4 additions & 0 deletions analytics/classes/analysable.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@
/**
* Any element analysers can analyse.
*
* Analysers get_analysers method return all analysable elements in the site;
* it is important that analysable elements implement lazy loading to avoid
* big memory footprints. See \core_analytics\course example.
*
* @package core_analytics
* @copyright 2016 David Monllao {@link http://www.davidmonllao.com}
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
Expand Down
140 changes: 79 additions & 61 deletions analytics/classes/course.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,19 @@
class course implements \core_analytics\analysable {

/**
* @var \core_analytics\course[] $instances
* @var bool Has this course data been already loaded.
*/
protected static $instances = array();
protected $loaded = false;

/**
* @var int $cachedid self::$cachedinstance analysable id.
*/
protected static $cachedid = 0;

/**
* @var \core_analytics\course $cachedinstance
*/
protected static $cachedinstance = null;

/**
* Course object
Expand Down Expand Up @@ -122,43 +132,27 @@ class course implements \core_analytics\analysable {
* Use self::instance() instead to get cached copies of the course. Instances obtained
* through this constructor will not be cached.
*
* Loads course students and teachers.
* Lazy load of course data, students and teachers.
*
* @param int|stdClass $course Course id
* @return void
*/
public function __construct($course) {

if (is_scalar($course)) {
$this->course = get_course($course);
$this->course = new \stdClass();
$this->course->id = $course;
} else {
$this->course = $course;
}

$this->coursecontext = \context_course::instance($this->course->id);

$this->now = time();

// Get the course users, including users assigned to student and teacher roles at an higher context.
$cache = \cache::make_from_params(\cache_store::MODE_REQUEST, 'core_analytics', 'rolearchetypes');

if (!$studentroles = $cache->get('student')) {
$studentroles = array_keys(get_archetype_roles('student'));
$cache->set('student', $studentroles);
}
$this->studentids = $this->get_user_ids($studentroles);

if (!$teacherroles = $cache->get('teacher')) {
$teacherroles = array_keys(get_archetype_roles('editingteacher') + get_archetype_roles('teacher'));
$cache->set('teacher', $teacherroles);
}
$this->teacherids = $this->get_user_ids($teacherroles);
}

/**
* Returns an analytics course instance.
*
* @param int|stdClass $course Course id
* Lazy load of course data, students and teachers.
*
* @param int|stdClass $course Course object or course id
* @return \core_analytics\course
*/
public static function instance($course) {
Expand All @@ -168,31 +162,59 @@ public static function instance($course) {
$courseid = $course->id;
}

if (!empty(self::$instances[$courseid])) {
return self::$instances[$courseid];
if (self::$cachedid === $courseid) {
return self::$cachedinstance;
}

$instance = new \core_analytics\course($course);
self::$instances[$courseid] = $instance;
return self::$instances[$courseid];
$cachedinstance = new \core_analytics\course($course);
self::$cachedinstance = $cachedinstance;
self::$cachedid = (int)$courseid;
return self::$cachedinstance;
}

/**
* Clears all statically cached instances.
* get_id
*
* @return void
* @return int
*/
public static function reset_caches() {
self::$instances = array();
public function get_id() {
return $this->course->id;
}

/**
* get_id
* Loads the analytics course object.
*
* @return int
* @return null
*/
public function get_id() {
return $this->course->id;
protected function load() {

// The instance constructor could be already loaded with the full course object. Using shortname
// because it is a required course field.
if (empty($this->course->shortname)) {
$this->course = get_course($this->course->id);
}

$this->coursecontext = $this->get_context();

$this->now = time();

// Get the course users, including users assigned to student and teacher roles at an higher context.
$cache = \cache::make_from_params(\cache_store::MODE_REQUEST, 'core_analytics', 'rolearchetypes');

// Flag the instance as loaded.
$this->loaded = true;

if (!$studentroles = $cache->get('student')) {
$studentroles = array_keys(get_archetype_roles('student'));
$cache->set('student', $studentroles);
}
$this->studentids = $this->get_user_ids($studentroles);

if (!$teacherroles = $cache->get('teacher')) {
$teacherroles = array_keys(get_archetype_roles('editingteacher') + get_archetype_roles('teacher'));
$cache->set('teacher', $teacherroles);
}
$this->teacherids = $this->get_user_ids($teacherroles);
}

/**
Expand All @@ -201,7 +223,7 @@ public function get_id() {
* @return string
*/
public function get_name() {
return format_string($this->course->shortname, true, array('context' => $this->get_context()));
return format_string($this->get_course_data()->shortname, true, array('context' => $this->get_context()));
}

/**
Expand All @@ -228,8 +250,8 @@ public function get_start() {
}

// The field always exist but may have no valid if the course is created through a sync process.
if (!empty($this->course->startdate)) {
$this->starttime = (int)$this->course->startdate;
if (!empty($this->get_course_data()->startdate)) {
$this->starttime = (int)$this->get_course_data()->startdate;
} else {
$this->starttime = 0;
}
Expand All @@ -256,7 +278,7 @@ public function guess_start() {

// We first try to find current course student logs.
$firstlogs = array();
foreach ($this->studentids as $studentid) {
foreach ($this->get_students() as $studentid) {
// Grrr, we are limited by logging API, we could do this easily with a
// select min(timecreated) from xx where courseid = yy group by userid.

Expand All @@ -278,7 +300,7 @@ public function guess_start() {
sort($firstlogs);
$firstlogsmedian = $this->median($firstlogs);

$studentenrolments = enrol_get_course_users($this->get_id(), $this->studentids);
$studentenrolments = enrol_get_course_users($this->get_id(), $this->get_students());
if (empty($studentenrolments)) {
return 0;
}
Expand Down Expand Up @@ -306,8 +328,8 @@ public function get_end() {
}

// The enddate field is only available from Moodle 3.2 (MDL-22078).
if (!empty($this->course->enddate)) {
$this->endtime = (int)$this->course->enddate;
if (!empty($this->get_course_data()->enddate)) {
$this->endtime = (int)$this->get_course_data()->enddate;
return $this->endtime;
}

Expand Down Expand Up @@ -362,21 +384,12 @@ public function guess_end() {
* @return \stdClass
*/
public function get_course_data() {
return $this->course;
}

/**
* Is the course valid to extract indicators from it?
*
* @return bool
*/
public function is_valid() {

if (!$this->was_started() || !$this->is_finished()) {
return false;
if (!$this->loaded) {
$this->load();
}

return true;
return $this->course;
}

/**
Expand Down Expand Up @@ -427,7 +440,7 @@ public function is_finished() {
public function get_user_ids($roleids) {

// We need to index by ra.id as a user may have more than 1 $roles role.
$records = get_role_users($roleids, $this->coursecontext, true, 'ra.id, u.id AS userid, r.id AS roleid', 'ra.id ASC');
$records = get_role_users($roleids, $this->get_context(), true, 'ra.id, u.id AS userid, r.id AS roleid', 'ra.id ASC');

// If a user have more than 1 $roles role array_combine will discard the duplicate.
$callable = array($this, 'filter_user_id');
Expand All @@ -441,6 +454,11 @@ public function get_user_ids($roleids) {
* @return stdClass[]
*/
public function get_students() {

if (!$this->loaded) {
$this->load();
}

return $this->studentids;
}

Expand All @@ -453,7 +471,7 @@ public function get_total_logs() {
global $DB;

// No logs if no students.
if (empty($this->studentids)) {
if (empty($this->get_students())) {
return 0;
}

Expand Down Expand Up @@ -606,7 +624,7 @@ protected function completed_by(\cm_info $activity, $starttime, $endtime) {

// When the course is using format weeks we use the week's end date.
$format = course_get_format($activity->get_modinfo()->get_course());
if ($this->course->format === 'weeks') {
if ($this->get_course_data()->format === 'weeks') {
$dates = $format->get_section_dates($section);

// We need to consider the +2 hours added by get_section_dates.
Expand All @@ -628,7 +646,7 @@ protected function completed_by(\cm_info $activity, $starttime, $endtime) {
return false;
}

if (!course_format_uses_sections($this->course->format)) {
if (!course_format_uses_sections($this->get_course_data()->format)) {
// If it does not use sections and there are no availability conditions to access it it is available
// and we can not magically classify it into any other time range than this one.
return true;
Expand Down Expand Up @@ -731,7 +749,7 @@ protected function course_students_query_filter($prefix = false) {
}

// Check the amount of student logs in the 4 previous weeks.
list($studentssql, $studentsparams) = $DB->get_in_or_equal($this->studentids, SQL_PARAMS_NAMED);
list($studentssql, $studentsparams) = $DB->get_in_or_equal($this->get_students(), SQL_PARAMS_NAMED);
$filterselect = $prefix . 'courseid = :courseid AND ' . $prefix . 'userid ' . $studentssql;
$filterparams = array('courseid' => $this->course->id) + $studentsparams;

Expand Down
4 changes: 2 additions & 2 deletions analytics/classes/local/analyser/by_course.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,14 @@ public function get_analysables() {
$courses = $this->options['filter'];
} else {
// Iterate through all potentially valid courses.
$courses = get_courses('all', 'c.sortorder ASC');
$courses = get_courses('all', 'c.sortorder ASC', 'c.id');
}
unset($courses[SITEID]);

$analysables = array();
foreach ($courses as $course) {
// Skip the frontpage course.
$analysable = \core_analytics\course::instance($course);
$analysable = \core_analytics\course::instance($course->id);
$analysables[$analysable->get_id()] = $analysable;
}

Expand Down
10 changes: 4 additions & 6 deletions analytics/tests/course_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,47 +86,45 @@ public function test_course_validation() {
$courseman = new \core_analytics\course($this->course->id);
$this->assertFalse($courseman->was_started());
$this->assertFalse($courseman->is_finished());
$this->assertFalse($courseman->is_valid());

// Nothing should change when assigning as teacher.
for ($i = 0; $i < 10; $i++) {
$user = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($user->id, $this->course->id, $this->teacherroleid);
}
$courseman = new \core_analytics\course($this->course->id);
$this->assertFalse($courseman->is_valid());
$this->assertFalse($courseman->was_started());
$this->assertFalse($courseman->is_finished());

// More students now.
for ($i = 0; $i < 10; $i++) {
$user = $this->getDataGenerator()->create_user();
$this->getDataGenerator()->enrol_user($user->id, $this->course->id, $this->studentroleid);
}
$courseman = new \core_analytics\course($this->course->id);
$this->assertFalse($courseman->is_valid());
$this->assertFalse($courseman->was_started());
$this->assertFalse($courseman->is_finished());

// Valid start date unknown end date.
$this->course->startdate = gmmktime('0', '0', '0', 10, 24, 2015);
$DB->update_record('course', $this->course);
$courseman = new \core_analytics\course($this->course->id);
$this->assertTrue($courseman->was_started());
$this->assertFalse($courseman->is_finished());
$this->assertFalse($courseman->is_valid());

// Valid start and end date.
$this->course->enddate = gmmktime('0', '0', '0', 8, 27, 2016);
$DB->update_record('course', $this->course);
$courseman = new \core_analytics\course($this->course->id);
$this->assertTrue($courseman->was_started());
$this->assertTrue($courseman->is_finished());
$this->assertTrue($courseman->is_valid());

// Valid start and ongoing course.
$this->course->enddate = gmmktime('0', '0', '0', 8, 27, 2286);
$DB->update_record('course', $this->course);
$courseman = new \core_analytics\course($this->course->id);
$this->assertTrue($courseman->was_started());
$this->assertFalse($courseman->is_finished());
$this->assertFalse($courseman->is_valid());
}

/**
Expand Down
1 change: 1 addition & 0 deletions analytics/tests/prediction_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@

defined('MOODLE_INTERNAL') || die();

global $CFG;
require_once(__DIR__ . '/fixtures/test_indicator_max.php');
require_once(__DIR__ . '/fixtures/test_indicator_min.php');
require_once(__DIR__ . '/fixtures/test_indicator_fullname.php');
Expand Down
3 changes: 2 additions & 1 deletion course/tests/indicators_test.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@

defined('MOODLE_INTERNAL') || die();

global $CFG;
require_once(__DIR__ . '/../../lib/completionlib.php');
require_once(__DIR__ . '/../../completion/criteria/completion_criteria_self.php');


/**
* Unit tests for core_course indicators.
*
Expand Down
1 change: 0 additions & 1 deletion lib/phpunit/classes/util.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ public static function reset_all_data($detectchanges = false) {
if (class_exists('core_media_manager', false)) {
core_media_manager::reset_caches();
}
\core_analytics\course::reset_caches();

// Reset static unit test options.
if (class_exists('\availability_date\condition', false)) {
Expand Down

0 comments on commit 1e67827

Please sign in to comment.