Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add separate page for editing another user's name #2497

Merged
merged 8 commits into from
Nov 8, 2023
32 changes: 32 additions & 0 deletions app/controllers/users/names_controller.rb
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions app/views/layouts/admin_layout.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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) %>
Expand Down
5 changes: 2 additions & 3 deletions app/views/users/_form_fields.html.erb
Original file line number Diff line number Diff line change
@@ -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">
Expand Down
60 changes: 60 additions & 0 deletions app/views/users/names/edit.html.erb
Original file line number Diff line number Diff line change
@@ -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>
1 change: 1 addition & 0 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
190 changes: 190 additions & 0 deletions test/controllers/users/names_controller_test.rb
Original file line number Diff line number Diff line change
@@ -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
Loading