Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CPDNPQ-2076] Admin Management #1781

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,9 @@ def structure
prefix: "/npq-separation/admin/lead_providers",
) => [],
Node.new(
name: "Settings",
href: "#",
prefix: "/npq-separation/admin/settings",
name: "Admins",
href: npq_separation_admin_admins_path,
prefix: "/npq-separation/admin/admins",
) => [],
}
end
Expand Down
62 changes: 60 additions & 2 deletions app/controllers/npq_separation/admin/admins_controller.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,61 @@
class NpqSeparation::Admin::AdminsController < ApplicationController
def index = head(:method_not_allowed)
class NpqSeparation::Admin::AdminsController < NpqSeparation::AdminController
before_action :require_super_admin

def index
@admins = Admin.all
end

def new
@admin = Admin.new
end

def create
@admin = Admin.new(admin_params)

if @admin.save
flash[:success] = t(".success", email: @admin.email)
redirect_to action: :index
else
render :new, status: :unprocessable_entity
end
end

def update
@admin = Admin.find(params[:id])

if [email protected]_admin? && @admin.update(super_admin: true)
flash[:success] = t(".success", email: @admin.email)
else
flash[:error] = t(".failure", email: @admin.email)
end

redirect_to action: :index
end

def destroy
@admin = Admin.find(params[:id])

if @admin.super_admin?
flash[:error] = t(".forbidden")
elsif @admin.destroy
flash[:success] = t(".success", email: @admin.email)
else
flash[:error] = t(".failure", email: @admin.email)
end

redirect_to action: :index
end

private

def require_super_admin
unless current_admin.super_admin?
flash[:negative] = { title: "Unauthorized", text: "Sign in with your admininstrator account" }
redirect_to sign_in_path
end
end

def admin_params
params.require(:admin).permit(:email, :full_name)
end
end
12 changes: 12 additions & 0 deletions app/helpers/admin_management_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,16 @@ def elevate_to_super_admin_cell_contents(user)

govuk_link_to(t(".buttons.elevate"), admin_super_admin_path(user), method: :patch)
end

def npq_separation_destroy_admin_cell_contents(admin)
return "" if admin.super_admin?

govuk_button_link_to(t(".buttons.delete"), npq_separation_admin_admin_path(admin), method: :delete, warning: true)
end

def npq_separation_elevate_to_super_admin_cell_contents(admin)
return "" if admin.super_admin?

govuk_link_to(t(".buttons.elevate"), npq_separation_admin_admin_path(admin), method: :patch)
end
end
3 changes: 2 additions & 1 deletion app/models/admin.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ class Admin < ApplicationRecord

validates :email,
presence: { message: "Enter an email address" },
length: { maximum: 64, message: "Email must be shorter than 64 characters" }
length: { maximum: 64, message: "Email must be shorter than 64 characters" },
uniqueness: { message: "An admin with this email address already exists" }

# Whether this user has admin access to the feature flagging interface
def flipper_access?
Expand Down
33 changes: 33 additions & 0 deletions app/views/npq_separation/admin/admins/index.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<h1 class="govuk-heading-l">Admins</h1>

<%= govuk_button_link_to("Add new admin", new_npq_separation_admin_admin_path) %>

<%= govuk_details(summary_text: t('.super_admins.summary_text'), text: t('.super_admins.text_html')) %>

<%=
govuk_table do |table|
table.with_head do |head|
head.with_row do |row|
row.with_cell(header: true, text: "Name")
row.with_cell(header: true, text: "Email")
row.with_cell(header: true, text: "")
row.with_cell(header: true, text: "")
row.with_cell(header: true, text: "")
end
end

table.with_body do |body|
@admins.each do |user|
body.with_row do |row|
row.with_cell(text: user.full_name.presence || "-")
row.with_cell(text: user.email.presence || "No Email Found")
row.with_cell(text: admin_type_cell_contents(user))
row.with_cell(text: npq_separation_destroy_admin_cell_contents(user))
row.with_cell(text: npq_separation_elevate_to_super_admin_cell_contents(user))
end
end
end
end
%>

<%== govuk_pagination(pagy: @pagy) %>
22 changes: 22 additions & 0 deletions app/views/npq_separation/admin/admins/new.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<% content_for :before_content do %>
<%= render GovukComponent::BackLinkComponent.new(
text: "Back",
href: :back
) %>
<% end %>

<h1 class="govuk-heading-l"><%= t(".title") %></h1>

<%= form_with model: @admin, url: npq_separation_admin_admins_path do |f| %>
<%= f.govuk_error_summary %>

<%= f.govuk_email_field :email,
label: { text: "Email address" },
width: "three-quarters" %>

<%= f.govuk_email_field :full_name,
label: { text: "Full name" },
width: "three-quarters" %>

<%= f.govuk_submit "Add admin" %>
<% end %>
19 changes: 17 additions & 2 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -468,7 +468,7 @@ en:
update:
success: Super admin permissions granted to %{email}
failure: Failed to grant super admin permissions to %{email}, please contact technical support if this problem persists.
admins:
admins: &admins
new:
title: "Add a new admin"
index:
Expand Down Expand Up @@ -575,7 +575,7 @@ en:
not_eligible_for_funding: "If your provider does not confirm you've started the course before %{date}, your registration will expire. You can register again later."

helpers:
warnings:
warnings:
registration_wizard:
check_provider_is_open: "Before selecting your provider, you must check that they are currently open to accepting NPQ applications."
title:
Expand Down Expand Up @@ -807,3 +807,18 @@ en:
national_insurance_number: "National Insurance number (optional)"

date_of_birth: What is your date of birth?

npq_separation:
admin:
admins:
<<: *admins
create:
success: Created admin %{email}
failure: Failed to create admin for %{email}, please contact technical support if this problem persists.
update:
success: Super admin permissions granted to %{email}
failure: Failed to grant super admin permissions to %{email}, please contact technical support if this problem persists.
destroy:
forbidden: You cannot remove admin permissions from yourself or another super admin.
success: Deleted admin %{email}
failure: Failed to remove admin permissions from %{email}, please contact technical support if this problem persists.
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@
end

resources :lead_providers, only: %i[index show], path: "lead-providers"
resources :admins, only: %i[index]
resources :admins, except: :edit
Copy link
Member

@peteryates peteryates Oct 4, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think referring to 'admins' inside the admin section could possibly be a bit confusing. As these people will be DfE staff, maybe /admin/staff?

Suggested change
resources :admins, except: :edit
resources :admins, except: :edit, path: "staff"

end
end

Expand Down
5 changes: 5 additions & 0 deletions db/migrate/20241001141442_add_unique_index_to_admins.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddUniqueIndexToAdmins < ActiveRecord::Migration[7.1]
def change
add_index :admins, :email, unique: true
end
end
3 changes: 2 additions & 1 deletion db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.1].define(version: 2024_09_18_145749) do
ActiveRecord::Schema[7.1].define(version: 2024_10_01_141442) do
# These are extensions that must be enabled in order to support this database
enable_extension "btree_gin"
enable_extension "citext"
Expand Down Expand Up @@ -39,6 +39,7 @@
t.datetime "otp_expires_at", precision: nil
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.index ["email"], name: "index_admins_on_email", unique: true
end

create_table "api_tokens", force: :cascade do |t|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"Finance" => "/npq-separation/admin/finance/statements",
"Schools" => "/npq-separation/admin/schools",
"Lead providers" => "/npq-separation/admin/lead-providers",
"Settings" => "#",
"Admins" => "/npq-separation/admin/admins",
}.each_with_index do |(name, href), i|
it "#{name} with href #{href} is at position #{i + 1}" do
expect(subject[i].name).to eql(name)
Expand Down
101 changes: 98 additions & 3 deletions spec/controllers/npq_separation/admin/admins_controller_spec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,104 @@
require "rails_helper"

RSpec.describe NpqSeparation::Admin::AdminsController, type: :request do
describe("index") do
before { get(npq_separation_admin_admins_path) }
include Helpers::NPQSeparationAdminLogin

specify { expect(response).to(be_method_not_allowed) }
let!(:admin) { create :admin }
let!(:super_admin) { create :super_admin }

shared_examples "unauthorised" do
specify "index is inaccessible" do
get npq_separation_admin_admins_path
expect(response).to redirect_to(sign_in_path)
end

specify "new is inaccessible" do
get new_npq_separation_admin_admin_path
expect(response).to redirect_to(sign_in_path)
end

specify "create is inaccessible" do
post npq_separation_admin_admins_path
expect(response).to redirect_to(sign_in_path)
end

specify "update is inaccessible" do
patch npq_separation_admin_admin_path(0)
expect(response).to redirect_to(sign_in_path)
end

it "is inaccessible" do
delete npq_separation_admin_admin_path(0)
expect(response).to redirect_to(sign_in_path)
end
end

context "when not signed in" do
include_examples "unauthorised"
end

context "when signed in as a normal admin" do
before { sign_in_as_admin }

include_examples "unauthorised"
end

context "when signed in as a super admin" do
before { sign_in_as_admin super_admin: true }

specify "index is successful" do
get npq_separation_admin_admins_path
expect(response).to have_http_status(:success)
end

specify "new is successful" do
get new_npq_separation_admin_admin_path
expect(response).to have_http_status(:success)
end

specify "create is successful" do
expect {
post npq_separation_admin_admins_path, params: { admin: { email: "[email protected]", full_name: "Name" } }
}.to change(Admin, :count).by(1)

expect(response).to redirect_to(npq_separation_admin_admins_path)
expect(flash[:success]).to be_present
end

specify "update updates regular admin to super admin" do
expect {
patch npq_separation_admin_admin_path(admin)
}.to change { admin.reload.super_admin? }.from(false).to(true)

expect(response).to redirect_to(npq_separation_admin_admins_path)
expect(flash[:success]).to be_present
end

specify "update does not change super admins" do
expect {
patch npq_separation_admin_admin_path(super_admin)
}.not_to(change { super_admin.reload.super_admin? })

expect(response).to redirect_to(npq_separation_admin_admins_path)
expect(flash[:error]).to be_present
end

specify "destroy deletes regular admins" do
expect {
delete npq_separation_admin_admin_path(admin)
}.to change(Admin, :count).by(-1)

expect(response).to redirect_to(npq_separation_admin_admins_path)
expect(flash[:success]).to be_present
end

specify "destroy does nothing for super admins" do
expect {
delete npq_separation_admin_admin_path(super_admin)
}.not_to change(Admin, :count)

expect(response).to redirect_to(npq_separation_admin_admins_path)
expect(flash[:error]).to be_present
end
end
end
Loading
Loading