Moodle
  1. Moodle
  2. MDL-39412

Disable the manual graded behaviour in the UI

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.3.6, 2.4.3
    • Fix Version/s: 2.5
    • Component/s: Questions, Quiz
    • Labels:
    • Testing Instructions:
      Hide
      1. Install a new Moodle.
      2. Go to Admin -> Plugins -> Question behaviours -> Manage behaviours, and verify that the manually graded behaviour is not available.
      3. Install an older version of Moodle. Go to Admin -> Plugins -> Question behaviours -> Manage behaviours and disable some behaviours, but not manually graded.
      4. Upgrade to the latest version. Verify that manually graded behaviour is now disabled, and that all the other behaviours that you previously disabled are still disabled.
      Show
      Install a new Moodle. Go to Admin -> Plugins -> Question behaviours -> Manage behaviours, and verify that the manually graded behaviour is not available. Install an older version of Moodle. Go to Admin -> Plugins -> Question behaviours -> Manage behaviours and disable some behaviours, but not manually graded. Upgrade to the latest version. Verify that manually graded behaviour is now disabled, and that all the other behaviours that you previously disabled are still disabled.
    • Affected Branches:
      MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_25_STABLE
    • Pull from Repository:
    • Pull 2.4 Branch:
    • Pull Master Branch:
    • Rank:
      50053

      Description

      There have now been two places where people have got into a serious mess by selecting "How questions behave: Manually graded" in the quiz UI.

      This options is very unlikely to be useful. We should disable it by default. (If a teacher really needs it, their admin can re-enable it.)

        Issue Links

          Activity

          Hide
          Tim Hunt added a comment -

          A behaviour being disabled just means that users cannot select it as the behaviour to use for all questions in a quiz. Manual grading of essay questions will still work.

          Also, any quiz that currently uses Manual grading will go on using manual grading. (When you edit the quiz settings, the currently selected behaviour is alwasy offered as an option, even if it has subsequently been disabled.)

          I am taking the unusual setp of unconditionally changing the configuration of existing Moodle installs as part of the upgrade. In this case, I think the current defaults are sufficitently dangerous; and the number of people who might acutally want to manually grade multiple choice questions so small, that it is better to make this change.

          The alternative is to suggested to admins that they manually disable this option in their Moodle sites. That call woudl largely be ignored. Better to disable it unconditionally, then put a bit in the release notes saying how to re-enable it.

          Show
          Tim Hunt added a comment - A behaviour being disabled just means that users cannot select it as the behaviour to use for all questions in a quiz. Manual grading of essay questions will still work. Also, any quiz that currently uses Manual grading will go on using manual grading. (When you edit the quiz settings, the currently selected behaviour is alwasy offered as an option, even if it has subsequently been disabled.) I am taking the unusual setp of unconditionally changing the configuration of existing Moodle installs as part of the upgrade. In this case, I think the current defaults are sufficitently dangerous; and the number of people who might acutally want to manually grade multiple choice questions so small, that it is better to make this change. The alternative is to suggested to admins that they manually disable this option in their Moodle sites. That call woudl largely be ignored. Better to disable it unconditionally, then put a bit in the release notes saying how to re-enable it.
          Hide
          Tim Hunt added a comment -

          Please peer review, including commenting on the idea of back-porting this to stable branches. (I think we should.)

          Show
          Tim Hunt added a comment - Please peer review, including commenting on the idea of back-porting this to stable branches. (I think we should.)
          Hide
          Dan Poltawski added a comment -

          Hi Tim,

          The code looks fine and your argument makes sense for 2.5 with big release notes notes.

          With regards to the backport to the stables, I trust you have an idea of how many people this is affecting and would vote in favour of it, but I think that this needs to be handled seperately and voted on according to the backporting policy:
          http://docs.moodle.org/dev/Integration_Review#Process_for_requesting_a_non_bug-fix_backport

          Show
          Dan Poltawski added a comment - Hi Tim, The code looks fine and your argument makes sense for 2.5 with big release notes notes. With regards to the backport to the stables, I trust you have an idea of how many people this is affecting and would vote in favour of it, but I think that this needs to be handled seperately and voted on according to the backporting policy: http://docs.moodle.org/dev/Integration_Review#Process_for_requesting_a_non_bug-fix_backport
          Hide
          Tim Hunt added a comment -

          OK, I will submit for integration in 2.5 only. Thanks Dan.

          Show
          Tim Hunt added a comment - OK, I will submit for integration in 2.5 only. Thanks Dan.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Not really sure about these 3 points, so asking for confirmation before pushing:

          1) Logic question: Instead of unconditionally disabling the behavior... wouldn't be better to do so if there isn't any quiz using it? I understood your explanations above, so current quizzes using that behavior for all questions will continue doing so, just guessing if we should check that to decide about the disable.

          2) Coding question: Why are we using main install/upgrade code to achieve this? Shouldn't that go to the plugin (behavior) code instead?

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Not really sure about these 3 points, so asking for confirmation before pushing: 1) Logic question: Instead of unconditionally disabling the behavior... wouldn't be better to do so if there isn't any quiz using it? I understood your explanations above, so current quizzes using that behavior for all questions will continue doing so, just guessing if we should check that to decide about the disable. 2) Coding question: Why are we using main install/upgrade code to achieve this? Shouldn't that go to the plugin (behavior) code instead? Ciao
          Hide
          Tim Hunt added a comment -

          1) I tried to explain this above, and in the commit comment. So far, I have not heard of anyone using this feature for anything useful. I have heard of two teachers losing entire quiz results because they did not know what they were doing, and accidentally selected this option.

          2) Errm... Good question. Of course doing it in the plugin install.php file is better. I will fix that. Please reopen.

          Show
          Tim Hunt added a comment - 1) I tried to explain this above, and in the commit comment. So far, I have not heard of anyone using this feature for anything useful. I have heard of two teachers losing entire quiz results because they did not know what they were doing, and accidentally selected this option. 2) Errm... Good question. Of course doing it in the plugin install.php file is better. I will fix that. Please reopen.
          Hide
          Dan Poltawski added a comment -

          Grr. Why didn't I think of that file.

          Show
          Dan Poltawski added a comment - Grr. Why didn't I think of that file.
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Tim Hunt added a comment -

          OK, I changed this to do things in the qbehaviour_manualgraded. I also back-ported this. I think we need to protect users from this.

          Submitting for integration.

          Show
          Tim Hunt added a comment - OK, I changed this to do things in the qbehaviour_manualgraded. I also back-ported this. I think we need to protect users from this. Submitting for integration.
          Hide
          Tim Hunt added a comment -

          How timely. A third user has just been hit by this: https://moodle.org/mod/forum/discuss.php?d=227731

          Show
          Tim Hunt added a comment - How timely. A third user has just been hit by this: https://moodle.org/mod/forum/discuss.php?d=227731
          Hide
          Damyon Wiese added a comment -

          Thanks Tim,

          I did some testing on this and found no issues. Integrated to master only - will open the backport request.

          Show
          Damyon Wiese added a comment - Thanks Tim, I did some testing on this and found no issues. Integrated to master only - will open the backport request.
          Hide
          Damyon Wiese added a comment -

          The docs required label is still on this issue - it should be mentioned in the release notes.

          Show
          Damyon Wiese added a comment - The docs required label is still on this issue - it should be mentioned in the release notes.
          Show
          Tim Hunt added a comment - Dev docs: http://docs.moodle.org/dev/Moodle_2.5_release_notes#Manual_grading_option_in_the_quiz_UI
          Hide
          Tim Hunt added a comment -

          I cannot understand what you hope to achieve by delaying integration into stable branches.

          Show
          Tim Hunt added a comment - I cannot understand what you hope to achieve by delaying integration into stable branches.
          Hide
          Rajesh Taneja added a comment - - edited

          Thanks for fixing this Tim,

          It works as you mentioned in testing instructions. Although there two cases which you might want to consider:

          Scenario 1:

          1. Upgrading from 2.4 to master.
          2. While upgrade set "How questions behave" = "Manually Graded" under "New settings - Question preview defaults" settings.
          3. Save changes and then go to Admin -> Plugins -> Question behaviours -> Manage behaviours
          4. "Manually Graded", should be enabled.
          5. Create quiz and default value should be "Manually Graded"

          Scenario 2:

          1. Go to Admin -> Plugins -> Question behaviours -> Manage behaviours and disable Manually Graded behaviour
          2. Go to Site administration ► Plugins ► Question types ► Question preview defaults and make sure "How questions behave" should only list enabled behaviours. (Currently it's showing all installed options.)

          Failing it as user (admin) selection should be respected. Also, in 2.5 because of collapsed form this might not be expected behaviour.

          Show
          Rajesh Taneja added a comment - - edited Thanks for fixing this Tim, It works as you mentioned in testing instructions. Although there two cases which you might want to consider: Scenario 1: Upgrading from 2.4 to master. While upgrade set "How questions behave" = "Manually Graded" under "New settings - Question preview defaults" settings. Save changes and then go to Admin -> Plugins -> Question behaviours -> Manage behaviours "Manually Graded", should be enabled. Create quiz and default value should be "Manually Graded" Scenario 2: Go to Admin -> Plugins -> Question behaviours -> Manage behaviours and disable Manually Graded behaviour Go to Site administration ► Plugins ► Question types ► Question preview defaults and make sure "How questions behave" should only list enabled behaviours. (Currently it's showing all installed options.) Failing it as user (admin) selection should be respected. Also, in 2.5 because of collapsed form this might not be expected behaviour.
          Hide
          Tim Hunt added a comment -

          I believe that the number of Moodle sites in the world where Question preview defaults -> How questions behave is set to Manually graded is zero. (The same for the similar setting on Plugins -> Activity Modules -> Quiz.)

          If that number is actually non-zero, then I am sure that would only have been set by mistake.

          So, what we should do is, on upgrade, if those settings are currently manually graded, then we should change them to deferred feedback.

          You are right, those admin setting should only list enabled behaviours.

          I am not back at work until Wednesday, but I can probably do a patch before then.

          Show
          Tim Hunt added a comment - I believe that the number of Moodle sites in the world where Question preview defaults -> How questions behave is set to Manually graded is zero. (The same for the similar setting on Plugins -> Activity Modules -> Quiz.) If that number is actually non-zero, then I am sure that would only have been set by mistake. So, what we should do is, on upgrade, if those settings are currently manually graded, then we should change them to deferred feedback. You are right, those admin setting should only list enabled behaviours. I am not back at work until Wednesday, but I can probably do a patch before then.
          Hide
          Dan Poltawski added a comment -

          > I cannot understand what you hope to achieve by delaying integration into stable branches.

          See how many people scream, this seems like the exact scenario where our scream monitors are useful.

          Show
          Dan Poltawski added a comment - > I cannot understand what you hope to achieve by delaying integration into stable branches. See how many people scream, this seems like the exact scenario where our scream monitors are useful.
          Hide
          Rajesh Taneja added a comment -

          Thanks Tim,

          One small point you might want to consider again:

          1. While upgrading from 2.4 to 2.5 admin see "New settings - Question preview defaults" config options.
          2. Which has option for admin to select "How questions behave" and admin can set it to "Manually Graded".
          3. Should we actually give that option to admin?
            • If yes, then we might have to enable "manually graded"
            • If No, then we have to hide that selection while upgrade.
          Show
          Rajesh Taneja added a comment - Thanks Tim, One small point you might want to consider again: While upgrading from 2.4 to 2.5 admin see "New settings - Question preview defaults" config options. Which has option for admin to select "How questions behave" and admin can set it to "Manually Graded". Should we actually give that option to admin? If yes, then we might have to enable "manually graded" If No, then we have to hide that selection while upgrade.
          Hide
          Tim Hunt added a comment -

          Good catch Rajesh.

          Here is an extra commit added to the branch https://github.com/timhunt/moodle/compare/master...MDL-39412 which does this:

          1. It changes the behaviour admin settings to only let you select enabled behaviours.
          2. It does some additional upgrade, to update the two admin settings that let you select a behaviour, so that if they are currently set to Manually graded, they become set to deferredfeedback if possible, failing that, the first behaviour alphabetically.

          We need to do this extra testing:

          1. Upgrading from 2.4 -> 2.5, On the new settings page, you should be offerend a choice of default behaviour for previews, and Manual graded should not be an option.

          2. Any upgrade that passes these upgrade steps: if either of question_preview/behaviour or quiz/preferredbehaviour are currently set to manualgraded, then that should be changed to deferredfeedback. Other values for those settings should not be changed.

          3. If you disabled deferredfeedback before the upgrade, then those settings should be change from manualgraded to something else.

          (Sorry it took me so long to get to this.)

          Show
          Tim Hunt added a comment - Good catch Rajesh. Here is an extra commit added to the branch https://github.com/timhunt/moodle/compare/master...MDL-39412 which does this: It changes the behaviour admin settings to only let you select enabled behaviours. It does some additional upgrade, to update the two admin settings that let you select a behaviour, so that if they are currently set to Manually graded, they become set to deferredfeedback if possible, failing that, the first behaviour alphabetically. We need to do this extra testing: 1. Upgrading from 2.4 -> 2.5, On the new settings page, you should be offerend a choice of default behaviour for previews, and Manual graded should not be an option. 2. Any upgrade that passes these upgrade steps: if either of question_preview/behaviour or quiz/preferredbehaviour are currently set to manualgraded, then that should be changed to deferredfeedback. Other values for those settings should not be changed. 3. If you disabled deferredfeedback before the upgrade, then those settings should be change from manualgraded to something else. (Sorry it took me so long to get to this.)
          Hide
          Damyon Wiese added a comment -

          Thanks Tim,

          This is giving warnings on upgrade (I'm looking at this now).

          Warning: Missing argument 1 for question_engine::get_behaviour_options(), called in /home/damyonw/Documents/Moodle/instances/i24/moodle/lib/adminlib.php on line 4145 and defined in /home/damyonw/Documents/Moodle/instances/i24/moodle/question/engine/lib.php on line 301 Call Stack: 0.0003 669688 1.

          {main}

          ()
          ...
          110653920 9. question_engine::get_behaviour_options() /home/damyonw/Documents/Moodle/instances/i24/moodle/lib/adminlib.php:4145

          Show
          Damyon Wiese added a comment - Thanks Tim, This is giving warnings on upgrade (I'm looking at this now). Warning: Missing argument 1 for question_engine::get_behaviour_options(), called in /home/damyonw/Documents/Moodle/instances/i24/moodle/lib/adminlib.php on line 4145 and defined in /home/damyonw/Documents/Moodle/instances/i24/moodle/question/engine/lib.php on line 301 Call Stack: 0.0003 669688 1. {main} () ... 110653920 9. question_engine::get_behaviour_options() /home/damyonw/Documents/Moodle/instances/i24/moodle/lib/adminlib.php:4145
          Hide
          Damyon Wiese added a comment -

          I have integrated your fix Tim with an extra one for the warnings on upgrade.

          Back to testing...

          Show
          Damyon Wiese added a comment - I have integrated your fix Tim with an extra one for the warnings on upgrade. Back to testing...
          Hide
          Tim Hunt added a comment -

          Thanks Damyon. Sorry about the missing arg. Your fix looks right.

          Show
          Tim Hunt added a comment - Thanks Damyon. Sorry about the missing arg. Your fix looks right.
          Hide
          Rajesh Taneja added a comment -

          Thanks Tim,

          Works as expected

          Show
          Rajesh Taneja added a comment - Thanks Tim, Works as expected
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Did you think this day was not going to arrive ever?

          Your patience has been rewarded, yay, sent upstream, thanks!

          Closing...ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Did you think this day was not going to arrive ever? Your patience has been rewarded, yay, sent upstream, thanks! Closing...ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: