MDL-57791 analytics: Fixes during integration review

This commit includes the following changes:
- cibot complains fixes
- removed randomly failing test
- fixed course_dropout return
- other minor fixes

admin/tool/models/amd/src/log_info.js

@@ -39,9 +39,9 @@ define(['jquery', 'core/str', 'core/modal_factory', 'core/notification'], functi
             str.get_string('loginfo', 'tool_models').then(function(langString) {
 
                 var bodyInfo = $("<ul>");
-                for (var i in info) {
-                    bodyInfo.append("<li>" + info[i] + "</li>");
-                }
+                info.forEach(function(item) {
+                    bodyInfo.append('<li>' + item + '</li>');
+                });
                 bodyInfo.append("</ul>");
 
                 return ModalFactory.create({

admin/tool/models/classes/output/renderer.php

@@ -42,7 +42,7 @@ class renderer extends plugin_renderer_base {
     /**
      * Defer to template.
      *
-     * @param \tool_models\output\models_list $templatable
+     * @param \tool_models\output\models_list $modelslist
      * @return string HTML
      */
     protected function render_models_list(\tool_models\output\models_list $modelslist) {

analytics/classes/calculable.php

@@ -126,9 +126,7 @@ public function get_display_value($value, $subtype = false) {
      * @param string|false $subtype
      * @return int
      */
-    public function get_calculation_outcome($value, $subtype = false) {
-        throw new \coding_exception('Please overwrite get_calculation_outcome method');
-    }
+    abstract public function get_calculation_outcome($value, $subtype = false);
 
     /**
      * Retrieve the specified element associated to $sampleid.

analytics/classes/local/target/base.php

@@ -181,7 +181,7 @@ public function generate_insight_notifications($modelid, $samplecontexts) {
 
                 $message->fullmessage = get_string('insightinfomessage', 'analytics', $insighturl->out());
                 $message->fullmessageformat = FORMAT_PLAIN;
-                $message->fullmessagehtml = get_string('insightinfomessage', 'analytics', $insighturl->out());
+                $message->fullmessagehtml = get_string('insightinfomessagehtml', 'analytics', $insighturl->out());
                 $message->smallmessage = get_string('insightinfomessage', 'analytics', $insighturl->out());
                 $message->contexturl = $insighturl->out(false);
 

analytics/classes/manager.php

@@ -100,7 +100,7 @@ public static function get_all_models($enabled = false, $trained = false, $predi
                 $conditions[] = 'am.trained = :trained';
                 $params['trained'] = 1;
             }
-            $sql .= ' WHERE ' . implode(' AND ', $conditions);
+            $sql .= ' WHERE ' . implode(' AND ', $conditions) . ' ORDER BY am.timemodified DESC';
         }
         $modelobjs = $DB->get_records_sql($sql, $params);
 
@@ -390,7 +390,7 @@ public static function get_prediction($predictionid, $requirelogin = false) {
      */
     public static function add_builtin_models() {
 
-        $target = \core_analytics\manager::get_target('\core\analytics\target\course_dropout');
+        $target = self::get_target('\core\analytics\target\course_dropout');
 
         // Community of inquiry indicators.
         $coiindicators = array(
@@ -439,17 +439,17 @@ public static function add_builtin_models() {
         );
         $indicators = array();
         foreach ($coiindicators as $coiindicator) {
-            $indicator = \core_analytics\manager::get_indicator($coiindicator);
+            $indicator = self::get_indicator($coiindicator);
             $indicators[$indicator->get_id()] = $indicator;
         }
         if (!\core_analytics\model::exists($target, $indicators)) {
             $model = \core_analytics\model::create($target, $indicators);
         }
 
         // No teaching model.
-        $target = \core_analytics\manager::get_target('\core\analytics\target\no_teaching');
+        $target = self::get_target('\core\analytics\target\no_teaching');
         $timesplittingmethod = '\core\analytics\time_splitting\single_range';
-        $noteacher = \core_analytics\manager::get_indicator('\core_course\analytics\indicator\no_teacher');
+        $noteacher = self::get_indicator('\core_course\analytics\indicator\no_teacher');
         $indicators = array($noteacher->get_id() => $noteacher);
         if (!\core_analytics\model::exists($target, $indicators)) {
             \core_analytics\model::create($target, $indicators, $timesplittingmethod);

analytics/classes/model.php

@@ -1066,7 +1066,6 @@ public function get_predictions(\context $context, $page = false, $perpage = 100
 
         list($unused, $samplesdata) = $this->get_analyser()->get_samples($sampleids);
 
-
         $current = 0;
 
         if ($page !== false) {

analytics/tests/fixtures/test_static_target_shortname.php

@@ -22,10 +22,10 @@
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-require_once(__DIR__ . '/test_target_shortname.php');
-
 defined('MOODLE_INTERNAL') || die();
 
+require_once(__DIR__ . '/test_target_shortname.php');
+
 /**
  * Test static target.
  *

analytics/tests/prediction_test.php

@@ -280,19 +280,7 @@ public function test_ml_evaluation($modelquality, $ncourses, $expected, $predict
      */
     public function provider_ml_test_evaluation() {
 
-        $notenoughandlowscore = \core_analytics\model::EVALUATE_NOT_ENOUGH_DATA + \core_analytics\model::EVALUATE_LOW_SCORE;
         $cases = array(
-            'bad-and-no-enough-data' => array(
-                'modelquality' => 'random',
-                'ncourses' => 5,
-                'expectedresults' => array(
-                    // The course duration is too much to be processed by in weekly basis.
-                    '\core\analytics\time_splitting\weekly' => \core_analytics\model::NO_DATASET,
-                    // 10 samples is not enough to process anything.
-                    '\core\analytics\time_splitting\single_range' => $notenoughandlowscore,
-                    '\core\analytics\time_splitting\quarters' => $notenoughandlowscore,
-                )
-            ),
             'bad' => array(
                 'modelquality' => 'random',
                 'ncourses' => 50,

lang/en/analytics.php

@@ -53,7 +53,8 @@
 $string['eventpredictionactionstarted'] = 'Prediction action started';
 $string['insightmessagesubject'] = 'New insight for "{$a->contextname}": {$a->insightname}';
 $string['insightinfo'] = '{$a->insightname} - {$a->contextname}';
-$string['insightinfomessage'] = 'There are some insights you may find useful. Check out {$a}';
+$string['insightinfomessage'] = 'The system generated some insights for you: {$a}';
+$string['insightinfomessagehtml'] = 'The system generated some insights for you: <a href="{$a}">{$a}</a>.';
 $string['invalidtimesplitting'] = 'Model with id {$a} needs a time splitting method before it can be used to train';
 $string['invalidanalysablefortimesplitting'] = 'It can not be analysed using {$a} time splitting method';
 $string['modeloutputdir'] = 'Models output directory';

lib/classes/analytics/target/course_dropout.php

@@ -198,7 +198,7 @@ public function is_valid_sample($sampleid, \core_analytics\analysable $course, $
      * @param \core_analytics\analysable $course
      * @param int $starttime
      * @param int $endtime
-     * @return float
+     * @return float 0 -> not at risk, 1 -> at risk
      */
     protected function calculate_sample($sampleid, \core_analytics\analysable $course, $starttime = false, $endtime = false) {
 
@@ -226,8 +226,8 @@ protected function calculate_sample($sampleid, \core_analytics\analysable $cours
         $params = array('userid' => $userenrol->userid, 'courseid' => $course->get_id(), 'limit' => $limit);
         $nlogs = $logstore->get_events_select_count($select, $params);
         if ($nlogs == 0) {
-            return 0;
+            return 1;
         }
-        return 1;
+        return 0;
     }
 }

lib/tests/indicators_test.php

@@ -18,7 +18,7 @@
  * Unit tests for core indicators.
  *
  * @package   core
- * @category  phpunit
+ * @category  analytics
  * @copyright 2017 David Monllaó {@link http://www.davidmonllao.com}
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -33,7 +33,7 @@
  * Unit tests for core indicators.
  *
  * @package   core
- * @category  phpunit
+ * @category  analytics
  * @copyright 2017 David Monllaó {@link http://www.davidmonllao.com}
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */

lib/tests/time_splittings_test.php

@@ -18,7 +18,7 @@
  * Unit tests for core time splitting methods.
  *
  * @package   core
- * @category  phpunit
+ * @category  analytics
  * @copyright 2017 David Monllaó {@link http://www.davidmonllao.com}
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
@@ -32,7 +32,7 @@
  * Unit tests for core time splitting methods.
  *
  * @package   core
- * @category  phpunit
+ * @category  analytics
  * @copyright 2017 David Monllaó {@link http://www.davidmonllao.com}
  * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */