Moodle
  1. Moodle
  2. MDL-33536

Hidden categories are not hidden in the breadcrumbs and cause problems in Navigation block

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 2.2.3, 2.4.7, 2.5.3
    • Fix Version/s: 2.4.8, 2.5.4
    • Component/s: Navigation
    • Labels:
    • Testing Instructions:
      Hide
      1. Log in as an admin
      2. Create a category and a sub category
      3. In the sub category create a course and enrol a student.
      4. Log in as a student
      5. Browse the course and ensure the category is shown in the navbar
      6. Log in as an admin
      7. Hide the category
      8. Make the course visible again
      9. Log in as the student
      10. Browse to the course
      11. Ensure the category is not shown in the navbar
      Show
      Log in as an admin Create a category and a sub category In the sub category create a course and enrol a student. Log in as a student Browse the course and ensure the category is shown in the navbar Log in as an admin Hide the category Make the course visible again Log in as the student Browse to the course Ensure the category is not shown in the navbar
    • Affected Branches:
      MOODLE_22_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_24_STABLE, MOODLE_25_STABLE
    • Pull Master Branch:
      33536-26
    • Pull Master Diff URL:

      Description

      Admin hides a course category but makes the courses in the category visible. Admin can see the category link dimmed in the navbar, as expected. Other users can see the link in the navbar, not dimmed and accessible. When a user clicks the visible link of the hidden category the user is notified: Sorry, but you do not currently have permissions to do that (See hidden categories).

      Expected behavior: If Admin hides a category then the user should not see a link to it to begin with.

      Possible fix: add to the navbar method of core_renderer (outputrenderers) a condition on the node's display property

      if (!$item->display) {
          continue;
      }

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Michael de Raadt added a comment -

            I was trying to replicate this and I thought I should check if I'm seeing the same thing you are. I will attach a screenshot.

            I can create a hidden category with an unhidden course inside. Only users enrolled in the unhidden course can see the link. They can't see the category link but they can see the unhidden course.

            Show
            Michael de Raadt added a comment - I was trying to replicate this and I thought I should check if I'm seeing the same thing you are. I will attach a screenshot. I can create a hidden category with an unhidden course inside. Only users enrolled in the unhidden course can see the link. They can't see the category link but they can see the unhidden course.
            Hide
            Michael de Raadt added a comment -

            If what I am seeing is the same as what you're seeing, I think the current behaviour is reasonable. A deliberately unhidden course should appear. If you're seeing something different, then we have an issue.

            Show
            Michael de Raadt added a comment - If what I am seeing is the same as what you're seeing, I think the current behaviour is reasonable. A deliberately unhidden course should appear. If you're seeing something different, then we have an issue.
            Hide
            Itamar Tzadok added a comment - - edited

            As shown in the attached image admin (top) sees the hidden cat as hidden but guest (bottom) sees an active link. The link will also be visible to a logged in user in a course the user is not enrolled to but allows guest access.

            Show
            Itamar Tzadok added a comment - - edited As shown in the attached image admin (top) sees the hidden cat as hidden but guest (bottom) sees an active link. The link will also be visible to a logged in user in a course the user is not enrolled to but allows guest access.
            Hide
            Michael de Raadt added a comment -

            OK. I thought you were talking about the navigation block.

            I also noted that with a hidden category a few other things misbehave for an administrator. On a few pages, the Navigation > Courses link disappears. I found this for...

            • Navigation > Site pages > Site blogs
            • Navigation > My profile > Messages
            • Navigation > My profile > My private files
            • Navigation > My profile > Notes

            The page Navigation > My profile > Forum posts > Posts dies with a missing string error "someinstancesdisabled".

            It looks like this needs a bit of work.

            Thanks for the report.

            Show
            Michael de Raadt added a comment - OK. I thought you were talking about the navigation block. I also noted that with a hidden category a few other things misbehave for an administrator. On a few pages, the Navigation > Courses link disappears. I found this for... Navigation > Site pages > Site blogs Navigation > My profile > Messages Navigation > My profile > My private files Navigation > My profile > Notes The page Navigation > My profile > Forum posts > Posts dies with a missing string error "someinstancesdisabled". It looks like this needs a bit of work. Thanks for the report.
            Hide
            Marina Glancy added a comment -

            Sam, I can see that you are assignee for this issue.
            This is a small fix for the breadcrumb. Does it solve this issue?
            https://github.com/marinaglancy/moodle/compare/moodle:master...wip-MDL-33536-master

            Show
            Marina Glancy added a comment - Sam, I can see that you are assignee for this issue. This is a small fix for the breadcrumb. Does it solve this issue? https://github.com/marinaglancy/moodle/compare/moodle:master...wip-MDL-33536-master
            Hide
            Marina Glancy added a comment -

            Sam, can you peer review the patch please?

            Show
            Marina Glancy added a comment - Sam, can you peer review the patch please?
            Hide
            Sam Hemelryk added a comment -

            Hi Marina - I've looked at this as promised.
            I think however that the correct solution is probably to hide the category from the navbar altogether if the category is hidden.
            Hiding categories but allowing courses to be visible was always a grey area but I believe the above would be accurate to how its traditionally worked.

            I'm heading off for lunch but will give the above a shot afterwards

            Show
            Sam Hemelryk added a comment - Hi Marina - I've looked at this as promised. I think however that the correct solution is probably to hide the category from the navbar altogether if the category is hidden. Hiding categories but allowing courses to be visible was always a grey area but I believe the above would be accurate to how its traditionally worked. I'm heading off for lunch but will give the above a shot afterwards
            Hide
            Sam Hemelryk added a comment -

            What about something like this Marina?

            git fetch git://github.com/samhemelryk/moodle.git 33536-26
            http://icodedthat.com/33536-26

            That ensures that a hidden category is not added to the navigation if the user is not able to see it, and that it gets dimmed if the user is able to see it.

            Show
            Sam Hemelryk added a comment - What about something like this Marina? git fetch git://github.com/samhemelryk/moodle.git 33536-26 http://icodedthat.com/33536-26 That ensures that a hidden category is not added to the navigation if the user is not able to see it, and that it gets dimmed if the user is able to see it.
            Hide
            Marina Glancy added a comment -

            Hi Sam, this is an interesting suggestion - if user is not allowed to view hidden categories, he will see the course as if it was in the nearest visible parent category. Looks good to me!

            Show
            Marina Glancy added a comment - Hi Sam, this is an interesting suggestion - if user is not allowed to view hidden categories, he will see the course as if it was in the nearest visible parent category. Looks good to me!
            Hide
            Sam Hemelryk added a comment -

            Thanks Marina - putting this up for integration review.

            Show
            Sam Hemelryk added a comment - Thanks Marina - putting this up for integration review.
            Hide
            Marina Glancy added a comment -

            I swapped assignee and peer reviewer

            Show
            Marina Glancy added a comment - I swapped assignee and peer reviewer
            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
            Damyon Wiese added a comment -

            I noticed that there is no explicit include for course/lib.php (required for get_category_or_system_context)

            I pushed some branches with an extra commit

            git://github.com/damyon/moodle.git

            Branches: MDL-33536-24, MDL-33536-25, MDL-33536-master

            Can you give these a quick look over before I push them Marina?

            Thanks!

            Show
            Damyon Wiese added a comment - I noticed that there is no explicit include for course/lib.php (required for get_category_or_system_context) I pushed some branches with an extra commit git://github.com/damyon/moodle.git Branches: MDL-33536 -24, MDL-33536 -25, MDL-33536 -master Can you give these a quick look over before I push them Marina? Thanks!
            Hide
            Marina Glancy added a comment -

            Looks good Damyon, thanks for fix

            Show
            Marina Glancy added a comment - Looks good Damyon, thanks for fix
            Hide
            Damyon Wiese added a comment -

            Integrated the branches with the fixes to 24, 25 and master.

            Thanks Sam (and Marina).

            Show
            Damyon Wiese added a comment - Integrated the branches with the fixes to 24, 25 and master. Thanks Sam (and Marina).
            Hide
            Barbara Ramiro added a comment -

            It works as expected based on the testing instructions. I noticed though that there is no option to make the subcategories visible on master unlike on 2.5 and 2.4

            Show
            Barbara Ramiro added a comment - It works as expected based on the testing instructions. I noticed though that there is no option to make the subcategories visible on master unlike on 2.5 and 2.4
            Hide
            Barbara Ramiro added a comment -

            Created this issue about the subcategories not having the option to show when categories are hidden MDL-42858

            Show
            Barbara Ramiro added a comment - Created this issue about the subcategories not having the option to show when categories are hidden MDL-42858
            Hide
            Eloy Lafuente (stronk7) added a comment -

            It's Friday, I'm tired so I won't be very imaginative today.

            No matter of that, yes, you did it! Thanks for your collaboration!

            Closing this as fixed!

            Show
            Eloy Lafuente (stronk7) added a comment - It's Friday, I'm tired so I won't be very imaginative today. No matter of that, yes, you did it! Thanks for your collaboration! Closing this as fixed!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                6 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: