Skip to content

Commit

Permalink
User role cannot be changed (#340)
Browse files Browse the repository at this point in the history
  • Loading branch information
timpeat authored Jul 7, 2023
1 parent 7e650ca commit 55f0fa5
Show file tree
Hide file tree
Showing 14 changed files with 19 additions and 244 deletions.
30 changes: 1 addition & 29 deletions app/controllers/admin/manage_users/active_users_controller.rb
Original file line number Diff line number Diff line change
@@ -1,36 +1,8 @@
module Admin
module ManageUsers
class ActiveUsersController < ManageUsersController
before_action :set_user, only: [:edit, :update]

def index
@users = user_scope.order(
first_name: :asc, last_name: :asc
).page params[:page]
end

def edit; end

def update
can_manage_others = params[:can_manage_others] ? true : false

if @user.allow_admin_right_change?(can_manage_others) && @user.update(can_manage_others:)
set_flash(:user_updated, user_name: @user.name)
redirect_to admin_manage_users_root_path
else
set_flash(:deactivation_denied, success: false)
redirect_to edit_admin_manage_users_active_user_path(@user)
end
end

private

def user_scope
User.active
end

def set_user
@user = user_scope.find(params[:id])
@users = User.active.order(first_name: :asc, last_name: :asc).page params[:page]
end
end
end
Expand Down
8 changes: 1 addition & 7 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class CannotReactivate < StandardError; end
raise CannotDestroyIfActive if activated?
end

attr_readonly :email
attr_readonly :email, :can_manage_others

validates :email, uniqueness: true, on: :create
validates :email, presence: true, format: { with: URI::MailTo::EMAIL_REGEXP }
Expand Down Expand Up @@ -118,12 +118,6 @@ def inactive_message
end
end

def allow_admin_right_change?(new_can_manage_others_value)
return true if new_can_manage_others_value

deactivatable?
end

class << self
#
# For GDPR caseworker personal data is not stored in the event stream.
Expand Down
3 changes: 0 additions & 3 deletions app/views/admin/manage_users/active_users/_actions.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@
<%= govuk_link_to action_text(:revive), '#', { no_visited_state: true } %>
</li>
<% else %>
<li class="govuk-summary-list__actions-list-item">
<%= govuk_link_to action_text(:edit), edit_admin_manage_users_active_user_path(user), { no_visited_state: true } %>
</li>
<li class="govuk-summary-list__actions-list-item">
<%= govuk_link_to action_text(:deactivate), new_admin_manage_users_deactivated_user_path(id: user), { no_visited_state: true } %>
</li>
Expand Down
33 changes: 0 additions & 33 deletions app/views/admin/manage_users/active_users/edit.html.erb

This file was deleted.

2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
namespace :admin do
namespace :manage_users do
root 'active_users#index'
resources :active_users, only: [:index, :edit, :update]
resources :active_users, only: [:index]
resources :history, only: [:show], controller: :history

resources :invitations, only: [:index, :new, :destroy, :create, :update] do
Expand Down
4 changes: 1 addition & 3 deletions spec/shared_contexts/when_logged_in_user_is_admin.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
RSpec.shared_context 'when logged in user is admin', shared_context: :metadata do
before do
current_user.update(can_manage_others: true)
end
let(:current_user_can_manage_others) { true }
end
5 changes: 4 additions & 1 deletion spec/shared_contexts/with_a_logged_in_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,13 @@
last_name: 'EXAMPLE',
auth_subject_id: current_user_auth_subject_id,
first_auth_at: 1.month.ago,
last_auth_at: 1.hour.ago
last_auth_at: 1.hour.ago,
can_manage_others: current_user_can_manage_others
)
end

let(:current_user_can_manage_others) { false }

let(:current_user_id) { current_user.id }

let(:current_user_auth_subject_id) { SecureRandom.uuid }
Expand Down
5 changes: 4 additions & 1 deletion spec/shared_contexts/with_an_existing_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@
first_name: 'Zoe',
last_name: 'Blogs',
email: '[email protected]',
auth_subject_id: SecureRandom.uuid
auth_subject_id: SecureRandom.uuid,
can_manage_others: user_can_manage_others
)
end

let(:user_can_manage_others) { false }

let(:user_row) do
find(
:xpath,
Expand Down
5 changes: 0 additions & 5 deletions spec/system/accessibility_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,11 +67,6 @@
expect(page).to be_axe_clean.according_to accessibility_standards
end

it 'edit user page has no axe detectible accessibility issues' do
visit edit_admin_manage_users_active_user_path(active_user)
expect(page).to be_axe_clean.according_to accessibility_standards
end

it 'reassign crime application page has no axe detectible accessibility issues' do
visit new_crime_application_reassign_path(crime_application_id)
expect(page).to be_axe_clean.according_to accessibility_standards
Expand Down
6 changes: 3 additions & 3 deletions spec/system/authenticating/an_invited_user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,14 @@
end

describe 'viewing the activation in the user\'s account history' do
let(:invited_user) { User.create(email: '[email protected]', can_manage_others: true) }
let(:cells) { page.first('table tbody tr').all('td') }

before do
invited_user.update(can_manage_others: true)
click_on 'Sign in'
click_on 'Invited Test'
end

let(:cells) { page.first('table tbody tr').all('td') }

it 'describes the event' do
expect(cells[1]).to have_content 'Account activated'
end
Expand Down
2 changes: 1 addition & 1 deletion spec/system/header_nav_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@
end

context 'when a user has access to manage other users' do
include_context 'when logged in user is admin'
before do
User.update(current_user_id, can_manage_others: true)
visit admin_manage_users_root_path
end

Expand Down
3 changes: 2 additions & 1 deletion spec/system/manage_users/deactivate_a_user_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,10 @@ def do_deactivate_journey
end

describe 'with only 2 active admins' do
let(:user_can_manage_others) { true }

before do
User.create!(can_manage_others: true, deactivated_at: Time.zone.now, email: '[email protected]') # Inactive
active_user.update(can_manage_others: true)

do_deactivate_journey
click_on('Yes, deactivate')
Expand Down
154 changes: 0 additions & 154 deletions spec/system/manage_users/edit_user_spec.rb

This file was deleted.

3 changes: 1 addition & 2 deletions spec/system/manage_users/viewing_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,11 @@
end

context 'with pagination' do
include_context 'when logged in user is admin'
let(:last_auth_at) { Time.zone.now }

before do
make_users(100)
visit '/'
User.update(current_user_id, can_manage_others: true, last_auth_at: last_auth_at)
visit '/admin/manage_users?page=2'
end

Expand Down

0 comments on commit 55f0fa5

Please sign in to comment.