Moodle
  1. Moodle
  2. MDL-30754

Inconsistent breadcrumb when browsing categories

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.1.3, 2.3.2, 2.4
    • Fix Version/s: 2.3.3
    • Component/s: Navigation
    • Labels:
    • Testing Instructions:
      Hide
      1. Log in as an admin
      2. Create a new user
      3. Create a new category
      4. Assign your new user as a manager in that role
      5. Create a new course
      6. Enrol your user as a manager in that course
      7. Log out
      8. Log in as the manager
      9. Browse to the category page (/course/category.php?id=2)
      10. Check you get a breadcrumb there
      Show
      Log in as an admin Create a new user Create a new category Assign your new user as a manager in that role Create a new course Enrol your user as a manager in that course Log out Log in as the manager Browse to the category page (/course/category.php?id=2) Check you get a breadcrumb there
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-MDL-30754-m24
    • Rank:
      33682

      Description

      Breadcrumbs are sometimes shown and sometimes not when browsing categories (course/category.php?id=N).
      Example:

      • create a user, 1 category and 1 course inside that category
      • login as a user and go to that category (course/category.php?id=2)
      • notice that breadcrumbs are shown (see breadcrumbs-when-no-course.png)
      • enroll user in any course
      • go to the same category again (course/category.php?id=2)
      • notice that breadcrumbs are not shown (see breadcrums-when-enrolled.png)

      Tested on 2.1.3+ (Build: 20111205) and latest 2.3dev (Build: 20111209)

        Issue Links

          Activity

          Hide
          Tomasz Muras added a comment -

          The behavior was changed in commit c6a3aa720d33b058fbf8d199418cd8d120165b65, especially this change:

          diff --git a/lib/navigationlib.php b/lib/navigationlib.php
          index 625a516..64aa8c0 100644
          --- a/lib/navigationlib.php
          +++ b/lib/navigationlib.php
          @@ -1060,7 +1060,7 @@ class global_navigation extends navigation_node {
                   $mycourses = enrol_get_my_courses(NULL, 'visible DESC,sortorder ASC', $limit);
                   $showallcourses = (count($mycourses) == 0 || !empty($CFG->navshowallcourses));
                   $showcategories = ($showallcourses && $this->show_categories());
          -        $issite = ($this->page->course->id != SITEID);
          +        $issite = ($this->page->course->id == SITEID);
                   $ismycourse = (array_key_exists($this->page->course->id, $mycourses));
           
                   // Check if any courses were returned.
          
          Show
          Tomasz Muras added a comment - The behavior was changed in commit c6a3aa720d33b058fbf8d199418cd8d120165b65, especially this change: diff --git a/lib/navigationlib.php b/lib/navigationlib.php index 625a516..64aa8c0 100644 --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -1060,7 +1060,7 @@ class global_navigation extends navigation_node { $mycourses = enrol_get_my_courses(NULL, 'visible DESC,sortorder ASC', $limit); $showallcourses = (count($mycourses) == 0 || !empty($CFG->navshowallcourses)); $showcategories = ($showallcourses && $ this ->show_categories()); - $issite = ($ this ->page->course->id != SITEID); + $issite = ($ this ->page->course->id == SITEID); $ismycourse = (array_key_exists($ this ->page->course->id, $mycourses)); // Check if any courses were returned.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that.

          I've put that on the backlog.

          In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.

          Show
          Michael de Raadt added a comment - Thanks for reporting that. I've put that on the backlog. In the meantime feel free to help us work on this issue. If you are able to provide a patch, please add a patch label so we will spot it.
          Hide
          Sam Hemelryk added a comment -

          Hi Tomasz, I've just linked a couple of issues that duplicate this issue.
          I have a solution for this issue now and have commented on MDL-31631 to sort out with others who are working on it which issues we will use.
          Hopefully there will be a solution in core for this within a week or two.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Tomasz, I've just linked a couple of issues that duplicate this issue. I have a solution for this issue now and have commented on MDL-31631 to sort out with others who are working on it which issues we will use. Hopefully there will be a solution in core for this within a week or two. Cheers Sam
          Hide
          Didier Raboud added a comment -

          We are having the same issue on a 2.3 Moodle: Course category managers see no breadcrumbs where all other users see them for the same course category page.

          Show
          Didier Raboud added a comment - We are having the same issue on a 2.3 Moodle: Course category managers see no breadcrumbs where all other users see them for the same course category page.
          Hide
          Didier Raboud added a comment -

          I needed 4 hours but I found where this was failing (for the above problem we were having).

          The problem is in line 1201 in lib/navigationlib.php and can get solved that way:

          --- a/lib/navigationlib.php
          +++ b/lib/navigationlib.php
          @@ -1197,7 +1197,7 @@ class global_navigation extends navigation_node {
                           break;
                       case CONTEXT_COURSECAT :
                           // This has already been loaded we just need to map the variable
          -                if ($showcategories) {
          +                if ($this->show_categories()) {
                               $this->load_all_categories($this->page->context->instanceid, true);
                           }
                           break;
          

          I pushed that change to https://github.com/OdyX/moodle/tree/wip-MDL-30754-MOODLE_23_STABLE

          Cheers,

          OdyX

          Show
          Didier Raboud added a comment - I needed 4 hours but I found where this was failing (for the above problem we were having). The problem is in line 1201 in lib/navigationlib.php and can get solved that way: --- a/lib/navigationlib.php +++ b/lib/navigationlib.php @@ -1197,7 +1197,7 @@ class global_navigation extends navigation_node { break; case CONTEXT_COURSECAT : // This has already been loaded we just need to map the variable - if ($showcategories) { + if ($this->show_categories()) { $this->load_all_categories($this->page->context->instanceid, true); } break; I pushed that change to https://github.com/OdyX/moodle/tree/wip-MDL-30754-MOODLE_23_STABLE Cheers, OdyX
          Hide
          Sam Hemelryk added a comment -

          Thanks OdyX, I've had a look and a test, you change works perfectly and looks spot on. I've created branches for each version this should land to and am putting it up for integration now.

          Many thanks
          Sam

          Show
          Sam Hemelryk added a comment - Thanks OdyX, I've had a look and a test, you change works perfectly and looks spot on. I've created branches for each version this should land to and am putting it up for integration now. Many thanks Sam
          Hide
          Aparup Banerjee added a comment -

          thanks, thats integrated into master and 23.

          Show
          Aparup Banerjee added a comment - thanks, thats integrated into master and 23.
          Hide
          Brendan Barlow added a comment - - edited

          Applied the changes and it seems to work in Moodle 2.3.1 (Build: 20120706). Great work.
          Many thanks.

          Show
          Brendan Barlow added a comment - - edited Applied the changes and it seems to work in Moodle 2.3.1 (Build: 20120706). Great work. Many thanks.
          Hide
          Andrew Davis added a comment -

          Works as described in master and 2.3. Passing.

          Show
          Andrew Davis added a comment - Works as described in master and 2.3. Passing.
          Hide
          Andrew Davis added a comment -

          I've linked a bug I spotted while testing this.

          Show
          Andrew Davis added a comment - I've linked a bug I spotted while testing this.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          From somewhere within the clouds...

          Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration!

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - From somewhere within the clouds... Congrats, this has been sent upstream and is now part of Moodle (your favorite LMS platform). Many thanks for your awesome collaboration! Ciao

            People

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

              Dates

              • Created:
                Updated:
                Resolved: