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