Extract user policy

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

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

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),

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) }
 

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