From 24bfdd85c49a192f5ed578e4d29106d38fa15e41 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Tue, 24 May 2022 10:10:32 +0100 Subject: [PATCH] CLDC-1124: User search (#600) * Add search by name for users Co-authored-by: baarkerlounger * Search is now non case sensitive * Made search work for data co-ordinators Co-authored-by: baarkerlounger * Refactored to scope * Added search by email Co-authored-by: baarkerlounger * WIP Commit - added test for if search term matches a name and an email address simultaneously. Also changed search result caption for organisations to display "Matches X of Y users" * Rubocop * Preload org * Linting * Refactor filtered_users into module * Only adjust query param if searched * Add data coordinator tests * Add table caption spec * Dupe attribute * Refactor into Search ViewComponent * Rubocop * Unit test user scopes Co-authored-by: Ted Co-authored-by: baarkerlounger --- app/components/search_component.html.erb | 16 ++ app/components/search_component.rb | 13 ++ app/controllers/modules/users_filter.rb | 10 ++ app/controllers/organisations_controller.rb | 4 +- app/controllers/users_controller.rb | 4 +- app/frontend/styles/_search.scss | 24 +++ app/frontend/styles/application.scss | 1 + app/models/user.rb | 5 + app/views/users/index.html.erb | 9 +- spec/components/search_component_spec.rb | 19 +++ spec/models/user_spec.rb | 29 ++++ spec/requests/users_controller_spec.rb | 160 ++++++++++++++++++-- 12 files changed, 279 insertions(+), 15 deletions(-) create mode 100644 app/components/search_component.html.erb create mode 100644 app/components/search_component.rb create mode 100644 app/controllers/modules/users_filter.rb create mode 100644 app/frontend/styles/_search.scss create mode 100644 spec/components/search_component_spec.rb diff --git a/app/components/search_component.html.erb b/app/components/search_component.html.erb new file mode 100644 index 000000000..cf5da1898 --- /dev/null +++ b/app/components/search_component.html.erb @@ -0,0 +1,16 @@ +<%= form_with model: @user, url: path(current_user), method: "get", local: true do |f| %> +
+
+ + + +
+ + + +
+<% end %> diff --git a/app/components/search_component.rb b/app/components/search_component.rb new file mode 100644 index 000000000..dbab17245 --- /dev/null +++ b/app/components/search_component.rb @@ -0,0 +1,13 @@ +class SearchComponent < ViewComponent::Base + attr_reader :current_user, :label + + def initialize(current_user:, label:) + @current_user = current_user + @label = label + super + end + + def path(current_user) + current_user.support? ? users_path : users_organisation_path(current_user.organisation) + end +end diff --git a/app/controllers/modules/users_filter.rb b/app/controllers/modules/users_filter.rb new file mode 100644 index 000000000..1b6ba4c53 --- /dev/null +++ b/app/controllers/modules/users_filter.rb @@ -0,0 +1,10 @@ +module Modules::UsersFilter + def filtered_users(base_collection) + search_param = params["search-field"] + if search_param.present? + base_collection.search_by(search_param) + else + base_collection + end.filter_by_active.includes(:organisation) + end +end diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 8468a3f9f..17d3b8a3f 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -1,6 +1,7 @@ class OrganisationsController < ApplicationController include Pagy::Backend include Modules::CaseLogsFilter + include Modules::UsersFilter before_action :authenticate_user!, except: [:index] before_action :find_resource, except: [:index] @@ -17,7 +18,8 @@ class OrganisationsController < ApplicationController end def users - @pagy, @users = pagy(@organisation.users.where(active: true)) + @pagy, @users = pagy(filtered_users(@organisation.users)) + @searched = params["search-field"].present? render "users/index" end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 1bd7c4c7e..8e58f7396 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,6 +2,7 @@ class UsersController < ApplicationController include Pagy::Backend include Devise::Controllers::SignInOut include Helpers::Email + include Modules::UsersFilter before_action :authenticate_user! before_action :find_resource, except: %i[new create] before_action :authenticate_scope!, except: %i[new] @@ -9,7 +10,8 @@ class UsersController < ApplicationController def index redirect_to users_organisation_path(current_user.organisation) unless current_user.support? - @pagy, @users = pagy(User.all.where(active: true).includes(:organisation)) + @pagy, @users = pagy(filtered_users(User.all)) + @searched = params["search-field"].present? respond_to do |format| format.html diff --git a/app/frontend/styles/_search.scss b/app/frontend/styles/_search.scss new file mode 100644 index 000000000..91399479a --- /dev/null +++ b/app/frontend/styles/_search.scss @@ -0,0 +1,24 @@ +.app-search { + @include govuk-responsive-margin(6, "bottom"); + display: flex; + align-items: end; +} + +.app-search__form-group { + flex: 1; + margin-bottom: 0; +} + +.app-search__input { + background-image: url("data:image/svg+xml,%3Csvg class='app-search__icon' width='20' height='20' viewBox='0 0 27 27' fill='none' xmlns='http://www.w3.org/2000/svg' aria-hidden='true' focusable='false'%3E%3Ccircle cx='12.0161' cy='11.0161' r='8.51613' stroke='currentColor' stroke-width='3'%3E%3C/circle%3E%3Cline x1='17.8668' y1='17.3587' x2='26.4475' y2='25.9393' stroke='currentColor' stroke-width='3'%3E%3C/line%3E%3C/svg%3E"); + background-repeat: no-repeat; + background-size: 1em 1em; + background-position: 7px center; + padding-left: govuk-spacing(6); +} + +.app-search__button { + margin-left: govuk-spacing(1); + margin-bottom: 2px; + width: auto; +} diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss index ebe0af05f..883ea5e63 100644 --- a/app/frontend/styles/application.scss +++ b/app/frontend/styles/application.scss @@ -39,6 +39,7 @@ $govuk-breakpoints: ( @import "pagination"; @import "panel"; @import "primary-navigation"; +@import "search"; // App utilities .app-\!-colour-muted { diff --git a/app/models/user.rb b/app/models/user.rb index f32096ad6..97cc43c62 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -32,6 +32,11 @@ class User < ApplicationRecord enum role: ROLES + scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } + scope :search_by_email, ->(email) { where("email ILIKE ?", "%#{email}%") } + scope :filter_by_active, -> { where(active: true) } + scope :search_by, ->(param) { search_by_name(param).or(search_by_email(param)) } + def case_logs if support? CaseLog.all diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index 87e5b37a0..44f401515 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -7,10 +7,17 @@ <% if current_user.data_coordinator? || current_user.support? %> <%= 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") %> + <%= govuk_table do |table| %> <%= table.caption(size: "s", classes: %w[govuk-!-text-align-left govuk-!-margin-top-4 govuk-!-margin-bottom-4]) do |caption| %> - <%= @pagy.count %> total users + <% if @searched %> + Matches <%= @pagy.count %> of <%= User.count %> total users + <% else %> + <%= @pagy.count %> total users + <% end %> <% if current_user.support? %> <%= govuk_link_to "Download (CSV)", "/users.csv", type: "text/csv" %> diff --git a/spec/components/search_component_spec.rb b/spec/components/search_component_spec.rb new file mode 100644 index 000000000..d79ad53ca --- /dev/null +++ b/spec/components/search_component_spec.rb @@ -0,0 +1,19 @@ +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(:page) { Capybara::Node::Simple.new(rendered_component) } + + before do + render_inline(described_class.new(current_user:, label:)) + end + + it "renders a search bar" do + expect(page).to have_field("search-field", type: "search") + end + + it "renders the given label" do + expect(page).to have_content(label) + end +end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 8863f3bc4..df3f82911 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -185,4 +185,33 @@ RSpec.describe User, type: :model do }.not_to change(user.versions, :count) end end + + describe "scopes" do + before do + FactoryBot.create(:user, name: "Joe Bloggs", email: "joe@example.com") + FactoryBot.create(:user, name: "Tom Smith", email: "tom@example.com") + FactoryBot.create(:user, name: "Jenny Ford", email: "jenny@smith.com") + end + + context "when searching by name" do + it "returns case insensitive matching records" do + expect(described_class.search_by_name("Joe").count).to eq(1) + expect(described_class.search_by_name("joe").count).to eq(1) + end + end + + context "when searching by email" do + it "returns case insensitive matching records" do + expect(described_class.search_by_email("Example").count).to eq(2) + expect(described_class.search_by_email("example").count).to eq(2) + end + end + + context "when searching by all searchable field" do + it "returns case insensitive matching records" do + expect(described_class.search_by("Smith").count).to eq(2) + expect(described_class.search_by("smith").count).to eq(2) + end + end + end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 679cc02c6..beb81a993 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -335,22 +335,92 @@ RSpec.describe UsersController, type: :request do end context "when user is signed in as a data coordinator" do - let(:user) { FactoryBot.create(:user, :data_coordinator) } - let(:other_user) { FactoryBot.create(:user, organisation: user.organisation) } + let(:user) { FactoryBot.create(:user, :data_coordinator, email: "coordinator@example.com") } + let!(:other_user) { FactoryBot.create(:user, organisation: user.organisation, name: "filter name", email: "filter@example.com") } describe "#index" do before do sign_in user - get "/users", headers:, params: {} end - it "redirects to the organisation user path" do - follow_redirect! - expect(path).to match("/organisations/#{user.organisation.id}/users") + context "when there are no url params" do + before do + get "/users", headers:, params: {} + end + + it "redirects to the organisation user path" do + follow_redirect! + expect(path).to match("/organisations/#{user.organisation.id}/users") + end + + it "does not show the download csv link" do + expect(page).not_to have_link("Download (CSV)", href: "/users.csv") + end + + it "shows a search bar" do + follow_redirect! + expect(page).to have_field("search-field", type: "search") + end end - it "does not show the download csv link" do - expect(page).not_to have_link("Download (CSV)", href: "/users.csv") + context "when a search parameter is passed" do + let!(:other_user_2) { FactoryBot.create(:user, organisation: user.organisation, name: "joe", email: "other@example.com") } + let!(:other_user_3) { FactoryBot.create(:user, name: "User 5", organisation: user.organisation, email: "joe@example.com") } + 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}" + end + + context "when our search string matches case" do + let(:search_param) { "filter" } + + it "returns only matching results" do + expect(page).not_to have_content(user.name) + expect(page).to have_content(other_user.name) + end + + it "updates the table caption" do + expect(page).to have_content("Matches 1 of 5 total users") + end + end + + context "when we need case insensitive search" do + let(:search_param) { "Filter" } + + it "returns only matching results" do + expect(page).not_to have_content(user.name) + expect(page).to have_content(other_user.name) + end + end + + context "when our search term matches an email" do + let(:search_param) { "other@example.com" } + + it "returns only matching result within the same organisation" do + expect(page).not_to have_content(user.name) + expect(page).to have_content(other_user_2.name) + expect(page).not_to have_content(other_user.name) + expect(page).not_to have_content(other_user_3.name) + expect(page).not_to have_content(other_org_user.name) + end + + context "when our search term matches an email and a name" do + let(:search_param) { "joe" } + + it "returns any results including joe within the same organisation" do + expect(page).to have_content(other_user_2.name) + expect(page).to have_content(other_user_3.name) + expect(page).not_to have_content(other_user.name) + expect(page).not_to have_content(other_org_user.name) + expect(page).not_to have_content(user.name) + end + + it "updates the table caption" do + expect(page).to have_content("Matches 2 of 5 total users") + end + end + end end end @@ -613,7 +683,7 @@ RSpec.describe UsersController, type: :request do it "does update other values" do expect { patch "/users/#{other_user.id}", headers:, params: } - .to change { other_user.reload.name }.from("Danny Rojas").to("new name") + .to change { other_user.reload.name }.from("filter name").to("new name") end end end @@ -727,9 +797,9 @@ RSpec.describe UsersController, type: :request do end describe "#index" do - let!(:other_user) { FactoryBot.create(:user, organisation: user.organisation, name: "User 2") } - let!(:inactive_user) { FactoryBot.create(:user, organisation: user.organisation, active: false, name: "User 3") } - let!(:other_org_user) { FactoryBot.create(:user, name: "User 4") } + let!(:other_user) { FactoryBot.create(:user, organisation: user.organisation, name: "User 2", email: "other@example.com") } + let!(:inactive_user) { FactoryBot.create(:user, organisation: user.organisation, active: false, name: "User 3", email: "inactive@example.com") } + let!(:other_org_user) { FactoryBot.create(:user, name: "User 4", email: "other_org@other_example.com") } before do sign_in user @@ -750,6 +820,72 @@ RSpec.describe UsersController, type: :request do it "shows the download csv link" do expect(page).to have_link("Download (CSV)", href: "/users.csv") end + + it "shows a search bar" do + expect(page).to have_field("search-field", type: "search") + end + + context "when a search parameter is passed" do + before do + get "/users?search-field=#{search_param}" + end + + context "when our search term matches a name" do + context "when our search string matches case" do + let(:search_param) { "Danny" } + + it "returns only matching results" do + expect(page).to have_content(user.name) + expect(page).not_to have_content(other_user.name) + expect(page).not_to have_content(inactive_user.name) + expect(page).not_to have_content(other_org_user.name) + end + + it "updates the table caption" do + expect(page).to have_content("Matches 1 of 4 total users") + end + end + + context "when we need case insensitive search" do + let(:search_param) { "danny" } + + it "returns only matching results" do + expect(page).to have_content(user.name) + expect(page).not_to have_content(other_user.name) + expect(page).not_to have_content(inactive_user.name) + expect(page).not_to have_content(other_org_user.name) + end + end + + context "when our search term matches an email" do + let(:search_param) { "other_org@other_example.com" } + + it "returns only matching result" do + expect(page).not_to have_content(user.name) + expect(page).not_to have_content(other_user.name) + expect(page).not_to have_content(inactive_user.name) + expect(page).to have_content(other_org_user.name) + end + end + + context "when our search term matches an email and a name" do + let!(:other_user) { FactoryBot.create(:user, organisation: user.organisation, name: "joe", email: "other@example.com") } + let!(:other_org_user) { FactoryBot.create(:user, name: "User 4", email: "joe@other_example.com") } + let(:search_param) { "joe" } + + it "returns any results including joe" do + expect(page).to have_content(other_user.name) + expect(page).not_to have_content(inactive_user.name) + expect(page).to have_content(other_org_user.name) + expect(page).not_to have_content(user.name) + end + + it "updates the table caption" do + expect(page).to have_content("Matches 2 of 4 total users") + end + end + end + end end describe "CSV download" do