MDL-64783 analytics: Activities due insight forwards to calendar

The patch includes changes applied after the peer review.
This commit is contained in:
David Monllaó
2019-04-09 00:29:57 +02:00
committed by Eloy Lafuente (stronk7)
parent 93663fa1a8
commit 982fef46f4
24 changed files with 119 additions and 78 deletions
+27 -22
View File
@@ -164,12 +164,6 @@ class analysis {
*/
public function process_analysable(\core_analytics\analysable $analysable): array {
$options = $this->analyser->get_options();
// Default returns.
$files = array();
$message = null;
// Target instances scope is per-analysable (it can't be lower as calculations run once per
// analysable, not time splitting method nor time range).
$target = call_user_func(array($this->analyser->get_target(), 'instance'));
@@ -192,7 +186,7 @@ class analysis {
$cachedresult = $this->result->retrieve_cached_result($timesplitting, $analysable);
if ($cachedresult) {
$result = new \stdClass();
$result->result = $previousanalysis;
$result->result = $cachedresult;
$results[$timesplitting->get_id()] = $result;
continue;
}
@@ -351,8 +345,7 @@ class analysis {
}
// We need to pass all the analysis data.
$formattedresult = $this->result->format_result($data, $target, $timesplitting, $analysable,
$this->analyser->get_modelid(), $this->includetarget, $options);
$formattedresult = $this->result->format_result($data, $target, $timesplitting, $analysable);
} catch (\Throwable $e) {
$this->finish_analysable_analysis();
@@ -381,7 +374,7 @@ class analysis {
* @param array $sampleids
* @param array $ranges
* @param \core_analytics\local\target\base $target
* @return array|bool
* @return array|null
*/
public function calculate(\core_analytics\local\time_splitting\base $timesplitting, array &$sampleids,
array $ranges, \core_analytics\local\target\base $target): ?array {
@@ -389,7 +382,7 @@ class analysis {
$calculatedtarget = null;
if ($this->includetarget) {
// We first calculate the target because analysable data may still be invalid or none
// of the analysable samples may be valid ($sampleids is also passed by reference).
// of the analysable samples may be valid.
$calculatedtarget = $target->calculate($sampleids, $timesplitting->get_analysable());
// We remove samples we can not calculate their target.
@@ -403,13 +396,13 @@ class analysis {
// No need to continue calculating if the target couldn't be calculated for any sample.
if (empty($sampleids)) {
return false;
return null;
}
$dataset = $this->calculate_indicators($timesplitting, $sampleids, $ranges);
if (empty($dataset)) {
return false;
return null;
}
// Now that we have the indicators in place we can add the time range indicators (and target if provided) to each of them.
@@ -552,7 +545,7 @@ class analysis {
*
* @param \core_analytics\local\time_splitting\base $timesplitting
* @param array $dataset
* @param ?array $calculatedtarget
* @param array|null $calculatedtarget
* @return null
*/
protected function fill_dataset(\core_analytics\local\time_splitting\base $timesplitting,
@@ -731,7 +724,7 @@ class analysis {
if (!$predictedrange) {
// Nothing to filter out.
return;
return null;
}
$predictedrange->sampleids = json_decode($predictedrange->sampleids, true);
@@ -739,7 +732,7 @@ class analysis {
if (count($missingsamples) === 0) {
// All samples already calculated.
unset($ranges[$rangeindex]);
return;
return null;
}
// Replace the list of samples by the one excluding samples that already got predictions at this range.
@@ -748,6 +741,13 @@ class analysis {
return $predictedrange;
}
/**
* Returns a predict samples record.
*
* @param \core_analytics\local\time_splitting\base $timesplitting
* @param int $rangeindex
* @return \stdClass|false
*/
private function get_predict_samples_record(\core_analytics\local\time_splitting\base $timesplitting, int $rangeindex) {
global $DB;
@@ -785,11 +785,11 @@ class analysis {
* @param int[] $sampleids
* @param array $ranges
* @param \core_analytics\local\time_splitting\base $timesplitting
* @param ?\stdClass $predictsamplesrecord The existing record or null if there is no record yet.
* @param \stdClass|null $predictsamplesrecord The existing record or null if there is no record yet.
* @return null
*/
protected function save_prediction_samples(array $sampleids, array $ranges,
\core_analytics\local\time_splitting\base $timesplitting, ?\stdClass $predictsamplesrecord) {
\core_analytics\local\time_splitting\base $timesplitting, ?\stdClass $predictsamplesrecord = null) {
global $DB;
if (count($ranges) > 1) {
@@ -804,8 +804,11 @@ class analysis {
$predictsamplesrecord->timemodified = time();
$DB->update_record('analytics_predict_samples', $predictsamplesrecord);
} else {
$predictsamplesrecord = (object)['modelid' => $this->analyser->get_modelid(), 'analysableid' => $timesplitting->get_analysable()->get_id(),
'timesplitting' => $timesplitting->get_id(), 'rangeindex' => $rangeindex];
$predictsamplesrecord = (object)[
'modelid' => $this->analyser->get_modelid(),
'analysableid' => $timesplitting->get_analysable()->get_id(),
'timesplitting' => $timesplitting->get_id(), 'rangeindex' => $rangeindex
];
$predictsamplesrecord->sampleids = json_encode($sampleids);
$predictsamplesrecord->timecreated = time();
$predictsamplesrecord->timemodified = $predictsamplesrecord->timecreated;
@@ -870,12 +873,14 @@ class analysis {
private static function get_insert_batch_size(): int {
global $DB;
$dbconfig = $DB->export_dbconfig();
// 500 is pgsql default so using 1000 is fine, no other db driver uses a hardcoded value.
if (empty($DB->dboptions['bulkinsertsize'])) {
if (empty($dbconfig) || empty($dbconfig->dboptions) || empty($dbconfig->dboptions['bulkinsertsize'])) {
return 1000;
}
$bulkinsert = $DB->dboptions['bulkinsertsize'];
$bulkinsert = $dbconfig->dboptions['bulkinsertsize'];
if ($bulkinsert < 1000) {
return $bulkinsert;
}
+2 -2
View File
@@ -133,7 +133,7 @@ class course implements \core_analytics\analysable {
* through this constructor will not be cached.
*
* @param int|\stdClass $course Course id or mdl_course record
* @param ?\context $context
* @param \context|null $context
* @return void
*/
public function __construct($course, ?\context $context = null) {
@@ -156,7 +156,7 @@ class course implements \core_analytics\analysable {
* Lazy load of course data, students and teachers.
*
* @param int|\stdClass $course Course object or course id
* @param ?\context $context
* @param \context|null $context
* @return \core_analytics\course
*/
public static function instance($course, ?\context $context = null) {
+6 -4
View File
@@ -66,8 +66,8 @@ class insights_generator {
/**
* Generates insight notifications.
*
* @param array $samplecontexts The contexts these predictions belong to
* @param \core_analytics\prediction $predictions The prediction records
* @param array $samplecontexts The contexts these predictions belong to
* @param \core_analytics\prediction[] $predictions The prediction records
* @return null
*/
public function generate($samplecontexts, $predictions) {
@@ -89,7 +89,8 @@ class insights_generator {
$insighturl = $this->target->get_insight_context_url($this->modelid, $context);
$fullmessage = get_string('insightinfomessage', 'analytics', $insighturl->out(false));
$fullmessagehtml = $OUTPUT->render_from_template('core_analytics/insight_info_message', ['url' => $insighturl->out(false)]);
$fullmessagehtml = $OUTPUT->render_from_template('core_analytics/insight_info_message',
['url' => $insighturl->out(false)]);
$this->notifications($context, $insighturl, $fullmessage, $fullmessagehtml);
}
}
@@ -189,7 +190,8 @@ class insights_generator {
$messageactions[] = $actiondata;
}
$fullmessagehtml = $OUTPUT->render_from_template('core_analytics/insight_info_message_prediction', ['actions' => $messageactions]);
$fullmessagehtml = $OUTPUT->render_from_template('core_analytics/insight_info_message_prediction',
['actions' => $messageactions]);
return [$insighturl, $fullmessageplaintext, $fullmessagehtml];
}
}
+7 -3
View File
@@ -110,11 +110,15 @@ abstract class base {
*
* \core_analytics\local\analyser\by_course and \core_analytics\local\analyser\sitewide are implementing
* this method returning site courses (by_course) and the whole system (sitewide) as analysables.
*
* @todo MDL-65284 This will be removed in Moodle 4.1
* @deprecated
* @see get_analysables_iterator
* @throws \coding_exception
* @return \core_analytics\analysable[] Array of analysable elements using the analysable id as array key.
*/
public function get_analysables() {
// This function should only be called from get_analysables_iterator and we keep it here until php 4.1
// This function should only be called from get_analysables_iterator and we keep it here until Moodle 4.1
// for backwards compatibility.
throw new \coding_exception('This method is deprecated in favour of get_analysables_iterator.');
}
@@ -126,7 +130,7 @@ abstract class base {
* have already been processed and the order in which they have been processed. Helper methods are available
* to ease to implementation of get_analysables_iterator: get_iterator_sql and order_sql.
*
* @param ?string $action 'prediction', 'training' or null if no specific action needed.
* @param string|null $action 'prediction', 'training' or null if no specific action needed.
* @return \Iterator
*/
public function get_analysables_iterator(?string $action = null) {
@@ -402,7 +406,7 @@ abstract class base {
}
/**
* Get the sql of a default implementaion of the iterator.
* Get the sql of a default implementation of the iterator.
*
* This method only works for analysers that return analysable elements which ids map to a context instance ids.
*
@@ -38,7 +38,7 @@ abstract class by_course extends base {
/**
* Return the list of courses to analyse.
*
* @param ?string $action 'prediction', 'training' or null if no specific action needed.
* @param string|null $action 'prediction', 'training' or null if no specific action needed.
* @return \Iterator
*/
public function get_analysables_iterator(?string $action = null) {
@@ -65,7 +65,7 @@ abstract class by_course extends base {
if (!$recordset->valid()) {
$this->add_log(get_string('nocourses', 'analytics'));
return [];
return new \ArrayIterator([]);
}
return new \core\dml\recordset_walk($recordset, function($record) {
@@ -36,9 +36,9 @@ defined('MOODLE_INTERNAL') || die();
abstract class sitewide extends base {
/**
* Return the list of courses to analyse.
* Return the list of analysables to analyse.
*
* @param ?string $action 'prediction', 'training' or null if no specific action needed.
* @param string|null $action 'prediction', 'training' or null if no specific action needed.
* @return \Iterator
*/
public function get_analysables_iterator(?string $action = null) {
+27 -1
View File
@@ -33,7 +33,7 @@ defined('MOODLE_INTERNAL') || die();
* @copyright 2019 David Monllao {@link http://www.davidmonllao.com}
* @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
*/
class result {
abstract class result {
/**
* @var int
@@ -73,4 +73,30 @@ class result {
\core_analytics\analysable $analysable) {
return false;
}
/**
* Stores the analysis results.
*
* @param array $results
* @return bool True if anything was successfully analysed
*/
abstract public function add_analysable_results(array $results): bool;
/**
* Formats the result.
*
* @param array $data
* @param \core_analytics\local\target\base $target
* @param \core_analytics\local\time_splitting\base $timesplitting
* @param \core_analytics\analysable $analysable
* @return mixed It can be in whatever format the result uses
*/
abstract public function format_result(array $data, \core_analytics\local\target\base $target,
\core_analytics\local\time_splitting\base $timesplitting, \core_analytics\analysable $analysable);
/**
* Returns the results of the analysis.
* @return array
*/
abstract public function get(): array;
}
@@ -73,14 +73,10 @@ class result_array extends result {
* @param \core_analytics\local\target\base $target
* @param \core_analytics\local\time_splitting\base $timesplitting
* @param \core_analytics\analysable $analysable
* @param int $modelid
* @param bool $includetarget
* @param array $options
* @return mixed A \stored_file in this case
* @return mixed The data as it comes
*/
public function format_result(array $data, \core_analytics\local\target\base $target,
\core_analytics\local\time_splitting\base $timesplitting, \core_analytics\analysable $analysable,
int $modelid, bool $includetarget, array $options) {
\core_analytics\local\time_splitting\base $timesplitting, \core_analytics\analysable $analysable) {
return $data;
}
@@ -100,22 +100,18 @@ class result_file extends result {
* @param \core_analytics\local\target\base $target
* @param \core_analytics\local\time_splitting\base $timesplitting
* @param \core_analytics\analysable $analysable
* @param int $modelid
* @param bool $includetarget
* @param array $options
* @return mixed A \stored_file in this case
*/
public function format_result(array $data, \core_analytics\local\target\base $target,
\core_analytics\local\time_splitting\base $timesplitting, \core_analytics\analysable $analysable,
int $modelid, bool $includetarget, array $options) {
\core_analytics\local\time_splitting\base $timesplitting, \core_analytics\analysable $analysable) {
if (!empty($includetarget)) {
if (!empty($this->includetarget)) {
$filearea = \core_analytics\dataset_manager::LABELLED_FILEAREA;
} else {
$filearea = \core_analytics\dataset_manager::UNLABELLED_FILEAREA;
}
$dataset = new \core_analytics\dataset_manager($modelid, $analysable->get_id(),
$timesplitting->get_id(), $filearea, $options['evaluation']);
$dataset = new \core_analytics\dataset_manager($this->modelid, $analysable->get_id(),
$timesplitting->get_id(), $filearea, $this->options['evaluation']);
// Add extra metadata.
$this->add_model_metadata($data, $timesplitting, $target);
+3 -1
View File
@@ -122,9 +122,11 @@ abstract class base extends \core_analytics\calculable {
*
* @param \core_analytics\prediction $prediction
* @param bool $includedetailsaction
* @param bool $isinsightuser
* @return \core_analytics\prediction_action[]
*/
public function prediction_actions(\core_analytics\prediction $prediction, $includedetailsaction = false, $isinsightuser = false) {
public function prediction_actions(\core_analytics\prediction $prediction, $includedetailsaction = false,
$isinsightuser = false) {
global $PAGE;
$predictionid = $prediction->get_prediction_data()->id;
@@ -43,7 +43,7 @@ abstract class periodic extends base {
abstract protected function periodicity();
/**
* Returns whether the course can be processed by this time splitting method or not.
* Returns whether the analysable can be processed by this time splitting method or not.
*
* @param \core_analytics\analysable $analysable
* @return bool
@@ -83,7 +83,7 @@ abstract class periodic extends base {
$nextrange = $this->get_next_range($next);
if ($this->ready_to_predict($nextrange) && (empty($end) || $next < $end)) {
// Add the next one if we we have not reached the analysable end yet.
// Add the next one if we have not reached the analysable end yet.
// It will be used to get predictions.
$ranges[] = $nextrange;
}
-1
View File
@@ -954,7 +954,6 @@ class model {
* Get predictions from a static model.
*
* @param array $indicatorcalculations
* @param string[] $headers
* @return \stdClass[]
*/
protected function get_static_predictions(&$indicatorcalculations) {
+2 -2
View File
@@ -71,7 +71,7 @@ class user implements \core_analytics\analysable {
* through this constructor will not be cached.
*
* @param int|\stdClass $user User id
* @param ?\context $context
* @param \context|null $context
* @return void
*/
public function __construct($user, ?\context $context = null) {
@@ -94,7 +94,7 @@ class user implements \core_analytics\analysable {
* Lazy load of analysable data.
*
* @param int|\stdClass $user User object or user id
* @param ?\context $context
* @param \context|null $context
* @return \core_analytics\user
*/
public static function instance($user, ?\context $context = null) {