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

          Attachments

            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