diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb index f0c2dcbfd..cc5751edf 100644 --- a/app/controllers/organisations_controller.rb +++ b/app/controllers/organisations_controller.rb @@ -17,7 +17,7 @@ class OrganisationsController < ApplicationController end def users - @pagy, @users = pagy(User.filter_by_name(params["user-search-field"]).filter_by_active) + @pagy, @users = pagy(filtered_users) render "users/index" end @@ -58,6 +58,14 @@ class OrganisationsController < ApplicationController private + def filtered_users + if search_param = params["user-search-field"] + User.search_by(search_param) + else + User.all + end.filter_by_active + end + def org_params params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone) end diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index 0640fbf18..375a1052c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -9,7 +9,7 @@ class UsersController < ApplicationController def index redirect_to users_organisation_path(current_user.organisation) unless current_user.support? - @pagy, @users = pagy(User.filter_by_name(params["user-search-field"]).filter_by_active) + @pagy, @users = pagy(filtered_users) respond_to do |format| format.html @@ -77,6 +77,14 @@ class UsersController < ApplicationController private + def filtered_users + if search_param = params["user-search-field"] + User.search_by(search_param) + else + User.all + end.filter_by_active + end + def format_error_messages errors = @user.errors.to_hash @user.errors.clear diff --git a/app/models/user.rb b/app/models/user.rb index 2cae675ad..97cc43c62 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -32,8 +32,10 @@ class User < ApplicationRecord enum role: ROLES - scope :filter_by_name, ->(name) { name.present? ? where("name ILIKE ?", "%#{name}%") : User.all } - scope :filter_by_active, ->() { where(active: true) } + 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? diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb index 76a992bc6..8fdaa7b93 100644 --- a/spec/requests/users_controller_spec.rb +++ b/spec/requests/users_controller_spec.rb @@ -335,8 +335,8 @@ 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, name: "filter name") } + 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 @@ -364,7 +364,8 @@ RSpec.describe UsersController, type: :request do end context "when our search string matches case" do - let (:search_param){"filter"} + 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) @@ -372,7 +373,8 @@ RSpec.describe UsersController, type: :request do end context "when we need case insensitive search" do - let (:search_param){"Filter"} + 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) @@ -754,9 +756,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 @@ -787,23 +789,38 @@ RSpec.describe UsersController, type: :request do get "/users?user-search-field=#{search_param}" end - 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) + 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 + 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 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) + 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).not_to have_content(other_org_user.name) + expect(page).to have_content(other_org_user.name) end end end