<% items.each do |item| %>
- <% if highlighted_tab?(item, request.fullpath) %>
+ <% if 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" } %>
<% else %>
-
- <%= govuk_link_to item[:name], item[:url], class: "app-primary-navigation__link" %>
+ <%= govuk_link_to item[:text], item[:href], class: "app-primary-navigation__link" %>
<% end %>
<% end %>
diff --git a/app/components/primary_navigation_component.rb b/app/components/primary_navigation_component.rb
index 41ed73535..f9668f812 100644
--- a/app/components/primary_navigation_component.rb
+++ b/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
diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb
new file mode 100644
index 000000000..970f89993
--- /dev/null
+++ b/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
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb
index 6866095da..7a3e85d89 100644
--- a/app/views/layouts/application.html.erb
+++ b/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 %>
diff --git a/spec/components/primary_navigation_component_spec.rb b/spec/components/primary_navigation_component_spec.rb
index ef2db67de..46f45166d 100644
--- a/spec/components/primary_navigation_component_spec.rb
+++ b/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
diff --git a/spec/features/user_spec.rb b/spec/features/user_spec.rb
index d142ad872..dc4a24591 100644
--- a/spec/features/user_spec.rb
+++ b/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
diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb
new file mode 100644
index 000000000..9a060c759
--- /dev/null
+++ b/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