Merge pull request #2509 from alphagov/split-out-edit-user-email-page…

…-from-user-edit-page

Add separate page for editing another user's email

app/controllers/users/emails_controller.rb

@@ -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])
+  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

app/controllers/users/names_controller.rb

@@ -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
 
@@ -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

app/controllers/users_controller.rb

@@ -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

app/views/users/_form_fields.html.erb

@@ -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) %>
 </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">

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

@@ -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>

config/routes.rb

@@ -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"
 

test/controllers/account/emails_controller_test.rb

@@ -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
 

test/controllers/invitations_controller_test.rb

@@ -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
@@ -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
@@ -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

test/controllers/two_step_verification_controller_test.rb

@@ -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
 

test/controllers/user_research_recruitment_controller_test.rb

@@ -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