Merge pull request #526 from communitiesuk/primary-navigation-highlighting

Add navigation items helper

app/components/primary_navigation_component.html.erb

@@ -2,13 +2,13 @@
   <div class="govuk-width-container">
     <ul class="app-primary-navigation__list">
       <% items.each do |item| %>
-        <% if highlighted_tab?(item, request.fullpath) %>
+        <% if item.current %>
           <li class="app-primary-navigation__item app-primary-navigation__item--current">
-            <%= govuk_link_to item[:name], item[:url], class: "app-primary-navigation__link", aria: { current: "page" } %>
+            <%= govuk_link_to item[:text], item[:href], class: "app-primary-navigation__link", aria: { current: "page" } %>
           </li>
         <% else %>
           <li class="app-primary-navigation__item">
-            <%= govuk_link_to item[:name], item[:url], class: "app-primary-navigation__link" %>
+            <%= govuk_link_to item[:text], item[:href], class: "app-primary-navigation__link" %>
           </li>
         <% end %>
       <% end %>

app/components/primary_navigation_component.rb

@@ -6,7 +6,7 @@ class PrimaryNavigationComponent < ViewComponent::Base
     super
   end
 
-  def highlighted_tab?(item, path)
-    item.fetch(:current, false) || item.fetch(:comparable_urls).any? { |url| path.include?(url) }
+  def highlighted_tab?(item, _path)
+    item[:current]
   end
 end

app/helpers/navigation_items_helper.rb

@@ -0,0 +1,33 @@
+module NavigationItemsHelper
+  NavigationItem = Struct.new(:text, :href, :current, :classes)
+
+  def primary_items(path, current_user)
+    if current_user.support?
+      [
+        NavigationItem.new("Organisations", organisations_path, organisation_current?(path)),
+        NavigationItem.new("Users", "/users", users_current?(path)),
+        NavigationItem.new("Logs", case_logs_path, logs_current?(path)),
+      ]
+    else
+      [
+        NavigationItem.new("Logs", case_logs_path, logs_current?(path)),
+        NavigationItem.new("Users", users_organisation_path(current_user.organisation), users_current?(path)),
+        NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", organisation_current?(path)),
+      ]
+    end
+  end
+
+private
+
+  def logs_current?(path)
+    path.include?("/logs")
+  end
+
+  def users_current?(path)
+    path.include?("/users")
+  end
+
+  def organisation_current?(path)
+    path.include?("/organisations") && !path.include?("/users")
+  end
+end

app/views/layouts/application.html.erb

@@ -65,20 +65,9 @@
     ) %>
 
     <% if !current_user.nil? %>
-      <% if current_user.support? %>
-        <% items = [
-          { name: "Organisations", url: "/organisations", comparable_urls: ["/details", "/organisations"] },
-          { name: "Users", url: "/users", comparable_urls: ["/users", "/account"] },
-          { name: "Logs", url: case_logs_path, comparable_urls: ["/logs"] },
-        ] %>
-      <% else %>
-        <% items = [
-          { name: "Logs", url: case_logs_path, comparable_urls: ["/logs"] },
-          { name: "Users", url: users_organisation_path(current_user.organisation), comparable_urls: ["/users", "/account"] },
-          { name: "About your organisation", url: "/organisations/#{current_user.organisation.id}", comparable_urls: ["/organisations"] },
-        ] %>
-      <% end %>
-      <%= render PrimaryNavigationComponent.new(items:) %>
+      <%= render PrimaryNavigationComponent.new(
+        items: primary_items(request.path, current_user),
+      ) %>
     <% end %>
 
     <div class="govuk-width-container">

spec/components/primary_navigation_component_spec.rb

@@ -2,9 +2,11 @@ require "rails_helper"
 
 RSpec.describe PrimaryNavigationComponent, type: :component do
   let(:items) do
-    [{ name: "Organisations", url: "/organisations", current: true, comparable_urls: ["/organisations", "/something-else"] },
-     { name: "Users", url: "/users", comparable_urls: ["/users"] },
-     { name: "Logs ", url: "/logs", comparable_urls: ["/logs"] }]
+    [
+      NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true),
+      NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
+      NavigationItemsHelper::NavigationItem.new("Logs ", "/logs", false),
+    ]
   end
 
   context "when the item is 'current' in nav tabs" do

spec/features/user_spec.rb

@@ -198,6 +198,38 @@ RSpec.describe "User Features" do
   context "when signed in as a data coordinator" do
     let!(:user) { FactoryBot.create(:user, :data_coordinator, last_sign_in_at: Time.zone.now) }
 
+    context "when viewing users" do
+      before do
+        visit("/logs")
+        fill_in("user[email]", with: user.email)
+        fill_in("user[password]", with: "pAssword1")
+        click_button("Sign in")
+        click_link("Users")
+      end
+
+      it "highlights the users navigation tab" do
+        expect(page).to have_css('[aria-current="page"]', text: "Users")
+        expect(page).not_to have_css('[aria-current="page"]', text: "About your organisation")
+        expect(page).not_to have_css('[aria-current="page"]', text: "Logs")
+      end
+    end
+
+    context "when viewing your organisation details" do
+      before do
+        visit("/logs")
+        fill_in("user[email]", with: user.email)
+        fill_in("user[password]", with: "pAssword1")
+        click_button("Sign in")
+        click_link("About your organisation")
+      end
+
+      it "highlights the users navigation tab" do
+        expect(page).to have_css('[aria-current="page"]', text: "About your organisation")
+        expect(page).not_to have_css('[aria-current="page"]', text: "Users")
+        expect(page).not_to have_css('[aria-current="page"]', text: "Logs")
+      end
+    end
+
     context "when viewing your account" do
       before do
         visit("/logs")
@@ -213,6 +245,11 @@ RSpec.describe "User Features" do
         expect(page).to have_current_path("/account")
       end
 
+      it "does not highlight the users navigation tab" do
+        visit("/account")
+        expect(page).not_to have_css('[aria-current="page"]', text: "Users")
+      end
+
       it "can navigate to change your password page from main account page" do
         visit("/account")
         find('[data-qa="change-password"]').click

spec/helpers/navigation_items_helper_spec.rb

@@ -0,0 +1,86 @@
+require "rails_helper"
+
+RSpec.describe NavigationItemsHelper do
+  let(:current_user) { FactoryBot.create(:user, :data_coordinator) }
+
+  let(:users_path) { "/organisations/#{current_user.organisation.id}/users" }
+  let(:organisation_path) { "/organisations/#{current_user.organisation.id}" }
+
+  describe "#primary items" do
+    context "when the user is a data coordinator" do
+      context "when the user is on the users page" do
+        let(:expected_navigation_items) do
+          [
+            NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false),
+            NavigationItemsHelper::NavigationItem.new("Users", users_path, true),
+            NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
+          ]
+        end
+
+        it "returns navigation items with the users item set as current" do
+          expect(primary_items(users_path, current_user)).to eq(expected_navigation_items)
+        end
+      end
+
+      context "when the user is on their organisation details page" do
+        let(:expected_navigation_items) do
+          [
+            NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false),
+            NavigationItemsHelper::NavigationItem.new("Users", users_path, false),
+            NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, true),
+          ]
+        end
+
+        it "returns navigation items with the users item set as current" do
+          expect(primary_items("#{organisation_path}/details", current_user)).to eq(expected_navigation_items)
+        end
+      end
+
+      context "when the user is on the account page" do
+        let(:expected_navigation_items) do
+          [
+            NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false),
+            NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false),
+            NavigationItemsHelper::NavigationItem.new("About your organisation", organisation_path, false),
+          ]
+        end
+
+        it "returns navigation items with the users item set as current" do
+          expect(primary_items("/account", current_user)).to eq(expected_navigation_items)
+        end
+      end
+    end
+
+    context "when the user is a support user" do
+      let(:current_user) { FactoryBot.create(:user, :support) }
+
+      context "when the user is on the users page" do
+        let(:expected_navigation_items) do
+          [
+            NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
+            NavigationItemsHelper::NavigationItem.new("Users", "/users", true),
+            NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false),
+          ]
+        end
+
+        it "returns navigation items with the users item set as current" do
+          expect(primary_items("/users", current_user)).to eq(expected_navigation_items)
+        end
+      end
+
+      context "when the user is on the account page" do
+        let(:expected_navigation_items) do
+          [
+            NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", false),
+            NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
+            NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false),
+          ]
+        end
+
+        it "returns navigation items with the users item set as current" do
+          expect(primary_items("/account", current_user)).to eq(expected_navigation_items)
+        end
+      end
+    end
+  end
+end