Uploaded image for project: '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
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.4.3
    • Fix Version/s: 2.4.5, 2.5.1
    • Component/s: Navigation
    • Labels:
      None
    • Testing Instructions:
      Hide
      1. Create several categories and several courses, enrol student in some of them
      2. Enable categories in my courses
      3. Make sure only courses where user is enrolled are shown under "My courses"
      4. Configure navigation block to link to categories
      5. Make sure categories in "My courses" have links now
      Show
      Create several categories and several courses, enrol student in some of them Enable categories in my courses Make sure only courses where user is enrolled are shown under "My courses" Configure navigation block to link to categories Make sure categories in "My courses" have links now
    • Affected Branches:
      MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      wip-MDL-38631-master

      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

        Gliffy Diagrams

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

          Issue Links

            Activity

            Hide
            matthias.baume@tum.de 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@tum.de 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@tum.de 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@tum.de 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
            salvetore 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
            salvetore 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@tum.de 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@tum.de 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
            salvetore Michael de Raadt added a comment -

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

            Show
            salvetore Michael de Raadt added a comment - It looks like this was reported earlier and the earlier issue has now been fixed.
            Hide
            marina 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 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 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 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 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 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
            poltawski 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
            poltawski 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@tum.de 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@tum.de 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 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 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 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 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
            samhemelryk Sam Hemelryk added a comment -

            Thanks guys, this has been integrated now

            Show
            samhemelryk Sam Hemelryk added a comment - Thanks guys, this has been integrated now
            Hide
            stronk7 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
            stronk7 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
            alendit 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
            alendit 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
            stronk7 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
            stronk7 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
            stronk7 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
            stronk7 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:
                  Fix Release Date:
                  8/Jul/13