Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-29818

Specifying a grade but no answer gives the wrong error on multichoice questions

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Trivial
    • Resolution: Fixed
    • Affects Version/s: 2.1.2, 2.2
    • Fix Version/s: 2.1.3
    • Component/s: Questions
    • Labels:
      None
    • Database:
      Any
    • Testing Instructions:
      Hide
      • Create new quiz
      • Create new question of type 'Multiple choice'
      • Fill in the question name 'A or B?'
      • Set 'One or multiple answers?' to 'Multiple answers allowed'
      • Choice 1: Set Answer to 'A'; Set Grade to 50%
      • Choice 2: Do not set Answer; Set Grade to 50%
      • Choose 'Save changes' at the bottom of the form

      An error message should be shown beside any question which has a grade but no answer indicating that answer is missing

      Show
      Create new quiz Create new question of type 'Multiple choice' Fill in the question name 'A or B?' Set 'One or multiple answers?' to 'Multiple answers allowed' Choice 1: Set Answer to 'A'; Set Grade to 50% Choice 2: Do not set Answer; Set Grade to 50% Choose 'Save changes' at the bottom of the form An error message should be shown beside any question which has a grade but no answer indicating that answer is missing
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-29818-master

      Description

      When creating a multiple choice question, if you have an answer with a grade, but nothing in the 'answer' section, the error message returned suggests that the answers do not add up to 100%

      We've had users do this by inadvertently filling in the Feedback section instead of the answer section

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            dobedobedoh Andrew Nicols added a comment -

            This patch modifies the logic to include answers which have a grade in the validation.

            This does have the effect that to remove an answer you must now unset both the answer, and the grade; but does ensure that a more accurate error message is given.

            Show
            dobedobedoh Andrew Nicols added a comment - This patch modifies the logic to include answers which have a grade in the validation. This does have the effect that to remove an answer you must now unset both the answer, and the grade; but does ensure that a more accurate error message is given.
            Hide
            timhunt Tim Hunt added a comment -

            Basically right, but your patch is a bit sucky.

            1. Please change the error string to "Grade set, but the Answer is blank." (If you agree with me that it is clearer.)

            2. Your change to the code seems unnecessarily spaghetti-like.

            if (empty($trimmedanswer) && empty($data['fraction'][$key])) {
                continue;
            }
            if (empty($trimmedanswer)) {
                $errors['fraction[' . $key . ']'] = get_string('errnoanswerwithgrade', 'qtype_multichoice');
            }

            Seems simpler to me.

            Show
            timhunt Tim Hunt added a comment - Basically right, but your patch is a bit sucky. 1. Please change the error string to "Grade set, but the Answer is blank." (If you agree with me that it is clearer.) 2. Your change to the code seems unnecessarily spaghetti-like. if (empty($trimmedanswer) && empty($data['fraction'][$key])) { continue; } if (empty($trimmedanswer)) { $errors['fraction[' . $key . ']'] = get_string('errnoanswerwithgrade', 'qtype_multichoice'); } Seems simpler to me.
            Hide
            timhunt Tim Hunt added a comment -

            I mean to add. If you can fix your commit, and cherry-pick to all reasonable branches, I will be happy to submit this for integration. Thanks for working on it.

            Show
            timhunt Tim Hunt added a comment - I mean to add. If you can fix your commit, and cherry-pick to all reasonable branches, I will be happy to submit this for integration. Thanks for working on it.
            Hide
            dobedobedoh Andrew Nicols added a comment -

            Cheers Tim,

            I've updated with your comments and pushed branches for 2.1, and master. 2.0 makes life harder because it tests whether the answer is empty a couple of times – I can try and find a non-sucky solution if you wish.

            I've had to cast the fraction to a float first and test that because $data['fraction'] contains string values of 0.0 (which are never empty).

            Thanks for the review

            Show
            dobedobedoh Andrew Nicols added a comment - Cheers Tim, I've updated with your comments and pushed branches for 2.1, and master. 2.0 makes life harder because it tests whether the answer is empty a couple of times – I can try and find a non-sucky solution if you wish. I've had to cast the fraction to a float first and test that because $data ['fraction'] contains string values of 0.0 (which are never empty). Thanks for the review
            Hide
            dobedobedoh Andrew Nicols added a comment -

            I've updated with your comments, and adjusted to cast the fraction to a float first.

            Thank you for the review

            Show
            dobedobedoh Andrew Nicols added a comment - I've updated with your comments, and adjusted to cast the fraction to a float first. Thank you for the review
            Hide
            timhunt Tim Hunt added a comment -

            Thanks Andrew. Submitting for integration now.

            Show
            timhunt Tim Hunt added a comment - Thanks Andrew. Submitting for integration now.
            Hide
            stronk7 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.

            TIA and ciao

            Show
            stronk7 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. TIA and ciao
            Hide
            nebgor Aparup Banerjee added a comment -

            Thanks guys, this has been integrated and is up for testing.

            Show
            nebgor Aparup Banerjee added a comment - Thanks guys, this has been integrated and is up for testing.
            Hide
            phalacee Jason Fowler added a comment -

            All good

            Show
            phalacee Jason Fowler added a comment - All good
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Done, your delicious hacks have been sent upstream, many thanks!

            Closing as fixed, ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Done, your delicious hacks have been sent upstream, many thanks! Closing as fixed, ciao

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                1 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  28/Nov/11