Moodle
  1. Moodle
  2. MDL-34011

Incorrect display of student attempt for Short Answer questions in Lessons

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.3, 2.3, 2.3.5, 2.4.2
    • Fix Version/s: 2.3.6, 2.4.3
    • Component/s: Lesson
    • Labels:
    • Testing Instructions:
      Hide
      1. Create a Short Answer question: e.g. 2+2=
      2. Add two correct answer choices: 4 and four
      3. Take a lesson as a student, typing first possible correct answer: 4
      4. Review the student's result as an instructor and notice that the answer is marked correctly.
      5. Take a lesson as a student again, typing last possible correct answer: four
      6. Review student's result as an instructor to see that the answer is marked to be correct
      Show
      Create a Short Answer question: e.g. 2+2= Add two correct answer choices: 4 and four Take a lesson as a student, typing first possible correct answer: 4 Review the student's result as an instructor and notice that the answer is marked correctly. Take a lesson as a student again, typing last possible correct answer: four Review student's result as an instructor to see that the answer is marked to be correct
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
      MDL-34011_m24
    • Pull Master Branch:
    • Rank:
      42123

      Description

      If you have 2 or more possible correct answers for the Short Answer question, usually only the LAST answer from the list will be marked as correct on the Reports > Individual student report screen for the instructor. Other correct choices will be marked as wrong on that screen, and for some reason they will be duplicated.

      Student will see "correct" message though when taking the Lesson, and calculation of the total for the lesson attempt will be fine, so this is mostly confusion to the instructor.

      • Create a Short Answer question: e.g. 2+2=
      • Add two correct answer choices: 4 and four
      • Take a lesson as a student, typing first possible correct answer: 4
      • Review results as an instructor and notice that the answer is marked to be wrong
      • Take a lesson as a student again, typing last possible correct answer: four
      • Review results as an instructor to see that the answer is marked to be correct

      Discussion: http://moodle.org/mod/forum/discuss.php?d=204882

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          Thanks for reporting this.

          I've put that on the backlog.

          In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.

          Show
          Michael de Raadt added a comment - Thanks for reporting this. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.
          Hide
          Ankit Agarwal added a comment -

          Hi Rosie,
          The patch looks good. Please submit for integration when all branches are ready.
          [y] Syntax
          [y] Output
          [y] Whitespace
          [na] Language
          [na] Databases
          [y] Testing
          [na] Security
          [na] Documentation
          [y] Git
          [y] Sanity check (Not sure if lesson answer loops where ever sane )

          Thanks

          Show
          Ankit Agarwal added a comment - Hi Rosie, The patch looks good. Please submit for integration when all branches are ready. [y] Syntax [y] Output [y] Whitespace [na] Language [na] Databases [y] Testing [na] Security [na] Documentation [y] Git [y] Sanity check (Not sure if lesson answer loops where ever sane ) Thanks
          Hide
          Rossiani Wijaya added a comment -

          Thanks Ankit for reviewing.

          Created patches for the reset of supported branches.

          Submitting for integration review.

          Show
          Rossiani Wijaya added a comment - Thanks Ankit for reviewing. Created patches for the reset of supported branches. Submitting for integration review.
          Hide
          Damyon Wiese added a comment -

          Hi Rosie,

          Thanks for the patch. I found the changes confusing though - you have added local variables, and unneeded if statements - I removed the code that looked redundant and refactored a little and all I was left with was:

          --- a/mod/lesson/pagetypes/shortanswer.php
          +++ b/mod/lesson/pagetypes/shortanswer.php
          @@ -289,6 +289,8 @@ class lesson_page_type_shortanswer extends lesson_page {
                               } else {
                                   $answerdata->score = get_string("didnotreceivecredit", "lesson");
                               }
          +                    // We have found the correct answer, do not process any more answers.
          +                    break;
                           } else {
                               $answerdata->response = get_string("thatsthewronganswer", "lesson");
                               if ($this->lesson->custom) {
          

          I would prefer this patch as it is much smaller/less risk for backporting and the resulting code is much easier to read.

          Can you take a look at this alternative and let me know if you agree?

          Thanks, Damyon

          Show
          Damyon Wiese added a comment - Hi Rosie, Thanks for the patch. I found the changes confusing though - you have added local variables, and unneeded if statements - I removed the code that looked redundant and refactored a little and all I was left with was: --- a/mod/lesson/pagetypes/shortanswer.php +++ b/mod/lesson/pagetypes/shortanswer.php @@ -289,6 +289,8 @@ class lesson_page_type_shortanswer extends lesson_page { } else { $answerdata->score = get_string( "didnotreceivecredit" , "lesson" ); } + // We have found the correct answer, do not process any more answers. + break ; } else { $answerdata->response = get_string( "thatsthewronganswer" , "lesson" ); if ($ this ->lesson->custom) { I would prefer this patch as it is much smaller/less risk for backporting and the resulting code is much easier to read. Can you take a look at this alternative and let me know if you agree? Thanks, Damyon
          Hide
          Rossiani Wijaya added a comment -

          Hi Damyon,

          Thank you for the suggestion. It does simplified the code however it breaks when the user's answer is matched with the first answer option.

          I'm investigating it right now.

          Show
          Rossiani Wijaya added a comment - Hi Damyon, Thank you for the suggestion. It does simplified the code however it breaks when the user's answer is matched with the first answer option. I'm investigating it right now.
          Hide
          Rossiani Wijaya added a comment -

          Found the bug described above.

          Updated the patch for all branches.

          Thanks Damyon.

          Show
          Rossiani Wijaya added a comment - Found the bug described above. Updated the patch for all branches. Thanks Damyon.
          Hide
          Damyon Wiese added a comment -

          Thanks for rechecking this and finding that bug Rosie,

          Integrated to master, 24 and 23 now.

          Cheers - Damyon

          Show
          Damyon Wiese added a comment - Thanks for rechecking this and finding that bug Rosie, Integrated to master, 24 and 23 now. Cheers - Damyon
          Hide
          Adrian Greeve added a comment -

          Tested on the 2.3, 2.4 and master integration branches.
          No problems found.
          Test passed.

          Show
          Adrian Greeve added a comment - Tested on the 2.3, 2.4 and master integration branches. No problems found. Test passed.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).

          Thanks!

          PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

          Show
          Eloy Lafuente (stronk7) added a comment - This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads). Thanks! PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: