Allow several variables in combined algebra subquestions

5 files changed, 59 insertions, 31 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 34d0b36..f333eba 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,9 +3,8 @@
 The algebra question type can now be used as a subquestion of a combined question (see https://moodle.org/plugins/qtype_combined)
 There are some limitations when you insert an algebra question in a combined question:
 
-  - There is no Answer box prefix as this is not needed in algebra subquestion included in combined question because you can write what you want in the combined question text before the algebra subquestion.
-  - Allowed Functions is always "All" as this feature was never implemented yeux grands ouverts (I must remove it one day or finish it, it is here like that from the beginning when Roger Moore created this question type)
-  - There is only one variable allowed in each algebra subquestion (this is the most important limitation unfortunately, I am still trying to allow several variables but I face some difficulties)
+  - There is no Answer box prefix because you can write what you want in the combined question text before the algebra subquestion.
+  - Allowed Functions is always "All" as this feature was never implemented (I must remove it one day or finish it, it is here like that from the beginning when Roger Moore created this question type)
   - Each answer is always 100% and there is no answer feedback, only a global feedback for the subquestion when the student response is not correct (All combinable question types seems to have the same limitations so I don't know if this is a limitation of the combined API)
 
 The algebra question now support Moodle mobile (only available with Moodle 3.5 and ulterior versions, will not work with Moodle previous versions). A big thank you to Marcus Green for all his work on adding mobile support to question types.
diff --git a/combinable/combinable.php b/combinable/combinable.php
index ad4bfaf..cbc06e7 100644
--- a/combinable/combinable.php
+++ b/combinable/combinable.php
@@ -28,6 +28,8 @@ require_once($CFG->dirroot . '/question/type/algebra/parser.php');
 
 define('SYMB_QUESTION_NUMANS_START', 2);
 define('SYMB_QUESTION_NUMANS_ADD', 1);
+define('SYMB_QUESTION_NUMVARS_ADD', 1);
+define('SYMB_QUESTION_NUMVARS_START', 1);
 
 class qtype_combined_combinable_type_algebra extends qtype_combined_combinable_type_base {
 
@@ -51,6 +53,19 @@ class qtype_combined_combinable_type_algebra extends qtype_combined_combinable_t
 
 
 class qtype_combined_combinable_algebra extends qtype_combined_combinable_text_entry {
+    /**
+     * Get the form fields needed to edit one variable.
+     * @param MoodleQuickForm $mform the form being built.
+     * @return array of form fields.
+     */
+    protected function variable_group($mform) {
+        $grouparray = array();
+        $grouparray[] = $mform->createElement('text', $this->form_field_name('variable'), get_string('variablename', 'qtype_algebra'), array('size' => 10));
+        $grouparray[] = $mform->createElement('text', $this->form_field_name('varmin'), get_string('varmin', 'qtype_algebra'), array('size' => 10));
+        $grouparray[] = $mform->createElement('text', $this->form_field_name('varmax'), get_string('varmax', 'qtype_algebra'), array('size' => 10));
+
+        return $grouparray;
+    }
 
     /**
      * @param moodleform      $combinedform
@@ -78,21 +93,25 @@ class qtype_combined_combinable_algebra extends qtype_combined_combinable_text_e
         // Add an entry for a disallowed expression.
         $mform->addElement('text', $this->form_field_name('disallow'), get_string('disallow', 'qtype_algebra'), array('size' => 55));
         $mform->setType($this->form_field_name('disallow'), PARAM_RAW);
-        $varels = array();
-        $varels[] = $mform->createElement('text', $this->form_field_name('variable[0]'), get_string('variablename', 'qtype_algebra'), array('size' => 10));
-        $mform->setType($this->form_field_name('variable'), PARAM_RAW);
-        $varels[] = $mform->createElement('text', $this->form_field_name('varmin[0]'), get_string('varmin', 'qtype_algebra'), array('size' => 10));
-        $mform->setType($this->form_field_name('varmin'), PARAM_RAW);
-        $varels[] = $mform->createElement('text', $this->form_field_name('varmax[0]'), get_string('varmax', 'qtype_algebra'), array('size' => 10));
-        $mform->setType($this->form_field_name('varmax'), PARAM_RAW);
-        $mform->addGroup($varels, $this->form_field_name('variables'),
-                                                                        get_string('variable', 'qtype_algebra'), '', false);
-        $mform->setDefault($this->form_field_name('applydictionarycheck'), 1);
+        if ($this->questionrec !== null) {
+            $countvars = count($this->questionrec->options->variables);
+            $repeatsatstart = max($countvars + SYMB_QUESTION_NUMVARS_ADD, SYMB_QUESTION_NUMVARS_START);
+        } else {
+            $countvars = 0;
+            $repeatsatstart = SYMB_QUESTION_NUMVARS_START;
+        }
+
+        $variablefields = array($mform->createElement('group', $this->form_field_name('variables'),
+                 get_string('variablex', 'qtype_algebra'), $this->variable_group($mform), null, false));
+        $repeatedoptions[$this->form_field_name('variable')]['type'] = PARAM_RAW;
+        $repeatedoptions[$this->form_field_name('varmin')]['type'] = PARAM_RAW;
+        $repeatedoptions[$this->form_field_name('varmax')]['type'] = PARAM_RAW;
+        $combinedform->repeat_elements($variablefields, $repeatsatstart, $repeatedoptions, $this->form_field_name('novariables'), $this->form_field_name('addvariables'),
+                               SYMB_QUESTION_NUMVARS_ADD, get_string('addmorevariableblanks', 'qtype_algebra'), true);
         $answerel = array($mform->createElement('text',
                                                 $this->form_field_name('answer'),
                                                 get_string('answerx', 'qtype_algebra'),
                                                 array('size' => 57, 'class' => 'tweakcss')));
-
         if ($this->questionrec !== null) {
             $countanswers = count($this->questionrec->options->answers);
         } else {
@@ -123,10 +142,12 @@ class qtype_combined_combinable_algebra extends qtype_combined_combinable_text_e
             foreach ($this->questionrec->options->answers as $answer) {
                 $answers['answer'][] = $answer->answer;
             }
-            $variable = array_pop($this->questionrec->options->variables);
-            $variables['variable'][] = $variable->name;
-            $variables['varmin'][] = $variable->min;
-            $variables['varmax'][] = $variable->max;
+
+            foreach ($this->questionrec->options->variables as $variable) {
+                $variables['variable'][] = $variable->name;
+                $variables['varmin'][] = $variable->min;
+                $variables['varmax'][] = $variable->max;
+            }
         }
         $data = parent::data_to_form($context, $fileoptions) + $answers + $variables;
         return $data;
@@ -152,11 +173,11 @@ class qtype_combined_combinable_algebra extends qtype_combined_combinable_text_e
             }
             // Check that this variable does not have the same name as a function.
             if (in_array($trimvar, qtype_algebra_parser::$functions) or in_array($trimvar, qtype_algebra_parser::$specials)) {
-                $errors[$this->form_field_name("variables")] = get_string('illegalvarname', 'qtype_algebra', $trimvar);
+                $errors[$this->form_field_name('variables['.$key.']')] = get_string('illegalvarname', 'qtype_algebra', $trimvar);
             }
             // Check that this variable has not been defined before.
             if (in_array($trimvar, $varlist)) {
-                $errors[$this->form_field_name("variables")] = get_string('duplicatevar', 'qtype_algebra', $trimvar);
+                $errors[$this->form_field_name('variables['.$key.']')] = get_string('duplicatevar', 'qtype_algebra', $trimvar);
             } else {
                 // Add the variable to the list of defined variables.
                 $varlist[] = $trimvar;
@@ -167,26 +188,26 @@ class qtype_combined_combinable_algebra extends qtype_combined_combinable_text_e
             if ($this->formdata->compareby == 'eval') {
                 // Check that a minimum has been defined.
                 if ($trimmin == '') {
-                    $errors[$this->form_field_name("variables")] = get_string('novarmin', 'qtype_algebra');
+                    $errors[$this->form_field_name('variables['.$key.']')] = get_string('novarmin', 'qtype_algebra');
                 } else if (!preg_match($renumber, $trimmin)) {
                     // If there is one check that it's a number.
-                    $errors[$this->form_field_name("variables")] = get_string('notanumber', 'qtype_algebra');
+                    $errors[$this->form_field_name('variables['.$key.']')] = get_string('notanumber', 'qtype_algebra');
                 }
                 if ($trimmax == '') {
-                    $errors[$this->form_field_name("variables")] = get_string('novarmax', 'qtype_algebra');
+                    $errors[$this->form_field_name('variables['.$key.']')] = get_string('novarmax', 'qtype_algebra');
                 } else if (!preg_match($renumber, $trimmax)) {
                     // If there is one check that it is a number.
-                    $errors[$this->form_field_name("variables")] = get_string('notanumber', 'qtype_algebra');
+                    $errors[$this->form_field_name('variables['.$key.']')] = get_string('notanumber', 'qtype_algebra');
                 }
                 // Check that the minimum is less that the maximum!
                 if ((float)$trimmin > (float)$trimmax) {
-                    $errors[$this->form_field_name("variable")] = get_string('varmingtmax', 'qtype_algebra');
+                    $errors[$this->form_field_name('variables['.$key.']')] = get_string('varmingtmax', 'qtype_algebra');
                 }
             } // End check for eval type.
         }     // End loop over variables.
         // Check that at least one variable is defined.
         if (count($varlist) == 0) {
-            $errors[$this->form_field_name('variables')] = get_string('notenoughvars', 'qtype_algebra');
+            $errors[$this->form_field_name('variables|0]')] = get_string('notenoughvars', 'qtype_algebra');
         }
 
         // Now perform the sanity checks on the answers.
@@ -245,7 +266,7 @@ class qtype_combined_combinable_algebra extends qtype_combined_combinable_text_e
                 $trimvar = trim($var);
                 // If the variable is in the unused array then add the error message to that variable.
                 if (in_array($trimvar, $d)) {
-                    $errors[$this->form_field_name('variable['.$key.']')] = get_string('unusedvar', 'qtype_algebra');
+                    $errors[$this->form_field_name('variables['.$key.']')] = get_string('unusedvar', 'qtype_algebra');
                 }
             }
         }
diff --git a/db/upgrade.php b/db/upgrade.php
index e3fc7df..87ee235 100644
--- a/db/upgrade.php
+++ b/db/upgrade.php
@@ -41,7 +41,7 @@ function xmldb_qtype_algebra_upgrade($oldversion=0) {
         upgrade_plugin_savepoint(true, 2008061500, 'qtype', 'algebra');
     }
 
-    // Drop the answers and variables fields wich are totaly redundants.
+    // Drop the answers and variables fields wich are totally redundant.
     if ($oldversion < 2011072800) {
         $table = new xmldb_table('question_algebra');
         $field = new xmldb_field('answers');
diff --git a/questiontype.php b/questiontype.php
index 263408e..972b291 100644
--- a/questiontype.php
+++ b/questiontype.php
@@ -120,6 +120,15 @@ class qtype_algebra extends question_type {
     }
 
     /**
+     * Returns true is answer with the $key is empty in the question data and should not be saved in DB.
+     * @param object $questiondata This holds the information from the question editing form or import.
+     * @param int $key A key of the answer in question.
+     * @return bool True if answer shouldn't be saved in DB.
+     */
+    protected function is_answer_empty($questiondata, $key) {
+        return trim($questiondata->answer[$key]) == '';
+    }
+    /**
      * Saves question-type specific options
      *
      * This is called by {@link save_question()} to save the question-type specific data from a
@@ -156,7 +165,6 @@ class qtype_algebra extends question_type {
             $result->error = $e->getMessage();
             return $result;
         }
-
         // Process the allowed functions field. This code just sets up the variable, it is saved
         // in the parent class' save_question_options method called at the end of this method
         // Look for the 'all' option. If we find it then set the string to an empty value.
diff --git a/version.php b/version.php
index 1488c77..747e880 100644
--- a/version.php
+++ b/version.php
@@ -23,8 +23,8 @@
 defined('MOODLE_INTERNAL') || die();
 
 $plugin->component = 'qtype_algebra';
-$plugin->version   = 2019042707;
+$plugin->version   = 2019042800;
 
 $plugin->requires  = 2016052300;
-$plugin->release   = '1.9 for Moodle 3.0 ... 3.7';
+$plugin->release   = '1.91 for Moodle 3.1 ... 3.7';
 $plugin->maturity  = MATURITY_STABLE;