Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Blocker Blocker
    • Resolution: Fixed
    • Affects Version/s: 2.0
    • Fix Version/s: 2.1
    • Component/s: Questions, Quiz
    • Labels:
    • Rank:
      1243

      Description

      See http://docs.moodle.org/en/Development:Question_Engine_2

      More detailed tracking currently only in http://lts-strat-dev-1.open.ac.uk/bugzilla/show_bug.cgi?id=8553, which is only available on the OU network. Sorry.

      To be honest, I am only really creating this bug as a place to store Balsamiq UI mockups.

      1. Question preview UI.bmml
        10 kB
        Tim Hunt
      2. Question preview UI.bmml
        8 kB
        Tim Hunt
      3. Question preview UI.bmml
        8 kB
        Tim Hunt
      4. The various bits of a question.bmml
        7 kB
        Tim Hunt
      1. Question preview UI.png
        76 kB
      2. screenshot-1.jpg
        75 kB
      3. screenshot-2.jpg
        29 kB
      4. screenshot-3.jpg
        30 kB
      5. screenshot-4.jpg
        79 kB
      6. screenshot-5.jpg
        32 kB
      7. screenshot-6.jpg
        98 kB
      8. The various bits of a question.png
        117 kB

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          Added UI Mockup: <Question preview UI>

          Show
          Tim Hunt added a comment - Added UI Mockup: <Question preview UI>
          Hide
          Tim Hunt added a comment -

          Added UI Mockup: <The various bits of a question>

          Show
          Tim Hunt added a comment - Added UI Mockup: <The various bits of a question>
          Hide
          Pierre Pichet added a comment -

          Tim
          "Sorry" , I am using this bug as a chanel to technical questions about the project...

          In HEAD I add a multichoice option to calculated question which was a good idea as this do not increase the number of moodle supported question type.
          Given the new engine reworking, this increase the complexity more than if there exist two question types i.e calculated and calculated multichoice.
          Splitting will not increase the amount of code already in moodle as the actual calculated contains all the necessary code for the two "virtual" question types.

          Splitting will simplify the code and facilitate the conformity of the two calculated versions to the new engine.

          If you agree, create the necessary sub-task .

          Show
          Pierre Pichet added a comment - Tim "Sorry" , I am using this bug as a chanel to technical questions about the project... In HEAD I add a multichoice option to calculated question which was a good idea as this do not increase the number of moodle supported question type. Given the new engine reworking, this increase the complexity more than if there exist two question types i.e calculated and calculated multichoice. Splitting will not increase the amount of code already in moodle as the actual calculated contains all the necessary code for the two "virtual" question types. Splitting will simplify the code and facilitate the conformity of the two calculated versions to the new engine. If you agree, create the necessary sub-task .
          Hide
          Tim Hunt added a comment -

          My guess is that splitting, or not, is something we can decide later when we know more about how the new system works.

          Show
          Tim Hunt added a comment - My guess is that splitting, or not, is something we can decide later when we know more about how the new system works.
          Hide
          Pierre Pichet added a comment - - edited

          Sooner is better than later as the code will not have to handle the case of calculated being truly one or multichoice ones.

          As HEAD is very experimental at its actual state, we don't have to worry about calculated questions being truly one or multichoice ones.

          A first version can be done very rapidly (i.e. for next Weekly Code Review).

          The main idea is once the

          {param}

          are replaced and the question text value or answer values are calculated, the multichoice version cannot de distinguished from a normal multichoice. The only supplementary element to saved in the attempt is the datasetitem number.
          Actually the calculated use in the save and restore process after saving used either the numerical or the multichoice code after taking care of the datasetitem number.

           example in restore_session_and_responses
                  if (!preg_match('~^dataset([0-9]+)[^-]*-(.*)$~',
                          $state->responses[''], $regs)) {
                      echo $OUTPUT->....
                      return false;
                  }
          
                  // Restore the chosen dataset
                  $state->options->datasetitem = $regs[1];
                  $state->options->dataset =
                   $this->pick_question_dataset($question,$state->options->datasetitem);
                  $state->responses = array('' => $regs[2]);
                  $virtualqtype = $this->get_virtual_qtype( $question);
                 //  $virtualqtype BEING  either numerical or multichoice{                   
                      return $virtualqtype->restore_session_and_responses($question, $state);
          
          

          This give me the idea on how to test your new engine code renderer.

          Show
          Pierre Pichet added a comment - - edited Sooner is better than later as the code will not have to handle the case of calculated being truly one or multichoice ones. As HEAD is very experimental at its actual state, we don't have to worry about calculated questions being truly one or multichoice ones. A first version can be done very rapidly (i.e. for next Weekly Code Review). The main idea is once the {param} are replaced and the question text value or answer values are calculated, the multichoice version cannot de distinguished from a normal multichoice. The only supplementary element to saved in the attempt is the datasetitem number. Actually the calculated use in the save and restore process after saving used either the numerical or the multichoice code after taking care of the datasetitem number. example in restore_session_and_responses if (!preg_match('~^dataset([0-9]+)[^-]*-(.*)$~', $state->responses[''], $regs)) { echo $OUTPUT->.... return false ; } // Restore the chosen dataset $state->options->datasetitem = $regs[1]; $state->options->dataset = $ this ->pick_question_dataset($question,$state->options->datasetitem); $state->responses = array('' => $regs[2]); $virtualqtype = $ this ->get_virtual_qtype( $question); // $virtualqtype BEING either numerical or multichoice{ return $virtualqtype->restore_session_and_responses($question, $state); This give me the idea on how to test your new engine code renderer.
          Hide
          Pierre Pichet added a comment -

          As you wrote; wait and see at least until 2.0 is created.

          Show
          Pierre Pichet added a comment - As you wrote; wait and see at least until 2.0 is created.
          Hide
          Pierre Pichet added a comment -

          Here the calculated first shot
          I can go south...

          Show
          Pierre Pichet added a comment - Here the calculated first shot I can go south...
          Hide
          Pierre Pichet added a comment -

          Here the zip file with echos and everything
          remain to add the equation that can be in the question text
          see the actual print-question
          //evaluate the equations i.e

          {=5+4) $qtext = ""; $qtextremaining = $numericalquestion->questiontext ; while (ereg('\{=([^[:space:]}

          ]*)}', $qtextremaining, $regs1)) {
          $qtextsplits = explode($regs1[0], $qtextremaining, 2);
          $qtext =$qtext.$qtextsplits[0];
          $qtextremaining = $qtextsplits[1];
          if (empty($regs1[1]))

          { $str = ''; }

          else {
          if( $formulaerrors = qtype_calculated_find_formula_errors($regs1[1]))

          { $str=$formulaerrors ; }

          else

          { eval('$str = '.$regs1[1].';'); }

          }
          $qtext = $qtext.$str ;
          }
          $numericalquestion->questiontext = $qtext.$qtextremaining ; // end replace equations

          Show
          Pierre Pichet added a comment - Here the zip file with echos and everything remain to add the equation that can be in the question text see the actual print-question //evaluate the equations i.e {=5+4) $qtext = ""; $qtextremaining = $numericalquestion->questiontext ; while (ereg('\{=([^[:space:]} ]*)}', $qtextremaining, $regs1)) { $qtextsplits = explode($regs1 [0] , $qtextremaining, 2); $qtext =$qtext.$qtextsplits [0] ; $qtextremaining = $qtextsplits [1] ; if (empty($regs1 [1] )) { $str = ''; } else { if( $formulaerrors = qtype_calculated_find_formula_errors($regs1 [1] )) { $str=$formulaerrors ; } else { eval('$str = '.$regs1[1].';'); } } $qtext = $qtext.$str ; } $numericalquestion->questiontext = $qtext.$qtextremaining ; // end replace equations
          Hide
          Tim Hunt added a comment -

          Wow! fast work. I am hoping that this means that the new system is easy to work with.

          Show
          Tim Hunt added a comment - Wow! fast work. I am hoping that this means that the new system is easy to work with.
          Hide
          Pierre Pichet added a comment -

          Once you find where to look.
          You have to code how the save the answer and the datasetitem in your new attempt structure.
          But this you know better than me.
          Once the

          {a}

          ,

          {b}

          are converted, a calculated is essentially a numerical question.

          Show
          Pierre Pichet added a comment - Once you find where to look. You have to code how the save the answer and the datasetitem in your new attempt structure. But this you know better than me. Once the {a} , {b} are converted, a calculated is essentially a numerical question.
          Hide
          Pierre Pichet added a comment -

          You model suppose that the feedback will be output after the question elements.
          This is set in the core renderer class
          For multianswer questions that are in-line, the feedback is output in a pop-up window that appears when the user use the mouse.

          How can I create a specific qtype core renderer class to control the entire question flow?

          I understand that modifying the general model is perhaps not an easy question

          How about the count-down for 2,0 ?

          The fiance of my wife mother is dying (at 94 years old so he has got his chance with life) and, among other things, this will slow down my moodle code progress for this week.

          Show
          Pierre Pichet added a comment - You model suppose that the feedback will be output after the question elements. This is set in the core renderer class For multianswer questions that are in-line, the feedback is output in a pop-up window that appears when the user use the mouse. How can I create a specific qtype core renderer class to control the entire question flow? I understand that modifying the general model is perhaps not an easy question How about the count-down for 2,0 ? The fiance of my wife mother is dying (at 94 years old so he has got his chance with life) and, among other things, this will slow down my moodle code progress for this week.
          Hide
          Tim Hunt added a comment -

          I am sorry to hear about your father-in-law.

          You can output feedback during ->formuation_and_controls. For example multichoice already does so for the choice-specific feedback.

          Show
          Tim Hunt added a comment - I am sorry to hear about your father-in-law. You can output feedback during ->formuation_and_controls. For example multichoice already does so for the choice-specific feedback.
          Hide
          Pierre Pichet added a comment - - edited

          This is somewhat more complicated as the correct response ( shortanswer and numerical) is also included in the pop-up window.

          Perhaps we should add another function to qtype-question or renderer classes so that the core renderer can check if these elements were already rendered in the print_formulation.. step.

          Show
          Pierre Pichet added a comment - - edited This is somewhat more complicated as the correct response ( shortanswer and numerical) is also included in the pop-up window. Perhaps we should add another function to qtype-question or renderer classes so that the core renderer can check if these elements were already rendered in the print_formulation.. step.
          Hide
          Pierre Pichet added a comment - - edited

          On the other end,
          As the core renderer will only work on the main multiasnwer without knowing the existence of the subquestions, the main multianswer has the complete control on all the steps of the subquestions.
          So this is inside multianswer question and renderer that the coding should be done.

          How about the count-down for 2,0 ?

          Show
          Pierre Pichet added a comment - - edited On the other end, As the core renderer will only work on the main multiasnwer without knowing the existence of the subquestions, the main multianswer has the complete control on all the steps of the subquestions. So this is inside multianswer question and renderer that the coding should be done. How about the count-down for 2,0 ?
          Hide
          Pierre Pichet added a comment - - edited

          My father-in-law died without sufferig yesterday at noon.
          So this will perturbate everyday life for the next 4 days at teast...
          a technical question
          the classes for the subquestions could be either
          class qtype_multianswer_shortanswer_renderer extends qtype_shortanswer_renderer {

          or

          class qtype_shortanswer_multianswer_renderer extends qtype_shortanswer_renderer {

          in either case this should be put in multianswer/question.php with the necessary require_once(..)

          As they will only be called inside the multianswer renderer, is one of the 2 classes is the best or there is a better solution.

          appart from the inital class calls that are split in two parts (qytpe, specific) the othe calls are in one word like
          protected function make_question_instance($questiondata) {
          question_bank::load_question_definition_classes($this->name());
          if ($questiondata->options->single)

          { $class = 'qtype_multichoice_single_question'; }

          else

          { $class = 'qtype_multichoice_multi_question'; }

          return new $class();
          }

          Show
          Pierre Pichet added a comment - - edited My father-in-law died without sufferig yesterday at noon. So this will perturbate everyday life for the next 4 days at teast... a technical question the classes for the subquestions could be either class qtype_multianswer_shortanswer_renderer extends qtype_shortanswer_renderer { or class qtype_shortanswer_multianswer_renderer extends qtype_shortanswer_renderer { in either case this should be put in multianswer/question.php with the necessary require_once(..) As they will only be called inside the multianswer renderer, is one of the 2 classes is the best or there is a better solution. appart from the inital class calls that are split in two parts (qytpe, specific) the othe calls are in one word like protected function make_question_instance($questiondata) { question_bank::load_question_definition_classes($this->name()); if ($questiondata->options->single) { $class = 'qtype_multichoice_single_question'; } else { $class = 'qtype_multichoice_multi_question'; } return new $class(); }
          Hide
          Pierre Pichet added a comment -

          I just found the public static function load_question_definition_classes($qtypename) {
          which takes care of loading the classes.

          Show
          Pierre Pichet added a comment - I just found the public static function load_question_definition_classes($qtypename) { which takes care of loading the classes.
          Hide
          Pierre Pichet added a comment -

          I am able to address the subquestions to there specific classes (using the qtype_multianswer_shortanswer_ etc convention) and initialise the subquestions instances with the correct data .
          The classes are unique for shortanswer and numerical but there are three variants of the multiplechoice.(inline using select element, vertical and horizontal).
          Next step, displaying the complete question.
          one sub question type after the other, shortanswer first

          Show
          Pierre Pichet added a comment - I am able to address the subquestions to there specific classes (using the qtype_multianswer_shortanswer_ etc convention) and initialise the subquestions instances with the correct data . The classes are unique for shortanswer and numerical but there are three variants of the multiplechoice.(inline using select element, vertical and horizontal). Next step, displaying the complete question. one sub question type after the other, shortanswer first
          Hide
          Pierre Pichet added a comment -

          Can display shortanswer and numerical inputs at the rigth place in the question text.

          Show
          Pierre Pichet added a comment - Can display shortanswer and numerical inputs at the rigth place in the question text.
          Hide
          Pierre Pichet added a comment -

          Ok for multiplechoice.
          Then the next step (also a major one) is to create and store the attempts of subquestions.
          This should be done so that another qtype could be easily added.
          all the process should be done by the subquestions which is different from the actual code as the attempts results are stored in one field.
          As each subquestion is a real question stored in the question table, it has a question id
          we could go as far as having a complete attempt object for each subquestion.
          A multianswer is a question which has no anwer by itself , only the subquestions are real questions.
          Multianswer is more a quiz than a question ,

          Show
          Pierre Pichet added a comment - Ok for multiplechoice. Then the next step (also a major one) is to create and store the attempts of subquestions. This should be done so that another qtype could be easily added. all the process should be done by the subquestions which is different from the actual code as the attempts results are stored in one field. As each subquestion is a real question stored in the question table, it has a question id we could go as far as having a complete attempt object for each subquestion. A multianswer is a question which has no anwer by itself , only the subquestions are real questions. Multianswer is more a quiz than a question ,
          Hide
          Tim Hunt added a comment -

          I don't think we should have a separate question_attempt stored in the database.

          I think the multianswer question should use a single question_attempt. But it will need to take the $qa->get_question_data(), and split that array up to separate the data belonging to the different subquestions, and then put it back together again.

          That may involve constructing new question_attempt objects in memory holding a subset of the data.

          At least that seems like a clever idea in principle. I don't know if all the details will work out.

          Show
          Tim Hunt added a comment - I don't think we should have a separate question_attempt stored in the database. I think the multianswer question should use a single question_attempt. But it will need to take the $qa->get_question_data(), and split that array up to separate the data belonging to the different subquestions, and then put it back together again. That may involve constructing new question_attempt objects in memory holding a subset of the data. At least that seems like a clever idea in principle. I don't know if all the details will work out.
          Hide
          Pierre Pichet added a comment -

          You are rethinking the question engine so that it will be set for the future.
          The cloze question give us the opportunity to work on a different case and elaborate a structure with some supplementary flexibility as was the parent parameter on question.

          "constructing new question_attempt " just in memory and storing them in a structure to be created means a very complex coding that could mean that cloze is somehow a dead end.

          I understand that this means some rewrite but I understand also that you want a somehow general and permanent solution.

          for example cloze by itself fit more the description model as
          public function get_expected_data()

          { return array(); }

          public function get_correct_response() { return array(); }

          all the data or response are handled by the subquestions.

          Just to work on displaying the input element for shortanswer, numerical and multiple choice I need to add supplementary parameters so that the subquestions and the renderer knows where to find their data (their subquestion index) in the subquestions structure.

          It is in working for the attempt data and processing that I figure out how simpler it will be to be able to store everything in subattempt table so the idea of parent parameter.

          As we want to extend cloze and in the same way to extend where we can use the same question engine (quiz, lesson or...), some rethinking is perhaps necessary.

          If I say it in other words, what could be the structure that will allow to plug-in easily other question types in cloze.

          Perhaps parent in not the best idea.

          Show
          Pierre Pichet added a comment - You are rethinking the question engine so that it will be set for the future. The cloze question give us the opportunity to work on a different case and elaborate a structure with some supplementary flexibility as was the parent parameter on question. "constructing new question_attempt " just in memory and storing them in a structure to be created means a very complex coding that could mean that cloze is somehow a dead end. I understand that this means some rewrite but I understand also that you want a somehow general and permanent solution. for example cloze by itself fit more the description model as public function get_expected_data() { return array(); } public function get_correct_response() { return array(); } all the data or response are handled by the subquestions. Just to work on displaying the input element for shortanswer, numerical and multiple choice I need to add supplementary parameters so that the subquestions and the renderer knows where to find their data (their subquestion index) in the subquestions structure. It is in working for the attempt data and processing that I figure out how simpler it will be to be able to store everything in subattempt table so the idea of parent parameter. As we want to extend cloze and in the same way to extend where we can use the same question engine (quiz, lesson or...), some rethinking is perhaps necessary. If I say it in other words, what could be the structure that will allow to plug-in easily other question types in cloze. Perhaps parent in not the best idea.
          Hide
          Tim Hunt added a comment -

          I think that for cloze, it should be something like:

          public function get_expected_data() {
              $expected = array();
              foreach ($this->subquestions as $i => $sq) {
                  $sqexpected = $sq->get_expected_data();
                  foreach ($sqexpected as $name => $type) {
                      $expected['sub' . $i . '-' . $name] = $type;
                  }
              }
              return $expected;
          

          That is, we delegate as much as possible to the real question code for the subquestions. We try to avoid making any assumptions about what the subquestions are.

          Show
          Tim Hunt added a comment - I think that for cloze, it should be something like: public function get_expected_data() { $expected = array(); foreach ($ this ->subquestions as $i => $sq) { $sqexpected = $sq->get_expected_data(); foreach ($sqexpected as $name => $type) { $expected['sub' . $i . '-' . $name] = $type; } } return $expected; That is, we delegate as much as possible to the real question code for the subquestions. We try to avoid making any assumptions about what the subquestions are.
          Hide
          Pierre Pichet added a comment -

          OK I will try to set the code following these guidelines.

          Show
          Pierre Pichet added a comment - OK I will try to set the code following these guidelines.
          Hide
          Pierre Pichet added a comment -

          Using your suggestion, I defined the input elements following the 'sub' . $i . '-' . $name where $name is generally answer but can be choice in multiplechoice.

          You build a beautifull code for this project using all the possibilities of class structure at the image of your expertise.

          The consequence is a quite step learning curve when you have to push the system limits.

          My experimental code is full of echos just to follow the beast.

          Being more a student than a professor is also the part of the pleasure of coding moodle.

          I had to change some protected to public status for example when using numerical initialise_num...() inside multianswer/questiontype.php but these are normal things.

          I can retrieve the expected data in the preview process.

          Initially I did not add answers to multianswer question object as multianswer has no answers for itself.
          All the answers are in the subquestions.
          But this did not work as it is difficult for the interaction models to work with a question that do not have answers or responses.
          So I put all the answers of the subquestions in the multianswer->answers with the subquestionindex as array.

          I am redefining the various functions to work correctly although the code trail is somehow surprising (at least for me ).
          Some times it access the multianswer question.php code, at other steps it comes to the subquestions code.

          I need more echos to complete...and to identify what is related to the engine code and what is related to my modifications.

          In any cases I need to fully understand this beautifull engine to continue my work on questions.

          How about the time delays and the 2.0 release?

          Show
          Pierre Pichet added a comment - Using your suggestion, I defined the input elements following the 'sub' . $i . '-' . $name where $name is generally answer but can be choice in multiplechoice. You build a beautifull code for this project using all the possibilities of class structure at the image of your expertise. The consequence is a quite step learning curve when you have to push the system limits. My experimental code is full of echos just to follow the beast. Being more a student than a professor is also the part of the pleasure of coding moodle. I had to change some protected to public status for example when using numerical initialise_num...() inside multianswer/questiontype.php but these are normal things. I can retrieve the expected data in the preview process. Initially I did not add answers to multianswer question object as multianswer has no answers for itself. All the answers are in the subquestions. But this did not work as it is difficult for the interaction models to work with a question that do not have answers or responses. So I put all the answers of the subquestions in the multianswer->answers with the subquestionindex as array. I am redefining the various functions to work correctly although the code trail is somehow surprising (at least for me ). Some times it access the multianswer question.php code, at other steps it comes to the subquestions code. I need more echos to complete...and to identify what is related to the engine code and what is related to my modifications. In any cases I need to fully understand this beautifull engine to continue my work on questions. How about the time delays and the 2.0 release?
          Hide
          Pierre Pichet added a comment - - edited

          "Some times it access the multianswer question.php code, at other steps it comes to the subquestions code."

          As the subquestions were defined inside multianswer/ question.php they are defined as
          qtype_multianswer_numerical_question extends qtype_numerical_question `{
          public $subquestionindex ;
          public function get_renderer()

          { return renderer_factory::get_renderer('qtype_multianswer', 'numerical'); }

          }
          and
          class qtype_multianswer_numerical_renderer extends qtype_numerical_renderer {
          where only the
          public function formulation_and_controls(question_attempt $qa,
          question_display_options $options,$key) {
          has been modified to exclude the question text which is render by the multianswer renderer, only the input elements and eventually the popup window are displayed.
          // $question = $qa->get_question();

          I just realized ( from a print_r($qa) ) that the subquestions question type is multianswer.
          This could be the origin of the surprising steps being sometimes in multianswer or in numerical.

          Should they be defined reverse i.e
          qtype_numerical_multianswer_question extends qtype_numerical_question `{
          so that there qtypes being numerical, shortanswer etc.
          ?
          However I remember that somewhere you have a function that verifiy that the code(or file) for a given qtype question is present and the this verification imply that the code is in the directory related to the qtype.
          This was the reason why I defined them as qtype_multianswer_numerical_

          Show
          Pierre Pichet added a comment - - edited "Some times it access the multianswer question.php code, at other steps it comes to the subquestions code." As the subquestions were defined inside multianswer/ question.php they are defined as qtype_multianswer_numerical_question extends qtype_numerical_question `{ public $subquestionindex ; public function get_renderer() { return renderer_factory::get_renderer('qtype_multianswer', 'numerical'); } } and class qtype_multianswer_numerical_renderer extends qtype_numerical_renderer { where only the public function formulation_and_controls(question_attempt $qa, question_display_options $options,$key) { has been modified to exclude the question text which is render by the multianswer renderer, only the input elements and eventually the popup window are displayed. // $question = $qa->get_question(); I just realized ( from a print_r($qa) ) that the subquestions question type is multianswer. This could be the origin of the surprising steps being sometimes in multianswer or in numerical. Should they be defined reverse i.e qtype_numerical_multianswer_question extends qtype_numerical_question `{ so that there qtypes being numerical, shortanswer etc. ? However I remember that somewhere you have a function that verifiy that the code(or file) for a given qtype question is present and the this verification imply that the code is in the directory related to the qtype. This was the reason why I defined them as qtype_multianswer_numerical_
          Hide
          Pierre Pichet added a comment -

          I am testing the reverse i.e qtype_shortanswer_multianswer

          Show
          Pierre Pichet added a comment - I am testing the reverse i.e qtype_shortanswer_multianswer
          Hide
          Tim Hunt added a comment -

          I think the class names could be either way round - if necessary we fix that code that makes sure the right classes are loaded. It is more important to make the code work nicely.

          Show
          Tim Hunt added a comment - I think the class names could be either way round - if necessary we fix that code that makes sure the right classes are loaded. It is more important to make the code work nicely.
          Hide
          Pierre Pichet added a comment -

          Changing the qtype after the default initialisation solve the problem i.e the processing is done on multianswer object.
          foreach($questiondata->options->questions as $key =>$wrapped) {
          $class = "";
          switch ($wrapped->qtype){
          case "shortanswer":$class = "qtype_shortanswer_multianswer_question";
          $question->subquestions[$key]= new $class();
          $question->subquestions[$key]->subquestionindex = $key ;
          parent::initialise_question_instance($question->subquestions[$key], $wrapped);
          $question->subquestions[$key]->qtype = $QTYPES['shortanswer'] ;
          $this->initialise_question_answers($question->subquestions[$key], $wrapped);
          break;

          Effectively the name is not important.

          So next step: set correctly the multianswer functions to handle the subquestions.

          Show
          Pierre Pichet added a comment - Changing the qtype after the default initialisation solve the problem i.e the processing is done on multianswer object. foreach($questiondata->options->questions as $key =>$wrapped) { $class = ""; switch ($wrapped->qtype){ case "shortanswer":$class = "qtype_shortanswer_multianswer_question"; $question->subquestions [$key] = new $class(); $question->subquestions [$key] ->subquestionindex = $key ; parent::initialise_question_instance($question->subquestions [$key] , $wrapped); $question->subquestions [$key] ->qtype = $QTYPES ['shortanswer'] ; $this->initialise_question_answers($question->subquestions [$key] , $wrapped); break; Effectively the name is not important. So next step: set correctly the multianswer functions to handle the subquestions.
          Hide
          Pierre Pichet added a comment -

          Hourra
          Working for one shortanswer either Fill in correct and Submit and finish.

          other modifications needed for more than one subquestion

          Show
          Pierre Pichet added a comment - Hourra Working for one shortanswer either Fill in correct and Submit and finish. other modifications needed for more than one subquestion
          Hide
          Pierre Pichet added a comment -

          OK for more than one shortanswer subquestion, the input element is correctly display with the color related to the grade.
          So we can conclude that the main structure is OK.

          Let's work with the other question types and the grading and...

          Unless there is a major problem, my next comments will be when the code is ready for your final touch...

          Show
          Pierre Pichet added a comment - OK for more than one shortanswer subquestion, the input element is correctly display with the color related to the grade. So we can conclude that the main structure is OK. Let's work with the other question types and the grading and... Unless there is a major problem, my next comments will be when the code is ready for your final touch...
          Hide
          Pierre Pichet added a comment -

          Given the new possibilities of the engine, I propose that we add the shuffle option and the multiple response option.

          Shuffling could be added to all multiple choice options but multiple response only to MCV or MCH

          new options could be

          MMCV
          MMCH

          and for shuffling
          MCS
          MCVS
          MCHS
          MMCVS
          MMCHS

          There will be minor changes in the decoding of the qtype_multianswer_extract_question() and in the question editor

          This could be done in 2.0 once the engine is included (2,1) and your special 1.9 Open University

          In any cases I prepare the necessary code in the multianswer question.

          Show
          Pierre Pichet added a comment - Given the new possibilities of the engine, I propose that we add the shuffle option and the multiple response option. Shuffling could be added to all multiple choice options but multiple response only to MCV or MCH new options could be MMCV MMCH and for shuffling MCS MCVS MCHS MMCVS MMCHS There will be minor changes in the decoding of the qtype_multianswer_extract_question() and in the question editor This could be done in 2.0 once the engine is included (2,1) and your special 1.9 Open University In any cases I prepare the necessary code in the multianswer question.
          Hide
          Pierre Pichet added a comment -

          I modify the multianswer regexp question analyzer to detect all variants
          The edit_multianswer_form can then create the complete set of options for multiple questions inside cloze.
          The shuffling works correctly but I have to modify the 'sub' . $i . '-' . $name
          to '_sub' . $i . '-' . $name as
          $step->set_qt_var('_sub' . $this->subquestionindex . ''.'_order', implode(',', $this>order));
          wil not work will variables name not preceeded with a _ .

          Next step multiple responses multianswer and then formatting correctly every variants.

          How about the 1 march delay for 2,0 as I have things to tests on calculated and, incidently , I am not full time moodle developper although my wife and my boss are beginning to be convinced of this.

          Show
          Pierre Pichet added a comment - I modify the multianswer regexp question analyzer to detect all variants The edit_multianswer_form can then create the complete set of options for multiple questions inside cloze. The shuffling works correctly but I have to modify the 'sub' . $i . '-' . $name to '_sub' . $i . '-' . $name as $step->set_qt_var('_sub' . $this->subquestionindex . ' '.'_order', implode(',', $this >order)); wil not work will variables name not preceeded with a _ . Next step multiple responses multianswer and then formatting correctly every variants. How about the 1 march delay for 2,0 as I have things to tests on calculated and, incidently , I am not full time moodle developper although my wife and my boss are beginning to be convinced of this.
          Hide
          Tim Hunt added a comment -

          Well, the latest information about the 2.0 release is on http://docs.moodle.org/en/Roadmap, and in the planning document that is linked from there. I think it is unlikely they will have everything done by the end of march but the will be close. But then there will be several months of testing before the final 2.0 release, so plenty of time to finish things off. The issue for me is how much time the OU wants me to work on OU-specific projects, and how much time I will have for bug-fixing 2.0. But they are normally quite enlightened about that sort of thing.

          I still don't know if I am going to finish these changes in time to go into Moodle 2.0. The quiz reports are proving a real beast, but I am making some progress.

          Show
          Tim Hunt added a comment - Well, the latest information about the 2.0 release is on http://docs.moodle.org/en/Roadmap , and in the planning document that is linked from there. I think it is unlikely they will have everything done by the end of march but the will be close. But then there will be several months of testing before the final 2.0 release, so plenty of time to finish things off. The issue for me is how much time the OU wants me to work on OU-specific projects, and how much time I will have for bug-fixing 2.0. But they are normally quite enlightened about that sort of thing. I still don't know if I am going to finish these changes in time to go into Moodle 2.0. The quiz reports are proving a real beast, but I am making some progress.
          Hide
          Pierre Pichet added a comment -

          I worried because I read from the Road map 'March 2010: Moodle 2.0 Beta release' as the March 1th. ....

          Show
          Pierre Pichet added a comment - I worried because I read from the Road map 'March 2010: Moodle 2.0 Beta release' as the March 1th. ....
          Hide
          Tim Hunt added a comment -
          Show
          Tim Hunt added a comment - No, your memory is correct http://docs.moodle.org/en/index.php?title=Roadmap&diff=68164&oldid=68046
          Hide
          Pierre Pichet added a comment -

          I need an universal is_same_response(array ($prevresponse, array $newresponse)
          Could this do the job?

          public function is_same_response(array $prevresponse, array $newresponse) {

          if ( ($newresponse == '' && $prevresponse != '') || count($prevresponse) != count($newresponse))

          { return false ; }

          foreach($newresponse as $arraykey => $value){
          if( ! array_key_exists($arraykey, $prevresponse) || $value !== $prevresponse[$arraykey])

          { return false ; }

          }
          return true ;
          }
          incidently in multichoice/question.php, this seems more as an is _complete then a is_same ?

          public function is_same_response(array $prevresponse, array $newresponse) {
          $same = true;
          foreach ($this->order as $key => $notused)

          { $fieldname = $this->field($key); $same = $same && array_key_exists($fieldname, $newresponse) == array_key_exists($fieldname, $prevresponse) && (!array_key_exists($fieldname, $prevresponse) || $newresponse[$fieldname] == $prevresponse[$fieldname]); }

          return $same;
          }

          Show
          Pierre Pichet added a comment - I need an universal is_same_response(array ($prevresponse, array $newresponse) Could this do the job? public function is_same_response(array $prevresponse, array $newresponse) { if ( ($newresponse == '' && $prevresponse != '') || count($prevresponse) != count($newresponse)) { return false ; } foreach($newresponse as $arraykey => $value){ if( ! array_key_exists($arraykey, $prevresponse) || $value !== $prevresponse [$arraykey] ) { return false ; } } return true ; } incidently in multichoice/question.php, this seems more as an is _complete then a is_same ? public function is_same_response(array $prevresponse, array $newresponse) { $same = true; foreach ($this->order as $key => $notused) { $fieldname = $this->field($key); $same = $same && array_key_exists($fieldname, $newresponse) == array_key_exists($fieldname, $prevresponse) && (!array_key_exists($fieldname, $prevresponse) || $newresponse[$fieldname] == $prevresponse[$fieldname]); } return $same; }
          Hide
          Tim Hunt added a comment -

          No, that is definitely an is_same method. It has to check that the arrays have the same keys, and then if they do, that the values are the same. I agree it is not the most readable code I have ever written.

          Can't multianswer do is_same by looping over the subquestions and asking each of them if the response is the same, and &&ing together all the results?

          Also, just to warn you, I just pushed quite a lot of changes to github. In particular, questions now need to implement summarise_response, and get_question_summary. That is to make the reports nicer.

          Show
          Tim Hunt added a comment - No, that is definitely an is_same method. It has to check that the arrays have the same keys, and then if they do, that the values are the same. I agree it is not the most readable code I have ever written. Can't multianswer do is_same by looping over the subquestions and asking each of them if the response is the same, and &&ing together all the results? Also, just to warn you, I just pushed quite a lot of changes to github. In particular, questions now need to implement summarise_response, and get_question_summary. That is to make the reports nicer.
          Hide
          Pierre Pichet added a comment -

          I just fully understand yesterday when adressing code for grading that I need to rewrite each functions used by the four main question types I want to used in cloze question type (i.e short, numerical, multichoicesingle and multichocie multi).
          In short, numerical, multichoicesingle answer is named answer and each time I have to change it to _sub1_answer because 'answer' is written as hardcode .
          In multichoice multi answer where choice is a parameter you defined this differently as
          $fieldname = $this->field($key);

          I think that this convention should be use everywhere in the question engine to be able to use other question type in a given question type as I am doing .

          Using sub questions the $fieldname need also a prefix identifier.
          These necessary parameer should be defined

          This will give us a more complex but more flexible code for future questions (how about random a match code ?)

          I will experiment by first changing the multiplechoice simple code to these convention and create the necessary code in multianswer to handle the two multiple choice question types.

          Unless you have a better solution.

          Show
          Pierre Pichet added a comment - I just fully understand yesterday when adressing code for grading that I need to rewrite each functions used by the four main question types I want to used in cloze question type (i.e short, numerical, multichoicesingle and multichocie multi). In short, numerical, multichoicesingle answer is named answer and each time I have to change it to _sub1_answer because 'answer' is written as hardcode . In multichoice multi answer where choice is a parameter you defined this differently as $fieldname = $this->field($key); I think that this convention should be use everywhere in the question engine to be able to use other question type in a given question type as I am doing . Using sub questions the $fieldname need also a prefix identifier. These necessary parameer should be defined This will give us a more complex but more flexible code for future questions (how about random a match code ?) I will experiment by first changing the multiplechoice simple code to these convention and create the necessary code in multianswer to handle the two multiple choice question types. Unless you have a better solution.
          Hide
          Tim Hunt added a comment -

          Can't grade_response do something like

          public funcition grade_response(array $response) {
              // Split responses into the separate bits for each question.
              $subresponses = array();
              foreach ($response as $key => $value) {
                  if (preg_match('/^sub(\d+)_(.*)$', $key, $matches)) {
                      $subresponses[$matches[1]][$matches[2]] = $value;
                  }
              }
              
              // Now grade each bit and  combine.
              foreach ($this->subquestions as $i => $subq) {
                  if (!empty($subresponses[$i])) {
                      $result = $subq->grade_responses($subresponses[$i]);
                      // Combine $result from each subquestion some how ...
                  }
              }
          
              return ...
          }
          

          Actually, if (!empty($subresponses[$i])) is not quite right. It should probably call $subq->is_gradable_response(), and I don't know exactly how to combine the separate results for each part.

          Show
          Tim Hunt added a comment - Can't grade_response do something like public funcition grade_response(array $response) { // Split responses into the separate bits for each question. $subresponses = array(); foreach ($response as $key => $value) { if (preg_match('/^sub(\d+)_(.*)$', $key, $matches)) { $subresponses[$matches[1]][$matches[2]] = $value; } } // Now grade each bit and combine. foreach ($ this ->subquestions as $i => $subq) { if (!empty($subresponses[$i])) { $result = $subq->grade_responses($subresponses[$i]); // Combine $result from each subquestion some how ... } } return ... } Actually, if (!empty($subresponses [$i] )) is not quite right. It should probably call $subq->is_gradable_response(), and I don't know exactly how to combine the separate results for each part.
          Hide
          Pierre Pichet added a comment -

          For is_same to compare two arrays we can build a general solution.

          The problem is related to the hard coding of 'answer' which should use the more general flexible class passing parameters. Renderer needs them also.
          You had the same problem with multichoice multi answer where the field is 'choice' not 'answer'.

          We shoud use the same solution all across the question engine using public parameter and functions like
          public $fieldid = '' ; // to be filled by the question (multianswer) using this question (multiplechoice)
          public $fieldname = 'answer';
          // public $identifiedfieldname = $fieldid.$fieldname ;
          public function field($key)

          { return $this->fieldid.$this->fieldname.$key; }

          Not necessary to say that I have try it and it works

          As you are the expert, define which names are the best .

          It's somewhat a pain is the ass to do all the changes but they are needed at least for multianswer...

          We could also standardize the subquestions names and $fielidid ( sub.subquestion[$key])

          Show
          Pierre Pichet added a comment - For is_same to compare two arrays we can build a general solution. The problem is related to the hard coding of 'answer' which should use the more general flexible class passing parameters. Renderer needs them also. You had the same problem with multichoice multi answer where the field is 'choice' not 'answer'. We shoud use the same solution all across the question engine using public parameter and functions like public $fieldid = '' ; // to be filled by the question (multianswer) using this question (multiplechoice) public $fieldname = 'answer'; // public $identifiedfieldname = $fieldid.$fieldname ; public function field($key) { return $this->fieldid.$this->fieldname.$key; } Not necessary to say that I have try it and it works As you are the expert, define which names are the best . It's somewhat a pain is the ass to do all the changes but they are needed at least for multianswer... We could also standardize the subquestions names and $fielidid ( sub.subquestion [$key] )
          Hide
          Pierre Pichet added a comment -

          This will suffice is the question field is exchanged is only answer.
          However other parameters are necessary to exchange and stored in the attempt (example is 'order' on shuffled questions) .
          I had also to use the _sub idea on order . I need to look more closely for this parameter to see what happen when I have more than one multichoice in the multianswer question...

          Show
          Pierre Pichet added a comment - This will suffice is the question field is exchanged is only answer. However other parameters are necessary to exchange and stored in the attempt (example is 'order' on shuffled questions) . I had also to use the _sub idea on order . I need to look more closely for this parameter to see what happen when I have more than one multichoice in the multianswer question...
          Hide
          Pierre Pichet added a comment -

          I will do the changes for the three question types implied (i.e. shortanswer,numerical and multichoice) and give you the results.
          I understand that or other parameters like order only the $fieldid is necessary as order is used in a common way between questions so multianswer can handle it with a common function.
          For grading, the 'answer' is not the same and not used in a common way, so we need to pass the parameter by field() .

          Show
          Pierre Pichet added a comment - I will do the changes for the three question types implied (i.e. shortanswer,numerical and multichoice) and give you the results. I understand that or other parameters like order only the $fieldid is necessary as order is used in a common way between questions so multianswer can handle it with a common function. For grading, the 'answer' is not the same and not used in a common way, so we need to pass the parameter by field() .
          Hide
          Pierre Pichet added a comment -

          Changes done on shortasnwer and regular shortanswer is working normally at least for deffered feedback.

          The task is quite easy to do once, as usual, you know where to do it

          A very god way to learn about the question engine...

          Show
          Pierre Pichet added a comment - Changes done on shortasnwer and regular shortanswer is working normally at least for deffered feedback. The task is quite easy to do once, as usual, you know where to do it A very god way to learn about the question engine...
          Hide
          Tim Hunt added a comment -

          Sorry for not replying sooner, before you did the work. Busy morning.

          I don't think we should have to change the names the other question types use for their answer variables. That breaks encapsulation is a way that feels wrong to me.

          We should not have to change the other question classes very much to support multianswer. But I had not previously thought about the way init_first_step needs to set things. I now think that the right solution is to build a temporary wrapper round question_attempt_step the modifies access to the data values, to add the prefix, like this:

          /**
           * This is an adapter class that wraps a {@link question_attempt_step} and
           * modifies the get/set_*_data methods so that they operate only on the parts
           * that belong to a particular subquestion, as indicated by an extra prefix.
           *
           * @copyright © 2010 The Open University
           * @license http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
           */
          class question_attempt_step_subquestion_adapter extends question_attempt_step {
              /** @var question_attempt_step the step we are wrapping. */
              protected $realqas;
              /** @var string the exta prefix on fields we work with. */
              protected $extraprefix;
          
              /**
               * 
               * @param question_attempt_step $realqas the step to wrap.
               * @param unknown_type $extraprefix the extra prefix that is used for date fields.
               */
              public function __construct(question_attempt_step $realqas, $extraprefix) {
                  $this->realqas = $realqas;
                  $this->extraprefix = $extraprefix;
              }
          
              /**
               * Add the extra prefix to a field name.
               * @param string $field the plain field name.
               * @return string the field name with the extra bit of prefix added.
               */
              protected function adjust_field_name($field) {
                  if (substr($field, 0, 2) == '!_') {
                      return '!_' . $this->extraprefix . substr($field, 2);
                  } else if (substr($field, 0, 2) == '!') {
                      return '!' . $this->extraprefix . substr($field, 1);
                  } else if (substr($field, 0, 2) == '_') {
                      return '_' . $this->extraprefix . substr($field, 1);
                  } else {
                      return $this->extraprefix . $field;
                  }
              }
          
              public function get_state() {
                  return $this->realqas->get_state();
              }
          
              public function set_state($state) {
                  throw new Exception('Cannot modify a question_attempt_step_read_only.');
              }
          
              public function get_fraction() {
                  return $this->realqas->get_fraction();
              }
          
              public function set_fraction($fraction) {
                  throw new Exception('Cannot modify a question_attempt_step_read_only.');
              }
          
              public function get_user_id() {
                  return $this->realqas->get_user_id;
              }
          
              public function get_timecreated() {
                  return $this->realqas->get_timecreated();
              }
          
              public function has_qt_var($name) {
                  return $this->realqas->has_qt_var($this->adjust_field_name($name));
              }
          
              public function get_qt_var($name) {
                  return $this->realqas->get_qt_var($this->adjust_field_name($name));
              }
          
              public function set_qt_var($name, $value) {
                  return $this->realqas->set_qt_var($this->adjust_field_name($name), $value);
              }
          
              public function get_qt_data() {
                  $regex = '/^' . $this->extraprefix . '[^!]/';
                  $prefixlen = strlen($this->extraprefix);
                  $result = array();
                  foreach ($this->data as $name => $value) {
                      if (preg_match($regex, $name)) {
                          $result[substr($name, $prefixlen)] = $value;
                      }
                  }
                  return $result;
              }
          
              public function has_im_var($name) {
                  return $this->realqas->has_im_var($this->adjust_field_name($name));
              }
          
              public function get_im_var($name) {
                  return $this->realqas->get_im_var($this->adjust_field_name($name));
              }
          
              public function set_im_var($name, $value) {
                  return $this->realqas->set_im_var($this->adjust_field_name($name), $value);
              }
          
              public function get_im_data() {
                  $regex = '/^' . $this->extraprefix . '!/';
                  $prefixlen = strlen($this->extraprefix) + 1;
                  $result = array();
                  foreach ($this->data as $name => $value) {
                      if (preg_match($regex, $name)) {
                          $result[substr($name, $prefixlen)] = $value;
                      }
                  }
                  return $result;
              }
          
              public function get_submitted_data() {
                  $regex = '/^' . $this->extraprefix . '(?!!_|_)(.*)$/';
                  $prefixlen = strlen($this->extraprefix);
                  $result = array();
                  foreach ($this->data as $name => $value) {
                      if (preg_match($regex, $name)) {
                          $result[substr($name, $prefixlen)] = $value;
                      }
                  }
                  return $result;
              }
          
              public function get_all_data() {
                  $regex = '/^' . $this->extraprefix . '(?!!_|_)(.*)$/';
                  $result = array();
                  foreach ($this->data as $name => $value) {
                      if (strpos($name, $this->extraprefix) === 0) {
                          $result[substr($name, $prefixlen)] = $value;
                      }
                  }
                  return $result;
              }
          }
          

          Then you can do

          public function init_first_step(question_attempt_step $step) {
              foreach ($this->subquestions as $i => $subq) {
                  $substep = new question_attempt_step_subquestion_adapter($step, 'sub' . $i);
                  $subq->init_first_step($substep);
              }
          }
          

          and it should all magically work.

          Show
          Tim Hunt added a comment - Sorry for not replying sooner, before you did the work. Busy morning. I don't think we should have to change the names the other question types use for their answer variables. That breaks encapsulation is a way that feels wrong to me. We should not have to change the other question classes very much to support multianswer. But I had not previously thought about the way init_first_step needs to set things. I now think that the right solution is to build a temporary wrapper round question_attempt_step the modifies access to the data values, to add the prefix, like this: /** * This is an adapter class that wraps a {@link question_attempt_step} and * modifies the get/set_*_data methods so that they operate only on the parts * that belong to a particular subquestion, as indicated by an extra prefix. * * @copyright © 2010 The Open University * @license http: //www.gnu.org/copyleft/gpl.html GNU GPL v3 or later */ class question_attempt_step_subquestion_adapter extends question_attempt_step { /** @ var question_attempt_step the step we are wrapping. */ protected $realqas; /** @ var string the exta prefix on fields we work with. */ protected $extraprefix; /** * * @param question_attempt_step $realqas the step to wrap. * @param unknown_type $extraprefix the extra prefix that is used for date fields. */ public function __construct(question_attempt_step $realqas, $extraprefix) { $ this ->realqas = $realqas; $ this ->extraprefix = $extraprefix; } /** * Add the extra prefix to a field name. * @param string $field the plain field name. * @ return string the field name with the extra bit of prefix added. */ protected function adjust_field_name($field) { if (substr($field, 0, 2) == '!_') { return '!_' . $ this ->extraprefix . substr($field, 2); } else if (substr($field, 0, 2) == '!') { return '!' . $ this ->extraprefix . substr($field, 1); } else if (substr($field, 0, 2) == '_') { return '_' . $ this ->extraprefix . substr($field, 1); } else { return $ this ->extraprefix . $field; } } public function get_state() { return $ this ->realqas->get_state(); } public function set_state($state) { throw new Exception('Cannot modify a question_attempt_step_read_only.'); } public function get_fraction() { return $ this ->realqas->get_fraction(); } public function set_fraction($fraction) { throw new Exception('Cannot modify a question_attempt_step_read_only.'); } public function get_user_id() { return $ this ->realqas->get_user_id; } public function get_timecreated() { return $ this ->realqas->get_timecreated(); } public function has_qt_var($name) { return $ this ->realqas->has_qt_var($ this ->adjust_field_name($name)); } public function get_qt_var($name) { return $ this ->realqas->get_qt_var($ this ->adjust_field_name($name)); } public function set_qt_var($name, $value) { return $ this ->realqas->set_qt_var($ this ->adjust_field_name($name), $value); } public function get_qt_data() { $regex = '/^' . $ this ->extraprefix . '[^!]/'; $prefixlen = strlen($ this ->extraprefix); $result = array(); foreach ($ this ->data as $name => $value) { if (preg_match($regex, $name)) { $result[substr($name, $prefixlen)] = $value; } } return $result; } public function has_im_var($name) { return $ this ->realqas->has_im_var($ this ->adjust_field_name($name)); } public function get_im_var($name) { return $ this ->realqas->get_im_var($ this ->adjust_field_name($name)); } public function set_im_var($name, $value) { return $ this ->realqas->set_im_var($ this ->adjust_field_name($name), $value); } public function get_im_data() { $regex = '/^' . $ this ->extraprefix . '!/'; $prefixlen = strlen($ this ->extraprefix) + 1; $result = array(); foreach ($ this ->data as $name => $value) { if (preg_match($regex, $name)) { $result[substr($name, $prefixlen)] = $value; } } return $result; } public function get_submitted_data() { $regex = '/^' . $ this ->extraprefix . '(?!!_|_)(.*)$/'; $prefixlen = strlen($ this ->extraprefix); $result = array(); foreach ($ this ->data as $name => $value) { if (preg_match($regex, $name)) { $result[substr($name, $prefixlen)] = $value; } } return $result; } public function get_all_data() { $regex = '/^' . $ this ->extraprefix . '(?!!_|_)(.*)$/'; $result = array(); foreach ($ this ->data as $name => $value) { if (strpos($name, $ this ->extraprefix) === 0) { $result[substr($name, $prefixlen)] = $value; } } return $result; } } Then you can do public function init_first_step(question_attempt_step $step) { foreach ($ this ->subquestions as $i => $subq) { $substep = new question_attempt_step_subquestion_adapter($step, 'sub' . $i); $subq->init_first_step($substep); } } and it should all magically work.
          Hide
          Pierre Pichet added a comment -

          Your "attempt_step_subquestion" is a variant of one of my suggestions to create attempts using a parent field like we have parent question.
          So I will try it to make it "magically" works which again remembers me the sorcerer's apprentice

          However the hard coding of 'answer' is not optimal for future developments.
          I think that the solution used in multiplechoice multi answers should be generalyzed.

          Let's back to work and the 2 hours needed to test my first hypothesis will be usefull to continue

          Show
          Pierre Pichet added a comment - Your "attempt_step_subquestion" is a variant of one of my suggestions to create attempts using a parent field like we have parent question. So I will try it to make it "magically" works which again remembers me the sorcerer's apprentice However the hard coding of 'answer' is not optimal for future developments. I think that the solution used in multiplechoice multi answers should be generalyzed. Let's back to work and the 2 hours needed to test my first hypothesis will be usefull to continue
          Hide
          Tim Hunt added a comment -

          I disagree. One of the aims of the new system is to make it as easy as possible for people creating a new question type. They should be able to do simple things like decide to call their answer field 'answer', and have the system take care of adding a prefix at the front to make it unique.

          Show
          Tim Hunt added a comment - I disagree. One of the aims of the new system is to make it as easy as possible for people creating a new question type. They should be able to do simple things like decide to call their answer field 'answer', and have the system take care of adding a prefix at the front to make it unique.
          Hide
          Pierre Pichet added a comment -

          I like your disagrement because it means that even if you are a great computer expert,
          you are concern by users for which effectively a parameter called 'answer' has a common sense.

          Efficient code can sometimes be very obscure like a regexp expression...

          Show
          Pierre Pichet added a comment - I like your disagrement because it means that even if you are a great computer expert, you are concern by users for which effectively a parameter called 'answer' has a common sense. Efficient code can sometimes be very obscure like a regexp expression...
          Hide
          Pierre Pichet added a comment -

          I put your code and it seems to work as best as the one I have already set.
          Its ok to fill the correct answer, to display the correct color of the short and numerical related to corrcect or not (green and red) and multichoice radio but cannot set correctly the select element although it is set correctly when filled with the correct values.
          In any case the grading process remain to be solved.
          Its almost midnight so monday for the next news...

          Show
          Pierre Pichet added a comment - I put your code and it seems to work as best as the one I have already set. Its ok to fill the correct answer, to display the correct color of the short and numerical related to corrcect or not (green and red) and multichoice radio but cannot set correctly the select element although it is set correctly when filled with the correct values. In any case the grading process remain to be solved. Its almost midnight so monday for the next news...
          Hide
          Pierre Pichet added a comment -

          The sorcerer's apprentice is very sad ;( .
          The sorcerer's recipe is not used at all

          So I will look deeper inside the question engine to understand why.
          This is becoming the trail to complete my appenticeship

          Show
          Pierre Pichet added a comment - The sorcerer's apprentice is very sad ;( . The sorcerer's recipe is not used at all So I will look deeper inside the question engine to understand why. This is becoming the trail to complete my appenticeship
          Hide
          Pierre Pichet added a comment -

          My actual understanding is the following
          We need to encapsulate the subquestionsstep data in the attempt data and your suggestion is one way to do it.

          We need to encapsulate the subquestions data in the question form so we need to create new renderer which need their subquestion (index).. My actual code is one way to do it as it wokrs correctly.
          The remainig problem is subquestion data that will come from the attempt step data and use in the various multianswer functions.
          first we an use a general function like the is same I have created.
          or
          decapsulate the subquestion attempt data before sending them to their specific subquestions

          and give a global result as return but this will not always work as the function that use the return i.e grade is not set necessary able to handle this global return . it has been build to handle individual question data.

          So we need to know precisely the code flow that control the question->function calls.

          This code flow ignores that the return data contain subquestions datas.

          I am studying the code flow of deferred feedback to understand these constraints.

          Show
          Pierre Pichet added a comment - My actual understanding is the following We need to encapsulate the subquestionsstep data in the attempt data and your suggestion is one way to do it. We need to encapsulate the subquestions data in the question form so we need to create new renderer which need their subquestion (index).. My actual code is one way to do it as it wokrs correctly. The remainig problem is subquestion data that will come from the attempt step data and use in the various multianswer functions. first we an use a general function like the is same I have created. or decapsulate the subquestion attempt data before sending them to their specific subquestions and give a global result as return but this will not always work as the function that use the return i.e grade is not set necessary able to handle this global return . it has been build to handle individual question data. So we need to know precisely the code flow that control the question->function calls. This code flow ignores that the return data contain subquestions datas. I am studying the code flow of deferred feedback to understand these constraints.
          Hide
          Pierre Pichet added a comment -

          To illustrat the problem
          Actually I am stop at the finish process ,
          if ($this->qa->get_state()->is_finished())

          { return question_attempt::DISCARD; }

          $response = $this->qa->get_last_step()->get_qt_data();
          if (!$this->question->is_gradable_response($response))

          { $pendingstep->set_state(question_state::$gaveup); }

          else

          { list($fraction, $state) = $this->question->grade_response($response); $pendingstep->set_fraction($fraction); $pendingstep->set_state($state); }

          and inside questionbase
          is_gradable_response($response) has a specific meaning

          abstract class question_graded_automatically extends question_with_responses
          implements question_automatically_gradable {
          public function is_gradable_response(array $response)

          { return $this->is_complete_response($response); }

          and the I have to check if it is the same in
          class qtype_shortanswer_question extends question_graded_by_strategy

          And always the same sequence in different model
          for the actual three main question types in multianswer...

          a good exam training

          Show
          Pierre Pichet added a comment - To illustrat the problem Actually I am stop at the finish process , if ($this->qa->get_state()->is_finished()) { return question_attempt::DISCARD; } $response = $this->qa->get_last_step()->get_qt_data(); if (!$this->question->is_gradable_response($response)) { $pendingstep->set_state(question_state::$gaveup); } else { list($fraction, $state) = $this->question->grade_response($response); $pendingstep->set_fraction($fraction); $pendingstep->set_state($state); } and inside questionbase is_gradable_response($response) has a specific meaning abstract class question_graded_automatically extends question_with_responses implements question_automatically_gradable { public function is_gradable_response(array $response) { return $this->is_complete_response($response); } and the I have to check if it is the same in class qtype_shortanswer_question extends question_graded_by_strategy And always the same sequence in different model for the actual three main question types in multianswer... a good exam training
          Hide
          Pierre Pichet added a comment -

          The suggested
          if (preg_match('/^_sub([0-9])-(.*)/', $key, $matches))

          { $subresponses[$matches[1]][$matches[2]] = $value; }

          }
          is working at least to is_complete

          Show
          Pierre Pichet added a comment - The suggested if (preg_match('/^_sub( [0-9] )-(.*)/', $key, $matches)) { $subresponses[$matches[1]][$matches[2]] = $value; } } is working at least to is_complete
          Hide
          Pierre Pichet added a comment -

          I have solved the problem of calculate the fraction related to the grade_response() and stored the individual fraction and state so that I can use them in the display.
          I can return the total fraction weighted with the individual max grade of the subquestions.
          However what will be the total state ??

          Show
          Pierre Pichet added a comment - I have solved the problem of calculate the fraction related to the grade_response() and stored the individual fraction and state so that I can use them in the display. I can return the total fraction weighted with the individual max grade of the subquestions. However what will be the total state ??
          Hide
          Pierre Pichet added a comment - - edited

          screen shot 2
          grading and rendering of multiple shorts
          subquestion grades are 1, 2, 3, 4 total = 10
          This is working but the actual code is tricky...

          Show
          Pierre Pichet added a comment - - edited screen shot 2 grading and rendering of multiple shorts subquestion grades are 1, 2, 3, 4 total = 10 This is working but the actual code is tricky...
          Hide
          Pierre Pichet added a comment -

          Grading of numerical is working.

          Show
          Pierre Pichet added a comment - Grading of numerical is working.
          Hide
          Pierre Pichet added a comment -

          Shortanswer, numerical and multichoice inline grading done.
          other multichoices types then finish feedback and pop-up.

          Show
          Pierre Pichet added a comment - Shortanswer, numerical and multichoice inline grading done. other multichoices types then finish feedback and pop-up.
          Hide
          Pierre Pichet added a comment -

          In your model if a question is not complete the grading is not done.
          In actual multianswer this is not the case. You have a grade even if all your questions (or subquestions) are not completed.
          Should I modify the is_complete logic so that it is true (for the global multianswer) if at least one question is complete?

          Show
          Pierre Pichet added a comment - In your model if a question is not complete the grading is not done. In actual multianswer this is not the case. You have a grade even if all your questions (or subquestions) are not completed. Should I modify the is_complete logic so that it is true (for the global multianswer) if at least one question is complete?
          Hide
          Pierre Pichet added a comment -

          So we need two parameter types one related to the all question is completed and this subquestion is completed.
          In either we can grade (or not) or do special things related to the inteactions...

          Show
          Pierre Pichet added a comment - So we need two parameter types one related to the all question is completed and this subquestion is completed. In either we can grade (or not) or do special things related to the inteactions...
          Hide
          Pierre Pichet added a comment -

          Other multichoices grading seems OK,

          Show
          Pierre Pichet added a comment - Other multichoices grading seems OK,
          Hide
          Tim Hunt added a comment -

          The theory with complete/gradable is this:

          A. Complete is used in places like the summary page. The aim there is to show a student if they still have work to do, so a question is complete when all parts are done. (An example of this in another question type is matching.)

          B. If, however, a student submits a question with some parts done, and some parts not done, we should still grade the parts they have sumbmitted (and the other parts will have to be graded wrong.)

          C. However, if a student does not answer any part of the question, we want to record that they did not attempt the question at all (question_state::$gaveup).

          So, those are the three states to distinguish:
          A. All parts done, is_complete_response = true, is_gradeable_response = true.
          B. Some parts done, is_complete_response = false, is_gradeable_response = true.
          A. No parts done, is_complete_response = false, is_gradeable_response = false.

          And, as I say, it should work just like matching.

          There is one state, as in $qa->get_state() for the whole question. We do not track the state for sub-parts.

          I suppose that means that for multianswer, is_complete for the whole question is true only if all sub-part are complete; and the whole question is gradeable if any sub-part is.

          Show
          Tim Hunt added a comment - The theory with complete/gradable is this: A. Complete is used in places like the summary page. The aim there is to show a student if they still have work to do, so a question is complete when all parts are done. (An example of this in another question type is matching.) B. If, however, a student submits a question with some parts done, and some parts not done, we should still grade the parts they have sumbmitted (and the other parts will have to be graded wrong.) C. However, if a student does not answer any part of the question, we want to record that they did not attempt the question at all (question_state::$gaveup). So, those are the three states to distinguish: A. All parts done, is_complete_response = true, is_gradeable_response = true. B. Some parts done, is_complete_response = false, is_gradeable_response = true. A. No parts done, is_complete_response = false, is_gradeable_response = false. And, as I say, it should work just like matching. There is one state, as in $qa->get_state() for the whole question. We do not track the state for sub-parts. I suppose that means that for multianswer, is_complete for the whole question is true only if all sub-part are complete; and the whole question is gradeable if any sub-part is.
          Hide
          Pierre Pichet added a comment - - edited

          You clarify the situation very well .
          However in the different interaction models this could mean that any meanfull feedback (i.e. adaptative) will not be availabel until ALL the subquestions has been completed or are these feedback available when the state is is_gradeable_response = true.?
          Or I can use match as an example ?

          Show
          Pierre Pichet added a comment - - edited You clarify the situation very well . However in the different interaction models this could mean that any meanfull feedback (i.e. adaptative) will not be availabel until ALL the subquestions has been completed or are these feedback available when the state is is_gradeable_response = true.? Or I can use match as an example ?
          Hide
          Tim Hunt added a comment -

          Use match as an example.

          When the student takes a definite action like clicking a Submit button, it uses is_gradeable_response to decide whether it should do anything.

          Show
          Tim Hunt added a comment - Use match as an example. When the student takes a definite action like clicking a Submit button, it uses is_gradeable_response to decide whether it should do anything.
          Hide
          Pierre Pichet added a comment - - edited

          Ok I understand the parrallel between multianswer and match.
          The two question types have subquestions, match is more simpler has the subquestions are all multiple choice so the code can assume what will be the various functions (i.e _gradeable_response) and code them directly in match.

          In multianswer I must do a call to say the subquestion->gradeable_response() without assuming anything about the result .
          and handle the result in the same way.
          i.e. there should as many results as they are subquestions in the multianswer question.

          For the first functions like public function get_correct_response() {
          the return is an array and the code handling this return in the interaction models take account that it is an array of a given kind.
          but
          public function is_complete_response(array $response) {
          $complete = true;
          foreach ($this->stemorder as $key => $stemid)

          { $complete = $complete && !empty($response[$this->field($key)]); }

          return $complete;
          }
          the return $complete is a bool not an array of bools (for each subquestions).

          So let me follow the trails of these bool array returns to see how to handle them...

          I want a solution that can also handle multichoice multi answer but perhaps I am too ambitious although I think that this is a good test of the engin design flexibility.
          However I think that I am quite near the solution. everything is OK, the only things that remain is to control the grading or not of each subquestion

          i.e. is_complete_response(),is_gradable_response() ,
          however lines like these in adaptative model code which are outside the question.php code
          public function process_submit(question_attempt_step $pendingstep) {
          $status = $this->process_save($pendingstep);

          $response = $pendingstep->get_qt_data();

          $response = $pendingstep->get_qt_data();
          if (!$this->question->is_gradable_response($response)) {
          $pendingstep->set_state(question_state::$invalid);
          if ($this->qa->get_state() != question_state::$invalid)

          { $status = question_attempt::KEEP; }

          return $status;
          }
          worry me as the same is_gradable_response() is used for example in match code
          public function get_validation_error(array $response) {
          if ($this->is_gradable_response($response))

          { return ''; }

          return get_string('youmustselectananswer', 'qtype_multichoice');
          }
          where I can set new code to handle the multiple array bool return from is_gradable_response()...

          Is there something I don't catch ?

          Some news monday....

          Show
          Pierre Pichet added a comment - - edited Ok I understand the parrallel between multianswer and match. The two question types have subquestions, match is more simpler has the subquestions are all multiple choice so the code can assume what will be the various functions (i.e _gradeable_response) and code them directly in match. In multianswer I must do a call to say the subquestion->gradeable_response() without assuming anything about the result . and handle the result in the same way. i.e. there should as many results as they are subquestions in the multianswer question. For the first functions like public function get_correct_response() { the return is an array and the code handling this return in the interaction models take account that it is an array of a given kind. but public function is_complete_response(array $response) { $complete = true; foreach ($this->stemorder as $key => $stemid) { $complete = $complete && !empty($response[$this->field($key)]); } return $complete; } the return $complete is a bool not an array of bools (for each subquestions). So let me follow the trails of these bool array returns to see how to handle them... I want a solution that can also handle multichoice multi answer but perhaps I am too ambitious although I think that this is a good test of the engin design flexibility. However I think that I am quite near the solution. everything is OK, the only things that remain is to control the grading or not of each subquestion i.e. is_complete_response(),is_gradable_response() , however lines like these in adaptative model code which are outside the question.php code public function process_submit(question_attempt_step $pendingstep) { $status = $this->process_save($pendingstep); $response = $pendingstep->get_qt_data(); $response = $pendingstep->get_qt_data(); if (!$this->question->is_gradable_response($response)) { $pendingstep->set_state(question_state::$invalid); if ($this->qa->get_state() != question_state::$invalid) { $status = question_attempt::KEEP; } return $status; } worry me as the same is_gradable_response() is used for example in match code public function get_validation_error(array $response) { if ($this->is_gradable_response($response)) { return ''; } return get_string('youmustselectananswer', 'qtype_multichoice'); } where I can set new code to handle the multiple array bool return from is_gradable_response()... Is there something I don't catch ? Some news monday....
          Hide
          Pierre Pichet added a comment -

          One solution will be to store these subquestion bool returns somewhere and just return a single bool as the model expect.
          Store them in the step data or in the subquestions i.e
          $questiontot = $qa->get_question();
          $question = $questiontot->subquestions[$qa->subquestionindex];

          Show
          Pierre Pichet added a comment - One solution will be to store these subquestion bool returns somewhere and just return a single bool as the model expect. Store them in the step data or in the subquestions i.e $questiontot = $qa->get_question(); $question = $questiontot->subquestions [$qa->subquestionindex] ;
          Hide
          Pierre Pichet added a comment - - edited

          As far as I can see is_gradable_response is defined in type/questionbase.php

          abstract class question_graded_automatically extends question_with_responses
          implements question_automatically_gradable {
          public function is_gradable_response(array $response)

          { return $this->is_complete_response($response); }
          and in multichoice it is also defined as
          public function is_gradable_response(array $response) { return $this->is_complete_response($response); }

          in match it is defined as
          public function is_gradable_response(array $response) {
          foreach ($this->stemorder as $key => $stemid) {
          if (!empty($response[$this->field($key)]))

          { return true; }

          }
          return false;
          }

          The multianswer just need to handle as the actual questiontypes (except match) handle it i.e do nothing special as multianswer->is_complete_response() is working correctly.

          What I did not understand was that the rendering also give feedback on empty responses.
          Then I found in match renderer.php that the is_complete() function equivalent test by hard coding

          foreach ($stemorder as $key => $stemid) {
          ...
          if (array_key_exists($fieldname, $response))

          { $selected = $response[$fieldname]; }

          else

          { $selected = 0; }

          $selected being the equivalent of is_complete() internal test.
          Tis can be done as match/renderer knows the question structure.

          Given the actual questions we want to use in multiasnwer, using the is_gradable_response() to control the display inside each subquestion renderer should be the solution at least for single response types and creating a similar $stemorder loop than match for multanswer multichoices

          Show
          Pierre Pichet added a comment - - edited As far as I can see is_gradable_response is defined in type/questionbase.php abstract class question_graded_automatically extends question_with_responses implements question_automatically_gradable { public function is_gradable_response(array $response) { return $this->is_complete_response($response); } and in multichoice it is also defined as public function is_gradable_response(array $response) { return $this->is_complete_response($response); } in match it is defined as public function is_gradable_response(array $response) { foreach ($this->stemorder as $key => $stemid) { if (!empty($response [$this->field($key)] )) { return true; } } return false; } The multianswer just need to handle as the actual questiontypes (except match) handle it i.e do nothing special as multianswer->is_complete_response() is working correctly. What I did not understand was that the rendering also give feedback on empty responses. Then I found in match renderer.php that the is_complete() function equivalent test by hard coding foreach ($stemorder as $key => $stemid) { ... if (array_key_exists($fieldname, $response)) { $selected = $response[$fieldname]; } else { $selected = 0; } $selected being the equivalent of is_complete() internal test. Tis can be done as match/renderer knows the question structure. Given the actual questions we want to use in multiasnwer, using the is_gradable_response() to control the display inside each subquestion renderer should be the solution at least for single response types and creating a similar $stemorder loop than match for multanswer multichoices
          Hide
          Pierre Pichet added a comment -

          Preliminary tests seem OK.

          Show
          Pierre Pichet added a comment - Preliminary tests seem OK.
          Hide
          Tim Hunt added a comment -

          The code in the match renderer is just getting the current selected value for the choice, so it can display what the student selected. It is not re-implementing is_complete.

          Handling multichoice multianswer is a good test of the new system. (multichoice multianswer is a bit like match too.)

          Anyway, it sounds like you nearly have things working. I would be interested to see your code some time.

          Show
          Tim Hunt added a comment - The code in the match renderer is just getting the current selected value for the choice, so it can display what the student selected. It is not re-implementing is_complete. Handling multichoice multianswer is a good test of the new system. (multichoice multianswer is a bit like match too.) Anyway, it sounds like you nearly have things working. I would be interested to see your code some time.
          Hide
          Pierre Pichet added a comment - - edited

          "I would be interested to see your code some time."

          I have set in questionbase.php so it appears anywhere just in case.
          public $fieldid = '';

          public $fieldname = 'answer';

          public function field($key = '')

          { return $this->fieldid.$this->fieldname.$key; }

          and change all 'answer' to
          $answername = $this->field('');
          return array($answername => $answer->answer);

          I set back to 'answer' in all functions that have been changed like is_complete().
          here a cleaned but not finished example

            public function grade_response(array $response) {
              $subresponses = array();
              foreach ($response as $key => $value) {
               if (preg_match('/^_sub(\d+)-(.*)/', $key, $matches)) {
                      $subresponses[$matches[1]][$matches[2]] = $value;
                  }
              }
              
              // Now grade each bit and  combine.
              $totfraction = 0 ;
              foreach ($this->subquestions as $i => $subq) { // subquestion
                   if (!empty($subresponses[$i])) {
                      $res = $subq->grade_response($subresponses[$i]); 
                      $totfraction += $res[0] * $subq->defaultmark/$this->defaultmark ;
                  }
             }
                  return array($totfraction, question_state::graded_state_for_fraction($totfraction));
              }
          
          

          As I need the subquestion grade in the renderer (this (i.e. pop-up window for inline ) is not done yet), I have to choose between
          store the $res as qa->question->subquestions[$i]->$calculated_grade to retrieve it in the renderer code
          or to grade_response() again when rendering.

          When everything will work correctly, I will set back to 'answer' everywhere it is unnecessary , step by step and test .

          This is a "primitive or aborignal" way to walk through a code that you not fully understand .

          Show
          Pierre Pichet added a comment - - edited "I would be interested to see your code some time." I have set in questionbase.php so it appears anywhere just in case. public $fieldid = ''; public $fieldname = 'answer'; public function field($key = '') { return $this->fieldid.$this->fieldname.$key; } and change all 'answer' to $answername = $this->field(''); return array($answername => $answer->answer); I set back to 'answer' in all functions that have been changed like is_complete(). here a cleaned but not finished example public function grade_response(array $response) { $subresponses = array(); foreach ($response as $key => $value) { if (preg_match('/^_sub(\d+)-(.*)/', $key, $matches)) { $subresponses[$matches[1]][$matches[2]] = $value; } } // Now grade each bit and combine. $totfraction = 0 ; foreach ($this->subquestions as $i => $subq) { // subquestion if (!empty($subresponses[$i])) { $res = $subq->grade_response($subresponses[$i]); $totfraction += $res[0] * $subq->defaultmark/$this->defaultmark ; } } return array($totfraction, question_state::graded_state_for_fraction($totfraction)); } As I need the subquestion grade in the renderer (this (i.e. pop-up window for inline ) is not done yet), I have to choose between store the $res as qa->question->subquestions [$i] ->$calculated_grade to retrieve it in the renderer code or to grade_response() again when rendering. When everything will work correctly, I will set back to 'answer' everywhere it is unnecessary , step by step and test . This is a "primitive or aborignal" way to walk through a code that you not fully understand .
          Hide
          Pierre Pichet added a comment - - edited
            if (!empty($subresponses[$i])) {
          

          empty() is a bad test as empty(0) is true
          I use the actual 1.9 code to create the select element for multiplechoice inline
          and the select has the following structure .

          <select name="q507,1__sub3-answer">
          <option></option>
          <option value="0" selected="selected">Wrong answer</option>
          <option value="1">Another wrong answer</option>
          <option value="2">Answer that gives half the credit</option>
          <option value="3">Correct answer</option></select>

          in some cases due to shuffling this could become
          <select name="q507,1__sub3-answer">
          <option></option>
          <option value="0" selected="selected">Correct answer</option>
          <option value="1">Another wrong answer</option>
          <option value="2">Answer that gives half the credit</option>
          <option value="3">Wrong answer</option></select>

          Not an evident bug
          In shortanswer or numerical the good answer could be 0 so definitively empty() should be used with care or other tests like isset or == 0.
          For multichoice , I will do the changes for the option values as in match.

          Grade is under control.
          Reviewing the format and feedback for each variant.
          The cloze convention is to give no feedback if the answer is empty.
          So we need a supplementary condition to if ( $options-feedback && $currentanswer != '')
          etc. etc.
          There was also some code in multianswer for screen-reader as some of the questions are inline with the question text.

          I notice also that as we are in 1.9, the output calls will need to be changed for 2,0.

          Show
          Pierre Pichet added a comment - - edited if (!empty($subresponses[$i])) { empty() is a bad test as empty(0) is true I use the actual 1.9 code to create the select element for multiplechoice inline and the select has the following structure . <select name="q507,1__sub3-answer"> <option></option> <option value="0" selected="selected">Wrong answer</option> <option value="1">Another wrong answer</option> <option value="2">Answer that gives half the credit</option> <option value="3">Correct answer</option></select> in some cases due to shuffling this could become <select name="q507,1__sub3-answer"> <option></option> <option value="0" selected="selected">Correct answer</option> <option value="1">Another wrong answer</option> <option value="2">Answer that gives half the credit</option> <option value="3">Wrong answer</option></select> Not an evident bug In shortanswer or numerical the good answer could be 0 so definitively empty() should be used with care or other tests like isset or == 0. For multichoice , I will do the changes for the option values as in match. Grade is under control. Reviewing the format and feedback for each variant. The cloze convention is to give no feedback if the answer is empty. So we need a supplementary condition to if ( $options-feedback && $currentanswer != '') etc. etc. There was also some code in multianswer for screen-reader as some of the questions are inline with the question text. I notice also that as we are in 1.9, the output calls will need to be changed for 2,0.
          Hide
          Pierre Pichet added a comment -

          Pop-up feedback is working for short answer

          Show
          Pierre Pichet added a comment - Pop-up feedback is working for short answer
          Hide
          Pierre Pichet added a comment -

          Good news, apparently the numerical answer can use the shortanswer_renderer
          public function formulation_and_controls()

          if defined as class qtype_multianswer_numerical_renderer extends qtype_multianswer_shortanswer_renderer
          This is possible because there are no units in multianswer/numerical

          Show
          Pierre Pichet added a comment - Good news, apparently the numerical answer can use the shortanswer_renderer public function formulation_and_controls() if defined as class qtype_multianswer_numerical_renderer extends qtype_multianswer_shortanswer_renderer This is possible because there are no units in multianswer/numerical
          Hide
          Pierre Pichet added a comment -

          I have added subquestion grade to the pop-up window.
          This was not available in actual cloze question.
          What will be the correct options to display these individual grades appart that the response is gradeable ?

          Show
          Pierre Pichet added a comment - I have added subquestion grade to the pop-up window. This was not available in actual cloze question. What will be the correct options to display these individual grades appart that the response is gradeable ?
          Hide
          Pierre Pichet added a comment - - edited

          All 12 variants are working although the displays remain to be done i.e. you can filled them with the correct answer and the output will be graded correctly and most element are colored correctly.
          However the vertical or horizontal display of the elements is not set at the moment but they are displayed in a different renderer
          I will send you the address and the password for the site so you can see the results.
          The editmultianswer has been modified so you can create new questions but the validation is not completed.
          Last nigth I put back 'answer' and some little modifications but I loose the grading of numerical in the total grade although I could grade the subquestion.
          This is related to a call in lib.php public function get_qt_data() in class question_attempt_step {

          So I move backward ...

          Show
          Pierre Pichet added a comment - - edited All 12 variants are working although the displays remain to be done i.e. you can filled them with the correct answer and the output will be graded correctly and most element are colored correctly. However the vertical or horizontal display of the elements is not set at the moment but they are displayed in a different renderer I will send you the address and the password for the site so you can see the results. The editmultianswer has been modified so you can create new questions but the validation is not completed. Last nigth I put back 'answer' and some little modifications but I loose the grading of numerical in the total grade although I could grade the subquestion. This is related to a call in lib.php public function get_qt_data() in class question_attempt_step { So I move backward ...
          Hide
          Pierre Pichet added a comment - - edited

          I don't know about your timing or workload.
          I presume that it will be best if I continue on my own with all the your comments about the rendering (i.e. how to name and display the individual subquestion grading results either in pop-up or in the table elements for vertical and horizontal multichoices).

          Show
          Pierre Pichet added a comment - - edited I don't know about your timing or workload. I presume that it will be best if I continue on my own with all the your comments about the rendering (i.e. how to name and display the individual subquestion grading results either in pop-up or in the table elements for vertical and horizontal multichoices).
          Hide
          Pierre Pichet added a comment -

          I think that the main problem is related to questionbase.php
          abstract class question_graded_by_strategy extends question_graded_automatically {
          /** @var question_grading_strategy the strategy to use for grading. */
          protected $gradingstrategy;

          /** @param question_grading_strategy $strategy the strategy to use for grading. */
          public function __construct(question_grading_strategy $strategy)

          { parent::__construct(); $this->gradingstrategy = $strategy; }

          public function get_correct_response() {
          $answer = $this->get_correct_answer();
          if (!$answer)

          { return array(); }
          return array('answer' => $answer->answer);
          }
          Things can work when I define at the main class
          public $fieldid = '';

          public $fieldname = 'answer';

          public function field($key = '') { return $this->fieldid.$this->fieldname.$key; }

          and modify get_correct_response() to

          public function get_correct_response() {
          $answer = $this->get_correct_answer();
          if (!$answer) { return array(); }

          $answername = $this->field('');
          return array($answername => $answer->answer);
          }

          On the other end, the other questions don't work correctly outside multianswer.

          For multichoice multi answers even if it is of class question_graded_automatically
          there was a need to redefine get_correct_response to change 'answer' by 'choice'

          public function get_correct_response() {
          $response = array();
          foreach ($this->order as $key => $ans) {
          if (question_state::graded_state_for_fraction($this->answers[$ans]->fraction) !=
          question_state::$gradedwrong)

          { $response[$this->field($key)] = 1; }

          }
          return $response;
          }

          Probably the solution is to redefine get_correct_response() in all subquestions used in multianswer if they are of graded_by_strategy class as shortanswer , numerical and even for multichoice single response.

          So I will remove the changes in questionbase and do them locally in multianswer subquestions.

          This should work ??

          Show
          Pierre Pichet added a comment - I think that the main problem is related to questionbase.php abstract class question_graded_by_strategy extends question_graded_automatically { /** @var question_grading_strategy the strategy to use for grading. */ protected $gradingstrategy; /** @param question_grading_strategy $strategy the strategy to use for grading. */ public function __construct(question_grading_strategy $strategy) { parent::__construct(); $this->gradingstrategy = $strategy; } public function get_correct_response() { $answer = $this->get_correct_answer(); if (!$answer) { return array(); } return array('answer' => $answer->answer); } Things can work when I define at the main class public $fieldid = ''; public $fieldname = 'answer'; public function field($key = '') { return $this->fieldid.$this->fieldname.$key; } and modify get_correct_response() to public function get_correct_response() { $answer = $this->get_correct_answer(); if (!$answer) { return array(); } $answername = $this->field(''); return array($answername => $answer->answer); } On the other end, the other questions don't work correctly outside multianswer. For multichoice multi answers even if it is of class question_graded_automatically there was a need to redefine get_correct_response to change 'answer' by 'choice' public function get_correct_response() { $response = array(); foreach ($this->order as $key => $ans) { if (question_state::graded_state_for_fraction($this->answers [$ans] ->fraction) != question_state::$gradedwrong) { $response[$this->field($key)] = 1; } } return $response; } Probably the solution is to redefine get_correct_response() in all subquestions used in multianswer if they are of graded_by_strategy class as shortanswer , numerical and even for multichoice single response. So I will remove the changes in questionbase and do them locally in multianswer subquestions. This should work ??
          Hide
          Pierre Pichet added a comment -

          THIS IS THE SOLUTION ....

          Show
          Pierre Pichet added a comment - THIS IS THE SOLUTION ....
          Hide
          Tim Hunt added a comment -

          "What will be the correct options to display these individual grades"?

          All decisions about what to display should be based on the question_display_options $options parameter. In this case, it will be $options->marks you should be testing. Note that that field has three settings:
          question_display_options::HIDDEN
          question_display_options::MAX_ONLY
          question_display_options::MARK_AND_MAX

          "I don't know about your timing or workload."

          I am pretty busy, still working on the quiz reports. And I am having a short holiday, from Wednesday this week until next Monday.

          For get_correct_response, can't that call get_correct_response on each subquestion, and then adjust the array keys when you combine the separate bits in multianswer?

          Show
          Tim Hunt added a comment - "What will be the correct options to display these individual grades"? All decisions about what to display should be based on the question_display_options $options parameter. In this case, it will be $options->marks you should be testing. Note that that field has three settings: question_display_options::HIDDEN question_display_options::MAX_ONLY question_display_options::MARK_AND_MAX "I don't know about your timing or workload." I am pretty busy, still working on the quiz reports. And I am having a short holiday, from Wednesday this week until next Monday. For get_correct_response, can't that call get_correct_response on each subquestion, and then adjust the array keys when you combine the separate bits in multianswer?
          Hide
          Pierre Pichet added a comment -

          elemenary horizontal and vertical display done using simple table element.
          Users knowing this can build the display of their subquestions more easily.
          More stuctured display of the input element, text, feedback and grading are the next step.

          Show
          Pierre Pichet added a comment - elemenary horizontal and vertical display done using simple table element. Users knowing this can build the display of their subquestions more easily. More stuctured display of the input element, text, feedback and grading are the next step.
          Hide
          Pierre Pichet added a comment -

          in multichoice/renderer.php
          line 112 get_string('selectone', 'qtype_multichoice'));
          should be replace by a different text for multi answers i.e. select at least one.
          as the actual 1.9 code unless this have changed for some reasons...
          $answerprompt = ($question->options->single) ? get_string('singleanswer', 'quiz') :
          get_string('multipleanswers', 'quiz');

          Show
          Pierre Pichet added a comment - in multichoice/renderer.php line 112 get_string('selectone', 'qtype_multichoice')); should be replace by a different text for multi answers i.e. select at least one. as the actual 1.9 code unless this have changed for some reasons... $answerprompt = ($question->options->single) ? get_string('singleanswer', 'quiz') : get_string('multipleanswers', 'quiz');
          Hide
          Tim Hunt added a comment -

          Oops. Thanks Pierre. I've fixed that now.

          Last night, I noticed that you are not supposed to use the characters , ! or # in <input name="..." or id="..." attributes, so I changed it (the most significant change is ! -> -). I don't that that should break anything you have done, but I thought I should warn you. I just pushed the new code to github.

          Show
          Tim Hunt added a comment - Oops. Thanks Pierre. I've fixed that now. Last night, I noticed that you are not supposed to use the characters , ! or # in <input name="..." or id="..." attributes, so I changed it (the most significant change is ! -> -). I don't that that should break anything you have done, but I thought I should warn you. I just pushed the new code to github.
          Hide
          Pierre Pichet added a comment - - edited

          Just notice your initial suggestion
          I think that for cloze, it should be something like:

          public function get_expected_data() {
              $expected = array();
              foreach ($this->subquestions as $i => $sq) {
                  $sqexpected = $sq->get_expected_data();
                  foreach ($sqexpected as $name => $type) {
                      $expected['sub' . $i . '-' . $name] = $type;
                  }
              }
              return $expected;
          
          I have changed to 
          '_sub' . $i . '-' . $name as _ was necessary to the names convention in steps
          
          Should I  change the - in  
          '_sub' . $i . '-' . $name to 
          
          '_sub' . $i . '_' . $name  ?
          
          Show
          Pierre Pichet added a comment - - edited Just notice your initial suggestion I think that for cloze, it should be something like: public function get_expected_data() { $expected = array(); foreach ($this->subquestions as $i => $sq) { $sqexpected = $sq->get_expected_data(); foreach ($sqexpected as $name => $type) { $expected['sub' . $i . '-' . $name] = $type; } } return $expected; I have changed to '_sub' . $i . '-' . $name as _ was necessary to the names convention in steps Should I change the - in '_sub' . $i . '-' . $name to '_sub' . $i . '_' . $name ?
          Hide
          Tim Hunt added a comment -

          No. _ stays as _.

          Show
          Tim Hunt added a comment - No. _ stays as _.
          Hide
          Pierre Pichet added a comment -

          That "you are not supposed to use the characters , ! or # in <input name="..." or id="..." attributes,"

          means as a first character .

          So I do not have to change _sub9-choice to _sub9_choice ?

          Show
          Pierre Pichet added a comment - That "you are not supposed to use the characters , ! or # in <input name="..." or id="..." attributes," means as a first character . So I do not have to change _sub9-choice to _sub9_choice ?
          Hide
          Tim Hunt added a comment -

          Correct, no need to change.

          Show
          Tim Hunt added a comment - Correct, no need to change.
          Hide
          Pierre Pichet added a comment -

          Answer numbering corrected for multichoices

          Show
          Pierre Pichet added a comment - Answer numbering corrected for multichoices
          Hide
          Pierre Pichet added a comment -

          Individual grading done for all 12 variants and display as
          Mark 0.50 out of 1.00
          or other messages (i.e. validation_error)
          Todo: Testing on various models and cleaning the code

          Show
          Pierre Pichet added a comment - Individual grading done for all 12 variants and display as Mark 0.50 out of 1.00 or other messages (i.e. validation_error) Todo: Testing on various models and cleaning the code
          Hide
          Pierre Pichet added a comment -

          To correclty display the green ( correct response) color around a select element , it needs to be put in a table
          <table class =\" que answer\"><tr><td class =\"control correct\" ><select
          however this does not give the necessary inline display.

          We need to include .que in a span element to keep inline features.

          You are the css Moodle expert

          Show
          Pierre Pichet added a comment - To correclty display the green ( correct response) color around a select element , it needs to be put in a table <table class =\" que answer\"><tr><td class =\"control correct\" ><select however this does not give the necessary inline display. We need to include .que in a span element to keep inline features. You are the css Moodle expert
          Hide
          Pierre Pichet added a comment -

          Trying to simplify the code, I understand more clearly how the match question structure could be used to handle multianswer.
          Match is built from subquestions of the multichoice type using a select element like the multianswer in-line multichoice.
          The other multichoices in multianswer use checkbox or radio elements.
          With carefull design these could be handle in one renderer as multichoice renderer is already handling all multichoice with the exception of the match select element.
          The short answer and the numerical are handled in just one renderer in my actual multianswer

          So I am going back to the drawing board to include the renderers already set for my actual multianswer in a match question structure where the subquestions will not be shuffled

          When I write in just one renderer, I mean in one common formulation_and_controls function.
          We should come with 2 such functions , one for the multichoices and one for shortanswer and
          numerical.
          A main multianswer formulation_and_controls function will dispatch the task to the subquestions formulation_and_controls function as I am already using in the actual multianswer code.

          Show
          Pierre Pichet added a comment - Trying to simplify the code, I understand more clearly how the match question structure could be used to handle multianswer. Match is built from subquestions of the multichoice type using a select element like the multianswer in-line multichoice. The other multichoices in multianswer use checkbox or radio elements. With carefull design these could be handle in one renderer as multichoice renderer is already handling all multichoice with the exception of the match select element. The short answer and the numerical are handled in just one renderer in my actual multianswer So I am going back to the drawing board to include the renderers already set for my actual multianswer in a match question structure where the subquestions will not be shuffled When I write in just one renderer, I mean in one common formulation_and_controls function. We should come with 2 such functions , one for the multichoices and one for shortanswer and numerical. A main multianswer formulation_and_controls function will dispatch the task to the subquestions formulation_and_controls function as I am already using in the actual multianswer code.
          Hide
          Pierre Pichet added a comment -

          On a closer comparison of the match structure and on the structure I have build to solve the multianswer, I just find that I have created a structure similar to the match structure

          In match the subquestions called stems are called following a shuffle order.
          In multianswer the subquestions are called following their order in the question text

          To created the multianswer, I start from the multichoice model as I need a question_graded_automatically and I want to integrate all multiple choice types.

          So my first drawing was the good one

          Show
          Pierre Pichet added a comment - On a closer comparison of the match structure and on the structure I have build to solve the multianswer, I just find that I have created a structure similar to the match structure In match the subquestions called stems are called following a shuffle order. In multianswer the subquestions are called following their order in the question text To created the multianswer, I start from the multichoice model as I need a question_graded_automatically and I want to integrate all multiple choice types. So my first drawing was the good one
          Hide
          Pierre Pichet added a comment -

          So main task this week is to set correct display and subquestion grade and feedback to all options settings...

          Show
          Pierre Pichet added a comment - So main task this week is to set correct display and subquestion grade and feedback to all options settings...
          Hide
          Pierre Pichet added a comment - - edited

          In the code flow of core_renderer

              public function question(question_attempt $qa, qim_renderer $qimoutput,
                      qtype_renderer $qtoutput, question_display_options $options, $number) {
          
                  $output = '';
                  $output .= $this->output_start_tag('div', array(
                      'id' => 'q' . $qa->get_number_in_usage(),
                      'class' => 'que ' . $qa->get_question()->qtype->name() . ' ' .
                              $qa->get_interaction_model_name(),
                  ));
          
                  $output .= $this->output_tag('div', array('class' => 'info'),
                          $this->info($qa, $qimoutput, $qtoutput, $options, $number));
          
                  $output .= $this->output_start_tag('div', array('class' => 'content'));
          
                  $output .= $this->output_tag('div', array('class' => 'formulation'),
                          $this->formulation($qa, $qimoutput, $qtoutput, $options));
                  $output .= $this->output_nonempty_tag('div', array('class' => 'outcome'),
                          $this->outcome($qa, $qimoutput, $qtoutput, $options));
                  $output .= $this->output_nonempty_tag('div', array('class' => 'comment'),
                          $qimoutput->manual_comment($qa, $options));
                  $output .= $this->output_nonempty_tag('div', array('class' => 'history'),
                          $this->response_history($qa, $qimoutput, $qtoutput, $options));
          
                  $output .= $this->output_end_tag('div');
                  $output .= $this->output_end_tag('div');
                  return $output;
              }
          

          The info giving the question status is generated before the formulation.
          To correctly give the info of each subquestions, in qtype_renderer $qtoutput->formulation_and_controls($qa, $options);
          I need access to the qim_renderer .
          This could be possible if we change the call to include the qim_renderer as it is available in the
          protected function formulation.
          So somethinng like

              protected function formulation(question_attempt $qa, qim_renderer $qimoutput,
                      qtype_renderer $qtoutput, question_display_options $options) {
                  $output = '';
                  $output .= $qtoutput->formulation_and_controls($qa, $qimoutput, $options);
                  $output .= $this->output_nonempty_tag('div', array('class' => 'im-controls'),
                          $qimoutput->controls($qa, $options));
                  return $output;
              }
          

          then in formulation_and_controls I could call

              protected function status(question_attempt $qa, qim_renderer $qimoutput, question_display_options $options) {
                  if ($options->correctness) {
                      return $this->output_tag('div', array('class' => 'state'),
                              $qimoutput->get_state_string($qa));
                  } else {
                      return '';
                  }
              }
          

          Or call directly
          $qimoutput->get_state_string($qa));
          if don't want the div array('class' => 'state').

          Unless you have a better way to do it.

          Show
          Pierre Pichet added a comment - - edited In the code flow of core_renderer public function question(question_attempt $qa, qim_renderer $qimoutput, qtype_renderer $qtoutput, question_display_options $options, $number) { $output = ''; $output .= $this->output_start_tag('div', array( 'id' => 'q' . $qa->get_number_in_usage(), 'class' => 'que ' . $qa->get_question()->qtype->name() . ' ' . $qa->get_interaction_model_name(), )); $output .= $this->output_tag('div', array('class' => 'info'), $this->info($qa, $qimoutput, $qtoutput, $options, $number)); $output .= $this->output_start_tag('div', array('class' => 'content')); $output .= $this->output_tag('div', array('class' => 'formulation'), $this->formulation($qa, $qimoutput, $qtoutput, $options)); $output .= $this->output_nonempty_tag('div', array('class' => 'outcome'), $this->outcome($qa, $qimoutput, $qtoutput, $options)); $output .= $this->output_nonempty_tag('div', array('class' => 'comment'), $qimoutput->manual_comment($qa, $options)); $output .= $this->output_nonempty_tag('div', array('class' => 'history'), $this->response_history($qa, $qimoutput, $qtoutput, $options)); $output .= $this->output_end_tag('div'); $output .= $this->output_end_tag('div'); return $output; } The info giving the question status is generated before the formulation. To correctly give the info of each subquestions, in qtype_renderer $qtoutput->formulation_and_controls($qa, $options); I need access to the qim_renderer . This could be possible if we change the call to include the qim_renderer as it is available in the protected function formulation. So somethinng like protected function formulation(question_attempt $qa, qim_renderer $qimoutput, qtype_renderer $qtoutput, question_display_options $options) { $output = ''; $output .= $qtoutput->formulation_and_controls($qa, $qimoutput, $options); $output .= $this->output_nonempty_tag('div', array('class' => 'im-controls'), $qimoutput->controls($qa, $options)); return $output; } then in formulation_and_controls I could call protected function status(question_attempt $qa, qim_renderer $qimoutput, question_display_options $options) { if ($options->correctness) { return $this->output_tag('div', array('class' => 'state'), $qimoutput->get_state_string($qa)); } else { return ''; } } Or call directly $qimoutput->get_state_string($qa)); if don't want the div array('class' => 'state'). Unless you have a better way to do it.
          Hide
          Tim Hunt added a comment -

          It feels wrong for me for the qim and qt renderers to have to interact.

          Well, may-be not wrong exactly, but a bad idea. That way lies spaghetti.

          The idea is that all the information needed to do the rendering should be contained in $qa and $options.

          Can't you get the particular information you want by calling $qa->get_state_string? (I remember adding that because I needed it somewhere.)

          Show
          Tim Hunt added a comment - It feels wrong for me for the qim and qt renderers to have to interact. Well, may-be not wrong exactly, but a bad idea. That way lies spaghetti. The idea is that all the information needed to do the rendering should be contained in $qa and $options. Can't you get the particular information you want by calling $qa->get_state_string? (I remember adding that because I needed it somewhere.)
          Hide
          Pierre Pichet added a comment -

          My first comment is that I have always see Cloze as a quiz inside a quiz.

          So as a quiz, Cloze needs the outcome of the subquestions to show and handle correctly the output from each subquestions.

          The problem is that the good treatment of the question output are related to the interaction model i.e. either an empty answer is really a final non valid answer or the nonvalid state is used by the interaction model to give the advice to "Choose at least a response".

          Only the qim contains the code that convert invalid to "Choose at least a response" or "Invalid aswer".

          On the other end why do you give to $this->formulation($qa, $qimoutput, $qtoutput, $options) the $qimoutput parameter ?

          In my actual code as a temporary solution I copy some of the core_renderer functions as mark_summary() to obtain the good output.

          Show
          Pierre Pichet added a comment - My first comment is that I have always see Cloze as a quiz inside a quiz. So as a quiz, Cloze needs the outcome of the subquestions to show and handle correctly the output from each subquestions. The problem is that the good treatment of the question output are related to the interaction model i.e. either an empty answer is really a final non valid answer or the nonvalid state is used by the interaction model to give the advice to "Choose at least a response". Only the qim contains the code that convert invalid to "Choose at least a response" or "Invalid aswer". On the other end why do you give to $this->formulation($qa, $qimoutput, $qtoutput, $options) the $qimoutput parameter ? In my actual code as a temporary solution I copy some of the core_renderer functions as mark_summary() to obtain the good output.
          Hide
          Pierre Pichet added a comment -

          I just want from the qim the infos or outputs related to the subquestions states.
          There are no interference in the qim analysis process of the subquestion states so this can be done without harm to qim process.

          Show
          Pierre Pichet added a comment - I just want from the qim the infos or outputs related to the subquestions states. There are no interference in the qim analysis process of the subquestion states so this can be done without harm to qim process.
          Hide
          Pierre Pichet added a comment -

          This can be seen also as some modifications of the public function question(question_attempt $qa, qim_renderer $qimoutput,
          qtype_renderer $qtoutput, question_display_options $options, $number) to Cloze question structure and rendering.

          Show
          Pierre Pichet added a comment - This can be seen also as some modifications of the public function question(question_attempt $qa, qim_renderer $qimoutput, qtype_renderer $qtoutput, question_display_options $options, $number) to Cloze question structure and rendering.
          Hide
          Pierre Pichet added a comment -

          In the case that you give to Cloze the access to qim, as qim works with question_attempt $qa, Cloze code will need to create a copy of the exisiting $qa in which the subquestion implicated will replace the $qa->question as subquestions are not a structural element of $qa structure as understanded by the actual qim code as far as I understand correctly the actual code.

          Show
          Pierre Pichet added a comment - In the case that you give to Cloze the access to qim, as qim works with question_attempt $qa, Cloze code will need to create a copy of the exisiting $qa in which the subquestion implicated will replace the $qa->question as subquestions are not a structural element of $qa structure as understanded by the actual qim code as far as I understand correctly the actual code.
          Hide
          Tim Hunt added a comment -

          I think it is getting difficult for me to comment without me being able to see your code.

          The first priority is to get some code that works, whatever it takes. Once we have working code, then we can spend some time moving stuff around to improve the design, if necessary.

          But I still think that multianswer_renderer should not talk to the interaction model directly, only $qa and $options. (However, $qa is allowed to talk to the interaction model. That is how $qa->get_state_string() works. This sort of indirection may seem silly, but one of the goals of object-oriented design is to reduce dependencies between classes as much as possible.)

          Show
          Tim Hunt added a comment - I think it is getting difficult for me to comment without me being able to see your code. The first priority is to get some code that works, whatever it takes. Once we have working code, then we can spend some time moving stuff around to improve the design, if necessary. But I still think that multianswer_renderer should not talk to the interaction model directly, only $qa and $options. (However, $qa is allowed to talk to the interaction model. That is how $qa->get_state_string() works. This sort of indirection may seem silly, but one of the goals of object-oriented design is to reduce dependencies between classes as much as possible.)
          Hide
          Pierre Pichet added a comment -

          The actual code is difficult to read as it contains remnants of the actual cloze code. as comments ( /* */ ).

          I don't have the time today to clean it.

          However you can go back to the site where you go last week and create a quiz.
          Actually I have done something so that the quiz is not working but the preview isworking.

          Preview the MEGA QUESTION ALL 12 VARIANTS and you will see how I manage to render the marks for multichoice variants

          If you don't select anything for a multichoice multianswer and click the Submit and finish button then you are ask to Choose at least one answer which is the default validation string for multichoice multianswer.

          However on a regular multichoice multianswer if you don't select anything and click the Submit and finish button then the question invalid state is rendered as Not answered or different strings following the model.

          How can I render inside the question renderer formulation() for each subquestion these strings that vary following the model ?

          For marks I cheat by copying in question renderer the mark code of qim renderer ....
          This can work as this is constant for most models.
          This is not true for the other parts of models outputs.

          Show
          Pierre Pichet added a comment - The actual code is difficult to read as it contains remnants of the actual cloze code. as comments ( /* */ ). I don't have the time today to clean it. However you can go back to the site where you go last week and create a quiz. Actually I have done something so that the quiz is not working but the preview isworking. Preview the MEGA QUESTION ALL 12 VARIANTS and you will see how I manage to render the marks for multichoice variants If you don't select anything for a multichoice multianswer and click the Submit and finish button then you are ask to Choose at least one answer which is the default validation string for multichoice multianswer. However on a regular multichoice multianswer if you don't select anything and click the Submit and finish button then the question invalid state is rendered as Not answered or different strings following the model. How can I render inside the question renderer formulation() for each subquestion these strings that vary following the model ? For marks I cheat by copying in question renderer the mark code of qim renderer .... This can work as this is constant for most models. This is not true for the other parts of models outputs.
          Hide
          Pierre Pichet added a comment - - edited

          Screenshot-4 illustrate the actual rendering problem.

          In multichoice the correct answer is not shown yet in the text but the elements have their corresponsding grade (or fraction) colors .

          Show
          Pierre Pichet added a comment - - edited Screenshot-4 illustrate the actual rendering problem. In multichoice the correct answer is not shown yet in the text but the elements have their corresponsding grade (or fraction) colors .
          Hide
          Pierre Pichet added a comment -

          Lets's take an example

          class qim_adaptive_renderer extends qim_renderer {
              protected function get_graded_step(question_attempt $qa) {
                  foreach ($qa->get_reverse_step_iterator() as $step) {
                      if ($step->has_im_var('_try')) {
                          return $step;
                      }
                  }
              }
          
              public function get_state_string(question_attempt $qa) {
                  $state = $qa->get_state();
          
                  $laststep = $qa->get_last_step();
                  if ($laststep->has_im_var('_try')) {
                      $state = question_state::graded_state_for_fraction($laststep->get_im_var('_rawfraction'));
          
                  }
                  return $state->default_string();
              }
          

          Here we are refering to the complete question $state not the subquestions.

          I think it is perhaps too ambitious to display all the subquestions states as if the subquestions were normal independent questions.
          This could lead to an almost impossible logic given that the subquestions don't have separate submit buttons.

          Giving the individual grade, the correct response and a Choose at least one answer or Empty response could be sufficient for a first version.

          P.S. even if some code (mark) is copied from the qim

          Show
          Pierre Pichet added a comment - Lets's take an example class qim_adaptive_renderer extends qim_renderer { protected function get_graded_step(question_attempt $qa) { foreach ($qa->get_reverse_step_iterator() as $step) { if ($step->has_im_var('_try')) { return $step; } } } public function get_state_string(question_attempt $qa) { $state = $qa->get_state(); $laststep = $qa->get_last_step(); if ($laststep->has_im_var('_try')) { $state = question_state::graded_state_for_fraction($laststep->get_im_var('_rawfraction')); } return $state->default_string(); } Here we are refering to the complete question $state not the subquestions. I think it is perhaps too ambitious to display all the subquestions states as if the subquestions were normal independent questions. This could lead to an almost impossible logic given that the subquestions don't have separate submit buttons. Giving the individual grade, the correct response and a Choose at least one answer or Empty response could be sufficient for a first version. P.S. even if some code (mark) is copied from the qim
          Hide
          Pierre Pichet added a comment -

          Reducing the code
          The initial code define for the subquestions 7 questions
          qtype_shortanswer_multianswer_question
          qtype_multianswer_numerical_question
          qtype_multianswer_multichoice_single_vertical_question
          qtype_multianswer_multichoice_single_horizontal_question
          qtype_multianswer_multichoice_single_inline_question (using the select element)
          qtype_multianswer_multichoice_multi_vertical_question
          qtype_multianswer_multichoice_multi_horizontal_question

          each with there own renderer having there own public function formulation_and_controls

          I have been able to fusion vertical and horizontal so 5 questions remain
          qtype_shortanswer_multianswer_question
          qtype_multianswer_numerical_question
          qtype_multianswer_multichoice_single_question
          qtype_multianswer_multichoice_single_inline_question (using the select element)
          qtype_multianswer_multichoice_multi_question

          qtype_shortanswer_multianswer_question and qtype_multianswer_numerical_question use the same
          public function formulation_and_controls
          qtype_multianswer_multichoice_single_inline_question use its own public function formulation_and_controls

          and I should be able to fusion ( little differences remain) the qtype_multianswer_multichoice_single_question and qtype_multianswer_multichoice_multi_question so that they use the same public function formulation_and_controls
          So apart the necessary function formulation_and_controls for the multianswer itself which use the actual code of multianswer to split the subquestions from the question text only 3 function formulation_and_controls will be need to render the 12 variants.

          Show
          Pierre Pichet added a comment - Reducing the code The initial code define for the subquestions 7 questions qtype_shortanswer_multianswer_question qtype_multianswer_numerical_question qtype_multianswer_multichoice_single_vertical_question qtype_multianswer_multichoice_single_horizontal_question qtype_multianswer_multichoice_single_inline_question (using the select element) qtype_multianswer_multichoice_multi_vertical_question qtype_multianswer_multichoice_multi_horizontal_question each with there own renderer having there own public function formulation_and_controls I have been able to fusion vertical and horizontal so 5 questions remain qtype_shortanswer_multianswer_question qtype_multianswer_numerical_question qtype_multianswer_multichoice_single_question qtype_multianswer_multichoice_single_inline_question (using the select element) qtype_multianswer_multichoice_multi_question qtype_shortanswer_multianswer_question and qtype_multianswer_numerical_question use the same public function formulation_and_controls qtype_multianswer_multichoice_single_inline_question use its own public function formulation_and_controls and I should be able to fusion ( little differences remain) the qtype_multianswer_multichoice_single_question and qtype_multianswer_multichoice_multi_question so that they use the same public function formulation_and_controls So apart the necessary function formulation_and_controls for the multianswer itself which use the actual code of multianswer to split the subquestions from the question text only 3 function formulation_and_controls will be need to render the 12 variants.
          Hide
          Pierre Pichet added a comment -

          Revet to original shortanswer and numerical code i.e. 'answer' defined as usual

          Show
          Pierre Pichet added a comment - Revet to original shortanswer and numerical code i.e. 'answer' defined as usual
          Hide
          Pierre Pichet added a comment - - edited

          All 12 variants are working without any modifications to the original shortanswer , numerical and multichoice question.php and renderer.php code.
          to-do:
          Complete the validation process of edit_multianswer_form .

          Reviewing the code in a systematic way.

          Functions that are related to the question code like init_first_step that $step->set_qt_var() are handled correctly by the new step code .
          They can use initial parameter like _order directly, the step code takes care correctly of adding or removing the _sub key.
          However for all functions that are handled by the interaction models the multianswer code (question.php and renderer.php) need to add or remove the _sub key or create an universal code like in function is_same_response().

          So multianswer questions.php redefine for the subquestions the numerical, shortanswer or the various multichoices functions like init_first_step(), get_expected_data(), get_correct_response().

          In the same way multianswer renderer.php redefine get_input_name(), get_response() etc.

          Once the systematic is well understood (and the code is working rigth ), I will put the infos on the docs.

          and again testing (quiz, interaction models and ...)

          Show
          Pierre Pichet added a comment - - edited All 12 variants are working without any modifications to the original shortanswer , numerical and multichoice question.php and renderer.php code. to-do: Complete the validation process of edit_multianswer_form . Reviewing the code in a systematic way. Functions that are related to the question code like init_first_step that $step->set_qt_var() are handled correctly by the new step code . They can use initial parameter like _order directly, the step code takes care correctly of adding or removing the _sub key. However for all functions that are handled by the interaction models the multianswer code (question.php and renderer.php) need to add or remove the _sub key or create an universal code like in function is_same_response(). So multianswer questions.php redefine for the subquestions the numerical, shortanswer or the various multichoices functions like init_first_step(), get_expected_data(), get_correct_response(). In the same way multianswer renderer.php redefine get_input_name(), get_response() etc. Once the systematic is well understood (and the code is working rigth ), I will put the infos on the docs. and again testing (quiz, interaction models and ...)
          Hide
          Pierre Pichet added a comment -

          The new step code does not address all problems as $qa code imply only one question that is not necessarily addressed directly by the interaction model as for shortanswer)

              public function correct_response(question_attempt $qa) {
                  $question = $qa->get_question();
          
                  $answer = reset($question->get_answers());
                  if (!$answer) {
                      return '';
                  }
          
                  return get_string('correctansweris', 'qtype_shortanswer', s($answer->answer));
              }
           

          which must be modified to get the subquestion answers as

               public function correct_response(question_attempt $qa) {
                  $question = $qa->get_question();
                  $subquestion = $question->subquestions[$qa->subquestionindex];
          
                  $answer = reset($subquestion->get_answers());
          
                  if (!$answer) {
                      return '';
                  }
                   return get_string('correctansweris', 'qtype_shortanswer', s($answer->answer));
              }
           

          One solution is to change the call to

               public function correct_response(question $q) {
           
                  $answer = reset($question->get_answers());
          
                  if (!$answer) {
                      return '';
                  }
                   return get_string('correctansweris', 'qtype_shortanswer', s($answer->answer));
              }
           

          Incidently this actual of shortanswer correct_response() does not give the correctanswer.

          On your todo list

          Show
          Pierre Pichet added a comment - The new step code does not address all problems as $qa code imply only one question that is not necessarily addressed directly by the interaction model as for shortanswer) public function correct_response(question_attempt $qa) { $question = $qa->get_question(); $answer = reset($question->get_answers()); if (!$answer) { return ''; } return get_string('correctansweris', 'qtype_shortanswer', s($answer->answer)); } which must be modified to get the subquestion answers as public function correct_response(question_attempt $qa) { $question = $qa->get_question(); $subquestion = $question->subquestions[$qa->subquestionindex]; $answer = reset($subquestion->get_answers()); if (!$answer) { return ''; } return get_string('correctansweris', 'qtype_shortanswer', s($answer->answer)); } One solution is to change the call to public function correct_response(question $q) { $answer = reset($question->get_answers()); if (!$answer) { return ''; } return get_string('correctansweris', 'qtype_shortanswer', s($answer->answer)); } Incidently this actual of shortanswer correct_response() does not give the correctanswer. On your todo list
          Hide
          Pierre Pichet added a comment -

          Actual display for numerical of Response status popup.

          Show
          Pierre Pichet added a comment - Actual display for numerical of Response status popup.
          Hide
          Pierre Pichet added a comment -

          Common function formulation_and_controls for single and multi multichoice as in multiple choice question.

          So only 4 function formulation_and_controls remain
          1. multianswer who analyse the question text and swith to subquestion renderers
          2. multianswer short answer that displays shortanswer and numerical
          3. multianswer multichoice inline select element
          4. multianswer multiplechoice base renderer that displays multichoice single and multi response.

          Show
          Pierre Pichet added a comment - Common function formulation_and_controls for single and multi multichoice as in multiple choice question. So only 4 function formulation_and_controls remain 1. multianswer who analyse the question text and swith to subquestion renderers 2. multianswer short answer that displays shortanswer and numerical 3. multianswer multichoice inline select element 4. multianswer multiplechoice base renderer that displays multichoice single and multi response.
          Hide
          Pierre Pichet added a comment -

          Here (screenshot-6) is the rendering for multianswer multichoice in adaptative mode.
          Most elements are included (correctness, correct response, mark).
          Seems OK for me as a first version.

          Show
          Pierre Pichet added a comment - Here (screenshot-6) is the rendering for multianswer multichoice in adaptative mode. Most elements are included (correctness, correct response, mark). Seems OK for me as a first version.
          Hide
          Pierre Pichet added a comment -

          Tim, in Chat you wrote
          My question stuff is so nearly finished it is annoying that it will just miss 2.0 (I expect).
          Martin : is quiz/questions working in 2.0?
          Tim :I think there are issues. The themes broke it. It was working in November.
          I want to do my UK Moodle presentation next week in the form of a Moodle quiz, so I may have to fix it.

          Should I conclude that it will be more useful to stop documenting and cleaning new engine multianswer
          and go back to fix what I have put on HEAD for other questiontypes to be prepared for HEAD to 2.0 conversion?

          Show
          Pierre Pichet added a comment - Tim, in Chat you wrote My question stuff is so nearly finished it is annoying that it will just miss 2.0 (I expect). Martin : is quiz/questions working in 2.0? Tim :I think there are issues. The themes broke it. It was working in November. I want to do my UK Moodle presentation next week in the form of a Moodle quiz, so I may have to fix it. Should I conclude that it will be more useful to stop documenting and cleaning new engine multianswer and go back to fix what I have put on HEAD for other questiontypes to be prepared for HEAD to 2.0 conversion?
          Hide
          Tim Hunt added a comment -

          I'm afraid my answer to that is still "I don't know".

          I think it is OK to clean-up what is in Moodle 2.0 after the beta release. For now go on working on the new stuff. There is still a chance it can go into Moodle 2.0.

          Show
          Tim Hunt added a comment - I'm afraid my answer to that is still "I don't know". I think it is OK to clean-up what is in Moodle 2.0 after the beta release. For now go on working on the new stuff. There is still a chance it can go into Moodle 2.0.
          Hide
          Pierre Pichet added a comment -

          So I continue the doc exercise which is a good one as a complete check of the code and my understanding of the new engine.

          I you need it at this moment, I can put a .zip containing the code for multianswer questiontype.php. , question.php, renderer.php and edit_multianswer_form.php.
          All the diagnostic echo or print_r remain as comment so they can be reactivated for further testing.

          Evrything seems to work correctly

          Show
          Pierre Pichet added a comment - So I continue the doc exercise which is a good one as a complete check of the code and my understanding of the new engine. I you need it at this moment, I can put a .zip containing the code for multianswer questiontype.php. , question.php, renderer.php and edit_multianswer_form.php. All the diagnostic echo or print_r remain as comment so they can be reactivated for further testing. Evrything seems to work correctly
          Hide
          Tim Hunt added a comment -

          This is a busy week for me. I am away this weekend. Then I have the UK Moodle Moot in the middle of the week, and then I am away again next weekend. Oh, and then I have an exam the week after that. Perhaps it would be best if you don't expect me to do anything much until after 20th April, however, I will try to read your document sooner than that.

          Show
          Tim Hunt added a comment - This is a busy week for me. I am away this weekend. Then I have the UK Moodle Moot in the middle of the week, and then I am away again next weekend. Oh, and then I have an exam the week after that. Perhaps it would be best if you don't expect me to do anything much until after 20th April, however, I will try to read your document sooner than that.
          Hide
          Pierre Pichet added a comment -

          Let's wait after 20th April and good luck for your exam

          Show
          Pierre Pichet added a comment - Let's wait after 20th April and good luck for your exam
          Hide
          Tim Hunt added a comment -

          You are probably going to hate me. I decided overt the weekend that question_interaction_model was too long, so I renamed it to question_behaviour, and qim_... had become qbehaviour_...

          I think it will be worth it in the long run, so I have just committed a very big change http://github.com/timhunt/Moodle-Question-Engine-2/commit/6f920a44e487e705f1e9850b7565ea12cf42a456, but I apologise for the short-term pain. When you next update, it will break your multianswer qtype until you have made similar changes.

          Show
          Tim Hunt added a comment - You are probably going to hate me. I decided overt the weekend that question_interaction_model was too long, so I renamed it to question_behaviour, and qim_... had become qbehaviour_... I think it will be worth it in the long run, so I have just committed a very big change http://github.com/timhunt/Moodle-Question-Engine-2/commit/6f920a44e487e705f1e9850b7565ea12cf42a456 , but I apologise for the short-term pain. When you next update, it will break your multianswer qtype until you have made similar changes.
          Hide
          Pierre Pichet added a comment -

          No problemo,
          It just make me love the fact that the question and their renderer have no access to the interaction model.
          This was one of our discussion object.
          You wrote:
          "But I still think that multianswer_renderer should not talk to the interaction model directly, only $qa and $options. (However, $qa is allowed to talk to the interaction model. That is how $qa->get_state_string() works. This sort of indirection may seem silly, but one of the goals of object-oriented design is to reduce dependencies between classes as much as possible.)"

          I will download the new version to check that everything is OK just in case...

          Show
          Pierre Pichet added a comment - No problemo, It just make me love the fact that the question and their renderer have no access to the interaction model. This was one of our discussion object. You wrote: "But I still think that multianswer_renderer should not talk to the interaction model directly, only $qa and $options. (However, $qa is allowed to talk to the interaction model. That is how $qa->get_state_string() works. This sort of indirection may seem silly, but one of the goals of object-oriented design is to reduce dependencies between classes as much as possible.)" I will download the new version to check that everything is OK just in case...
          Hide
          Pierre Pichet added a comment -

          The problems where
          on public status of shortanswer, numerical and multichoice initialise_question_answers() which is used by multianswer/questiontype.php

          You forgot to add the class question_attempt_step_subquestion_adapter extends question_attempt_step { in lib.php

          I had to create a temporay dummy
          public function summarise_response(array $response) {
          return "test" ;

          But finally the Big 12 variants question seems to work correctly.

          And I continue to love you

          Show
          Pierre Pichet added a comment - The problems where on public status of shortanswer, numerical and multichoice initialise_question_answers() which is used by multianswer/questiontype.php You forgot to add the class question_attempt_step_subquestion_adapter extends question_attempt_step { in lib.php I had to create a temporay dummy public function summarise_response(array $response) { return "test" ; But finally the Big 12 variants question seems to work correctly. And I continue to love you
          Hide
          Pierre Pichet added a comment -

          Doing the analysis , I found a simpler way to use the encode and decode subquestion data process.
          Reviewing all the code.
          More news in 2-3 days.

          Show
          Pierre Pichet added a comment - Doing the analysis , I found a simpler way to use the encode and decode subquestion data process. Reviewing all the code. More news in 2-3 days.
          Hide
          Pierre Pichet added a comment - - edited

          Tim
          Here is a .zip containing multianswer/question.php, renderer.php,edit_multianswer_form.php, questiontype.php and lang files.

          You need to change from private to public
          public function initialise_question_instance()
          for shortanswer, numerical and multichoice questiontype.php

          The code has been cleaned and seems to work correctly for the all questions variations.

          The Modle Doc page is not up-to-date.

          This works on your code loaded 12/04/2010

          P.S. your actual correct response code is not working correctly for shortanswer or numerical
          (i.e. $answer = reset($question->get_answers()); )

          Show
          Pierre Pichet added a comment - - edited Tim Here is a .zip containing multianswer/question.php, renderer.php,edit_multianswer_form.php, questiontype.php and lang files. You need to change from private to public public function initialise_question_instance() for shortanswer, numerical and multichoice questiontype.php The code has been cleaned and seems to work correctly for the all questions variations. The Modle Doc page is not up-to-date. This works on your code loaded 12/04/2010 P.S. your actual correct response code is not working correctly for shortanswer or numerical (i.e. $answer = reset($question->get_answers()); )
          Hide
          Pierre Pichet added a comment -

          With your new version, I had to change some output to html_writer::
          and a few thing.
          You forgot to add your question_attempt_step_subquestion_adapter to lib !!!

          I will add a new zip

          Show
          Pierre Pichet added a comment - With your new version, I had to change some output to html_writer:: and a few thing. You forgot to add your question_attempt_step_subquestion_adapter to lib !!! I will add a new zip
          Hide
          Pierre Pichet added a comment -

          This .zip contains all the necessary files from 21/042010 loading from git and modified to use with the multianswer including the multianswer files.

          Show
          Pierre Pichet added a comment - This .zip contains all the necessary files from 21/042010 loading from git and modified to use with the multianswer including the multianswer files.
          Hide
          Pierre Pichet added a comment -

          On a closer look only numerical questiontype.php functions (initialise_numerical_answers(), initialise_numerical_units() ) needs to be modified for private to public in relation to the calls

          $QTYPES['numerical']>initialise_numerical_answers($question>subquestions[$key], $wrapped);
          $QTYPES['numerical']>initialise_numerical_units($question>subquestions[$key], $wrapped);

          Show
          Pierre Pichet added a comment - On a closer look only numerical questiontype.php functions (initialise_numerical_answers(), initialise_numerical_units() ) needs to be modified for private to public in relation to the calls $QTYPES ['numerical'] >initialise_numerical_answers($question >subquestions [$key] , $wrapped); $QTYPES ['numerical'] >initialise_numerical_units($question >subquestions [$key] , $wrapped);
          Hide
          Pierre Pichet added a comment - - edited

          to give a more coherent representation in the attempt data the subi- should be replaced by _subi
          so in multianswer/questiontype.php
          initialise_question_instance()
          line 165, 175 and 197

                  $question->subquestions[$key]->fieldid = '_sub' . $key . '_';
          

          in so in multianswer/question.php
          line 73

                    if (preg_match('/^_sub(\d+)_(.*)/', $key, $matches)) {
          
          Show
          Pierre Pichet added a comment - - edited to give a more coherent representation in the attempt data the subi- should be replaced by _subi so in multianswer/questiontype.php initialise_question_instance() line 165, 175 and 197 $question->subquestions[$key]->fieldid = '_sub' . $key . '_'; in so in multianswer/question.php line 73 if (preg_match('/^_sub(\d+)_(.*)/', $key, $matches)) {
          Hide
          Pierre Pichet added a comment -

          New version ready to use back from UK Moodle Moot

          I will look at the videos to-night

          Show
          Pierre Pichet added a comment - New version ready to use back from UK Moodle Moot I will look at the videos to-night
          Hide
          Pierre Pichet added a comment -

          Can I conclude from your recent dependency additions and your recent work on github that you are confident to put the new engine on 2.0 ?

          Show
          Pierre Pichet added a comment - Can I conclude from your recent dependency additions and your recent work on github that you are confident to put the new engine on 2.0 ?
          Hide
          Tim Hunt added a comment -

          No. It is still very unlikely. However, we are starting to test it for release here at the OU, where we are still using our own cusomised version of Moodle 1.9 (and there is still a lot of work before even that is finished.)

          Show
          Tim Hunt added a comment - No. It is still very unlikely. However, we are starting to test it for release here at the OU, where we are still using our own cusomised version of Moodle 1.9 (and there is still a lot of work before even that is finished.)
          Hide
          Pierre Pichet added a comment - - edited

          So I will continue my work on cleaning the code and testing the various calculated questions and unit handling.
          By the way, could you do the little patches to css MDL-22350 although I am not sure that 2 of them ( on standardold/style/ ) need to be applied for 2.0.

          Show
          Pierre Pichet added a comment - - edited So I will continue my work on cleaning the code and testing the various calculated questions and unit handling. By the way, could you do the little patches to css MDL-22350 although I am not sure that 2 of them ( on standardold/style/ ) need to be applied for 2.0.
          Hide
          Pierre Pichet added a comment - - edited

          first problem

          Debug info: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'FROM mdl_question_categories qc ON qc.id = q.category
          JOIN md' at line 13
          
          SELECT q.id AS questionid,
          q.questiontext,
          q.questiontextformat,
          qc.contextid,
          qno.id AS qnoid,
          qno.instructions,
          qno.instructionsformat,
          qno.showunits,
          qno.unitsleft,
          qnu.unit AS defaultunit
          
          FROM mdl_question q
          FROM mdl_question_categories qc ON qc.id = q.category
          JOIN mdl_question_numerical_options qno ON qno.question = q.id
          JOIN mdl_question_numerical_units qnu ON qnu.id = (
          SELECT min(id)
          FROM mdl_question_numerical_units
          WHERE question = q.id AND ABS(multiplier - 1) < 0.0000000001)
          [array (
          )]
          Stack trace:
          
              * line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown
              * line 749 of \lib\dml\mysqli_native_moodle_database.php: call to moodle_database->query_end()
              * line 128 of \question\type\numerical\db\upgrade.php: call to mysqli_native_moodle_database->get_recordset_sql()
              * line 373 of \lib\upgradelib.php: call to xmldb_qtype_numerical_upgrade()
              * line 1426 of \lib\upgradelib.php: call to upgrade_plugins()
              * line 290 of \admin\index.php: call to upgrade_noncore()
          
          

          Perhaps we should set a separate bug ?

          As I was working in an almost empty database, I bypass this part of upgrading the question text and let it continue.
          the instructions fields were deleted correctly.
          the code to add the instruction field and the unit 1 option 2 seems good although I could not test it

          Show
          Pierre Pichet added a comment - - edited first problem Debug info: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'FROM mdl_question_categories qc ON qc.id = q.category JOIN md' at line 13 SELECT q.id AS questionid, q.questiontext, q.questiontextformat, qc.contextid, qno.id AS qnoid, qno.instructions, qno.instructionsformat, qno.showunits, qno.unitsleft, qnu.unit AS defaultunit FROM mdl_question q FROM mdl_question_categories qc ON qc.id = q.category JOIN mdl_question_numerical_options qno ON qno.question = q.id JOIN mdl_question_numerical_units qnu ON qnu.id = ( SELECT min(id) FROM mdl_question_numerical_units WHERE question = q.id AND ABS(multiplier - 1) < 0.0000000001) [array ( )] Stack trace: * line 391 of \lib\dml\moodle_database.php: dml_read_exception thrown * line 749 of \lib\dml\mysqli_native_moodle_database.php: call to moodle_database->query_end() * line 128 of \question\type\numerical\db\upgrade.php: call to mysqli_native_moodle_database->get_recordset_sql() * line 373 of \lib\upgradelib.php: call to xmldb_qtype_numerical_upgrade() * line 1426 of \lib\upgradelib.php: call to upgrade_plugins() * line 290 of \admin\index.php: call to upgrade_noncore() Perhaps we should set a separate bug ? As I was working in an almost empty database, I bypass this part of upgrading the question text and let it continue. the instructions fields were deleted correctly. the code to add the instruction field and the unit 1 option 2 seems good although I could not test it
          Hide
          Pierre Pichet added a comment - - edited

          As the user can puit himself the unit beside the input element, we need less options
          and you definition of pre 2,0 default is good

          	<option value="3" selected="selected">Units are not used at all. Only the numerical value is graded.</option>
          
          	<option value="0">Units are optional. If a unit is entered, it is used to convert the reponse to Unit 1 before grading.</option>
          	<option value="1">The unit must be given, and will be graded.</option>
          

          The values set are stored correctly (unit, grading type).

          Show
          Pierre Pichet added a comment - - edited As the user can puit himself the unit beside the input element, we need less options and you definition of pre 2,0 default is good <option value="3" selected="selected">Units are not used at all. Only the numerical value is graded.</option> <option value="0">Units are optional. If a unit is entered, it is used to convert the reponse to Unit 1 before grading.</option> <option value="1">The unit must be given, and will be graded.</option> The values set are stored correctly (unit, grading type).
          Hide
          Pierre Pichet added a comment - - edited

          However in

          @@ -361,20 +362,21 @@ class qtype_numerical extends question_type {
                               $a->fraction, $a->feedback, $a->feedbackformat, $a->tolerance);
                   }
               }
           
               protected function make_answer_processor($units, $unitsleft) {
          -        if (empty($questiondata->options->units)) {
          +       if (empty($units)) {
                       return new qtype_numerical_answer_processor(array());
                   }
           
          -        $units = array();
          -        foreach ($questiondata->options->units as $unit) {
          -            $units[$unit->unit] = $unit->multiplier;
          +        $newunits = array();
          +        foreach ($units as $unit) {
          +            $newunits[$unit->unit] = $unit->multiplier;
                   }
           
          -        return new qtype_numerical_answer_processor($units, $questiondata->options->unitsleft);
          +        return new qtype_numerical_answer_processor($newunits, $unitsleft);
               }
          
          Show
          Pierre Pichet added a comment - - edited However in @@ -361,20 +362,21 @@ class qtype_numerical extends question_type { $a->fraction, $a->feedback, $a->feedbackformat, $a->tolerance); } } protected function make_answer_processor($units, $unitsleft) { - if (empty($questiondata->options->units)) { + if (empty($units)) { return new qtype_numerical_answer_processor(array()); } - $units = array(); - foreach ($questiondata->options->units as $unit) { - $units[$unit->unit] = $unit->multiplier; + $newunits = array(); + foreach ($units as $unit) { + $newunits[$unit->unit] = $unit->multiplier; } - return new qtype_numerical_answer_processor($units, $questiondata->options->unitsleft); + return new qtype_numerical_answer_processor($newunits, $unitsleft); }
          Hide
          Pierre Pichet added a comment -

          Testing with 65 with units as years, 1 and century 100 and unitgrading to 0,2 give grade 1,00 for 65 years or 0.65 century or grade 0.8 for 65 yes or 65 without units.
          The correct_response remains 65 but this is an other story.

          Show
          Pierre Pichet added a comment - Testing with 65 with units as years, 1 and century 100 and unitgrading to 0,2 give grade 1,00 for 65 years or 0.65 century or grade 0.8 for 65 yes or 65 without units. The correct_response remains 65 but this is an other story.
          Hide
          Pierre Pichet added a comment -

          Last comments before new subtasks
          I tested your multiple choice. Seems OK.
          Using a select element is better for in-line than radio button.
          also the Choose default is more intuitive to the mandatory status of the unit.

          How much feedback should be given as default by the system on unit or number response is not easy to set.
          If we follow the new engine philosophy, we should effectively create a unit feedback and let the teacher fill it.
          In all cases moodle should explain the grading number-unit penalty result.

          Show
          Pierre Pichet added a comment - Last comments before new subtasks I tested your multiple choice. Seems OK. Using a select element is better for in-line than radio button. also the Choose default is more intuitive to the mandatory status of the unit. How much feedback should be given as default by the system on unit or number response is not easy to set. If we follow the new engine philosophy, we should effectively create a unit feedback and let the teacher fill it. In all cases moodle should explain the grading number-unit penalty result.
          Hide
          Tim Hunt added a comment -

          I agree we should create a unit feedback field that the teacher can set, but I decided that is a feature we can add later.

          As I said, I am not going to do more work on this qtype now. However, if you want to experiment on a branch in git, you are welcome to try some things.

          Show
          Tim Hunt added a comment - I agree we should create a unit feedback field that the teacher can set, but I decided that is a feature we can add later. As I said, I am not going to do more work on this qtype now. However, if you want to experiment on a branch in git, you are welcome to try some things.
          Hide
          Pierre Pichet added a comment -

          The select element does not render easily subscript or superscript.
          There is another project for OU
          a variant of shortanswer/numerical qtype where students can easily enter superscripts and subscripts;
          http://moodle.org/mod/forum/discuss.php?d=159747
          how this could be merged ?

          Show
          Pierre Pichet added a comment - The select element does not render easily subscript or superscript. There is another project for OU a variant of shortanswer/numerical qtype where students can easily enter superscripts and subscripts; http://moodle.org/mod/forum/discuss.php?d=159747 how this could be merged ?
          Hide
          Tim Hunt added a comment -

          Just a heads-up that I have now merged the latest master branch from moodle.org into the qe2_wip branch.

          Show
          Tim Hunt added a comment - Just a heads-up that I have now merged the latest master branch from moodle.org into the qe2_wip branch.
          Hide
          Pierre Pichet added a comment -

          Interesting to test new engine in a Moodle almost 2,1 live code

          Show
          Pierre Pichet added a comment - Interesting to test new engine in a Moodle almost 2,1 live code
          Hide
          Martin Dougiamas added a comment -

          I'm assuming this is the best bug to keep track of the integration into 2.1 ...

          Show
          Martin Dougiamas added a comment - I'm assuming this is the best bug to keep track of the integration into 2.1 ...
          Hide
          Tim Hunt added a comment -

          I think we are in a position where this could be merged into Master tomorrow (Friday 27th). I need to catch up with Eloy and discuss that with him.

          The only thing I need to do before that is another merge from master branch, which means sorting out lib/db/upgrade.php and version.php. I only want to do that once, so I will wait until other people agree that we are ready to merge.

          The other thing I could usefully do before the merge is a little bit of tinkering with local/qeupgradehelper, but that could easily be done later with no problem.

          git diff --stat master..qe2_wip
          currently gives me
          462 files changed, 68007 insertions, 23795 deletions
          I am sure Eloy has carefully looked at each one of those lines

          Just so people know my movements between now and the release, let me spell them out here:

          • I am at work on Friday 27th.
          • I am at home on Saturday 28th, and can be online almost all day.
          • I am at home on Sunday 29th, but have visitors, so can only be online morning and evening.
          • Mon 30th to Saturday 4th June I am on holiday. Where I am going does have internet, and I probably won't be able to resist checking my email, but I actually I think I should try to have a decent break, so I can come back refreshed for ...
          • Monday 6th - 30th I am available for bug-fixing and any finishing off that needs to be done.

          I'm looking forwards to having this done

          Show
          Tim Hunt added a comment - I think we are in a position where this could be merged into Master tomorrow (Friday 27th). I need to catch up with Eloy and discuss that with him. The only thing I need to do before that is another merge from master branch, which means sorting out lib/db/upgrade.php and version.php. I only want to do that once, so I will wait until other people agree that we are ready to merge. The other thing I could usefully do before the merge is a little bit of tinkering with local/qeupgradehelper, but that could easily be done later with no problem. git diff --stat master..qe2_wip currently gives me 462 files changed, 68007 insertions , 23795 deletions I am sure Eloy has carefully looked at each one of those lines Just so people know my movements between now and the release, let me spell them out here: I am at work on Friday 27th. I am at home on Saturday 28th, and can be online almost all day. I am at home on Sunday 29th, but have visitors, so can only be online morning and evening. Mon 30th to Saturday 4th June I am on holiday. Where I am going does have internet, and I probably won't be able to resist checking my email, but I actually I think I should try to have a decent break, so I can come back refreshed for ... Monday 6th - 30th I am available for bug-fixing and any finishing off that needs to be done. I'm looking forwards to having this done
          Hide
          Tim Hunt added a comment -

          I am slightly concerned by the lack of any contact from Eloy. I hope he is OK.

          I just spent some time doing a final clean-up of my wip branch to make a branch suitable for integration: https://github.com/timhunt/moodle/compare/master...MDL-20636_master_new_question_engine

          Compared to my old qe2_wip branch (https://github.com/timhunt/Moodle-Question-Engine-2/commits/qe2_wip) the differences are:

          1. I did a final merge from the main master branch. That involved sorting out merge conflicts in lib/db/upgrade.php and version.php. I chose to use the version numbers 2011052800.00 to 2011052800.13. I hope that is OK. If this is integrated this week, then the integrators need to be careful of any other commits that do main database changes. If it is not integrated this week, then someone (me, I guess) will have to re-do this merge.

          2. It turns out that thanks to working on Windows, I had a bunch of files with file permissions 755 rather than 644. I sorted all those out throughout the entire history of the branch using some git filter-branch magic.

          3. Similarly, I updated all the commit comments on my branch that did not already start MDL-... to start MDL-20636.

          This history is still not perfect, and never will be, but I think it is worth preserving. In other words I think the best option is for the integrators to merge my MDL-20636_master_new_question_engine into master. The only other alternative would be to do git diff --binary master..qe2_qip | git apply, that is, to turn all my changes into a single huge commit, but I don't really think that is a good idea.

          I am, of course, happy to discuss all this with anyone. If Eloy, or anyone else, has any code review comments that require me to changes things, then I am happy to make changes, but you have to actually communicate with me about what changes are required. I feel I am operating in a bit of a vacuum here at the moment.

          Show
          Tim Hunt added a comment - I am slightly concerned by the lack of any contact from Eloy. I hope he is OK. I just spent some time doing a final clean-up of my wip branch to make a branch suitable for integration: https://github.com/timhunt/moodle/compare/master...MDL-20636_master_new_question_engine Compared to my old qe2_wip branch ( https://github.com/timhunt/Moodle-Question-Engine-2/commits/qe2_wip ) the differences are: 1. I did a final merge from the main master branch. That involved sorting out merge conflicts in lib/db/upgrade.php and version.php. I chose to use the version numbers 2011052800.00 to 2011052800.13. I hope that is OK. If this is integrated this week, then the integrators need to be careful of any other commits that do main database changes. If it is not integrated this week, then someone (me, I guess) will have to re-do this merge. 2. It turns out that thanks to working on Windows, I had a bunch of files with file permissions 755 rather than 644. I sorted all those out throughout the entire history of the branch using some git filter-branch magic. 3. Similarly, I updated all the commit comments on my branch that did not already start MDL-... to start MDL-20636 . This history is still not perfect, and never will be, but I think it is worth preserving. In other words I think the best option is for the integrators to merge my MDL-20636 _master_new_question_engine into master. The only other alternative would be to do git diff --binary master..qe2_qip | git apply, that is, to turn all my changes into a single huge commit, but I don't really think that is a good idea. I am, of course, happy to discuss all this with anyone. If Eloy, or anyone else, has any code review comments that require me to changes things, then I am happy to make changes, but you have to actually communicate with me about what changes are required. I feel I am operating in a bit of a vacuum here at the moment.
          Hide
          Tim Hunt added a comment -

          Just kicking the workflow into an appropriate state. I will not submit this for integration until I here from someone from HQ.

          If, however, I don't here anything from anyone at HQ before it is time for me to go on holiday, I will submit this for integration.

          Show
          Tim Hunt added a comment - Just kicking the workflow into an appropriate state. I will not submit this for integration until I here from someone from HQ. If, however, I don't here anything from anyone at HQ before it is time for me to go on holiday, I will submit this for integration.
          Hide
          Tim Hunt added a comment -

          I will just add that Eloy has been doing peer review for some weeks. I only just got around to updating the workflow state now.

          Show
          Tim Hunt added a comment - I will just add that Eloy has been doing peer review for some weeks. I only just got around to updating the workflow state now.
          Hide
          Tim Hunt added a comment -

          OK, submitting for integration as promised. I will be interested to see what has happened by the time I get back from holiday. Anyway it is in your hands now.

          About the remaining subtasks, which are loose ends I plan to tidy up in June. Perhaps we should create a new META issue: question engine cleaning up (or something) and move the incomplete subtasks there?

          Anyway, I'll be back on Saturday 4th (assuming I can stay offline until then).

          Show
          Tim Hunt added a comment - OK, submitting for integration as promised. I will be interested to see what has happened by the time I get back from holiday. Anyway it is in your hands now. About the remaining subtasks, which are loose ends I plan to tidy up in June. Perhaps we should create a new META issue: question engine cleaning up (or something) and move the incomplete subtasks there? Anyway, I'll be back on Saturday 4th (assuming I can stay offline until then).
          Hide
          Pierre Pichet added a comment -

          I should be able to set a proposal on numerical question issues i.e. no 2,10 and 11 hopefully for monday 6th as my local workload is not empty

          Show
          Pierre Pichet added a comment - I should be able to set a proposal on numerical question issues i.e. no 2,10 and 11 hopefully for monday 6th as my local workload is not empty
          Hide
          Martin Dougiamas added a comment -

          Ladies and gentlemen, please finish your mojitos and return your tray tables and seats to the upright position. This issue is preparing to land sometime this week.

          Show
          Martin Dougiamas added a comment - Ladies and gentlemen, please finish your mojitos and return your tray tables and seats to the upright position. This issue is preparing to land sometime this week.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          Show
          Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          Right now, after the weekly and versions have been stabilized, I start working on this by:

          1) Move all the upgrade code to proper versions at the end of the upgrade.php scripts. At the same time, adjusting $version and $require where necessary.

          2) Try to fix a bunch of inconsistencies found between one 2.1 installed site and one 2.0 => 2.1 upgraded site (list below). The approach I'm going to follow is to assume the definition in the install.xml files are the correct ones always, making modifications always in upgrade scripts to make everything match. Please review them in case that assumption is wrong in some case.

          3) Tim, any preference about your history fully merged/rebased/squashed? If all the upgrade changes are ok, this will land before Monday.

          4) About the local plugin: qeupgradehelper, should we push it both to 20_STABLE and master and make it standard one?

          5) I've the review written (I pasted URL in the HQ chat) but I'm waiting for (dev) Moodle Docs to be migrated. I think there is nothing stoping the re-write to land but the points (1-4) above.

          And that's all, really impressed by your code. I hope it will, also, work :-P Seriously, congrats!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited Right now, after the weekly and versions have been stabilized, I start working on this by: 1) Move all the upgrade code to proper versions at the end of the upgrade.php scripts. At the same time, adjusting $version and $require where necessary. 2) Try to fix a bunch of inconsistencies found between one 2.1 installed site and one 2.0 => 2.1 upgraded site (list below). The approach I'm going to follow is to assume the definition in the install.xml files are the correct ones always, making modifications always in upgrade scripts to make everything match. Please review them in case that assumption is wrong in some case. 3) Tim, any preference about your history fully merged/rebased/squashed? If all the upgrade changes are ok, this will land before Monday. 4) About the local plugin: qeupgradehelper, should we push it both to 20_STABLE and master and make it standard one? 5) I've the review written (I pasted URL in the HQ chat) but I'm waiting for (dev) Moodle Docs to be migrated. I think there is nothing stoping the re-write to land but the points (1-4) above. And that's all, really impressed by your code. I hope it will, also, work :-P Seriously, congrats! Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          List of DB inconsistencies found (point 2 above), "First DB" is the installed one, "Second DB" is the upgraded one:

            Column defaultmark of table question difference found in type: decimal !== bigint
            Column defaultmark of table question difference found in max_length: 12 !== 10
            Column defaultmark of table question difference found in scale: 7 !== null
            Column defaultmark of table question difference found in unsigned: null !== true
            Column defaultmark of table question difference found in default_value: 1.0000000 !== 1
            Column defaultmark of table question difference found in meta_type: N !== I
            Column penalty of table question difference found in type: decimal !== double
            Column penalty of table question difference found in max_length: 12 !== null
            Column penalty of table question difference found in scale: 7 !== null
            Column variant of table question_attempts difference found in has_default: false !== true
            Column variant of table question_attempts difference found in default_value: null !== 1
            Column hintformat of table question_hints only available in second DB
            Column showblocks of table quiz only available in second DB
            Column attemptonlast of table quiz difference found in unsigned: true !== false
            Column questiondecimalpoints of table quiz difference found in default_value: -1 !== -2
            Column maxgrade of table quiz_question_statistics only available in first DB
            Column maxmark of table quiz_question_statistics only available in second DB
            Index (name) of table quiz_reports only available in second DB
          
          Show
          Eloy Lafuente (stronk7) added a comment - List of DB inconsistencies found (point 2 above), "First DB" is the installed one, "Second DB" is the upgraded one: Column defaultmark of table question difference found in type: decimal !== bigint Column defaultmark of table question difference found in max_length: 12 !== 10 Column defaultmark of table question difference found in scale: 7 !== null Column defaultmark of table question difference found in unsigned: null !== true Column defaultmark of table question difference found in default_value: 1.0000000 !== 1 Column defaultmark of table question difference found in meta_type: N !== I Column penalty of table question difference found in type: decimal !== double Column penalty of table question difference found in max_length: 12 !== null Column penalty of table question difference found in scale: 7 !== null Column variant of table question_attempts difference found in has_default: false !== true Column variant of table question_attempts difference found in default_value: null !== 1 Column hintformat of table question_hints only available in second DB Column showblocks of table quiz only available in second DB Column attemptonlast of table quiz difference found in unsigned: true !== false Column questiondecimalpoints of table quiz difference found in default_value: -1 !== -2 Column maxgrade of table quiz_question_statistics only available in first DB Column maxmark of table quiz_question_statistics only available in second DB Index (name) of table quiz_reports only available in second DB
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          For reference, all the changes before integration are happening here:

          https://github.com/stronk7/moodle/tree/qe2_integration

          This is the list of commits applied until now (all the commits in the qe2_integration branch not being in Tim's qe2_wip branch):

          git log master...qe2_integration --not questionengine/qe2_wip --oneline
          
          2542f5d MDL-20636 changes to quiz and quiz reports upgrade code
          8ab25a2 MDL-20636 changes to core & questions upgrade code
          fd4b30e MDL-20636 fix permissions
          7bd034c Merge branch qe2_wip into qe2_integration
          a09bf04 MDL-20636 bring independence from html2text to upgrade tests
          3e3feea MDL-20636 improve a bit html2text to avoid some ugly & unnecessary trailing spaces
          a2126b5 MDL-20636 Whitespace fixes
          a3fd17b MDL-20636 Prepare stuff for cleaner merging
          

          Right now I'm fixing the list of DB differences, so expect 2 more commits to arrive.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - - edited For reference, all the changes before integration are happening here: https://github.com/stronk7/moodle/tree/qe2_integration This is the list of commits applied until now (all the commits in the qe2_integration branch not being in Tim's qe2_wip branch): git log master...qe2_integration --not questionengine/qe2_wip --oneline 2542f5d MDL-20636 changes to quiz and quiz reports upgrade code 8ab25a2 MDL-20636 changes to core & questions upgrade code fd4b30e MDL-20636 fix permissions 7bd034c Merge branch qe2_wip into qe2_integration a09bf04 MDL-20636 bring independence from html2text to upgrade tests 3e3feea MDL-20636 improve a bit html2text to avoid some ugly & unnecessary trailing spaces a2126b5 MDL-20636 Whitespace fixes a3fd17b MDL-20636 Prepare stuff for cleaner merging Right now I'm fixing the list of DB differences, so expect 2 more commits to arrive. Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          3 more commits added to:

          https://github.com/stronk7/moodle/tree/qe2_integration

          to make 2.1 intalled and upgraded to 100% match. Here it is the list of changes performed, they need review!!

            Column defaultmark of table question difference found in type: decimal !== bigint
            Column defaultmark of table question difference found in max_length: 12 !== 10
            Column defaultmark of table question difference found in scale: 7 !== null
            Column defaultmark of table question difference found in unsigned: null !== true
            Column defaultmark of table question difference found in default_value: 1.0000000 !== 1
            Column defaultmark of table question difference found in meta_type: N !== I
          
              Changed to specs: decimal(12, 7) not null default 1
              (based on install.xml, changes only needed in upgrade.php)
          
          
            Column penalty of table question difference found in type: decimal !== double
            Column penalty of table question difference found in max_length: 12 !== null
            Column penalty of table question difference found in scale: 7 !== null
          
              Changed to specs: decimal(12, 7) not null default 0.3333333
              (based on install.xml, but changed also wrong default in install.xml - was 0.1000)
          
          
            Column variant of table question_attempts difference found in has_default: false !== true
            Column variant of table question_attempts difference found in default_value: null !== 1
          
              Changed to specs: integer(10) unsigned not null default 1
              (based on upgrade.php, changes only needed in install.xml - was no default)
          
          
            Column hintformat of table question_hints only available in second DB
          
              Added to install.xml file with specs: integer(4) unsigned not null default 0
              (based on upgrade.php, changes only needed in install.xml - didn't exist)
          
          
            Column showblocks of table quiz only available in second DB
          
              Added to install.xml file with specs: integer(4) signed not null default 0
              (based on upgrade.php, changes only needed in install.xml - 2nd time it disappears!!)
          
          
            Column attemptonlast of table quiz difference found in unsigned: true !== false
          
              Changed to specs: integer(4) signed not null default 0. And reordered after "attempts"
              (based on upgrade.php and old installations changed only in install.xml)
          
          
            Column questiondecimalpoints of table quiz difference found in default_value: -1 !== -2
          
              Changed to specs: integer(4) signed not null default -2
              (based on upgrade.php and old installations changed only in install.xml)
          
          
            Column maxgrade of table quiz_question_statistics only available in first DB
            Column maxmark of table quiz_question_statistics only available in second DB
          
              Changed to specs: decimal(12,7) null not default
              (based on upgrade.php, changes only needed in install.xml, rename was missing there)
          
          
            Index (name) of table quiz_reports only available in second DB
          
              Added to install.xml file with specs: unique index on (name)
              (based on upgrade.php, the index was missing on install)
          
          Show
          Eloy Lafuente (stronk7) added a comment - 3 more commits added to: https://github.com/stronk7/moodle/tree/qe2_integration to make 2.1 intalled and upgraded to 100% match. Here it is the list of changes performed, they need review!! Column defaultmark of table question difference found in type: decimal !== bigint Column defaultmark of table question difference found in max_length: 12 !== 10 Column defaultmark of table question difference found in scale: 7 !== null Column defaultmark of table question difference found in unsigned: null !== true Column defaultmark of table question difference found in default_value: 1.0000000 !== 1 Column defaultmark of table question difference found in meta_type: N !== I Changed to specs: decimal(12, 7) not null default 1 (based on install.xml, changes only needed in upgrade.php) Column penalty of table question difference found in type: decimal !== double Column penalty of table question difference found in max_length: 12 !== null Column penalty of table question difference found in scale: 7 !== null Changed to specs: decimal(12, 7) not null default 0.3333333 (based on install.xml, but changed also wrong default in install.xml - was 0.1000) Column variant of table question_attempts difference found in has_default: false !== true Column variant of table question_attempts difference found in default_value: null !== 1 Changed to specs: integer(10) unsigned not null default 1 (based on upgrade.php, changes only needed in install.xml - was no default ) Column hintformat of table question_hints only available in second DB Added to install.xml file with specs: integer(4) unsigned not null default 0 (based on upgrade.php, changes only needed in install.xml - didn't exist) Column showblocks of table quiz only available in second DB Added to install.xml file with specs: integer(4) signed not null default 0 (based on upgrade.php, changes only needed in install.xml - 2nd time it disappears!!) Column attemptonlast of table quiz difference found in unsigned: true !== false Changed to specs: integer(4) signed not null default 0. And reordered after "attempts" (based on upgrade.php and old installations changed only in install.xml) Column questiondecimalpoints of table quiz difference found in default_value: -1 !== -2 Changed to specs: integer(4) signed not null default -2 (based on upgrade.php and old installations changed only in install.xml) Column maxgrade of table quiz_question_statistics only available in first DB Column maxmark of table quiz_question_statistics only available in second DB Changed to specs: decimal(12,7) null not default (based on upgrade.php, changes only needed in install.xml, rename was missing there) Index (name) of table quiz_reports only available in second DB Added to install.xml file with specs: unique index on (name) (based on upgrade.php, the index was missing on install)
          Hide
          Eloy Lafuente (stronk7) added a comment -
          Show
          Eloy Lafuente (stronk7) added a comment - Reorganized review available @ http://docs.moodle.org/dev/Question_Engine_2:Review_2011-05#Conclusions Ciao
          Hide
          Tim Hunt added a comment -

          I just moved all open sub-tasks to be children of a new META issue MDL-27738.

          Show
          Tim Hunt added a comment - I just moved all open sub-tasks to be children of a new META issue MDL-27738 .
          Hide
          Tim Hunt added a comment -

          Eloy,

          I think I have added comments to all the points at http://docs.moodle.org/dev/Question_Engine_2:Review_2011-05 (apart from the ones you had already done).

          I have not added strike to the things I think are dealt with. I was hoping that you would review all the comments I made above (and code changes, new MDL issues where appropriate) and add strike-out to the ones you are happy with.

          The latest code has been pushed to https://github.com/timhunt/moodle/commits/MDL-20636_master_new_question_engine (and I updated the links above).

          I am about to go home now, but will probably look at this later today and tomorrow morning.

          Show
          Tim Hunt added a comment - Eloy, I think I have added comments to all the points at http://docs.moodle.org/dev/Question_Engine_2:Review_2011-05 (apart from the ones you had already done). I have not added strike to the things I think are dealt with. I was hoping that you would review all the comments I made above (and code changes, new MDL issues where appropriate) and add strike-out to the ones you are happy with. The latest code has been pushed to https://github.com/timhunt/moodle/commits/MDL-20636_master_new_question_engine (and I updated the links above). I am about to go home now, but will probably look at this later today and tomorrow morning.
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          This has been integrated, yay!

          As commented next steps will be:

          Ciao

          And big congrats!

          Show
          Eloy Lafuente (stronk7) added a comment - - edited This has been integrated, yay! As commented next steps will be: Review http://docs.moodle.org/dev/Question_Engine_2:Review_2011-05 striking the already fixed/agreed. Add subtasks with all the pending ones to MDL-27738 Ciao And big congrats!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          As part of initial testing of this I've planned to:

          • Install on all 4 DB.
          • Upgrade all 4DB from 2.0 to 2.1 containing one course using quiz and all qtypes.
          • Run unit-tests under all them

          That would be enough for getting this upstream as far as guarantees installation and basic usage. All the rest of testing should be done under QA iteration once available.

          Show
          Eloy Lafuente (stronk7) added a comment - As part of initial testing of this I've planned to: Install on all 4 DB. Upgrade all 4DB from 2.0 to 2.1 containing one course using quiz and all qtypes. Run unit-tests under all them That would be enough for getting this upstream as far as guarantees installation and basic usage. All the rest of testing should be done under QA iteration once available.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Tested under MySQL and PostgreSQL (couldn't do that under MSSQL and Oracle, grrr).

          I've created MDL-27777 (nice number!) about the need to create extensive QA tests for all this stuff. Stay tuned there, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Tested under MySQL and PostgreSQL (couldn't do that under MSSQL and Oracle, grrr). I've created MDL-27777 (nice number!) about the need to create extensive QA tests for all this stuff. Stay tuned there, thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          And paaaaaasssssssssiiiiiiiiinnnnnnnnngggggggg!

          Show
          Eloy Lafuente (stronk7) added a comment - And paaaaaasssssssssiiiiiiiiinnnnnnnnngggggggg!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This is now upstream, yay! Many thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - This is now upstream, yay! Many thanks!

            People

            • Votes:
              2 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: