Moodle
  1. Moodle
  2. MDL-30653

Participants listed twice in navbar

    Details

    • Testing Instructions:
      Hide

      Testing instructions

      • login as any user;
      • navigate to a course; and
      • from the 'Navigation' block, choose 'My Courses' => $coursename => 'Participants'

      Expected Result

      The navigation bar should read:

      Home / ► Courses / ► coursea / ► Participants

      where each component is a link to the relevant page

      Actual Result

      The navigation bar reads:

      Home / ► Courses / ► coursea / ► Participants / ► Participants

      where the final component is not a link

      Show
      Testing instructions login as any user; navigate to a course; and from the 'Navigation' block, choose 'My Courses' => $coursename => 'Participants' Expected Result The navigation bar should read: Home / ► Courses / ► coursea / ► Participants where each component is a link to the relevant page Actual Result The navigation bar reads: Home / ► Courses / ► coursea / ► Participants / ► Participants where the final component is not a link
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_21_STABLE, MOODLE_22_STABLE
    • Fixed Branches:
      MOODLE_20_STABLE, MOODLE_21_STABLE, MOODLE_22_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      MDL-30653-master-2

      Description

      When viewing the list of participants for a course, the Navigation Bar/breadcrumb reads:

      Home / ► Courses / ► coursea / ► Participants / ► Participants

      Where the final 'Participants' crumb is not a link

        Gliffy Diagrams

          Issue Links

            Activity

            Hide
            Andrew Nicols added a comment -

            Cherry-picks cleanly to 2.1 (and 2.2)

            Show
            Andrew Nicols added a comment - Cherry-picks cleanly to 2.1 (and 2.2)
            Hide
            Jason Fowler added a comment -

            Code looks good, makes sense too

            Show
            Jason Fowler added a comment - Code looks good, makes sense too
            Hide
            Eloy Lafuente (stronk7) 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
            Eloy Lafuente (stronk7) 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
            Sam Hemelryk added a comment -

            Hi Andrew,

            I've just looked at this now and I'm sending it back as there is one regression.
            If you view the front page participants page (admin can access) you will see there is no longer a navbar.

            I had a quick look at surrounding code, line 139 prevents the front page from using the active node on the navigation. I remember this being done so that `Site pages` isn't shown on the navbar as requested by Martin.
            The solution to this issue is to move the navbar->add call within that SITEID if statement above it, rather than removing it altogther.

            Cheers
            Sam

            Show
            Sam Hemelryk added a comment - Hi Andrew, I've just looked at this now and I'm sending it back as there is one regression. If you view the front page participants page (admin can access) you will see there is no longer a navbar. I had a quick look at surrounding code, line 139 prevents the front page from using the active node on the navigation. I remember this being done so that `Site pages` isn't shown on the navbar as requested by Martin. The solution to this issue is to move the navbar->add call within that SITEID if statement above it, rather than removing it altogther. Cheers Sam
            Hide
            Sam Hemelryk added a comment -

            Actually having looked at other site pages I see that they don't bother trying to remove site pages from the nav bar so perhaps a better solution is to remove the siteid if statement so that `site pages` is shown consistently.

            Show
            Sam Hemelryk added a comment - Actually having looked at other site pages I see that they don't bother trying to remove site pages from the nav bar so perhaps a better solution is to remove the siteid if statement so that `site pages` is shown consistently.
            Hide
            Andrew Nicols added a comment -

            I've updated the commit with your suggestion to remove the navbar->ignore_active() but in my testing I found that it was still more consistent to remove the navbar->add for the participants text (non-link).

            This has the same/similar breadcrumb organisation now as the site Tags, and site Blogs. Notes should be fixed by my commit in MDL-29195

            Show
            Andrew Nicols added a comment - I've updated the commit with your suggestion to remove the navbar->ignore_active() but in my testing I found that it was still more consistent to remove the navbar->add for the participants text (non-link). This has the same/similar breadcrumb organisation now as the site Tags, and site Blogs. Notes should be fixed by my commit in MDL-29195
            Hide
            Sam Hemelryk added a comment -

            Thanks for cleaning that up Andrew - your changes have been integrated now

            Show
            Sam Hemelryk added a comment - Thanks for cleaning that up Andrew - your changes have been integrated now
            Hide
            Ankit Agarwal added a comment -

            Everything working Great!
            Test passed!

            Show
            Ankit Agarwal added a comment - Everything working Great! Test passed!
            Hide
            Eloy Lafuente (stronk7) added a comment -

            Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks!

            Now... disconnect, relax and enjoy the next days, yay!

            Closing...ciao

            Show
            Eloy Lafuente (stronk7) added a comment - Whoever decided one week was worth 14 days had really one bad idea. Anyway, the nightmare is over, so thanks for your, once again, amazing contributions. Many, many thanks! Now... disconnect, relax and enjoy the next days, yay! Closing...ciao

              People

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

                Dates

                • Created:
                  Updated:
                  Resolved: