MDL-59153 report_insights: UI improvements

- Accessible info tables - Outcome icons - Navigation bar for missing contexts - Heading contents standarisation
moodle · Sep 1, 2017 · 1d5b1d0 · 1d5b1d0
1 parent 3e0f33a
commit 1d5b1d0
Show file tree

Hide file tree

Showing 15 changed files with 576 additions and 79 deletions.
diff --git a/lang/en/analytics.php b/lang/en/analytics.php
@@ -54,7 +54,6 @@
 $string['errorunknownaction'] = 'Unknown action';
 $string['eventpredictionactionstarted'] = 'Prediction action started';
 $string['insightmessagesubject'] = 'New insight for "{$a->contextname}": {$a->insightname}';
-$string['insightinfo'] = '{$a->insightname} - {$a->contextname}';
 $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';

diff --git a/report/insights/classes/output/insight.php b/report/insights/classes/output/insight.php
@@ -73,20 +73,22 @@ public function __construct(\core_analytics\prediction $prediction, \core_analyt
     public function export_for_template(\renderer_base $output) {
 
         $data = new \stdClass();
+        $data->insightname = format_string($this->model->get_target()->get_name());
 
         // Sample info (determined by the analyser).
         list($data->sampledescription, $samplerenderable) = $this->model->prediction_sample_description($this->prediction);
 
         // Sampleimage is a renderable we should pass it to HTML.
         if ($samplerenderable) {
-            $data->samplelink = $output->render($samplerenderable);
+            $data->sampleimage = $output->render($samplerenderable);
         }
 
         // Prediction info.
         $predictedvalue = $this->prediction->get_prediction_data()->prediction;
         $predictionid = $this->prediction->get_prediction_data()->id;
         $data->predictiondisplayvalue = $this->model->get_target()->get_display_value($predictedvalue);
-        $data->predictionstyle = $this->get_calculation_style($this->model->get_target(), $predictedvalue);
+        list($data->style, $data->outcomeicon) = $this->get_calculation_display($this->model->get_target(), $predictedvalue,
+            $output);
 
         $actions = $this->model->get_target()->prediction_actions($this->prediction, $this->includedetailsaction);
         if ($actions) {
@@ -122,46 +124,66 @@ public function export_for_template(\renderer_base $output) {
             $obj = new \stdClass();
             $obj->name = call_user_func(array($calculation->indicator, 'get_name'));
             $obj->displayvalue = $calculation->indicator->get_display_value($calculation->value, $calculation->subtype);
-            $obj->style = $this->get_calculation_style($calculation->indicator, $calculation->value, $calculation->subtype);
+            list($obj->style, $obj->outcomeicon) = $this->get_calculation_display($calculation->indicator, $calculation->value,
+                $output, $calculation->subtype);
 
             $data->calculations[] = $obj;
         }
 
+        if (empty($data->calculations)) {
+            $data->nocalculations = (object)array(
+                'message' => get_string('nodetailsavailable', 'report_insights'),
+                'closebutton' => false
+            );
+        }
+
         return $data;
     }
 
     /**
-     * Returns a CSS class from the calculated value outcome.
+     * Returns display info for the calculated value outcome.
      *
      * @param \core_analytics\calculable $calculable
      * @param float $value
+     * @param \renderer_base $output
      * @param string|false $subtype
-     * @return string
+     * @return array The style as 'success', 'info', 'warning' or 'danger' and pix_icon
      */
-    protected function get_calculation_style(\core_analytics\calculable $calculable, $value, $subtype = false) {
+    protected function get_calculation_display(\core_analytics\calculable $calculable, $value, $output, $subtype = false) {
         $outcome = $calculable->get_calculation_outcome($value, $subtype);
         switch ($outcome) {
             case \core_analytics\calculable::OUTCOME_NEUTRAL:
                 $style = '';
+                $text = get_string('outcomeneutral', 'report_insights');
+                $icon = 't/check';
                 break;
             case \core_analytics\calculable::OUTCOME_VERY_POSITIVE:
-                $style = 'alert alert-success';
+                $style = 'success';
+                $text = get_string('outcomeverypositive', 'report_insights');
+                $icon = 't/approve';
                 break;
             case \core_analytics\calculable::OUTCOME_OK:
-                $style = 'alert alert-info';
+                $style = 'info';
+                $text = get_string('outcomeok', 'report_insights');
+                $icon = 't/check';
                 break;
             case \core_analytics\calculable::OUTCOME_NEGATIVE:
-                $style = 'alert alert-warning';
+                $style = 'warning';
+                $text = get_string('outcomenegative', 'report_insights');
+                $icon = 'i/warning';
                 break;
             case \core_analytics\calculable::OUTCOME_VERY_NEGATIVE:
-                $style = 'alert alert-danger';
+                $style = 'danger';
+                $text = get_string('outcomeverynegative', 'report_insights');
+                $icon = 'i/warning';
                 break;
             default:
                 throw new \coding_exception('The outcome returned by ' . get_class($calculable) . '::get_calculation_outcome is ' .
                     'not one of the accepted values. Please use \core_analytics\calculable::OUTCOME_VERY_POSITIVE, ' .
                     '\core_analytics\calculable::OUTCOME_OK, \core_analytics\calculable::OUTCOME_NEGATIVE, ' .
                     '\core_analytics\calculable::OUTCOME_VERY_NEGATIVE or \core_analytics\calculable::OUTCOME_NEUTRAL');
         }
-        return $style;
+        $icon = new \pix_icon($icon, $text);
+        return array($style, $icon->export_for_template($output));
     }
 }
diff --git a/report/insights/classes/output/insights_list.php b/report/insights/classes/output/insights_list.php
@@ -88,6 +88,8 @@ public function export_for_template(\renderer_base $output) {
         global $PAGE;
 
         $data = new \stdClass();
+        $data->insightname = format_string($this->model->get_target()->get_name());
+
         $total = 0;
 
         if ($this->model->uses_insights()) {

diff --git a/report/insights/insights.php b/report/insights/insights.php
@@ -23,6 +23,7 @@
  */
 
 require_once(__DIR__ . '/../../config.php');
+require_once($CFG->libdir . '/adminlib.php');
 
 $contextid = required_param('contextid', PARAM_INT);
 $modelid = optional_param('modelid', false, PARAM_INT);
@@ -52,15 +53,32 @@
     unset($othermodels[$modelid]);
 }
 
+// The URL in navigation only contains the contextid.
 $params = array('contextid' => $contextid);
-$url = new \moodle_url('/report/insights/insights.php', $params);
+$navurl = new \moodle_url('/report/insights/insights.php', $params);
+
+// This is the real page url, we need it to include the modelid so pagination and
+// other stuff works as expected.
+$url = clone $navurl;
 if ($modelid) {
     $url->param('modelid', $modelid);
 }
 
 $PAGE->set_url($url);
 $PAGE->set_pagelayout('report');
 
+if ($context->contextlevel === CONTEXT_SYSTEM) {
+    admin_externalpage_setup('reportinsights', '', null, '', array('pagelayout' => 'report'));
+} else if ($context->contextlevel === CONTEXT_USER) {
+    $user = \core_user::get_user($context->instanceid, '*', MUST_EXIST);
+    $PAGE->navigation->extend_for_user($user);
+    $PAGE->add_report_nodes($user->id, array(
+        'name' => get_string('insights', 'report_insights'),
+        'url' => $url
+    ));
+}
+$PAGE->navigation->override_active_url($navurl);
+
 $renderer = $PAGE->get_renderer('report_insights');
 
 // No models with insights available at this context level.
@@ -74,7 +92,6 @@
 $insightinfo = new stdClass();
 $insightinfo->contextname = $context->get_context_name();
 $insightinfo->insightname = $model->get_target()->get_name();
-$title = get_string('insightinfo', 'analytics', $insightinfo);
 
 if (!$model->is_enabled()) {
     echo $renderer->render_model_disabled($insightinfo);
@@ -86,8 +103,8 @@
     exit(0);
 }
 
-$PAGE->set_title($title);
-$PAGE->set_heading($title);
+$PAGE->set_title($insightinfo->insightname);
+$PAGE->set_heading($insightinfo->contextname);
 
 echo $OUTPUT->header();
 

diff --git a/report/insights/lang/en/report_insights.php b/report/insights/lang/en/report_insights.php
@@ -22,10 +22,22 @@
  * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
  */
 
-
+$string['calculatedvalue'] = 'Calculated value';
 $string['disabledmodel'] = 'Sorry, this model has been disabled by the administrator';
+$string['indicator'] = 'Indicator';
+$string['insightprediction'] = '{$a} prediction';
+$string['insight'] = 'Insight';
 $string['insights'] = 'Insights';
+$string['outcome'] = 'Outcome';
+$string['outcomenegative'] = 'Negative outcome';
+$string['outcomeneutral'] = 'Neutral outcome';
+$string['outcomeok'] = 'Ok outcome';
+$string['outcomepositive'] = 'Positive outcome';
+$string['outcomeverypositive'] = 'Very positive outcome';
+$string['outcomeverynegative'] = 'Very negative outcome';
 $string['pluginname'] = 'Insights';
 $string['prediction'] = 'Prediction';
+$string['predictioncalculations'] = 'Indicator calculations';
 $string['predictiondetails'] = 'Prediction details';
+$string['nodetailsavailable'] = 'No prediction details are relevant.';
 $string['selectotherinsights'] = 'Select other insights...';
diff --git a/report/insights/lib.php b/report/insights/lib.php
@@ -36,20 +36,85 @@ function report_insights_extend_navigation_course($navigation, $course, $context
 
     if (has_capability('moodle/analytics:listinsights', $context)) {
 
-        $cache = \cache::make('core', 'contextwithinsights');
-        $modelids = $cache->get($context->id);
-        if ($modelids === false) {
-            // They will be full unless a model has been cleared.
-            $models = \core_analytics\manager::get_models_with_insights($context);
-            $modelids = array_keys($models);
-            $cache->set($context->id, $modelids);
+        $modelids = report_insights_context_insights($context);
+        if (!empty($modelids)) {
+            $url = new moodle_url('/report/insights/insights.php', array('contextid' => $context->id));
+            $node = navigation_node::create(get_string('insights', 'report_insights'), $url, navigation_node::TYPE_SETTING,
+                null, null, new pix_icon('i/report', get_string('insights', 'report_insights')));
+            $navigation->add_node($node);
         }
+    }
+}
+
+/**
+ * Add nodes to myprofile page.
+ *
+ * @param \core_user\output\myprofile\tree $tree Tree object
+ * @param stdClass $user user object
+ * @param bool $iscurrentuser
+ * @param stdClass $course Course object
+ *
+ * @return bool
+ */
+function report_insights_myprofile_navigation(core_user\output\myprofile\tree $tree, $user, $iscurrentuser, $course) {
 
+    $context = \context_user::instance($user->id);
+    if (has_capability('moodle/analytics:listinsights', $context)) {
+
+        $modelids = report_insights_context_insights($context);
         if (!empty($modelids)) {
             $url = new moodle_url('/report/insights/insights.php', array('contextid' => $context->id));
-            $settingsnode = navigation_node::create(get_string('insights', 'report_insights'), $url, navigation_node::TYPE_SETTING,
-                null, null, new pix_icon('i/settings', ''));
-            $navigation->add_node($settingsnode);
+            $node = new core_user\output\myprofile\node('reports', 'insights', get_string('insights', 'report_insights'),
+                null, $url);
+            $tree->add_node($node);
         }
     }
 }
+
+/**
+ * Adds nodes to category navigation
+ *
+ * @param navigation_node $navigation The navigation node to extend
+ * @param context $context The context of the course
+ * @return void|null return null if we don't want to display the node.
+ */
+function report_insights_extend_navigation_category_settings($navigation, $context) {
+
+    if (has_capability('moodle/analytics:listinsights', $context)) {
+
+        $modelids = report_insights_context_insights($context);
+        if (!empty($modelids)) {
+            $url = new moodle_url('/report/insights/insights.php', array('contextid' => $context->id));
+
+            $node = navigation_node::create(
+                get_string('insights', 'report_insights'),
+                $url,
+                navigation_node::NODETYPE_LEAF,
+                null,
+                'insights',
+                new pix_icon('i/report', get_string('insights', 'report_insights'))
+            );
+
+            $navigation->add_node($node);
+        }
+    }
+}
+
+/**
+ * Returns the models that generated insights in the provided context.
+ *
+ * @param \context $context
+ * @return int[]
+ */
+function report_insights_context_insights(\context $context) {
+
+    $cache = \cache::make('core', 'contextwithinsights');
+    $modelids = $cache->get($context->id);
+    if ($modelids === false) {
+        // They will be full unless a model has been cleared.
+        $models = \core_analytics\manager::get_models_with_insights($context);
+        $modelids = array_keys($models);
+        $cache->set($context->id, $modelids);
+    }
+    return $modelids;
+}
diff --git a/report/insights/prediction.php b/report/insights/prediction.php
@@ -23,6 +23,7 @@
  */
 
 require_once(__DIR__ . '/../../config.php');
+require_once($CFG->libdir . '/adminlib.php');
 
 $predictionid = required_param('id', PARAM_INT);
 
@@ -37,12 +38,27 @@
 $PAGE->set_url($url);
 $PAGE->set_pagelayout('report');
 
+$navurl = new \moodle_url('/report/insights/insights.php', array('contextid' => $context->id));
+if ($context->contextlevel === CONTEXT_SYSTEM) {
+    admin_externalpage_setup('reportinsights', '', null, '', array('pagelayout' => 'report'));
+} else if ($context->contextlevel === CONTEXT_USER) {
+    $user = \core_user::get_user($context->instanceid, '*', MUST_EXIST);
+    $PAGE->navigation->extend_for_user($user);
+
+    $modelinsightsurl = clone $navurl;
+    $modelinsightsurl->param('modelid', $model->get_id());
+    $PAGE->add_report_nodes($user->id, array(
+        'name' => get_string('insights', 'report_insights'),
+        'url' => $url
+    ));
+}
+$PAGE->navigation->override_active_url($navurl);
+
 $renderer = $PAGE->get_renderer('report_insights');
 
 $insightinfo = new stdClass();
 $insightinfo->contextname = $context->get_context_name();
 $insightinfo->insightname = $model->get_target()->get_name();
-$title = get_string('insightinfo', 'analytics', $insightinfo);
 
 $modelready = $model->is_enabled() && $model->is_trained() && $model->predictions_exist($context);
 if (!$modelready) {
@@ -55,8 +71,8 @@
     exit(0);
 }
 
-$PAGE->set_title($title);
-$PAGE->set_heading($title);
+$PAGE->set_title($insightinfo->insightname);
+$PAGE->set_heading($insightinfo->contextname);
 
 echo $OUTPUT->header();
 

diff --git a/report/insights/settings.php b/report/insights/settings.php
@@ -0,0 +1,34 @@
+<?php
+// 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/>.
+
+/**
+ * Links and settings
+ *
+ * Contains settings used by insights report.
+ *
+ * @package    report_insights
+ * @copyright  2017 David Monllao {@link http://www.davidmonllao.com}
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die;
+
+// Just a link to course report.
+$ADMIN->add('reports', new admin_externalpage('reportinsights', get_string('insights', 'report_insights'),
+        $CFG->wwwroot . "/report/insights/insights.php?contextid=" . SYSCONTEXTID, 'moodle/analytics:listinsights'));
+
+// No report settings.
+$settings = null;