Hide inactive users and allow support users to view all users

app/controllers/organisations_controller.rb

@@ -4,9 +4,7 @@ class OrganisationsController < ApplicationController
   before_action :authenticate_scope!
 
   def index
-    unless current_user.support?
-      redirect_to user_path(current_user)
-    end
+    @organisations = current_user.support? ? Organisation.all : @user.organisation
   end
 
   def show
@@ -14,7 +12,8 @@ class OrganisationsController < ApplicationController
   end
 
   def users
-    render "users"
+    @users = @organisation.users.where(active: true)
+    render "users/index"
   end
 
   def details

app/controllers/users_controller.rb

@@ -6,11 +6,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?
+
+    @users = User.all.where(active: true)
   end
 
+  def show; end
+
   def update
     if @user.update(user_params)
       if @user == current_user

app/views/organisations/index.html.erb

@@ -0,0 +1 @@
+

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 %>

app/views/users/index.html.erb

@@ -0,0 +1,27 @@
+<% 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 %>
+  <% @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 %>

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,13 @@ 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
         end
 
         context "with an organisation that are not in scope for the user, i.e. that they do not belong to" do

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,24 @@ 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
+    end
+
     describe "#show" do
       context "when the current user matches the user ID" do
         before do