Browse Source

Hide inactive users and allow support users to view all users (#576)

* Hide inactive users and allow support users to view all users

* Enable support users to invite users to any organisation

* Add pagination to user views
pull/619/head
baarkerlounger 3 years ago committed by baarkerlounger
parent
commit
bd88eadb35
  1. 10
      app/controllers/organisations_controller.rb
  2. 25
      app/controllers/users_controller.rb
  3. 1
      app/views/organisations/index.html.erb
  4. 27
      app/views/organisations/users.html.erb
  5. 33
      app/views/users/index.html.erb
  6. 13
      app/views/users/new.html.erb
  7. 15
      spec/requests/organisations_controller_spec.rb
  8. 53
      spec/requests/users_controller_spec.rb

10
app/controllers/organisations_controller.rb

@ -1,12 +1,13 @@
class OrganisationsController < ApplicationController
include Pagy::Backend
before_action :authenticate_user!, except: [:index]
before_action :find_resource, except: [:index]
before_action :authenticate_scope!
def index
unless current_user.support?
redirect_to user_path(current_user)
end
redirect_to organisation_path(current_user.organisation) unless current_user.support?
@pagy, @organisations = pagy(Organisation.all)
end
def show
@ -14,7 +15,8 @@ class OrganisationsController < ApplicationController
end
def users
render "users"
@pagy, @users = pagy(@organisation.users.where(active: true))
render "users/index"
end
def details

25
app/controllers/users_controller.rb

@ -1,4 +1,5 @@
class UsersController < ApplicationController
include Pagy::Backend
include Devise::Controllers::SignInOut
include Helpers::Email
before_action :authenticate_user!
@ -6,11 +7,13 @@ class UsersController < ApplicationController
before_action :authenticate_scope!, except: %i[new]
def index
unless current_user.support?
redirect_to user_path(@user)
end
redirect_to users_organisation_path(current_user.organisation) unless current_user.support?
@pagy, @users = pagy(User.all.where(active: true))
end
def show; end
def update
if @user.update(user_params)
if @user == current_user
@ -49,7 +52,7 @@ class UsersController < ApplicationController
user = User.create(user_params.merge(org_params).merge(password_params))
if user.persisted?
user.send_reset_password_instructions
redirect_to users_organisation_path(current_user.organisation)
redirect_to created_user_redirect_path
else
@resource.errors.add :email, I18n.t("validations.email.taken")
render :new, status: :unprocessable_entity
@ -81,6 +84,8 @@ private
end
def org_params
return {} if current_user.support?
{ organisation: current_user.organisation }
end
@ -91,8 +96,18 @@ private
else
params.require(:user).permit(:email, :name, :password, :password_confirmation)
end
elsif current_user.data_coordinator? || current_user.support?
elsif current_user.data_coordinator?
params.require(:user).permit(:email, :name, :role, :is_dpo, :is_key_contact)
elsif current_user.support?
params.require(:user).permit(:email, :name, :role, :is_dpo, :is_key_contact, :organisation_id)
end
end
def created_user_redirect_path
if current_user.support?
users_path
else
users_organisation_path(current_user.organisation)
end
end

1
app/views/organisations/index.html.erb

@ -0,0 +1 @@

27
app/views/organisations/users.html.erb

@ -1,27 +0,0 @@
<% content_for :title, "Your organisation (Users)" %>
<% content_for :tab_title do %>
<%= "Users" %>
<% end %>
<% if current_user.data_coordinator? || current_user.support? %>
<%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %>
<% end %>
<%= govuk_table do |table| %>
<%= table.head do |head| %>
<%= head.row do |row| %>
<% row.cell(header: true, text: "Name and email adress") %>
<% row.cell(header: true, text: "Organisation and role") %>
<% row.cell(header: true, text: "Last logged in") %>
<% end %>
<% end %>
<% @organisation.users.each do |user| %>
<%= table.body do |body| %>
<%= body.row do |row| %>
<% row.cell(text: simple_format(user_cell(user), {}, wrapper_tag: "div")) %>
<% 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 %>

33
app/views/users/index.html.erb

@ -0,0 +1,33 @@
<% content_for :title, "Your organisation (Users)" %>
<% content_for :tab_title do %>
<%= "Users" %>
<% end %>
<% if current_user.data_coordinator? || current_user.support? %>
<%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %>
<% end %>
<%= govuk_table do |table| %>
<%= table.caption(size: "s", classes: %w[govuk-!-text-align-left govuk-!-margin-top-4 govuk-!-margin-bottom-4]) do |caption| %>
<span class="govuk-!-margin-right-4">
<strong><%= @pagy.count %></strong><span style="font-weight: normal"> total users</span>
</span>
<% end %>
<%= table.head do |head| %>
<%= head.row do |row| %>
<% row.cell(header: true, text: "Name and email adress") %>
<% row.cell(header: true, text: "Organisation and role") %>
<% row.cell(header: true, text: "Last logged in") %>
<% end %>
<% end %>
<% @users.each do |user| %>
<%= table.body do |body| %>
<%= body.row do |row| %>
<% row.cell(text: simple_format(user_cell(user), {}, wrapper_tag: "div")) %>
<% 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 %>
<%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "users" } %>

13
app/views/users/new.html.erb

@ -26,6 +26,19 @@
spellcheck: "false",
value: @resource.email %>
<% if current_user.support? %>
<% 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 %>
<%= f.govuk_collection_select :organisation_id,
answer_options,
:id,
:name,
label: { text: "Organisation", size: "m" },
options: { disabled: [""], selected: "" } %>
<% end %>
<% roles = current_user.assignable_roles.map { |key, _| OpenStruct.new(id: key, name: key.to_s.humanize) } %>
<%= f.govuk_collection_radio_buttons :role,

15
spec/requests/organisations_controller_spec.rb

@ -101,6 +101,10 @@ RSpec.describe OrganisationsController, type: :request do
context "when accessing the users tab" do
context "with an organisation that the user belongs to" 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") }
before do
sign_in user
get "/organisations/#{organisation.id}/users", headers:, params: {}
@ -125,6 +129,17 @@ RSpec.describe OrganisationsController, type: :request do
expected_html = "<h2 class=\"govuk-visually-hidden\"> Users"
expect(response.body).to include(expected_html)
end
it "shows only active users in the current user's organisation" do
expect(page).to have_content(user.name)
expect(page).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 "shows the pagination count" do
expect(page).to have_content("2 total users")
end
end
context "with an organisation that are not in scope for the user, i.e. that they do not belong to" do

53
spec/requests/users_controller_spec.rb

@ -338,6 +338,18 @@ RSpec.describe UsersController, type: :request do
let(:user) { FactoryBot.create(:user, :data_coordinator) }
let(:other_user) { FactoryBot.create(:user, organisation: user.organisation) }
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")
end
end
describe "#show" do
context "when the current user matches the user ID" do
before do
@ -696,6 +708,28 @@ RSpec.describe UsersController, type: :request do
allow(user).to receive(:need_two_factor_authentication?).and_return(false)
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") }
before do
sign_in user
get "/users", headers:, params: {}
end
it "shows all active users" do
expect(page).to have_content(user.name)
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)
end
it "shows the pagination count" do
expect(page).to have_content("3 total users")
end
end
describe "#show" do
context "when the current user matches the user ID" do
before do
@ -1041,12 +1075,15 @@ RSpec.describe UsersController, type: :request do
end
describe "#create" do
let(:organisation) { FactoryBot.create(:organisation) }
let(:email) { "new_user@example.com" }
let(:params) do
{
"user": {
name: "new user",
email: "new_user@example.com",
email:,
role: "data_coordinator",
organisation_id: organisation.id,
},
}
end
@ -1060,9 +1097,14 @@ RSpec.describe UsersController, type: :request do
expect { request }.to change(User, :count).by(1)
end
it "redirects back to organisation users page" do
it "adds the user to the correct organisation" do
request
expect(response).to redirect_to("/organisations/#{user.organisation.id}/users")
expect(User.find_by(email:).organisation).to eq(organisation)
end
it "redirects back to users page" do
request
expect(response).to redirect_to("/users")
end
context "when the email is already taken" do
@ -1103,6 +1145,11 @@ RSpec.describe UsersController, type: :request 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")
end
end
end

Loading…
Cancel
Save