Fix Banisher Updates (#1704)

* delete activity before updating username

* fix linting

* remove unnecssary return

app/models/user.rb

@@ -286,6 +286,10 @@ class User < ApplicationRecord
     has_role? :warned
   end
 
+  def banished?
+    user.notes.where(reason: "banned", content: "spam account").any? && user.banned
+  end
+
   def banned_from_mentorship
     has_role? :banned_from_mentorship
   end

app/services/moderator/banisher.rb

@@ -22,7 +22,7 @@ module Moderator
         new_name = "spam_#{rand(10000)}"
         new_username = "spam_#{rand(10000)}"
       end
-      user.update_columns(name: new_name, username: new_username, old_username: user.name)
+      user.update_columns(name: new_name, username: new_username, old_username: user.username)
       CacheBuster.new.bust("/#{user.old_username}")
     end
 
@@ -90,12 +90,11 @@ module Moderator
 
     def banish
       user.unsubscribe_from_newsletters
-      reassign_and_bust_username
       remove_profile_info
       add_banned_role
       delete_user_activity
       user.remove_from_algolia_index
-      user.save!
+      reassign_and_bust_username
     end
   end
 end

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

@@ -1,5 +1,5 @@
 <h1>
-    <%= @user.name%><a href="/<%= @user.username %>" target="_blank"> (@<%= @user.username %>)</a>
+    <%= @user.name %><a href="/<%= @user.username %>" target="_blank"> (@<%= @user.username %>)</a>
 </h1>
 <p><u><a style="color: green" href="/internal/users/<%= @user.id %>">click here view user details & take other actions</a></u></p>
 <p><em>Member since <%= @user.created_at.strftime("%b %e '%y") %></em></p>
@@ -11,17 +11,19 @@
 </ul>
   <div class="row">
     <h2>Banish User</h2>
-    <% if @user.comments.where("created_at < ?", 100.days.ago).empty? && @user.created_at < 100.days.ago %>
+    <% if @user.banished? %>
+      <p>User has been banished.</p>
+    <% elsif @user.comments.where("created_at < ?", 100.days.ago).empty? && @user.created_at < 100.days.ago %>
         <p><em>This is extremely destructive. Banishing will delete all the user's existing content and change their username to @spam_###.</em></p>
         <p><strong>Do not do this lightly.</strong></p>
-         <%= form_for(@user, :url => banish_internal_user_path(@user), :html => {:method => :post, :onsubmit => "return confirm('Are you sure? This is extremely destructive and irreversible. Banishing will delete all articles and turn their username into @spam_###')"}) do %>
+        <%= form_for(@user, url: banish_internal_user_path(@user), html: { method: :post, onsumbit: "return confirm('Are you sure? This is extremely destructive and irreversible. Banishing will delete all articles and turn their username into @spam_###')" }) do %>
           <button class="btn btn-danger" style="font-size:2em;">Banish User for Spam</button>
         <% end %>
     <% elsif current_user.has_role?(:super_admin) %>
       <p><strong>This is not a new user.</strong> You are only allowed to take this action because you are a <strong>super admin.</strong></p>
       <p><em>This is extremely destructive. Banishing will delete all the user's existing content and change their username to @spam_###.</em></p>
       <p><strong>Do not do this lightly.</strong></p>
-      <%= form_for(@user, :url => banish_internal_user_path(@user), :html => {:method => :post, :onsubmit => "return confirm('Are you sure? This is extremely destructive and irreversible.Banishing will delete all articles and turn their username into @spam_###')"}) do %>
+      <%= form_for(@user, url: banish_internal_user_path(@user), html: { method: :post, onsumbit: "return confirm('Are you sure? This is extremely destructive and irreversible.Banishing will delete all articles and turn their username into @spam_###')" }) do %>
         <button class="btn btn-danger" style="font-size:2em;">Banish User for Spam!</button>
       <% end %>
     <% else %>
@@ -34,12 +36,10 @@
   <% if current_user.has_role?(:super_admin) %>
     <p>This will <strong>completely destroy the user</strong> and all of their activity from our database. This action is irreversible.</p>
     <p><strong>Do not do this lightly.</strong></p>
-    <%= form_for(@user, :url => full_delete_internal_user_path(@user), :html => {:method => :post, :onsubmit => "return confirm('Are you sure? This is extremely destructive and irreversible.')"}) do %>
+    <%= form_for(@user, url: full_delete_internal_user_path(@user), html: { method: :post, onsubmit: "return confirm('Are you sure? This is extremely destructive and irreversible.')" }) do %>
       <button class="btn btn-danger" style="font-size:2em;">Fully Delete User & All Activity</button>
     <% end %>
   <% else %>
     <p>Only super admins can fully delete users.</p>
   <% end %>
 </div>
-
-