Refactor Admin::SpamLogsController to block user before destroying

6 files changed, 46 insertions, 48 deletions

diff --git a/app/controllers/admin/spam_logs_controller.rb b/app/controllers/admin/spam_logs_controller.rb
index 69f94dd3ba8..377e9741e5f 100644
--- a/app/controllers/admin/spam_logs_controller.rb
+++ b/app/controllers/admin/spam_logs_controller.rb
@@ -1,33 +1,17 @@
 class Admin::SpamLogsController < Admin::ApplicationController
-  before_action :set_spam_log, only: [:destroy]
-
   def index
-    @spam_logs = SpamLog.order(created_at: :desc).page(params[:page])
+    @spam_logs = SpamLog.order(id: :desc).page(params[:page])
   end
 
   def destroy
-    @spam_log.destroy
-    message = 'Spam log was successfully destroyed.'
+    spam_log = SpamLog.find(params[:id])
 
     if params[:remove_user]
-      username = @spam_log.user.username
-      @spam_log.user.destroy
-      message = "User #{username} was successfully destroyed."
-    end
-
-    respond_to do |format|
-      format.json { render json: '{}' }
-      format.html { redirect_to admin_spam_logs_path, notice: message }
+      spam_log.remove_user
+      redirect_to admin_spam_logs_path, notice: "User #{spam_log.user.username} was successfully removed."
+    else
+      spam_log.destroy
+      render nothing: true
     end
   end
-
-  private
-
-  def set_spam_log
-    @spam_log = SpamLog.find(params[:id])
-  end
-
-  def spam_log_params
-    params[:spam_log]
-  end
 end
diff --git a/app/models/spam_log.rb b/app/models/spam_log.rb
index b7b32b478a7..12df68ef83b 100644
--- a/app/models/spam_log.rb
+++ b/app/models/spam_log.rb
@@ -2,4 +2,9 @@ class SpamLog < ActiveRecord::Base
   belongs_to :user
 
   validates :user, presence: true
+
+  def remove_user
+    user.block
+    user.destroy
+  end
 end
diff --git a/app/views/admin/spam_logs/_spam_log.html.haml b/app/views/admin/spam_logs/_spam_log.html.haml
index 51e94acce22..8aea67f4497 100644
--- a/app/views/admin/spam_logs/_spam_log.html.haml
+++ b/app/views/admin/spam_logs/_spam_log.html.haml
@@ -4,13 +4,15 @@
     = time_ago_with_tooltip(spam_log.created_at)
   %td
     - if user
-      = link_to user.name, user
+      = link_to user.name, [:admin, user]
+      .light.small
+        Joined #{time_ago_with_tooltip(user.created_at)}
     - else
       (removed)
   %td
     = spam_log.source_ip
   %td
-    = spam_log.via_api ? 'Y' : 'N'
+    = spam_log.via_api? ? 'Y' : 'N'
   %td
     = spam_log.noteable_type
   %td
@@ -27,4 +29,4 @@
     - else
       .btn.btn-xs.disabled
         Already Blocked
-    = link_to 'Remove log', [:admin, spam_log, format: :json], remote: true, method: :delete, class: "btn btn-xs btn-close js-remove-tr"
+    = link_to 'Remove log', [:admin, spam_log], remote: true, method: :delete, class: "btn btn-xs btn-close js-remove-tr"
diff --git a/app/views/admin/spam_logs/index.html.haml b/app/views/admin/spam_logs/index.html.haml
index 439468e572b..0fdd5bd9960 100644
--- a/app/views/admin/spam_logs/index.html.haml
+++ b/app/views/admin/spam_logs/index.html.haml
@@ -9,7 +9,7 @@
           %th Date
           %th User
           %th Source IP
-          %th Via API?
+          %th API?
           %th Type
           %th Title
           %th Description
diff --git a/spec/controllers/admin/spam_logs_controller_spec.rb b/spec/controllers/admin/spam_logs_controller_spec.rb
index 2486298fc78..6beebc362e9 100644
--- a/spec/controllers/admin/spam_logs_controller_spec.rb
+++ b/spec/controllers/admin/spam_logs_controller_spec.rb
@@ -1,8 +1,10 @@
 require 'spec_helper'
 
 describe Admin::SpamLogsController do
-  let(:admin)    { create(:admin) }
-  let(:spam_log) { create(:spam_log, user: admin) }
+  let(:admin) { create(:admin) }
+  let(:user) { create(:user) }
+  let!(:first_spam) { create(:spam_log, user: user) }
+  let!(:second_spam) { create(:spam_log, user: user) }
 
   before do
     sign_in(admin)
@@ -11,37 +13,28 @@ describe Admin::SpamLogsController do
   describe '#index' do
     it 'lists all spam logs' do
       get :index
+
       expect(response.status).to eq(200)
     end
   end
 
   describe '#destroy' do
-    it 'destroys just spam log' do
-      user = spam_log.user
-      delete :destroy, id: spam_log.id
+    it 'removes only the spam log when removing log' do
+      expect {
+        delete :destroy, id: first_spam.id
+      }.to change { SpamLog.count }.by(-1)
 
-      expect(SpamLog.all.count).to eq(0)
       expect(User.find(user.id)).to be_truthy
-      expect(response.status).to eq(302)
+      expect(response.status).to eq(200)
     end
 
-    it 'destroys user and his spam logs' do
-      user = spam_log.user
-      delete :destroy, id: spam_log.id, remove_user: true
+    it 'removes user and his spam logs when removing the user' do
+      delete :destroy, id: first_spam.id, remove_user: true
 
-      expect(SpamLog.all.count).to eq(0)
-      expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
+      expect(flash[:notice]).to eq "User #{user.username} was successfully removed."
       expect(response.status).to eq(302)
-    end
-
-    it 'destroys user and his spam logs with JSON format' do
-      user = spam_log.user
-      delete :destroy, id: spam_log.id, remove_user: true, format: :json
-
-      expect(SpamLog.all.count).to eq(0)
+      expect(SpamLog.count).to eq(0)
       expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
-      expect(JSON.parse(response.body)).to eq({})
-      expect(response.status).to eq(200)
     end
   end
 end
diff --git a/spec/models/spam_log_spec.rb b/spec/models/spam_log_spec.rb
index 076f3e44c70..c4ec7625cb0 100644
--- a/spec/models/spam_log_spec.rb
+++ b/spec/models/spam_log_spec.rb
@@ -8,4 +8,18 @@ describe SpamLog, models: true do
   describe 'validations' do
     it { is_expected.to validate_presence_of(:user) }
   end
+
+  describe '#remove_user' do
+    it 'blocks the user' do
+      spam_log = build(:spam_log)
+
+      expect { spam_log.remove_user }.to change { spam_log.user.blocked? }.to(true)
+    end
+
+    it 'removes the user' do
+      spam_log = build(:spam_log)
+
+      expect { spam_log.remove_user }.to change { User.count }.by(-1)
+    end
+  end
 end