Moodle
  1. Moodle
  2. MDL-13831

Grade to pass can't be set from update activity screen

    Details

    • Testing Instructions:
      Hide
      1. Use the test course generator to create a 'Small' course (or use an existing course with activities and resources)
      2. Edit an activity (assignment) and set a grade to pass
      3. Edit the activity in the gradebook and ensure the grade to pass field is set correctly
      4. Re-edit the activity and add a grade to pass that is greater than the current grade. Ensure a validation message is displayed
      5. Re-edit the activity and set the Grade type to 'None'. Ensure the grade to pass field is disabled
      6. Add a workshop activity. Ensure you can set a Grade to pass for both the submission and assessment phase
      7. Edit a resource or activity without grades. Save the form and ensure no errors are displayed
      Show
      Use the test course generator to create a 'Small' course (or use an existing course with activities and resources) Edit an activity (assignment) and set a grade to pass Edit the activity in the gradebook and ensure the grade to pass field is set correctly Re-edit the activity and add a grade to pass that is greater than the current grade. Ensure a validation message is displayed Re-edit the activity and set the Grade type to 'None'. Ensure the grade to pass field is disabled Add a workshop activity. Ensure you can set a Grade to pass for both the submission and assessment phase Edit a resource or activity without grades. Save the form and ensure no errors are displayed
    • Affected Branches:
      MOODLE_19_STABLE, MOODLE_24_STABLE, MOODLE_28_STABLE
    • Pull Master Branch:
      master_MDL-13831

      Description

      The current way of setting it is a bit obscure and I think it should really be available on all the 'update this activity' pages.

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Ray Morris added a comment -

            It really would be nice to be able to set "passing" grade within the Grade section when you create or edit an activity. I may be willing to code this if there is agreement that it's a desired improvement and it would, properly coded, be accepted into mainline.

            Show
            Ray Morris added a comment - It really would be nice to be able to set "passing" grade within the Grade section when you create or edit an activity. I may be willing to code this if there is agreement that it's a desired improvement and it would, properly coded, be accepted into mainline.
            Hide
            Davo Smith added a comment -

            Not free to tidy this up properly, but this patch was working last time I tried it:

            ------------------------------ course/modedit.php ------------------------------
            index 56bf28a..5b38259 100644
            @@ -215,10 +215,16 @@ if (!empty($add))

            { // do not set if mixed categories present $data->gradecat = $gradecat; }

            }

            + $item = grade_item::fetch(array('itemtype'=>'mod', 'itemmodule'=>$data->modulename,
            + 'iteminstance'=>$data->instance, 'courseid'=>$course->id));
            + if (!empty($item->gradepass))

            { + $data->gradepass = $item->gradepass; + }

            +
            $sectionname = get_section_name($course, $cw);
            $fullmodulename = get_string('modulename', $module->name);

            if ($data->section && $course->format != 'site') {
            $heading = new stdClass();
            @@ 517,10 +523,14 @@ if ($mform>is_cancelled()) {
            'iteminstance'=>$fromform->instance, 'itemnumber'=>0, 'courseid'=>$course->id))) {
            if ($grade_item->idnumber != $fromform->cmidnumber)

            { $grade_item->idnumber = $fromform->cmidnumber; $grade_item->update(); }

            + if ($grade_item->gradepass != $fromform->gradepass)

            { + $grade_item->gradepass = $fromform->gradepass; + $grade_item->update(); + }

            }

            $items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$fromform->modulename,
            'iteminstance'=>$fromform->instance, 'courseid'=>$course->id));

            -------------------------- course/moodleform_mod.php --------------------------
            index b4feb9d..afc7862 100644
            @@ -756,10 +756,15 @@ abstract class moodleform_mod extends moodleform {

            //if supports grades and grades arent being handled via ratings
            if (!$this->_features->rating)

            { $mform->addElement('modgrade', 'grade', get_string('grade')); $mform->setDefault('grade', 100); + + $mform->addElement('text', 'gradepass', get_string('gradepass', 'grades')); + $mform->setDefault('gradepass', ''); + $mform->setType('gradepass', PARAM_FLOAT); + $mform->setAdvanced('gradepass', true); }

            if ($this->_features->advancedgrading
            and !empty($this->current->_advancedgradingdata['methods'])
            and !empty($this->current->_advancedgradingdata['areas'])) {

            Show
            Davo Smith added a comment - Not free to tidy this up properly, but this patch was working last time I tried it: ------------------------------ course/modedit.php ------------------------------ index 56bf28a..5b38259 100644 @@ -215,10 +215,16 @@ if (!empty($add)) { // do not set if mixed categories present $data->gradecat = $gradecat; } } + $item = grade_item::fetch(array('itemtype'=>'mod', 'itemmodule'=>$data->modulename, + 'iteminstance'=>$data->instance, 'courseid'=>$course->id)); + if (!empty($item->gradepass)) { + $data->gradepass = $item->gradepass; + } + $sectionname = get_section_name($course, $cw); $fullmodulename = get_string('modulename', $module->name); if ($data->section && $course->format != 'site') { $heading = new stdClass(); @@ 517,10 +523,14 @@ if ($mform >is_cancelled()) { 'iteminstance'=>$fromform->instance, 'itemnumber'=>0, 'courseid'=>$course->id))) { if ($grade_item->idnumber != $fromform->cmidnumber) { $grade_item->idnumber = $fromform->cmidnumber; $grade_item->update(); } + if ($grade_item->gradepass != $fromform->gradepass) { + $grade_item->gradepass = $fromform->gradepass; + $grade_item->update(); + } } $items = grade_item::fetch_all(array('itemtype'=>'mod', 'itemmodule'=>$fromform->modulename, 'iteminstance'=>$fromform->instance, 'courseid'=>$course->id)); -------------------------- course/moodleform_mod.php -------------------------- index b4feb9d..afc7862 100644 @@ -756,10 +756,15 @@ abstract class moodleform_mod extends moodleform { //if supports grades and grades arent being handled via ratings if (!$this->_features->rating) { $mform->addElement('modgrade', 'grade', get_string('grade')); $mform->setDefault('grade', 100); + + $mform->addElement('text', 'gradepass', get_string('gradepass', 'grades')); + $mform->setDefault('gradepass', ''); + $mform->setType('gradepass', PARAM_FLOAT); + $mform->setAdvanced('gradepass', true); } if ($this->_features->advancedgrading and !empty($this->current->_advancedgradingdata ['methods'] ) and !empty($this->current->_advancedgradingdata ['areas'] )) {
            Hide
            Davo Smith added a comment -

            Actually, that didn't copy + paste very well, so I've attached it as a text file instead.

            Note it probably can't be applied automatically, but it's simple enough to be possible to copy + paste manually.

            Show
            Davo Smith added a comment - Actually, that didn't copy + paste very well, so I've attached it as a text file instead. Note it probably can't be applied automatically, but it's simple enough to be possible to copy + paste manually.
            Hide
            Paula Clough added a comment -

            Having the setting within the assignment would make this feature much more easily used by Instructors. It would also make it easier to check what has happened when we get the call that says... the student only got 70% and it's marking them as passed.
            Thanks,
            Paula

            Show
            Paula Clough added a comment - Having the setting within the assignment would make this feature much more easily used by Instructors. It would also make it easier to check what has happened when we get the call that says... the student only got 70% and it's marking them as passed. Thanks, Paula
            Hide
            Davo Smith added a comment -

            Patch submitted for peer review.

            I can see a lot of benefit in having this option on the settings page, but at the same time I'm more than happy for this implementation to be struck down as not being suitable (for example, I don't think it works well with grading scales, but then it is no different from the same setting when accessed directly through the gradebook).

            Show
            Davo Smith added a comment - Patch submitted for peer review. I can see a lot of benefit in having this option on the settings page, but at the same time I'm more than happy for this implementation to be struck down as not being suitable (for example, I don't think it works well with grading scales, but then it is no different from the same setting when accessed directly through the gradebook).
            Hide
            Tim Hunt added a comment -

            Thanks for doing this Davo.

            If this is the same UI as in the gradebook, then it is good enough for now, even if both should be improved later.

            Note that PARAM_FLOAT is problematic. It accepts a PHP-formatted float, like 0.1. Europeans and many others would more naturally write 0,1. However, this is a global problem, and I think from previous discussion with Petr, it is only fixable be creating a new float formslib field type.

            My remaining worry is, what about a module like Workshop, where one activity has two associated grade items? I can't work out what this code will do.

            Actually, it would be good if you wrote some testing instructions. That would make it quick and easy for lazy people like me to see how it is supposed to behave.

            Show
            Tim Hunt added a comment - Thanks for doing this Davo. If this is the same UI as in the gradebook, then it is good enough for now, even if both should be improved later. Note that PARAM_FLOAT is problematic. It accepts a PHP-formatted float, like 0.1. Europeans and many others would more naturally write 0,1. However, this is a global problem, and I think from previous discussion with Petr, it is only fixable be creating a new float formslib field type. My remaining worry is, what about a module like Workshop, where one activity has two associated grade items? I can't work out what this code will do. Actually, it would be good if you wrote some testing instructions. That would make it quick and easy for lazy people like me to see how it is supposed to behave.
            Hide
            Davo Smith added a comment -

            Tim - given the number of free evenings I have before flying to Dublin (0), maybe it's something to look at during Thursday's hackfest?

            Show
            Davo Smith added a comment - Tim - given the number of free evenings I have before flying to Dublin (0), maybe it's something to look at during Thursday's hackfest?
            Hide
            Tim Hunt added a comment -

            Sure.

            Show
            Tim Hunt added a comment - Sure.
            Hide
            Ray Morris added a comment - - edited

            > My remaining worry is, what about a module like Workshop, where one activity has two
            > associated grade items? I can't work out what this code will do.

            I believe the module can set which grade should be used. By default the first grade is used. (Grade #0).
            See course_modules.completiongradeitemnumber

            Show
            Ray Morris added a comment - - edited > My remaining worry is, what about a module like Workshop, where one activity has two > associated grade items? I can't work out what this code will do. I believe the module can set which grade should be used. By default the first grade is used. (Grade #0). See course_modules.completiongradeitemnumber
            Hide
            Ray Morris added a comment - - edited

            The patch as currently posted may at times throw an error when grade_item::fetch sees more than one row:

            Debug info: 
            Error code: morethanonerecordinfetch
            Stack trace:
            line 467 of \lib\setuplib.php: moodle_exception thrown
            line 163 of \lib\grade\grade_object.php: call to print_error()
            line 326 of \lib\grade\grade_item.php: call to grade_object::fetch_helper()
            line 220 of \course\modedit.php: call to grade_item::fetch()
            

            I haven't yet looked in to it in detail.

            Show
            Ray Morris added a comment - - edited The patch as currently posted may at times throw an error when grade_item::fetch sees more than one row: Debug info: Error code: morethanonerecordinfetch Stack trace: line 467 of \lib\setuplib.php: moodle_exception thrown line 163 of \lib\grade\grade_object.php: call to print_error() line 326 of \lib\grade\grade_item.php: call to grade_object::fetch_helper() line 220 of \course\modedit.php: call to grade_item::fetch() I haven't yet looked in to it in detail.
            Hide
            Tim Hunt added a comment -

            So, it looks like there are some issues to sort out Davo. Are you likely to get a chance to work on this some more?

            Show
            Tim Hunt added a comment - So, it looks like there are some issues to sort out Davo. Are you likely to get a chance to work on this some more?
            Hide
            Davo Smith added a comment -

            Possibly, but not quite sure when - I've got quite a bit to do just at the moment.

            Show
            Davo Smith added a comment - Possibly, but not quite sure when - I've got quite a bit to do just at the moment.
            Hide
            Rob Monk added a comment - - edited

            Here here. What a good feature.
            Surely, if in the Assignment setting screen we can set the Gradebook category, we should be able to set a passing grade.
            And I want a check box that says "Add to progress bar" as well.
            A one stop assignment shop.
            At the moment the work flow is. Set the assignment. Get into Gradebook > Categories and Items > Edit settings of the task to set a pass mark (Annoyingly fiddley). Get into the progress bar settings to Add the task to the progress bar selecting "Passed" as the criteria for completion. So I have to go to Assignment and Gradebook and Progress Bar just to set one task. YUK.
            All this should be able to be done in the one spot when setting the assignment!!

            Show
            Rob Monk added a comment - - edited Here here. What a good feature. Surely, if in the Assignment setting screen we can set the Gradebook category, we should be able to set a passing grade. And I want a check box that says "Add to progress bar" as well. A one stop assignment shop. At the moment the work flow is. Set the assignment. Get into Gradebook > Categories and Items > Edit settings of the task to set a pass mark (Annoyingly fiddley). Get into the progress bar settings to Add the task to the progress bar selecting "Passed" as the criteria for completion. So I have to go to Assignment and Gradebook and Progress Bar just to set one task. YUK. All this should be able to be done in the one spot when setting the assignment!!
            Hide
            Michael de Raadt added a comment -

            It would be great to see this work continue. I think would be a welcome change for something that most people don't even know about. It's also used in a few places now, so making it more accessible would be drive those areas also.

            Show
            Michael de Raadt added a comment - It would be great to see this work continue. I think would be a welcome change for something that most people don't even know about. It's also used in a few places now, so making it more accessible would be drive those areas also.
            Hide
            Adam Jenkins added a comment -

            Yes, if we could add this to the "Edit categories and items: Full view" screen, that would be quite helpful.

            Show
            Adam Jenkins added a comment - Yes, if we could add this to the "Edit categories and items: Full view" screen, that would be quite helpful.
            Hide
            Tõnis Tartes added a comment -

            Growing need for this feature.

            Show
            Tõnis Tartes added a comment - Growing need for this feature.
            Hide
            Marina Glancy added a comment -

            Hi Davo Smith, I can see your name on this issue. Are you actively working on it?

            Show
            Marina Glancy added a comment - Hi Davo Smith , I can see your name on this issue. Are you actively working on it?
            Hide
            Tõnis Tartes added a comment -

            Bump! Any updated on this?

            Show
            Tõnis Tartes added a comment - Bump! Any updated on this?
            Hide
            Marina Glancy added a comment -

            Thanks for bumping, I unassigned it from Davo so anybody else can work on it.
            The patch needs some modifications - first of all, it does not apply to master cleanly and second it does not take into account activities like workshop that may have several grade items

            Show
            Marina Glancy added a comment - Thanks for bumping, I unassigned it from Davo so anybody else can work on it. The patch needs some modifications - first of all, it does not apply to master cleanly and second it does not take into account activities like workshop that may have several grade items
            Hide
            Rob Monk added a comment -

            If would be great if Micheal de Raadt could be involved here as well so we could add assignments to the progress bar as well as set pass marks, all in the assignment setting screen.
            A one stop assignment shop.
            At the moment the work flow is. Set the assignment. Get into Gradebook > Categories and Items > Edit settings of the task to set a pass mark (Annoyingly fiddley). Get into the progress bar settings to Add the task to the progress bar selecting "Passed" as the criteria for completion. So I have to go to Assignment and Gradebook and Progress Bar just to set one task. YUK.
            All this should be able to be done in the one spot when setting the assignment!!

            Show
            Rob Monk added a comment - If would be great if Micheal de Raadt could be involved here as well so we could add assignments to the progress bar as well as set pass marks, all in the assignment setting screen. A one stop assignment shop. At the moment the work flow is. Set the assignment. Get into Gradebook > Categories and Items > Edit settings of the task to set a pass mark (Annoyingly fiddley). Get into the progress bar settings to Add the task to the progress bar selecting "Passed" as the criteria for completion. So I have to go to Assignment and Gradebook and Progress Bar just to set one task. YUK. All this should be able to be done in the one spot when setting the assignment!!
            Hide
            Eric Merrill added a comment -

            Part of the problem is that the grade-to-pass UI is completely bad, it doesn't work for scales at all, and setting 'no grade to pass' is bizarre. So it might be a matter of fixing the grade-to-pass UI before we add that UI to modules.
            MDL-12117 talks about it some (closed for inactivity), and I've been working on it a bit in MDL-48692. Any bright ideas on the scale issue are welcome.

            Show
            Eric Merrill added a comment - Part of the problem is that the grade-to-pass UI is completely bad, it doesn't work for scales at all, and setting 'no grade to pass' is bizarre. So it might be a matter of fixing the grade-to-pass UI before we add that UI to modules. MDL-12117 talks about it some (closed for inactivity), and I've been working on it a bit in MDL-48692 . Any bright ideas on the scale issue are welcome.
            Hide
            Michael de Raadt added a comment -

            Thanks for bringing up the Progress Bar, Rob. I know a lot of people use it with passing grades and have expressed to me the awkwardness of achieving this.

            Unfortunately I don't think settings from one plugin, like the Progress Bar block, could be incorporated into the settings of another, especially as the Progress Bar is contributed and Assignment is core.

            At least, if you are able to set a passing mark when you create an assignment, when you add and configure a Progress Bar block, it will recognise there is already a pass mark and present this option.

            Show
            Michael de Raadt added a comment - Thanks for bringing up the Progress Bar, Rob. I know a lot of people use it with passing grades and have expressed to me the awkwardness of achieving this. Unfortunately I don't think settings from one plugin, like the Progress Bar block, could be incorporated into the settings of another, especially as the Progress Bar is contributed and Assignment is core. At least, if you are able to set a passing mark when you create an assignment, when you add and configure a Progress Bar block, it will recognise there is already a pass mark and present this option.
            Hide
            Don Hinkelman added a comment -

            Next month, before the school year begins, I will set up my Progress Bars for nearly 100 courses. It will take me a day, mainly because setting "Grade to pass" is so convoluted. I don't understand what the problem is, but we have to fix this. I agree that the Progress Bar should detect the settings in the Quiz and Assignment modules, but not "do" the setting itself. So why don't the Quiz and Assignment modules have a setting for "Grade to pass"? By the way, isn't it odd that the #1 gamification tool in Moodle is not in core?

            Show
            Don Hinkelman added a comment - Next month, before the school year begins, I will set up my Progress Bars for nearly 100 courses. It will take me a day, mainly because setting "Grade to pass" is so convoluted. I don't understand what the problem is, but we have to fix this. I agree that the Progress Bar should detect the settings in the Quiz and Assignment modules, but not "do" the setting itself. So why don't the Quiz and Assignment modules have a setting for "Grade to pass"? By the way, isn't it odd that the #1 gamification tool in Moodle is not in core?
            Hide
            Greg added a comment -

            I've attached a branch that is based off of Davo's original branch. It takes into account mod/workshop (although alters workshop and isn't generic for any activities that handle grades differently).

            I've also added validation to ensure the grade to pass field isn't greater than the grade or scale being used. I've noticed that there is no validation in the gradebook, meaning you can enter a grade to pass of 101 for an activity with a maximum grade of 100.

            I looked at incorporating the grade to pass field into the lib/form/modgrade, but couldn't figure out how to deal with https://github.com/moodle/moodle/blob/master/lib/form/modgrade.php#L286.

            Greg

            Show
            Greg added a comment - I've attached a branch that is based off of Davo's original branch. It takes into account mod/workshop (although alters workshop and isn't generic for any activities that handle grades differently). I've also added validation to ensure the grade to pass field isn't greater than the grade or scale being used. I've noticed that there is no validation in the gradebook, meaning you can enter a grade to pass of 101 for an activity with a maximum grade of 100. I looked at incorporating the grade to pass field into the lib/form/modgrade, but couldn't figure out how to deal with https://github.com/moodle/moodle/blob/master/lib/form/modgrade.php#L286 . Greg
            Hide
            Greg added a comment -

            Have just realise I haven't created any tests. I'll try and do this tomorrow.

            Show
            Greg added a comment - Have just realise I haven't created any tests. I'll try and do this tomorrow.
            Hide
            CiBoT added a comment -

            Fails against automated checks.

            Checked MDL-13831 using repository: git://github.com/greg-or/moodle-mod_assign.git

            More information about this report

            Show
            CiBoT added a comment - Fails against automated checks. Checked MDL-13831 using repository: git://github.com/greg-or/moodle-mod_assign.git master (3 errors / 1 warnings) [branch: master_MDL-13831 | CI Job ] phplint (0/0) , php (3/0) , js (0/0) , css (0/0) , phpdoc (0/0) , commit (0/1) , savepoint (0/0) , thirdparty (0/0) , More information about this report
            Hide
            Marina Glancy added a comment -

            Thanks Greg, I did not test it but it looks pretty good. You should add a note in mod/upgrade.txt no notify other module developers. Also I will recommend to add a help icon for this field that also explains how this field work in case of scales.

            David Mudrak, you might be interested in reviewing the workshop part of code.

            Show
            Marina Glancy added a comment - Thanks Greg, I did not test it but it looks pretty good. You should add a note in mod/upgrade.txt no notify other module developers. Also I will recommend to add a help icon for this field that also explains how this field work in case of scales. David Mudrak , you might be interested in reviewing the workshop part of code.
            Hide
            Greg added a comment -

            Thanks Marina. I've described this change in mod/upgrade.txt and added the help strings using existing language from grades. I haven't altered this help string to explain scales (seems to be a separate issue).

            Also added behat tests.

            Show
            Greg added a comment - Thanks Marina. I've described this change in mod/upgrade.txt and added the help strings using existing language from grades. I haven't altered this help string to explain scales (seems to be a separate issue). Also added behat tests.
            Hide
            David Mudrak added a comment -

            Thanks Marina for pinging me. And thanks Greg for your work on this.

            In mod/workshop/lib.php change, I am wondering if we could check for

            if ($gradeitem->gradepass != $workshop->submissiongradepass)
            

            before actually changing it via $gradeitem->update() - like we do for the category. Currently, this would trigger update() all the time without actually changing anything.

            I admit that the new Workshop strings 'Grade to pass for submission' and 'Grade to pass for assessment' do not sound quite right to me. But it's beyond my English knowledge. Maybe Helen Foster can find a minute and look at it to confirm it's ok to go? TIA

            Show
            David Mudrak added a comment - Thanks Marina for pinging me. And thanks Greg for your work on this. In mod/workshop/lib.php change, I am wondering if we could check for if ($gradeitem->gradepass != $workshop->submissiongradepass) before actually changing it via $gradeitem->update() - like we do for the category. Currently, this would trigger update() all the time without actually changing anything. I admit that the new Workshop strings 'Grade to pass for submission' and 'Grade to pass for assessment' do not sound quite right to me. But it's beyond my English knowledge. Maybe Helen Foster can find a minute and look at it to confirm it's ok to go? TIA
            Hide
            Helen Foster added a comment -

            Thanks David for asking my opinion on new en lang strings. How about 'Submission grade to pass' and 'Assessment grade to pass'? They are a little bit shorter than 'Grade to pass for submission' and I think sound a little bit better.

            Greg, I notice another new string gradepassgreaterthangrade 'The grade to pass entered is greater than the grade'. Should this be ''The grade to pass entered is greater than the maximum grade.'?

            Show
            Helen Foster added a comment - Thanks David for asking my opinion on new en lang strings. How about 'Submission grade to pass' and 'Assessment grade to pass'? They are a little bit shorter than 'Grade to pass for submission' and I think sound a little bit better. Greg, I notice another new string gradepassgreaterthangrade 'The grade to pass entered is greater than the grade'. Should this be ''The grade to pass entered is greater than the maximum grade.'?
            Hide
            Greg added a comment -

            Thanks David and Helen, I've made the suggested changes and updated my branch.

            Greg

            Show
            Greg added a comment - Thanks David and Helen, I've made the suggested changes and updated my branch. Greg
            Hide
            Tim Hunt added a comment -

            I peer reveiwed an earlier iteration of this, but I don't really have time now. Removing myself.

            Show
            Tim Hunt added a comment - I peer reveiwed an earlier iteration of this, but I don't really have time now. Removing myself.

              People

              • Votes:
                34 Vote for this issue
                Watchers:
                27 Start watching this issue

                Dates

                • Created:
                  Updated: