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

Feedback items cannot be unrequired using the checkbox on the several item forms

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 2.1.7, 2.2.4, 2.3.1
    • Fix Version/s: 2.2.7, 2.3.4, 2.4.1
    • Component/s: Feedback
    • Labels:
    • Testing Instructions:
      Hide
      1. Enable the feedback activity in the Site administration
      2. Create a new Feedback activity
      3. Add and save a new multichoice question, making it required
      4. Click the pen-in-hand icon to edit the question
      5. Uncheck the "Required" box and save changes
      6. The question is not required anymore.
      Show
      Enable the feedback activity in the Site administration Create a new Feedback activity Add and save a new multichoice question, making it required Click the pen-in-hand icon to edit the question Uncheck the "Required" box and save changes The question is not required anymore.
    • Workaround:
      Hide

      On the main "Edit questions" screen you can use the exclamation point icon to toggle whether or not a question is required.

      Show
      On the main "Edit questions" screen you can use the exclamation point icon to toggle whether or not a question is required.
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE

      Description

      Create a new Feedback activity
      Add and save a new multichoice question, making it required
      Click the pen-in-hand icon to edit the question
      Uncheck the "Required" box and save changes

      The question should no longer be required, but it still is.

      The problem affects all question types that can be required.

      To fix the problem use 'advcheckbox' instead of 'checkbox' so that an unchecked box will submit a zero. I will post patches for all versions shortly.

        Gliffy Diagrams

          Attachments

            Issue Links

              Activity

              Hide
              grabs Andreas Grabs added a comment -

              Hi Kevin, thank you very much!
              Andreas

              Show
              grabs Andreas Grabs added a comment - Hi Kevin, thank you very much! Andreas
              Hide
              salvetore Michael de Raadt added a comment -

              When you're ready, Andreas, please request a peer review on this.

              Show
              salvetore Michael de Raadt added a comment - When you're ready, Andreas, please request a peer review on this.
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Hi Andreas,
              The patch looks good. I linked the generic checkbox issue for reference.

              However I don’t follow why you have set the class to "checkbox1" . can you justify that please?
              Also it will be good to have some testing instructions.

              Thanks for working on the issue.

              Show
              ankit_frenz Ankit Agarwal added a comment - Hi Andreas, The patch looks good. I linked the generic checkbox issue for reference. However I don’t follow why you have set the class to "checkbox1" . can you justify that please? Also it will be good to have some testing instructions. Thanks for working on the issue.
              Hide
              grabs Andreas Grabs added a comment -

              Hi Ankit,
              the patch isn't mine. But it looks ok.
              I don't know what class do you mean. Where is a class set to "checkbox1"?
              The testing instructions come soon.
              Andreas

              Show
              grabs Andreas Grabs added a comment - Hi Ankit, the patch isn't mine. But it looks ok. I don't know what class do you mean. Where is a class set to "checkbox1"? The testing instructions come soon. Andreas
              Hide
              ankit_frenz Ankit Agarwal added a comment - - edited

              Hi Andreas,
              Sorry didnt notice branches where not on your repo.

              Also it is "checkboxgroup1". Sorry for the typo.

              I am talking about the argument array('group' => 1) for the advcheckbox elements. That is generating the class identifier.
              Have a look on how its handled in moodleQuickForm_advcheckbox::__constructor()

              Thanks

              Show
              ankit_frenz Ankit Agarwal added a comment - - edited Hi Andreas, Sorry didnt notice branches where not on your repo. Also it is "checkboxgroup1". Sorry for the typo. I am talking about the argument array('group' => 1) for the advcheckbox elements. That is generating the class identifier. Have a look on how its handled in moodleQuickForm_advcheckbox::__constructor() Thanks
              Hide
              kwiliarty Kevin Wiliarty added a comment -

              Shall I change that parameter to 'null'? I agree that it is not serving a purpose to assign it to a class when there is no controller (and no need for one).

              Show
              kwiliarty Kevin Wiliarty added a comment - Shall I change that parameter to 'null'? I agree that it is not serving a purpose to assign it to a class when there is no controller (and no need for one).
              Hide
              kwiliarty Kevin Wiliarty added a comment -

              I'm going to go ahead and switch those fifth arguments to nulls.

              Show
              kwiliarty Kevin Wiliarty added a comment - I'm going to go ahead and switch those fifth arguments to nulls.
              Hide
              kwiliarty Kevin Wiliarty added a comment -

              Okay. All the patch branches on my github repo should now have

              $mform->addElement('advcheckbox', 'required', get_string('required', 'feedback'), '' , null , array(0, 1));

              in place of

              $mform->addElement('advcheckbox', 'required', get_string('required', 'feedback'), '' , array('group'=>1) , array(0, 1));

              in all the relevant files.

              Thanks for the input!

              Show
              kwiliarty Kevin Wiliarty added a comment - Okay. All the patch branches on my github repo should now have $mform->addElement('advcheckbox', 'required', get_string('required', 'feedback'), '' , null , array(0, 1)); in place of $mform->addElement('advcheckbox', 'required', get_string('required', 'feedback'), '' , array('group'=>1) , array(0, 1)); in all the relevant files. Thanks for the input!
              Hide
              ankit_frenz Ankit Agarwal added a comment -

              Thanks for the update Kevin.
              Submitting for integration.

              Show
              ankit_frenz Ankit Agarwal added a comment - Thanks for the update Kevin. Submitting for integration.
              Hide
              samhemelryk Sam Hemelryk added a comment -

              Thanks Kevin, your fix has been integrated now.

              Show
              samhemelryk Sam Hemelryk added a comment - Thanks Kevin, your fix has been integrated now.
              Hide
              rajeshtaneja Rajesh Taneja added a comment -

              Thanks Andreas,

              Works fine.

              Show
              rajeshtaneja Rajesh Taneja added a comment - Thanks Andreas, Works fine.
              Hide
              grabs Andreas Grabs added a comment -

              Thank you too. But it wasn't me. The patch was contributed by Kevin.
              Thank you Kevin!
              Best regards
              Andreas

              Show
              grabs Andreas Grabs added a comment - Thank you too. But it wasn't me. The patch was contributed by Kevin. Thank you Kevin! Best regards Andreas
              Hide
              kwiliarty Kevin Wiliarty added a comment -

              I'm still learning the procedures. Is it for me to close the ticket, then?
              Kevin

              Show
              kwiliarty Kevin Wiliarty added a comment - I'm still learning the procedures. Is it for me to close the ticket, then? Kevin
              Hide
              stronk7 Eloy Lafuente (stronk7) added a comment -

              Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now!

              Closing, ciao

              Show
              stronk7 Eloy Lafuente (stronk7) added a comment - Many thanks for your effort, the whole Moodle Community will be enjoying your great solutions starting now! Closing, ciao

                People

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

                  Dates

                  • Created:
                    Updated:
                    Resolved:
                    Fix Release Date:
                    14/Jan/13