User search fixes (#607)

* Update query message

* Add clear search link

* Set input value

* Use gem component

* Move to list partial pattern

* Partial path

* Update spec

* Rubocop

* Unit test filter module

* Rubocop

* Add search result to page title if searched

* Add missing horizontal rule

* Use form_group attributes for search input

Co-authored-by: Paul Robert Lloyd <me+git@paulrobertlloyd.com>

app/components/search_component.html.erb

@@ -1,16 +1,15 @@
 <%= form_with model: @user, url: path(current_user), method: "get", local: true do |f| %>
-<div class="app-search govuk-!-margin-bottom-4">
-  <div class="govuk-form-group app-search__form-group">
-    <label class="govuk-label govuk-!-margin-bottom-2" for="search-field">
-      <%= label %>
-    </label>
+  <div class="app-search govuk-!-margin-bottom-4">
+    <%= f.govuk_text_field :search,
+      form_group: {
+        class: "app-search__form-group",
+      },
+      label: { text: search_label },
+      type: "search",
+      value:,
+      autocomplete: "off",
+      class: "app-search__input" %>
 
-    <input class="govuk-input app-search__input" id="search-field" name="search-field" type="search" autocomplete="off">
+    <%= f.govuk_submit "Search", classes: "app-search__button" %>
   </div>
-
-  <button class="govuk-button app-search__button undefined" data-module="govuk-button">
-    Search
-  </button>
-
-</div>
 <% end %>

app/components/search_component.rb

@@ -1,9 +1,10 @@
 class SearchComponent < ViewComponent::Base
-  attr_reader :current_user, :label
+  attr_reader :current_user, :search_label, :value
 
-  def initialize(current_user:, label:)
+  def initialize(current_user:, search_label:, value: nil)
     @current_user = current_user
-    @label = label
+    @search_label = search_label
+    @value = value
     super
   end
 

app/controllers/modules/users_filter.rb

@@ -1,8 +1,7 @@
 module Modules::UsersFilter
-  def filtered_users(base_collection)
-    search_param = params["search-field"]
-    if search_param.present?
-      base_collection.search_by(search_param)
+  def filtered_users(base_collection, search_term = nil)
+    if search_term.present?
+      base_collection.search_by(search_term)
     else
       base_collection
     end.filter_by_active.includes(:organisation)

app/controllers/organisations_controller.rb

@@ -18,8 +18,8 @@ class OrganisationsController < ApplicationController
   end
 
   def users
-    @pagy, @users = pagy(filtered_users(@organisation.users))
-    @searched = params["search-field"].present?
+    @pagy, @users = pagy(filtered_users(@organisation.users, params["search"]))
+    @searched = params["search"].presence
     render "users/index"
   end
 

app/controllers/users_controller.rb

@@ -10,8 +10,8 @@ class UsersController < ApplicationController
   def index
     redirect_to users_organisation_path(current_user.organisation) unless current_user.support?
 
-    @pagy, @users = pagy(filtered_users(User.all))
-    @searched = params["search-field"].present?
+    @pagy, @users = pagy(filtered_users(User.all, params["search"]))
+    @searched = params["search"].presence
 
     respond_to do |format|
       format.html

app/views/users/_user_list.html.erb

@@ -0,0 +1,53 @@
+<%= govuk_table do |table| %>
+  <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %>
+    <span class="govuk-!-margin-right-4">
+      <% if searched.present? %>
+        <strong><%= pagy.count %></strong> <%= item_label %> found matching ‘<%= searched %>’ of <strong><%= total_user_count %></strong> total users. <%= govuk_link_to("Clear search", request.path) %>
+      <% else %>
+        <strong><%= pagy.count %></strong> total users.
+      <% end %>
+    </span>
+    <% if current_user.support? %>
+      <%= govuk_link_to "Download (CSV)", "/users.csv", type: "text/csv" %>
+    <% end %>
+  <% end %>
+  <%= table.head do |head| %>
+    <%= head.row do |row| %>
+      <% row.cell(header: true, text: "Name and email adress", html_attributes: {
+        scope: "col",
+      }) %>
+      <% row.cell(header: true, text: "Organisation and role", html_attributes: {
+        scope: "col",
+      }) %>
+      <% row.cell(header: true, text: "Last logged in", html_attributes: {
+        scope: "col",
+      }) %>
+    <% end %>
+  <% end %>
+  <% users.each do |user| %>
+    <%= table.body do |body| %>
+      <%= body.row do |row| %>
+        <% row.cell(header: true, html_attributes: {
+          scope: "row",
+        }) do %>
+          <%= simple_format(user_cell(user), {}, wrapper_tag: "span") %>
+          <% if user.is_data_protection_officer? || user.is_key_contact? %>
+            <br>
+          <% end %>
+          <%= user.is_data_protection_officer? ? govuk_tag(
+            classes: "app-tag--small",
+            colour: "turquoise",
+            text: "Data protection officer",
+          ) : "" %>
+          <%= user.is_key_contact? ? govuk_tag(
+            classes: "app-tag--small",
+            colour: "turquoise",
+            text: "Key contact",
+          ) : "" %>
+        <% end %>
+        <% row.cell(text: simple_format(org_cell(user), {}, wrapper_tag: "div")) %>
+        <% row.cell(text: user.last_sign_in_at&.to_formatted_s(:govuk_date)) %>
+      <% end %>
+    <% end %>
+  <% end %>
+<% end %>

app/views/users/index.html.erb

@@ -1,4 +1,14 @@
-<% content_for :title, "Your organisation (Users)" %>
+<% if @searched.present? %>
+  <% item_label = @pagy.count > 1 ? "users" : "user" %>
+  <% total_user_count = User.all.count %>
+  <% title = "Your organisation (#{@pagy.count} #{item_label} matching ‘#{@searched}’ of #{total_user_count} total users)" %>
+<% else %>
+  <% item_label = "" %>
+  <% total_user_count = nil %>
+  <% title = "Your organisation (Users)" %>
+<% end %>
+
+<% content_for :title, title %>
 
 <% content_for :tab_title do %>
   <%= "Users" %>
@@ -8,59 +18,9 @@
   <%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %>
 <% end %>
 
-<%= render SearchComponent.new(current_user:, label: "Search by name or email address") %>
+<%= render SearchComponent.new(current_user:, search_label: "Search by name or email address", value: @searched) %>
 
-<%= govuk_table do |table| %>
-  <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %>
-    <span class="govuk-!-margin-right-4">
-      <% if @searched %>
-        Matches <strong><%= @pagy.count %></strong> of <strong><%= User.count %></strong> total users.
-      <% else %>
-        <strong><%= @pagy.count %></strong> total users.
-      <% end %>
-    </span>
-    <% if current_user.support? %>
-      <%= govuk_link_to "Download (CSV)", "/users.csv", type: "text/csv" %>
-    <% end %>
-  <% end %>
-  <%= table.head do |head| %>
-    <%= head.row do |row| %>
-      <% row.cell(header: true, text: "Name and email adress", html_attributes: {
-        scope: "col",
-      }) %>
-      <% row.cell(header: true, text: "Organisation and role", html_attributes: {
-        scope: "col",
-      }) %>
-      <% row.cell(header: true, text: "Last logged in", html_attributes: {
-        scope: "col",
-      }) %>
-    <% end %>
-  <% end %>
-  <% @users.each do |user| %>
-    <%= table.body do |body| %>
-      <%= body.row do |row| %>
-        <% row.cell(header: true, html_attributes: {
-          scope: "row",
-        }) do %>
-          <%= simple_format(user_cell(user), {}, wrapper_tag: "span") %>
-          <% if user.is_data_protection_officer? || user.is_key_contact? %>
-            <br>
-          <% end %>
-          <%= user.is_data_protection_officer? ? govuk_tag(
-            classes: "app-tag--small",
-            colour: "turquoise",
-            text: "Data protection officer",
-          ) : "" %>
-          <%= user.is_key_contact? ? govuk_tag(
-            classes: "app-tag--small",
-            colour: "turquoise",
-            text: "Key contact",
-          ) : "" %>
-        <% end %>
-        <% row.cell(text: simple_format(org_cell(user), {}, wrapper_tag: "div")) %>
-        <% row.cell(text: user.last_sign_in_at&.to_formatted_s(:govuk_date)) %>
-      <% end %>
-    <% end %>
-  <% end %>
-<% end %>
+<hr class="govuk-section-break govuk-section-break--visible govuk-section-break--m">
+
+<%= render partial: "users/user_list", locals: { users: @users, title:, pagy: @pagy, searched: @searched, item_label:, total_user_count: } %>
 <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "users" } %>

spec/components/search_component_spec.rb

@@ -2,11 +2,12 @@ require "rails_helper"
 
 RSpec.describe SearchComponent, type: :component do
   let(:current_user) { FactoryBot.create(:user, :support) }
-  let(:label) { "Search by name or email address" }
+  let(:search_label) { "Search by name or email address" }
   let(:page) { Capybara::Node::Simple.new(rendered_component) }
+  let(:value) { nil }
 
   before do
-    render_inline(described_class.new(current_user:, label:))
+    render_inline(described_class.new(current_user:, search_label:, value:))
   end
 
   it "renders a search bar" do
@@ -14,6 +15,14 @@ RSpec.describe SearchComponent, type: :component do
   end
 
   it "renders the given label" do
-    expect(page).to have_content(label)
+    expect(page).to have_content(search_label)
+  end
+
+  context "when a search term has been entered" do
+    let(:value) { "search term" }
+
+    it "shows the search term in the input field" do
+      expect(page).to have_field("search-field", type: "search", with: value)
+    end
   end
 end

spec/controllers/modules/users_filter_spec.rb

@@ -0,0 +1,31 @@
+require "rails_helper"
+
+RSpec.describe Modules::UsersFilter do
+  describe "filtered_users" do
+    subject(:instance) { Class.new.include(described_class).new }
+
+    before do
+      FactoryBot.create_list(:user, 5)
+      FactoryBot.create(:user, name: "Joe Blogg")
+      FactoryBot.create(:user, name: "Tom Blogg", active: false)
+    end
+
+    let(:user_list) { User.all }
+
+    context "when given a search term" do
+      let(:search_term) { "Blogg" }
+
+      it "filters the collection on search term and active users" do
+        expect(instance.filtered_users(user_list, search_term).count).to eq(1)
+      end
+    end
+
+    context "when not given a search term" do
+      let(:search_term) { nil }
+
+      it "filters the collection on active users" do
+        expect(instance.filtered_users(user_list, search_term).count).to eq(6)
+      end
+    end
+  end
+end

spec/requests/users_controller_spec.rb

@@ -359,7 +359,7 @@ RSpec.describe UsersController, type: :request do
 
         it "shows a search bar" do
           follow_redirect!
-          expect(page).to have_field("search-field", type: "search")
+          expect(page).to have_field("search", type: "search")
         end
       end
 
@@ -369,7 +369,7 @@ RSpec.describe UsersController, type: :request do
         let!(:other_org_user) { FactoryBot.create(:user, name: "User 4", email: "joe@other_example.com") }
 
         before do
-          get "/organisations/#{user.organisation.id}/users?search-field=#{search_param}"
+          get "/organisations/#{user.organisation.id}/users?search=#{search_param}"
         end
 
         context "when our search string matches case" do
@@ -381,7 +381,7 @@ RSpec.describe UsersController, type: :request do
           end
 
           it "updates the table caption" do
-            expect(page).to have_content("Matches 1 of 5 total users.")
+            expect(page).to have_content("1 user found matching ‘filter’ of 5 total users.")
           end
         end
 
@@ -417,7 +417,7 @@ RSpec.describe UsersController, type: :request do
             end
 
             it "updates the table caption" do
-              expect(page).to have_content("Matches 2 of 5 total users.")
+              expect(page).to have_content("2 users found matching ‘joe’ of 5 total users.")
             end
           end
         end
@@ -822,12 +822,12 @@ RSpec.describe UsersController, type: :request do
       end
 
       it "shows a search bar" do
-        expect(page).to have_field("search-field", type: "search")
+        expect(page).to have_field("search", type: "search")
       end
 
       context "when a search parameter is passed" do
         before do
-          get "/users?search-field=#{search_param}"
+          get "/users?search=#{search_param}"
         end
 
         context "when our search term matches a name" do
@@ -842,7 +842,7 @@ RSpec.describe UsersController, type: :request do
             end
 
             it "updates the table caption" do
-              expect(page).to have_content("Matches 1 of 4 total users.")
+              expect(page).to have_content("1 user found matching ‘Danny’ of 4 total users.")
             end
           end
 
@@ -881,7 +881,7 @@ RSpec.describe UsersController, type: :request do
             end
 
             it "updates the table caption" do
-              expect(page).to have_content("Matches 2 of 4 total users.")
+              expect(page).to have_content("2 users found matching ‘joe’ of 4 total users.")
             end
           end
         end