[MDL-30880] Editing a non-core module grade producing a php notice message and prevents the page from redirecting automatically - Moodle Tracker

Details

Type: Bug
Status: Closed
Priority: Minor
Resolution: Fixed
Affects Version/s: 2.1.2
Fix Version/s: 2.1.7, 2.2.4
Component/s: Gradebook
Labels:
- partner
- patch
- triaged

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:
git://github.com/andyjdavis/moodle.git
Pull Master Branch:
~~MDL-30880~~_feedback_property
Pull Master Diff URL:
https://github.com/andyjdavis/moodle/compare/master...MDL-30880_feedback_property

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:

Log in as an admin/instructor
Go into the course and click Grades
Turn on editing
Edit the grade for the non-core plug-in
Check 'locked' and save
Go back and edit that same grade
Un-check 'locked' click save
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.

Gliffy Diagrams

Attachments

Activity

Ascending order - Click to sort in descending order

Hide

Permalink

Michael de Raadt added a comment - 25/Jan/12 11:15 AM

Thanks for reporting that and providing a solution.

Show

Michael de Raadt added a comment - 25/Jan/12 11:15 AM Thanks for reporting that and providing a solution.

Hide

Permalink

Andrew Davis added a comment - 28/May/12 9:19 AM

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 - 28/May/12 9:19 AM 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 - 28/May/12 9:20 AM

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 - 28/May/12 9:20 AM 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 - 28/May/12 9:37 PM

Thanks!

Show

Akin Delamarre added a comment - 28/May/12 9:37 PM Thanks!

Hide

Permalink

Ankit Agarwal added a comment - 31/May/12 3:12 PM

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 - 31/May/12 3:12 PM 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 - 01/Jun/12 11:46 AM

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 - 01/Jun/12 11:46 AM 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 - 01/Jun/12 5:31 PM

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 - 01/Jun/12 5:31 PM 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 - 05/Jun/12 11:59 AM

Integrated, thanks Andrew

Show

Dan Poltawski added a comment - 05/Jun/12 11:59 AM Integrated, thanks Andrew

Hide

Permalink

Frédéric Massart added a comment - 05/Jun/12 3:59 PM

Success on 2.1, 2.2 and master!

Show

Frédéric Massart added a comment - 05/Jun/12 3:59 PM Success on 2.1, 2.2 and master!

Hide

Permalink

Eloy Lafuente (stronk7) added a comment - 12/Jun/12 9:46 AM

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 - 12/Jun/12 9:46 AM 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

Assignee:

Andrew Davis

Reporter:

Akin Delamarre

Peer reviewer:

Ankit Agarwal

Integrator:

Dan Poltawski

Tester:

Frédéric Massart

Participants:

Akin Delamarre, Andrew Davis, Ankit Agarwal, Dan Poltawski, Eloy Lafuente (stronk7), Frédéric Massart, Michael de Raadt

Votes:

1 Vote for this issue

Watchers:

0 Start watching this issue

Dates

Created:

23/Dec/11 2:13 AM

Updated:

12/Jun/12 9:46 AM

Resolved:

12/Jun/12 9:46 AM

Fix Release Date:

9/Jul/12