diff --git a/app/components/search_component.rb b/app/components/search_component.rb
index 676257263..eefdc5309 100644
--- a/app/components/search_component.rb
+++ b/app/components/search_component.rb
@@ -9,10 +9,12 @@ class SearchComponent < ViewComponent::Base
end
def path(current_user)
- if request.path.include?("users")
- user_path(current_user)
+ if request.path.include?("organisations") && request.path.include?("users")
+ request.path
elsif request.path.include?("organisations") && request.path.include?("logs")
request.path
+ elsif request.path.include?("users")
+ user_path(current_user)
elsif request.path.include?("organisations")
organisations_path
elsif request.path.include?("logs")
diff --git a/app/controllers/organisations_controller.rb b/app/controllers/organisations_controller.rb
index 921953779..4452fc4a0 100644
--- a/app/controllers/organisations_controller.rb
+++ b/app/controllers/organisations_controller.rb
@@ -24,7 +24,11 @@ class OrganisationsController < ApplicationController
@pagy, @users = pagy(filtered_users(@organisation.users.sorted_by_organisation_and_role, search_term))
@searched = search_term.presence
@total_count = @organisation.users.size
- render "users/index"
+ if current_user.support?
+ render "users", layout: "application"
+ else
+ render "/users/index"
+ end
end
def details
diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index 7c52cdc0d..e0df2c2e6 100644
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -62,6 +62,7 @@ class UsersController < ApplicationController
end
def new
+ @organisation_id = params["organisation_id"]
@resource = User.new
end
diff --git a/app/helpers/navigation_items_helper.rb b/app/helpers/navigation_items_helper.rb
index 0115d6a24..1ebf17d11 100644
--- a/app/helpers/navigation_items_helper.rb
+++ b/app/helpers/navigation_items_helper.rb
@@ -4,23 +4,24 @@ module NavigationItemsHelper
def primary_items(path, current_user)
if current_user.support?
[
- NavigationItem.new("Organisations", organisations_path, organisation_current?(path)),
+ NavigationItem.new("Organisations", organisations_path, organisations_current?(path)),
NavigationItem.new("Users", "/users", users_current?(path)),
NavigationItem.new("Logs", case_logs_path, logs_current?(path)),
]
else
[
NavigationItem.new("Logs", case_logs_path, logs_current?(path)),
- NavigationItem.new("Users", users_organisation_path(current_user.organisation), users_current?(path)),
- NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", organisation_current?(path)),
+ NavigationItem.new("Users", users_organisation_path(current_user.organisation), subnav_users_path?(path)),
+ NavigationItem.new("About your organisation", "/organisations/#{current_user.organisation.id}", subnav_details_path?(path)),
]
end
end
def secondary_items(path, current_organisation_id)
[
- NavigationItem.new("Logs", "/organisations/#{current_organisation_id}/logs", organisation_logs_current?(path, current_organisation_id)),
- NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", about_organisation_current?(path, current_organisation_id)),
+ NavigationItem.new("Logs", "/organisations/#{current_organisation_id}/logs", subnav_logs_path?(path)),
+ NavigationItem.new("Users", "/organisations/#{current_organisation_id}/users", subnav_users_path?(path)),
+ NavigationItem.new("About this organisation", "/organisations/#{current_organisation_id}", subnav_details_path?(path)),
]
end
@@ -31,18 +32,22 @@ private
end
def users_current?(path)
- path.include?("/users")
+ path == "/users"
end
- def organisation_current?(path)
- path.include?("/organisations") && !path.include?("/users")
+ def organisations_current?(path)
+ path == "/organisations" || subnav_users_path?(path) || subnav_logs_path?(path) || subnav_details_path?(path)
end
- def about_organisation_current?(path, organisation_id)
- path.include?("/organisations/#{organisation_id}/details") || path.include?("/organisations/#{organisation_id}/edit")
+ def subnav_users_path?(path)
+ path.include?("/organisations") && path.include?("/users")
end
- def organisation_logs_current?(path, organisation_id)
- path == "/organisations/#{organisation_id}/logs"
+ def subnav_logs_path?(path)
+ path.include?("/organisations") && path.include?("/logs")
+ end
+
+ def subnav_details_path?(path)
+ path.include?("/organisations") && path.include?("/details")
end
end
diff --git a/app/helpers/tab_nav_helper.rb b/app/helpers/tab_nav_helper.rb
index bc3e10f86..1157b83e6 100644
--- a/app/helpers/tab_nav_helper.rb
+++ b/app/helpers/tab_nav_helper.rb
@@ -3,7 +3,7 @@ module TabNavHelper
def user_cell(user)
link_text = user.name.presence || user.email
- [govuk_link_to(link_text, user), "#{user.email}"].join("\n")
+ [govuk_link_to(link_text, user), "User #{user.email}"].join("\n")
end
def org_cell(user)
diff --git a/app/views/organisations/users.html.erb b/app/views/organisations/users.html.erb
new file mode 100644
index 000000000..67df43109
--- /dev/null
+++ b/app/views/organisations/users.html.erb
@@ -0,0 +1,27 @@
+<% item_label = @pagy.count > 1 ? "users" : "user" %>
+<% if @searched.present? %>
+ <% title = "Your organisation (#{@pagy.count} #{item_label} matching ‘#{@searched}’ of #{@total_count} total users)" %>
+<% else %>
+ <% title = "Your organisation (User)" %>
+<% end %>
+
+<% content_for :title, title %>
+
+<% content_for :tab_title do %>
+ <%= "Users" %>
+<% end %>
+
+<%= render SubNavigationComponent.new(
+ items: secondary_items(request.path, @organisation.id),
+) %>
+
+<% if current_user.data_coordinator? || current_user.support? %>
+ <%= govuk_button_link_to "Invite user", new_user_path(organisation_id: @organisation.id), html: { method: :get } %>
+<% end %>
+
+<%= render SearchComponent.new(current_user:, search_label: "Search by name or email address", value: @searched) %>
+
+
+
+<%= 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/app/views/users/_user_list.html.erb b/app/views/users/_user_list.html.erb
index 780d481f0..fb370c914 100644
--- a/app/views/users/_user_list.html.erb
+++ b/app/views/users/_user_list.html.erb
@@ -49,6 +49,9 @@
<% end %>
<% row.cell(text: simple_format(org_cell(user), {}, wrapper_tag: "div")) %>
<% row.cell(text: user.active? ? user.last_sign_in_at&.to_formatted_s(:govuk_date) : "Deactivated") %>
+ <%= govuk_link_to users_path(user) do %>
+ User <%= user.id %>
+ <% end %>
<% end %>
<% end %>
<% end %>
diff --git a/app/views/users/new.html.erb b/app/views/users/new.html.erb
index 642337fc4..fa9fc26fc 100644
--- a/app/views/users/new.html.erb
+++ b/app/views/users/new.html.erb
@@ -30,13 +30,17 @@
<% null_option = [OpenStruct.new(id: "", name: "Select an option")] %>
<% organisations = Organisation.all.map { |org| OpenStruct.new(id: org.id, name: org.name) } %>
<% answer_options = null_option + organisations %>
+ <% if @organisation_id %>
+ <% organisation = Organisation.find(@organisation_id) %>
+ <% answer_options = [OpenStruct.new(id: organisation.id, name: organisation.name)] %>
+ <% end %>
<%= f.govuk_collection_select :organisation_id,
answer_options,
:id,
:name,
label: { text: "Organisation", size: "m" },
- options: { disabled: [""], selected: "" } %>
+ options: { disabled: [""], selected: @organisation_id ? answer_options.first : "" } %>
<% end %>
<% roles = current_user.assignable_roles.map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %>
diff --git a/spec/features/log_spec.rb b/spec/features/log_spec.rb
index 2e8812b63..31b556a5b 100644
--- a/spec/features/log_spec.rb
+++ b/spec/features/log_spec.rb
@@ -2,7 +2,7 @@ require "rails_helper"
RSpec.describe "Log Features" do
context "when searching for specific logs" do
- context "when I am logged in and there are logs in the database" do
+ context "when I am signed in and there are logs in the database" do
let(:user) { FactoryBot.create(:user, last_sign_in_at: Time.zone.now) }
let!(:log_to_search) { FactoryBot.create(:case_log, owning_organisation: user.organisation) }
let!(:same_organisation_log) { FactoryBot.create(:case_log, owning_organisation: user.organisation) }
diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb
index 8d00bf2dd..99e3657c6 100644
--- a/spec/features/organisation_spec.rb
+++ b/spec/features/organisation_spec.rb
@@ -5,6 +5,7 @@ RSpec.describe "User Features" do
include Helpers
let(:organisation) { user.organisation }
let(:org_id) { organisation.id }
+ let(:org_name) { organisation.name }
let(:set_password_template_id) { User::CONFIRMABLE_TEMPLATE_ID }
let(:notify_client) { instance_double(Notifications::Client) }
let(:confirmation_token) { "MCDH5y6Km-U7CFPgAMVS" }
@@ -15,7 +16,10 @@ RSpec.describe "User Features" do
allow(devise_notify_mailer).to receive(:notify_client).and_return(notify_client)
allow(Devise).to receive(:friendly_token).and_return(confirmation_token)
allow(notify_client).to receive(:send_email).and_return(true)
- sign_in user
+ visit("/organisations")
+ fill_in("user[email]", with: user.email)
+ fill_in("user[password]", with: user.password)
+ click_button("Sign in")
end
context "when user is a data coordinator" do
@@ -82,21 +86,28 @@ RSpec.describe "User Features" do
end
context "when user is support user" do
+ let(:otp) { "999111" }
+ let(:user) { FactoryBot.create(:user, :support) }
+
+ before do
+ allow(SecureRandom).to receive(:random_number).and_return(otp)
+ click_link("Sign out")
+ visit("/organisations")
+ fill_in("user[email]", with: user.email)
+ fill_in("user[password]", with: user.password)
+ click_button("Sign in")
+ fill_in("code", with: otp)
+ click_button("Submit")
+ end
+
context "when viewing logs for specific organisation" do
- let(:user) { FactoryBot.create(:user, :support) }
let(:first_log) { organisation.case_logs.first }
- let(:otp) { "999111" }
let!(:log_to_search) { FactoryBot.create(:case_log, owning_organisation: user.organisation, managing_organisation_id: organisation.id) }
let!(:other_logs) { FactoryBot.create_list(:case_log, 4, owning_organisation_id: organisation.id, managing_organisation_id: organisation.id) }
let(:number_of_case_logs) { CaseLog.count }
before do
first_log.update!(startdate: Time.utc(2022, 6, 2, 10, 36, 49))
- allow(SecureRandom).to receive(:random_number).and_return(otp)
- click_link("Sign out")
- sign_in user
- fill_in("code", with: otp)
- click_button("Submit")
visit("/organisations/#{org_id}/logs")
end
@@ -153,5 +164,51 @@ RSpec.describe "User Features" do
expect(page).not_to have_link first_log.id.to_s, href: "/logs/#{first_log.id}"
end
end
+
+ context "when I search for users belonging to a specific organisation" do
+ context "when I am signed in and there are users in the database" do
+ let!(:user_list) { FactoryBot.create_list(:user, 4, organisation: user.organisation) }
+
+ context "when I visit the organisation page" do
+ before do
+ visit("/organisations/#{org_id}")
+ end
+
+ it "has link to the organisations users tab" do
+ expect(page).to have_link("Users", href: "/organisations/#{org_id}/users")
+ end
+
+ context "when I click users link in submenu" do
+ before do
+ click_link("Users", href: "/organisations/#{org_id}/users")
+ end
+
+ it "shows list of users belonging to the same organisation" do
+ user_list.each do |user|
+ expect(page).to have_content(user.email)
+ end
+ end
+
+ it "shows submenu for selected orgnisation" do
+ expect(page).to have_css('[aria-current="page"]', text: "Users")
+ expect(page).to have_current_path("/organisations/#{org_id}/users")
+ expect(page).to have_link("Logs")
+ expect(page).to have_link("About this organisation")
+ end
+
+ context "when I click on Invite user and there are multiple organisations in the database" do
+ before do
+ FactoryBot.create_list(:organisation, 5)
+ click_link(text: "Invite user")
+ end
+
+ it "has only specific organisation name in the dropdown" do
+ expect(page).to have_select("user-organisation-id-field", options: [org_name])
+ end
+ end
+ end
+ end
+ end
+ end
end
end
diff --git a/spec/helpers/navigation_items_helper_spec.rb b/spec/helpers/navigation_items_helper_spec.rb
index 9a060c759..1bcf23f1a 100644
--- a/spec/helpers/navigation_items_helper_spec.rb
+++ b/spec/helpers/navigation_items_helper_spec.rb
@@ -81,6 +81,80 @@ RSpec.describe NavigationItemsHelper do
expect(primary_items("/account", current_user)).to eq(expected_navigation_items)
end
end
+
+ context "when the user is on the specific organisation's page" do
+ context "when the user is on organisation logs page" do
+ let(:required_sub_path) { "logs" }
+ let(:expected_navigation_items) do
+ [
+ NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true),
+ NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
+ NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false),
+ ]
+ end
+
+ let(:expected_secondary_navigation_items) do
+ [
+ NavigationItemsHelper::NavigationItem.new("Logs", "/organisations/#{current_user.organisation.id}/logs", true),
+ NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false),
+ NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false),
+ ]
+ end
+
+ it "returns navigation items with the logs item set as current" do
+ expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items)
+ expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items)
+ end
+ end
+
+ context "when the user is on organisation users page" do
+ let(:required_sub_path) { "users" }
+ let(:expected_navigation_items) do
+ [
+ NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true),
+ NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
+ NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false),
+ ]
+ end
+
+ let(:expected_secondary_navigation_items) do
+ [
+ NavigationItemsHelper::NavigationItem.new("Logs", "/organisations/#{current_user.organisation.id}/logs", false),
+ NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", true),
+ NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", false),
+ ]
+ end
+
+ it "returns navigation items with the logs item set as current" do
+ expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items)
+ expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items)
+ end
+ end
+
+ context "when the user is on organisation details page" do
+ let(:required_sub_path) { "details" }
+ let(:expected_navigation_items) do
+ [
+ NavigationItemsHelper::NavigationItem.new("Organisations", "/organisations", true),
+ NavigationItemsHelper::NavigationItem.new("Users", "/users", false),
+ NavigationItemsHelper::NavigationItem.new("Logs", "/logs", false),
+ ]
+ end
+
+ let(:expected_secondary_navigation_items) do
+ [
+ NavigationItemsHelper::NavigationItem.new("Logs", "/organisations/#{current_user.organisation.id}/logs", false),
+ NavigationItemsHelper::NavigationItem.new("Users", "/organisations/#{current_user.organisation.id}/users", false),
+ NavigationItemsHelper::NavigationItem.new("About this organisation", "/organisations/#{current_user.organisation.id}", true),
+ ]
+ end
+
+ it "returns navigation items with the logs item set as current" do
+ expect(primary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user)).to eq(expected_navigation_items)
+ expect(secondary_items("/organisations/#{current_user.organisation.id}/#{required_sub_path}", current_user.organisation.id)).to eq(expected_secondary_navigation_items)
+ end
+ end
+ end
end
end
end
diff --git a/spec/helpers/tab_nav_helper_spec.rb b/spec/helpers/tab_nav_helper_spec.rb
index 4930d33b5..b59bb5d05 100644
--- a/spec/helpers/tab_nav_helper_spec.rb
+++ b/spec/helpers/tab_nav_helper_spec.rb
@@ -6,7 +6,7 @@ RSpec.describe TabNavHelper do
describe "#user_cell" do
it "returns user link and email separated by a newline character" do
- expected_html = "Danny Rojas\n#{user.email}"
+ expected_html = "#{user.name}\nUser #{user.email}"
expect(user_cell(user)).to match(expected_html)
end
end
diff --git a/spec/requests/organisations_controller_spec.rb b/spec/requests/organisations_controller_spec.rb
index d49e6a61a..5647cfe7a 100644
--- a/spec/requests/organisations_controller_spec.rb
+++ b/spec/requests/organisations_controller_spec.rb
@@ -131,9 +131,11 @@ RSpec.describe OrganisationsController, type: :request do
expect(response.body).to include(user.email)
end
- it "has a hidden header title" do
- expected_html = " Users"
- expect(response.body).to include(expected_html)
+ it "shows hidden accesibility fields only for active users in the current user's organisation" do
+ expected_case_row_log = "User #{user.email}"
+ unauthorized_case_row_log = "User #{other_org_user.email}"
+ expect(CGI.unescape_html(response.body)).to include(expected_case_row_log)
+ expect(CGI.unescape_html(response.body)).not_to include(unauthorized_case_row_log)
end
it "shows only active users in the current user's organisation" do
@@ -517,6 +519,138 @@ RSpec.describe OrganisationsController, type: :request do
end
end
+ context "when viewing a specific organisation users" do
+ let!(:users) { FactoryBot.create_list(:user, 5, organisation:) }
+ let!(:different_org_users) { FactoryBot.create_list(:user, 5) }
+
+ before do
+ get "/organisations/#{organisation.id}/users", headers:, params: {}
+ end
+
+ it "displays the name of the organisation" do
+ expect(page).to have_content(organisation.name)
+ end
+
+ it "has a sub-navigation with correct tabs" do
+ expect(page).to have_css(".app-sub-navigation")
+ expect(page).to have_content("Users")
+ end
+
+ it "displays users for this organisation" do
+ expect(page).to have_content(user.email)
+ users.each do |user|
+ expect(page).to have_content(user.email)
+ end
+ end
+
+ it "doesn't display users for other organisations" do
+ different_org_users.each do |different_org_user|
+ expect(page).not_to have_content(different_org_user.email)
+ end
+ end
+
+ context "when a search parameter is passed" do
+ let!(:matching_user) { FactoryBot.create(:user, organisation:, name: "joe", email: "matching@example.com") }
+ let(:org_user_count) { User.where(organisation:).count }
+
+ before do
+ get "/organisations/#{user.organisation.id}/users?search=#{search_param}"
+ end
+
+ context "when our search string matches case" do
+ let(:search_param) { "joe" }
+
+ it "returns only matching results" do
+ expect(page).to have_content(matching_user.name)
+ expect(page).not_to have_link(user.name)
+
+ different_org_users.each do |different_org_user|
+ expect(page).not_to have_content(different_org_user.email)
+ end
+
+ users.each do |org_user|
+ expect(page).not_to have_content(org_user.email)
+ end
+ end
+
+ it "updates the table caption" do
+ expect(page).to have_content("1 user found matching ‘#{search_param}’ of #{org_user_count} total users.")
+ end
+
+ context "when we need case insensitive search" do
+ let(:search_param) { "Joe" }
+
+ it "returns only matching results" do
+ expect(page).to have_content(matching_user.name)
+ expect(page).not_to have_link(user.name)
+
+ different_org_users.each do |different_org_user|
+ expect(page).not_to have_content(different_org_user.email)
+ end
+
+ users.each do |org_user|
+ expect(page).not_to have_content(org_user.email)
+ end
+ end
+
+ it "updates the table caption" do
+ expect(page).to have_content("1 user found matching ‘#{search_param}’ of #{org_user_count} total users.")
+ end
+ end
+ end
+
+ context "when our search term matches an email" do
+ let(:search_param) { "matching@example.com" }
+
+ it "returns only matching results" do
+ expect(page).to have_content(matching_user.name)
+ expect(page).not_to have_link(user.name)
+
+ different_org_users.each do |different_org_user|
+ expect(page).not_to have_content(different_org_user.email)
+ end
+
+ users.each do |org_user|
+ expect(page).not_to have_content(org_user.email)
+ end
+ end
+
+ it "updates the table caption" do
+ expect(page).to have_content("1 user found matching ‘#{search_param}’ of #{org_user_count} total users.")
+ end
+
+ context "when our search term matches an email and a name" do
+ let!(:matching_user) { FactoryBot.create(:user, organisation:, name: "Foobar", email: "some@example.com") }
+ let!(:another_matching_user) { FactoryBot.create(:user, organisation:, name: "Joe", email: "foobar@example.com") }
+ let!(:org_user_count) { User.where(organisation:).count }
+ let(:search_param) { "Foobar" }
+
+ before do
+ get "/organisations/#{user.organisation.id}/users?search=#{search_param}"
+ end
+
+ it "returns only matching results" do
+ expect(page).to have_link(matching_user.name)
+ expect(page).to have_link(another_matching_user.name)
+ expect(page).not_to have_link(user.name)
+
+ different_org_users.each do |different_org_user|
+ expect(page).not_to have_content(different_org_user.email)
+ end
+
+ users.each do |org_user|
+ expect(page).not_to have_content(org_user.email)
+ end
+ end
+
+ it "updates the table caption" do
+ expect(page).to have_content("2 users found matching ‘#{search_param}’ of #{org_user_count} total users.")
+ end
+ end
+ end
+ end
+ end
+
context "when viewing a specific organisation details" do
before do
get "/organisations/#{organisation.id}/details", headers:, params: {}
diff --git a/spec/requests/users_controller_spec.rb b/spec/requests/users_controller_spec.rb
index 9b951b742..7831a45d9 100644
--- a/spec/requests/users_controller_spec.rb
+++ b/spec/requests/users_controller_spec.rb
@@ -1593,16 +1593,31 @@ RSpec.describe UsersController, type: :request do
describe "#new" do
before do
sign_in user
+ FactoryBot.create(:organisation, name: "other org")
end
- it "can assign support role to the new user" do
- get "/users/new"
- expect(page).to have_field("user-role-support-field")
- end
+ context "when support user" do
+ it "can assign support role to the new user" do
+ get "/users/new"
+ expect(page).to have_field("user-role-support-field")
+ end
- it "can assign organisation to the new user" do
- get "/users/new"
- expect(page).to have_field("user-organisation-id-field")
+ it "can assign organisation to the new user" do
+ get "/users/new"
+ expect(page).to have_field("user-organisation-id-field")
+ end
+
+ it "has all organisation names in the dropdown" do
+ get "/users/new"
+ expect(page).to have_select("user-organisation-id-field", with_options: Organisation.pluck(:name))
+ end
+
+ context "when organisation id is present in params and there are multiple organisations in the database" do
+ it "has only specific organisation name in the dropdown" do
+ get "/users/new", params: { organisation_id: user.organisation.id }
+ expect(page).to have_select("user-organisation-id-field", options: [user.organisation.name])
+ end
+ end
end
end
end