Moodle
  1. Moodle
  2. MDL-29818

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

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Trivial 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
    • Rank:
      19344

      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

        Activity

        Hide
        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
        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
        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
        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
        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
        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
        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
        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
        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
        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
        Tim Hunt added a comment -

        Thanks Andrew. Submitting for integration now.

        Show
        Tim Hunt added a comment - Thanks Andrew. Submitting for integration now.
        Hide
        Eloy Lafuente (stronk7) added a comment -

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

        TIA and ciao

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

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

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

        All good

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

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

        Closing as fixed, ciao

        Show
        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: