Merge pull request #2497 from alphagov/add-edit-user-name-page

Add separate page for editing user's name

app/controllers/users/names_controller.rb

@@ -0,0 +1,32 @@
+class Users::NamesController < ApplicationController
+  layout "admin_layout"
+
+  before_action :authenticate_user!
+  before_action :load_user
+  before_action :authorize_user
+
+  def edit; end
+
+  def update
+    if @user.update(user_params)
+      EventLog.record_event(@user, EventLog::ACCOUNT_UPDATED, initiator: current_user, ip_address: user_ip_address)
+      redirect_to user_path(@user), notice: "Updated user #{@user.email} successfully"
+    else
+      render :edit
+    end
+  end
+
+private
+
+  def load_user
+    @user = User.find(params[:user_id])
+  end
+
+  def authorize_user
+    authorize @user
+  end
+
+  def user_params
+    params.require(:user).permit(*current_user.permitted_params.intersection([:name]))
+  end
+end

app/controllers/users_controller.rb

@@ -127,7 +127,7 @@ def user_params
   end
 
   def permitted_user_params
-    @permitted_user_params ||= params.require(:user).permit(:user, :name, :email, :organisation_id, :require_2sv, :role, :skip_update_user_permissions, supported_permission_ids: []).to_h
+    @permitted_user_params ||= params.require(:user).permit(:user, :email, :organisation_id, :require_2sv, :role, :skip_update_user_permissions, supported_permission_ids: []).to_h
   end
 
   def filter_params

app/models/user.rb

@@ -320,6 +320,10 @@ def manageable_organisations
     role_class.manageable_organisations_for(self).order(:name)
   end
 
+  def permitted_params
+    role_class.permitted_user_params
+  end
+
   # Make devise send all emails using ActiveJob
   def send_devise_notification(notification, *args)
     devise_mailer.send(notification, self, *args).deliver_later

app/views/layouts/admin_layout.html.erb

@@ -37,6 +37,9 @@
 
       <div class="govuk-grid-row">
         <div class="<%= yield(:top_right).present? ? 'govuk-grid-column-one-third' : 'govuk-grid-column-two-thirds' %>">
+          <% if yield(:title_caption).present? %>
+            <span class="govuk-caption-l"><%= yield(:title_caption) %></span>
+          <% end %>
           <h1 class="govuk-heading-l">
             <% if yield(:page_heading).present? %>
               <%= yield(:page_heading) %>

app/views/users/_form_fields.html.erb

@@ -1,6 +1,5 @@
-<p class="form-group">
-  <%= f.label :name %>
-  <%= f.text_field :name, autofocus: true, autocomplete: "off", class: 'form-control input-md-6 ' %>
+<p>
+  <strong>Name:</strong> <%= @user.name %> <%= link_to 'Change<span class="invisible"> name</span>'.html_safe, edit_user_name_path(@user) %>
 </p>
 
 <p class="form-group">

app/views/users/names/edit.html.erb

@@ -0,0 +1,60 @@
+<% content_for :title_caption, "Manage other users" %>
+<% content_for :title, "Change name for #{@user.name_was}" %>
+
+<% content_for :breadcrumbs,
+   render("govuk_publishing_components/components/breadcrumbs", {
+     collapse_on_mobile: true,
+     breadcrumbs: [
+       {
+         title: "Dashboard",
+         url: root_path,
+       },
+       {
+         title: "Users",
+         url: users_path,
+       },
+       {
+         title: @user.name_was,
+         url: edit_user_path(@user),
+       },
+       {
+         title: "Change name",
+       }
+     ]
+   })
+%>
+
+<% if @user.errors.count > 0 %>
+  <% content_for :error_summary do %>
+    <%= render "govuk_publishing_components/components/error_summary", {
+      title: "There is a problem",
+      items: @user.errors.map do |error|
+        {
+          text: error.full_message,
+          href: "#user_#{error.attribute}",
+        }
+      end,
+    } %>
+  <% end %>
+<% end %>
+
+<div class="govuk-grid-row">
+  <div class="govuk-grid-column-two-thirds">
+    <%= form_for @user, url: user_name_path do |f| %>
+      <%= render "govuk_publishing_components/components/input", {
+        label: {
+          text: "Name"
+        },
+        name: "user[name]",
+        type: "name",
+        id: "user_name",
+        value: @user.name,
+        autocomplete: "off",
+        error_items: @user.errors.full_messages_for(:name).map { |message| { text: message } }
+      } %>
+      <%= render "govuk_publishing_components/components/button", {
+        text: "Change name"
+      } %>
+    <% end %>
+  </div>
+</div>

config/routes.rb

@@ -45,6 +45,7 @@
       patch :reset_two_step_verification
       get :require_2sv
     end
+    resource :name, only: %i[edit update], controller: "users/names"
   end
   get "user", to: "oauth_users#show"
 

test/controllers/users/names_controller_test.rb

@@ -0,0 +1,190 @@
+require "test_helper"
+
+class Users::NamesControllerTest < ActionController::TestCase
+  context "GET edit" do
+    context "signed in as Admin user" do
+      setup do
+        sign_in(create(:admin_user))
+      end
+
+      should "display form with name field" do
+        user = create(:user, name: "user-name")
+
+        get :edit, params: { user_id: user }
+
+        assert_template :edit
+        assert_select "form[action='#{user_name_path(user)}']" do
+          assert_select "input[name='user[name]']", value: "user-name"
+        end
+      end
+
+      should "authorize access if UserPolicy#edit? returns true" do
+        user = create(:user)
+
+        user_policy = stub_everything("user-policy", edit?: true)
+        UserPolicy.stubs(:new).returns(user_policy)
+
+        get :edit, params: { user_id: user }
+
+        assert_template :edit
+      end
+
+      should "not authorize access if UserPolicy#edit? returns false" do
+        user = create(:user)
+
+        user_policy = stub_everything("user-policy", edit?: false)
+        UserPolicy.stubs(:new).returns(user_policy)
+
+        get :edit, params: { user_id: user }
+
+        assert_not_authorised
+      end
+    end
+
+    context "signed in as Normal user" do
+      setup do
+        sign_in(create(:user))
+      end
+
+      should "not be authorized" do
+        user = create(:user)
+
+        get :edit, params: { user_id: user }
+
+        assert_not_authorised
+      end
+    end
+
+    context "not signed in" do
+      should "not be allowed access" do
+        user = create(:user)
+
+        get :edit, params: { user_id: user }
+
+        assert_redirected_to new_user_session_path
+      end
+    end
+  end
+
+  context "PUT update" do
+    context "signed in as Admin user" do
+      setup do
+        @admin = create(:admin_user)
+        sign_in(@admin)
+      end
+
+      should "update user name" do
+        user = create(:user, name: "user-name")
+
+        put :update, params: { user_id: user, user: { name: "new-user-name" } }
+
+        assert_equal "new-user-name", user.reload.name
+      end
+
+      should "record account updated event" do
+        user = create(:user)
+
+        @controller.stubs(:user_ip_address).returns("1.1.1.1")
+
+        EventLog.expects(:record_event).with(
+          user,
+          EventLog::ACCOUNT_UPDATED,
+          initiator: @admin,
+          ip_address: "1.1.1.1",
+        )
+
+        put :update, params: { user_id: user, user: { name: "new-user-name" } }
+      end
+
+      should "redirect to user page and display success notice" do
+        user = create(:user, email: "user@gov.uk")
+
+        put :update, params: { user_id: user, user: { name: "new-user-name" } }
+
+        assert_redirected_to user_path(user), notice: "Updated name of user@gov.uk successfully"
+      end
+
+      should "update user name if UserPolicy#update? returns true" do
+        user = create(:user, name: "user-name")
+
+        user_policy = stub_everything("user-policy", update?: true)
+        UserPolicy.stubs(:new).returns(user_policy)
+
+        put :update, params: { user_id: user, user: { name: "new-user-name" } }
+
+        assert_equal "new-user-name", user.reload.name
+      end
+
+      should "not update user name if UserPolicy#update? returns false" do
+        user = create(:user, name: "user-name")
+
+        user_policy = stub_everything("user-policy", update?: false)
+        UserPolicy.stubs(:new).returns(user_policy)
+
+        put :update, params: { user_id: user, user: { name: "new-user-name" } }
+
+        assert_equal "user-name", user.reload.name
+        assert_not_authorised
+      end
+
+      should "redisplay form if name is not valid" do
+        user = create(:user, name: "user-name")
+
+        put :update, params: { user_id: user, user: { name: "" } }
+
+        assert_template :edit
+        assert_select "form[action='#{user_name_path(user)}']" do
+          assert_select "input[name='user[name]']", value: ""
+        end
+      end
+
+      should "use original name in page title, heading & breadcrumbs if new name was not valid" do
+        user = create(:user, name: "user-name")
+
+        put :update, params: { user_id: user, user: { name: "" } }
+
+        assert_select "head title", text: /^Change name for user-name/
+        assert_select "h1", text: "Change name for user-name"
+        assert_select ".govuk-breadcrumbs li", text: "user-name"
+      end
+
+      should "display errors if name is not valid" do
+        user = create(:user)
+
+        put :update, params: { user_id: user, user: { name: "" } }
+
+        assert_select ".govuk-error-summary" do
+          assert_select "a", href: "#user_name", text: "Name can't be blank"
+        end
+        assert_select ".govuk-form-group" do
+          assert_select ".govuk-error-message", text: "Error: Name can't be blank"
+          assert_select "input[name='user[name]'].govuk-input--error"
+        end
+      end
+    end
+
+    context "signed in as Normal user" do
+      setup do
+        sign_in(create(:user))
+      end
+
+      should "update user name" do
+        user = create(:user, name: "user-name")
+
+        put :update, params: { user_id: user, user: { name: "new-user-name" } }
+
+        assert_not_authorised
+      end
+    end
+
+    context "not signed in" do
+      should "not be allowed access" do
+        user = create(:user)
+
+        get :edit, params: { user_id: user }
+
+        assert_redirected_to new_user_session_path
+      end
+    end
+  end
+end