Merge pull request #477 from communitiesuk/CLDC-1103-secondary-navigation-bar

Add navigation tab, remove the old navigation

app/components/primary_navigation_component.html.erb

@@ -0,0 +1,17 @@
+<nav class="app-primary-navigation" aria-label="primary">
+  <div class="govuk-width-container">
+    <ul class="app-primary-navigation__list">
+      <% items.each do |item| %>
+        <% if item.fetch(:current, false) || current_page?(item.fetch(:url)) || current_page?("#{item.fetch(:url)}/details") %>
+          <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" } %>
+          </li>
+        <% else %>
+          <li class="app-primary-navigation__item">
+            <%= govuk_link_to item[:name], item[:url], class: "app-primary-navigation__link" %>
+          </li>
+        <% end %>
+      <% end %>
+    </ul>
+  </div>
+</nav>

app/components/primary_navigation_component.rb

@@ -0,0 +1,8 @@
+class PrimaryNavigationComponent < ViewComponent::Base
+  attr_reader :items
+
+  def initialize(items:)
+    @items = items
+    super
+  end
+end

app/components/tab_navigation_component.html.erb

@@ -1,15 +0,0 @@
-<nav class="app-tab-navigation" aria-label="sub menu">
-  <ul class="app-tab-navigation__list">
-    <% items.each do |item| %>
-      <% if item.fetch(:current, false) || current_page?(strip_query(item.fetch(:url))) %>
-        <li class="app-tab-navigation__item app-tab-navigation__item--current">
-          <%= govuk_link_to item[:name], item[:url], class: "app-tab-navigation__link", aria: { current: "page" } %>
-        </li>
-      <% else %>
-        <li class="app-tab-navigation__item">
-          <%= govuk_link_to item[:name], item[:url], class: "app-tab-navigation__link" %>
-        </li>
-      <% end %>
-    <% end %>
-  </ul>
-</nav>

app/components/tab_navigation_component.rb

@@ -1,14 +0,0 @@
-class TabNavigationComponent < ViewComponent::Base
-  attr_reader :items
-
-  def initialize(items:)
-    @items = items
-    super
-  end
-
-  def strip_query(url)
-    url = Addressable::URI.parse(url)
-    url.query_values = nil
-    url.to_s
-  end
-end

app/controllers/organisations_controller.rb

@@ -1,8 +1,14 @@
 class OrganisationsController < ApplicationController
-  before_action :authenticate_user!
+  before_action :authenticate_user!, except: [:index]
-  before_action :find_resource
+  before_action :find_resource, except: [:index]
   before_action :authenticate_scope!
 
+  def index
+    unless current_user.support?
+      redirect_to user_path(current_user)
+    end
+  end
+
   def show
     redirect_to details_organisation_path(@organisation)
   end
@@ -41,10 +47,12 @@ private
   end
 
   def authenticate_scope!
-    render_not_found if current_user.organisation != @organisation
+    render_not_found if current_user.organisation != @organisation && !current_user.support?
   end
 
   def find_resource
+    return if current_user.support?
+
     @organisation = Organisation.find(params[:id])
   end
 end

app/controllers/users_controller.rb

@@ -5,6 +5,12 @@ class UsersController < ApplicationController
   before_action :find_resource, except: %i[new create]
   before_action :authenticate_scope!, except: %i[new]
 
+  def index
+    unless current_user.support?
+      redirect_to user_path(@user)
+    end
+  end
+
   def update
     if @user.update(user_params)
       if @user == current_user

app/frontend/styles/_header.scss

@@ -0,0 +1,23 @@
+.app-header {
+  border-bottom: govuk-spacing(2) solid $govuk-brand-colour;
+
+  .govuk-header__logo {
+    @include govuk-media-query($from: desktop) {
+      width: 60%;
+    }
+
+    @include govuk-media-query($from: 860px) {
+      width: 75%;
+    }
+  }
+
+  .govuk-header__content {
+    @include govuk-media-query($from: desktop) {
+      width: 40%;
+    }
+
+    @include govuk-media-query($from: 860px) {
+      width: 25%;
+    }
+  }
+}

app/frontend/styles/_primary-navigation.scss

@@ -0,0 +1,69 @@
+.app-primary-navigation {
+    @include govuk-font(19, $weight: bold);
+    background-color: govuk-colour("light-grey");
+    border-bottom: 1px solid $govuk-border-colour;
+  }
+  
+  .govuk-phase-banner + .app-primary-navigation {
+    margin-top: -1px;
+  }
+  
+  .app-primary-navigation__list {
+    @include govuk-clearfix;
+    left: govuk-spacing(-3);
+    list-style: none;
+    margin: 0;
+    padding: 0;
+    position: relative;
+    right: govuk-spacing(-3);
+    width: calc(100% + #{govuk-spacing(6)});
+  }
+  
+  .app-primary-navigation__item {
+    box-sizing: border-box;
+    display: block;
+    float: left;
+    line-height: 50px;
+    height: 50px;
+    padding: 0 govuk-spacing(3);
+    position: relative;
+  }
+  
+  .app-primary-navigation__item--current {
+    border-bottom: $govuk-border-width-narrow solid $govuk-link-colour;
+  
+    &:hover {
+      border-bottom-color: $govuk-link-hover-colour;
+    }
+  
+    &:active {
+      border-bottom-color: $govuk-link-active-colour;
+    }
+  }
+  
+  .app-primary-navigation__item--align-right {
+    @include govuk-media-query($from: tablet) {
+      float: right;
+    }
+  }
+  
+  .app-primary-navigation__link {
+    @include govuk-link-common;
+    @include govuk-link-style-no-visited-state;
+    @include govuk-link-style-no-underline;
+    @include govuk-typography-weight-bold;
+  
+    // Extend the touch area of the link to the list
+    &:after {
+      bottom: 0;
+      content: "";
+      left: 0;
+      position: absolute;
+      right: 0;
+      top: 0;
+    }
+  }
+  
+  .app-primary-navigation__item--current .app-primary-navigation__link:hover {
+    text-decoration: none;
+  }

app/frontend/styles/_tab-navigation.scss

@@ -1,76 +0,0 @@
-.app-tab-navigation {
-  @include govuk-font(19, $weight: bold);
-  @include govuk-responsive-margin(6, "bottom");
-}
-
-.app-tab-navigation__list {
-  @include govuk-clearfix;
-  left: govuk-spacing(-3);
-  list-style: none;
-  margin: 0;
-  padding: 0;
-  position: relative;
-  right: govuk-spacing(-3);
-  width: calc(100% + #{govuk-spacing(6)});
-
-  @include govuk-media-query($from: tablet) {
-    box-shadow: inset 0 -1px 0 $govuk-border-colour;
-  }
-}
-
-.app-tab-navigation__item {
-  box-sizing: border-box;
-  display: block;
-  line-height: 40px;
-  height: 40px;
-  padding: 0 govuk-spacing(3);
-
-  @include govuk-media-query($from: tablet) {
-    box-shadow: none;
-    display: block;
-    float: left;
-    line-height: 50px;
-    height: 50px;
-    padding: 0 govuk-spacing(3);
-    position: relative;
-  }
-}
-
-.app-tab-navigation__item--current {
-  @include govuk-media-query($until: tablet) {
-    border-left: 4px solid $govuk-link-colour;
-    padding-left: 11px;
-  }
-
-  @include govuk-media-query($from: tablet) {
-    border-bottom: 4px solid $govuk-link-colour;
-    padding-left: govuk-spacing(3);
-  }
-}
-
-.app-tab-navigation__link {
-  @include govuk-link-common;
-  @include govuk-link-style-no-visited-state;
-  @include govuk-link-style-no-underline;
-  @include govuk-typography-weight-bold;
-
-  &:not(:focus):hover {
-    color: $govuk-link-colour;
-  }
-
-  // Extend the touch area of the link to the list
-  &:after {
-    bottom: 0;
-    content: "";
-    left: 0;
-    position: absolute;
-    right: 0;
-    top: 0;
-  }
-}
-
-.app-tab-navigation__item--current .app-tab-navigation__link {
-  &:hover {
-    text-decoration: none;
-  }
-}

app/frontend/styles/application.scss

@@ -20,15 +20,16 @@ $govuk-new-link-styles: true;
 @import "figure";
 @import "filter";
 @import "filter-layout";
+@import "header";
 @import "input";
 @import "related-navigation";
 @import "section-skip-link";
-@import "tab-navigation";
 @import "table-group";
 @import "task-list";
 @import "template";
 @import "pagination";
 @import "panel";
+@import "primary-navigation";
 
 // App utilities
 .app-\!-colour-muted {

app/views/layouts/application.html.erb

@@ -43,29 +43,45 @@
     <%= govuk_skip_link %>
 
     <%= govuk_header(
-      logotype: "GOV.UK",
+      classes: "app-header",
-      service_name: t("service_name"),
       service_url: current_user.nil? ? "/" : "/logs",
+      navigation_classes: "govuk-header__navigation--end"
     ) do |component|
+      component.product_name(name: t("service_name"))
       if current_user.nil?
         component.navigation_item(text: "Sign in", href: user_session_path)
       else
-        component.navigation_item(text: "Logs", href: case_logs_path)
-        component.navigation_item(text: "Your organisation", href: "/organisations/#{current_user.organisation.id}")
         component.navigation_item(text: "Your account", href: account_path)
         component.navigation_item(text: "Sign out", href: destroy_user_session_path)
       end
     end %>
 
+    <%= govuk_phase_banner(
+      classes: "govuk-width-container",
+      tag: { text: "Beta" },
+      text: "This is a new service – help us improve it by <a class=\"govuk-link\" href=\"#{t('feedback_form')}\" rel=\"noreferrer noopener\" target=\"_blank\">giving us your feedback (opens in a new tab)</a>".html_safe,
+    ) %>
+
+    <% if !current_user.nil? %>
+      <% if current_user.support? %>
+        <% items = [
+          { name: "Organisations", url: "/organisations" },
+          { name: "Users", url: "/users" },
+          { name: "Logs", url: case_logs_path },
+        ] %>
+      <% else %>
+        <% items = [
+          { name: "Logs", url: case_logs_path },
+          { name: "Users", url: users_organisation_path(current_user.organisation) },
+          { name: "About your organisation", url: "/organisations/#{current_user.organisation.id}" },
+        ] %>
+      <% end %>
+      <%= render PrimaryNavigationComponent.new(items:) %>
+    <% end %>
+
     <div class="govuk-width-container">
-      <aside>
+      <%= content_for(:breadcrumbs) %>
-        <%= govuk_phase_banner(
+      <%= content_for(:before_content) %>
-          tag: { text: "Beta" },
-          text: "This is a new service – help us improve it by <a class=\"govuk-link\" href=\"#{t('feedback_form')}\" rel=\"noreferrer noopener\" target=\"_blank\">giving us your feedback (opens in a new tab)</a>".html_safe,
-        ) %>
-        <%= content_for(:breadcrumbs) %>
-        <%= content_for(:before_content) %>
-      </aside>
 
       <main class="govuk-main-wrapper" id="main-content" role="main">
         <% if flash.notice && !flash.notice.include?("translation missing") %>

app/views/layouts/organisations.html.erb

@@ -5,8 +5,6 @@
 
   <% items = tab_items(current_user) %>
 
-  <%= render TabNavigationComponent.new(items:) %>
-
   <h2 class="govuk-visually-hidden"><%= content_for(:tab_title) %></h2>
 
   <%= content_for?(:organisations_content) ? yield(:organisations_content) : yield %>

db/schema.rb

@@ -305,12 +305,12 @@ ActiveRecord::Schema[7.0].define(version: 2022_04_11_092231) do
     t.string "last_sign_in_ip"
     t.integer "role"
     t.string "old_user_id"
+    t.string "phone"
     t.integer "failed_attempts", default: 0
     t.string "unlock_token"
     t.datetime "locked_at", precision: nil
     t.boolean "is_dpo", default: false
     t.boolean "is_key_contact", default: false
-    t.string "phone"
     t.integer "second_factor_attempts_count", default: 0
     t.string "encrypted_otp_secret_key"
     t.string "encrypted_otp_secret_key_iv"

spec/components/primary_navigation_component_spec.rb

@@ -0,0 +1,27 @@
+require "rails_helper"
+
+RSpec.describe PrimaryNavigationComponent, type: :component do
+  let(:items) do
+    [{ name: "Organisations", url: "#", current: true },
+     { name: "Users", url: "#" },
+     { name: "Logs ", url: "#" }]
+  end
+
+  context "when the item is 'current' in nav tabs" do
+    it "then that tab appears as selected" do
+      result = render_inline(described_class.new(items:))
+
+      expect(result.css('.app-primary-navigation__link[aria-current="page"]').text).to include("Organisations")
+    end
+  end
+
+  context "when rendering tabs" do
+    it "all of the nav tabs specified in the items hash are passed to it" do
+      result = render_inline(described_class.new(items:))
+
+      expect(result.text).to include("Organisations")
+      expect(result.text).to include("Users")
+      expect(result.text).to include("Logs")
+    end
+  end
+end

spec/components/tab_navigation_component_spec.rb

@@ -1,27 +0,0 @@
-require "rails_helper"
-
-RSpec.describe TabNavigationComponent, type: :component do
-  let(:items) do
-    [{ name: "Application", url: "#", current: true },
-     { name: "Notes", url: "#" },
-     { name: "Timeline", url: "#" }]
-  end
-
-  context "when the item is 'current' in nav tabs" do
-    it "then that tab appears as selected" do
-      result = render_inline(described_class.new(items:))
-
-      expect(result.css('.app-tab-navigation__link[aria-current="page"]').text).to include("Application")
-    end
-  end
-
-  context "when rendering tabs" do
-    it "all of the nav tabs specified in the items hash are passed to it" do
-      result = render_inline(described_class.new(items:))
-
-      expect(result.text).to include("Application")
-      expect(result.text).to include("Notes")
-      expect(result.text).to include("Timeline")
-    end
-  end
-end

spec/features/organisation_spec.rb

@@ -24,7 +24,7 @@ RSpec.describe "User Features" do
     context "when viewing organisation page" do
       it "defaults to organisation details" do
         visit("/logs")
-        click_link("Your organisation")
+        click_link("About your organisation")
         expect(page).to have_content(user.organisation.name)
       end
 
@@ -32,7 +32,7 @@ RSpec.describe "User Features" do
         visit("/organisations/#{org_id}")
         click_link("Users")
         expect(page).to have_current_path("/organisations/#{org_id}/users")
-        click_link("Details")
+        click_link("About your organisation")
         expect(page).to have_current_path("/organisations/#{org_id}/details")
       end
     end
@@ -72,10 +72,11 @@ RSpec.describe "User Features" do
     context "when viewing organisation page" do
       it "can see the details tab and users tab" do
         visit("/logs")
-        click_link("Your organisation")
+        click_link("About your organisation")
         expect(page).to have_current_path("/organisations/#{org_id}/details")
-        expect(page).to have_link("Details")
+        expect(page).to have_link("Logs")
         expect(page).to have_link("Users")
+        expect(page).to have_link("About your organisation")
       end
     end
   end

spec/requests/organisations_controller_spec.rb

@@ -66,7 +66,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
 
           it "shows the tab navigation" do
-            expected_html = "<nav class=\"app-tab-navigation\""
+            expected_html = "<nav class=\"app-primary-navigation\""
             expect(response.body).to include(expected_html)
           end
 
@@ -107,7 +107,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
 
           it "shows the tab navigation" do
-            expected_html = "<nav class=\"app-tab-navigation\""
+            expected_html = "<nav class=\"app-primary-navigation\""
             expect(response.body).to include(expected_html)
           end
 
@@ -218,7 +218,7 @@ RSpec.describe OrganisationsController, type: :request do
           end
 
           it "shows the tab navigation" do
-            expected_html = "<nav class=\"app-tab-navigation\""
+            expected_html = "<nav class=\"app-primary-navigation\""
             expect(response.body).to include(expected_html)
           end
 

spec/requests/users_controller_spec.rb

@@ -103,8 +103,11 @@ RSpec.describe UsersController, type: :request do
 
     describe "title link" do
       it "routes user to the /logs page" do
+        sign_in user
         get "/", headers:, params: {}
-        expected_link = "href=\"/\">#{I18n.t('service_name')}</a>"
+        follow_redirect!
+        expect(path).to include("/logs")
+        expected_link = "<a class=\"govuk-header__link govuk-header__link--homepage\" href=\"/\">"
         expect(CGI.unescape_html(response.body)).to include(expected_link)
       end
     end
@@ -1110,8 +1113,9 @@ RSpec.describe UsersController, type: :request do
 
     it "routes user to the /logs page" do
       get "/", headers:, params: {}
-      expected_link = "href=\"/logs\">#{I18n.t('service_name')}</a>"
       follow_redirect!
+      expect(path).to include("/logs")
+      expected_link = "<a class=\"govuk-header__link govuk-header__link--homepage\" href=\"/\">"
       expect(CGI.unescape_html(response.body)).to include(expected_link)
     end
   end