Skip to content

Commit

Permalink
Return Role class from User#role
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisroos committed Jan 25, 2024
1 parent 65751ca commit 006a3c8
Show file tree
Hide file tree
Showing 19 changed files with 42 additions and 38 deletions.
2 changes: 1 addition & 1 deletion app/controllers/api_users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def api_user_params_for_create
def sanitise(permitted_user_params)
UserParameterSanitiser.new(
user_params: permitted_user_params.to_h,
current_user_role: current_user.role.to_sym,
current_user_role: current_user.role.role_name.to_sym,
).sanitise
end

Expand Down
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def export
def user_params
UserParameterSanitiser.new(
user_params: params.require(:user).permit(:require_2sv).to_h,
current_user_role: current_user.role.to_sym,
current_user_role: current_user.role.role_name.to_sym,
).sanitise
end

Expand Down
2 changes: 1 addition & 1 deletion app/helpers/users_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ def summary_list_item_for_organisation(user)
end

def summary_list_item_for_role(user)
item = { field: "Role", value: user.role.humanize.capitalize }
item = { field: "Role", value: user.role.role_name.humanize.capitalize }
item[:edit] = { href: edit_user_role_path(user) } if policy(user).assign_role?
item
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/batch_invitation_user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ def invite_user_with_attributes(sanitised_attributes, inviting_user)
def sanitise_attributes_for_inviting_user_role(raw_attributes, inviting_user)
UserParameterSanitiser.new(
user_params: raw_attributes,
current_user_role: inviting_user.role.to_sym,
current_user_role: inviting_user.role.role_name.to_sym,
).sanitise
end

Expand Down
18 changes: 11 additions & 7 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ class User < ApplicationRecord
:confirmable,
:password_archivable # in signon/lib/devise/models/password_archivable.rb

delegate :manageable_roles, to: :role_class
delegate :manageable_roles, to: :role

encrypts :otp_secret_key

validates :name, presence: true
validates :email, reject_non_governmental_email_addresses: true
validates :reason_for_suspension, presence: true, if: proc { |u| u.suspended? }
validates :role, inclusion: { in: Roles.names }
validates :role, inclusion: { in: Roles.all }
validate :user_can_be_exempted_from_2sv
validate :organisation_admin_belongs_to_organisation
validate :email_is_ascii_only
Expand Down Expand Up @@ -127,6 +127,10 @@ def self.with_default_permissions
new(supported_permissions: SupportedPermission.default)
end

def role
Roles.find(self[:role])
end

def require_2sv?
return require_2sv unless organisation

Expand Down Expand Up @@ -306,11 +310,11 @@ def not_setup_2sv?
end

def can_manage?(other_user)
role_class.can_manage?(other_user.role)
role.can_manage?(other_user.role)
end

def manageable_organisations
role_class.manageable_organisations_for(self).order(:name)
role.manageable_organisations_for(self).order(:name)
end

# Make devise send all emails using ActiveJob
Expand All @@ -325,7 +329,7 @@ def need_two_step_verification?
def set_2sv_for_admin_roles
return unless GovukEnvironment.production?

self.require_2sv = true if role_changed? && role_class.require_2sv?
self.require_2sv = true if role_changed? && role.require_2sv?
end

def reset_2sv_exemption_reason
Expand Down Expand Up @@ -424,12 +428,12 @@ def mark_two_step_mandated_changed
end

def user_can_be_exempted_from_2sv
errors.add(:reason_for_2sv_exemption, "#{role} users cannot be exempted from 2SV. Remove the user's exemption to change their role.") if exempt_from_2sv? && role_class.require_2sv?
errors.add(:reason_for_2sv_exemption, "#{role.role_name} users cannot be exempted from 2SV. Remove the user's exemption to change their role.") if exempt_from_2sv? && role.require_2sv?
end

def organisation_admin_belongs_to_organisation
if publishing_manager? && organisation_id.blank?
errors.add(:organisation_id, "can't be 'None' for #{role.titleize}")
errors.add(:organisation_id, "can't be 'None' for #{role.role_name.titleize}")
end
end

Expand Down
2 changes: 1 addition & 1 deletion app/policies/user_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ def new?
alias_method :create?, :new?

def edit?
case current_user.role
case current_user.role.role_name
when Roles::Superadmin.role_name
true
when Roles::Admin.role_name
Expand Down
2 changes: 1 addition & 1 deletion app/presenters/user_export_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def row(user)
[
user.name,
user.email,
user.role.humanize,
user.role.role_name.humanize,
user.organisation.try(:name),
user.sign_in_count,
user.current_sign_in_at.try(:to_formatted_s, :db),
Expand Down
2 changes: 1 addition & 1 deletion app/views/account/roles/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@
<% end %>
<% else %>
<%= render "govuk_publishing_components/components/inset_text", {
text: current_user.role.humanize,
text: current_user.role.role_name.humanize,
} %>
<% end %>
</div>
Expand Down
2 changes: 1 addition & 1 deletion app/views/devise/invitations/new.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@
name: "user[role]",
label: "Role",
hint: user_role_select_hint,
options: options_for_role_select(selected: f.object.role),
options: options_for_role_select(selected: f.object.role.role_name),
} %>
<% end %>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
[
{ text: formatted_user_name(user), format: user_name_format(user) },
{ text: user.email, format: 'email' },
{ text: user.role.humanize },
{ text: user.role.role_name.humanize },
{ text: user.organisation_name },
{ text: user.sign_in_count },
{ text: formatted_last_sign_in(user) },
Expand Down
2 changes: 1 addition & 1 deletion app/views/users/index.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
[
{ text: user_name(user) },
{ text: user.email },
{ text: user.role.humanize },
{ text: user.role.role_name.humanize },
{ text: status(user) },
{ text: two_step_status(user) },
]
Expand Down
4 changes: 2 additions & 2 deletions app/views/users/roles/edit.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,15 @@
<%= form_for @user, url: user_role_path(@user) do %>
<% if @user.exempt_from_2sv? %>
<%= render "govuk_publishing_components/components/inset_text", {
text: "This user's role is set to #{@user.role.humanize}. They are currently exempted from 2-step verification, meaning that their role cannot be changed as admins are required to have 2-step verification.",
text: "This user's role is set to #{@user.role.role_name.humanize}. They are currently exempted from 2-step verification, meaning that their role cannot be changed as admins are required to have 2-step verification.",
} %>
<% else %>
<%= render "govuk_publishing_components/components/select", {
id: "user_role",
name: "user[role]",
label: "Role",
hint: user_role_select_hint,
options: current_user.manageable_roles.map { |role| { text: role.humanize, value: role, selected: @user.role == role } },
options: current_user.manageable_roles.map { |role| { text: role.humanize, value: role, selected: @user.role&.role_name == role } },
error_message: @user.errors[:role].any? ? @user.errors.full_messages_for(:role).to_sentence : nil
} %>
<div class="govuk-button-group">
Expand Down
2 changes: 1 addition & 1 deletion lib/roles.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ def role_class

all.each do |role_class|
define_method("#{role_class.role_name}?") do
role == role_class.role_name
role == role_class
end
end

Expand Down
2 changes: 1 addition & 1 deletion lib/roles/base.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
module Roles
class Base
def self.can_manage?(other_role)
manageable_roles.include?(other_role)
manageable_roles.include?(other_role.role_name)
end
end
end
4 changes: 2 additions & 2 deletions test/controllers/invitations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ class InvitationsControllerTest < ActionController::TestCase
assert_equal "invitee", invitee.name
assert_equal "invitee@gov.uk", invitee.email
assert_equal @organisation, invitee.organisation
assert_equal Roles::OrganisationAdmin.role_name, invitee.role
assert_equal Roles::OrganisationAdmin, invitee.role
assert_equal [permission], invitee.supported_permissions
end

Expand Down Expand Up @@ -439,7 +439,7 @@ class InvitationsControllerTest < ActionController::TestCase

invitee = User.last
assert invitee.present?
default_role = Roles::Normal.role_name
default_role = Roles::Normal
assert_equal default_role, invitee.role
end
end
Expand Down
8 changes: 4 additions & 4 deletions test/controllers/users/roles_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ class Users::RolesControllerTest < ActionController::TestCase

put :update, params: { user_id: user, user: { role: Roles::OrganisationAdmin.role_name } }

assert_equal Roles::OrganisationAdmin.role_name, user.reload.role
assert_equal Roles::OrganisationAdmin, user.reload.role
end

should "record account updated & role change events" do
Expand Down Expand Up @@ -154,7 +154,7 @@ class Users::RolesControllerTest < ActionController::TestCase

put :update, params: { user_id: user, user: { role: Roles::OrganisationAdmin.role_name } }

assert_equal Roles::OrganisationAdmin.role_name, user.reload.role
assert_equal Roles::OrganisationAdmin, user.reload.role
end

should "not update user role if UserPolicy#assign_role? returns false" do
Expand All @@ -164,7 +164,7 @@ class Users::RolesControllerTest < ActionController::TestCase

put :update, params: { user_id: user, user: { role: Roles::OrganisationAdmin.role_name } }

assert_equal Roles::Normal.role_name, user.reload.role
assert_equal Roles::Normal, user.reload.role
assert_not_authorised
end

Expand All @@ -173,7 +173,7 @@ class Users::RolesControllerTest < ActionController::TestCase

put :update, params: { user_id: user, user: { role: Roles::OrganisationAdmin.role_name } }

assert_equal Roles::Normal.role_name, user.reload.role
assert_equal Roles::Normal, user.reload.role

assert_select ".govuk-error-summary" do
assert_select "li", text: /#{Roles::OrganisationAdmin.role_name} users cannot be exempted from 2SV/
Expand Down
4 changes: 2 additions & 2 deletions test/integration/change_user_role_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def sign_in_as_and_edit_user(sign_in_as, user_to_edit)

assert page.has_no_select?("Role")

assert page.has_text? "This user's role is set to #{user.role.humanize}. They are currently exempted from 2-step verification, meaning that their role cannot be changed as admins are required to have 2-step verification."
assert page.has_text? "This user's role is set to #{user.role.role_name.humanize}. They are currently exempted from 2-step verification, meaning that their role cannot be changed as admins are required to have 2-step verification."
end
end

Expand All @@ -41,7 +41,7 @@ def sign_in_as_and_edit_user(sign_in_as, user_to_edit)
sign_in_as_and_edit_user(create(:admin_user, organisation: user.organisation), user)

assert page.has_no_select?("Role")
assert page.has_no_text? "This user's role is set to #{user.role}. They are currently exempted from 2sv, meaning that their role cannot be changed as admins are required to have 2sv."
assert page.has_no_text? "This user's role is set to #{user.role.role_name.humanize}. They are currently exempted from 2sv, meaning that their role cannot be changed as admins are required to have 2sv."
end
end
end
6 changes: 3 additions & 3 deletions test/lib/tasks/permissions_promoter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,15 @@ class PermissionsPromoterTest < ActiveSupport::TestCase

@task.invoke

assert non_gds_user.reload.role == "normal"
assert non_gds_user.reload.role == Roles::Normal
end

should "not update a non-GDS user who already has an admin role" do
admin_user = user_with_permissions(:admin_user, @org, { @first_app => ["Managing Editor"] })

@task.invoke

assert admin_user.reload.role == Roles::Admin.role_name
assert admin_user.reload.role == Roles::Admin
end

should "not update a non-GDS user who is suspended" do
Expand All @@ -53,7 +53,7 @@ class PermissionsPromoterTest < ActiveSupport::TestCase

@task.invoke

assert user.reload.role == "normal"
assert user.reload.role == Roles::Normal
end

should "not update GDS users" do
Expand Down
12 changes: 6 additions & 6 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -858,13 +858,13 @@ def setup
end
end

context "#role_class" do
context "#role" do
should "return the role class" do
assert_equal Roles::Normal, build(:user).role_class
assert_equal Roles::OrganisationAdmin, build(:organisation_admin_user).role_class
assert_equal Roles::SuperOrganisationAdmin, build(:super_organisation_admin_user).role_class
assert_equal Roles::Admin, build(:admin_user).role_class
assert_equal Roles::Superadmin, build(:superadmin_user).role_class
assert_equal Roles::Normal, build(:user).role
assert_equal Roles::OrganisationAdmin, build(:organisation_admin_user).role
assert_equal Roles::SuperOrganisationAdmin, build(:super_organisation_admin_user).role
assert_equal Roles::Admin, build(:admin_user).role
assert_equal Roles::Superadmin, build(:superadmin_user).role
end
end

Expand Down

0 comments on commit 006a3c8

Please sign in to comment.