Review feedback

.rubocop.yml

@@ -42,7 +42,11 @@ Layout/TrailingWhitespace:
   Enabled: false
 
 Lint/AmbiguousBlockAssociation:
-  Enabled: false
+  Exclude:
+    # Exception for specs where we use change matchers:
+    # https://github.com/rubocop-hq/rubocop/issues/4222
+    - 'features/step_definitions/**/*.rb'
+    - 'spec/**/*.rb'
 
 Lint/AmbiguousRegexpLiteral:
   Enabled: false

app/controllers/pseuds_controller.rb

@@ -28,9 +28,7 @@ def index
   def show
     raise ActiveRecord::RecordNotFound, t(".could_not_find_user", username: params[:user_id]) if @user.blank?
 
-    @pseud = @user.pseuds.find_by(name: params[:id])
-    raise ActiveRecord::RecordNotFound, t(".could_not_find_pseud", pseud: params[:id]) unless @pseud
-
+    @pseud = @user.pseuds.find_by!(name: params[:id])
     @page_subtitle = @pseud.name
 
     # very similar to show under users - if you change something here, change it there too

app/models/user.rb

@@ -91,7 +91,7 @@ class User < ApplicationRecord
   before_update :add_renamed_at, if: :will_save_change_to_login?
   after_update :update_pseud_name
   after_update :send_wrangler_username_change_notification, if: :is_tag_wrangler?
-  after_update :log_change_if_login_was_edited
+  after_update :log_change_if_login_was_edited, if: :saved_change_to_login?
   after_update :log_email_change, if: :saved_change_to_email?
 
   after_commit :reindex_user_creations_after_rename
@@ -578,15 +578,13 @@ def add_renamed_at
   end
 
   def log_change_if_login_was_edited
-    return unless saved_change_to_login?
-
     current_admin = User.current_user if User.current_user.is_a?(Admin)
     options = {
       action: ArchiveConfig.ACTION_RENAME,
       admin: current_admin
     }
     options[:note] = if current_admin
-                       "Old Username: #{login_before_last_save}, New Username: #{login}, Changed by: #{User.current_user&.login}, Ticket ID: ##{ticket_number}"
+                       "Old Username: #{login_before_last_save}, New Username: #{login}, Changed by: #{current_admin.login}, Ticket ID: ##{ticket_number}"
                      else
                        "Old Username: #{login_before_last_save}; New Username: #{login}"
                      end

app/views/admin/admin_invitations/find.html.erb

@@ -1,30 +1,35 @@
 <!--Descriptive page name, messages and instructions-->
-<h2 class="heading"><%h= 'Track invitations' %></h2>
+<h2 class="heading"><%= t(".page_heading") %></h2>
 <!--/descriptions-->
 
 <!--subnav-->
 <!--/subnav-->
 
 <!--main content-->
-<%= form_tag url_for(:controller => 'admin/admin_invitations', :action => 'find'), :method => :get do %>
+<%= form_tag url_for(controller: "admin/admin_invitations", action: "find"), method: :get do %>
   <dl>
-    <dt><%= label_tag "invitation[user_name]", t(".username") %>:</dt>
-    <dd><%= text_field_tag "invitation[user_name]", params[:invitation][:user_name]  %></dd>
-    <dt><%= label_tag "invitation[token]", t(".token") %>:</dt>
-    <dd><%= text_field_tag "invitation[token]", params[:invitation][:token]  %></dd>
-    <dt><%= label_tag "invitee_email", t(".email") %>:</dt>
-    <dd><%= text_field_tag "invitation[invitee_email]", params[:invitation][:invitee_email], id: "invitee_email" %></dd>
+    <dt><%= label_tag "invitation[user_name]", t(".username") %></dt>
+    <dd><%= text_field_tag "invitation[user_name]", params[:invitation][:user_name], autocomplete: "on" %></dd>
+    <dt><%= label_tag "invitation[token]", t(".token") %></dt>
+    <dd><%= text_field_tag "invitation[token]", params[:invitation][:token], autocomplete: "on" %></dd>
+    <dt><%= label_tag "invitee_email", t(".email") %></dt>
+    <dd>
+      <%= text_field_tag "invitation[invitee_email]",
+            params[:invitation][:invitee_email],
+            autocomplete: "email",
+            id: "invitee_email" %>
+    </dd>
   </dl>
   <p class="submit actions"><%= submit_tag "Go" %></p>
 <% end %>
 
 <% if @user %>
-<h3 class="heading"><%h= 'Invitations for' %> <%= link_to @user.login, user_invitations_path(@user) %></h3>
-<%= render :partial => 'invitations/user_invitations', :locals => {:invitations => @invitations} %>
+<h3 class="heading"><%= t(".invitations_for") %> <%= link_to @user.login, user_invitations_path(@user) %></h3>
+<%= render partial: "invitations/user_invitations", locals: { invitations: @invitations } %>
 <% elsif @invitation %>
-<%= render :partial => 'invitations/invitation', :locals => {:invitation => @invitation} %>
+<%= render partial: "invitations/invitation", locals: { invitation: @invitation } %>
 <% elsif @invitations %>
-<%= render :partial => 'invitations/user_invitations', :locals => {:invitations => @invitations} %>
+<%= render partial: "invitations/user_invitations", locals: { invitations: @invitations } %>
 <% end %>
 <!--/content-->
 

app/views/challenge/gift_exchange/_challenge_signups.html.erb

@@ -33,7 +33,7 @@
 
   <dl class="index group">
     <% @challenge_signups.each do |signup| %>
-      <dt class="participant" title="<%= t(".particpant_username") %>">
+      <dt class="participant" title="<%= t(".participant_username") %>">
         <%= link_to signup.pseud.name, collection_signup_path(@collection, signup) %>
         <%= mailto_link signup.pseud.user, subject: "[#{h(@collection.title)}] Message from Collection Maintainer" %>
       </dt>

app/views/users/passwords/new.html.erb

@@ -7,7 +7,7 @@
 <%= form_for resource, url: user_password_path, html: { method: :post, html: "reset password simple post" } do |f| %>
   <%= error_messages_for resource %>
   <p>
-    <%= label_tag :reset_login, t(".email_or_username_html", or: style_bold(t(".or"))) %>
+    <%= label_tag :reset_login, t(".email_or_username_html", or: tag.strong(t(".or"))) %>
     <%= f.text_field :login, id: :reset_login %>
     <%= f.submit t(".reset_password") %>
   </p>

config/locales/controllers/en.yml

@@ -126,7 +126,6 @@ en:
       not_deleted: The pseud was not deleted.
       successfully_deleted: The pseud was successfully deleted.
     show:
-      could_not_find_pseud: Couldn't find pseud '%{pseud}'
       could_not_find_user: Couldn't find user '%{username}'
     update:
       successfully_updated: Pseud was successfully updated.

config/locales/views/en.yml

@@ -82,9 +82,11 @@ en:
         target: Target
     admin_invitations:
       find:
-        email: Enter all or part of an email address
-        token: Enter an invite token
-        username: Enter a username
+        email: 'Enter all or part of an email address:'
+        invitations_for: Invitations for
+        page_heading: Track invitations
+        token: 'Enter an invite token:'
+        username: 'Enter a username:'
       index:
         find:
           email: Enter all or part of an email address
@@ -510,7 +512,7 @@ en:
           search_by_pseud: Search By Pseud
         no_sign_ups_yet: No sign-ups yet!
         offers_html: Offers %{down_arrow}
-        particpant_username: username
+        participant_username: username
         requests_html: Requests %{down_arrow}
   challenge_signups:
     signup_form:

features/admins/users/admin_abuse_users.feature

@@ -227,3 +227,13 @@ Feature: Admin Abuse actions
     Then I should see "Are you sure you want to delete"
     When I press "Yes, Delete All Spammer Creations"
     Then I should see "All creations by user Spamster have been deleted."
+
+  Scenario: Rename a user with an inappropriate username
+    Given the user "otheruserstinks" exists and is activated
+      And I am logged in as a "policy_and_abuse" admin
+      And an abuse ticket ID exists
+    When I visit the change username page for otheruserstinks
+      And I fill in "Ticket ID" with "480000"
+      And I press "Change Username"
+    Then I should see "Username has been successfully updated."
+      But I should not see "otheruserstinks" within "h2.heading"

spec/models/user_spec.rb

@@ -236,6 +236,7 @@
         it "does not prevent changing the username" do
           allow(existing_user).to receive(:justification_enabled?).and_return(false)
           existing_user.update!(login: "user#{existing_user.id}")
+          expect(existing_user.login).to eq("user#{existing_user.id}")
         end
       end
     end