Moodle
  1. Moodle
  2. MDL-38631

Navigation of "my courses" shows all courses and course categories of the system when navshowmycoursecategories is set to "on"

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.4.3
    • Fix Version/s: 2.4.5, 2.5.1
    • Component/s: Navigation
    • Labels:
      None
    • Rank:
      48664

      Description

      In the moodle navigation is an area for "my courses" where you normally see only the courses you are enroled in.

      If you set navshowmycoursecategories in Site Administration --> Appearance --> Navigation to "off", you receive a flat list of your enroled courses (as expected).
      If you set navshowmycoursecategories to "on", you receive ALL courses and ALL course categories of the system, however depending on the variable navcourselimit.

      Replication steps:
      This requires a site with multiple courses in more than one category. There should be a student enrolled in one or more of the courses, but not all.

      1. Log in as a student in one browser.
      2. Click on Navigation > Home > My courses
      3. Note the courses shown
      4. Log in as an admin in another browser
      5. Navigate to Site Administration > Appearance > Navigation
      6. Check "Show my course categories" (navshowmycoursecategories)
      7. Save settings
      8. Go to student browser
      9. Refresh
      10. Click on Navigation > Home > My courses

      Expected result: only enrolled courses should be shown

      Actual result: all courses and categories are shown

      1. mycoursesexpanded.png
        8 kB
      2. screenshot-1.jpg
        195 kB
      3. screenshot-2.jpg
        202 kB

        Issue Links

          Activity

          Hide
          Matthias Baume added a comment -

          If you set navshowmycoursecategories in Site Administration --> Appearance --> Navigation to "off", you receive a flat list of your enroled courses (as expected).

          Show
          Matthias Baume added a comment - If you set navshowmycoursecategories in Site Administration --> Appearance --> Navigation to "off", you receive a flat list of your enroled courses (as expected).
          Hide
          Matthias Baume added a comment -

          If you set navshowmycoursecategories to "on", you receive ALL courses and ALL course categories of the system, however depending on the variable navcourselimit.

          Show
          Matthias Baume added a comment - If you set navshowmycoursecategories to "on", you receive ALL courses and ALL course categories of the system, however depending on the variable navcourselimit.
          Hide
          Michael de Raadt added a comment -

          Thanks for reporting that, Matthias.

          I've set this to be a minor security issue, but we may change that later if we decide this is not really a security concern.

          I was able to replicate the problem.

          I'll leave the rest to Sam to triage.

          Show
          Michael de Raadt added a comment - Thanks for reporting that, Matthias. I've set this to be a minor security issue, but we may change that later if we decide this is not really a security concern. I was able to replicate the problem. I'll leave the rest to Sam to triage.
          Hide
          Matthias Baume added a comment - - edited

          Dear Michael,

          we possibly found a fix for this issue.

          The most likely reason for the error is a faulty merge conflict resolution between commits d863b9 and 785103. See branch MDL-38631 in git://129.187.81.134/moodle/moodle.git . With this fix only user's own courses in "My Courses" are shown.

          After we fixed this problem, another issue became apparent - empty categories were still shown. The fix for this problem can be found in the branch hide_empty in the same repository. This implementation hides all the empty categories in "My courses".

          Should I open another ticket for the issue with the empty categories?

          Best regards,

          Matthias.

          Show
          Matthias Baume added a comment - - edited Dear Michael, we possibly found a fix for this issue. The most likely reason for the error is a faulty merge conflict resolution between commits d863b9 and 785103. See branch MDL-38631 in git://129.187.81.134/moodle/moodle.git . With this fix only user's own courses in "My Courses" are shown. After we fixed this problem, another issue became apparent - empty categories were still shown. The fix for this problem can be found in the branch hide_empty in the same repository. This implementation hides all the empty categories in "My courses". Should I open another ticket for the issue with the empty categories? Best regards, Matthias.
          Hide
          Michael de Raadt added a comment -

          It looks like this was reported earlier and the earlier issue has now been fixed.

          Show
          Michael de Raadt added a comment - It looks like this was reported earlier and the earlier issue has now been fixed.
          Hide
          Marina Glancy added a comment -

          Hi Matthias,
          thanks a lot for your solution. I tested it and it works fine! Looks like magic
          This issue was not closed mistakenly because the issue with absolutely the same contents was reported earlier (MDL-37329). But since you provided a fix under this issue number, I'm reopening this one and will shortly submit your solution for integration.
          Marina

          PS I'm assigning myself as an assignee because you are not in developers group but the commit will have your name

          Show
          Marina Glancy added a comment - Hi Matthias, thanks a lot for your solution. I tested it and it works fine! Looks like magic This issue was not closed mistakenly because the issue with absolutely the same contents was reported earlier ( MDL-37329 ). But since you provided a fix under this issue number, I'm reopening this one and will shortly submit your solution for integration. Marina PS I'm assigning myself as an assignee because you are not in developers group but the commit will have your name
          Hide
          Marina Glancy added a comment -

          I added commit that prevents links to the categories for My courses (unless there is a setting in navigation block)

          Show
          Marina Glancy added a comment - I added commit that prevents links to the categories for My courses (unless there is a setting in navigation block)
          Hide
          Marina Glancy added a comment -

          To testers: in case you experience AJAX category expansion problems in 2.5 and master - here is the issue MDL-39982

          Show
          Marina Glancy added a comment - To testers: in case you experience AJAX category expansion problems in 2.5 and master - here is the issue MDL-39982
          Hide
          Dan Poltawski added a comment -

          The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

          TIA and ciao

          Show
          Dan Poltawski added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
          Hide
          Matthias Baume added a comment - - edited

          Hello Moodle-Team,

          we've rebased the PULL branches on the latest MOODLE_24_STABLE. Branch MDL-38631 contains the fix for the "all categories are shown"-issue. There is also an additional branch, MDL-38631-hide_empty which adds empty category hiding. It would be awesome, if you coult take a look at the latter branch, too, since this functionality came quite handy to us and I can't think of a reason for have empty categories shown in "My Courses".

          Regards,

          Matthias Baume.

          Show
          Matthias Baume added a comment - - edited Hello Moodle-Team, we've rebased the PULL branches on the latest MOODLE_24_STABLE. Branch MDL-38631 contains the fix for the "all categories are shown"-issue. There is also an additional branch, MDL-38631 -hide_empty which adds empty category hiding. It would be awesome, if you coult take a look at the latter branch, too, since this functionality came quite handy to us and I can't think of a reason for have empty categories shown in "My Courses". Regards, Matthias Baume.
          Hide
          Marina Glancy added a comment -

          Hi Matthias, I have picked up your commit fixing the bug in my github branch and submitted for integration. I did not pick the commit with version bump because it's not needed. Also I added a commit removing the links under the categories.

          Please create a separate issue about the hiding empty categories. Thanks

          Show
          Marina Glancy added a comment - Hi Matthias, I have picked up your commit fixing the bug in my github branch and submitted for integration. I did not pick the commit with version bump because it's not needed. Also I added a commit removing the links under the categories. Please create a separate issue about the hiding empty categories. Thanks
          Hide
          Marina Glancy added a comment -

          TO INTEGRATORS:
          The submitted branch creates links under categories in "My courses" if the option "link categories" is set in navigation block.
          But I personally the links under categories in "My courses" extremely confusing, because clicking on it shows the list of ALL courses in category and highlights the node in the "Courses".
          This is an alternative solution that always removes the links under categories in "My courses":
          https://github.com/marinaglancy/moodle/compare/moodle:MOODLE_24_STABLE...wip-MDL-38631-m24alt

          Show
          Marina Glancy added a comment - TO INTEGRATORS: The submitted branch creates links under categories in "My courses" if the option "link categories" is set in navigation block. But I personally the links under categories in "My courses" extremely confusing, because clicking on it shows the list of ALL courses in category and highlights the node in the "Courses". This is an alternative solution that always removes the links under categories in "My courses": https://github.com/marinaglancy/moodle/compare/moodle:MOODLE_24_STABLE...wip-MDL-38631-m24alt
          Hide
          Sam Hemelryk added a comment -

          Thanks guys, this has been integrated now

          Show
          Sam Hemelryk added a comment - Thanks guys, this has been integrated now
          Hide
          Eloy Lafuente (stronk7) added a comment - - edited

          I've created this structure:

          • cat1, with course c1
          • cat2, with course c2
          • cat3, without courses and with subcategories:
            • cat31, with course c31
            • cat32, with course c32
            • cat33, with courses c331 and c332

          Then I've enrolled the user in all the "odd" courses (those which name ends with "1").

          • With $CFG->navshowmycoursecategories disabled I get the raw list of courses in the block (c1, c31 and c331). Correct.
          • With $CFG->navshowmycoursecategories enabled I get the categories tree and only the enrolled courses. Correct. But I also get the "cat32" category in the tree (non-expandable), but the user is not enrolled in any course there. See screenshot:

          Is correct to show the "c32" category in the block if the user has not enrolments there? I'm failing this as prevention, plz, clarify.

          Also, unrelated... I've seen that both in the front page (where I've combo list in settings) and in the course/index.php page... I get a list of the categories but the expand/collapse "triangles" don't seem to work at all. More specifically:

          • Frontpage: The subcategory triangles (cat31, cat32, cat33) in the combo do not work. The main ones (cat1, cat2, cat3) work ok.
          • course/index.php: No triangle works at all.

          Edited: The frontpage problem was because I had maxcategorydepth = 2... but still, I think that the courses under level2 cats should be shown. Uhm...

          Show
          Eloy Lafuente (stronk7) added a comment - - edited I've created this structure: cat1, with course c1 cat2, with course c2 cat3, without courses and with subcategories: cat31, with course c31 cat32, with course c32 cat33, with courses c331 and c332 Then I've enrolled the user in all the "odd" courses (those which name ends with "1"). With $CFG->navshowmycoursecategories disabled I get the raw list of courses in the block (c1, c31 and c331). Correct. With $CFG->navshowmycoursecategories enabled I get the categories tree and only the enrolled courses. Correct. But I also get the "cat32" category in the tree (non-expandable), but the user is not enrolled in any course there. See screenshot: Is correct to show the "c32" category in the block if the user has not enrolments there? I'm failing this as prevention, plz, clarify. Also, unrelated... I've seen that both in the front page (where I've combo list in settings) and in the course/index.php page... I get a list of the categories but the expand/collapse "triangles" don't seem to work at all. More specifically: Frontpage: The subcategory triangles (cat31, cat32, cat33) in the combo do not work. The main ones (cat1, cat2, cat3) work ok. course/index.php: No triangle works at all. Edited: The frontpage problem was because I had maxcategorydepth = 2... but still, I think that the courses under level2 cats should be shown. Uhm...
          Hide
          Dmytro Vorona added a comment -

          Hello Eloy,

          glad you asked. As requested, we've reported the empty category issue separatelly. Please see MDL-40297. The fix for this issue is in the same repository in the branch MDL-38631-hide_empty. Hope it finds it's way into Moodle Core, too.

          Show
          Dmytro Vorona added a comment - Hello Eloy, glad you asked. As requested, we've reported the empty category issue separatelly. Please see MDL-40297 . The fix for this issue is in the same repository in the branch MDL-38631 -hide_empty. Hope it finds it's way into Moodle Core, too.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Aha, thanks for creating MDL-40297, with that issue fixing the "empty categories" case, I think this can be safely passed, as far as the $CFG->navshowmycoursecategories and "link to categories" block setting are working as expected.

          So, passing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Aha, thanks for creating MDL-40297 , with that issue fixing the "empty categories" case, I think this can be safely passed, as far as the $CFG->navshowmycoursecategories and "link to categories" block setting are working as expected. So, passing, thanks!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Thanks for giving me joys and smiles
          Thanks for sharing my trouble's pile

          Thanks for wipeing the tears of my eye
          Thanks for showing me the glad view of sky

          Thanks for lending me your shoulders to lean
          Thanks for giving my words a proper mean

          Thanks for telling me the value of life
          Thanks for showing me the rules to survive

          Thanks for lending me the sympathetic ears
          Thanks for showing how much you care

          From all this what I mean in the end
          Is thanks for being my special friend.

          – Seema Chowdhury

          Sent upstream so... closing, thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Thanks for giving me joys and smiles Thanks for sharing my trouble's pile Thanks for wipeing the tears of my eye Thanks for showing me the glad view of sky Thanks for lending me your shoulders to lean Thanks for giving my words a proper mean Thanks for telling me the value of life Thanks for showing me the rules to survive Thanks for lending me the sympathetic ears Thanks for showing how much you care From all this what I mean in the end Is thanks for being my special friend. – Seema Chowdhury Sent upstream so... closing, thanks!

            People

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

              Dates

              • Created:
                Updated:
                Resolved: