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

Disable the manual graded behaviour in the UI

    Details

    • Type: Improvement
    • Status: Closed
    • Priority: 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 Master Branch:

      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.)

        Gliffy Diagrams

          Issue Links

            Activity

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

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

            Show
            timhunt Tim Hunt added a comment - OK, I will submit for integration in 2.5 only. Thanks Dan.
            Hide
            stronk7 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
            stronk7 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
            timhunt 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
            timhunt 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
            poltawski Dan Poltawski added a comment -

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

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

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

            Show
            cibot CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
            Hide
            timhunt 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
            timhunt 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
            timhunt 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
            timhunt 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 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 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 Damyon Wiese added a comment -

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

            Show
            damyon Damyon Wiese added a comment - The docs required label is still on this issue - it should be mentioned in the release notes.
            Show
            timhunt 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
            timhunt Tim Hunt added a comment -

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

            Show
            timhunt Tim Hunt added a comment - I cannot understand what you hope to achieve by delaying integration into stable branches.
            Hide
            rajeshtaneja 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
            rajeshtaneja 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
            timhunt 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
            timhunt 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
            poltawski 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
            poltawski 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
            rajeshtaneja 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
            rajeshtaneja 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
            timhunt 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
            timhunt 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 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 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 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 Damyon Wiese added a comment - I have integrated your fix Tim with an extra one for the warnings on upgrade. Back to testing...
            Hide
            timhunt Tim Hunt added a comment -

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

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

            Thanks Tim,

            Works as expected

            Show
            rajeshtaneja Rajesh Taneja added a comment - Thanks Tim, Works as expected
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  14/May/13