From 46a220ae3c0e646aac63a3230399fcc8979df6ec Mon Sep 17 00:00:00 2001
From: Robert Speicher <rspeicher@gmail.com>
Date: Mon, 4 Jan 2016 18:59:42 -0500
Subject: [PATCH] Add `AbuseReport#notify`

Tell, Don't Ask.
---
 app/controllers/abuse_reports_controller.rb   |  4 +-
 app/models/abuse_report.rb                    |  6 ++
 .../abuse_reports_controller_spec.rb          | 80 ++++++-------------
 spec/models/abuse_report_spec.rb              | 17 ++++
 4 files changed, 49 insertions(+), 58 deletions(-)

diff --git a/app/controllers/abuse_reports_controller.rb b/app/controllers/abuse_reports_controller.rb
index 5718fd22de9..38814459f66 100644
--- a/app/controllers/abuse_reports_controller.rb
+++ b/app/controllers/abuse_reports_controller.rb
@@ -9,9 +9,7 @@ class AbuseReportsController < ApplicationController
     @abuse_report.reporter = current_user
 
     if @abuse_report.save
-      if current_application_settings.admin_notification_email.present?
-        AbuseReportMailer.notify(@abuse_report.id).deliver_later
-      end
+      @abuse_report.notify
 
       message = "Thank you for your report. A GitLab administrator will look into it shortly."
       redirect_to @abuse_report.user, notice: message
diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb
index 89b3116b9f2..55864236b2f 100644
--- a/app/models/abuse_report.rb
+++ b/app/models/abuse_report.rb
@@ -18,4 +18,10 @@ class AbuseReport < ActiveRecord::Base
   validates :user, presence: true
   validates :message, presence: true
   validates :user_id, uniqueness: true
+
+  def notify
+    return unless self.persisted?
+
+    AbuseReportMailer.notify(self.id).deliver_later
+  end
 end
diff --git a/spec/controllers/abuse_reports_controller_spec.rb b/spec/controllers/abuse_reports_controller_spec.rb
index 15824a1c67f..80a418feb3e 100644
--- a/spec/controllers/abuse_reports_controller_spec.rb
+++ b/spec/controllers/abuse_reports_controller_spec.rb
@@ -1,76 +1,46 @@
 require 'spec_helper'
 
 describe AbuseReportsController do
-  let(:reporter)    { create(:user) }
-  let(:user)        { create(:user) }
-  let(:message)     { "This user is a spammer" }
+  let(:reporter) { create(:user) }
+  let(:user)     { create(:user) }
+  let(:attrs) do
+    attributes_for(:abuse_report) do |hash|
+      hash[:user_id] = user.id
+    end
+  end
 
   before do
     sign_in(reporter)
   end
 
-  describe "POST create" do
-    context "with admin notification email set" do
-      let(:admin_email) { "admin@example.com"}
-
-      before(:each) do
-        stub_application_setting(admin_notification_email: admin_email)
+  describe 'POST create' do
+    context 'with valid attributes' do
+      it 'saves the abuse report' do
+        expect do
+          post :create, abuse_report: attrs
+        end.to change { AbuseReport.count }.by(1)
       end
 
-      it "sends a notification email" do
-        perform_enqueued_jobs do
-          post :create,
-            abuse_report: {
-              user_id: user.id,
-              message: message
-            }
+      it 'calls notify' do
+        expect_any_instance_of(AbuseReport).to receive(:notify)
 
-          email = ActionMailer::Base.deliveries.last
-
-          expect(email.to).to eq([admin_email])
-          expect(email.subject).to include(user.username)
-          expect(email.text_part.body).to include(message)
-        end
+        post :create, abuse_report: attrs
       end
 
-      it "saves the abuse report" do
-        perform_enqueued_jobs do
-          expect do
-            post :create,
-              abuse_report: {
-                user_id: user.id,
-                message: message
-              }
-          end.to change { AbuseReport.count }.by(1)
-        end
+      it 'redirects back to the reported user' do
+        post :create, abuse_report: attrs
+
+        expect(response).to redirect_to user
       end
     end
 
-    context "without admin notification email set" do
-      before(:each) do
-        stub_application_setting(admin_notification_email: nil)
-      end
+    context 'with invalid attributes' do
+      it 'renders new' do
+        attrs.delete(:user_id)
+        post :create, abuse_report: attrs
 
-      it "does not send a notification email" do
-        expect do
-          post :create,
-            abuse_report: {
-              user_id: user.id,
-              message: message
-            }
-        end.not_to change { ActionMailer::Base.deliveries.count }
-      end
-
-      it "saves the abuse report" do
-        expect do
-          post :create,
-            abuse_report: {
-              user_id: user.id,
-              message: message
-            }
-        end.to change { AbuseReport.count }.by(1)
+        expect(response).to render_template(:new)
       end
     end
   end
-
 end
diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb
index d45319b25d4..46cab1644c7 100644
--- a/spec/models/abuse_report_spec.rb
+++ b/spec/models/abuse_report_spec.rb
@@ -28,4 +28,21 @@ RSpec.describe AbuseReport, type: :model do
     it { is_expected.to validate_presence_of(:message) }
     it { is_expected.to validate_uniqueness_of(:user_id) }
   end
+
+  describe '#notify' do
+    it 'delivers' do
+      expect(AbuseReportMailer).to receive(:notify).with(subject.id).
+        and_return(spy)
+
+      subject.notify
+    end
+
+    it 'returns early when not persisted' do
+      report = build(:abuse_report)
+
+      expect(AbuseReportMailer).not_to receive(:notify)
+
+      report.notify
+    end
+  end
 end