diff --git a/app/helpers/user_helper.rb b/app/helpers/user_helper.rb index d6eebafee..76fb78f57 100644 --- a/app/helpers/user_helper.rb +++ b/app/helpers/user_helper.rb @@ -7,35 +7,7 @@ module UserHelper current_user == user ? "Are you" : "Is this person" end - def can_edit_names?(user, current_user) - (current_user == user || current_user.data_coordinator? || current_user.support?) && user.active? - end - - def can_edit_emails?(user, current_user) - (current_user == user || current_user.data_coordinator? || current_user.support?) && user.active? - end - - def can_edit_password?(user, current_user) - current_user == user - end - - def can_edit_roles?(user, current_user) - (current_user.data_coordinator? || current_user.support?) && user.active? - end - - def can_edit_dpo?(user, current_user) - (current_user.data_coordinator? || current_user.support?) && user.active? - end - - def can_edit_key_contact?(user, current_user) - (current_user.data_coordinator? || current_user.support?) && user.active? - end - def can_edit_org?(current_user) current_user.data_coordinator? || current_user.support? end - - def can_edit_telephone_numbers?(user, current_user) - (current_user == user || current_user.data_coordinator? || current_user.support?) && user.active? - end end diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb new file mode 100644 index 000000000..a4b1a3d5c --- /dev/null +++ b/app/policies/user_policy.rb @@ -0,0 +1,36 @@ +class UserPolicy + attr_reader :current_user, :user + + def initialize(current_user, user) + @current_user = current_user + @user = user + end + + def edit_password? + @current_user == @user + end + + def edit_roles? + (@current_user.data_coordinator? || @current_user.support?) && @user.active? + end + + %w[ + edit_roles? + edit_dpo? + edit_key_contact? + ].each do |method_name| + define_method method_name do + (@current_user.data_coordinator? || @current_user.support?) && @user.active? + end + end + + %w[ + edit_emails? + edit_telephone_numbers? + edit_names? + ].each do |method_name| + define_method method_name do + (@current_user == @user || @current_user.data_coordinator? || @current_user.support?) && @user.active? + end + end +end diff --git a/app/views/users/show.html.erb b/app/views/users/show.html.erb index 20f2e807f..2602d0291 100644 --- a/app/views/users/show.html.erb +++ b/app/views/users/show.html.erb @@ -24,7 +24,7 @@ <%= summary_list.row do |row| row.key { "Name" } row.value { @user.name } - if can_edit_names?(@user, current_user) + if UserPolicy.new(current_user, @user).edit_names? row.action(visually_hidden_text: "name", href: aliased_user_edit(@user, current_user), html_attributes: { "data-qa": "change-name" }) else row.action @@ -34,7 +34,7 @@ <%= summary_list.row do |row| row.key { "Email address" } row.value { @user.email } - if can_edit_emails?(@user, current_user) + if UserPolicy.new(current_user, @user).edit_emails? row.action(visually_hidden_text: "email address", href: aliased_user_edit(@user, current_user), html_attributes: { "data-qa": "change-email-address" }) else row.action @@ -44,7 +44,7 @@ <%= summary_list.row do |row| row.key { "Telephone number" } row.value { @user.phone } - if can_edit_telephone_numbers?(@user, current_user) + if UserPolicy.new(current_user, @user).edit_telephone_numbers? row.action(visually_hidden_text: "telephone number", href: aliased_user_edit(@user, current_user), html_attributes: { "data-qa": "change-telephone-number" }) else row.action @@ -54,7 +54,7 @@ <%= summary_list.row do |row| row.key { "Password" } row.value { "••••••••" } - if can_edit_password?(@user, current_user) + if UserPolicy.new(current_user, @user).edit_password? row.action( visually_hidden_text: "password", href: edit_password_account_path, @@ -74,7 +74,7 @@ <%= summary_list.row do |row| row.key { "Role" } row.value { @user.role&.humanize } - if can_edit_roles?(@user, current_user) + if UserPolicy.new(current_user, @user).edit_roles? row.action( visually_hidden_text: "role", href: aliased_user_edit(@user, current_user), @@ -88,7 +88,7 @@ <%= summary_list.row do |row| row.key { "Data protection officer" } row.value { @user.is_data_protection_officer? ? "Yes" : "No" } - if can_edit_dpo?(@user, current_user) + if UserPolicy.new(current_user, @user).edit_dpo? row.action( visually_hidden_text: "if data protection officer", href: user_edit_dpo_path(@user), @@ -102,7 +102,7 @@ <%= summary_list.row do |row| row.key { "Key contact" } row.value { @user.is_key_contact? ? "Yes" : "No" } - if can_edit_key_contact?(@user, current_user) + if UserPolicy.new(current_user, @user).edit_key_contact? row.action( visually_hidden_text: "if a key contact", href: user_edit_key_contact_path(@user), diff --git a/spec/helpers/user_helper_spec.rb b/spec/helpers/user_helper_spec.rb index e49c7d6a2..829195f6c 100644 --- a/spec/helpers/user_helper_spec.rb +++ b/spec/helpers/user_helper_spec.rb @@ -37,105 +37,6 @@ RSpec.describe UserHelper do end describe "change button permissions" do - context "when the user is a data provider viewing their own details" do - let(:current_user) { FactoryBot.create(:user, :data_provider) } - let(:user) { current_user } - - it "allows changing name" do - expect(can_edit_names?(user, current_user)).to be true - end - - it "allows changing email" do - expect(can_edit_emails?(user, current_user)).to be true - end - - it "allows changing password" do - expect(can_edit_password?(user, current_user)).to be true - end - - it "does not allow changing roles" do - expect(can_edit_roles?(user, current_user)).to be false - end - - it "does not allow changing dpo" do - expect(can_edit_dpo?(user, current_user)).to be false - end - - it "does not allow changing key contact" do - expect(can_edit_key_contact?(user, current_user)).to be false - end - end - - context "when the user is a data coordinator viewing another user's details" do - it "allows changing name" do - expect(can_edit_names?(user, current_user)).to be true - end - - it "allows changing email" do - expect(can_edit_emails?(user, current_user)).to be true - end - - it "allows changing password" do - expect(can_edit_password?(user, current_user)).to be false - end - - it "does not allow changing roles" do - expect(can_edit_roles?(user, current_user)).to be true - end - - it "does not allow changing dpo" do - expect(can_edit_dpo?(user, current_user)).to be true - end - - it "does not allow changing key contact" do - expect(can_edit_key_contact?(user, current_user)).to be true - end - - context "when the user is a data coordinator viewing their own details" do - let(:user) { current_user } - - it "allows changing password" do - expect(can_edit_password?(user, current_user)).to be true - end - end - end - - context "when the user is a support user viewing another user's details" do - let(:current_user) { FactoryBot.create(:user, :support) } - - it "allows changing name" do - expect(can_edit_names?(user, current_user)).to be true - end - - it "allows changing email" do - expect(can_edit_emails?(user, current_user)).to be true - end - - it "allows changing password" do - expect(can_edit_password?(user, current_user)).to be false - end - - it "does not allow changing roles" do - expect(can_edit_roles?(user, current_user)).to be true - end - - it "does not allow changing dpo" do - expect(can_edit_dpo?(user, current_user)).to be true - end - - it "does not allow changing key contact" do - expect(can_edit_key_contact?(user, current_user)).to be true - end - - context "when the user is a support user viewing their own details" do - let(:user) { current_user } - - it "allows changing password" do - expect(can_edit_password?(user, current_user)).to be true - end - end - end - context "when the user is a data provider viewing organisation details" do let(:current_user) { FactoryBot.create(:user, :data_provider) } diff --git a/spec/policies/user_policy_spec.rb b/spec/policies/user_policy_spec.rb new file mode 100644 index 000000000..ec84fbceb --- /dev/null +++ b/spec/policies/user_policy_spec.rb @@ -0,0 +1,103 @@ +require "rails_helper" +# rubocop:disable RSpec/RepeatedExample + +RSpec.describe UserPolicy do + subject(:policy) { described_class } + + let(:data_provider) { FactoryBot.create(:user, :data_provider) } + let(:data_coordinator) { FactoryBot.create(:user, :data_coordinator) } + let(:support) { FactoryBot.create(:user, :support) } + + permissions :edit_names? do + it "allows changing their own name" do + expect(policy).to permit(data_provider, data_provider) + end + + it "as a coordinator it allows changing other user's name" do + expect(policy).to permit(data_coordinator, data_provider) + end + + it "as a support user it allows changing other user's name" do + expect(policy).to permit(support, data_provider) + end + end + + permissions :edit_emails? do + it "allows changing their own email" do + expect(policy).to permit(data_provider, data_provider) + end + + it "as a coordinator it allows changing other user's email" do + expect(policy).to permit(data_coordinator, data_provider) + end + + it "as a support user it allows changing other user's email" do + expect(policy).to permit(support, data_provider) + end + end + + permissions :edit_password? do + it "as a provider it allows changing their own password" do + expect(policy).to permit(data_provider, data_provider) + end + + it "as a coordinator it allows changing their own password" do + expect(policy).to permit(data_coordinator, data_coordinator) + end + + it "as a support user it allows changing their own password" do + expect(policy).to permit(support, support) + end + + it "as a coordinator it does not allow changing other user's password" do + expect(policy).not_to permit(data_coordinator, data_provider) + end + + it "as a support user it does not allow changing other user's password" do + expect(policy).not_to permit(support, data_provider) + end + end + + permissions :edit_roles? do + it "as a provider it does not allow changing roles" do + expect(policy).not_to permit(data_provider, data_provider) + end + + it "as a coordinator allows changing other user's roles" do + expect(policy).to permit(data_coordinator, data_provider) + end + + it "as a support user allows changing other user's roles" do + expect(policy).to permit(support, data_provider) + end + end + + permissions :edit_dpo? do + it "as a provider it does not allow changing dpo" do + expect(policy).not_to permit(data_provider, data_provider) + end + + it "as a coordinator allows changing other user's dpo" do + expect(policy).to permit(data_coordinator, data_provider) + end + + it "as a support user allows changing other user's dpo" do + expect(policy).to permit(support, data_provider) + end + end + + permissions :edit_key_contact? do + it "as a provider it does not allow changing key_contact" do + expect(policy).not_to permit(data_provider, data_provider) + end + + it "as a coordinator allows changing other user's key_contact" do + expect(policy).to permit(data_coordinator, data_provider) + end + + it "as a support user allows changing other user's key_contact" do + expect(policy).to permit(support, data_provider) + end + end +end +# rubocop:enable RSpec/RepeatedExample