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
    • Rank:
      33470

      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

        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: