Refactor `SpamCheckService` to make it cleaner and clearer.

CHANGELOG

@@ -93,7 +93,6 @@ v 8.10.0
   - Fix viewing notification settings when a project is pending deletion
   - Updated compare dropdown menus to use GL dropdown
   - Redirects back to issue after clicking login link
-  - Submit issues created via the WebUI by non project members to Akismet !5333
   - All created issues, API or WebUI, can be submitted to Akismet for spam check !5333
   - Eager load award emoji on notes
   - Allow to define manual actions/builds on Pipelines and Environments

app/controllers/projects/issues_controller.rb

@@ -81,15 +81,9 @@ class Projects::IssuesController < Projects::ApplicationController
   def create
     @issue = Issues::CreateService.new(project, current_user, issue_params.merge({ request: request })).execute
 
-    if @issue.nil?
-      @issue = @project.issues.new
-      flash[:notice] = 'Your issue has been recognized as spam and has been discarded.'
-      render :new and return
-    end
-
     respond_to do |format|
       format.html do
-        if @issue.valid?
+        if @issue.errors.empty? && @issue.valid?
           redirect_to issue_path(@issue)
         else
           render :new

app/services/issues/create_service.rb

@@ -6,8 +6,9 @@ module Issues
       issue = project.issues.new(params.except(:label_ids, :request))
       issue.author = params[:author] || current_user
 
-      if Issues::SpamCheckService.new(project, current_user, params).spam_detected?
-        return nil
+      if SpamCheckService.new(project, current_user, params).spam_detected?
+        issue.errors.add(:base, 'Your issue has been recognized as spam and has been discarded.')
+        return issue
       end
 
       if issue.save

app/services/issues/spam_check_service.rb

-module Issues
-  class SpamCheckService < BaseService
-    include Gitlab::AkismetHelper
-
-    def spam_detected?
-      text = [params[:title], params[:description]].reject(&:blank?).join("\n")
-      request = params[:request]
-
-      if request
-        if check_for_spam?(project) && is_spam?(request.env, current_user, text)
-          attrs = {
-            user_id: current_user.id,
-            project_id: project.id,
-            title: params[:title],
-            description: params[:description]
-          }
-          create_spam_log(project, current_user, attrs, request.env, api: false)
-          return true
-        end
-      end
-
-      false
-    end
-  end
-end

app/services/spam_check_service.rb

+class SpamCheckService
+  include Gitlab::AkismetHelper
+
+  attr_accessor :subject, :current_user, :params
+
+  def initialize(subject, user, params = {})
+    @subject, @current_user, @params = subject, user, params.dup
+  end
+
+  def spam_detected?
+    request = params[:request]
+    return false unless request || check_for_spam?(subject)
+
+    text = [params[:title], params[:description]].reject(&:blank?).join("\n")
+
+    return false unless is_spam?(request.env, current_user, text)
+
+    attrs = {
+      user_id: current_user.id,
+      project_id: subject.id,
+      title: params[:title],
+      description: params[:description]
+    }
+    create_spam_log(subject, current_user, attrs, request.env, api: false)
+
+    true
+  end
+end

lib/api/issues.rb

@@ -160,11 +160,13 @@ module API
 
         issue = ::Issues::CreateService.new(project, current_user, attrs.merge({ request: request })).execute
 
-        if issue.nil?
-          render_api_error!({ error: 'Spam detected' }, 400)
-        end
-
         if issue.valid?
+          # Need to check if id is nil here, because if issue is spam, errors
+          # get added, but Rails still thinks it's valid, but it is never saved
+          # so id will be nil
+          if issue.id.nil?
+            render_api_error!({ error: 'Spam detected' }, 400)
+          end
           # Find or create labels and attach to issue. Labels are valid because
           # we already checked its name, so there can't be an error here
           if params[:labels].present?