Moodle
  1. Moodle
  2. MDL-32611

Fatal error in editcategory.php when users has manager role at category level

    Details

    • Testing Instructions:
      Hide
      1. Log in as an admin
      2. Create a new category called "Management test"
      3. Browse to that category and turn on editing if its not already on.
      4. Click the button to add a sub category and check there are no errors on the page.
      5. Create a new user called "manager"
      6. Browse to the management test category and assign the manager user the role of manager
      7. Log in as manager
      8. Browse to the management test category and turn editing on
      9. Click the button to add a sub category and check you don't get any errors.
      Show
      Log in as an admin Create a new category called "Management test" Browse to that category and turn on editing if its not already on. Click the button to add a sub category and check there are no errors on the page. Create a new user called "manager" Browse to the management test category and assign the manager user the role of manager Log in as manager Browse to the management test category and turn editing on Click the button to add a sub category and check you don't get any errors.
    • Affected Branches:
      MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull Master Branch:
      wip-MDL-32611-m23
    • Rank:
      39538

      Description

      To reporduce in Moodle 2.2.2+ (Build: 20120412)

      • as admin 1) create a user USR 2) create a category CAT 3) turn debugging on
      • assign to previously created user USR the manager role at category level CAT
      • login as USR
      • go to the category USR has manager role in
      • turn editing on
      • click on "add a sub category"
      • the following error is displayed: Fatal error: Call to a member function get() on a non-object in ..../editcategory.php on line 139

      It appears something related to navigation, as the line is

      $PAGE->settingsnav->get('root')->get('courses')->get('coursemgmt')->make_active();
      

        Issue Links

          Activity

          Hide
          Andrea Bicciolo added a comment -

          Assigning to Michael as per MD suggestion

          Show
          Andrea Bicciolo added a comment - Assigning to Michael as per MD suggestion
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that. I've passed the buck to Sam as he is responsible for navigation.

          Show
          Michael de Raadt added a comment - Thanks for reporting that. I've passed the buck to Sam as he is responsible for navigation.
          Hide
          Andrea Bicciolo added a comment - - edited

          After digging a bit I found that $PAGE->settingsnav->get('root') may returns null in the scenario reported.

          As we need to fix this, we produced the above patch, which looks really temporary. I'd appreciate if someone could take a look to it and propose improvements. If nothing better pops up in a reasonable time frame, we can try to send it for integration review.

          Show
          Andrea Bicciolo added a comment - - edited After digging a bit I found that $PAGE->settingsnav->get('root') may returns null in the scenario reported. As we need to fix this, we produced the above patch, which looks really temporary. I'd appreciate if someone could take a look to it and propose improvements. If nothing better pops up in a reasonable time frame, we can try to send it for integration review.
          Hide
          Sam Hemelryk added a comment -

          Thanks for locating the cause of this issue and reporting it Andrea.
          I've just been looking into this now and have reworked the code around that area removing the try...catch (as exceptions don't occur there) and have instead implemented it as a series of checks while digging down to find the desired node.

          Putting this straight up for integration as integration starts today and this is a simple fix.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Thanks for locating the cause of this issue and reporting it Andrea. I've just been looking into this now and have reworked the code around that area removing the try...catch (as exceptions don't occur there) and have instead implemented it as a series of checks while digging down to find the desired node. Putting this straight up for integration as integration starts today and this is a simple fix. Cheers Sam
          Hide
          Dan Poltawski added a comment -

          Thanks Sam, i've integrated this now

          Show
          Dan Poltawski added a comment - Thanks Sam, i've integrated this now
          Hide
          Frédéric Massart added a comment -

          Successful test on master, 2.2 and 2.1

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

          U P S T R E A M I Z E D !

          Many thanks for the hard work, closing this as fixed.

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks for the hard work, closing this as fixed. Ciao
          Hide
          Ralf Krause added a comment -

          Hi,
          could there be a difference with this error between old users (first logged in 2008) and new users. Our Moodle is upgraded from 1.9 to 2.2.2 and I found a user who gets the same error but now in line 140.
          Ralf

          Show
          Ralf Krause added a comment - Hi, could there be a difference with this error between old users (first logged in 2008) and new users. Our Moodle is upgraded from 1.9 to 2.2.2 and I found a user who gets the same error but now in line 140. Ralf

            People

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

              Dates

              • Created:
                Updated:
                Resolved: