Moodle
  1. Moodle
  2. MDL-28967

Categories intermittently disappearing from Navigation block's "Courses" list

    Details

    • Workaround:
      Hide

      Set $CFG->navcourselimit to be higher than the number of courses in your Moodle instance.

      Show
      Set $CFG->navcourselimit to be higher than the number of courses in your Moodle instance.
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE
    • Rank:
      18515

      Description

      I've found that after the changes to the navigation block for MDL-27809, the categories listed in the Navigation block's "Courses" list come and go. Before that change, all the top-level categories were listed. Now, if I'm in a course then some categories will be missing. I haven't been able to figure out exactly what configuration options are necessary to make this come and go, but I have been able to create a test setup that causes this problem:

      Create this hierarchy of categories and courses:

      Category 1

      • Category 1.1
        • Course 1.1a
        • Course 1.1b
        • Course 1.1c
      • Category 1.2
        • Course 1.2a
        • Course 1.2b
        • Course 1.2c
      • Course 1a
      • Course 1b

      Category 2

      • Course 2a

      Turn on $CFG->navshowcategories and $CFG->navshowallcourses, and set $CFG->navcourselimit to 5.

      Now, from the /course/index.php page or other site pages, you'll see all the courses and categories listed in the Navigation block. However, if you navigate to course 1.2c, You'll find that Category 2 no longer shows up in the Navigation block.

      The expected behavior is that Category 1 and Category 2, as the top level categories, should show up on every page. Or there should at least be a "Show all courses/categories" link to indicate that some categories are hidden.

      If I revert commit e26507b361969c3e1f1f, then I find that it behaves as expected, and all the categories are shown when I'm in course 1.2c.

        Issue Links

        Progress
        Resolved Sub-Tasks

        Sub-Tasks

        There are no Sub-Tasks for this issue.

          Activity

          Hide
          Aaron Wells added a comment -

          Attaching screenshots of what the Navigation block looks like when replicating the issue. Notice how in "navigation-on-site-page.png", Category 2 shows up, while in "navigation-on-course-page.png", Category 2 is missing entirely, and all categories are expanded but there is no "Show all courses/categories" link to indicate that the list is truncated.

          Show
          Aaron Wells added a comment - Attaching screenshots of what the Navigation block looks like when replicating the issue. Notice how in "navigation-on-site-page.png", Category 2 shows up, while in "navigation-on-course-page.png", Category 2 is missing entirely, and all categories are expanded but there is no "Show all courses/categories" link to indicate that the list is truncated.
          Hide
          Michael de Raadt added a comment -

          I'm not sure if this is a bug or by design. There may be performance issues related to this.

          Show
          Michael de Raadt added a comment - I'm not sure if this is a bug or by design. There may be performance issues related to this.
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          I have just been looking into this, it is unfortunately a regression caused by the navigation performance changes.
          The problem appears to be arising because of the bulk loading of courses being impacted by the limit.

          I'll continue to investigate a look for a solution.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, I have just been looking into this, it is unfortunately a regression caused by the navigation performance changes. The problem appears to be arising because of the bulk loading of courses being impacted by the limit. I'll continue to investigate a look for a solution. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Putting this up for peer-review now. I will also ask Apu to look at this especially.

          Show
          Sam Hemelryk added a comment - Putting this up for peer-review now. I will also ask Apu to look at this especially.
          Hide
          Sam Hemelryk added a comment -

          Just noting that these changes do have performance implications the previous loading of courses we incomplete and on average to correct it I have had to introduce an additional 2 DB queries.
          Please note that in fact I introduced more queries but by tiding up existing procedures I have managed to cut out nearly as many as I introduced.

          Show
          Sam Hemelryk added a comment - Just noting that these changes do have performance implications the previous loading of courses we incomplete and on average to correct it I have had to introduce an additional 2 DB queries. Please note that in fact I introduced more queries but by tiding up existing procedures I have managed to cut out nearly as many as I introduced.
          Hide
          Aparup Banerjee added a comment -

          ok, this looks good to me.
          Its a large patch but i couldn't spot a thing wrong.

          The large test , is that in MDLQA tests?

          sending this up for integration as per Sam's request (after peer-review).

          Show
          Aparup Banerjee added a comment - ok, this looks good to me. Its a large patch but i couldn't spot a thing wrong. The large test , is that in MDLQA tests? sending this up for integration as per Sam's request (after peer-review).
          Hide
          Sam Hemelryk added a comment -

          Hi Apu,

          Thanks for looking at that for me!
          The tests aren't QA tests unfortunately, they are my attempt to summarise the situation with a set of tests that focus on the three settings.
          In hindsight (what a wonderful thing) I really dislike those settings, they are at best confusing, and I think in most cases more trouble than they are worth.
          But what is done is done, I would like in the next release perhaps to refactor the generation code to reorganise it and straighten out these settings at the same time.
          I think I had been with Moodle just a couple of months when I wrote the navigation and for sure I have a much better understanding now!

          Anyway thanks for checking that out, I will bring this to Eloy's attention when I get a chance.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Apu, Thanks for looking at that for me! The tests aren't QA tests unfortunately, they are my attempt to summarise the situation with a set of tests that focus on the three settings. In hindsight (what a wonderful thing) I really dislike those settings, they are at best confusing, and I think in most cases more trouble than they are worth. But what is done is done, I would like in the next release perhaps to refactor the generation code to reorganise it and straighten out these settings at the same time. I think I had been with Moodle just a couple of months when I wrote the navigation and for sure I have a much better understanding now! Anyway thanks for checking that out, I will bring this to Eloy's attention when I get a chance. Cheers Sam
          Hide
          Eloy Lafuente (stronk7) added a comment -

          After chatting a bit about this... and being paranoid about performance and complexity we have agreed to delay it after release, to be able to, mainly, test it under big simulated environment, with all roles, dispersion of enrolled courses...

          So this is going to be reopen, aiming for some time to revisit and test it. Thanks Sam!

          PS: Also, to avoid forgetting it, I proposed to reconsider why the hell we need to show all that "recursive" information there. There is already one UI to navigate/search courses and categories, and also, showing activities here is highly arguable. Surely this needs broad discussion. My basic proposition is to have one "simplernav" setting taking rid of all that (cat/course/section/activity) complexity.

          Show
          Eloy Lafuente (stronk7) added a comment - After chatting a bit about this... and being paranoid about performance and complexity we have agreed to delay it after release, to be able to, mainly, test it under big simulated environment, with all roles, dispersion of enrolled courses... So this is going to be reopen, aiming for some time to revisit and test it. Thanks Sam! PS: Also, to avoid forgetting it, I proposed to reconsider why the hell we need to show all that "recursive" information there. There is already one UI to navigate/search courses and categories, and also, showing activities here is highly arguable. Surely this needs broad discussion. My basic proposition is to have one "simplernav" setting taking rid of all that (cat/course/section/activity) complexity.
          Hide
          Sam Hemelryk added a comment -

          Thanks Eloy - just noting that I will revisitng this immediately after the release of 2.2 and aiming to get it in and then opening an issue for master only to see those settings tidied up and investigate navigation use and whether we can cut things back now.

          Show
          Sam Hemelryk added a comment - Thanks Eloy - just noting that I will revisitng this immediately after the release of 2.2 and aiming to get it in and then opening an issue for master only to see those settings tidied up and investigate navigation use and whether we can cut things back now.
          Hide
          Sam Hemelryk added a comment -

          Ok I've now updated this after the 2.2 release and have fixed one minor regression.
          Just need to test this on a site with thousands of courses in hundreds of categories

          Show
          Sam Hemelryk added a comment - Ok I've now updated this after the 2.2 release and have fixed one minor regression. Just need to test this on a site with thousands of courses in hundreds of categories
          Hide
          Michael de Raadt added a comment -

          Carrying this over to the next STABLE Sprint.

          Show
          Michael de Raadt added a comment - Carrying this over to the next STABLE Sprint.
          Hide
          Brian King added a comment -

          I've tested this out on Moodle 2.2.2, as an alternative to the fix for MDL-31986, and can report that it works well for my case.

          I would however propose either changing the behaviour, or adding a config option to change the behaviour for the following situations:

          • When on a course category page: do not auto-expand the courses node of the navigation. The main view is what most likely interests the user, the expanded courses node is just extra noise in most cases.
          • Do not auto-expand the courses node when in one of "my courses".

          On a Moodle with a lot of courses and categories, an expanded courses node of the navigation block makes the navigation block so tall that any blocks underneath it will be harder to reach (e.g. require scrolling to reach). Perhaps a config option to never auto-expand the courses node would be helpful.

          Show
          Brian King added a comment - I've tested this out on Moodle 2.2.2, as an alternative to the fix for MDL-31986 , and can report that it works well for my case. I would however propose either changing the behaviour, or adding a config option to change the behaviour for the following situations: When on a course category page: do not auto-expand the courses node of the navigation. The main view is what most likely interests the user, the expanded courses node is just extra noise in most cases. Do not auto-expand the courses node when in one of "my courses". On a Moodle with a lot of courses and categories, an expanded courses node of the navigation block makes the navigation block so tall that any blocks underneath it will be harder to reach (e.g. require scrolling to reach). Perhaps a config option to never auto-expand the courses node would be helpful.
          Hide
          Michael de Raadt added a comment -

          Carrying over to the new sprint.

          Show
          Michael de Raadt added a comment - Carrying over to the new sprint.
          Hide
          Sam Hemelryk added a comment -

          Increasing the priority of this issue as there are several duplicates now.

          Show
          Sam Hemelryk added a comment - Increasing the priority of this issue as there are several duplicates now.
          Hide
          Howard Miller added a comment -

          I'm seeing what looks like this problem in 2.2. I take it (looking at the duplicates) that this is the same issue?

          Show
          Howard Miller added a comment - I'm seeing what looks like this problem in 2.2. I take it (looking at the duplicates) that this is the same issue?
          Hide
          Sam Hemelryk added a comment -

          Hi Howard,

          There is a good chance that what you are seeing is this issue, and it will be affecting 21, 22, and master.
          It will lead to course and categories to be intermittently missing from the navigation as the user moves around the site, or in some cases missed altogether.
          The problem is stemming from overzealous limiting, there is a navigation fix landing this week to improve the categories navigation and I hope to have the fix for this issue in next week (waiting presently for the weekly release).

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Howard, There is a good chance that what you are seeing is this issue, and it will be affecting 21, 22, and master. It will lead to course and categories to be intermittently missing from the navigation as the user moves around the site, or in some cases missed altogether. The problem is stemming from overzealous limiting, there is a navigation fix landing this week to improve the categories navigation and I hope to have the fix for this issue in next week (waiting presently for the weekly release). Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Hi guys,

          The solution for this issue is quite large and complex, because of this I have decided that it is best to implement this on master 1 or 2 weeks in advance of backporting it to Moodle 2.2+ and Moodle 2.1+.
          This way we can weed out any bugs or performance issues before this gets into the stable branches.

          I've create MDL-32267 to see it integrated on the master branch, and MDL-32268 to backport it to 2.2+ and 2.1+ when we are ready.
          I have the master branch changes up for peer-review now.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi guys, The solution for this issue is quite large and complex, because of this I have decided that it is best to implement this on master 1 or 2 weeks in advance of backporting it to Moodle 2.2+ and Moodle 2.1+. This way we can weed out any bugs or performance issues before this gets into the stable branches. I've create MDL-32267 to see it integrated on the master branch, and MDL-32268 to backport it to 2.2+ and 2.1+ when we are ready. I have the master branch changes up for peer-review now. Cheers Sam
          Hide
          Jay Knight added a comment -

          Since MDL-32367 is marked as a duplicate of this, does the work on this issue deal with the "student should not been shown the courses he is not enrolled in and cannot enroll himself manually" part of that issue? I'm looking for a way to "hide" courses from users that they can't see or self-enroll in (see also MDL-23548). MDL-23548 has a little patch that can show/hide courses based on the show_enrolme_link function in the selected enrol_plugins and guest access settings for each course.

          I'm just trying to figure out if these changes will have any impact toward getting something like that to work.

          Show
          Jay Knight added a comment - Since MDL-32367 is marked as a duplicate of this, does the work on this issue deal with the "student should not been shown the courses he is not enrolled in and cannot enroll himself manually" part of that issue? I'm looking for a way to "hide" courses from users that they can't see or self-enroll in (see also MDL-23548 ). MDL-23548 has a little patch that can show/hide courses based on the show_enrolme_link function in the selected enrol_plugins and guest access settings for each course. I'm just trying to figure out if these changes will have any impact toward getting something like that to work.
          Hide
          Eloy Lafuente (stronk7) added a comment -

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

          Many thanks, this is now available in all the repos (git & cvs).

          Closing, ciao

          Show
          Eloy Lafuente (stronk7) added a comment - U P S T R E A M I Z E D ! Many thanks, this is now available in all the repos (git & cvs). Closing, ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Doh,

          somehow this issue was closed incorrectly when processing all the integrated issues this week. (sort of most voted and current in integration filters mix). Apologies for the confusion, reseting to previous status!

          Ciao, Eloy

          Show
          Eloy Lafuente (stronk7) added a comment - Doh, somehow this issue was closed incorrectly when processing all the integrated issues this week. (sort of most voted and current in integration filters mix). Apologies for the confusion, reseting to previous status! Ciao, Eloy
          Hide
          Daniel Kaelin added a comment -

          I'm having a problem where the courses aren't listing under the Navigation block at all.

          The navigation block itself is edited to show "Everything" and on Site Administration -> Appearance -> Navigation I have it set to show all courses and all categories. I tried modifying the number of courses to show to various counts and that did not help the issue.

          Are there any other settings on a Moodle portal that would result in the Navigation block not displaying any courses, even for administrators?

          Show
          Daniel Kaelin added a comment - I'm having a problem where the courses aren't listing under the Navigation block at all. The navigation block itself is edited to show "Everything" and on Site Administration -> Appearance -> Navigation I have it set to show all courses and all categories. I tried modifying the number of courses to show to various counts and that did not help the issue. Are there any other settings on a Moodle portal that would result in the Navigation block not displaying any courses, even for administrators?
          Hide
          Sam Hemelryk added a comment -

          Hi Daniel, what version of Moodle are you using?
          Also just to clarify, you're not seeing any courses at all in the navigation block?

          Show
          Sam Hemelryk added a comment - Hi Daniel, what version of Moodle are you using? Also just to clarify, you're not seeing any courses at all in the navigation block?
          Hide
          Sam Hemelryk added a comment -

          Hmm that is a curious issue.

          Perhaps you'd mind checking/trying a couple of things?

          1. Check the server logs for the site in question, just to make sure that there's nothing nasty happening that you're not seeing reported.
          2. Enable navshowallcourses if it is not currently enabled. This forces the navigation to load everything it can.
          3. Question: Is the admin enrolled in any courses?
          4. Question: Do students see the courses populated, and do they see the courses they are enrolled in under my courses?

          The following you could try if you are able to edit the source code file.
          1. Open up lib/navigationlib.php, goto line 1435, you should see a call to $DB->get_records_sql, add the following immediately after that:

          echo "<p>Total course count: ".count($courses)."</p>";
          

          That will cause the number of courses to be displayed when you view a page.
          Open up the front page in your browser and log in as an admin, find the line that reads "Total course count: ", if its 0 then we've found the problem otherwise we know its deeper.

          Hope some of this helps.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hmm that is a curious issue. Perhaps you'd mind checking/trying a couple of things? 1. Check the server logs for the site in question, just to make sure that there's nothing nasty happening that you're not seeing reported. 2. Enable navshowallcourses if it is not currently enabled. This forces the navigation to load everything it can. 3. Question: Is the admin enrolled in any courses? 4. Question: Do students see the courses populated, and do they see the courses they are enrolled in under my courses? The following you could try if you are able to edit the source code file. 1. Open up lib/navigationlib.php, goto line 1435, you should see a call to $DB->get_records_sql, add the following immediately after that: echo "<p>Total course count: " .count($courses). "</p>" ; That will cause the number of courses to be displayed when you view a page. Open up the front page in your browser and log in as an admin, find the line that reads "Total course count: ", if its 0 then we've found the problem otherwise we know its deeper. Hope some of this helps. Cheers Sam

            People

            • Votes:
              21 Vote for this issue
              Watchers:
              27 Start watching this issue

              Dates

              • Created:
                Updated: