Merge pull request #2531 from alphagov/misc-improvements-to-admin-for…

…ms-for-editing-user

Miscellaneous improvements to admin forms for editing user

app/views/users/_form_fields.html.erb

@@ -1,13 +1,21 @@
 <p>
-  <strong>Name:</strong> <%= @user.name %> <%= link_to 'Change<span class="invisible"> name</span>'.html_safe, edit_user_name_path(@user) %>
+  <strong>Name:</strong> <%= @user.name %>
+  <%= link_to edit_user_name_path(@user) do %>
+    Change<span class="invisible"> name</span>
+  <% end %>
 </p>
 
 <p>
-  <strong>Email:</strong> <%= @user.email %> <%= link_to 'Change<span class="invisible"> email</span>'.html_safe, edit_user_email_path(@user) %>
+  <strong>Email:</strong> <%= @user.email %>
+  <%= link_to edit_user_email_path(@user) do %>
+    Change<span class="invisible"> email</span>
+  <% end %>
 </p>
 
 <% if policy(User).assign_role? %>
-  <% if @user.reason_for_2sv_exemption.blank? %>
+  <% if @user.exempt_from_2sv? %>
+    <p>This user's role is set to <%= @user.role %>. They are currently exempted from 2-step verification, meaning that their role cannot be changed as admins are required to have 2-step verification.</p>
+  <% else %>
     <p class="form-group">
       <%= f.label :role %><br />
       <%= f.select :role, options_for_select(assignable_user_roles.map(&:humanize).zip(assignable_user_roles), f.object.role), {}, class: "chosen-select form-control", 'data-module' => 'chosen' %>
@@ -18,8 +26,6 @@
         <strong>Organisation Admins</strong> can unlock and unsuspend their organisation accounts.
       </span>
     </p>
-  <% else %>
-    <p>This user's role is set to <%= @user.role %>. They are currently exempted from 2-step verification, meaning that their role cannot be changed as admins are required to have 2-step verification.</p>
   <% end %>
 <% end %>
 

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

@@ -64,9 +64,12 @@
         autocomplete: "off",
         error_items: @user.errors.full_messages_for(:email).map { |message| { text: message } }
       } %>
-      <%= render "govuk_publishing_components/components/button", {
-        text: "Change email"
-      } %>
+      <div class="govuk-button-group">
+        <%= render "govuk_publishing_components/components/button", {
+          text: "Change email",
+        } %>
+        <%= link_to "Cancel", edit_user_path(@user), class: "govuk-link govuk-link--no-visited-state" %>
+      </div>
     <% end %>
   </div>
 </div>

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

@@ -51,9 +51,12 @@
         autocomplete: "off",
         error_items: @user.errors.full_messages_for(:name).map { |message| { text: message } }
       } %>
-      <%= render "govuk_publishing_components/components/button", {
-        text: "Change name"
-      } %>
+      <div class="govuk-button-group">
+        <%= render "govuk_publishing_components/components/button", {
+          text: "Change name",
+        } %>
+        <%= link_to "Cancel", edit_user_path(@user), class: "govuk-link govuk-link--no-visited-state" %>
+      </div>
     <% end %>
   </div>
 </div>

test/controllers/users/emails_controller_test.rb

@@ -19,6 +19,8 @@ class Users::EmailsControllerTest < ActionController::TestCase
         assert_template :edit
         assert_select "form[action='#{user_email_path(user)}']" do
           assert_select "input[name='user[email]']", value: "user@gov.uk"
+          assert_select "button[type='submit']", text: "Change email"
+          assert_select "a[href='#{edit_user_path(user)}']", text: "Cancel"
         end
       end
 

test/controllers/users/names_controller_test.rb

@@ -16,6 +16,8 @@ class Users::NamesControllerTest < ActionController::TestCase
         assert_template :edit
         assert_select "form[action='#{user_name_path(user)}']" do
           assert_select "input[name='user[name]']", value: "user-name"
+          assert_select "button[type='submit']", text: "Change name"
+          assert_select "a[href='#{edit_user_path(user)}']", text: "Cancel"
         end
       end
 

test/integration/change_user_role_test.rb

@@ -13,7 +13,7 @@ def sign_in_as_and_edit_user(sign_in_as, user_to_edit)
   end
 
   context "when logged in as a super admin" do
-    should "be able to change the role of a user who has no 2SV exemption reason" do
+    should "be able to change the role of a user who is not exempt from 2SV" do
       user = create(:user)
       sign_in_as_and_edit_user(@super_admin, user)
 
@@ -23,7 +23,7 @@ def sign_in_as_and_edit_user(sign_in_as, user_to_edit)
       assert user.reload.admin?
     end
 
-    should "not be able to change the role of a user who has a 2SV exemption reason" do
+    should "not be able to change the role of a user who is exempt from 2SV" do
       user = create(:two_step_exempted_user)
       sign_in_as_and_edit_user(@super_admin, user)