MDL-57791 analytics: Replace settings by get_archetype_roles

diff --git a/admin/settings/analytics.php b/admin/settings/analytics.php
index 59f25f1..32a26d9 100644 (file)
--- a/admin/settings/analytics.php
+++ b/admin/settings/analytics.php
@@ -78,29 +78,5 @@ if ($hassiteconfig) {
         $defaultmodeloutputdir = rtrim($CFG->dataroot, '/') . DIRECTORY_SEPARATOR . 'models';
         $settings->add(new admin_setting_configdirectory('analytics/modeloutputdir', new lang_string('modeloutputdir', 'analytics'),
             new lang_string('modeloutputdirinfo', 'analytics'), $defaultmodeloutputdir));
-        $studentdefaultroles = [];
-        $teacherdefaultroles = [];
-
-        // Student and teacher roles.
-        $allroles = role_fix_names(get_all_roles());
-        $rolechoices = [];
-        foreach ($allroles as $role) {
-            $rolechoices[$role->id] = $role->localname;
-
-            if ($role->shortname == 'student') {
-                $studentdefaultroles[] = $role->id;
-            } else if ($role->shortname == 'teacher') {
-                $teacherdefaultroles[] = $role->id;
-            } else if ($role->shortname == 'editingteacher') {
-                $teacherdefaultroles[] = $role->id;
-            }
-        }
-
-        $settings->add(new admin_setting_configmultiselect('analytics/teacherroles', new lang_string('teacherroles', 'analytics'),
-           '', $teacherdefaultroles, $rolechoices));
-
-        $settings->add(new admin_setting_configmultiselect('analytics/studentroles', new lang_string('studentroles', 'analytics'),
-           '', $studentdefaultroles, $rolechoices));
-
     }
 }

diff --git a/analytics/classes/course.php b/analytics/classes/course.php
index 942ed03..5a66179 100644 (file)
--- a/analytics/classes/course.php
+++ b/analytics/classes/course.php
@@ -39,8 +39,8 @@ class course implements \core_analytics\analysable {
 
     protected static $instances = array();
 
-    protected $studentroles = [];
-    protected $teacherroles = [];
+    protected static $studentroles = [];
+    protected static $teacherroles = [];
 
     protected $course = null;
     protected $coursecontext = null;
@@ -61,15 +61,11 @@ class course implements \core_analytics\analysable {
      * Course manager constructor.
      *
      * Use self::instance() instead to get cached copies of the course. Instances obtained
-     * through this constructor will not be cached either.
+     * through this constructor will not be cached.
      *
      * Loads course students and teachers.
      *
-     * Let's try to keep this computationally inexpensive.
-     *
      * @param int|stdClass $course Course id
-     * @param array $studentroles
-     * @param array $teacherroles
      * @return void
      */
     public function __construct($course) {
@@ -82,29 +78,25 @@ class course implements \core_analytics\analysable {
 
         $this->coursecontext = \context_course::instance($this->course->id);
 
-        $studentroles = get_config('analytics', 'studentroles');
-        $teacherroles = get_config('analytics', 'teacherroles');
-
-        if (empty($studentroles) || empty($teacherroles)) {
-            // Unexpected, site settings should be set with default values.
-            throw new \moodle_exception('errornoroles', 'analytics');
+        if (empty(self::$studentroles)) {
+            self::$studentroles = array_keys(get_archetype_roles('student'));
+        }
+        if (empty(self::$teacherroles)) {
+            self::$teacherroles = array_keys(get_archetype_roles('editingteacher') + get_archetype_roles('teacher'));
         }
-
-        $this->studentroles = explode(',', $studentroles);
-        $this->teacherroles = explode(',', $teacherroles);
 
         $this->now = time();
 
         // Get the course users, including users assigned to student and teacher roles at an higher context.
-        $this->studentids = $this->get_user_ids($this->studentroles);
-        $this->teacherids = $this->get_user_ids($this->teacherroles);
+        $this->studentids = $this->get_user_ids(self::$studentroles);
+        $this->teacherids = $this->get_user_ids(self::$teacherroles);
     }
 
     /**
-     * instance
+     * Returns an analytics course instance.
      *
      * @param int|stdClass $course Course id
-     * @return void
+     * @return \core_analytics\course
      */
     public static function instance($course) {
 

diff --git a/analytics/classes/local/analyser/by_course.php b/analytics/classes/local/analyser/by_course.php
index 65e224d..c531d80 100644 (file)
--- a/analytics/classes/local/analyser/by_course.php
+++ b/analytics/classes/local/analyser/by_course.php
@@ -38,15 +38,15 @@ abstract class by_course extends base {
 
         // Default to all system courses.
         if (!empty($this->options['filter'])) {
-            $courseids = $this->options['filter'];
+            $it = $this->options['filter'];
         } else {
             // Iterate through all potentially valid courses.
-            $courseids = $DB->get_fieldset_select('course', 'id', 'id != :frontpage', array('frontpage' => SITEID), 'sortorder ASC');
+            $it = $DB->get_recordset_select('course', 'id != :frontpage', array('frontpage' => SITEID), 'sortorder ASC');
         }
 
         $analysables = array();
-        foreach ($courseids as $courseid) {
-            $analysable = new \core_analytics\course($courseid);
+        foreach ($it as $course) {
+            $analysable = \core_analytics\course::instance($course);
             $analysables[$analysable->get_id()] = $analysable;
         }
 

diff --git a/analytics/classes/local/analyser/courses.php b/analytics/classes/local/analyser/courses.php
index 5db36b1..d87fea9 100644 (file)
--- a/analytics/classes/local/analyser/courses.php
+++ b/analytics/classes/local/analyser/courses.php
@@ -46,7 +46,7 @@ class courses extends by_course {
      * @return \core_analytics\analysable
      */
     public function get_sample_analysable($sampleid) {
-        return new \core_analytics\course($sampleid);
+        return \core_analytics\course::instance($sampleid);
     }
 
     protected function provided_sample_data() {

diff --git a/analytics/classes/local/analyser/sitewide.php b/analytics/classes/local/analyser/sitewide.php
index 3bae3bb..ed91ce5 100644 (file)
--- a/analytics/classes/local/analyser/sitewide.php
+++ b/analytics/classes/local/analyser/sitewide.php
@@ -47,7 +47,6 @@ abstract class sitewide extends base {
         $files = $this->process_analysable($analysable, $includetarget);
 
         // Copy to range files as there is just one analysable.
-        // TODO Not abstracted as we should ideally directly store it as range-scope file.
         foreach ($files as $timesplittingid => $file) {
 
             if ($this->options['evaluation'] === true) {
@@ -56,7 +55,6 @@ abstract class sitewide extends base {
             }
 
             // We use merge but it is just a copy
-            // TODO use copy or move if there are performance issues.
             $files[$timesplittingid] = \core_analytics\dataset_manager::merge_datasets(array($file), $this->modelid,
                 $timesplittingid, $this->options['evaluation'], $includetarget);
         }

diff --git a/analytics/classes/local/analyser/student_enrolments.php b/analytics/classes/local/analyser/student_enrolments.php
index bcb1132..970caba 100644 (file)
--- a/analytics/classes/local/analyser/student_enrolments.php
+++ b/analytics/classes/local/analyser/student_enrolments.php
@@ -49,8 +49,8 @@ class student_enrolments extends by_course {
     }
 
     public function get_sample_analysable($sampleid) {
-        $course = enrol_get_course_by_user_enrolment_id($ueid);
-        return \core_analytics\course($course);
+        $course = enrol_get_course_by_user_enrolment_id($sampleid);
+        return \core_analytics\course::instance($course);
     }
 
     protected function provided_sample_data() {

diff --git a/analytics/tests/course_test.php b/analytics/tests/course_test.php
index 2c8c618..ba8e4c5 100644 (file)
--- a/analytics/tests/course_test.php
+++ b/analytics/tests/course_test.php
@@ -54,10 +54,6 @@ class core_analytics_course_testcase extends advanced_testcase {
         $this->getDataGenerator()->enrol_user($this->both->id, $this->course->id, $this->editingteacherroleid);
         $this->getDataGenerator()->enrol_user($this->editingteacher->id, $this->course->id, $this->editingteacherroleid);
         $this->getDataGenerator()->enrol_user($this->teacher->id, $this->course->id, $this->teacherroleid);
-
-
-        set_config('studentroles', $this->studentroleid, 'analytics');
-        set_config('teacherroles', $this->editingteacherroleid . ',' . $this->teacherroleid, 'analytics');
     }
 
     /**

diff --git a/lang/en/analytics.php b/lang/en/analytics.php
index ce670a7..7d73779 100644 (file)
--- a/lang/en/analytics.php
+++ b/lang/en/analytics.php
@@ -75,9 +75,7 @@ $string['predictionsprocessor'] = 'Predictions processor';
 $string['predictionsprocessor_help'] = 'Prediction processors are the machine learning backends that process the datasets generated by calculating models\' indicators and targets.';
 $string['processingsitecontents'] = 'Processing site contents';
 $string['processingsitecontents'] = 'Processing site contents';
-$string['studentroles'] = 'Student roles';
 $string['successfullyanalysed'] = 'Successfully analysed';
-$string['teacherroles'] = 'Teacher roles';
 $string['timesplitting:deciles'] = 'Deciles';
 $string['timesplitting:decilesaccum'] = 'Deciles accumulative';
 $string['timesplitting:nosplitting'] = 'No time splitting';