From d222373387599a51f6b1e6a143e6a316438b98d5 Mon Sep 17 00:00:00 2001
From: J G <7750475+moarpheus@users.noreply.github.com>
Date: Wed, 8 Jun 2022 08:29:12 +0100
Subject: [PATCH] Dry searched logic (#643)
* spec for item_label
* class for item_label
* using helper to derrive plurality
* using helper in other views
* better test name
* test for title format functionality
* code for title format
* test for title format support user and all high level titles
* renamed helper
* failing test viewing logs for specific org as support user
* code for title format with sub nav for support user
* code for title format coordinator user
* correct titles for coordinator user
* switched users to using helper
* rubocop
* correct title for support about org
* massive refacotring
* cleaner code
* correct title for support users in about section
* refactored name
* refactored test names
---
app/helpers/title_helper.rb | 20 ++++
app/views/case_logs/index.html.erb | 8 +-
app/views/organisations/index.html.erb | 8 +-
app/views/organisations/logs.html.erb | 10 +-
app/views/organisations/show.html.erb | 3 +-
app/views/organisations/users.html.erb | 12 +-
app/views/users/index.html.erb | 8 +-
spec/helpers/title_helper_spec.rb | 122 +++++++++++++++++++++
spec/requests/case_logs_controller_spec.rb | 6 +-
9 files changed, 159 insertions(+), 38 deletions(-)
create mode 100644 app/helpers/title_helper.rb
create mode 100644 spec/helpers/title_helper_spec.rb
diff --git a/app/helpers/title_helper.rb b/app/helpers/title_helper.rb
new file mode 100644
index 000000000..25dcbc30c
--- /dev/null
+++ b/app/helpers/title_helper.rb
@@ -0,0 +1,20 @@
+module TitleHelper
+ def format_label(count, item)
+ count > 1 ? item.pluralize : item
+ end
+
+ def format_title(searched, page_title, current_user, item_label, count, organisation_name)
+ if searched.present?
+ actual_title = support_sab_nav?(current_user, organisation_name) ? organisation_name : page_title
+ "#{actual_title} (#{count} #{item_label} matching ‘#{searched}’)"
+ else
+ support_sab_nav?(current_user, organisation_name) ? "#{organisation_name} (#{page_title})" : page_title
+ end
+ end
+
+private
+
+ def support_sab_nav?(current_user, organisation_name)
+ current_user.support? && organisation_name
+ end
+end
diff --git a/app/views/case_logs/index.html.erb b/app/views/case_logs/index.html.erb
index c59d4f255..82fd29188 100644
--- a/app/views/case_logs/index.html.erb
+++ b/app/views/case_logs/index.html.erb
@@ -1,9 +1,5 @@
-<% item_label = @pagy.count > 1 ? "logs" : "log" %>
-<% if @searched.present? %>
- <% title = "#{current_user.support? ? 'Logs' : current_user.organisation.name} (#{@pagy.count} #{item_label} matching ‘#{@searched}’)" %>
-<% else %>
- <% title = "Logs" %>
-<% end %>
+<% item_label = format_label(@pagy.count, "log") %>
+<% title = format_title(@searched, "Logs", current_user, item_label, @pagy.count, nil) %>
<% content_for :title, title %>
diff --git a/app/views/organisations/index.html.erb b/app/views/organisations/index.html.erb
index c0587ab51..0455c23ad 100644
--- a/app/views/organisations/index.html.erb
+++ b/app/views/organisations/index.html.erb
@@ -1,9 +1,5 @@
-<% item_label = @pagy.count > 1 ? "organisations" : "organisation" %>
-<% if @searched.present? %>
- <% title = "Organisations (#{@pagy.count} #{item_label} matching ‘#{@searched}’)" %>
-<% else %>
- <% title = "Organisations" %>
-<% end %>
+<% item_label = format_label(@pagy.count, "organisation") %>
+<% title = format_title(@searched, "Organisations", current_user, item_label, @pagy.count, nil) %>
<% content_for :title, title %>
diff --git a/app/views/organisations/logs.html.erb b/app/views/organisations/logs.html.erb
index e9230f3e7..ee476a59d 100644
--- a/app/views/organisations/logs.html.erb
+++ b/app/views/organisations/logs.html.erb
@@ -1,14 +1,10 @@
-<% item_label = @pagy.count > 1 ? "logs" : "log" %>
-<% if @searched.present? %>
- <% title = "#{@organisation.name} (#{@pagy.count} #{item_label} matching ‘#{@searched}’)" %>
-<% else %>
- <% title = "#{@organisation.name} (Logs)" %>
-<% end %>
+<% item_label = format_label(@pagy.count, "log") %>
+<% title = format_title(@searched, "Logs", current_user, item_label, @pagy.count, @organisation.name) %>
<% content_for :title, title %>
- <%= @organisation.name %>
+ <%= @organisation.name %>
<%= render SubNavigationComponent.new(
diff --git a/app/views/organisations/show.html.erb b/app/views/organisations/show.html.erb
index c148f08d4..f19e76c1e 100644
--- a/app/views/organisations/show.html.erb
+++ b/app/views/organisations/show.html.erb
@@ -1,4 +1,5 @@
<% title = current_user.support? ? "#{@organisation.name} (Organisation details)" : "Organisation details" %>
+<% title = format_title(nil, current_user.support? ? "About this organisation" : "About your organisation", current_user, nil, nil, @organisation.name) %>
<% content_for :title, title %>
@@ -6,7 +7,7 @@
<%= current_user.organisation.name %>
About your organisation
<% else %>
- <%= @organisation.name %>
+ <%= @organisation.name %>
<% end %>
diff --git a/app/views/organisations/users.html.erb b/app/views/organisations/users.html.erb
index 1776bc4cd..918331435 100644
--- a/app/views/organisations/users.html.erb
+++ b/app/views/organisations/users.html.erb
@@ -1,16 +1,10 @@
-<% item_label = @pagy.count > 1 ? "users" : "user" %>
-<% if @searched.present? %>
- <% title = "#{@organisation.name} (#{@pagy.count} #{item_label} matching ‘#{@searched}’" %>
-<% else %>
- <% title = "#{@organisation.name} (Users)" %>
-<% end %>
+<% item_label = format_label(@pagy.count, "user") %>
+<% title = format_title(@searched, "Users", current_user, item_label, @pagy.count, @organisation.name) %>
<% content_for :title, title %>
- <% if current_user.support? %>
- <%= @organisation.name %>
- <% end %>
+ <%= @organisation.name %>
<%= render SubNavigationComponent.new(
diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb
index 81e4c3fa0..b9c76e8e5 100644
--- a/app/views/users/index.html.erb
+++ b/app/views/users/index.html.erb
@@ -1,9 +1,5 @@
-<% item_label = @pagy.count > 1 ? "users" : "user" %>
-<% if @searched.present? %>
- <% title = "#{current_user.support? ? 'Users' : current_user.organisation.name} (#{@pagy.count} #{item_label} matching ‘#{@searched}’" %>
-<% else %>
- <% title = "Users" %>
-<% end %>
+<% item_label = format_label(@pagy.count, "user") %>
+<% title = format_title(@searched, "Users", current_user, item_label, @pagy.count, nil) %>
<% content_for :title, title %>
diff --git a/spec/helpers/title_helper_spec.rb b/spec/helpers/title_helper_spec.rb
new file mode 100644
index 000000000..f38fc45c5
--- /dev/null
+++ b/spec/helpers/title_helper_spec.rb
@@ -0,0 +1,122 @@
+require "rails_helper"
+
+RSpec.describe TitleHelper do
+ describe "#format_label" do
+ let(:item) { "organisation" }
+
+ it "returns singular when count is 1" do
+ expect(format_label(1, item)).to eq("organisation")
+ end
+
+ it "returns plural when count greater than 1" do
+ expect(format_label(2, item)).to eq("organisations")
+ end
+ end
+
+ describe "#format_title" do
+ let(:page_title) { "Title" }
+ let(:item_label) { "label" }
+ let(:search_item) { nil }
+ let(:count) { 1 }
+ let(:organisation_name) { nil }
+
+ context "when provider user" do
+ let(:user) { FactoryBot.create(:user) }
+
+ context "when any specific path" do
+ let(:page_title) { "Users" }
+ let(:organisation_name) { nil }
+
+ context "when search is missing" do
+ let(:expected_title) { page_title }
+
+ it "returns expected title" do
+ expect(format_title(nil, page_title, user, item_label, count, organisation_name)).to eq(expected_title)
+ end
+
+ context "when search is present" do
+ let(:search_item) { "foobar" }
+ let(:expected_title) { "#{page_title} (#{count} #{item_label} matching ‘#{search_item}’)" }
+
+ it "returns expected title" do
+ expect(format_title(search_item, page_title, user, item_label, count, organisation_name)).to eq(expected_title)
+ end
+ end
+ end
+ end
+ end
+
+ context "when coordinator user" do
+ let(:user) { FactoryBot.create(:user, :data_coordinator) }
+
+ context "when any specific path" do
+ let(:page_title) { "Users" }
+ let(:organisation_name) { nil }
+
+ context "when search is missing" do
+ let(:expected_title) { page_title }
+
+ it "returns expected title" do
+ expect(format_title(nil, page_title, user, item_label, count, organisation_name)).to eq(expected_title)
+ end
+
+ context "when search is present" do
+ let(:search_item) { "foobar" }
+ let(:expected_title) { "#{page_title} (#{count} #{item_label} matching ‘#{search_item}’)" }
+
+ it "returns expected title" do
+ expect(format_title(search_item, page_title, user, item_label, count, organisation_name)).to eq(expected_title)
+ end
+ end
+ end
+ end
+ end
+
+ context "when support user" do
+ let(:user) { FactoryBot.create(:user, :support) }
+
+ context "when no organisation is specified" do
+ let(:page_title) { "Organisations" }
+
+ context "when search is missing" do
+ let(:expected_title) { page_title }
+
+ it "returns expected title" do
+ expect(format_title(nil, page_title, user, item_label, count, organisation_name)).to eq(expected_title)
+ end
+ end
+
+ context "when search is present" do
+ let(:search_item) { "foobar" }
+ let(:expected_title) { "#{page_title} (#{count} #{item_label} matching ‘#{search_item}’)" }
+
+ it "returns expected title" do
+ expect(format_title(search_item, page_title, user, item_label, count, organisation_name)).to eq(expected_title)
+ end
+ end
+ end
+
+ context "when organisation is specified" do
+ let(:page_title) { "Organisations" }
+ let(:organisation_name) { "Some Name" }
+
+ context "when search is missing" do
+ let(:expected_title) { "#{organisation_name} (#{page_title})" }
+
+ it "returns expected title" do
+ expect(format_title(nil, page_title, user, item_label, count, organisation_name)).to eq(expected_title)
+ end
+ end
+
+ context "when search is present" do
+ let(:search_item) { "foobar" }
+ let(:expected_title) { "#{organisation_name} (#{count} #{item_label} matching ‘#{search_item}’)" }
+
+ it "returns expected title" do
+ expect(format_title(search_item, page_title, user, item_label, count, organisation_name)).to eq(expected_title)
+ end
+ end
+ end
+ end
+ end
+end
diff --git a/spec/requests/case_logs_controller_spec.rb b/spec/requests/case_logs_controller_spec.rb
index 6f3275128..9713666cb 100644
--- a/spec/requests/case_logs_controller_spec.rb
+++ b/spec/requests/case_logs_controller_spec.rb
@@ -326,7 +326,7 @@ RSpec.describe CaseLogsController, type: :request do
it "has search results in the title" do
get "/logs?search=#{log_to_search.id}", headers: headers, params: {}
- expect(page).to have_title("#{log_to_search.owning_organisation.name} (1 log matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK")
+ expect(page).to have_title("Logs (1 log matching ‘#{log_to_search.id}’) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end
it "shows case logs matching the id" do
@@ -381,12 +381,12 @@ RSpec.describe CaseLogsController, type: :request do
it "has title with pagination details for page 1" do
get "/logs?search=#{logs[0].postcode_full}", headers: headers, params: {}
- expect(page).to have_title("#{log_to_search.owning_organisation.name} (#{logs.count} logs matching ‘#{postcode}’) (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
+ expect(page).to have_title("Logs (#{logs.count} logs matching ‘#{postcode}’) (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end
it "has title with pagination details for page 2" do
get "/logs?search=#{logs[0].postcode_full}&page=2", headers: headers, params: {}
- expect(page).to have_title("#{log_to_search.owning_organisation.name} (#{logs.count} logs matching ‘#{postcode}’) (page 2 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
+ expect(page).to have_title("Logs (#{logs.count} logs matching ‘#{postcode}’) (page 2 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK")
end
end