diff --git a/app/controllers/admin/spam_logs_controller.rb b/app/controllers/admin/spam_logs_controller.rb index bc6fce0ec4e..d15f00bf84c 100644 --- a/app/controllers/admin/spam_logs_controller.rb +++ b/app/controllers/admin/spam_logs_controller.rb @@ -1,4 +1,6 @@ class Admin::SpamLogsController < Admin::ApplicationController + include Gitlab::AkismetHelper + def index @spam_logs = SpamLog.order(id: :desc).page(params[:page]) end @@ -15,9 +17,14 @@ class Admin::SpamLogsController < Admin::ApplicationController end end - def ham + def mark_as_ham spam_log = SpamLog.find(params[:id]) - Gitlab::AkismetHelper.ham!(spam_log.source_ip, spam_log.user_agent, spam_log.description, spam_log.user) + if ham!(spam_log.source_ip, spam_log.user_agent, spam_log.text, spam_log.user) + spam_log.update_attribute(:submitted_as_ham, true) + redirect_to admin_spam_logs_path, notice: 'Spam log successfully submitted as ham.' + else + redirect_to admin_spam_logs_path, notice: 'Error with Akismet. Please check the logs for more info.' + end end end diff --git a/app/models/spam_log.rb b/app/models/spam_log.rb index 12df68ef83b..3b8b9833565 100644 --- a/app/models/spam_log.rb +++ b/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 diff --git a/app/views/admin/spam_logs/_spam_log.html.haml b/app/views/admin/spam_logs/_spam_log.html.haml index 8aea67f4497..f2b6106fb4a 100644 --- a/app/views/admin/spam_logs/_spam_log.html.haml +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index 9a98fab15a3..8214bc26d59 100644 --- a/config/routes.rb +++ b/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 diff --git a/db/migrate/20160801163709_add_submitted_as_ham_to_spam_logs.rb b/db/migrate/20160801163709_add_submitted_as_ham_to_spam_logs.rb new file mode 100644 index 00000000000..296f1dfac7b --- /dev/null +++ b/db/migrate/20160801163709_add_submitted_as_ham_to_spam_logs.rb @@ -0,0 +1,20 @@ +# 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 diff --git a/db/schema.rb b/db/schema.rb index cc881e54763..355eed13b68 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -931,6 +931,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do 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| diff --git a/lib/gitlab/akismet_helper.rb b/lib/gitlab/akismet_helper.rb index b74d8176cc7..bd71a1aaa51 100644 --- a/lib/gitlab/akismet_helper.rb +++ b/lib/gitlab/akismet_helper.rb @@ -52,8 +52,10 @@ module Gitlab begin client.submit_ham(ip_address, user_agent, params) + true rescue => e Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!") + false end end @@ -69,8 +71,10 @@ module Gitlab begin client.submit_spam(details.ip_address, details.user_agent, params) + true rescue => e Rails.logger.error("Unable to connect to Akismet: #{e}, skipping!") + false end end end diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 3f7d2721d22..7492c42f71e 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -45,10 +45,11 @@ describe Issue, 'Spammable' do describe 'AkismetMethods' do before do - allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(is_spam?: true, spam!: nil) + allow_any_instance_of(Gitlab::AkismetHelper).to receive_messages(is_spam?: true, spam!: true, akismet_enabled?: true) + allow_any_instance_of(Spammable).to receive(:can_be_submitted?).and_return(true) end it { expect(issue.spam_detected?(:mock_env)).to be_truthy } - it { expect(issue.submit_spam).to be_nil } + it { expect(issue.submit_spam).to be_truthy } end end