Moodle

Editing a non-core module grade producing a php notice message and prevents the page from redirecting automatically

Details

  • Testing Instructions:
    Hide

    Log in as an admin/instructor
    Go into the course and click Grades
    Turn on editing
    Click the hand/edit icon for a grade item
    Check 'locked' and save
    Go back and edit that same grade
    Un-check 'locked' click save
    No error should be displayed.

    Note: depending on your configuration you may see a warning like "Reduced maximum students per page from 100 to 83. Consider increasing the PHP setting max_input_vars from 1000." on your grader report. That is unrelated.

    Install a contributed activity that can have a grade. For example, http://moodle.org/plugins/view.php?plugin=assignment_onlineaudio which is installed in /mod/assignment/type/onlineaudio and is a new sub type for the old assignment module. Create an instance. This may display some warning along the way that are unrelated. Go into the grade book. Lock then unlock the grade for a student for the online audio activity. You should not get any warnings or errors during the locking/unlocking process.

    Show
    Log in as an admin/instructor Go into the course and click Grades Turn on editing Click the hand/edit icon for a grade item Check 'locked' and save Go back and edit that same grade Un-check 'locked' click save No error should be displayed. Note: depending on your configuration you may see a warning like "Reduced maximum students per page from 100 to 83. Consider increasing the PHP setting max_input_vars from 1000." on your grader report. That is unrelated. Install a contributed activity that can have a grade. For example, http://moodle.org/plugins/view.php?plugin=assignment_onlineaudio which is installed in /mod/assignment/type/onlineaudio and is a new sub type for the old assignment module. Create an instance. This may display some warning along the way that are unrelated. Go into the grade book. Lock then unlock the grade for a student for the online audio activity. You should not get any warnings or errors during the locking/unlocking process.
  • Affected Branches:
    MOODLE_21_STABLE
  • Fixed Branches:
    MOODLE_21_STABLE, MOODLE_22_STABLE
  • Pull from Repository:
  • Pull Master Branch:
    MDL-30880_feedback_property
  • Pull Master Diff URL:

Description

Editing a grade for a non-core module in the gradebook, checking locked and then un-checking locked produces a PHP notice and prevents the page from redirecting automatically.

Reproduction steps:

  1. Log in as an admin/instructor
  2. Go into the course and click Grades
  3. Turn on editing
  4. Edit the grade for the non-core plug-in
  5. Check 'locked' and save
  6. Go back and edit that same grade
  7. Un-check 'locked' click save
  8. The page displays the following message

    Notice: Undefined property: stdClass::$feedback in /var/www/html/ack/testsite/grade/edit/tree/grade.php on line 165 Call Stack: 0.0019 779336 1.

    Unknown macro: {main}

    () /var/www/html/ack/testsite/grade/edit/tree/grade.php:0
    Skip to main content

    This page should automatically redirect. If nothing is happening please use the continue link below.
    (Continue)

    Error output, so disabling automatic redirect.

Here is the line of code that produces that notice message

    if (is_array($data->feedback)) {
        $data->feedbackformat = $data->feedback['format'];
        $data->feedback = $data->feedback['text'];
    }

The code should first check to see if the feedback property exists and then check to see if it is an array

    if (isset($data->feedback) && is_array($data->feedback)) {
        $data->feedbackformat = $data->feedback['format'];
        $data->feedback = $data->feedback['text'];
    }

The same thing happens when editing core graded modules but for some reason the notice message does not appear.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Michael de Raadt added a comment -

Thanks for reporting that and providing a solution.

Show
Michael de Raadt added a comment - Thanks for reporting that and providing a solution.
Hide
Permalink
Andrew Davis added a comment -

Adding testing instructions and a git branch containing the suggested fix. Putting this up for peer review. I'll create 2.2 and 2.1 versions once that's done.

Show
Andrew Davis added a comment - Adding testing instructions and a git branch containing the suggested fix. Putting this up for peer review. I'll create 2.2 and 2.1 versions once that's done.
Hide
Permalink
Andrew Davis added a comment -

Although it may get integrated before then Im adding this to the next stable sprint to make sure it doesn't get forgotten.

Show
Andrew Davis added a comment - Although it may get integrated before then Im adding this to the next stable sprint to make sure it doesn't get forgotten.
Hide
Permalink
Akin Delamarre added a comment -

Thanks!

Show
Akin Delamarre added a comment - Thanks!
Hide
Permalink
Ankit Agarwal added a comment -

Hi Andrew,
Changes looks good and makes sense.
But I was not able to replicate this on master $data->feedback is always set for me. (Akin encountered it on non-contributed modules only as well). So it might be a good idea to update the testing instructions, to test this with some non-contributed core module.

Thanks

Show
Ankit Agarwal added a comment - Hi Andrew, Changes looks good and makes sense. But I was not able to replicate this on master $data->feedback is always set for me. (Akin encountered it on non-contributed modules only as well). So it might be a good idea to update the testing instructions, to test this with some non-contributed core module. Thanks
Hide
Permalink
Andrew Davis added a comment -

Adding 2.2 and 2.1 branches. I've updated the testing instructions to include testing with a contributed activity.

Show
Andrew Davis added a comment - Adding 2.2 and 2.1 branches. I've updated the testing instructions to include testing with a contributed activity.
Hide
Permalink
Dan Poltawski 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
Dan Poltawski 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
Permalink
Dan Poltawski added a comment -

Integrated, thanks Andrew

Show
Dan Poltawski added a comment - Integrated, thanks Andrew
Hide
Permalink
Frédéric Massart added a comment -

Success on 2.1, 2.2 and master!

Show
Frédéric Massart added a comment - Success on 2.1, 2.2 and master!
Hide
Permalink
Eloy Lafuente (stronk7) added a comment -

We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3.

Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish.

Many thanks for your collaboration!

Ciao

Show
Eloy Lafuente (stronk7) added a comment - We could celebrate it today... but better if we perform a bigger party after releasing Moodle 2.3. Print this message and come to Perth that day, it's valid for one beer, wine, coke or... water, as you wish. Many thanks for your collaboration! Ciao

People

Vote (1)
Watch (0)

Dates

  • Created:
    Updated:
    Resolved: