Allow changing DPO

app/controllers/users_controller.rb

@@ -7,8 +7,10 @@ class UsersController < ApplicationController
 
   def update
     if @user.update(user_params)
-      bypass_sign_in @user
-      flash[:notice] = I18n.t("devise.passwords.updated") if user_params.key?("password")
+      if @user == current_user
+        bypass_sign_in @user
+        flash[:notice] = I18n.t("devise.passwords.updated") if user_params.key?("password")
+      end
       redirect_to user_path(@user)
     elsif user_params.key?("password")
       format_error_messages
@@ -74,9 +76,9 @@ private
 
   def user_params
     if @user == current_user
-      params.require(:user).permit(:email, :name, :password, :password_confirmation, :role)
+      params.require(:user).permit(:email, :name, :password, :password_confirmation, :role, :is_dpo)
     else
-      params.require(:user).permit(:email, :name, :role)
+      params.require(:user).permit(:email, :name, :role, :is_dpo)
     end
   end
 

app/views/users/edit.html.erb

@@ -1,4 +1,4 @@
-<% content_for :title, current_user == @user ? "Change your personal details" : "Change #{@user.name}'s personal details" %>
+<% content_for :title, current_user == @user ? "Change your personal details" : "Change #{@user.name}’s personal details" %>
 
 <% content_for :before_content do %>
   <%= govuk_back_link(
@@ -7,7 +7,7 @@
   ) %>
 <% end %>
 
-<%= form_for(current_user, as: :user, html: { method: :patch }) do |f| %>
+<%= form_for(@user, as: :user, html: { method: :patch }) do |f| %>
   <div class="govuk-grid-row">
     <div class="govuk-grid-column-two-thirds">
       <%= f.govuk_error_summary %>
@@ -26,6 +26,10 @@
         spellcheck: "false"
       %>
 
+      <%= f.govuk_check_boxes_fieldset :is_dpo, multiple: false, legend: { text: "Are they a data protection officer?", size: "m" } do %>
+        <%= f.govuk_check_box :is_dpo, 1, 0, multiple: false, link_errors: true, label: { text: "Yes" } %>
+      <% end %>
+
       <%= f.govuk_submit "Save changes" %>
     </div>
   </div>

app/views/users/new.html.erb

@@ -31,6 +31,10 @@
         f.govuk_collection_radio_buttons :role, roles, :id, :name, legend: { text: "Role", size: "m" }
       %>
 
+      <%= f.govuk_check_boxes_fieldset :is_dpo, legend: { text: "Are they a data protection officer?", size: "m" } do %>
+        <%= f.govuk_check_box :is_dpo, :is_dpo, label: { text: "Yes" } %>
+      <% end %>
+
       <%= f.govuk_submit "Continue" %>
     </div>
   </div>

app/views/users/show.html.erb

@@ -1,4 +1,4 @@
-<% content_for :title, current_user == @user ? "Your account" : "#{@user.name}'s account" %>
+<% content_for :title, current_user == @user ? "Your account" : "#{@user.name}’s account" %>
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
@@ -13,33 +13,43 @@
     <%= govuk_summary_list do |summary_list| %>
       <%= summary_list.row do |row|
         row.key { 'Name' }
-        row.value { current_user.name }
+        row.value { @user.name }
         row.action(visually_hidden_text: 'name', href: edit_user_path, html_attributes: { 'data-qa': 'change-name' })
       end %>
 
       <%= summary_list.row() do |row|
         row.key { 'Email address' }
-        row.value { current_user.email }
+        row.value { @user.email }
         row.action(visually_hidden_text: 'email address', href: edit_user_path, html_attributes: { 'data-qa': 'change-email' })
       end %>
 
       <%= summary_list.row do |row|
         row.key { 'Password' }
         row.value { '••••••••' }
-        row.action(visually_hidden_text: 'password', href: password_edit_user_path, html_attributes: { 'data-qa': 'change-password' })
+        if current_user == @user
+          row.action(visually_hidden_text: 'password', href: password_edit_user_path, html_attributes: { 'data-qa': 'change-password' })
+        else
+          row.action()
+        end
       end %>
 
       <%= summary_list.row do |row|
         row.key { 'Organisation' }
-        row.value { current_user.organisation.name }
+        row.value { @user.organisation.name }
         row.action()
       end %>
 
       <%= summary_list.row do |row|
         row.key { 'Role' }
-        row.value { current_user.role.humanize }
+        row.value { @user.role.humanize }
         row.action()
       end %>
+
+      <%= summary_list.row do |row|
+        row.key { 'Data protection officer' }
+        row.value { @user.is_data_protection_officer? ? "Yes" : "No" }
+        row.action(visually_hidden_text: 'are they a data protection officer?', href: edit_user_path, html_attributes: { 'data-qa': 'change-are-they-a-data-protection-officer' })
+      end %>
     <% end %>
   </div>
 </div>

spec/requests/users_controller_spec.rb

@@ -5,8 +5,9 @@ RSpec.describe UsersController, type: :request do
   let(:other_user) { FactoryBot.create(:user) }
   let(:headers) { { "Accept" => "text/html" } }
   let(:page) { Capybara::Node::Simple.new(response.body) }
-  let(:new_value) { "new test name" }
-  let(:params) { { id: user.id, user: { name: new_value } } }
+  let(:new_name) { "new test name" }
+  let(:new_email) { "new@example.com" }
+  let(:params) { { id: user.id, user: { name: new_name } } }
   let(:notify_client) { instance_double(Notifications::Client) }
   let(:devise_notify_mailer) { DeviseNotifyMailer.new }
 
@@ -56,7 +57,7 @@ RSpec.describe UsersController, type: :request do
         context "when the reset token is valid" do
           let(:params) do
             {
-              id: user.id, user: { password: new_value, password_confirmation: "something_else" }
+              id: user.id, user: { password: new_name, password_confirmation: "something_else" }
             }
           end
 
@@ -78,8 +79,8 @@ RSpec.describe UsersController, type: :request do
             {
               id: user.id,
               user: {
-                password: new_value,
-                password_confirmation: new_value,
+                password: new_name,
+                password_confirmation: new_name,
                 reset_password_token: raw,
               },
             }
@@ -199,7 +200,7 @@ RSpec.describe UsersController, type: :request do
 
         it "updates the user" do
           user.reload
-          expect(user.name).to eq(new_value)
+          expect(user.name).to eq(new_name)
         end
 
         it "tracks who updated the record" do
@@ -208,6 +209,16 @@ RSpec.describe UsersController, type: :request do
           expect(whodunnit_actor).to be_a(User)
           expect(whodunnit_actor.id).to eq(user.id)
         end
+
+        context "when user changes email and dpo" do
+          let(:params) { { id: user.id, user: { name: new_name, email: new_email, is_dpo: "1" } } }
+
+          it "allows changing email and dpo" do
+            user.reload
+            expect(user.email).to eq(new_email)
+            expect(user.is_data_protection_officer?).to be true
+          end
+        end
       end
 
       context "when the update fails to persist" do
@@ -224,7 +235,7 @@ RSpec.describe UsersController, type: :request do
       end
 
       context "when the current user does not matches the user ID" do
-        let(:params) { { id: other_user.id, user: { name: new_value } } }
+        let(:params) { { id: other_user.id, user: { name: new_name } } }
 
         before do
           sign_in user
@@ -239,7 +250,7 @@ RSpec.describe UsersController, type: :request do
       context "when we update the user password" do
         let(:params) do
           {
-            id: user.id, user: { password: new_value, password_confirmation: "something_else" }
+            id: user.id, user: { password: new_name, password_confirmation: "something_else" }
           }
         end
 
@@ -284,7 +295,7 @@ RSpec.describe UsersController, type: :request do
           end
 
           it "shows the user details page" do
-            expect(page).to have_content("#{other_user.name}'s account")
+            expect(page).to have_content("#{other_user.name}’s account")
           end
         end
 
@@ -326,7 +337,7 @@ RSpec.describe UsersController, type: :request do
           end
 
           it "shows the user details page" do
-            expect(page).to have_content("Change #{other_user.name}'s personal details")
+            expect(page).to have_content("Change #{other_user.name}’s personal details")
           end
         end
 
@@ -377,7 +388,7 @@ RSpec.describe UsersController, type: :request do
 
         it "updates the user" do
           user.reload
-          expect(user.name).to eq(new_value)
+          expect(user.name).to eq(new_name)
         end
 
         it "tracks who updated the record" do
@@ -387,10 +398,20 @@ RSpec.describe UsersController, type: :request do
           expect(whodunnit_actor.id).to eq(user.id)
         end
 
+        context "when user changes email and dpo" do
+          let(:params) { { id: user.id, user: { name: new_name, email: new_email, is_dpo: "1" } } }
+
+          it "allows changing email and dpo" do
+            user.reload
+            expect(user.email).to eq(new_email)
+            expect(user.is_data_protection_officer?).to be true
+          end
+        end
+
         context "when we update the user password" do
           let(:params) do
             {
-              id: user.id, user: { password: new_value, password_confirmation: "something_else" }
+              id: user.id, user: { password: new_name, password_confirmation: "something_else" }
             }
           end
 
@@ -414,7 +435,7 @@ RSpec.describe UsersController, type: :request do
         context "when the user is part of the same organisation as the current user" do
           it "updates the user" do
             expect { patch "/users/#{other_user.id}", headers: headers, params: params }
-              .to change { other_user.reload.name }.from(other_user.name).to(new_value)
+              .to change { other_user.reload.name }.from(other_user.name).to(new_name)
           end
 
           it "tracks who updated the record" do
@@ -422,10 +443,28 @@ RSpec.describe UsersController, type: :request do
               .to change { other_user.reload.versions.last.actor&.id }.from(nil).to(user.id)
           end
 
+          context "when user changes email and dpo" do
+            let(:params) { { id: other_user.id, user: { name: new_name, email: new_email, is_dpo: "1" } } }
+
+            it "allows changing email and dpo" do
+              patch "/users/#{other_user.id}", headers: headers, params: params
+              other_user.reload
+              expect(other_user.email).to eq(new_email)
+              expect(other_user.is_data_protection_officer?).to be true
+            end
+          end
+
+          it "does not bypass sign in for the coordinator" do
+            patch "/users/#{other_user.id}", headers: headers, params: params
+            follow_redirect!
+            expect(page).to have_content("#{other_user.reload.name}’s account")
+            expect(page).to have_content(other_user.reload.email.to_s)
+          end
+
           context "when we try to update the user password" do
             let(:params) do
               {
-                id: user.id, user: { password: new_value, password_confirmation: new_value, name: "new name" }
+                id: user.id, user: { password: new_name, password_confirmation: new_name, name: "new name" }
               }
             end
 
@@ -444,7 +483,7 @@ RSpec.describe UsersController, type: :request do
         context "when the current user does not matches the user ID" do
           context "when the user is not part of the same organisation as the current user" do
             let(:other_user) { FactoryBot.create(:user) }
-            let(:params) { { id: other_user.id, user: { name: new_value } } }
+            let(:params) { { id: other_user.id, user: { name: new_name } } }
 
             before do
               sign_in user