Merge branch 'akismet-submittable' into 'master'

Submit to Akismet Part 1 (Issues)

Related to #5932 #5573 gitlab-com/infrastructure#14

See merge request !5538

CHANGELOG

@@ -82,6 +82,7 @@ v 8.11.0 (unreleased)
   - Make "New issue" button in Issue page less obtrusive !5457 (winniehell)
   - Gitlab::Metrics.current_transaction needs to be public for RailsQueueDuration
   - Fix search for notes which belongs to deleted objects
+  - Allow Akismet to be trained by submitting issues as spam or ham !5538
   - Add GitLab Workhorse version to admin dashboard (Katarzyna Kobierska Ula Budziszewska)
   - Allow branch names ending with .json for graph and network page !5579 (winniehell)
   - Add the `sprockets-es6` gem

app/assets/stylesheets/framework/buttons.scss

@@ -164,6 +164,10 @@
     @include btn-outline($white-light, $orange-normal, $orange-normal, $orange-light, $white-light, $orange-light);
   }
 
+  &.btn-spam {
+    @include btn-outline($white-light, $red-normal, $red-normal, $red-light, $white-light, $red-light);
+  }
+
   &.btn-danger,
   &.btn-remove,
   &.btn-red {

app/controllers/admin/spam_logs_controller.rb

@@ -14,4 +14,14 @@ class Admin::SpamLogsController < Admin::ApplicationController
       head :ok
     end
   end
+
+  def mark_as_ham
+    spam_log = SpamLog.find(params[:id])
+
+    if HamService.new(spam_log).mark_as_ham!
+      redirect_to admin_spam_logs_path, notice: 'Spam log successfully submitted as ham.'
+    else
+      redirect_to admin_spam_logs_path, alert: 'Error with Akismet. Please check the logs for more info.'
+    end
+  end
 end

app/controllers/concerns/spammable_actions.rb

+module SpammableActions
+  extend ActiveSupport::Concern
+
+  included do
+    before_action :authorize_submit_spammable!, only: :mark_as_spam
+  end
+
+  def mark_as_spam
+    if SpamService.new(spammable).mark_as_spam!
+      redirect_to spammable, notice: "#{spammable.class.to_s} was submitted to Akismet successfully."
+    else
+      redirect_to spammable, alert: 'Error with Akismet. Please check the logs for more info.'
+    end
+  end
+
+  private
+
+  def spammable
+    raise NotImplementedError, "#{self.class} does not implement #{__method__}"
+  end
+
+  def authorize_submit_spammable!
+    access_denied! unless current_user.admin?
+  end
+end

app/controllers/projects/issues_controller.rb

@@ -4,6 +4,7 @@ class Projects::IssuesController < Projects::ApplicationController
   include IssuableActions
   include ToggleAwardEmoji
   include IssuableCollections
+  include SpammableActions
 
   before_action :redirect_to_external_issue_tracker, only: [:index, :new]
   before_action :module_enabled
@@ -185,6 +186,7 @@ class Projects::IssuesController < Projects::ApplicationController
   alias_method :subscribable_resource, :issue
   alias_method :issuable, :issue
   alias_method :awardable, :issue
+  alias_method :spammable, :issue
 
   def authorize_read_issue!
     return render_404 unless can?(current_user, :read_issue, @issue)

app/models/concerns/spammable.rb

 module Spammable
   extend ActiveSupport::Concern
 
+  module ClassMethods
+    def attr_spammable(attr, options = {})
+      spammable_attrs << [attr.to_s, options]
+    end
+  end
+
   included do
+    has_one :user_agent_detail, as: :subject, dependent: :destroy
+
     attr_accessor :spam
+
     after_validation :check_for_spam, on: :create
+
+    cattr_accessor :spammable_attrs, instance_accessor: false do
+      []
+    end
+
+    delegate :ip_address, :user_agent, to: :user_agent_detail, allow_nil: true
+  end
+
+  def submittable_as_spam?
+    if user_agent_detail
+      user_agent_detail.submittable?
+    else
+      false
+    end
   end
 
   def spam?
@@ -13,4 +36,33 @@ module Spammable
   def check_for_spam
     self.errors.add(:base, "Your #{self.class.name.underscore} has been recognized as spam and has been discarded.") if spam?
   end
+
+  def spam_title
+    attr = self.class.spammable_attrs.find do |_, options|
+      options.fetch(:spam_title, false)
+    end
+
+    public_send(attr.first) if attr && respond_to?(attr.first.to_sym)
+  end
+
+  def spam_description
+    attr = self.class.spammable_attrs.find do |_, options|
+      options.fetch(:spam_description, false)
+    end
+
+    public_send(attr.first) if attr && respond_to?(attr.first.to_sym)
+  end
+
+  def spammable_text
+    result = self.class.spammable_attrs.map do |attr|
+      public_send(attr.first)
+    end
+
+    result.reject(&:blank?).join("\n")
+  end
+
+  # Override in Spammable if further checks are necessary
+  def check_for_spam?
+    true
+  end
 end

app/models/issue.rb

@@ -36,6 +36,9 @@ class Issue < ActiveRecord::Base
   scope :order_due_date_asc, -> { reorder('issues.due_date IS NULL, issues.due_date ASC') }
   scope :order_due_date_desc, -> { reorder('issues.due_date IS NULL, issues.due_date DESC') }
 
+  attr_spammable :title, spam_title: true
+  attr_spammable :description, spam_description: true
+
   state_machine :state, initial: :opened do
     event :close do
       transition [:reopened, :opened] => :closed
@@ -262,4 +265,9 @@ class Issue < ActiveRecord::Base
   def overdue?
     due_date.try(:past?) || false
   end
+
+  # Only issues on public projects should be checked for spam
+  def check_for_spam?
+    project.public?
+  end
 end

app/models/spam_log.rb

@@ -7,4 +7,8 @@ class SpamLog < ActiveRecord::Base
     user.block
     user.destroy
   end
+
+  def text
+    [title, description].join("\n")
+  end
 end

app/models/user_agent_detail.rb

+class UserAgentDetail < ActiveRecord::Base
+  belongs_to :subject, polymorphic: true
+
+  validates :user_agent, :ip_address, :subject_id, :subject_type, presence: true
+
+  def submittable?
+    !submitted?
+  end
+end

app/services/akismet_service.rb

+class AkismetService
+  attr_accessor :owner, :text, :options
+
+  def initialize(owner, text, options = {})
+    @owner = owner
+    @text = text
+    @options = options
+  end
+
+  def is_spam?
+    return false unless akismet_enabled?
+
+    params = {
+      type: 'comment',
+      text: text,
+      created_at: DateTime.now,
+      author: owner.name,
+      author_email: owner.email,
+      referrer: options[:referrer],
+    }
+
+    begin
+      is_spam, is_blatant = akismet_client.check(options[:ip_address], options[:user_agent], params)
+      is_spam || is_blatant
+    rescue => e
+      Rails.logger.error("Unable to connect to Akismet: #{e}, skipping check")
+      false
+    end
+  end
+
+  def submit_ham
+    return false unless akismet_enabled?
+
+    params = {
+      type: 'comment',
+      text: text,
+      author: owner.name,
+      author_email: owner.email
+    }
+
+    begin
+      akismet_client.submit_ham(options[:ip_address], options[:user_agent], params)
+      true
+    rescue => e
+      Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!")
+      false
+    end
+  end
+
+  def submit_spam
+    return false unless akismet_enabled?
+
+    params = {
+      type: 'comment',
+      text: text,
+      author: owner.name,
+      author_email: owner.email
+    }
+
+    begin
+      akismet_client.submit_spam(options[:ip_address], options[:user_agent], params)
+      true
+    rescue => e
+      Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!")
+      false
+    end
+  end
+
+  private
+
+  def akismet_client
+    @akismet_client ||= ::Akismet::Client.new(current_application_settings.akismet_api_key,
+                                              Gitlab.config.gitlab.url)
+  end
+
+  def akismet_enabled?
+    current_application_settings.akismet_enabled
+  end
+end

app/services/create_spam_log_service.rb

-class CreateSpamLogService < BaseService
-  def initialize(project, user, params)
-    super(project, user, params)
-  end
-
-  def execute
-    spam_params = params.merge({ user_id: @current_user.id,
-                                 project_id: @project.id } )
-    spam_log = SpamLog.new(spam_params)
-    spam_log.save
-    spam_log
-  end
-end

app/services/ham_service.rb

+class HamService
+  attr_accessor :spam_log
+
+  def initialize(spam_log)
+    @spam_log = spam_log
+  end
+
+  def mark_as_ham!
+    if akismet.submit_ham
+      spam_log.update_attribute(:submitted_as_ham, true)
+    else
+      false
+    end
+  end
+
+  private
+
+  def akismet
+    @akismet ||= AkismetService.new(
+      spam_log.user,
+      spam_log.text,
+      ip_address: spam_log.source_ip,
+      user_agent: spam_log.user_agent
+    )
+  end
+end

app/services/issues/create_service.rb

@@ -3,29 +3,34 @@ module Issues
     def execute
       filter_params
       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
+      @request = params.delete(:request)
+      @api = params.delete(:api)
+      @issue = project.issues.new(params)
+      @issue.author = params[:author] || current_user
 
-      issue.spam = spam_check_service.execute(request, api)
+      @issue.spam = spam_service.check(@api)
 
-      if issue.save
-        issue.update_attributes(label_ids: label_params)
-        notification_service.new_issue(issue, current_user)
-        todo_service.new_issue(issue, current_user)
-        event_service.open_issue(issue, current_user)
-        issue.create_cross_references!(current_user)
-        execute_hooks(issue, 'open')
+      if @issue.save
+        @issue.update_attributes(label_ids: label_params)
+        notification_service.new_issue(@issue, current_user)
+        todo_service.new_issue(@issue, current_user)
+        event_service.open_issue(@issue, current_user)
+        user_agent_detail_service.create
+        @issue.create_cross_references!(current_user)
+        execute_hooks(@issue, 'open')
       end
 
-      issue
+      @issue
     end
 
     private
 
-    def spam_check_service
-      SpamCheckService.new(project, current_user, params)
+    def spam_service
+      SpamService.new(@issue, @request)
+    end
+
+    def user_agent_detail_service
+      UserAgentDetailService.new(@issue, @request)
     end
   end
 end

app/services/spam_check_service.rb

-class SpamCheckService < BaseService
-  include Gitlab::AkismetHelper
-
-  attr_accessor :request, :api
-
-  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
-
-    true
-  end
-
-  private
-
-  def text
-    [params[:title], params[:description]].reject(&:blank?).join("\n")
-  end
-  
-  def spam_log_attrs
-    {
-      user_id: current_user.id,
-      project_id: project.id,
-      title: params[:title],
-      description: params[:description],
-      source_ip: client_ip(request.env),
-      user_agent: user_agent(request.env),
-      noteable_type: 'Issue',
-      via_api: api
-    }
-  end
-
-  def create_spam_log
-    CreateSpamLogService.new(project, current_user, spam_log_attrs).execute
-  end
-end

app/services/spam_service.rb

+class SpamService
+  attr_accessor :spammable, :request, :options
+
+  def initialize(spammable, request = nil)
+    @spammable = spammable
+    @request = request
+    @options = {}
+
+    if @request
+      @options[:ip_address] = @request.env['action_dispatch.remote_ip'].to_s
+      @options[:user_agent] = @request.env['HTTP_USER_AGENT']
+      @options[:referrer] = @request.env['HTTP_REFERRER']
+    else
+      @options[:ip_address] = @spammable.ip_address
+      @options[:user_agent] = @spammable.user_agent
+    end
+  end
+
+  def check(api = false)
+    return false unless request && check_for_spam?
+
+    return false unless akismet.is_spam?
+
+    create_spam_log(api)
+    true
+  end
+
+  def mark_as_spam!
+    return false unless spammable.submittable_as_spam?
+
+    if akismet.submit_spam
+      spammable.user_agent_detail.update_attribute(:submitted, true)
+    else
+      false
+    end
+  end
+
+  private
+
+  def akismet
+    @akismet ||= AkismetService.new(
+      spammable_owner,
+      spammable.spammable_text,
+      options
+    )
+  end
+
+  def spammable_owner
+    @user ||= User.find(spammable_owner_id)
+  end
+
+  def spammable_owner_id
+    @owner_id ||=
+      if spammable.respond_to?(:author_id)
+        spammable.author_id
+      elsif spammable.respond_to?(:creator_id)
+        spammable.creator_id
+      end
+  end
+
+  def check_for_spam?
+    spammable.check_for_spam?
+  end
+
+  def create_spam_log(api)
+    SpamLog.create(
+      {
+        user_id: spammable_owner_id,
+        title: spammable.spam_title,
+        description: spammable.spam_description,
+        source_ip: options[:ip_address],
+        user_agent: options[:user_agent],
+        noteable_type: spammable.class.to_s,
+        via_api: api
+      }
+    )
+  end
+end

app/services/user_agent_detail_service.rb

+class UserAgentDetailService
+  attr_accessor :spammable, :request
+
+  def initialize(spammable, request)
+    @spammable, @request = spammable, request
+  end
+
+  def create
+    return unless request
+
+    spammable.create_user_agent_detail(user_agent: request.env['HTTP_USER_AGENT'], ip_address: request.env['action_dispatch.remote_ip'].to_s)
+  end
+end

app/views/admin/spam_logs/_spam_log.html.haml

@@ -24,6 +24,11 @@
       = link_to 'Remove user', admin_spam_log_path(spam_log, remove_user: true),
         data: { confirm: "USER #{user.name} WILL BE REMOVED! Are you sure?" }, method: :delete, class: "btn btn-xs btn-remove"
   %td
+    - if spam_log.submitted_as_ham?
+      .btn.btn-xs.disabled
+        Submitted as ham
+    - else
+      = link_to 'Submit as ham', mark_as_ham_admin_spam_log_path(spam_log), method: :post, class: 'btn btn-xs btn-warning'
     - if user && !user.blocked?
       = link_to 'Block user', block_admin_user_path(user), data: {confirm: 'USER WILL BE BLOCKED! Are you sure?'}, method: :put, class: "btn btn-xs"
     - else

app/views/projects/issues/show.html.haml

@@ -37,14 +37,19 @@
                 = link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue'
               %li
                 = link_to 'Edit', edit_namespace_project_issue_path(@project.namespace, @project, @issue)
+            - if @issue.submittable_as_spam? && current_user.admin?
+              %li
+                = link_to 'Submit as spam', mark_as_spam_namespace_project_issue_path(@project.namespace, @project, @issue), method: :post, class: 'btn-spam', title: 'Submit as spam'
+
         - if can?(current_user, :create_issue, @project)
           = link_to new_namespace_project_issue_path(@project.namespace, @project), class: 'hidden-xs hidden-sm btn btn-grouped new-issue-link btn-new btn-inverted', title: 'New issue', id: 'new_issue_link' do
             New issue
         - if can?(current_user, :update_issue, @issue)
           = link_to 'Reopen issue', issue_path(@issue, issue: { state_event: :reopen }, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "hidden-xs hidden-sm btn btn-grouped btn-reopen #{issue_button_visibility(@issue, false)}", title: 'Reopen issue'
           = link_to 'Close issue', issue_path(@issue, issue: { state_event: :close }, status_only: true, format: 'json'), data: {no_turbolink: true}, class: "hidden-xs hidden-sm btn btn-grouped btn-close #{issue_button_visibility(@issue, true)}", title: 'Close issue'
-          = link_to edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'hidden-xs hidden-sm btn btn-grouped issuable-edit' do
-            Edit
+          - if @issue.submittable_as_spam? && current_user.admin?
+            = link_to 'Submit as spam', mark_as_spam_namespace_project_issue_path(@project.namespace, @project, @issue), method: :post, class: 'hidden-xs hidden-sm btn btn-grouped btn-spam', title: 'Submit as spam'
+          = link_to 'Edit', edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'hidden-xs hidden-sm btn btn-grouped issuable-edit'
 
 
 .issue-details.issuable-details

config/routes.rb

@@ -252,7 +252,11 @@ Rails.application.routes.draw do
     resource :impersonation, only: :destroy
 
     resources :abuse_reports, only: [:index, :destroy]
-    resources :spam_logs, only: [:index, :destroy]
+    resources :spam_logs, only: [:index, :destroy] do
+      member do
+        post :mark_as_ham
+      end
+    end
 
     resources :applications
 
@@ -813,6 +817,7 @@ Rails.application.routes.draw do
           member do
             post :toggle_subscription
             post :toggle_award_emoji
+            post :mark_as_spam
             get :referenced_merge_requests
             get :related_branches
             get :can_create_branch

db/migrate/20160727163552_create_user_agent_details.rb

+class CreateUserAgentDetails < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  def change
+    create_table :user_agent_details do |t|
+      t.string :user_agent, null: false
+      t.string :ip_address, null: false
+      t.integer :subject_id, null: false
+      t.string :subject_type, null: false
+      t.boolean :submitted, default: false, null: false
+
+      t.timestamps null: false
+    end
+  end
+end

db/migrate/20160729173930_remove_project_id_from_spam_logs.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveProjectIdFromSpamLogs < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = true
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  DOWNTIME_REASON = 'Removing a column that contains data that is not used anywhere.'
+
+  # When using the methods "add_concurrent_index" or "add_column_with_default"
+  # you must disable the use of transactions as these methods can not run in an
+  # existing transaction. When using "add_concurrent_index" make sure that this
+  # method is the _only_ method called in the migration, any other changes
+  # should go in a separate migration. This ensures that upon failure _only_ the
+  # index creation fails and can be retried or reverted easily.
+  #
+  # To disable transactions uncomment the following line and remove these
+  # comments:
+  # disable_ddl_transaction!
+
+  def change
+    remove_column :spam_logs, :project_id, :integer
+  end
+end

db/migrate/20160801163709_add_submitted_as_ham_to_spam_logs.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddSubmittedAsHamToSpamLogs < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  # Set this constant to true if this migration requires downtime.
+  DOWNTIME = false
+
+  # When a migration requires downtime you **must** uncomment the following
+  # constant and define a short and easy to understand explanation as to why the
+  # migration requires downtime.
+  # DOWNTIME_REASON = ''
+
+  disable_ddl_transaction!
+
+  def change
+    add_column_with_default :spam_logs, :submitted_as_ham, :boolean, default: false
+  end
+end

db/schema.rb

@@ -926,12 +926,12 @@ ActiveRecord::Schema.define(version: 20160810142633) do
     t.string   "source_ip"
     t.string   "user_agent"
     t.boolean  "via_api"
-    t.integer  "project_id"
     t.string   "noteable_type"
     t.string   "title"
     t.text     "description"
     t.datetime "created_at",    null: false
     t.datetime "updated_at",    null: false
+    t.boolean  "submitted_as_ham", default: false, null: false
   end
 
   create_table "subscriptions", force: :cascade do |t|
@@ -999,6 +999,16 @@ ActiveRecord::Schema.define(version: 20160810142633) do
   add_index "u2f_registrations", ["key_handle"], name: "index_u2f_registrations_on_key_handle", using: :btree
   add_index "u2f_registrations", ["user_id"], name: "index_u2f_registrations_on_user_id", using: :btree
 
+  create_table "user_agent_details", force: :cascade do |t|
+    t.string   "user_agent",   null: false
+    t.string   "ip_address",   null: false
+    t.integer  "subject_id",   null: false
+    t.string   "subject_type", null: false
+    t.boolean  "submitted",    default: false, null: false
+    t.datetime "created_at",   null: false
+    t.datetime "updated_at",   null: false
+  end
+
   create_table "users", force: :cascade do |t|
     t.string   "email",                       default: "",    null: false
     t.string   "encrypted_password",          default: "",    null: false

doc/integration/akismet.md

@@ -33,3 +33,26 @@ To use Akismet:
 7. Save the configuration.
 
 ![Screenshot of Akismet settings](img/akismet_settings.png)
+
+
+## Training
+
+> *Note:* Training the Akismet filter is only available in 8.11 and above.
+
+As a way to better recognize between spam and ham, you can train the Akismet
+filter whenever there is a false positive or false negative.
+
+When an entry is recognized as spam, it is rejected and added to the Spam Logs. 
+From here you can review if they are really spam. If one of them is not really
+spam, you can use the `Submit as ham` button to tell Akismet that it falsely 
+recognized an entry as spam.
+
+![Screenshot of Spam Logs](img/spam_log.png)
+
+If an entry that is actually spam was not recognized as such, you will be able
+to also submit this to Akismet. The `Submit as spam` button will only appear
+to admin users.
+
+![Screenshot of Issue](img/submit_issue.png)
+
+Training Akismet will help it to recognize spam more accurately in the future.

lib/api/issues.rb

@@ -3,8 +3,6 @@ module API
   class Issues < Grape::API
     before { authenticate! }
 
-    helpers ::Gitlab::AkismetHelper
-
     helpers do
       def filter_issues_state(issues, state)
         case state

lib/gitlab/akismet_helper.rb

-module Gitlab
-  module AkismetHelper
-    def akismet_enabled?
-      current_application_settings.akismet_enabled
-    end
-
-    def akismet_client
-      @akismet_client ||= ::Akismet::Client.new(current_application_settings.akismet_api_key,
-        Gitlab.config.gitlab.url)
-    end
-
-    def client_ip(env)
-      env['action_dispatch.remote_ip'].to_s
-    end
-
-    def user_agent(env)
-      env['HTTP_USER_AGENT']
-    end
-
-    def check_for_spam?(project)
-      akismet_enabled? && project.public?
-    end
-
-    def is_spam?(environment, user, text)
-      client = akismet_client
-      ip_address = client_ip(environment)
-      user_agent = user_agent(environment)
-
-      params = {
-        type: 'comment',
-        text: text,
-        created_at: DateTime.now,
-        author: user.name,
-        author_email: user.email,
-        referrer: environment['HTTP_REFERER'],
-      }
-
-      begin
-        is_spam, is_blatant = client.check(ip_address, user_agent, params)
-        is_spam || is_blatant
-      rescue => e
-        Rails.logger.error("Unable to connect to Akismet: #{e}, skipping check")
-        false
-      end
-    end
-  end
-end

spec/controllers/admin/spam_logs_controller_spec.rb

@@ -34,4 +34,16 @@ describe Admin::SpamLogsController do
       expect { User.find(user.id) }.to raise_error(ActiveRecord::RecordNotFound)
     end
   end
+
+  describe '#mark_as_ham' do
+    before do
+      allow_any_instance_of(AkismetService).to receive(:submit_ham).and_return(true)
+    end
+    it 'submits the log as ham' do
+      post :mark_as_ham, id: first_spam.id
+
+      expect(response).to have_http_status(302)
+      expect(SpamLog.find(first_spam.id).submitted_as_ham).to be_truthy
+    end
+  end
 end

spec/controllers/projects/issues_controller_spec.rb

@@ -274,8 +274,8 @@ describe Projects::IssuesController do
   describe 'POST #create' do
     context 'Akismet is enabled' do
       before do
-        allow_any_instance_of(Gitlab::AkismetHelper).to receive(:check_for_spam?).and_return(true)
-        allow_any_instance_of(Gitlab::AkismetHelper).to receive(:is_spam?).and_return(true)
+        allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
+        allow_any_instance_of(AkismetService).to receive(:is_spam?).and_return(true)
       end
 
       def post_spam_issue
@@ -300,6 +300,52 @@ describe Projects::IssuesController do
         expect(spam_logs[0].title).to eq('Spam Title')
       end
     end
+
+    context 'user agent details are saved' do
+      before do
+        request.env['action_dispatch.remote_ip'] = '127.0.0.1'
+      end
+
+      def post_new_issue
+        sign_in(user)
+        project = create(:empty_project, :public)
+        post :create, {
+          namespace_id: project.namespace.to_param,
+          project_id: project.to_param,
+          issue: { title: 'Title', description: 'Description' }
+        }
+      end
+
+      it 'creates a user agent detail' do
+        expect{ post_new_issue }.to change(UserAgentDetail, :count).by(1)
+      end
+    end
+  end
+
+  describe 'POST #mark_as_spam' do
+    context 'properly submits to Akismet' do
+      before do
+        allow_any_instance_of(AkismetService).to receive_messages(submit_spam: true)
+        allow_any_instance_of(ApplicationSetting).to receive_messages(akismet_enabled: true)
+      end
+
+      def post_spam
+        admin = create(:admin)
+        create(:user_agent_detail, subject: issue)
+        project.team << [admin, :master]
+        sign_in(admin)
+        post :mark_as_spam, {
+          namespace_id: project.namespace.path,
+          project_id: project.path,
+          id: issue.iid
+        }
+      end
+
+      it 'updates issue' do
+        post_spam
+        expect(issue.submittable_as_spam?).to be_falsey
+      end
+    end
   end
 
   describe "DELETE #destroy" do

spec/factories/user_agent_details.rb

+FactoryGirl.define do
+  factory :user_agent_detail do
+    ip_address '127.0.0.1'
+    user_agent 'AppleWebKit/537.36'
+    association :subject, factory: :issue
+  end
+end

spec/lib/gitlab/akismet_helper_spec.rb

-require 'spec_helper'
-
-describe Gitlab::AkismetHelper, type: :helper do
-  let(:project) { create(:project, :public) }
-  let(:user) { create(:user) }
-
-  before do
-    allow(Gitlab.config.gitlab).to receive(:url).and_return(Settings.send(:build_gitlab_url))
-    allow_any_instance_of(ApplicationSetting).to receive(:akismet_enabled).and_return(true)
-    allow_any_instance_of(ApplicationSetting).to receive(:akismet_api_key).and_return('12345')
-  end
-
-  describe '#check_for_spam?' do
-    it 'returns true for public project' do
-      expect(helper.check_for_spam?(project)).to eq(true)
-    end
-
-    it 'returns false for private project' do
-      project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PRIVATE)
-      expect(helper.check_for_spam?(project)).to eq(false)
-    end
-  end
-
-  describe '#is_spam?' do
-    it 'returns true for spam' do
-      environment = {
-        'action_dispatch.remote_ip' => '127.0.0.1',
-        'HTTP_USER_AGENT' => 'Test User Agent'
-      }
-
-      allow_any_instance_of(::Akismet::Client).to receive(:check).and_return([true, true])
-      expect(helper.is_spam?(environment, user, 'Is this spam?')).to eq(true)
-    end
-  end
-end

spec/models/concerns/spammable_spec.rb

+require 'spec_helper'
+
+describe Issue, 'Spammable' do
+  let(:issue) { create(:issue, description: 'Test Desc.') }
+
+  describe 'Associations' do
+    it { is_expected.to have_one(:user_agent_detail).dependent(:destroy) }
+  end
+
+  describe 'ClassMethods' do
+    it 'should return correct attr_spammable' do
+      expect(issue.spammable_text).to eq("#{issue.title}\n#{issue.description}")
+    end
+  end
+
+  describe 'InstanceMethods' do
+    it 'should be invalid if spam' do
+      issue = build(:issue, spam: true)
+      expect(issue.valid?).to be_falsey
+    end
+
+    describe '#check_for_spam?' do
+      it 'returns true for public project' do
+        issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC)
+        expect(issue.check_for_spam?).to eq(true)
+      end
+
+      it 'returns false for other visibility levels' do
+        expect(issue.check_for_spam?).to eq(false)
+      end
+    end
+  end
+end

spec/models/user_agent_detail_spec.rb

+require 'rails_helper'
+
+describe UserAgentDetail, type: :model do
+  describe '.submittable?' do
+    it 'is submittable when not already submitted' do
+      detail = build(:user_agent_detail)
+
+      expect(detail.submittable?).to be_truthy
+    end
+
+    it 'is not submittable when already submitted' do
+      detail = build(:user_agent_detail, submitted: true)
+
+      expect(detail.submittable?).to be_falsey
+    end
+  end
+
+  describe '.valid?' do
+    it 'is valid with a subject' do
+      detail = build(:user_agent_detail)
+
+      expect(detail).to be_valid
+    end
+
+    it 'is invalid without a subject' do
+      detail = build(:user_agent_detail, subject: nil)
+
+      expect(detail).not_to be_valid
+    end
+  end
+end

spec/requests/api/issues_spec.rb

@@ -531,8 +531,8 @@ describe API::API, api: true  do
 
   describe 'POST /projects/:id/issues with spam filtering' do
     before do
-      allow_any_instance_of(Gitlab::AkismetHelper).to receive(:check_for_spam?).and_return(true)
-      allow_any_instance_of(Gitlab::AkismetHelper).to receive(:is_spam?).and_return(true)
+      allow_any_instance_of(SpamService).to receive(:check_for_spam?).and_return(true)
+      allow_any_instance_of(AkismetService).to receive_messages(is_spam?: true)
     end
 
     let(:params) do
@@ -554,7 +554,6 @@ describe API::API, api: true  do
       expect(spam_logs[0].description).to eq('content here')
       expect(spam_logs[0].user).to eq(user)
       expect(spam_logs[0].noteable_type).to eq('Issue')
-      expect(spam_logs[0].project_id).to eq(project.id)
     end
   end