Refactor spam validation to a concern that can be easily reused and improve legibility in `SpamCheckService`akismet-ui-check

2 files changed, 37 insertions, 22 deletions

diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb
index 1085a1b93b8..5e2de2ccf64 100644
--- a/app/services/issues/create_service.rb
+++ b/app/services/issues/create_service.rb
@@ -2,14 +2,13 @@ module Issues
   class CreateService < Issues::BaseService
     def execute
       filter_params
-      label_params = params[:label_ids]
-      issue = project.issues.new(params.except(:label_ids, :request))
+      label_params = params.delete(:label_ids)
+      request = params.delete(:request)
+      api = params.delete(:api)
+      issue = project.issues.new(params)
       issue.author = params[:author] || current_user
 
-      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
+      issue.spam = spam_check_service.execute(request, api)
 
       if issue.save
         issue.update_attributes(label_ids: label_params)
@@ -22,5 +21,11 @@ module Issues
 
       issue
     end
+
+    private
+
+    def spam_check_service
+      SpamCheckService.new(project, current_user, params)
+    end
   end
 end
diff --git a/app/services/spam_check_service.rb b/app/services/spam_check_service.rb
index 6768047aa63..7c3e692bde9 100644
--- a/app/services/spam_check_service.rb
+++ b/app/services/spam_check_service.rb
@@ -1,28 +1,38 @@
-class SpamCheckService
+class SpamCheckService < BaseService
   include Gitlab::AkismetHelper
 
-  attr_accessor :subject, :current_user, :params
+  attr_accessor :request, :api
 
-  def initialize(subject, user, params = {})
-    @subject, @current_user, @params = subject, user, params.dup
-  end
+  def execute(request, api)
+    @request, @api = request, api
+    return false unless request || check_for_spam?(project)
+    return false unless is_spam?(request.env, current_user, text)
+    
+    create_spam_log
 
-  def spam_detected?
-    request = params[:request]
-    return false unless request || check_for_spam?(subject)
+    true
+  end
 
-    text = [params[:title], params[:description]].reject(&:blank?).join("\n")
+  private
 
-    return false unless is_spam?(request.env, current_user, text)
-
-    attrs = {
+  def text
+    [params[:title], params[:description]].reject(&:blank?).join("\n")
+  end
+  
+  def spam_log_attrs
+    {
       user_id: current_user.id,
-      project_id: subject.id,
+      project_id: project.id,
       title: params[:title],
-      description: params[:description]
+      description: params[:description],
+      source_ip: client_ip(request.env),
+      user_agent: user_agent(request.env),
+      noteable_type: 'Issue',
+      via_api: api
     }
-    create_spam_log(subject, current_user, attrs, request.env, api: false)
+  end
 
-    true
+  def create_spam_log
+    CreateSpamLogService.new(project, current_user, spam_log_attrs).execute
   end
 end