From f9e7447f4292942685fc99b37283f0bb8f5532cc Mon Sep 17 00:00:00 2001 From: David Monllao Date: Fri, 9 Jun 2017 17:05:29 +0200 Subject: [PATCH] MDL-57791 insights: Clarify insights-prediction boundaries --- .../classes/analytics/target/no_teaching.php | 5 ++ .../models/classes/output/models_list.php | 52 +++++++++++++------ admin/tool/models/lang/en/tool_models.php | 3 +- .../models/templates/models_list.mustache | 13 ++--- analytics/classes/course.php | 2 - analytics/classes/local/target/base.php | 11 ++++ analytics/classes/model.php | 28 ++++++++++ lang/en/analytics.php | 3 ++ .../output/{prediction.php => insight.php} | 6 +-- ...predictions_list.php => insights_list.php} | 34 ++++++++---- report/insights/classes/output/renderer.php | 38 ++++++++++---- report/insights/insights.php | 14 +++-- report/insights/lang/en/report_insights.php | 2 +- report/insights/prediction.php | 9 +++- .../{prediction.mustache => insight.mustache} | 4 +- ...ails.mustache => insight_details.mustache} | 4 +- ...s_list.mustache => insights_list.mustache} | 18 +++---- 17 files changed, 177 insertions(+), 69 deletions(-) rename report/insights/classes/output/{prediction.php => insight.php} (97%) rename report/insights/classes/output/{predictions_list.php => insights_list.php} (68%) rename report/insights/templates/{prediction.mustache => insight.mustache} (95%) rename report/insights/templates/{prediction_details.mustache => insight_details.mustache} (93%) rename report/insights/templates/{predictions_list.mustache => insights_list.mustache} (78%) diff --git a/admin/tool/models/classes/analytics/target/no_teaching.php b/admin/tool/models/classes/analytics/target/no_teaching.php index 3e862e6bac4..88536e2ae4d 100644 --- a/admin/tool/models/classes/analytics/target/no_teaching.php +++ b/admin/tool/models/classes/analytics/target/no_teaching.php @@ -57,6 +57,11 @@ class no_teaching extends \core_analytics\local\target\binary { $sampledata = $prediction->get_sample_data(); $course = $sampledata['course']; + $url = new \moodle_url('/course/view.php', array('id' => $course->id)); + $pix = new \pix_icon('i/course', get_string('enrolledusers', 'enrol')); + $actions['viewcourse'] = new \core_analytics\prediction_action('viewcourse', $prediction, + $url, $pix, get_string('view')); + if (has_capability('moodle/course:enrolreview', $sampledata['context'])) { $url = new \moodle_url('/enrol/users.php', array('id' => $course->id)); $pix = new \pix_icon('i/enrolusers', get_string('enrolledusers', 'enrol')); diff --git a/admin/tool/models/classes/output/models_list.php b/admin/tool/models/classes/output/models_list.php index 28ec1ae1e35..338a0c7f43c 100644 --- a/admin/tool/models/classes/output/models_list.php +++ b/admin/tool/models/classes/output/models_list.php @@ -56,26 +56,46 @@ class models_list implements \renderable, \templatable { $modeldata = $model->export(); // Model predictions list. - $predictioncontexts = $model->get_predictions_contexts(); - if ($predictioncontexts) { + if ($model->uses_insights()) { + $predictioncontexts = $model->get_predictions_contexts(); + if ($predictioncontexts) { - foreach ($predictioncontexts as $contextid => $unused) { - // We prepare this to be used as single_select template options. - $context = \context::instance_by_id($contextid); - if (empty($context)) { - // The context may have been deleted. - unset($predictioncontexts[$contextid]); - continue; + foreach ($predictioncontexts as $contextid => $unused) { + // We prepare this to be used as single_select template options. + $context = \context::instance_by_id($contextid); + if (empty($context)) { + // The context may have been deleted. + unset($predictioncontexts[$contextid]); + continue; + } + + // Special name for system level predictions as showing "System is not visually nice". + if ($contextid == SYSCONTEXTID) { + $contextname = get_string('allpredictions', 'tool_models'); + } else { + $contextname = shorten_text($context->get_context_name(true, true), 90); + } + $predictioncontexts[$contextid] = $contextname; } - $predictioncontexts[$contextid] = shorten_text($context->get_context_name(true, true), 90); - } - \core_collator::asort($predictioncontexts); + \core_collator::asort($predictioncontexts); - if (!empty($predictioncontexts)) { - $url = new \moodle_url('/report/insights/insights.php', array('modelid' => $model->get_id())); - $singleselect = new \single_select($url, 'contextid', $predictioncontexts); - $modeldata->predictions = $singleselect->export_for_template($output); + if (!empty($predictioncontexts)) { + $url = new \moodle_url('/report/insights/insights.php', array('modelid' => $model->get_id())); + $singleselect = new \single_select($url, 'contextid', $predictioncontexts); + $modeldata->insights = $singleselect->export_for_template($output); + } } + + if (empty($modeldata->insights)) { + if ($model->any_prediction_obtained()) { + $modeldata->noinsights = get_string('noinsights', 'analytics'); + } else { + $modeldata->noinsights = get_string('nopredictionsyet', 'analytics'); + } + } + + } else { + $modeldata->noinsights = get_string('noinsightsmodel', 'analytics'); } // Actions. diff --git a/admin/tool/models/lang/en/tool_models.php b/admin/tool/models/lang/en/tool_models.php index 7bb47afce61..0015d4f5c13 100644 --- a/admin/tool/models/lang/en/tool_models.php +++ b/admin/tool/models/lang/en/tool_models.php @@ -24,6 +24,7 @@ $string['accuracy'] = 'Accuracy'; $string['allindicators'] = 'All indicators'; +$string['allpredictions'] = 'All predictions'; $string['analysingsitedata'] = 'Analysing the site'; $string['analyticmodels'] = 'Analytic models'; $string['bettercli'] = 'To evaluate models and to get predictions are heavy processes, it is better to run them through command line interface'; @@ -53,6 +54,7 @@ $string['getpredictions'] = 'Get predictions'; $string['goodmodel'] = 'This is a good model and it can be used to predict, enable it to start getting predictions.'; $string['indicators'] = 'Indicators'; $string['info'] = 'Info'; +$string['insights'] = 'Insights'; $string['labelstudentdropoutyes'] = 'Student at risk of dropping out'; $string['labelstudentdropoutno'] = 'Not at risk'; $string['labelteachingyes'] = 'Users with teaching capabilities have access to the course'; @@ -88,6 +90,5 @@ $string['trainingprocessfinished'] = 'Training process finished'; $string['trainingresults'] = 'Training results'; $string['trainmodels'] = 'Train models'; $string['viewlog'] = 'Log'; -$string['viewpredictions'] = 'View model predictions'; $string['weeksenddateautomaticallyset'] = 'End date automatically set based on start date and the number of sections'; $string['weeksenddatedefault'] = 'End date would be automatically calculated from the course start date'; diff --git a/admin/tool/models/templates/models_list.mustache b/admin/tool/models/templates/models_list.mustache index 6ab51f9eec9..6c825272a66 100644 --- a/admin/tool/models/templates/models_list.mustache +++ b/admin/tool/models/templates/models_list.mustache @@ -45,7 +45,7 @@ {{#str}}enabled, tool_models{{/str}} {{#str}}indicators, tool_models{{/str}} {{#str}}modeltimesplitting, tool_models{{/str}} - {{#str}}viewpredictions, tool_models{{/str}} + {{#str}}insights, tool_models{{/str}} {{#str}}actions{{/str}} @@ -72,12 +72,13 @@ {{#timesplitting}}{{timesplitting}}{{/timesplitting}}{{^timesplitting}}{{#str}}notdefined, tool_models{{/str}}{{/timesplitting}} - {{#predictions}} + {{! models_list renderer is responsible of sending one or the other}} + {{#insights}} {{> core/single_select }} - {{/predictions}} - {{^predictions}} - {{#str}}nopredictionsyet, analytics{{/str}} - {{/predictions}} + {{/insights}} + {{#noinsights}} + {{.}} + {{/noinsights}} {{#actions}} diff --git a/analytics/classes/course.php b/analytics/classes/course.php index 3cb2fb143ea..1ffa28cf6d4 100644 --- a/analytics/classes/course.php +++ b/analytics/classes/course.php @@ -36,8 +36,6 @@ require_once($CFG->dirroot . '/lib/gradelib.php'); */ class course implements \core_analytics\analysable { - const MIN_STUDENT_LOGS_PERCENT = 90; - protected static $instances = array(); protected $studentroles = []; diff --git a/analytics/classes/local/target/base.php b/analytics/classes/local/target/base.php index f99aef22316..e31fad869c5 100644 --- a/analytics/classes/local/target/base.php +++ b/analytics/classes/local/target/base.php @@ -83,6 +83,17 @@ abstract class base extends \core_analytics\calculable { */ abstract protected function calculate_sample($sampleid, \core_analytics\analysable $analysable, $starttime = false, $endtime = false); + /** + * Is this target generating insights? + * + * Defaults to true. + * + * @return bool + */ + public static function uses_insights() { + return true; + } + /** * Based on facts (processed by machine learning backends) by default. * diff --git a/analytics/classes/model.php b/analytics/classes/model.php index 082c3ba5711..56b1d57acdb 100644 --- a/analytics/classes/model.php +++ b/analytics/classes/model.php @@ -89,6 +89,9 @@ class model { if (is_scalar($model)) { $model = $DB->get_record('analytics_models', array('id' => $model)); + if (!$model) { + throw new \moodle_exception('errorunexistingmodel', 'analytics', '', $model); + } } $this->model = $model; } @@ -821,6 +824,31 @@ class model { return $DB->get_records_sql($sql, array($this->model->id)); } + /** + * Has this model generated predictions? + * + * We don't check analytics_predictions table because targets have the ability to + * ignore some predicted values, if that is the case predictions are not even stored + * in db. + * + * @return bool + */ + public function any_prediction_obtained() { + global $DB; + return $DB->record_exists('analytics_predict_ranges', + array('modelid' => $this->model->id, 'timesplitting' => $this->model->timesplitting)); + } + + /** + * Whether this model generates insights or not (defined by the model's target). + * + * @return bool + */ + public function uses_insights() { + $target = $this->get_target(); + return $target::uses_insights(); + } + /** * Whether predictions exist for this context. * diff --git a/lang/en/analytics.php b/lang/en/analytics.php index e4ecddaeb67..7eaf6292228 100644 --- a/lang/en/analytics.php +++ b/lang/en/analytics.php @@ -42,6 +42,7 @@ $string['errorpredictwrongformat'] = 'The predictions processor return can not b $string['errorprocessornotready'] = 'The selected predictions processor is not ready: {$a}'; $string['errorsamplenotavailable'] = 'The predicted sample is not available anymore'; $string['errorunexistingtimesplitting'] = 'The selected time splitting method is not available'; +$string['errorunexistingmodel'] = 'Unexisting model {$a}'; $string['errorunknownaction'] = 'Unknown action'; $string['eventactionclicked'] = 'Prediction action clicked'; $string['indicator:accessesafterend'] = 'Accesses after the end date'; @@ -61,6 +62,8 @@ $string['modeloutputdirinfo'] = 'Directory where prediction processors store all $string['nocourses'] = 'No courses to analyse'; $string['nocoursestart'] = 'No course start'; $string['nodata'] = 'No data available'; +$string['noinsightsmodel'] = 'This model does not generate insights'; +$string['noinsights'] = 'No insights reported'; $string['nonewdata'] = 'No new data available'; $string['nonewtimeranges'] = 'No new time ranges, nothing to predict'; $string['nopredictionsyet'] = 'No predictions available yet'; diff --git a/report/insights/classes/output/prediction.php b/report/insights/classes/output/insight.php similarity index 97% rename from report/insights/classes/output/prediction.php rename to report/insights/classes/output/insight.php index 26ffedabbac..eb0ea15a722 100644 --- a/report/insights/classes/output/prediction.php +++ b/report/insights/classes/output/insight.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * Prediction view page. + * Single insight view page. * * @package report_insights * @copyright 2017 David Monllao {@link http://www.davidmonllao.com} @@ -27,13 +27,13 @@ namespace report_insights\output; defined('MOODLE_INTERNAL') || die(); /** - * Prediction view page. + * Single insight view page. * * @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 */ -class prediction implements \renderable, \templatable { +class insight implements \renderable, \templatable { /** * @var \core_analytics\model diff --git a/report/insights/classes/output/predictions_list.php b/report/insights/classes/output/insights_list.php similarity index 68% rename from report/insights/classes/output/predictions_list.php rename to report/insights/classes/output/insights_list.php index a8d861169ac..3c8a6a665aa 100644 --- a/report/insights/classes/output/predictions_list.php +++ b/report/insights/classes/output/insights_list.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * Predictions list page. + * Insights list page. * * @package report_insights * @copyright 2017 David Monllao {@link http://www.davidmonllao.com} @@ -27,13 +27,13 @@ namespace report_insights\output; defined('MOODLE_INTERNAL') || die(); /** - * Shows report_insights predictions list. + * Shows report_insights insights list. * * @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 */ -class predictions_list implements \renderable, \templatable { +class insights_list implements \renderable, \templatable { /** * @var \core_analytics\model @@ -67,17 +67,29 @@ class predictions_list implements \renderable, \templatable { $data = new \stdClass(); - $predictions = $this->model->get_predictions($this->context); + if ($this->model->uses_insights()) { + $predictions = $this->model->get_predictions($this->context); - $data->predictions = array(); - foreach ($predictions as $prediction) { - $predictionrenderable = new \report_insights\output\prediction($prediction, $this->model); - $data->predictions[] = $predictionrenderable->export_for_template($output); + $data->insights = array(); + foreach ($predictions as $prediction) { + $insightrenderable = new \report_insights\output\insight($prediction, $this->model); + $data->insights[] = $insightrenderable->export_for_template($output); + } + + if (empty($data->insights)) { + if ($this->model->any_prediction_obtained()) { + $data->noinsights = get_string('noinsights', 'analytics'); + } else { + $data->noinsights = get_string('nopredictionsyet', 'analytics'); + } + } + } else { + $data->noinsights = get_string('noinsights', 'analytics'); } - if (empty($data->predictions)) { - $notification = new \core\output\notification(get_string('nopredictionsyet', 'analytics')); - $data->nopredictions = $notification->export_for_template($output); + if (!empty($data->noinsights)) { + $notification = new \core\output\notification($data->noinsights); + $data->noinsights = $notification->export_for_template($output); } if ($this->othermodels) { diff --git a/report/insights/classes/output/renderer.php b/report/insights/classes/output/renderer.php index 2c0e1f6fc2d..219d95a97cf 100644 --- a/report/insights/classes/output/renderer.php +++ b/report/insights/classes/output/renderer.php @@ -40,25 +40,25 @@ use renderable; class renderer extends plugin_renderer_base { /** - * Renders the list of predictions + * Renders the list of insights * * @param renderable $renderable * @return string HTML */ - protected function render_predictions_list(renderable $renderable) { + protected function render_insights_list(renderable $renderable) { $data = $renderable->export_for_template($this); - return parent::render_from_template('report_insights/predictions_list', $data); + return parent::render_from_template('report_insights/insights_list', $data); } /** - * Renders a prediction + * Renders an insight * * @param renderable $renderable * @return string HTML */ - protected function render_prediction(renderable $renderable) { + protected function render_insight(renderable $renderable) { $data = $renderable->export_for_template($this); - return parent::render_from_template('report_insights/prediction_details', $data); + return parent::render_from_template('report_insights/insight_details', $data); } /** @@ -82,12 +82,12 @@ class renderer extends plugin_renderer_base { } /** - * Model without predictions info. + * Model without insights info. * * @param \context $context * @return string HTML */ - public function render_no_predictions(\context $context) { + public function render_no_insights(\context $context) { global $OUTPUT, $PAGE; // We don't want to disclose the name of the model if it has not been enabled. @@ -95,7 +95,27 @@ class renderer extends plugin_renderer_base { $PAGE->set_heading($context->get_context_name()); $output = $OUTPUT->header(); - $output .= $OUTPUT->notification(get_string('nopredictionsyet', 'analytics'), \core\output\notification::NOTIFY_INFO); + $output .= $OUTPUT->notification(get_string('noinsights', 'analytics'), \core\output\notification::NOTIFY_INFO); + $output .= $OUTPUT->footer(); + + return $output; + } + + /** + * Model which target does not generate insights. + * + * @param \context $context + * @return string HTML + */ + public function render_no_insights_model(\context $context) { + global $OUTPUT, $PAGE; + + // We don't want to disclose the name of the model if it has not been enabled. + $PAGE->set_title($context->get_context_name()); + $PAGE->set_heading($context->get_context_name()); + + $output = $OUTPUT->header(); + $output .= $OUTPUT->notification(get_string('noinsightsmodel', 'analytics'), \core\output\notification::NOTIFY_INFO); $output .= $OUTPUT->footer(); return $output; diff --git a/report/insights/insights.php b/report/insights/insights.php index 2c46eb66f27..66303cf467a 100644 --- a/report/insights/insights.php +++ b/report/insights/insights.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * View model predictions. + * View model insights. * * @package report_insights * @copyright 2017 David Monllao {@link http://www.davidmonllao.com} @@ -64,9 +64,9 @@ $PAGE->set_pagelayout('report'); $renderer = $PAGE->get_renderer('report_insights'); -// No models with predictions available at this context level. +// No models with insights available at this context level. if (!$modelid) { - echo $renderer->render_no_predictions($context); + echo $renderer->render_no_insights($context); exit(0); } @@ -77,18 +77,22 @@ $insightinfo->contextname = $context->get_context_name(); $insightinfo->insightname = $model->get_target()->get_name(); $title = get_string('insightinfo', 'analytics', $insightinfo); - if (!$model->is_enabled() && !has_capability('moodle/analytics:managemodels', $context)) { echo $renderer->render_model_disabled($insightinfo); exit(0); } +if (!$model->uses_insights()) { + echo $renderer->render_no_insights_model($context); + exit(0); +} + $PAGE->set_title($title); $PAGE->set_heading($title); echo $OUTPUT->header(); -$renderable = new \report_insights\output\predictions_list($model, $context, $othermodels); +$renderable = new \report_insights\output\insights_list($model, $context, $othermodels); echo $renderer->render($renderable); echo $OUTPUT->footer(); diff --git a/report/insights/lang/en/report_insights.php b/report/insights/lang/en/report_insights.php index bd651a75218..3efc313873c 100644 --- a/report/insights/lang/en/report_insights.php +++ b/report/insights/lang/en/report_insights.php @@ -25,9 +25,9 @@ $string['disabledmodel'] = 'Sorry, this model has been disabled by the administrator'; $string['errorpredictionnotfound'] = 'Prediction not found'; +$string['insight'] = 'Insight'; $string['insights'] = 'Insights'; $string['pluginname'] = 'Insights'; $string['prediction'] = 'Prediction'; $string['predictiondetails'] = 'Prediction details'; -$string['predictions'] = 'Predictions'; $string['selectotherinsights'] = 'Select other insights...'; diff --git a/report/insights/prediction.php b/report/insights/prediction.php index b6707f62a77..a78ab76101d 100644 --- a/report/insights/prediction.php +++ b/report/insights/prediction.php @@ -15,7 +15,7 @@ // along with Moodle. If not, see . /** - * View a prediction. + * View an insight. * * @package report_insights * @copyright 2017 David Monllao {@link http://www.davidmonllao.com} @@ -68,12 +68,17 @@ if (!$modelready && !has_capability('moodle/analytics:managemodels', $context)) exit(0); } +if (!$model->uses_insights()) { + echo $renderer->render_no_insights_model($context); + exit(0); +} + $PAGE->set_title($title); $PAGE->set_heading($title); echo $OUTPUT->header(); -$renderable = new \report_insights\output\prediction($prediction, $model); +$renderable = new \report_insights\output\insight($prediction, $model); echo $renderer->render($renderable); echo $OUTPUT->footer(); diff --git a/report/insights/templates/prediction.mustache b/report/insights/templates/insight.mustache similarity index 95% rename from report/insights/templates/prediction.mustache rename to report/insights/templates/insight.mustache index 895c9aea0c5..b24505eed4b 100644 --- a/report/insights/templates/prediction.mustache +++ b/report/insights/templates/insight.mustache @@ -15,9 +15,9 @@ along with Moodle. If not, see . }} {{! - @template report_insights/prediction + @template report_insights/insight - Template for a prediction. + Template for a insight. Classes required for JS: * none diff --git a/report/insights/templates/prediction_details.mustache b/report/insights/templates/insight_details.mustache similarity index 93% rename from report/insights/templates/prediction_details.mustache rename to report/insights/templates/insight_details.mustache index 9bcd97b34e8..77cd57cd3f3 100644 --- a/report/insights/templates/prediction_details.mustache +++ b/report/insights/templates/insight_details.mustache @@ -15,7 +15,7 @@ along with Moodle. If not, see . }} {{! - @template report_insights/prediction_details + @template report_insights/insight_details Actions panel at the bottom of the assignment grading UI. @@ -30,7 +30,7 @@ }}

{{#str}}prediction, report_insights{{/str}}

-{{> report_insights/prediction}} +{{> report_insights/insight}}

{{#str}} predictiondetails, report_insights {{/str}}

diff --git a/report/insights/templates/predictions_list.mustache b/report/insights/templates/insights_list.mustache similarity index 78% rename from report/insights/templates/predictions_list.mustache rename to report/insights/templates/insights_list.mustache index 1ab370f331e..93c0c837969 100644 --- a/report/insights/templates/predictions_list.mustache +++ b/report/insights/templates/insights_list.mustache @@ -15,9 +15,9 @@ along with Moodle. If not, see . }} {{! - @template report_insights/predictions_list + @template report_insights/insights_list - Template for the predictions list. + Template for the insights list. Classes required for JS: * none @@ -39,14 +39,14 @@
{{/modelselector}} -

{{#str}} predictions, report_insights {{/str}}

-
- {{#predictions}} - {{> report_insights/prediction}} - {{/predictions}} +

{{#str}} insights, report_insights {{/str}}

+
+ {{#insights}} + {{> report_insights/insight}} + {{/insights}}
-{{#nopredictions}} +{{#noinsights}}
{{> core/notification_info}}
-{{/nopredictions}} +{{/noinsights}}