From 38ce5978dd14695da8f02a55c957487340a5a807 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Tue, 5 Mar 2019 19:57:52 +0100 Subject: [PATCH 1/2] MDL-64996 analytics: Don't mark static model as untrained after clearing Static predictions models (i.e. those using a target based on assumptions, not facts) are always considered as trained. Clearing them must not mark them as untrained. Doing so would make them being skipped by the prediction scheduled task. --- analytics/classes/model.php | 5 ++++- analytics/tests/model_test.php | 24 ++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/analytics/classes/model.php b/analytics/classes/model.php index fa6e91eae88..02116a72ce6 100644 --- a/analytics/classes/model.php +++ b/analytics/classes/model.php @@ -1524,7 +1524,10 @@ class model { // 1 db read per context. $this->purge_insights_cache(); - $this->model->trained = 0; + if (!$this->is_static()) { + $this->model->trained = 0; + } + $this->model->timemodified = time(); $this->model->usermodified = $USER->id; $DB->update_record('analytics_models', $this->model); diff --git a/analytics/tests/model_test.php b/analytics/tests/model_test.php index 21009bace9b..00c43cde756 100644 --- a/analytics/tests/model_test.php +++ b/analytics/tests/model_test.php @@ -168,10 +168,34 @@ class analytics_model_testcase extends advanced_testcase { $this->assertEmpty($DB->count_records('analytics_predict_samples')); $this->assertEmpty($DB->count_records('analytics_used_files')); + // Check that the model is marked as not trained after clearing (as it is not a static one). + $this->assertEquals(0, $DB->get_field('analytics_models', 'trained', array('id' => $this->modelobj->id))); + set_config('enabled_stores', '', 'tool_log'); get_log_manager(true); } + /** + * Test behaviour of {\core_analytics\model::clear()} for static models. + */ + public function test_clear_static() { + global $DB; + $this->resetAfterTest(); + + $statictarget = new test_static_target_shortname(); + $indicators['test_indicator_max'] = \core_analytics\manager::get_indicator('test_indicator_max'); + $model = \core_analytics\model::create($statictarget, $indicators, '\core\analytics\time_splitting\quarters'); + $modelobj = $model->get_model_obj(); + + // Static models are always considered trained. + $this->assertEquals(1, $DB->get_field('analytics_models', 'trained', array('id' => $modelobj->id))); + + $model->clear(); + + // Check that the model is still marked as trained even after clearing. + $this->assertEquals(1, $DB->get_field('analytics_models', 'trained', array('id' => $modelobj->id))); + } + public function test_model_manager() { $this->resetAfterTest(true); From 6b1ce978946dc1c978ae7db646fad0d394aa8507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Mudr=C3=A1k?= Date: Tue, 5 Mar 2019 20:01:28 +0100 Subject: [PATCH 2/2] MDL-64996 analytics: Make sure the no_teaching model is trained Due to the bug, the no_teaching model might have been marked as not-trained in the database. Static predictions models (i.e. those using a target based on assumptions, not facts) are always considered as trained. If they were marked as not-trained, the prediction scheduled task would skip them and they would produce no predictions. Ideally, such a fix should be done for all static models. But there is no easy way to do it during the upgrade where accessing the analytics API is not possible. I don't think there are many models out there that would be affected by this so this seems to be good enough solution for now (and the future ability to reset models will cover the rest). --- lib/db/upgrade.php | 6 ++++++ version.php | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/db/upgrade.php b/lib/db/upgrade.php index c91de50dfb4..1e27470a16d 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -3050,5 +3050,11 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2018120303.02); } + if ($oldversion < 2018120303.05) { + // The no_teaching model might have been marked as not-trained by mistake (static models are always trained). + $DB->set_field('analytics_models', 'trained', 1, ['target' => '\core\analytics\target\no_teaching']); + upgrade_main_savepoint(true, 2018120303.05); + } + return true; } diff --git a/version.php b/version.php index d5efb8ec96d..7bf6234f16d 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2018120303.04; // 20181203 = branching date YYYYMMDD - do not modify! +$version = 2018120303.05; // 20181203 = branching date YYYYMMDD - do not modify! // RR = release increments - 00 in DEV branches. // .XX = incremental changes.