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 email #2509

Merged
merged 12 commits into from
Nov 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions app/controllers/users/emails_controller.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
class Users::EmailsController < ApplicationController
layout "admin_layout"

before_action :authenticate_user!
before_action :load_user
before_action :authorize_user
before_action :redirect_to_account_page_if_acting_on_own_user, only: %i[edit]

def edit; end

def update
updater = UserUpdate.new(@user, user_params, current_user, user_ip_address)
if updater.call
redirect_to edit_user_path(@user), notice: "Updated user #{@user.email} successfully"
else
render :edit
end
end

def resend_email_change
@user.resend_confirmation_instructions
if @user.errors.empty?
redirect_to edit_user_path(@user), notice: "Successfully resent email change email to #{@user.unconfirmed_email}"
else
redirect_to edit_user_email_path(@user), alert: "Failed to send email change email"
end
end

def cancel_email_change
@user.cancel_email_change!

redirect_to edit_user_path(@user)
end

private

def load_user
@user = User.find(params[:user_id])
chrisroos marked this conversation as resolved.
Show resolved Hide resolved
end

def authorize_user
authorize @user
end

def user_params
params.require(:user).permit(*current_user.permitted_params.intersection([:email]))
end

def redirect_to_account_page_if_acting_on_own_user
redirect_to edit_account_email_path if current_user == @user
end
end
5 changes: 5 additions & 0 deletions app/controllers/users/names_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ class Users::NamesController < ApplicationController
before_action :authenticate_user!
before_action :load_user
before_action :authorize_user
before_action :redirect_to_account_page_if_acting_on_own_user, only: %i[edit]

def edit; end

Expand All @@ -29,4 +30,8 @@ def authorize_user
def user_params
params.require(:user).permit(*current_user.permitted_params.intersection([:name]))
end

def redirect_to_account_page_if_acting_on_own_user
redirect_to account_path if current_user == @user
end
end
15 changes: 0 additions & 15 deletions app/controllers/users_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,21 +54,6 @@ def unlock
redirect_to users_path
end

def resend_email_change
@user.resend_confirmation_instructions
if @user.errors.empty?
redirect_to users_path, notice: "Successfully resent email change email to #{@user.unconfirmed_email}"
else
redirect_to edit_user_path(@user), alert: "Failed to send email change email"
end
end

def cancel_email_change
@user.cancel_email_change!

redirect_to users_path
end

def event_logs
authorize @user
@logs = @user.event_logs.page(params[:page]).per(100) if @user
Expand Down
20 changes: 2 additions & 18 deletions app/views/users/_form_fields.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,10 @@
<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">
<%= f.label :email %>
<%= f.text_field :email, autocomplete: "off", class: 'form-control input-md-6 add-label-margin' %>
<% if f.object.invited_but_not_yet_accepted? %>
<span class="help-block">Changes will trigger a new signup email.</span>
<% end %>
<p>
<strong>Email:</strong> <%= @user.email %> <%= link_to 'Change<span class="invisible"> email</span>'.html_safe, edit_user_email_path(@user) %>
chrisroos marked this conversation as resolved.
Show resolved Hide resolved
</p>

<% if f.object.unconfirmed_email.present? %>
<p class="form-group">
<%= f.label :unconfirmed_email, "Pending email" %>
<%= f.text_field :unconfirmed_email, readonly: "readonly", disabled: "disabled", class: 'form-control' %>

<div class="help-block add-bottom-margin">
<%= link_to "Resend confirmation email", resend_email_change_user_path(f.object), :method => :put, :class => "btn btn-primary add-right-margin" %>
<%= link_to "Cancel change", cancel_email_change_user_path(f.object), :method => :delete, :class => "btn btn-danger" %>
</div>
</p>
<% end %>

<% if policy(User).assign_role? %>
<% if @user.reason_for_2sv_exemption.blank? %>
<p class="form-group">
Expand Down
72 changes: 72 additions & 0 deletions app/views/users/emails/edit.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
<% content_for :title_caption, "Manage other users" %>
<% content_for :title, "Change email for #{@user.name}" %>

<% 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,
url: edit_user_path(@user),
},
{
title: "Change email",
}
]
})
%>

<% if @user.unconfirmed_email.present? %>
<%= render "govuk_publishing_components/components/notice", {
title: "A request has been made to change the email address"
} do %>
<p class="govuk-body">An email has been sent to <strong><%= @user.unconfirmed_email %></strong> with a link to confirm the change. If they haven't received this email, we can send it again:</p>

<%= link_to "Resend confirmation email", resend_email_change_user_email_path(@user), method: :put, class: "govuk-button app-button--no-margin" %>
<%= link_to "Cancel change", cancel_email_change_user_email_path(@user), method: :delete, class: "govuk-link app-link--inline" %>
<% end %>
<% end %>

<% 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_email_path do %>
<%= render "govuk_publishing_components/components/input", {
label: {
text: "Email"
},
name: "user[email]",
id: "user_email",
type: "email",
value: @user.email,
hint: @user.invited_but_not_yet_accepted? ? "Changes will trigger a new signup email." : nil,
autocomplete: "off",
error_items: @user.errors.full_messages_for(:email).map { |message| { text: message } }
} %>
<%= render "govuk_publishing_components/components/button", {
text: "Change email"
} %>
<% end %>
</div>
</div>
6 changes: 4 additions & 2 deletions config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@
resources :users, except: [:show] do
member do
post :unlock
put :resend_email_change
delete :cancel_email_change
get :event_logs
patch :reset_two_step_verification
get :require_2sv
end
resource :name, only: %i[edit update], controller: "users/names"
resource :email, only: %i[edit update], controller: "users/emails" do
put :resend_email_change
delete :cancel_email_change
end
end
get "user", to: "oauth_users#show"

Expand Down
2 changes: 1 addition & 1 deletion test/controllers/account/emails_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ class Account::EmailsControllerTest < ActionController::TestCase
perform_enqueued_jobs do
@user = create(
:user_with_pending_email_change,
:with_expired_confirmation_token,
confirmation_token: "old token",
confirmation_sent_at: 15.days.ago,
)
sign_in @user

Expand Down
6 changes: 3 additions & 3 deletions test/controllers/invitations_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ class InvitationsControllerTest < ActionController::TestCase
should "require inviter to be signed in" do
get :new

assert_redirected_to new_user_session_path
assert_not_authenticated
end
end
end
Expand Down Expand Up @@ -484,7 +484,7 @@ class InvitationsControllerTest < ActionController::TestCase
should "require inviter to be signed in" do
post :create

assert_redirected_to new_user_session_path
assert_not_authenticated
end
end
end
Expand Down Expand Up @@ -549,7 +549,7 @@ class InvitationsControllerTest < ActionController::TestCase
user_to_resend_for = create(:user)
post :resend, params: { id: user_to_resend_for }

assert_redirected_to new_user_session_path
assert_not_authenticated
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion test/controllers/two_step_verification_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class TwoStepVerificationControllerTest < ActionController::TestCase
should "redirect to login upon attempted prompt" do
get :prompt

assert_redirected_to new_user_session_path
assert_not_authenticated
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class UserResearchRecruitmentControllerTest < ActionController::TestCase
should "require users to be signed in" do
put :update

assert_redirected_to new_user_session_path
assert_not_authenticated
end

context "when user clicks the button to dismiss the banner" do
Expand Down
Loading