Moodle
  1. Moodle
  2. MDL-30880

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 Master Branch:
      MDL-30880_feedback_property
    • Rank:
      33889

      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

        Hide
        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
        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
        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
        Akin Delamarre added a comment -

        Thanks!

        Show
        Akin Delamarre added a comment - Thanks!
        Hide
        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
        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
        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
        Dan Poltawski added a comment -

        Integrated, thanks Andrew

        Show
        Dan Poltawski added a comment - Integrated, thanks Andrew
        Hide
        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
        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

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

            Dates

            • Created:
              Updated:
              Resolved: