Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-30754

Inconsistent breadcrumb when browsing categories

    Details

    • Type: Bug
    • Status: Closed
    • Priority: 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

      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)

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            tmuras 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
            tmuras 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
            salvetore 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
            salvetore 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
            samhemelryk 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
            samhemelryk 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
            odyx 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
            odyx 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
            odyx 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
            odyx 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
            samhemelryk 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
            samhemelryk 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
            nebgor Aparup Banerjee added a comment -

            thanks, thats integrated into master and 23.

            Show
            nebgor Aparup Banerjee added a comment - thanks, thats integrated into master and 23.
            Hide
            brendan.barlow5 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.barlow5 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
            andyjdavis Andrew Davis added a comment -

            Works as described in master and 2.3. Passing.

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

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

            Show
            andyjdavis Andrew Davis added a comment - I've linked a bug I spotted while testing this.
            Hide
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  12/Nov/12