Merge pull request #2423 from UKGovernmentBEIS/feature/users-separate…

…d-by-tabs

Separate /users page into active and inactive users

CHANGELOG.md

@@ -9,6 +9,7 @@
 [Full changelog][149]
 
 - Rails version updated to 6.1.7.10
+- Separate out users into tabbed view of active and inactive
 
 ## Release 148 - 2024-09-30
 

app/controllers/users_controller.rb

@@ -3,7 +3,13 @@ class UsersController < BaseController
 
   def index
     authorize :user, :index?
-    @users = policy_scope(User).includes(:organisation).joins(:organisation).order("users.active DESC, organisations.name ASC, users.name ASC")
+    @user_state = params[:user_state]
+    @users = policy_scope(User).includes(:organisation).joins(:organisation).order("organisations.name ASC, users.name ASC")
+    @users = if @user_state == "active"
+      @users.active
+    else
+      @users.inactive
+    end
   end
 
   def show

app/models/user.rb

@@ -14,6 +14,7 @@ class User < ApplicationRecord
   }.freeze
 
   scope :active, -> { where(active: true) }
+  scope :inactive, -> { where(active: false) }
 
   delegate :service_owner?, :partner_organisation?, to: :organisation
 

app/views/shared/users/_table.html.haml

@@ -8,8 +8,6 @@
           = t("table.header.user.email")
         %th.govuk-table__header
           = t("table.header.user.organisation")
-        %th.govuk-table__header
-          = t("table.header.user.active")
         %th.govuk-table__header
           %span.govuk-visually-hidden
             = t("table.header.default.actions")
@@ -20,7 +18,6 @@
           %td.govuk-table__cell= user.name
           %td.govuk-table__cell= user.email
           %td.govuk-table__cell.organisation= user.organisation.name
-          %td.govuk-table__cell= t("form.user.active.#{user.active}")
           %td.govuk-table__cell
             = a11y_action_link(t("default.link.show"), user_path(user), user.name)
             = a11y_action_link(t("default.link.edit"), edit_user_path(user), user.name, ["govuk-!-margin-left-3"])

app/views/users/index.html.haml

@@ -6,8 +6,20 @@
       %h1.govuk-heading-xl
         = t("page_title.users.index")
 
-  .govuk-grid-row
-    .govuk-grid-column-full
       - if policy(User).new?
         = link_to t("page_content.users.button.create"), new_user_path, class: "govuk-button"
-      = render partial: '/shared/users/table', locals: { users: @users }
+
+  .govuk-tabs{ data: { module: "govuk-tabs" } }
+    %h2.govuk-tabs__title
+      = t("page_title.report.index")
+    %ul.govuk-tabs__list{role: "tablist"}
+      - ["active", "inactive"].each do |state|
+        %li.govuk-tabs__list-item{role: "presentation", class: @user_state == state ? "govuk-tabs__list-item--selected" : ""}
+          %a.govuk-tabs__tab{href: users_index_path(user_state: state), role: "tab"}
+            =t("tabs.users.#{state}")
+
+    .govuk-tabs__panel
+
+      .govuk-grid-row
+        .govuk-grid-column-full
+          = render partial: '/shared/users/table', locals: { users: @users }

config/locales/models/user.en.yml

@@ -72,6 +72,10 @@ en:
       index: Users
       new: Create user
       show: User
+  tabs:
+    users:
+      active: Active
+      inactive: Inactive
   activerecord:
     attributes:
       user:

config/routes.rb

@@ -26,7 +26,10 @@
   end
 
   get "home", to: "home#show"
-  resources :users
+
+  get "/users", to: redirect("/users/active")
+  get "/users/:user_state" => "users#index", :constraints => {user_state: /(in)?active/}, :as => "users_index"
+  resources :users, except: [:index]
   resources :activities, only: [:index]
 
   roles = %w[implementing_organisations partner_organisations matched_effort_providers external_income_providers]

spec/controllers/users_controller_spec.rb

@@ -0,0 +1,31 @@
+RSpec.describe UsersController do
+  let(:beis_user) { create(:beis_user) }
+
+  before do
+    allow(subject).to receive(:current_user).and_return(beis_user)
+  end
+
+  describe "#index" do
+    it "accepts a parameter of `active` and renders" do
+      get :index, params: {user_state: "active"}
+
+      expect(response).to have_http_status(200)
+      expect(response).to render_template(:index)
+    end
+
+    it "accepts a parameter of `inactive` and renders" do
+      get :index, params: {user_state: "inactive"}
+
+      expect(response).to have_http_status(200)
+      expect(response).to render_template(:index)
+    end
+  end
+
+  describe "#show" do
+    it "renders a show template" do
+      get :show, params: {id: beis_user.id}
+
+      expect(response).to render_template(:show)
+    end
+  end
+end

spec/features/beis_users_can_edit_a_user_spec.rb

@@ -88,7 +88,7 @@
     authenticate!(user: administrator_user)
 
     # Navigate to the users page
-    visit users_path
+    visit users_index_path(user_state: "inactive")
 
     find("tr", text: user.name).click_link("Edit")
 

spec/features/beis_users_can_view_other_users_spec.rb

@@ -33,7 +33,6 @@
       expect(page).to have_content(another_user.name)
       expect(page).to have_content(another_user.email)
       expect(page).to have_content(another_user.organisation.name)
-      expect(page).to have_content(t("form.user.active.true"))
 
       # Navigate to the individual user page
       within(".users") do
@@ -43,7 +42,6 @@
       expect(page).to have_content(t("page_title.users.show"))
       expect(page).to have_content(another_user.name)
       expect(page).to have_content(another_user.email)
-      expect(page).to have_content("Active? Yes")
       expect(page).to have_content("Mobile number confirmed for authentication? Yes")
     end
 
@@ -77,9 +75,8 @@
       # Navigate from the landing page
       visit organisation_path(user.organisation)
       click_on(t("page_title.users.index"))
-
-      # The details include whether the user is active
-      expect(page).to have_content(t("form.user.active.false"))
+      # Navigate to inactive users tab
+      click_on(t("tabs.users.inactive"))
 
       # Navigate to the individual user page
       within(".users") do

spec/models/user_spec.rb

@@ -38,6 +38,26 @@
     it { is_expected.to delegate_method(:partner_organisation?).to(:organisation) }
   end
 
+  describe "scopes" do
+    it "shows active users for active scope" do
+      create(:administrator, active: true)
+      create(:administrator, active: true)
+      create(:administrator, active: false)
+
+      expect(User.active.size).to eq(2)
+      expect(User.active.last.active).to eq(true)
+    end
+
+    it "shows inactive users for inactive scope" do
+      create(:administrator, active: true)
+      create(:administrator, active: true)
+      create(:administrator, active: false)
+
+      expect(User.inactive.size).to eq(1)
+      expect(User.inactive.last.active).to eq(false)
+    end
+  end
+
   it "validates the email format" do
     user = build(:administrator, email: "bogus")
 

spec/requests/user_spec.rb

@@ -0,0 +1,16 @@
+require "rails_helper"
+
+RSpec.describe "Users", type: :request do
+  let(:beis_user) { create(:beis_user) }
+
+  before do
+    login_as(beis_user)
+  end
+
+  it "redirects /users with no parameter to /users/active" do
+    expect(get("/users")).to redirect_to("/users/active")
+    expect(response).to have_http_status(301)
+  end
+
+  after { logout }
+end