MDL-65633 tool_analytics: Notification for invalid analysis intervals

admin/tool/analytics/classes/output/form/edit_model.php

@@ -65,6 +65,15 @@ public function definition() {
             $mform->addRule('target', get_string('required'), 'required', null, 'client');
         }
 
+        if (!empty($this->_customdata['targetname']) && !empty($this->_customdata['targetclass'])) {
+            $mform->addElement('static', 'targetname', get_string('target', 'tool_analytics'), $this->_customdata['targetname']);
+            $mform->addElement('hidden', 'target',
+                \tool_analytics\output\helper::class_to_option($this->_customdata['targetclass']));
+            // We won't update the model's target so no worries about its format (we can't use PARAM_ALPHANUMEXT
+            // because of class_to_option).
+            $mform->setType('target', PARAM_TEXT);
+        }
+
         // Indicators.
         if (!$this->_customdata['staticmodel']) {
             $indicators = array();
@@ -81,6 +90,13 @@ public function definition() {
         }
 
         // Time-splitting methods.
+        if (!empty($this->_customdata['invalidcurrenttimesplitting'])) {
+            $mform->addElement('html', $OUTPUT->notification(
+                get_string('invalidcurrenttimesplitting', 'tool_analytics'),
+                \core\output\notification::NOTIFY_WARNING)
+            );
+        }
+
         $timesplittings = array('' => '');
         foreach ($this->_customdata['timesplittings'] as $classname => $timesplitting) {
             $optionname = \tool_analytics\output\helper::class_to_option($classname);
@@ -131,10 +147,17 @@ public function validation($data, $files) {
         $errors = parent::validation($data, $files);
 
         if (!empty($data['timesplitting'])) {
-            $realtimesplitting = \tool_analytics\output\helper::option_to_class($data['timesplitting']);
-            if (\core_analytics\manager::is_valid($realtimesplitting, '\core_analytics\local\time_splitting\base') === false) {
+            $timesplittingclass = \tool_analytics\output\helper::option_to_class($data['timesplitting']);
+            if (\core_analytics\manager::is_valid($timesplittingclass, '\core_analytics\local\time_splitting\base') === false) {
                 $errors['timesplitting'] = get_string('errorinvalidtimesplitting', 'analytics');
             }
+
+            $targetclass = \tool_analytics\output\helper::option_to_class($data['target']);
+            $timesplitting = \core_analytics\manager::get_time_splitting($timesplittingclass);
+            $target = \core_analytics\manager::get_target($targetclass);
+            if (!$target->can_use_timesplitting($timesplitting)) {
+                $errors['timesplitting'] = get_string('invalidtimesplitting', 'tool_analytics');
+            }
         }
 
         if (!$this->_customdata['staticmodel']) {

admin/tool/analytics/classes/output/models_list.php

@@ -96,6 +96,10 @@ public function export_for_template(\renderer_base $output) {
             $onlycli = 1;
         }
 
+        // Evaluation options.
+        $timesplittingsforevaluation = \core_analytics\manager::get_time_splitting_methods_for_evaluation(true);
+
+        $misconfiguredmodels = [];
         $data->models = array();
         foreach ($this->models as $model) {
             $modeldata = $model->export($output);
@@ -112,6 +116,10 @@ public function export_for_template(\renderer_base $output) {
                     describe its purpose.", DEBUG_DEVELOPER);
             }
 
+            if ($model->invalid_timesplitting_selected()) {
+                $misconfiguredmodels[$model->get_id()] = $model->get_name();
+            }
+
             // Check if there is a help icon for the indicators to show.
             if (!empty($modeldata->indicators)) {
                 $indicators = array();
@@ -206,20 +214,7 @@ public function export_for_template(\renderer_base $output) {
                 $actionid = 'evaluate-' . $model->get_id();
 
                 // Evaluation options.
-                $modeltimesplittingmethods = [
-                    ['id' => 'all', 'text' => get_string('alltimesplittingmethods', 'tool_analytics')],
-                ];
-                $potentialtimesplittingmethods = $model->get_potential_timesplittings();
-                foreach (\core_analytics\manager::get_time_splitting_methods_for_evaluation(true) as $timesplitting) {
-                    if (empty($potentialtimesplittingmethods[$timesplitting->get_id()])) {
-                        // This time-splitting method can not be used for this model.
-                        continue;
-                    }
-                    $modeltimesplittingmethods[] = [
-                        'id' => \tool_analytics\output\helper::class_to_option($timesplitting->get_id()),
-                        'text' => $timesplitting->get_name()->out(),
-                    ];
-                }
+                $modeltimesplittingmethods = $this->timesplittings_options_for_evaluation($model, $timesplittingsforevaluation);
 
                 // Include the current time-splitting method as the default selection method the model already have one.
                 if ($model->get_model_obj()->timesplitting) {
@@ -338,10 +333,10 @@ public function export_for_template(\renderer_base $output) {
             $data->models[] = $modeldata;
         }
 
+        $data->warnings = [];
+        $data->infos = [];
         if (!$onlycli) {
-            $data->warnings = array(
-                (object)array('message' => get_string('bettercli', 'tool_analytics'), 'closebutton' => true)
-            );
+            $data->warnings[] = (object)array('message' => get_string('bettercli', 'tool_analytics'), 'closebutton' => true);
         } else {
             $url = new \moodle_url('/admin/settings.php', array('section' => 'analyticssettings'),
                 'id_s_analytics_onlycli');
@@ -350,12 +345,43 @@ public function export_for_template(\renderer_base $output) {
             if (is_siteadmin()) {
                 $langstrid = 'clievaluationandpredictions';
             }
-            $data->infos = array(
-                (object)array('message' => get_string($langstrid, 'tool_analytics', $url->out()),
-                    'closebutton' => true)
-            );
+            $data->infos[] = (object)array('message' => get_string($langstrid, 'tool_analytics', $url->out()),
+                'closebutton' => true);
+        }
+
+        if ($misconfiguredmodels) {
+            $warningstr = get_string('invalidtimesplittinginmodels', 'tool_analytics', implode(', ', $misconfiguredmodels));
+            $data->warnings[] = (object)array('message' => $warningstr, 'closebutton' => true);
         }
 
         return $data;
     }
+
+    /**
+     * Returns the list of time splitting methods that are available for evaluation.
+     *
+     * @param  \core_analytics\model $model
+     * @param  array                 $timesplittingsforevaluation
+     * @return array
+     */
+    private function timesplittings_options_for_evaluation(\core_analytics\model $model,
+            array $timesplittingsforevaluation): array {
+
+        $modeltimesplittingmethods = [
+            ['id' => 'all', 'text' => get_string('alltimesplittingmethods', 'tool_analytics')],
+        ];
+        $potentialtimesplittingmethods = $model->get_potential_timesplittings();
+        foreach ($timesplittingsforevaluation as $timesplitting) {
+            if (empty($potentialtimesplittingmethods[$timesplitting->get_id()])) {
+                // This time-splitting method can not be used for this model.
+                continue;
+            }
+            $modeltimesplittingmethods[] = [
+                'id' => \tool_analytics\output\helper::class_to_option($timesplitting->get_id()),
+                'text' => $timesplitting->get_name()->out(),
+            ];
+        }
+
+        return $modeltimesplittingmethods;
+    }
 }

admin/tool/analytics/lang/en/tool_analytics.php

@@ -98,7 +98,10 @@
 $string['invalidanalysables'] = 'Invalid site elements';
 $string['invalidanalysablesinfo'] = 'This page lists analysable elements that can\'t be used by this prediction model. The listed elements can\'t be used either to train the prediction model nor can the prediction model obtain predictions for them.';
 $string['invalidanalysablestable'] = 'Invalid site analysable elements table';
-$string['invalidindicatorsremoved'] = 'A new model has been added. Indicators that do not work with the selected target have been automatically removed.';
+$string['invalidcurrenttimesplitting'] = 'The current analysis interval is invalid for the target of this model. Please select a different interval.';
+$string['invalidindicatorsremoved'] = 'A new model has been added. Indicators that don\'t work with the selected target have been automatically removed.';
+$string['invalidtimesplitting'] = 'The selected analysis interval is invalid for the selected target.';
+$string['invalidtimesplittinginmodels'] = 'The analysis interval used by some of the models is invalid. Please select a different interval for the following models: {$a}';
 $string['invalidprediction'] = 'Invalid to get predictions';
 $string['invalidtraining'] = 'Invalid to train the model';
 $string['loginfo'] = 'Log extra info';

admin/tool/analytics/model.php

@@ -113,12 +113,18 @@
     case 'edit':
         confirm_sesskey();
 
+        $invalidcurrenttimesplitting = $model->invalid_timesplitting_selected();
+        $potentialtimesplittings = $model->get_potential_timesplittings();
+
         $customdata = array(
             'id' => $model->get_id(),
             'trainedmodel' => $model->is_trained(),
             'staticmodel' => $model->is_static(),
+            'invalidcurrenttimesplitting' => (!empty($invalidcurrenttimesplitting)),
+            'targetclass' => $model->get_target()->get_id(),
+            'targetname' => $model->get_target()->get_name(),
             'indicators' => $model->get_potential_indicators(),
-            'timesplittings' => $model->get_potential_timesplittings(),
+            'timesplittings' => $potentialtimesplittings,
             'predictionprocessors' => \core_analytics\manager::get_all_prediction_processors()
         );
         $mform = new \tool_analytics\output\form\edit_model(null, $customdata);

analytics/classes/local/target/base.php

@@ -84,6 +84,16 @@ abstract public function is_valid_sample($sampleid, \core_analytics\analysable $
      */
     abstract protected function calculate_sample($sampleid, \core_analytics\analysable $analysable, $starttime = false, $endtime = false);
 
+    /**
+     * Can the provided time-splitting method be used on this target?.
+     *
+     * Time-splitting methods not matching the target requirements will not be selectable by models based on this target.
+     *
+     * @param  \core_analytics\local\time_splitting\base $timesplitting
+     * @return bool
+     */
+    abstract public function can_use_timesplitting(\core_analytics\local\time_splitting\base $timesplitting): bool;
+
     /**
      * Is this target generating insights?
      *
@@ -104,18 +114,6 @@ public static function based_on_assumptions() {
         return false;
     }
 
-    /**
-     * Can the provided time-splitting method be used on this target?.
-     *
-     * Time-splitting methods not matching the target requirements will not be selectable by models based on this target.
-     *
-     * @param  \core_analytics\local\time_splitting\base $timesplitting
-     * @return bool
-     */
-    public function can_use_timesplitting(\core_analytics\local\time_splitting\base $timesplitting): bool {
-        return true;
-    }
-
     /**
      * Update the last analysis time on analysable processed or always.
      *

analytics/classes/model.php

@@ -1799,6 +1799,26 @@ public function inplace_editable_name() {
             has_capability('moodle/analytics:managemodels', \context_system::instance()), $displayname, $this->model->name);
     }
 
+    /**
+     * Returns true if the time-splitting method used by this model is invalid for this model.
+     * @return  bool
+     */
+    public function invalid_timesplitting_selected(): bool {
+        $currenttimesplitting = $this->model->timesplitting;
+        if (empty($currenttimesplitting)) {
+            // Not set is different from invalid. This function is used to identify invalid
+            // time-splittings.
+            return false;
+        }
+
+        $potentialtimesplittings = $this->get_potential_timesplittings();
+        if ($currenttimesplitting && empty($potentialtimesplittings[$currenttimesplitting])) {
+            return true;
+        }
+
+        return false;
+    }
+
     /**
      * Adds the id from {analytics_predictions} db table to the prediction \stdClass objects.
      *

analytics/tests/fixtures/test_static_target_shortname.php

@@ -61,6 +61,16 @@ public static function based_on_assumptions() {
         return true;
     }
 
+    /**
+     * Everything yep, this is just for testing.
+     *
+     * @param  \core_analytics\local\time_splitting\base $timesplitting
+     * @return bool
+     */
+    public function can_use_timesplitting(\core_analytics\local\time_splitting\base $timesplitting): bool {
+        return true;
+    }
+
     /**
      * Different analyser just to test a different one.
      *

analytics/tests/fixtures/test_target_site_users.php

@@ -65,6 +65,16 @@ public function get_analyser_class() {
         return 'test_site_users_analyser';
     }
 
+    /**
+     * Everything yep, this is just for testing.
+     *
+     * @param  \core_analytics\local\time_splitting\base $timesplitting
+     * @return bool
+     */
+    public function can_use_timesplitting(\core_analytics\local\time_splitting\base $timesplitting): bool {
+        return true;
+    }
+
     /**
      * classes_description
      *

analytics/upgrade.txt

@@ -6,7 +6,7 @@ information provided here is intended especially for developers.
 * "Time-splitting method" have been replaced by "Analysis interval" for the language strings that are
   displayed in the Moodle UI. The name of several time-splitting methods have been updated according
   to the new description of the field.
-* A new target::can_use_timesplitting method can be used to discard time-splitting methods that can not
+* A new target::can_use_timesplitting method must be implemented to discard time-splitting methods that can not
   be used on a target.
 
 === 3.7 ===

lang/en/analytics.php

@@ -37,7 +37,7 @@
 $string['disabledmodel'] = 'Disabled model';
 $string['erroralreadypredict'] = 'File {$a} has already been used to generate predictions.';
 $string['errorcannotreaddataset'] = 'Dataset file {$a} cannot be read.';
-$string['errorcannotusetimesplitting'] = 'The provided analysis interval can not be used on this model.';
+$string['errorcannotusetimesplitting'] = 'The provided analysis interval can\'t be used on this model.';
 $string['errorcannotwritedataset'] = 'Dataset file {$a} cannot be written.';
 $string['errorexportmodelresult'] = 'The machine learning model cannot be exported.';
 $string['errorimport'] = 'Error importing the provided JSON file.';