From d8fa4df51584d40aa3381906f1dc1e1e7a465a45 Mon Sep 17 00:00:00 2001 From: baarkerlounger <5101747+baarkerlounger@users.noreply.github.com> Date: Wed, 25 May 2022 12:20:06 +0100 Subject: [PATCH] Organisation search (#610) * Add search to organisations * Fix title * Spec page title * Don't seed org in test --- app/components/search_component.rb | 10 +++ app/controllers/modules/search_filter.rb | 13 +++ app/controllers/modules/users_filter.rb | 9 -- app/controllers/organisations_controller.rb | 16 +++- app/controllers/users_controller.rb | 12 ++- app/models/organisation.rb | 3 + .../organisations/_organisation_list.html.erb | 8 +- app/views/organisations/index.html.erb | 15 +++- app/views/users/_user_list.html.erb | 2 +- app/views/users/index.html.erb | 9 +- db/seeds.rb | 90 ++++++++++--------- spec/components/search_component_spec.rb | 1 + .../controllers/modules/search_filter_spec.rb | 56 ++++++++++++ spec/controllers/modules/users_filter_spec.rb | 31 ------- spec/models/organisation_spec.rb | 21 +++++ .../requests/organisations_controller_spec.rb | 52 +++++++++++ spec/requests/users_controller_spec.rb | 4 +- 17 files changed, 248 insertions(+), 104 deletions(-) create mode 100644 app/controllers/modules/search_filter.rb delete mode 100644 app/controllers/modules/users_filter.rb create mode 100644 spec/controllers/modules/search_filter_spec.rb delete mode 100644 spec/controllers/modules/users_filter_spec.rb diff --git a/app/components/search_component.rb b/app/components/search_component.rb index aacc902e1..35eeefda7 100644 --- a/app/components/search_component.rb +++ b/app/components/search_component.rb @@ -9,6 +9,16 @@ class SearchComponent < ViewComponent::Base end def path(current_user) + if request.path.include?("users") + user_path(current_user) + elsif request.path.include?("organisations") + organisations_path + end + end + +private + + def user_path(current_user) current_user.support? ? users_path : users_organisation_path(current_user.organisation) end end diff --git a/app/controllers/modules/search_filter.rb b/app/controllers/modules/search_filter.rb new file mode 100644 index 000000000..09c387ee4 --- /dev/null +++ b/app/controllers/modules/search_filter.rb @@ -0,0 +1,13 @@ +module Modules::SearchFilter + def filtered_collection(base_collection, search_term = nil) + if search_term.present? + base_collection.search_by(search_term) + else + base_collection + end + end + + def filtered_users(base_collection, search_term = nil) + filtered_collection(base_collection, search_term).filter_by_active.includes(:organisation) + end +end diff --git a/app/controllers/modules/users_filter.rb b/app/controllers/modules/users_filter.rb deleted file mode 100644 index 1426f24fc..000000000 --- a/app/controllers/modules/users_filter.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Modules::UsersFilter - 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) - end -end diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index 4781da5e6..7b5eceac4 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -1,7 +1,7 @@ class OrganisationsController < ApplicationController include Pagy::Backend include Modules::CaseLogsFilter - include Modules::UsersFilter + include Modules::SearchFilter before_action :authenticate_user!, except: [:index] before_action :find_resource, except: [:index] @@ -10,7 +10,10 @@ class OrganisationsController < ApplicationController def index redirect_to organisation_path(current_user.organisation) unless current_user.support? - @pagy, @organisations = pagy(Organisation.all) + all_organisations = Organisation.all + @pagy, @organisations = pagy(filtered_collection(all_organisations, search_term)) + @searched = search_term.presence + @total_count = all_organisations.size end def show @@ -18,8 +21,9 @@ class OrganisationsController < ApplicationController end def users - @pagy, @users = pagy(filtered_users(@organisation.users, params["search"])) - @searched = params["search"].presence + @pagy, @users = pagy(filtered_users(@organisation.users, search_term)) + @searched = search_term.presence + @total_count = @organisation.users.size render "users/index" end @@ -64,6 +68,10 @@ private params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone) end + def search_term + params["search"] + end + def authenticate_scope! render_not_found if current_user.organisation != @organisation && !current_user.support? end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 7b2873461..bcfacd1a9 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -2,7 +2,7 @@ class UsersController < ApplicationController include Pagy::Backend include Devise::Controllers::SignInOut include Helpers::Email - include Modules::UsersFilter + include Modules::SearchFilter before_action :authenticate_user! before_action :find_resource, except: %i[new create] before_action :authenticate_scope!, except: %i[new] @@ -10,8 +10,10 @@ class UsersController < ApplicationController def index redirect_to users_organisation_path(current_user.organisation) unless current_user.support? - @pagy, @users = pagy(filtered_users(User.all, params["search"])) - @searched = params["search"].presence + all_users = User.all + @pagy, @users = pagy(filtered_users(all_users, search_term)) + @searched = search_term.presence + @total_count = all_users.size respond_to do |format| format.html @@ -91,6 +93,10 @@ private [attribute.to_s.humanize.capitalize, message].join(" ") end + def search_term + params["search"] + end + def password_params { password: SecureRandom.hex(8) } end diff --git a/app/models/organisation.rb b/app/models/organisation.rb index c52c3d922..6d22c303a 100644 --- a/app/models/organisation.rb +++ b/app/models/organisation.rb @@ -6,6 +6,9 @@ class Organisation < ApplicationRecord has_many :organisation_las has_many :organisation_rent_periods + scope :search_by_name, ->(name) { where("name ILIKE ?", "%#{name}%") } + scope :search_by, ->(param) { search_by_name(param) } + has_paper_trail PROVIDER_TYPE = { diff --git a/app/views/organisations/_organisation_list.html.erb b/app/views/organisations/_organisation_list.html.erb index a027718fa..591cedba7 100644 --- a/app/views/organisations/_organisation_list.html.erb +++ b/app/views/organisations/_organisation_list.html.erb @@ -1,8 +1,10 @@ -<% content_for :title, title %> - <%= govuk_table do |table| %> <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> - <%= @pagy.count %> total <%= title.downcase %>. + <% if searched.present? %> + <%= pagy.count %> <%= item_label %> found matching ‘<%= searched %>’ of <%= total_count %> total organisations. <%= govuk_link_to("Clear search", request.path) %> + <% else %> + <%= pagy.count %> total organisations. + <% end %> <% end %> <%= table.head do |head| %> <%= head.row do |row| %> diff --git a/app/views/organisations/index.html.erb b/app/views/organisations/index.html.erb index 3fe0ab053..b8560f072 100644 --- a/app/views/organisations/index.html.erb +++ b/app/views/organisations/index.html.erb @@ -1,2 +1,15 @@ -<%= render partial: "organisation_list", locals: { organisations: @organisations, title: "Organisations", pagy: @pagy } %> +<% item_label = @pagy.count > 1 ? "organisations" : "organisation" %> +<% if @searched.present? %> + <% title = "Organisations (#{@pagy.count} #{item_label} matching ‘#{@searched}’ of #{@total_count} total organisations)" %> +<% else %> + <% title = "Organisations" %> +<% end %> + +<% content_for :title, title %> + +<%= render SearchComponent.new(current_user:, search_label: "Search by organisation name", value: @searched) %> + +
+ +<%= render partial: "organisation_list", locals: { organisations: @organisations, title: "Organisations", pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "organisations" } %> diff --git a/app/views/users/_user_list.html.erb b/app/views/users/_user_list.html.erb index 6c0cc988a..1819fb0ac 100644 --- a/app/views/users/_user_list.html.erb +++ b/app/views/users/_user_list.html.erb @@ -2,7 +2,7 @@ <%= table.caption(classes: %w[govuk-!-font-size-19 govuk-!-font-weight-regular]) do |caption| %> <% if searched.present? %> - <%= pagy.count %> <%= item_label %> found matching ‘<%= searched %>’ of <%= total_user_count %> total users. <%= govuk_link_to("Clear search", request.path) %> + <%= pagy.count %> <%= item_label %> found matching ‘<%= searched %>’ of <%= total_count %> total users. <%= govuk_link_to("Clear search", request.path) %> <% else %> <%= pagy.count %> total users. <% end %> diff --git a/app/views/users/index.html.erb b/app/views/users/index.html.erb index ceeb563b9..1569a8097 100644 --- a/app/views/users/index.html.erb +++ b/app/views/users/index.html.erb @@ -1,10 +1,7 @@ +<% item_label = @pagy.count > 1 ? "users" : "user" %> <% 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)" %> + <% title = "Your organisation (#{@pagy.count} #{item_label} matching ‘#{@searched}’ of #{@total_count} total users)" %> <% else %> - <% item_label = "" %> - <% total_user_count = nil %> <% title = "Your organisation (Users)" %> <% end %> @@ -22,5 +19,5 @@
-<%= render partial: "users/user_list", locals: { users: @users, title:, pagy: @pagy, searched: @searched, item_label:, total_user_count: } %> +<%= render partial: "users/user_list", locals: { users: @users, title:, pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %> <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "users" } %> diff --git a/db/seeds.rb b/db/seeds.rb index 79f65f84c..54ffb843a 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -7,56 +7,58 @@ # Character.create(name: 'Luke', movie: movies.first) # rubocop:disable Rails/Output -org = Organisation.find_or_create_by!( - name: "DLUHC", - address_line1: "2 Marsham Street", - address_line2: "London", - postcode: "SW1P 4DF", - holds_own_stock: false, - other_stock_owners: "None", - managing_agents: "None", - provider_type: "LA", -) do - info = "Seeded DLUHC Organisation" - if Rails.env.development? - pp info - else - Rails.logger.info info +unless Rails.env.test? + org = Organisation.find_or_create_by!( + name: "DLUHC", + address_line1: "2 Marsham Street", + address_line2: "London", + postcode: "SW1P 4DF", + holds_own_stock: false, + other_stock_owners: "None", + managing_agents: "None", + provider_type: "LA", + ) do + info = "Seeded DLUHC Organisation" + if Rails.env.development? + pp info + else + Rails.logger.info info + end end -end -if Rails.env.development? && User.count.zero? - User.create!( - email: "provider@example.com", - password: "password", - organisation: org, - role: "data_provider", - confirmed_at: Time.zone.now, - ) + if Rails.env.development? && User.count.zero? + User.create!( + email: "provider@example.com", + password: "password", + organisation: org, + role: "data_provider", + confirmed_at: Time.zone.now, + ) - User.create!( - email: "coordinator@example.com", - password: "password", - organisation: org, - role: "data_coordinator", - confirmed_at: Time.zone.now, - ) + User.create!( + email: "coordinator@example.com", + password: "password", + organisation: org, + role: "data_coordinator", + confirmed_at: Time.zone.now, + ) - User.create!( - email: "support@example.com", - password: "password", - organisation: org, - role: "support", - confirmed_at: Time.zone.now, - ) + User.create!( + email: "support@example.com", + password: "password", + organisation: org, + role: "support", + confirmed_at: Time.zone.now, + ) - pp "Seeded 3 dummy users" -end + pp "Seeded 3 dummy users" + end -if LaRentRange.count.zero? && !Rails.env.test? - Dir.glob("config/rent_range_data/*.csv").each do |path| - start_year = File.basename(path, ".csv") - Rake::Task["data_import:rent_ranges"].invoke(start_year, path) + if LaRentRange.count.zero? + Dir.glob("config/rent_range_data/*.csv").each do |path| + start_year = File.basename(path, ".csv") + Rake::Task["data_import:rent_ranges"].invoke(start_year, path) + end end end # rubocop:enable Rails/Output diff --git a/spec/components/search_component_spec.rb b/spec/components/search_component_spec.rb index fb68267eb..8c2b8b7a7 100644 --- a/spec/components/search_component_spec.rb +++ b/spec/components/search_component_spec.rb @@ -7,6 +7,7 @@ RSpec.describe SearchComponent, type: :component do let(:value) { nil } before do + allow(request).to receive(:path).and_return("/users") render_inline(described_class.new(current_user:, search_label:, value:)) end diff --git a/spec/controllers/modules/search_filter_spec.rb b/spec/controllers/modules/search_filter_spec.rb new file mode 100644 index 000000000..4db149e69 --- /dev/null +++ b/spec/controllers/modules/search_filter_spec.rb @@ -0,0 +1,56 @@ +require "rails_helper" + +RSpec.describe Modules::SearchFilter do + subject(:instance) { Class.new.include(described_class).new } + + describe "filtered_collection" do + before do + FactoryBot.create_list(:organisation, 5) + FactoryBot.create(:organisation, name: "Acme LTD") + end + + let(:organisation_list) { Organisation.all } + + context "when given a search term" do + let(:search_term) { "Acme" } + + it "filters the collection on search term" do + expect(instance.filtered_collection(organisation_list, search_term).count).to eq(1) + end + end + + context "when not given a search term" do + let(:search_term) { nil } + + it "does not filter the given collection" do + expect(instance.filtered_collection(organisation_list, search_term).count).to eq(6) + end + end + end + + describe "filtered_users" do + 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 diff --git a/spec/controllers/modules/users_filter_spec.rb b/spec/controllers/modules/users_filter_spec.rb deleted file mode 100644 index 154f1ccb0..000000000 --- a/spec/controllers/modules/users_filter_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -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 diff --git a/spec/models/organisation_spec.rb b/spec/models/organisation_spec.rb index adfb40f68..55adc597d 100644 --- a/spec/models/organisation_spec.rb +++ b/spec/models/organisation_spec.rb @@ -138,4 +138,25 @@ RSpec.describe Organisation, type: :model do expect(organisation.paper_trail.previous_version.name).to eq("DLUHC") end end + + describe "scopes" do + before do + FactoryBot.create(:organisation, name: "Joe Bloggs") + FactoryBot.create(:organisation, name: "Tom Smith") + 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 all searchable field" do + it "returns case insensitive matching records" do + expect(described_class.search_by("Joe").count).to eq(1) + expect(described_class.search_by("joe").count).to eq(1) + end + end + end end diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb index 94f928a6b..74c1fe6b6 100644 --- a/spec/requests/organisations_controller_spec.rb +++ b/spec/requests/organisations_controller_spec.rb @@ -364,6 +364,10 @@ RSpec.describe OrganisationsController, type: :request do expect(page).to have_content("#{total_number_of_orgs} total organisations") end + it "shows a search bar" do + expect(page).to have_field("search", type: "search") + end + context "when viewing a specific organisation" do let(:number_of_org1_case_logs) { 2 } let(:number_of_org2_case_logs) { 4 } @@ -478,6 +482,54 @@ RSpec.describe OrganisationsController, type: :request do expect(page).to have_title("Organisations (page 2 of 2)") end end + + context "when searching" do + let!(:searched_organisation) { FactoryBot.create(:organisation, name: "Unusual name") } + let!(:other_organisation) { FactoryBot.create(:organisation, name: "Some other name") } + let(:search_param) { "Unusual" } + + before do + get "/organisations?search=#{search_param}" + end + + it "returns matching results" do + expect(page).to have_content(searched_organisation.name) + expect(page).not_to have_content(other_organisation.name) + end + + it "updates the table caption" do + expect(page).to have_content("1 organisation found matching ‘#{search_param}’ of 29 total organisations.") + end + + it "has search in the title" do + expect(page).to have_title("Organisations (1 organisation matching ‘#{search_param}’ of 29 total organisations) - Submit social housing lettings and sales data (CORE) - GOV.UK") + end + + context "when the search term matches more than 1 result" do + let(:search_param) { "name" } + + it "returns matching results" do + expect(page).to have_content(searched_organisation.name) + expect(page).to have_content(other_organisation.name) + end + + it "updates the table caption" do + expect(page).to have_content("2 organisations found matching ‘#{search_param}’ of 29 total organisations.") + end + + it "has search in the title" do + expect(page).to have_title("Organisations (2 organisations matching ‘#{search_param}’ of 29 total organisations) - Submit social housing lettings and sales data (CORE) - GOV.UK") + end + end + + context "when search results require pagination" do + let(:search_param) { "DLUHC" } + + it "has search and pagination in the title" do + expect(page).to have_title("Organisations (27 organisations matching ‘#{search_param}’ of 29 total organisations) (page 1 of 2) - Submit social housing lettings and sales data (CORE) - GOV.UK") + end + end + end end end end diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 54c4e3526..453e4d34b 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -381,7 +381,7 @@ RSpec.describe UsersController, type: :request do end it "updates the table caption" do - expect(page).to have_content("1 user found matching ‘filter’ of 5 total users.") + expect(page).to have_content("1 user found matching ‘filter’ of 4 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("2 users found matching ‘joe’ of 5 total users.") + expect(page).to have_content("2 users found matching ‘joe’ of 4 total users.") end end end