From da40274fdc60fe17f928b80eb71c211e27523d5e Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 12 Jan 2016 20:48:16 -0500 Subject: [PATCH] Block the reported user before destroying the record This is intended to prevent the user from creating new objects while the transaction that removes them is being run, resulting in objects with nil authors which can then not be edited. See https://gitlab.com/gitlab-org/gitlab-ce/issues/7117 --- .../admin/abuse_reports_controller.rb | 6 ++---- app/models/abuse_report.rb | 5 +++++ spec/models/abuse_report_spec.rb | 16 ++++++++++++++++ 3 files changed, 23 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/abuse_reports_controller.rb b/app/controllers/admin/abuse_reports_controller.rb index 38a5a9fca08..2463cfa87be 100644 --- a/app/controllers/admin/abuse_reports_controller.rb +++ b/app/controllers/admin/abuse_reports_controller.rb @@ -6,11 +6,9 @@ class Admin::AbuseReportsController < Admin::ApplicationController def destroy abuse_report = AbuseReport.find(params[:id]) - if params[:remove_user] - abuse_report.user.destroy - end - + abuse_report.remove_user if params[:remove_user] abuse_report.destroy + render nothing: true end end diff --git a/app/models/abuse_report.rb b/app/models/abuse_report.rb index 55864236b2f..2bc15c60d57 100644 --- a/app/models/abuse_report.rb +++ b/app/models/abuse_report.rb @@ -19,6 +19,11 @@ class AbuseReport < ActiveRecord::Base validates :message, presence: true validates :user_id, uniqueness: true + def remove_user + user.block + user.destroy + end + def notify return unless self.persisted? diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index 46cab1644c7..f9be8fcbcfe 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -29,6 +29,22 @@ RSpec.describe AbuseReport, type: :model do it { is_expected.to validate_uniqueness_of(:user_id) } end + describe '#remove_user' do + it 'blocks the user' do + report = build(:abuse_report) + + allow(report.user).to receive(:destroy) + + expect { report.remove_user }.to change { report.user.blocked? }.to(true) + end + + it 'removes the user' do + report = build(:abuse_report) + + expect { report.remove_user }.to change { User.count }.by(-1) + end + end + describe '#notify' do it 'delivers' do expect(AbuseReportMailer).to receive(:notify).with(subject.id).