CLDC-2245 Add user status filter (#1797)

* Add user status filters

* Add filters to the pages

* only show unconfirmed and deactivated logs as deactivated

* Look at last signed in at to determine unconfirmed users

app/controllers/organisations_controller.rb

@@ -6,7 +6,9 @@ class OrganisationsController < ApplicationController
   before_action :find_resource, except: %i[index new create]
   before_action :authenticate_scope!, except: [:index]
   before_action :session_filters, if: -> { current_user.support? || current_user.organisation.has_managing_agents? }, only: %i[lettings_logs sales_logs email_lettings_csv download_lettings_csv email_sales_csv download_sales_csv]
+  before_action :session_filters, only: %i[users]
   before_action -> { filter_manager.serialize_filters_to_session }, if: -> { current_user.support? || current_user.organisation.has_managing_agents? }, only: %i[lettings_logs sales_logs email_lettings_csv download_lettings_csv email_sales_csv download_sales_csv]
+  before_action -> { filter_manager.serialize_filters_to_session }, only: %i[users]
 
   def index
     redirect_to organisation_path(current_user.organisation) unless current_user.support?
@@ -31,13 +33,14 @@ class OrganisationsController < ApplicationController
 
   def users
     organisation_users = @organisation.users.sorted_by_organisation_and_role
-    unpaginated_filtered_users = filtered_collection(organisation_users, search_term)
+    unpaginated_filtered_users = filter_manager.filtered_users(organisation_users, search_term, session_filters)
 
     respond_to do |format|
       format.html do
         @pagy, @users = pagy(unpaginated_filtered_users)
         @searched = search_term.presence
         @total_count = @organisation.users.size
+        @filter_type = "users"
 
         if current_user.support?
           render "users", layout: "application"
@@ -204,6 +207,8 @@ private
       "lettings_logs"
     elsif params[:action].include?("sales")
       "sales_logs"
+    elsif params[:action].include?("users")
+      "users"
     end
   end
 

app/controllers/users_controller.rb

@@ -7,15 +7,18 @@ class UsersController < ApplicationController
   before_action :authenticate_user!
   before_action :find_resource, except: %i[new create]
   before_action :authenticate_scope!, except: %i[new]
+  before_action :session_filters, if: :current_user, only: %i[index]
+  before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user, only: %i[index]
 
   def index
     redirect_to users_organisation_path(current_user.organisation) unless current_user.support?
 
     all_users = User.sorted_by_organisation_and_role
-    filtered_users = filtered_users(all_users, search_term)
+    filtered_users = filter_manager.filtered_users(all_users, search_term, session_filters)
     @pagy, @users = pagy(filtered_users)
     @searched = search_term.presence
     @total_count = all_users.size
+    @filter_type = "users"
 
     respond_to do |format|
       format.html
@@ -194,4 +197,12 @@ private
         current_user.data_coordinator? || current_user.support? || current_user == @user
     end
   end
+
+  def filter_manager
+    FilterManager.new(current_user:, session:, params:, filter_type: "users")
+  end
+
+  def session_filters
+    filter_manager.session_filters
+  end
 end

app/helpers/filters_helper.rb

@@ -38,6 +38,14 @@ module FiltersHelper
     }.freeze
   end
 
+  def user_status_filters
+    {
+      "active" => "Active",
+      "deactivated" => "Deactivated",
+      "unconfirmed" => "Unconfirmed",
+    }.freeze
+  end
+
   def selected_option(filter, filter_type)
     return false unless session[session_name_for(filter_type)]
 

app/models/user.rb

@@ -56,6 +56,27 @@ class User < ApplicationRecord
   scope :filter_by_active, -> { where(active: true) }
   scope :search_by, ->(param) { search_by_name(param).or(search_by_email(param)) }
   scope :sorted_by_organisation_and_role, -> { joins(:organisation).order("organisations.name", role: :desc, name: :asc) }
+  scope :filter_by_status, lambda { |statuses, _user = nil|
+    filtered_records = all
+    scopes = []
+
+    statuses.each do |status|
+      status = status == "active" ? "active_status" : status
+      status = status == "unconfirmed" ? "not_signed_in" : status
+      if respond_to?(status, true)
+        scopes << send(status)
+      end
+    end
+
+    if scopes.any?
+      filtered_records = filtered_records.merge(scopes.reduce(&:or))
+    end
+
+    filtered_records
+  }
+  scope :not_signed_in, -> { where(last_sign_in_at: nil, active: true) }
+  scope :deactivated, -> { where(active: false) }
+  scope :active_status, -> { where(active: true).where.not(last_sign_in_at: nil) }
 
   def lettings_logs
     if support?

app/services/filter_manager.rb

@@ -39,6 +39,17 @@ class FilterManager
     end
   end
 
+  def self.filter_users(users, search_term, filters, user)
+    users = filter_by_search(users, search_term)
+
+    filters.each do |category, values|
+      next if Array(values).reject(&:empty?).blank?
+
+      users = users.public_send("filter_by_#{category}", values, user)
+    end
+    users
+  end
+
   def serialize_filters_to_session(specific_org: false)
     session[session_name_for(filter_type)] = session_filters(specific_org:).to_json
   end
@@ -54,13 +65,17 @@ class FilterManager
       current_user.logs_filters(specific_org:).each do |filter|
         new_filters[filter] = params[filter] if params[filter].present?
       end
-    end
 
-    new_filters = new_filters.except("owning_organisation") if params["owning_organisation_select"] == "all"
+      new_filters = new_filters.except("owning_organisation") if params["owning_organisation_select"] == "all"
-    new_filters = new_filters.except("managing_organisation") if params["managing_organisation_select"] == "all"
+      new_filters = new_filters.except("managing_organisation") if params["managing_organisation_select"] == "all"
+
+      new_filters = new_filters.except("user") if params["assigned_to"] == "all"
+      new_filters["user"] = current_user.id.to_s if params["assigned_to"] == "you"
+    end
 
-    new_filters = new_filters.except("user") if params["assigned_to"] == "all"
+    if @filter_type.include?("users") && params["status"].present?
-    new_filters["user"] = current_user.id.to_s if params["assigned_to"] == "you"
+      new_filters["status"] = params["status"]
+    end
 
     new_filters
   end
@@ -71,6 +86,10 @@ class FilterManager
     FilterManager.filter_logs(logs, search_term, filters, all_orgs, current_user)
   end
 
+  def filtered_users(users, search_term, filters)
+    FilterManager.filter_users(users, search_term, filters, current_user)
+  end
+
   def bulk_upload
     id = (logs_filters["bulk_upload_id"] || []).reject(&:blank?)[0]
     @bulk_upload ||= current_user.bulk_uploads.find_by(id:)

app/views/organisations/users.html.erb

@@ -15,10 +15,15 @@
 <% 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 %>
+<div class="app-filter-layout" data-controller="filter-layout">
+  <%= render partial: "users/user_filters" %>
+  <div class="app-filter-layout__content">
 
-<%= render SearchComponent.new(current_user:, search_label: "Search by name or email address", value: @searched) %>
+    <%= render SearchComponent.new(current_user:, search_label: "Search by name or email address", value: @searched) %>
 
-<%= govuk_section_break(visible: true, size: "m") %>
+    <%= govuk_section_break(visible: true, size: "m") %>
 
-<%= render partial: "users/user_list", locals: { users: @users, title:, pagy: @pagy, searched: @searched, item_label:, total_count: @total_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" } %>
+    <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "users" } %>
+  </div>
+</div>

app/views/users/_user_filters.html.erb

@@ -0,0 +1,29 @@
+<div class="app-filter-layout__filter">
+  <div class="app-filter">
+    <div class="app-filter__header">
+      <h2 class="govuk-heading-m">Filters</h2>
+    </div>
+
+    <div class="app-filter__content">
+      <%= form_with url: users_path, html: { method: :get } do |f| %>
+        <div class="govuk-grid-row" style="white-space: nowrap">
+          <p class="govuk-grid-column-one-half">
+            <%= filters_applied_text(@filter_type) %>
+          </p>
+          <p class="govuk-!-text-align-right govuk-grid-column-one-half">
+            <%= reset_filters_link(@filter_type) %>
+          </p>
+        </div>
+
+        <%= render partial: "filters/checkbox_filter",
+                   locals: {
+                    f:,
+                    options: user_status_filters,
+                    label: "Status",
+                    category: "status",
+                  } %>
+        <%= f.govuk_submit "Apply filters", class: "govuk-!-margin-bottom-0" %>
+      <% end %>
+    </div>
+  </div>
+</div>

app/views/users/index.html.erb

@@ -8,10 +8,14 @@
 <% if current_user.data_coordinator? || current_user.support? %>
   <%= govuk_button_link_to "Invite user", new_user_path, html: { method: :get } %>
 <% end %>
+<div class="app-filter-layout" data-controller="filter-layout">
+  <%= render partial: "users/user_filters" %>
+  <div class="app-filter-layout__content">
+    <%= render SearchComponent.new(current_user:, search_label: "Search by name or email address", value: @searched) %>
 
-<%= render SearchComponent.new(current_user:, search_label: "Search by name or email address", value: @searched) %>
+    <%= govuk_section_break(visible: true, size: "m") %>
 
-<%= govuk_section_break(visible: true, size: "m") %>
+    <%= 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" } %>
-<%= render partial: "users/user_list", locals: { users: @users, title:, pagy: @pagy, searched: @searched, item_label:, total_count: @total_count } %>
+  </div>
-<%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "users" } %>
+</div>

spec/features/user_spec.rb

@@ -228,6 +228,39 @@ RSpec.describe "User Features" do
         expect(page).not_to have_css('[aria-current="page"]', text: "About your organisation")
         expect(page).not_to have_css('[aria-current="page"]', text: "Logs")
       end
+
+      context "when filtering users" do
+        context "when no filters are selected" do
+          it "displays the filters component with no clear button" do
+            expect(page).to have_content("No filters applied")
+            expect(page).not_to have_content("Clear")
+          end
+        end
+
+        context "when I have selected filters" do
+          before do
+            check("Active")
+            check("Deactivated")
+            click_button("Apply filters")
+          end
+
+          it "displays the filters component with a correct count and clear button" do
+            expect(page).to have_content("2 filters applied")
+            expect(page).to have_content("Clear")
+          end
+
+          context "when clearing the filters" do
+            before do
+              click_link("Clear")
+            end
+
+            it "clears the filters and displays the filter component as before" do
+              expect(page).to have_content("No filters applied")
+              expect(page).not_to have_content("Clear")
+            end
+          end
+        end
+      end
     end
 
     context "when viewing your organisation details" do

spec/models/user_spec.rb

@@ -258,11 +258,11 @@ RSpec.describe User, type: :model do
   describe "scopes" do
     let(:organisation_1) { create(:organisation, :without_dpc, name: "A") }
     let(:organisation_2) { create(:organisation, :without_dpc, name: "B") }
-    let!(:user_1) { create(:user, name: "Joe Bloggs", email: "joe@example.com", organisation: organisation_1, role: "support") }
+    let!(:user_1) { create(:user, name: "Joe Bloggs", email: "joe@example.com", organisation: organisation_1, role: "support", last_sign_in_at: Time.zone.now) }
-    let!(:user_3) { create(:user, name: "Tom Smith", email: "tom@example.com", organisation: organisation_1, role: "data_provider") }
     let!(:user_2) { create(:user, name: "Jenny Ford", email: "jenny@smith.com", organisation: organisation_1, role: "data_coordinator") }
+    let!(:user_3) { create(:user, name: "Tom Smith", email: "tom@example.com", organisation: organisation_1, role: "data_provider") }
     let!(:user_4) { create(:user, name: "Greg Thomas", email: "greg@org2.com", organisation: organisation_2, role: "data_coordinator") }
-    let!(:user_5) { create(:user, name: "Adam Thomas", email: "adam@org2.com", organisation: organisation_2, role: "data_coordinator") }
+    let!(:user_5) { create(:user, name: "Adam Thomas", email: "adam@org2.com", organisation: organisation_2, role: "data_coordinator", last_sign_in_at: Time.zone.now) }
 
     context "when searching by name" do
       it "returns case insensitive matching records" do
@@ -290,6 +290,46 @@ RSpec.describe User, type: :model do
         expect(described_class.sorted_by_organisation_and_role.to_a).to eq([user_1, user_2, user_3, user_5, user_4])
       end
     end
+
+    context "when filtering by status" do
+      before do
+        user_2.update!(active: false)
+        user_3.update!(active: false, last_sign_in_at: nil)
+        user_4.update!(last_sign_in_at: nil)
+      end
+
+      context "when filtering by active status" do
+        it "returns only active users" do
+          expect(described_class.filter_by_status(%w[active]).count).to eq(2)
+          expect(described_class.filter_by_status(%w[active])).to include(user_1)
+          expect(described_class.filter_by_status(%w[active])).to include(user_5)
+        end
+      end
+
+      context "when filtering by deactivated status" do
+        it "returns only deactivated users" do
+          expect(described_class.filter_by_status(%w[deactivated]).count).to eq(2)
+          expect(described_class.filter_by_status(%w[deactivated])).to include(user_2)
+          expect(described_class.filter_by_status(%w[deactivated])).to include(user_3)
+        end
+      end
+
+      context "when filtering by unconfirmed status" do
+        it "returns only unconfirmed users" do
+          expect(described_class.filter_by_status(%w[unconfirmed]).count).to eq(1)
+          expect(described_class.filter_by_status(%w[unconfirmed])).to include(user_4)
+        end
+      end
+
+      context "when filtering by multiple statuses" do
+        it "returns relevant users" do
+          expect(described_class.filter_by_status(%w[active unconfirmed]).count).to eq(3)
+          expect(described_class.filter_by_status(%w[active])).to include(user_1)
+          expect(described_class.filter_by_status(%w[active])).to include(user_5)
+          expect(described_class.filter_by_status(%w[unconfirmed])).to include(user_4)
+        end
+      end
+    end
   end
 
   describe "validate" do

spec/requests/users_controller_spec.rb

@@ -471,6 +471,60 @@ RSpec.describe UsersController, type: :request do
           end
         end
       end
+
+      context "when filtering" do
+        context "with status filter" do
+          let!(:active_user) { create(:user, name: "active name", active: true, organisation: user.organisation, last_sign_in_at: Time.zone.now) }
+          let!(:deactivated_user) { create(:user, active: false, name: "deactivated name", organisation: user.organisation, last_sign_in_at: Time.zone.now) }
+          let!(:unconfirmed_user) { create(:user, last_sign_in_at: nil, name: "unconfirmed name", organisation: user.organisation) }
+
+          it "shows users for multiple selected statuses" do
+            get "/users?status[]=active&status[]=deactivated", headers:, params: {}
+            follow_redirect!
+            expect(page).to have_link(active_user.name)
+            expect(page).to have_link(deactivated_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+          end
+
+          it "shows filtered active users" do
+            get "/users?status[]=active", headers:, params: {}
+            follow_redirect!
+            expect(page).to have_link(active_user.name)
+            expect(page).not_to have_link(deactivated_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+          end
+
+          it "shows filtered deactivated users" do
+            get "/users?status[]=deactivated", headers:, params: {}
+            follow_redirect!
+            expect(page).to have_link(deactivated_user.name)
+            expect(page).not_to have_link(active_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+          end
+
+          it "shows filtered unconfirmed users" do
+            get "/users?status[]=unconfirmed", headers:, params: {}
+            follow_redirect!
+            expect(page).to have_link(unconfirmed_user.name)
+            expect(page).not_to have_link(active_user.name)
+            expect(page).not_to have_link(deactivated_user.name)
+          end
+
+          it "does not reset the filters" do
+            get "/users?status[]=deactivated", headers:, params: {}
+            follow_redirect!
+            expect(page).to have_link(deactivated_user.name)
+            expect(page).not_to have_link(active_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+
+            get "/users", headers:, params: {}
+            follow_redirect!
+            expect(page).to have_link(deactivated_user.name)
+            expect(page).not_to have_link(active_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+          end
+        end
+      end
     end
 
     describe "CSV download" do
@@ -1158,6 +1212,54 @@ RSpec.describe UsersController, type: :request do
           end
         end
       end
+
+      context "when filtering" do
+        context "with status filter" do
+          let!(:active_user) { create(:user, name: "active name", active: true, last_sign_in_at: Time.zone.now) }
+          let!(:deactivated_user) { create(:user, active: false, name: "deactivated name", last_sign_in_at: Time.zone.now) }
+          let!(:unconfirmed_user) { create(:user, last_sign_in_at: nil, name: "unconfirmed name") }
+
+          it "shows users for multiple selected statuses" do
+            get "/users?status[]=active&status[]=deactivated", headers:, params: {}
+            expect(page).to have_link(active_user.name)
+            expect(page).to have_link(deactivated_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+          end
+
+          it "shows filtered active users" do
+            get "/users?status[]=active", headers:, params: {}
+            expect(page).to have_link(active_user.name)
+            expect(page).not_to have_link(deactivated_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+          end
+
+          it "shows filtered deactivated users" do
+            get "/users?status[]=deactivated", headers:, params: {}
+            expect(page).to have_link(deactivated_user.name)
+            expect(page).not_to have_link(active_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+          end
+
+          it "shows filtered unconfirmed users" do
+            get "/users?status[]=unconfirmed", headers:, params: {}
+            expect(page).to have_link(unconfirmed_user.name)
+            expect(page).not_to have_link(active_user.name)
+            expect(page).not_to have_link(deactivated_user.name)
+          end
+
+          it "does not reset the filters" do
+            get "/users?status[]=deactivated", headers:, params: {}
+            expect(page).to have_link(deactivated_user.name)
+            expect(page).not_to have_link(active_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+
+            get "/users", headers:, params: {}
+            expect(page).to have_link(deactivated_user.name)
+            expect(page).not_to have_link(active_user.name)
+            expect(page).not_to have_link(unconfirmed_user.name)
+          end
+        end
+      end
     end
 
     describe "CSV download" do