Moodle
  1. Moodle
  2. MDL-10516

Pre-populated answer (template) in essay questions

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.8.1, 2.3.4, 2.4.1
    • Fix Version/s: 2.5
    • Component/s: Questions
    • Labels:
    • Testing Instructions:
      Hide

      Test upgrade

      • Test that new fields responsetemplate and responsetemplateformat are correctly created in the qtype_essay_options table and that these fields are correctly populated with empty string and 1 for existing essay questions.
      • Test that existing essay questions continue to work as before and that the answer field is empty when attempt starts.

      Test new functionalities

      • Edit an existing essay question and put some text in the response template field. Verify that for new attempts the text is displayed in the answer field when the attempt is started.
      • Create a new essay question with some text in the response template field. Verify that the text is displayed in the answer field when the attempt is started.
      • Try to include some HTML formatting in the response template when creating/editing an essay question selecting HTML Editor as response format and verify the text is correctly displayed during attempt in the answer editor
      • Verify that the filepicker is not available for the response template when creating/editing an essay question

      Test correct behaviour during attempts

      • Create a quiz with 2 questions (each question on a separate page) : first question is a non essay question and second question is an essay question with some text in the response template.
      • Attempt the quiz as a student and submit without going to the second page. Verify that on the summary page the question is marked as not yet answered. After clicking on submit all and finish verify that for the student when you review your attempt the answer to the essay question is blank and that the attempt has been automatically graded. Then login as a teacher and verify that the essay question is not marked as needing grading.
      • Attempt the quiz as a student and go on each page, verify that on page 2 the essay question is correctly displayed with the response template in the answer field, but don't change anything. Verify that on the summary page the essay question is marked as not yet answered. After clicking on submit all and finish verify that for the student when you review your attempt the answer to the essay question is blank and the attempt has been automatically graded. Then login as a teacher and verify that the essay question is not marked as needing grading.
      • Attempt the quiz as a student and completely change the answer to the essay question. Verify that you new response has been submitted as response to the essay question and that when the student review this attempt it is marked as not yet graded. Then login as a teacher and verify that the attempt is marked as needing manual grading.
      • Attempt the quiz and modify slightly the text of the template in the answer field of the essay question. Verify that your modification has been saved correctly when you change page and return to the essay question page. Verify that it has also been submitted as answer to the essay question when the attempt is submitted and that when you login as a teacher the attempt is marked as needing manual grading.

      Import/Export

      Try to import an old (exported before this issue is integrated) Moodle XML file with essay questions. Verify the responsetemplate and responsetemplateformat are correctly populated by empty string and 1 and that no error or warning is displayed during import.
      Export a category with some essay questions (both with nothing in responseformat and with some content in responseformat) using Moodle XML format. Verify that the new informations are present in the XML.
      Import these questions in another category and verify that responsetemplate is correctly imported and no error or warning is displayed during import.

      Backup/Restore

      Create a quiz with all questions from a category containing essay questions (both with nothing in responseformat and with some content in responseformat) verify that no information is lost when the backup is restored.
      Try to restore an 1.9 course backup with some essay questions and verify that essay questions are correctly restored with empty string as responsetemplate and 1 as responsetemplateformat.

      Show
      Test upgrade Test that new fields responsetemplate and responsetemplateformat are correctly created in the qtype_essay_options table and that these fields are correctly populated with empty string and 1 for existing essay questions. Test that existing essay questions continue to work as before and that the answer field is empty when attempt starts. Test new functionalities Edit an existing essay question and put some text in the response template field. Verify that for new attempts the text is displayed in the answer field when the attempt is started. Create a new essay question with some text in the response template field. Verify that the text is displayed in the answer field when the attempt is started. Try to include some HTML formatting in the response template when creating/editing an essay question selecting HTML Editor as response format and verify the text is correctly displayed during attempt in the answer editor Verify that the filepicker is not available for the response template when creating/editing an essay question Test correct behaviour during attempts Create a quiz with 2 questions (each question on a separate page) : first question is a non essay question and second question is an essay question with some text in the response template. Attempt the quiz as a student and submit without going to the second page. Verify that on the summary page the question is marked as not yet answered. After clicking on submit all and finish verify that for the student when you review your attempt the answer to the essay question is blank and that the attempt has been automatically graded. Then login as a teacher and verify that the essay question is not marked as needing grading. Attempt the quiz as a student and go on each page, verify that on page 2 the essay question is correctly displayed with the response template in the answer field, but don't change anything. Verify that on the summary page the essay question is marked as not yet answered. After clicking on submit all and finish verify that for the student when you review your attempt the answer to the essay question is blank and the attempt has been automatically graded. Then login as a teacher and verify that the essay question is not marked as needing grading. Attempt the quiz as a student and completely change the answer to the essay question. Verify that you new response has been submitted as response to the essay question and that when the student review this attempt it is marked as not yet graded. Then login as a teacher and verify that the attempt is marked as needing manual grading. Attempt the quiz and modify slightly the text of the template in the answer field of the essay question. Verify that your modification has been saved correctly when you change page and return to the essay question page. Verify that it has also been submitted as answer to the essay question when the attempt is submitted and that when you login as a teacher the attempt is marked as needing manual grading. Import/Export Try to import an old (exported before this issue is integrated) Moodle XML file with essay questions. Verify the responsetemplate and responsetemplateformat are correctly populated by empty string and 1 and that no error or warning is displayed during import. Export a category with some essay questions (both with nothing in responseformat and with some content in responseformat) using Moodle XML format. Verify that the new informations are present in the XML. Import these questions in another category and verify that responsetemplate is correctly imported and no error or warning is displayed during import. Backup/Restore Create a quiz with all questions from a category containing essay questions (both with nothing in responseformat and with some content in responseformat) verify that no information is lost when the backup is restored. Try to restore an 1.9 course backup with some essay questions and verify that essay questions are correctly restored with empty string as responsetemplate and 1 as responsetemplateformat.
    • Affected Branches:
      MOODLE_18_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull Master Branch:
    • Rank:
      3750

      Description

      A pre-populated essay question allows teachers to pre-populate the "Answer" area of the essay question (i.e., the html area in which the student types) with arbitrary html. For example, the teacher could insert an empty table that students fill in.

      This could be a new feature of the existing Essay question type or an altogether new question type.

      There have been numerous requests for special question types (e.g., for accounting) that would be satisfied by this simple feature.

        Issue Links

          Activity

          Hide
          Rick Barnes added a comment -

          This could be a very useful tool in on-line assignments as well.
          Would uploading a text or html file as the template be possible in the same way that you can upload a file as the first page of a wiki.
          Rick.

          Show
          Rick Barnes added a comment - This could be a very useful tool in on-line assignments as well. Would uploading a text or html file as the template be possible in the same way that you can upload a file as the first page of a wiki. Rick.
          Hide
          Thomas Park added a comment -

          I am teaching a cryptography course that would really benefit from allowing the instructor to create tables and pre-populated prompts that the student later fills in with values.

          Show
          Thomas Park added a comment - I am teaching a cryptography course that would really benefit from allowing the instructor to create tables and pre-populated prompts that the student later fills in with values.
          Hide
          Dan Jeffries added a comment -

          I agree with this - would be a great addition!

          Show
          Dan Jeffries added a comment - I agree with this - would be a great addition!
          Hide
          Lynne Burrow added a comment -

          For any even remotely mathematical or scientific subjects the ability to get students to fill in tables is a basic function.
          An additional one again would be to be able to draw charts from these tables!

          Show
          Lynne Burrow added a comment - For any even remotely mathematical or scientific subjects the ability to get students to fill in tables is a basic function. An additional one again would be to be able to draw charts from these tables!
          Hide
          CLAIRE BROWNE added a comment -

          I would like a template of a table in the answer text field in essay type questions.

          Show
          CLAIRE BROWNE added a comment - I would like a template of a table in the answer text field in essay type questions.
          Hide
          Jean-Michel Vedrine added a comment -

          The first thing to decide is:
          should this be a Moodle addon or a change to the core essay question type. Only Tim Hunt the question component maintainer can decide this.
          Then we can decide the UI and create the code.
          My personal vote would be to improve core question type and don't create a setting or a checkbox or any complicated thing to switch between essay without template and essay with template. As I see it we can just add 2 fields to the qtype_essay_options table and an editor field to the form to create/edit essay question.
          If I am not wrong this would not add too much overhead to the essay question (maybe just to deal with files associated with the content of this field if any image/sound, ... are included) so an essay without template would just be one were this field is empty and an essay with template one with this field containing something.
          Please can all interested people comment ?

          Show
          Jean-Michel Vedrine added a comment - The first thing to decide is: should this be a Moodle addon or a change to the core essay question type. Only Tim Hunt the question component maintainer can decide this. Then we can decide the UI and create the code. My personal vote would be to improve core question type and don't create a setting or a checkbox or any complicated thing to switch between essay without template and essay with template. As I see it we can just add 2 fields to the qtype_essay_options table and an editor field to the form to create/edit essay question. If I am not wrong this would not add too much overhead to the essay question (maybe just to deal with files associated with the content of this field if any image/sound, ... are included) so an essay without template would just be one were this field is empty and an essay with template one with this field containing something. Please can all interested people comment ?
          Hide
          Ben Reynolds added a comment -

          I'm rather amazed to vote for this today when the guy down the hall from me voted for it 6 years ago.
          J-M, I noticed your comment in the discussion about the odd title for this item. It will be difficult to explain how this essay question modification connects to filling in tables and such.

          Show
          Ben Reynolds added a comment - I'm rather amazed to vote for this today when the guy down the hall from me voted for it 6 years ago. J-M, I noticed your comment in the discussion about the odd title for this item. It will be difficult to explain how this essay question modification connects to filling in tables and such.
          Hide
          Tim Hunt added a comment -

          Jean-Michel: you are right. This should be added to the standard qtype_essay:

          1. two new DB fields in qtype_essay_options: responsetemplate and responsetemplateformat.
          2. one new field on the edit form.
          3. ensure the settings is saved when the form is saved, and included when it is edited.
          4. ensure the new data is included in backup and restore.
          5. ensure the new data is included in Moodle XML import/export (+ unit test).
          6. ensure the new data is included in init_question.
          7. ensure that the template is used when starting an attempt. I think the way to do this is in the renderer. If $qa->get_latest_qt_var('answer') returns null, then display the template in the editor when showing the editor.
          8. unit tests for this in the walkthrough test.
          Show
          Tim Hunt added a comment - Jean-Michel: you are right. This should be added to the standard qtype_essay: two new DB fields in qtype_essay_options: responsetemplate and responsetemplateformat. one new field on the edit form. ensure the settings is saved when the form is saved, and included when it is edited. ensure the new data is included in backup and restore. ensure the new data is included in Moodle XML import/export (+ unit test). ensure the new data is included in init_question. ensure that the template is used when starting an attempt. I think the way to do this is in the renderer. If $qa->get_latest_qt_var('answer') returns null, then display the template in the editor when showing the editor. unit tests for this in the walkthrough test.
          Hide
          Jean-Michel Vedrine added a comment -

          I began doing a branch but already has a question ! Sorry.
          In qtype_essay_question::check_file_access I am unsure of who should be able to visualize files from responsetemplate so what should I return when $component == 'qtype_essay' && $filearea == 'responsetemplate'

          Show
          Jean-Michel Vedrine added a comment - I began doing a branch but already has a question ! Sorry. In qtype_essay_question::check_file_access I am unsure of who should be able to visualize files from responsetemplate so what should I return when $component == 'qtype_essay' && $filearea == 'responsetemplate'
          Hide
          Jean-Michel Vedrine added a comment -

          Summary of the things still to do

          1. What should be the right version change ? I don't really know the rules here for a core plugin.
          2. should responsetemplate and responsetemplate format be filled with something for existing essay questions in upgrade.php
          3. The walkthrough test is not done
          4. And the big one: I don't know how to manage files in responsetemplate when I copy it into answer !! I must first understand how files are managed in responses and also look at what file library functions I should use to copy the files to another filearea
          Show
          Jean-Michel Vedrine added a comment - Summary of the things still to do What should be the right version change ? I don't really know the rules here for a core plugin. should responsetemplate and responsetemplate format be filled with something for existing essay questions in upgrade.php The walkthrough test is not done And the big one: I don't know how to manage files in responsetemplate when I copy it into answer !! I must first understand how files are managed in responses and also look at what file library functions I should use to copy the files to another filearea
          Hide
          Jean-Michel Vedrine added a comment -

          I think that what I need to do to manage files is something like

                      $fs = get_file_storage();
                      $files = $fs->get_area_files($contextid, 'qtype_essay', 'responsetemplate', $question->id);
                      foreach ($files as $file) {
                          $file_record = array('contextid' => $newcontextid, 'component' => 'question', 'filearea' => 'response_answer', 'itemid'=>$newitemid,
                                           'filepath'=>$newfilepath, 'userid'=>$newuserid);
                          $fs->create_file_from_storedfile($file_record, $file);
                      }
          

          But unfortunately I have some problems

          1. I don't know how to retreive $contextid of the question category
          2. I have some problem to understand some of the right values I must give to some elements of the new $file_record I am creating: $newcontextid, $newitemid, $newfilepath, $newuserid

          I will continue to work on this

          Show
          Jean-Michel Vedrine added a comment - I think that what I need to do to manage files is something like $fs = get_file_storage(); $files = $fs->get_area_files($contextid, 'qtype_essay', 'responsetemplate', $question->id); foreach ($files as $file) { $file_record = array('contextid' => $newcontextid, 'component' => 'question', 'filearea' => 'response_answer', 'itemid'=>$newitemid, 'filepath'=>$newfilepath, 'userid'=>$newuserid); $fs->create_file_from_storedfile($file_record, $file); } But unfortunately I have some problems I don't know how to retreive $contextid of the question category I have some problem to understand some of the right values I must give to some elements of the new $file_record I am creating: $newcontextid, $newitemid, $newfilepath, $newuserid I will continue to work on this
          Hide
          Jean-Michel Vedrine added a comment - - edited

          Hum in fact the previous code is wrong : we don't know if this question attempt will be saved or not so creating the file record in the response_answer filearea is not a good thing.
          I really need to understand all the stuff in the question engine related to response files before trying to write some code !!

          Show
          Jean-Michel Vedrine added a comment - - edited Hum in fact the previous code is wrong : we don't know if this question attempt will be saved or not so creating the file record in the response_answer filearea is not a good thing. I really need to understand all the stuff in the question engine related to response files before trying to write some code !!
          Hide
          Jean-Michel Vedrine added a comment -

          Other problem: what should happen if the question creator select "HTML editor" (without file picker) and include some images or other file in the template ??

          Show
          Jean-Michel Vedrine added a comment - Other problem: what should happen if the question creator select "HTML editor" (without file picker) and include some images or other file in the template ??
          Hide
          Jean-Michel Vedrine added a comment -

          Maybe I am not working at the good place in the renderer ? Maybe prepare_response_for_editing would be a far better place ?

          Show
          Jean-Michel Vedrine added a comment - Maybe I am not working at the good place in the renderer ? Maybe prepare_response_for_editing would be a far better place ?
          Hide
          Jean-Michel Vedrine added a comment -

          I have pushed a new version where file management works for qtype_essay_format_editorfilepicker_renderer using my last idea to modify prepare_response_for_editing.
          I was forced to change API for prepare_response_for_editing function adding $qa parameter. This should not be a big problem as this is surely for master branch only.
          Tim, I will now wait for your comments before going further, as I don't really know if the path I am following is the good one.

          Show
          Jean-Michel Vedrine added a comment - I have pushed a new version where file management works for qtype_essay_format_editorfilepicker_renderer using my last idea to modify prepare_response_for_editing. I was forced to change API for prepare_response_for_editing function adding $qa parameter. This should not be a big problem as this is surely for master branch only. Tim, I will now wait for your comments before going further, as I don't really know if the path I am following is the good one.
          Hide
          Tim Hunt added a comment -

          I am wondering if it is useful to support files in the template.

          It is much more difficult to do, and I cannot see much benefit.

          Perhaps we should just implement text-only (that is, html-only) templates.

          Show
          Tim Hunt added a comment - I am wondering if it is useful to support files in the template. It is much more difficult to do, and I cannot see much benefit. Perhaps we should just implement text-only (that is, html-only) templates.
          Hide
          Jean-Michel Vedrine added a comment -

          hello Tim,
          Maybe you are right. My initial thinking was that one day me or someone else might be able to finish PaintWeb TinyMCE plugin so that my students would be able to draw on the math images included in the template. But I am surely overcomplicating things.
          So if we limit template to (html) text.
          We want question creators to have an editor to build their templates don't we ?
          Should it be an editor without filepicker so that they can't inadvertently include files ?

          Show
          Jean-Michel Vedrine added a comment - hello Tim, Maybe you are right. My initial thinking was that one day me or someone else might be able to finish PaintWeb TinyMCE plugin so that my students would be able to draw on the math images included in the template. But I am surely overcomplicating things. So if we limit template to (html) text. We want question creators to have an editor to build their templates don't we ? Should it be an editor without filepicker so that they can't inadvertently include files ?
          Hide
          Tim Hunt added a comment -

          The paint-web use case is something for the future. For now, let us do the simple thing that will make a lot of people happy.

          Yes, HTML editor to create the template. There is an option for whether the editor allows the file-picker or not.

          Show
          Tim Hunt added a comment - The paint-web use case is something for the future. For now, let us do the simple thing that will make a lot of people happy. Yes, HTML editor to create the template. There is an option for whether the editor allows the file-picker or not.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          I pushed a new version where files are not allowed in templates: the filepicker is not allowed for the responsetemplate editor during essay question creation/edition and files are no more supported in all parts of the code.

          1. I am still unsure of what new version number I should use
          2. Should existing essay questions responsetemplate and responsetemplateformat fields be filled with something during upgrade ?
          3. What should I test during the walkthrough test ?
          Show
          Jean-Michel Vedrine added a comment - Hello Tim, I pushed a new version where files are not allowed in templates: the filepicker is not allowed for the responsetemplate editor during essay question creation/edition and files are no more supported in all parts of the code. I am still unsure of what new version number I should use Should existing essay questions responsetemplate and responsetemplateformat fields be filled with something during upgrade ? What should I test during the walkthrough test ?
          Hide
          Tim Hunt added a comment -

          1. set qtype_essay version to today's date (or some other date around now, the exact value does not matter).

          2. On upgrade, set responsetemplate to '' (empty string) and responsetemplateformat to FORMAT_HTML.

          3. For the test, you will need to add a new sample question to question/type/essay/tests/helper.php. Then I think you just need to test that when you render the question in its initial state, the template is in the editor. You should probably then submit a completely different response, and make sure that none of the template text appears after that.

          4. When restoring a backup from 2.4, the two new fields will not be there. You should set them. Something like

          if (!isset($data->responsetemplate)) {
              $data->responsetemplate = '';
          }
          
          if (!isset($data->responsetemplateformat)) {
              $data->responsetemplateformat = FORMAT_HTML;
          }
          

          in process_essay.

          5. I think we can improve the help string:
          $string['responsetemplate_help'] = 'If not empty, the content of the response template will be used to pre-populate student\'s response editor during attempt.';

          Actually, it is really hard to think of something better, but I think we should try. The string we have at the moment does not seem very user-friendly to me.

          6. I don't really like the way you have had to make a fake $step to use the template in the renderer. However, I can see that it was necessary, because of the way the qtype_essay_format_editor_renderer API works. It would be a big job to do it any other way, and it is not worth the effort.

          I don't think it is quite right, however. I think you also need to set 'answerformat' when creating it.

          When using the editor (+filepicker) input types, they call various methods on $step which use $step->id, which of course is not set on your fake step. As far as I can see, that will not actually break, but it is a bit scary.

          I wonder if it is worth recording any of this as a comment in the code.

          7. Here is another case to consider: Suppose we have a quiz where there is an essay question on page 2. The student starts and attempt. On page 1, the click the 'Finish attempt ...' link, and then click 'Submit all and finish'. On the review page, should any content be shown in the essay question? Is what is shown on the review page the same as what is shown in the Responses report?

          Once we decide what the correct behaviour is here, we should probably add unit tests to prove it.


          Good effort. This is a is going to be a really useful addition, and it did not turn out to be as simple as we thought at first, but we are nearly there.

          Show
          Tim Hunt added a comment - 1. set qtype_essay version to today's date (or some other date around now, the exact value does not matter). 2. On upgrade, set responsetemplate to '' (empty string) and responsetemplateformat to FORMAT_HTML. 3. For the test, you will need to add a new sample question to question/type/essay/tests/helper.php. Then I think you just need to test that when you render the question in its initial state, the template is in the editor. You should probably then submit a completely different response, and make sure that none of the template text appears after that. 4. When restoring a backup from 2.4, the two new fields will not be there. You should set them. Something like if (!isset($data->responsetemplate)) { $data->responsetemplate = ''; } if (!isset($data->responsetemplateformat)) { $data->responsetemplateformat = FORMAT_HTML; } in process_essay. 5. I think we can improve the help string: $string ['responsetemplate_help'] = 'If not empty, the content of the response template will be used to pre-populate student\'s response editor during attempt.'; Actually, it is really hard to think of something better, but I think we should try. The string we have at the moment does not seem very user-friendly to me. 6. I don't really like the way you have had to make a fake $step to use the template in the renderer. However, I can see that it was necessary, because of the way the qtype_essay_format_editor_renderer API works. It would be a big job to do it any other way, and it is not worth the effort. I don't think it is quite right, however. I think you also need to set 'answerformat' when creating it. When using the editor (+filepicker) input types, they call various methods on $step which use $step->id, which of course is not set on your fake step. As far as I can see, that will not actually break, but it is a bit scary. I wonder if it is worth recording any of this as a comment in the code. 7. Here is another case to consider: Suppose we have a quiz where there is an essay question on page 2. The student starts and attempt. On page 1, the click the 'Finish attempt ...' link, and then click 'Submit all and finish'. On the review page, should any content be shown in the essay question? Is what is shown on the review page the same as what is shown in the Responses report? Once we decide what the correct behaviour is here, we should probably add unit tests to prove it. Good effort. This is a is going to be a really useful addition, and it did not turn out to be as simple as we thought at first, but we are nearly there.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          Thanks a lot for these comments. I must admit I am rather frightened because as you say when I started I was thinking it was a simple change but I discovered problems while working on it.
          So I am a bit afraid to break something in a widely used question type.
          I will try as much as I can to do it right because I too think it will be a useful addition.
          I must say the idea to create a fake step was not mine. I was having problem to modify the renderer when someone posted a link to a customized question type called essaywithstart made by UW Madison. This is where I picked the idea. I used a code quite similar to to one in essaywith start, at that time I was thinking to make that fake step as similar as possible to a real one but I forgot to do it later. Can we do something about step->id ?
          Your question about review page and response report remind me that I forgot to ask "What is an unanswered essay question with template ? Is it's response empty or does it contains the unmodified template ? Does it depends if the student has seen the question or not ?" Despite having spend some time on this question I don't have a definite answer.

          Show
          Jean-Michel Vedrine added a comment - Hello Tim, Thanks a lot for these comments. I must admit I am rather frightened because as you say when I started I was thinking it was a simple change but I discovered problems while working on it. So I am a bit afraid to break something in a widely used question type. I will try as much as I can to do it right because I too think it will be a useful addition. I must say the idea to create a fake step was not mine. I was having problem to modify the renderer when someone posted a link to a customized question type called essaywithstart made by UW Madison. This is where I picked the idea. I used a code quite similar to to one in essaywith start, at that time I was thinking to make that fake step as similar as possible to a real one but I forgot to do it later. Can we do something about step->id ? Your question about review page and response report remind me that I forgot to ask "What is an unanswered essay question with template ? Is it's response empty or does it contains the unmodified template ? Does it depends if the student has seen the question or not ?" Despite having spend some time on this question I don't have a definite answer.
          Hide
          Jean-Michel Vedrine added a comment -

          I don't know if there isn't in fact a problem with the fake step not having a proper id.
          qtype_essay_format_editorfilepicker_renderer::prepare_response is calling $qa->rewrite_response_pluginfile_urls whith in turn call $step->rewrite_response_pluginfile_urls whith return question_rewrite_question_urls that's using $step->id as one of it's parameters.
          Of course we have prevented files in template but I need to test if when answerformat is set to editorfilepicker this is not causing problems for files added by students.

          Show
          Jean-Michel Vedrine added a comment - I don't know if there isn't in fact a problem with the fake step not having a proper id. qtype_essay_format_editorfilepicker_renderer::prepare_response is calling $qa->rewrite_response_pluginfile_urls whith in turn call $step->rewrite_response_pluginfile_urls whith return question_rewrite_question_urls that's using $step->id as one of it's parameters. Of course we have prevented files in template but I need to test if when answerformat is set to editorfilepicker this is not causing problems for files added by students.
          Hide
          Jean-Michel Vedrine added a comment -

          Well in fact it is not working as expected ! The first time the essay question is displayed (the one where the response field is filled with the template content) the editor don't appear on the page but if you change page and return to it then the editor is there.
          How did I missed that in my previous tests ? Or was it working at that time and has been broken since ?

          Show
          Jean-Michel Vedrine added a comment - Well in fact it is not working as expected ! The first time the essay question is displayed (the one where the response field is filled with the template content) the editor don't appear on the page but if you change page and return to it then the editor is there. How did I missed that in my previous tests ? Or was it working at that time and has been broken since ?
          Hide
          Jean-Michel Vedrine added a comment -

          Yes in fact I broke it when I tried this morning to set answerformat when creating my fake step !
          But why 'answerformat'=>$question->responseformat is wrong ?

          Show
          Jean-Michel Vedrine added a comment - Yes in fact I broke it when I tried this morning to set answerformat when creating my fake step ! But why 'answerformat'=>$question->responseformat is wrong ?
          Hide
          Jean-Michel Vedrine added a comment -

          After looking at the code I am inclined to think we should not set answerformat when creating the fake step so that in response_area_input the editors_get_preferred_editor is called with a null parameter as it is currently the case with an unanswered essay question.
          But in any case setting answerformat to question->responseformat was of course wrong, I should have looked at how it was using later.
          I will wait for your comments on this point and work on the other points of your review.

          Show
          Jean-Michel Vedrine added a comment - After looking at the code I am inclined to think we should not set answerformat when creating the fake step so that in response_area_input the editors_get_preferred_editor is called with a null parameter as it is currently the case with an unanswered essay question. But in any case setting answerformat to question->responseformat was of course wrong, I should have looked at how it was using later. I will wait for your comments on this point and work on the other points of your review.
          Hide
          Jean-Michel Vedrine added a comment -

          About the unfriendliness of the help string, after speaking to other teachers trying to explain what the template is used for, it seems that it helps a lot them if I say that it is used as a starting point (not sure if this is the correct English word) for the student response.
          For most of them the word template is not clear enough

          Show
          Jean-Michel Vedrine added a comment - About the unfriendliness of the help string, after speaking to other teachers trying to explain what the template is used for, it seems that it helps a lot them if I say that it is used as a starting point (not sure if this is the correct English word) for the student response. For most of them the word template is not clear enough
          Hide
          Jean-Michel Vedrine added a comment -

          Here is the current behaviour for a quiz with an essay question with a template on page 2

          1. If the student starts and attempt. On page 1, he click the 'Finish attempt ...' link, and then click 'Submit all and finish'. On the review page the essay question is shown as not answered and empty (the template don't appear). This true for both the student review and if the teacher click on Review attempt link. In the overview report the question is marked as wrong (red cross and red style with the dash used for empty responses. There is no "require grading" link. In the responses report the situation is pretty similar (red cross red style and dash)
          2. If the student start an attempt, click next to go to page 2 click next again without entering anything, in the quiz summary the question is marked as "answer saved". If he click to finish the attempt and submit, on the review page, the question is marked as complete and with the unmodified template as content. For the teacher in the overview report the question has the "Require grading link" and if the teacher click this link, he see the template content as the student answer. In the responses reoprt the content of the template is shown as the student response with the "Review response link".

          I am not saying this is the correct behaviour, just that this is what happen with the current code. As you say we must decide what should be the correct behaviour.

          Show
          Jean-Michel Vedrine added a comment - Here is the current behaviour for a quiz with an essay question with a template on page 2 If the student starts and attempt. On page 1, he click the 'Finish attempt ...' link, and then click 'Submit all and finish'. On the review page the essay question is shown as not answered and empty (the template don't appear). This true for both the student review and if the teacher click on Review attempt link. In the overview report the question is marked as wrong (red cross and red style with the dash used for empty responses. There is no "require grading" link. In the responses report the situation is pretty similar (red cross red style and dash) If the student start an attempt, click next to go to page 2 click next again without entering anything, in the quiz summary the question is marked as "answer saved". If he click to finish the attempt and submit, on the review page, the question is marked as complete and with the unmodified template as content. For the teacher in the overview report the question has the "Require grading link" and if the teacher click this link, he see the template content as the student answer. In the responses reoprt the content of the template is shown as the student response with the "Review response link". I am not saying this is the correct behaviour, just that this is what happen with the current code. As you say we must decide what should be the correct behaviour.
          Hide
          Tim Hunt added a comment -

          Like you, I cannot decide what the correct behaviour is. I will try to ask some other people at the Dublin MoodleMoot to see what they say.

          Just to note the other possible way to solve this:

          Instead of using the template when the question is rendered, the alternative would be to copy the template value into the $qa during start_attempt. I think, however, that causes more problems than it solves.

          Show
          Tim Hunt added a comment - Like you, I cannot decide what the correct behaviour is. I will try to ask some other people at the Dublin MoodleMoot to see what they say. Just to note the other possible way to solve this: Instead of using the template when the question is rendered, the alternative would be to copy the template value into the $qa during start_attempt. I think, however, that causes more problems than it solves.
          Hide
          Jean-Michel Vedrine added a comment -

          Yes I think asking other people is the way to go.
          Maybe you can also ask them what would be useful in the help string ?
          A summary of the current state of the points outlined in your review

          1. Done.
          2. Done.
          3. To do.
          4. Done.
          5. Help string should be improved if we can.
          6. If we keep the fake step method, should answerformat be initialized or not ? I must study this further. Also the problem with step->id should be clarified.
          7. What is the correct behaviour still need to be decided, implemented and tested
          Show
          Jean-Michel Vedrine added a comment - Yes I think asking other people is the way to go. Maybe you can also ask them what would be useful in the help string ? A summary of the current state of the points outlined in your review Done. Done. To do. Done. Help string should be improved if we can. If we keep the fake step method, should answerformat be initialized or not ? I must study this further. Also the problem with step->id should be clarified. What is the correct behaviour still need to be decided, implemented and tested
          Hide
          Helen Foster added a comment -

          Re. 5, I suggest the following for the help string:

          Any text entered here will be displayed in the text box when attempting the question.

          (My suggestion uses the word 'text box' rather than editor, since the text editor may be disabled.)

          Show
          Helen Foster added a comment - Re. 5, I suggest the following for the help string: Any text entered here will be displayed in the text box when attempting the question. (My suggestion uses the word 'text box' rather than editor, since the text editor may be disabled.)
          Hide
          Tim Hunt added a comment -

          That is going in a good direction. I wonder if we can get rid of the words 'editor' or 'text box' altogether. The important thing is that it is going to become the student's response, not what they will use to type it.

          Any text entered here will be used as a starting point for the response when the question is attempted.

          I worry that is too vague.

          Show
          Tim Hunt added a comment - That is going in a good direction. I wonder if we can get rid of the words 'editor' or 'text box' altogether. The important thing is that it is going to become the student's response, not what they will use to type it. Any text entered here will be used as a starting point for the response when the question is attempted. I worry that is too vague.
          Hide
          Helen Foster added a comment -

          An advantage of using 'text box' is that it makes it clear where the text will be displayed.

          I think 'starting point' could easily be misunderstood, however it's just my opinion, so please feel free to use whatever wording you think is best.

          Show
          Helen Foster added a comment - An advantage of using 'text box' is that it makes it clear where the text will be displayed. I think 'starting point' could easily be misunderstood, however it's just my opinion, so please feel free to use whatever wording you think is best.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Helen, hello Tim,
          I definitely can't help you for the help text as English is not my native language. Maybe there is a better word in English than starting point to express how this text will be used ?
          I am quite sure both of you will come with a perfect help text It has already improved quite a lot.
          In the meantime, I will work on the walkthrough test.

          Show
          Jean-Michel Vedrine added a comment - Hello Helen, hello Tim, I definitely can't help you for the help text as English is not my native language. Maybe there is a better word in English than starting point to express how this text will be used ? I am quite sure both of you will come with a perfect help text It has already improved quite a lot. In the meantime, I will work on the walkthrough test.
          Hide
          Tim Hunt added a comment -

          I will think about the help string tomorrow, when I am less tired. We are close, just need to make a final decision.

          Show
          Tim Hunt added a comment - I will think about the help string tomorrow, when I am less tired. We are close, just need to make a final decision.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          While writing the walkthrough test I have some problem finding the way to save the essay question without entering anything to verify that the submitted response is equal to the responsetemplate content. I know how to submit a different answer or a blank answer, but I must be missing something obvious.

          Show
          Jean-Michel Vedrine added a comment - Hello Tim, While writing the walkthrough test I have some problem finding the way to save the essay question without entering anything to verify that the submitted response is equal to the responsetemplate content. I know how to submit a different answer or a blank answer, but I must be missing something obvious.
          Hide
          Tim Hunt added a comment -

          If the user does not edit the template, and just clicks submit, then the template text will be sent to the server. You have to simulate that by explicitly putting the template text into the process_submission call.

          I have just noticed an odd thing. I wonder why the existing tests in /question/type/essay/tests/walkthrough_test.php are using $this->quba->process_all_actions instead of the convenience method $this->process_submission? I should have added a comment explaining that, but I did not. Oops!

          Show
          Tim Hunt added a comment - If the user does not edit the template, and just clicks submit, then the template text will be sent to the server. You have to simulate that by explicitly putting the template text into the process_submission call. I have just noticed an odd thing. I wonder why the existing tests in /question/type/essay/tests/walkthrough_test.php are using $this->quba->process_all_actions instead of the convenience method $this->process_submission? I should have added a comment explaining that, but I did not. Oops!
          Hide
          Jean-Michel Vedrine added a comment - - edited

          Hello Tim,
          Yes I too noticed the use of $this->quba->process_all_actions and wondered if there was a reason for this or not.
          I should have made clear what my problem was: I understand I must submit the student's response in all case including when this is just the content of the template but I wanted to take it from the form rather than just writing $fieldname => 'Once upon a time', as it was a new response (not sure I am clear here ?). Maybe I am wrong in trying to do that.
          Also maybe I should use a string with more strange characters than 'Once upon a time' ? I remember seeing a tracker issue recently about that and I also seems to remember you were part of this issue (reporter ?).

          Show
          Jean-Michel Vedrine added a comment - - edited Hello Tim, Yes I too noticed the use of $this->quba->process_all_actions and wondered if there was a reason for this or not. I should have made clear what my problem was: I understand I must submit the student's response in all case including when this is just the content of the template but I wanted to take it from the form rather than just writing $fieldname => 'Once upon a time', as it was a new response (not sure I am clear here ?). Maybe I am wrong in trying to do that. Also maybe I should use a string with more strange characters than 'Once upon a time' ? I remember seeing a tracker issue recently about that and I also seems to remember you were part of this issue (reporter ?).
          Hide
          Tim Hunt added a comment -

          It is good to want to take the submitted data from the previously rendered form. However, it would also be very hard to do, so instead just write $fieldname => 'Once upon a time' (or $fieldname => $question-> responsetemplate). (I had a similar problem when I was trying to test 'Clear incorrect responses for qtype_match with interactive behaviour, and I did not find a good solution then.)

          I think it is good to have a unit test that verifies the common case (e.g. 'Once upon a time') before testing crazy data, so ideally, do both tests, but it is not essential.

          Show
          Tim Hunt added a comment - It is good to want to take the submitted data from the previously rendered form. However, it would also be very hard to do, so instead just write $fieldname => 'Once upon a time' (or $fieldname => $question-> responsetemplate). (I had a similar problem when I was trying to test 'Clear incorrect responses for qtype_match with interactive behaviour, and I did not find a good solution then.) I think it is good to have a unit test that verifies the common case (e.g. 'Once upon a time') before testing crazy data, so ideally, do both tests, but it is not essential.
          Hide
          Jean-Michel Vedrine added a comment -

          hello Tim,
          I don't think this is quite ready.
          Do you think we can get this in before code freeze on March 29th ?
          Is there anything I can do ?

          Show
          Jean-Michel Vedrine added a comment - hello Tim, I don't think this is quite ready. Do you think we can get this in before code freeze on March 29th ? Is there anything I can do ?
          Hide
          Tim Hunt added a comment -

          I hope we can get this in before the code-freeze. I will be around and working on Moodle code between now and then. (Well, except that right now I am about to go to the pub.)

          To save me reading the whole bug again, what do you think still needs to be done. If need be, I will re-read the whole bug, and answer tomorrow.

          Show
          Tim Hunt added a comment - I hope we can get this in before the code-freeze. I will be around and working on Moodle code between now and then. (Well, except that right now I am about to go to the pub.) To save me reading the whole bug again, what do you think still needs to be done. If need be, I will re-read the whole bug, and answer tomorrow.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          Have a nice moment at the pub. you deserve it.
          Maybe you can focus your review on the following points:

          • Is what is presently happening with a quiz with 2 pages (first page any question type, second page essay with some text in the template) when a student don't visit the second page and click on "submit all and finish" the right thing ? Currently the question is marked as not answered. It seems to me this is in fact exactly the same thing that would happen for an essay question with nothing in the response template or what is happening before this feature is added to Moodle, so I really think this is the right thing to do, but I need your confirmation that this is OK
          • If the student did visit the page with the essay question don't change anything then go to another page or click on Submit all and finish the result is exaclty the same as if he typed the content of the response template. Here also I think this is the right thing to do but here also I need your confirmation.
          • I am not really sure testing instructions are complete. Do you see other things to test ?
          • Would it be interesting to add another phpunit test ?
          • I think that of your initial review only points 6 and 7 still need attention.

          But I must admit I am still fearing I missed some aspect of this functionality. My only relief is that after code freeze it is still allowed to fix bugs and regressions

          Show
          Jean-Michel Vedrine added a comment - Hello Tim, Have a nice moment at the pub. you deserve it. Maybe you can focus your review on the following points: Is what is presently happening with a quiz with 2 pages (first page any question type, second page essay with some text in the template) when a student don't visit the second page and click on "submit all and finish" the right thing ? Currently the question is marked as not answered. It seems to me this is in fact exactly the same thing that would happen for an essay question with nothing in the response template or what is happening before this feature is added to Moodle, so I really think this is the right thing to do, but I need your confirmation that this is OK If the student did visit the page with the essay question don't change anything then go to another page or click on Submit all and finish the result is exaclty the same as if he typed the content of the response template. Here also I think this is the right thing to do but here also I need your confirmation. I am not really sure testing instructions are complete. Do you see other things to test ? Would it be interesting to add another phpunit test ? I think that of your initial review only points 6 and 7 still need attention. But I must admit I am still fearing I missed some aspect of this functionality. My only relief is that after code freeze it is still allowed to fix bugs and regressions
          Hide
          Jean-Michel Vedrine added a comment -

          there is also the problem of improving the help text

          Show
          Jean-Michel Vedrine added a comment - there is also the problem of improving the help text
          Hide
          Tim Hunt added a comment -

          Right, so the real problem is if:

          • The students never visits the page with an essay question; or
          • The students does visit that page, but does not change the template.

          I think I have an idea for fixing that. I think the answer lies in the is_same_response method. I think we should just change that so that $this->responsetemplate and '' compare as equal.

          That way, if you go to the question, don't change anything, and then go to the next page, nothing will be saved, so the question stays in the Not answered state (Good), and the next time they visit the page, they see the template again.

          It also means that if the student deletes all the content from the editor, then when they go back to that page, they see the template again. This is perhaps a bit odd, but it is probably also good.

          What do you think.

          I have not done a another detailed code review yet. I will do that at some point, but I trust you to get a patch that is suitable for integration on your own.

          Show
          Tim Hunt added a comment - Right, so the real problem is if: The students never visits the page with an essay question; or The students does visit that page, but does not change the template. I think I have an idea for fixing that. I think the answer lies in the is_same_response method. I think we should just change that so that $this->responsetemplate and '' compare as equal. That way, if you go to the question, don't change anything, and then go to the next page, nothing will be saved, so the question stays in the Not answered state (Good), and the next time they visit the page, they see the template again. It also means that if the student deletes all the content from the editor, then when they go back to that page, they see the template again. This is perhaps a bit odd, but it is probably also good. What do you think. I have not done a another detailed code review yet. I will do that at some point, but I trust you to get a patch that is suitable for integration on your own.
          Hide
          Tim Hunt added a comment -

          Help string, my suggestion:

          Any text entered here will be displayed in the response input box when a new attempt at the question starts.

          I am reviewing now. I think you may already have started work on is_same_response. Please add unit tests. In particular, I think the code at the moment fails because of '0' == '' is true in PHP. I think you need ===.

          Basically, add tests

          $this->assertFalse($essay->is_same_response(
                          array('answer' =>'0'),
                          array('answer' =>''))); 
          $this->assertFalse($essay->is_same_response(
                          array('answer' =>''),
                          array('answer' =>'0'))); 
          

          Anwyay, overall it is looking very close to being ready for integration.

          Show
          Tim Hunt added a comment - Help string, my suggestion: Any text entered here will be displayed in the response input box when a new attempt at the question starts. I am reviewing now. I think you may already have started work on is_same_response. Please add unit tests. In particular, I think the code at the moment fails because of '0' == '' is true in PHP. I think you need ===. Basically, add tests $ this ->assertFalse($essay->is_same_response( array('answer' =>'0'), array('answer' =>''))); $ this ->assertFalse($essay->is_same_response( array('answer' =>''), array('answer' =>'0'))); Anwyay, overall it is looking very close to being ready for integration.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim
          Good plan: for the teacher unmodified template = no real student response
          I modified the is_same_response function. My code is not elegant, and I am not proud of it, but my excuse is that I had a hard day helping trying to help other teachers to write better multichoice questions and it was an utter failure because they didn't see the result as better (in fact they saw it as worse !) than what they had done before !
          As I was rather frightened to screw the is_same_response function I added some tests both with an empty and a non empty responsetemplate.
          Maybe I should also add a test to the walkthrough to test that if the unmodified template is submitted the question is in the todo state (in fact I know this works because this is the reason I was forced to change the walkthrough test to submit a modified answer )

          Show
          Jean-Michel Vedrine added a comment - Hello Tim Good plan: for the teacher unmodified template = no real student response I modified the is_same_response function. My code is not elegant, and I am not proud of it, but my excuse is that I had a hard day helping trying to help other teachers to write better multichoice questions and it was an utter failure because they didn't see the result as better (in fact they saw it as worse !) than what they had done before ! As I was rather frightened to screw the is_same_response function I added some tests both with an empty and a non empty responsetemplate. Maybe I should also add a test to the walkthrough to test that if the unmodified template is submitted the question is in the todo state (in fact I know this works because this is the reason I was forced to change the walkthrough test to submit a modified answer )
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          Are you sure that '0' == '' is true ? I was thinking that '' == 0

          Show
          Jean-Michel Vedrine added a comment - Hello Tim, Are you sure that '0' == '' is true ? I was thinking that '' == 0
          Hide
          Tim Hunt added a comment -

          I am not 100% sure. This is PHP after all. I just know that I broke shortanswer recently, and had to fix it.

          OK, so you are right, '' == 0 is true, '0' == '' is false.

          Which just leaves me puzzled why this change was necessary: https://github.com/moodle/moodle/commit/1736fe3f743ac77ebc01302f685ff66fb6fd604c

          Ah!
          !0
          !''
          !'0'
          are all true.

          Crazy! Can I suggest that you add the unit test anyway? (Don't worry if you are too tired.)

          Show
          Tim Hunt added a comment - I am not 100% sure. This is PHP after all. I just know that I broke shortanswer recently, and had to fix it. OK, so you are right, '' == 0 is true, '0' == '' is false. Which just leaves me puzzled why this change was necessary: https://github.com/moodle/moodle/commit/1736fe3f743ac77ebc01302f685ff66fb6fd604c Ah! !0 !'' !'0' are all true. Crazy! Can I suggest that you add the unit test anyway? (Don't worry if you are too tired.)
          Hide
          Jean-Michel Vedrine added a comment -

          the is_same_response should now work better and I have changed the help string.

          Show
          Jean-Michel Vedrine added a comment - the is_same_response should now work better and I have changed the help string.
          Hide
          Jean-Michel Vedrine added a comment -

          Crazy php compare rules ! But IMHO javascript is worst

          Show
          Jean-Michel Vedrine added a comment - Crazy php compare rules ! But IMHO javascript is worst
          Hide
          Tim Hunt added a comment -

          I think we are there. I guess we just need wait for the weekly release, then you can rebase and squash down to 1 commit, and then we can submit this for integration. Thanks.

          Show
          Tim Hunt added a comment - I think we are there. I guess we just need wait for the weekly release, then you can rebase and squash down to 1 commit, and then we can submit this for integration. Thanks.
          Hide
          Jean-Michel Vedrine added a comment -

          Hello Tim,
          As planned I have rebased and squashed into 1 commit. It's up to you to put in integration review if you agree.
          I have (one more time !) verified phpunit tests and run codechecker

          Show
          Jean-Michel Vedrine added a comment - Hello Tim, As planned I have rebased and squashed into 1 commit. It's up to you to put in integration review if you agree. I have (one more time !) verified phpunit tests and run codechecker
          Hide
          Tim Hunt added a comment -

          Yay! Thanks Jean-Michel. Have a good Easter.

          Show
          Tim Hunt added a comment - Yay! Thanks Jean-Michel. Have a good Easter.
          Hide
          Damyon Wiese added a comment -

          This change looks good to me - thanks Jean-Michel for working on this and Tim for the reviews.

          I fixed one whitespace issue and have integrated this to master only.

          Cheers, Damyon

          Show
          Damyon Wiese added a comment - This change looks good to me - thanks Jean-Michel for working on this and Tim for the reviews. I fixed one whitespace issue and have integrated this to master only. Cheers, Damyon
          Hide
          Jean-Michel Vedrine added a comment -

          thanks Damyon. Sorry for the whitespace but I wonder why the codechecker didn't reported it when I run it just before submitting ?

          Show
          Jean-Michel Vedrine added a comment - thanks Damyon. Sorry for the whitespace but I wonder why the codechecker didn't reported it when I run it just before submitting ?
          Hide
          Jean-Michel Vedrine added a comment -

          Silly me ! I ran the codechecker on the wrong repo !!

          Show
          Jean-Michel Vedrine added a comment - Silly me ! I ran the codechecker on the wrong repo !!
          Hide
          Mark Nelson added a comment -

          Hi Jean,

          Thanks for your contribution, it is appreciated. I just have two points.

          1. Attempt the quiz as a student and go on each page, verify that on page 2 the essay question is correctly displayed with the response template in the answer field, but don't change anything. Verify that on the summary page the essay question is marked as not yet answered.

          As a student I was not able to click on a link to view the summary page when I was viewing the question. Clicking on any question in the 'Quiz navigation' would save the answer, even if it was just the response template with nothing changed. If I visited the course page in the breadcrumbs and then clicked on the quiz again I was given the option to 'Continue the last attempt' which took me back to the page with this question. I don't see how you can get to the summary page without saving the answer (maybe I am missing something?). However, I copied the URL of the summary page and went to this page directly and did see the question marked as 'Not yet answered', so this does technically work, I just don't see how the user can get to the summary page w/o knowing the URL.

          2. Try to restore an 1.9 course backup with some essay questions and verify that essay questions are correctly restored with empty string as responsetemplate and 1 as responsetemplateformat.

          When I restored a 1.9 course the responsetemplateformat value for the essay questions I had created was set to 0, not 1.

          Show
          Mark Nelson added a comment - Hi Jean, Thanks for your contribution, it is appreciated. I just have two points. 1. Attempt the quiz as a student and go on each page, verify that on page 2 the essay question is correctly displayed with the response template in the answer field, but don't change anything. Verify that on the summary page the essay question is marked as not yet answered. As a student I was not able to click on a link to view the summary page when I was viewing the question. Clicking on any question in the 'Quiz navigation' would save the answer, even if it was just the response template with nothing changed. If I visited the course page in the breadcrumbs and then clicked on the quiz again I was given the option to 'Continue the last attempt' which took me back to the page with this question. I don't see how you can get to the summary page without saving the answer (maybe I am missing something?). However, I copied the URL of the summary page and went to this page directly and did see the question marked as 'Not yet answered', so this does technically work, I just don't see how the user can get to the summary page w/o knowing the URL. 2. Try to restore an 1.9 course backup with some essay questions and verify that essay questions are correctly restored with empty string as responsetemplate and 1 as responsetemplateformat. When I restored a 1.9 course the responsetemplateformat value for the essay questions I had created was set to 0, not 1.
          Hide
          Dan Poltawski added a comment -

          Hi,
          In order to keep the release moving, we're going to release with these testing failed issues in place as they only affect master and we'd prefer not to revert.

          I'll keep this issue open in its current state when we close the other issues, so we can resolve this issue when a fix arrives.

          Thanks.
          dan

          Show
          Dan Poltawski added a comment - Hi, In order to keep the release moving, we're going to release with these testing failed issues in place as they only affect master and we'd prefer not to revert. I'll keep this issue open in its current state when we close the other issues, so we can resolve this issue when a fix arrives. Thanks. dan
          Hide
          Jean-Michel Vedrine added a comment -

          Hello,
          Sorry I looked at this when I wake up this morning but it was still waiting for testing. I come back from work and see there was a problem, so I will look into this now. Unfortunately I can't follow tracker issues while giving my lessons

          Show
          Jean-Michel Vedrine added a comment - Hello, Sorry I looked at this when I wake up this morning but it was still waiting for testing. I come back from work and see there was a problem, so I will look into this now. Unfortunately I can't follow tracker issues while giving my lessons
          Hide
          Jean-Michel Vedrine added a comment -

          I forgot to thank Dan for his decision,
          I don't think point 1 is really an issue, it's just I wasn't clear enough in my testing instructions: I wanted the student to go on each page of the quiz, not submit anything on page 2 in the essay question then click on "Finish Attempt ..." link in the "Quiz navigation" to see the "Summary of attempt" with the "Not yet answered" message for question 2. So it somebody is able to reproduce this test I don't think there is anything to fix.

          Point 2 is a bug responsetemplateformat should be 1 for consistency even if user can change it later after restoring the 1.9 backup. I will provide a fix.

          Show
          Jean-Michel Vedrine added a comment - I forgot to thank Dan for his decision, I don't think point 1 is really an issue, it's just I wasn't clear enough in my testing instructions: I wanted the student to go on each page of the quiz, not submit anything on page 2 in the essay question then click on "Finish Attempt ..." link in the "Quiz navigation" to see the "Summary of attempt" with the "Not yet answered" message for question 2. So it somebody is able to reproduce this test I don't think there is anything to fix. Point 2 is a bug responsetemplateformat should be 1 for consistency even if user can change it later after restoring the 1.9 backup. I will provide a fix.
          Hide
          Jean-Michel Vedrine added a comment - - edited

          Heelo Tim, I need you advice,
          in question/essay/type/backup/moodle1/lib.php in process_question there was:

          'graderinfoformat'       => FORMAT_MOODLE,
          

          so I added:

          'responsetemplateformat' => FORMAT_MOODLE
          

          because I thought that was logical when I wrote it. But later when I wrote testing instructions, I wrote :
          Try to restore an 1.9 course backup with some essay questions and verify that essay questions are correctly restored with empty string as responsetemplate and 1 as responsetemplateformat.
          So we can decide my testing instructions were wrong and let the code as it is
          Or we can change the code to:

          'responsetemplateformat' => FORMAT_HTML
          

          and let the testing instructions as they are.
          My preference goes to the later: as we are creating a new empty content why force the user to change the format first, save and re-edit if he wants to use an editor to put something here surely FORMAT_htML is what most users want ? I even think that graderinfoformat could be changed to FORMAT_HTML too.

          Show
          Jean-Michel Vedrine added a comment - - edited Heelo Tim, I need you advice, in question/essay/type/backup/moodle1/lib.php in process_question there was: 'graderinfoformat' => FORMAT_MOODLE, so I added: 'responsetemplateformat' => FORMAT_MOODLE because I thought that was logical when I wrote it. But later when I wrote testing instructions, I wrote : Try to restore an 1.9 course backup with some essay questions and verify that essay questions are correctly restored with empty string as responsetemplate and 1 as responsetemplateformat. So we can decide my testing instructions were wrong and let the code as it is Or we can change the code to: 'responsetemplateformat' => FORMAT_HTML and let the testing instructions as they are. My preference goes to the later: as we are creating a new empty content why force the user to change the format first, save and re-edit if he wants to use an editor to put something here surely FORMAT_htML is what most users want ? I even think that graderinfoformat could be changed to FORMAT_HTML too.
          Hide
          Tim Hunt added a comment -

          Jean-Michel, so you think you will have time to look into this?

          Show
          Tim Hunt added a comment - Jean-Michel, so you think you will have time to look into this?
          Hide
          Tim Hunt added a comment -

          Doh! I did not refresh my browser before commenting

          Show
          Tim Hunt added a comment - Doh! I did not refresh my browser before commenting
          Hide
          Tim Hunt added a comment -

          I think change both responsetemplateformat and graderinfoformat to FORMAT_HTML when importing old questions. Thanks.

          Show
          Tim Hunt added a comment - I think change both responsetemplateformat and graderinfoformat to FORMAT_HTML when importing old questions. Thanks.
          Hide
          Jean-Michel Vedrine added a comment - - edited

          Thanks Mark, Dan and Tim,
          You can't guess how happy I am that his is now part of Moodle just in time for 2.5 freeze even if I didn't get Dan's funny message ("Did you remember to call thankDevelopers() for 'this_weeks_work ...") for this one (fortunately I got it for MDL-38808).
          I have created MDL-38981 to take care of the problem with responsetemplateformat value when restoring 1.9 backup.

          Show
          Jean-Michel Vedrine added a comment - - edited Thanks Mark, Dan and Tim, You can't guess how happy I am that his is now part of Moodle just in time for 2.5 freeze even if I didn't get Dan's funny message ("Did you remember to call thankDevelopers() for 'this_weeks_work ...") for this one (fortunately I got it for MDL-38808 ). I have created MDL-38981 to take care of the problem with responsetemplateformat value when restoring 1.9 backup.
          Hide
          Dan Poltawski added a comment -

          Did you remember to call thankDevelopers() for 'this_weeks_work'? Defaulting to PARAM_SHODDY thanking.
          line 1289 of \lib\changes.php: call to debugging()
          line 281 of \lib\are.php: call to moodleform->detectMissingThanks()
          line 202 of \lib\now.php: call to moodleform->_is_poor_form()
          line 73 of \course\upstream.php: call to moodleform->forgetingToThank()

          Show
          Dan Poltawski added a comment - Did you remember to call thankDevelopers() for 'this_weeks_work'? Defaulting to PARAM_SHODDY thanking. line 1289 of \lib\changes.php: call to debugging() line 281 of \lib\are.php: call to moodleform->detectMissingThanks() line 202 of \lib\now.php: call to moodleform->_is_poor_form() line 73 of \course\upstream.php: call to moodleform->forgetingToThank()
          Hide
          Dan Poltawski added a comment -

          (thanks, seems we've got it in hand in other issues )

          Show
          Dan Poltawski added a comment - (thanks, seems we've got it in hand in other issues )
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as this is documented here http://docs.moodle.org/25/en/Building_Quiz

          Show
          Mary Cooch added a comment - Removing docs_required label as this is documented here http://docs.moodle.org/25/en/Building_Quiz
          Hide
          Mary Cooch added a comment -

          thanks to Tim Hunt for pointing out better location is http://docs.moodle.org/25/en/Essay_question_type

          Show
          Mary Cooch added a comment - thanks to Tim Hunt for pointing out better location is http://docs.moodle.org/25/en/Essay_question_type

            People

            • Votes:
              16 Vote for this issue
              Watchers:
              12 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: