From c679d39c369eaa3df795c6172e84d7d49d84ea04 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 3756b907b73..6230b758698 100644 --- a/analytics/classes/model.php +++ b/analytics/classes/model.php @@ -1626,7 +1626,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 19ceb6166a0..2fc0fa97c06 100644 --- a/analytics/tests/model_test.php +++ b/analytics/tests/model_test.php @@ -169,10 +169,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 b29cfc582fb2d9b5c9e234bfa11e75de57ee1540 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 f37da21bbbc..d227891cbb8 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -2917,5 +2917,11 @@ function xmldb_main_upgrade($oldversion) { upgrade_main_savepoint(true, 2019031500.01); } + if ($oldversion < 2019032200.01) { + // 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, 2019032200.01); + } + return true; } diff --git a/version.php b/version.php index 6db178530a8..d9a50febb07 100644 --- a/version.php +++ b/version.php @@ -29,7 +29,7 @@ defined('MOODLE_INTERNAL') || die(); -$version = 2019032200.00; // YYYYMMDD = weekly release date of this DEV branch. +$version = 2019032200.01; // YYYYMMDD = weekly release date of this DEV branch. // RR = release increments - 00 in DEV branches. // .XX = incremental changes.