Address code review comments

This change addresses code review comments

app/controllers/admin/users_controller.rb

@@ -115,7 +115,7 @@ class Admin::UsersController < Admin::ApplicationController
 
     if result[:status] == :success
       redirect_to admin_user_path(user),
-        notice: _('Two-factor Authentication has been disabled for this user')
+        notice: _('Two-factor authentication has been disabled for this user')
     else
       redirect_to admin_user_path(user), alert: result[:message]
     end

app/helpers/emails_helper.rb

@@ -177,6 +177,27 @@ module EmailsHelper
     strip_tags(render_message(:footer_message, style: ''))
   end
 
+  def say_hi(user)
+    _('Hi %{username}!') % { username: sanitize_name(user.name) }
+  end
+
+  def two_factor_authentication_disabled_text
+    _('Two-factor authentication has been disabled for your GitLab account.')
+  end
+
+  def re_enable_two_factor_authentication_text(format: nil)
+    url = profile_two_factor_auth_url
+
+    case format
+    when :html
+      settings_link_to = link_to(_('two-factor authentication settings'), url, target: :_blank, rel: 'noopener noreferrer').html_safe
+      _("If you want to re-enable two-factor authentication, visit the %{settings_link_to} page.").html_safe % { settings_link_to: settings_link_to }
+    else
+      _('If you want to re-enable two-factor authentication, visit %{two_factor_link}') %
+        { two_factor_link: url }
+    end
+  end
+
   private
 
   def show_footer?

app/views/notify/disabled_two_factor_email.html.haml

 %p
-  = _('Hi %{username}!') % { username: sanitize_name(@user.name) }
+  = say_hi(@user)
 %p
-  = _('Two-factor authentication has been disabled for your GitLab account.')
+  = two_factor_authentication_disabled_text
 %p
-  - two_factor_page_start = '<a href="%{url}" target="_blank" rel="noopener noreferrer">'.html_safe % { url: profile_two_factor_auth_url }
-  = html_escape(_('If you want to re-enable Two-factor authentication, visit the %{two_factor_page_start}Two-factor authentication settings%{two_factor_page_end} page.')) % { two_factor_page_start: two_factor_page_start, two_factor_page_end: '</a>'.html_safe }
+  = re_enable_two_factor_authentication_text(format: :html)

app/views/notify/disabled_two_factor_email.text.erb

-<%= _('Hi %{username}!') % { username: sanitize_name(@user.name) } %>
+<%= say_hi(@user) %>
 
-<%= _('Two-factor authentication has been disabled for your GitLab account.') %>
+<%= two_factor_authentication_disabled_text %>
 
-<%= _('If you want to re-enable Two-factor authentication, visit %{two_factor_link}') % { two_factor_link: profile_two_factor_auth_url } %>
+<%= re_enable_two_factor_authentication_text %>

doc/user/profile/notifications.md

@@ -144,7 +144,7 @@ Users will be notified of the following events:
 | New email added              | User                | Security email, always sent. |
 | Email changed                | User                | Security email, always sent. |
 | Password changed             | User                | Security email, always sent. |
-| Two-factor Authentication disabled | User          | Security email, always sent. |
+| Two-factor authentication disabled | User          | Security email, always sent. |
 | New user created             | User                | Sent on user creation, except for OmniAuth (LDAP)|
 | User added to project        | User                | Sent when user is added to project |
 | Project access level changed | User                | Sent when user project access level is changed |

locale/gitlab.pot

@@ -12698,10 +12698,10 @@ msgstr ""
 msgid "If you remove this license, GitLab will fall back on the previous license, if any."
 msgstr ""
 
-msgid "If you want to re-enable Two-factor authentication, visit %{two_factor_link}"
+msgid "If you want to re-enable two-factor authentication, visit %{two_factor_link}"
 msgstr ""
 
-msgid "If you want to re-enable Two-factor authentication, visit the %{two_factor_page_start}Two-factor authentication settings%{two_factor_page_end} page."
+msgid "If you want to re-enable two-factor authentication, visit the %{settings_link_to} page."
 msgstr ""
 
 msgid "If your HTTP repository is not publicly accessible, add your credentials."
@@ -26004,15 +26004,15 @@ msgstr ""
 msgid "Two-factor Authentication Recovery codes"
 msgstr ""
 
-msgid "Two-factor Authentication has been disabled for this user"
-msgstr ""
-
 msgid "Two-factor authentication"
 msgstr ""
 
 msgid "Two-factor authentication disabled"
 msgstr ""
 
+msgid "Two-factor authentication has been disabled for this user"
+msgstr ""
+
 msgid "Two-factor authentication has been disabled for your GitLab account."
 msgstr ""
 
@@ -29850,6 +29850,9 @@ msgstr ""
 msgid "triggered"
 msgstr ""
 
+msgid "two-factor authentication settings"
+msgstr ""
+
 msgid "unicode domains should use IDNA encoding"
 msgstr ""
 

spec/controllers/admin/users_controller_spec.rb

@@ -239,7 +239,7 @@ RSpec.describe Admin::UsersController do
         subject
 
         expect(flash[:notice])
-          .to eq _('Two-factor Authentication has been disabled for this user')
+          .to eq _('Two-factor authentication has been disabled for this user')
       end
     end
 

spec/helpers/emails_helper_spec.rb

@@ -31,7 +31,7 @@ RSpec.describe EmailsHelper do
 
         context "and format is unknown" do
           it "returns plain text" do
-            expect(helper.closure_reason_text(merge_request, format: :text)).to eq("via merge request #{merge_request.to_reference} (#{merge_request_presenter.web_url})")
+            expect(helper.closure_reason_text(merge_request, format: 'unknown')).to eq("via merge request #{merge_request.to_reference} (#{merge_request_presenter.web_url})")
           end
         end
       end
@@ -110,6 +110,41 @@ RSpec.describe EmailsHelper do
     end
   end
 
+  describe '#say_hi' do
+    let(:user) { create(:user, name: 'John') }
+
+    it 'returns the greeting message for the given user' do
+      expect(say_hi(user)).to eq('Hi John!')
+    end
+  end
+
+  describe '#two_factor_authentication_disabled_text' do
+    it 'returns the message that 2FA is disabled' do
+      expect(two_factor_authentication_disabled_text).to eq(
+        _('Two-factor authentication has been disabled for your GitLab account.')
+      )
+    end
+  end
+
+  describe '#re_enable_two_factor_authentication_text' do
+    context 'format is html' do
+      it 'returns HTML' do
+        expect(re_enable_two_factor_authentication_text(format: :html)).to eq(
+          "If you want to re-enable two-factor authentication, visit the " \
+          "#{link_to('two-factor authentication settings', profile_two_factor_auth_url, target: :_blank, rel: 'noopener noreferrer')} page."
+        )
+      end
+    end
+
+    context 'format is not specified' do
+      it 'returns text' do
+        expect(re_enable_two_factor_authentication_text).to eq(
+          "If you want to re-enable two-factor authentication, visit #{profile_two_factor_auth_url}"
+        )
+      end
+    end
+  end
+
   describe 'password_reset_token_valid_time' do
     def validate_time_string(time_limit, expected_string)
       Devise.reset_password_within = time_limit