MDL-57791 analytics: Second review round

moodle · Jul 24, 2017 · 1cc2b4b · 1cc2b4b
1 parent 413f19b
commit 1cc2b4b
Show file tree

Hide file tree

Showing 38 changed files with 204 additions and 278 deletions.
diff --git a/admin/settings/analytics.php b/admin/settings/analytics.php
@@ -76,6 +76,11 @@
 
         // Predictions processor output dir.
         $defaultmodeloutputdir = rtrim($CFG->dataroot, '/') . DIRECTORY_SEPARATOR . 'models';
+        if (empty(get_config('analytics', 'modeloutputdir')) && !file_exists($defaultmodeloutputdir) &&
+                is_writable($defaultmodeloutputdir)) {
+            // Automatically create the dir for them so users don't see the invalid value red cross.
+            mkdir($defaultmodeloutputdir, $CFG->directorypermissions, true);
+        }
         $settings->add(new admin_setting_configdirectory('analytics/modeloutputdir', new lang_string('modeloutputdir', 'analytics'),
             new lang_string('modeloutputdirinfo', 'analytics'), $defaultmodeloutputdir));
     }

diff --git a/admin/tool/models/amd/src/log_info.js b/admin/tool/models/amd/src/log_info.js
@@ -1,20 +1,35 @@
-/*
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Shows a dialogue with info about this logs.
+ *
+ * @module     tool_models/log_info
+ * @class      log_info
  * @package    tool_models
  * @copyright  2017 David Monllao {@link http://www.davidmonllao.com}
  * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
-
- /**
-  * @module tool_models/log_info
-  */
 define(['jquery', 'core/str', 'core/modal_factory', 'core/notification'], function($, str, ModalFactory, Notification) {
 
-    return {
+    return /** @alias module:tool_models/log_info */ {
 
         /**
          * Prepares a modal info for a log's results.
          *
-         * @access public
+         * @method loadInfo
          * @param {int} id
          * @param {string[]} info
          */

diff --git a/admin/tool/models/classes/analytics/target/course_dropout.php b/admin/tool/models/classes/analytics/target/course_dropout.php
@@ -135,6 +135,11 @@ public function is_valid_analysable(\core_analytics\analysable $course, $fortrai
             return get_string('nocoursesections', 'tool_models');
         }
 
+        if ($course->get_start() == 0) {
+            // We require time start to be set.
+            return get_string('nocoursestarttime', 'tool_models');
+        }
+
         if ($course->get_end() == 0) {
             // We require time end to be set.
             return get_string('nocourseendtime', 'tool_models');

diff --git a/admin/tool/models/classes/analytics/target/no_teaching.php b/admin/tool/models/classes/analytics/target/no_teaching.php
@@ -61,7 +61,6 @@ public static function get_name() {
      * @return \core_analytics\prediction_action[]
      */
     public function prediction_actions(\core_analytics\prediction $prediction, $includedetailsaction = false) {
-        global $USER;
 
         // No need to call the parent as the parent's action is view details and this target only have 1 feature.
         $actions = array();

diff --git a/admin/tool/models/classes/output/model_logs.php b/admin/tool/models/classes/output/model_logs.php
@@ -78,7 +78,7 @@ public function __construct($uniqueid, $model) {
     /**
      * Generate the version column.
      *
-     * @param stdClass $log log data.
+     * @param \stdClass $log log data.
      * @return string HTML for the version column
      */
     public function col_version($log) {
@@ -89,7 +89,7 @@ public function col_version($log) {
     /**
      * Generate the time column.
      *
-     * @param stdClass $log log data.
+     * @param \stdClass $log log data.
      * @return string HTML for the time column
      */
     public function col_time($log) {
@@ -100,7 +100,7 @@ public function col_time($log) {
     /**
      * Generate the indicators column.
      *
-     * @param stdClass $log log data.
+     * @param \stdClass $log log data.
      * @return string HTML for the indicators column
      */
     public function col_indicators($log) {
@@ -120,7 +120,7 @@ public function col_indicators($log) {
     /**
      * Generate the context column.
      *
-     * @param stdClass $log log data.
+     * @param \stdClass $log log data.
      * @return string HTML for the context column
      */
     public function col_timesplitting($log) {
@@ -131,7 +131,7 @@ public function col_timesplitting($log) {
     /**
      * Generate the accuracy column.
      *
-     * @param stdClass $log log data.
+     * @param \stdClass $log log data.
      * @return string HTML for the accuracy column
      */
     public function col_accuracy($log) {
@@ -141,7 +141,7 @@ public function col_accuracy($log) {
     /**
      * Generate the info column.
      *
-     * @param stdClass $log log data.
+     * @param \stdClass $log log data.
      * @return string HTML for the score column
      */
     public function col_info($log) {
@@ -165,7 +165,7 @@ public function col_info($log) {
     /**
      * Generate the usermodified column.
      *
-     * @param stdClass $log log data.
+     * @param \stdClass $log log data.
      * @return string HTML for the usermodified column
      */
     public function col_usermodified($log) {

diff --git a/admin/tool/models/classes/output/renderer.php b/admin/tool/models/classes/output/renderer.php
@@ -42,11 +42,11 @@ class renderer extends plugin_renderer_base {
     /**
      * Defer to template.
      *
-     * @param templatable $renderable
+     * @param templatable $templatable
      * @return string HTML
      */
-    protected function render_models_list(templatable $renderable) {
-        $data = $renderable->export_for_template($this);
+    protected function render_models_list(templatable $templatable) {
+        $data = $templatable->export_for_template($this);
         return parent::render_from_template('tool_models/models_list', $data);
     }
 
@@ -140,13 +140,13 @@ public function render_evaluate_results($results, $logs = array()) {
     /**
      * Web interface training & prediction results.
      *
-     * @param array $trainresults
+     * @param \stdClass|false $trainresults
      * @param string[] $trainlogs
-     * @param array $predictresults
+     * @param \stdClass|false $predictresults
      * @param string[] $predictlogs
      * @return string HTML
      */
-    public function render_getpredictions_results($trainresults = false, $trainlogs = array(), $predictresults = false, $predictlogs = array()) {
+    public function render_get_predictions_results($trainresults = false, $trainlogs = array(), $predictresults = false, $predictlogs = array()) {
         global $OUTPUT;
 
         $output = '';
@@ -163,7 +163,7 @@ public function render_getpredictions_results($trainresults = false, $trainlogs
                 $output .= $OUTPUT->notification(get_string('nodatatotrain', 'tool_models'),
                     \core\output\notification::NOTIFY_WARNING);
             } else {
-                $output .= $OUTPUT->notification(get_string('generalerror', 'analytics', $result->status),
+                $output .= $OUTPUT->notification(get_string('generalerror', 'analytics', $trainresults->status),
                     \core\output\notification::NOTIFY_ERROR);
             }
         }
@@ -187,7 +187,7 @@ public function render_getpredictions_results($trainresults = false, $trainlogs
                 $output .= $OUTPUT->notification(get_string('nodatatopredict', 'tool_models'),
                     \core\output\notification::NOTIFY_WARNING);
             } else {
-                $output .= $OUTPUT->notification(get_string('generalerror', 'analytics', $result->status),
+                $output .= $OUTPUT->notification(get_string('generalerror', 'analytics', $predictresults->status),
                     \core\output\notification::NOTIFY_ERROR);
             }
         }

diff --git a/admin/tool/models/classes/task/predict_models.php b/admin/tool/models/classes/task/predict_models.php
@@ -63,7 +63,7 @@ public function execute() {
             if ($result) {
                 echo $OUTPUT->heading(get_string('modelresults', 'tool_models', $model->get_target()->get_name()));
                 $renderer = $PAGE->get_renderer('tool_models');
-                echo $renderer->render_getpredictions_results(false, array(), $result, $model->get_analyser()->get_logs());
+                echo $renderer->render_get_predictions_results(false, array(), $result, $model->get_analyser()->get_logs());
             }
         }
 

diff --git a/admin/tool/models/classes/task/train_models.php b/admin/tool/models/classes/task/train_models.php
@@ -75,7 +75,7 @@ public function execute() {
                 echo $OUTPUT->heading(get_string('modelresults', 'tool_models', $model->get_target()->get_name()));
 
                 $renderer = $PAGE->get_renderer('tool_models');
-                echo $renderer->render_getpredictions_results($result, $model->get_analyser()->get_logs());
+                echo $renderer->render_get_predictions_results($result, $model->get_analyser()->get_logs());
             }
         }
     }

diff --git a/admin/tool/models/cli/evaluate_model.php b/admin/tool/models/cli/evaluate_model.php
@@ -107,7 +107,7 @@
     // Select a dataset, train and enable the model.
     $input = cli_input(get_string('clienablemodel', 'tool_models'));
     while (!\core_analytics\manager::is_valid($input, '\core_analytics\local\time_splitting\base') && $input !== 'none') {
-        mtrace(get_string('errorunexistingtimesplitting', 'tool_models'));
+        mtrace(get_string('errorunexistingtimesplitting', 'analytics'));
         $input = cli_input(get_string('clienablemodel', 'tool_models'));
     }
 

diff --git a/admin/tool/models/cli/guess_course_start_and_end.php b/admin/tool/models/cli/guess_course_start_and_end.php
@@ -199,9 +199,9 @@ function tool_models_calculate_course_dates($course, $options) {
 
     }
 
-    echo mtrace($notification);
+    mtrace($notification);
 }
 
-echo mtrace(get_string('success'));
+mtrace(get_string('success'));
 
 exit(0);
diff --git a/admin/tool/models/db/install.php b/admin/tool/models/db/install.php
@@ -33,14 +33,62 @@ function xmldb_tool_models_install() {
 
     // Students at risk of dropping out of courses.
     $target = \core_analytics\manager::get_target('\tool_models\analytics\target\course_dropout');
+
+    // Community of inquiry indicators.
+    $indicators = array(
+        '\mod_assign\analytics\indicator\cognitive_depth',
+        '\mod_assign\analytics\indicator\social_breadth',
+        '\mod_book\analytics\indicator\cognitive_depth',
+        '\mod_book\analytics\indicator\social_breadth',
+        '\mod_chat\analytics\indicator\cognitive_depth',
+        '\mod_chat\analytics\indicator\social_breadth',
+        '\mod_choice\analytics\indicator\cognitive_depth',
+        '\mod_choice\analytics\indicator\social_breadth',
+        '\mod_data\analytics\indicator\cognitive_depth',
+        '\mod_data\analytics\indicator\social_breadth',
+        '\mod_feedback\analytics\indicator\cognitive_depth',
+        '\mod_feedback\analytics\indicator\social_breadth',
+        '\mod_folder\analytics\indicator\cognitive_depth',
+        '\mod_folder\analytics\indicator\social_breadth',
+        '\mod_forum\analytics\indicator\cognitive_depth',
+        '\mod_forum\analytics\indicator\social_breadth',
+        '\mod_glossary\analytics\indicator\cognitive_depth',
+        '\mod_glossary\analytics\indicator\social_breadth',
+        '\mod_imscp\analytics\indicator\cognitive_depth',
+        '\mod_imscp\analytics\indicator\social_breadth',
+        '\mod_label\analytics\indicator\cognitive_depth',
+        '\mod_label\analytics\indicator\social_breadth',
+        '\mod_lesson\analytics\indicator\cognitive_depth',
+        '\mod_lesson\analytics\indicator\social_breadth',
+        '\mod_lti\analytics\indicator\cognitive_depth',
+        '\mod_lti\analytics\indicator\social_breadth',
+        '\mod_page\analytics\indicator\cognitive_depth',
+        '\mod_page\analytics\indicator\social_breadth',
+        '\mod_quiz\analytics\indicator\cognitive_depth',
+        '\mod_quiz\analytics\indicator\social_breadth',
+        '\mod_resource\analytics\indicator\cognitive_depth',
+        '\mod_resource\analytics\indicator\social_breadth',
+        '\mod_scorm\analytics\indicator\cognitive_depth',
+        '\mod_scorm\analytics\indicator\social_breadth',
+        '\mod_survey\analytics\indicator\cognitive_depth',
+        '\mod_survey\analytics\indicator\social_breadth',
+        '\mod_url\analytics\indicator\cognitive_depth',
+        '\mod_url\analytics\indicator\social_breadth',
+        '\mod_wiki\analytics\indicator\cognitive_depth',
+        '\mod_wiki\analytics\indicator\social_breadth',
+        '\mod_workshop\analytics\indicator\cognitive_depth',
+        '\mod_workshop\analytics\indicator\social_breadth',
+    );
+    array_walk($indicators, function(&$indicator) {
+        $indicator = \core_analytics\manager::get_indicator($indicator);
+    });
+
     // We need the model to be created in order to know all its potential indicators and set them.
     $model = \core_analytics\model::create($target, array());
-    // TODO All of them for the moment, we will define a limited set of them once in core.
-    $model->update(0, $model->get_potential_indicators());
 
     // Course without teachers.
     $target = \core_analytics\manager::get_target('\tool_models\analytics\target\no_teaching');
-    $weekbeforestart = '\core_analytics\local\time_splitting\single_range';
+    $timesplittingmethod = '\core_analytics\local\time_splitting\single_range';
     $noteacher = \core_analytics\manager::get_indicator('\core_course\analytics\indicator\no_teacher');
-    \core_analytics\model::create($target, array($noteacher->get_id() => $noteacher), $weekbeforestart);
+    \core_analytics\model::create($target, array($noteacher->get_id() => $noteacher), $timesplittingmethod);
 }
diff --git a/admin/tool/models/index.php b/admin/tool/models/index.php
@@ -31,7 +31,7 @@
 
 echo $OUTPUT->header();
 
-$renderable = new \tool_models\output\models_list($models);
-echo $PAGE->get_renderer('tool_models')->render($renderable);
+$templatable = new \tool_models\output\models_list($models);
+echo $PAGE->get_renderer('tool_models')->render($templatable);
 
 echo $OUTPUT->footer();
diff --git a/admin/tool/models/lang/en/tool_models.php b/admin/tool/models/lang/en/tool_models.php
@@ -23,7 +23,6 @@
  */
 
 $string['accuracy'] = 'Accuracy';
-$string['allindicators'] = 'All indicators';
 $string['allpredictions'] = 'All predictions';
 $string['analysingsitedata'] = 'Analysing the site';
 $string['analyticmodels'] = 'Analytic models';
@@ -60,11 +59,9 @@
 $string['labelteachingyes'] = 'Users with teaching capabilities have access to the course';
 $string['labelteachingno'] = 'No teaching';
 $string['loginfo'] = 'Log extra info';
-$string['lowaccuracy'] = 'The model accuracy is low';
 $string['modelresults'] = '{$a} results';
 $string['modelslist'] = 'Models list';
 $string['modeltimesplitting'] = 'Time splitting';
-$string['nocompletiondetection'] = 'No method available to detect course completion (no completion nor competencies nor course grade pass)';
 $string['nocourseactivity'] = 'Not enough course activity between the start and the end of the course';
 $string['nocourseendtime'] = 'The course does not have an end time';
 $string['nocoursesections'] = 'No course sections';
@@ -84,8 +81,6 @@
 $string['target'] = 'Target';
 $string['target:coursedropout'] = 'Students at risk of dropping out';
 $string['target:noteachingactivity'] = 'No teaching';
-$string['target:coursedropoutinfo'] = 'Here you can find a list of students at risk of dropping out.';
-$string['timemodified'] = 'Last modification';
 $string['trainingprocessfinished'] = 'Training process finished';
 $string['trainingresults'] = 'Training results';
 $string['trainmodels'] = 'Train models';

diff --git a/admin/tool/models/model.php b/admin/tool/models/model.php
@@ -52,7 +52,7 @@
         $title = get_string('viewlog', 'tool_models');
         break;
     default:
-        throw new moodle_exception('errorunknownaction', 'tool_models');
+        throw new moodle_exception('errorunknownaction', 'analytics');
 }
 
 $PAGE->set_context($context);
@@ -133,7 +133,7 @@
         $predictlogs = $model->get_analyser()->get_logs();
 
         $renderer = $PAGE->get_renderer('tool_models');
-        echo $renderer->render_getpredictions_results($trainresults, $trainlogs, $predictresults, $predictlogs);
+        echo $renderer->render_get_predictions_results($trainresults, $trainlogs, $predictresults, $predictlogs);
         break;
 
     case 'log':

diff --git a/admin/tool/models/templates/models_list.mustache b/admin/tool/models/templates/models_list.mustache
@@ -43,7 +43,10 @@
                 "timesplitting": "Quarters",
                 "noinsights": "No insights available yet"
             }
-        ]
+        ],
+        "warnings": {
+            "message": "Hey, this is a warning"
+        }
     }
 }}
 

diff --git a/analytics/classes/calculable.php b/analytics/classes/calculable.php
@@ -135,7 +135,7 @@ public function get_calculation_outcome($value, $subtype = false) {
      *
      * @param string $elementname
      * @param int $sampleid
-     * @return \stdClass
+     * @return \stdClass|false An \stdClass object or false if it can not be found.
      */
     protected function retrieve($elementname, $sampleid) {
         if (empty($this->sampledata[$sampleid]) || empty($this->sampledata[$sampleid][$elementname])) {
@@ -193,9 +193,10 @@ protected function limit_value($calculatedvalue) {
      * - gt as 'greater than'
      * - ge as 'greater or equal than'
      *
+     * @throws \coding_exception
      * @param int|float $value
      * @param array $ranges e.g. [ ['lt', 20], ['ge', 20] ]
-     * @return void
+     * @return float
      */
     protected function classify_value($value, $ranges) {
 

diff --git a/analytics/classes/course.php b/analytics/classes/course.php
@@ -196,7 +196,6 @@ public function get_context() {
      * @return int Timestamp or 0 if has not started yet.
      */
     public function get_start() {
-        global $DB;
 
         if ($this->starttime !== null) {
             return $this->starttime;

diff --git a/analytics/classes/dataset_manager.php b/analytics/classes/dataset_manager.php
@@ -195,7 +195,6 @@ public static function get_previous_evaluation_file($modelid, $timesplittingid)
      * @return bool
      */
     public static function delete_previous_evaluation_file($modelid, $timesplittingid) {
-        $fs = get_file_storage();
         if ($file = self::get_previous_evaluation_file($modelid, $timesplittingid)) {
             $file->delete();
             return true;