Commit c2908b9e authored by Mayra Cabrera's avatar Mayra Cabrera

Merge branch 'revert-c6b84d75' into 'master'

Revert "Merge branch 'akismet-spammable-rename' into 'master'"

See merge request gitlab-org/gitlab!25272
parents b6f86010 268bff66
......@@ -11,7 +11,7 @@ module SpammableActions
end
def mark_as_spam
if Spam::MarkAsSpamService.new(target: spammable).execute
if Spam::MarkAsSpamService.new(spammable: spammable).execute
redirect_to spammable_path, notice: _("%{spammable_titlecase} was submitted to Akismet successfully.") % { spammable_titlecase: spammable.spammable_entity_type.titlecase }
else
redirect_to spammable_path, alert: _('Error with Akismet. Please check the logs for more info.')
......@@ -42,7 +42,7 @@ module SpammableActions
end
format.json do
locals = { target: spammable, script: false, has_submit: false }
locals = { spammable: spammable, script: false, has_submit: false }
recaptcha_html = render_to_string(partial: 'shared/recaptcha_form', formats: :html, locals: locals)
render json: { recaptcha_html: recaptcha_html }
......
......@@ -24,7 +24,7 @@ module Mutations
private
def mark_as_spam(snippet)
Spam::MarkAsSpamService.new(target: snippet).execute
Spam::MarkAsSpamService.new(spammable: snippet).execute
end
def authorized_resource?(snippet)
......
# frozen_string_literal: true
module AkismetMethods
def target_owner
@user ||= User.find(target.author_id)
def spammable_owner
@user ||= User.find(spammable.author_id)
end
def akismet
@akismet ||= Spam::AkismetService.new(
target_owner.name,
target_owner.email,
target.try(:spammable_text) || target&.text,
spammable_owner.name,
spammable_owner.email,
spammable.try(:spammable_text) || spammable&.text,
options
)
end
......
......@@ -2,7 +2,7 @@
# SpamCheckMethods
#
# Provide helper methods for checking if a given target spammable object has
# Provide helper methods for checking if a given spammable object has
# potential spam data.
#
# Dependencies:
......@@ -18,13 +18,13 @@ module SpamCheckMethods
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
# In order to be proceed to the spam check process, @target has to be
# In order to be proceed to the spam check process, @spammable has to be
# a dirty instance, which means it should be already assigned with the new
# attribute values.
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def spam_check(spammable, user)
Spam::SpamCheckService.new(
target: spammable,
spammable: spammable,
request: @request
).execute(
api: @api,
......
......@@ -4,23 +4,25 @@ module Spam
class HamService
include AkismetMethods
attr_accessor :target, :options
attr_accessor :spam_log, :options
def initialize(target)
@target = target
@user = target.user
def initialize(spam_log)
@spam_log = spam_log
@user = spam_log.user
@options = {
ip_address: target.source_ip,
user_agent: target.user_agent
ip_address: spam_log.source_ip,
user_agent: spam_log.user_agent
}
end
def execute
if akismet.submit_ham
target.update_attribute(:submitted_as_ham, true)
spam_log.update_attribute(:submitted_as_ham, true)
else
false
end
end
alias_method :spammable, :spam_log
end
end
......@@ -4,21 +4,21 @@ module Spam
class MarkAsSpamService
include ::AkismetMethods
attr_accessor :target, :options
attr_accessor :spammable, :options
def initialize(target:)
@target = target
def initialize(spammable:)
@spammable = spammable
@options = {}
@options[:ip_address] = @target.ip_address
@options[:user_agent] = @target.user_agent
@options[:ip_address] = @spammable.ip_address
@options[:user_agent] = @spammable.user_agent
end
def execute
return unless target.submittable_as_spam?
return unless spammable.submittable_as_spam?
return unless akismet.submit_spam
target.user_agent_detail.update_attribute(:submitted, true)
spammable.user_agent_detail.update_attribute(:submitted, true)
end
end
end
......@@ -4,11 +4,11 @@ module Spam
class SpamCheckService
include AkismetMethods
attr_accessor :target, :request, :options
attr_accessor :spammable, :request, :options
attr_reader :spam_log
def initialize(target:, request:)
@target = target
def initialize(spammable:, request:)
@spammable = spammable
@request = request
@options = {}
......@@ -17,8 +17,8 @@ module Spam
@options[:user_agent] = @request.env['HTTP_USER_AGENT']
@options[:referrer] = @request.env['HTTP_REFERRER']
else
@options[:ip_address] = @target.ip_address
@options[:user_agent] = @target.user_agent
@options[:ip_address] = @spammable.ip_address
@options[:user_agent] = @spammable.user_agent
end
end
......@@ -29,10 +29,10 @@ module Spam
SpamLog.verify_recaptcha!(user_id: user_id, id: spam_log_id)
else
# Otherwise, it goes to Akismet for spam check.
# If so, it assigns target spammable object as "spam" and creates a SpamLog record.
# If so, it assigns spammable object as "spam" and creates a SpamLog record.
possible_spam = check(api)
target.spam = possible_spam unless target.allow_possible_spam?
target.spam_log = spam_log
spammable.spam = possible_spam unless spammable.allow_possible_spam?
spammable.spam_log = spam_log
end
end
......@@ -48,18 +48,18 @@ module Spam
end
def check_for_spam?
target.check_for_spam?
spammable.check_for_spam?
end
def create_spam_log(api)
@spam_log = SpamLog.create!(
{
user_id: target.author_id,
title: target.spam_title,
description: target.spam_description,
user_id: spammable.author_id,
title: spammable.spam_title,
description: spammable.spam_description,
source_ip: options[:ip_address],
user_agent: options[:user_agent],
noteable_type: target.class.to_s,
noteable_type: spammable.class.to_s,
via_api: api
}
)
......
......@@ -4,10 +4,10 @@ require 'spec_helper'
describe Spam::HamService do
let_it_be(:user) { create(:user) }
let!(:target) { create(:spam_log, user: user, submitted_as_ham: false) }
let!(:spam_log) { create(:spam_log, user: user, submitted_as_ham: false) }
let(:fake_akismet_service) { double(:akismet_service) }
subject { described_class.new(target) }
subject { described_class.new(spam_log) }
before do
allow(Spam::AkismetService).to receive(:new).and_return fake_akismet_service
......@@ -24,23 +24,23 @@ describe Spam::HamService do
end
it 'does not update the record' do
expect { subject.execute }.not_to change { target.submitted_as_ham }
expect { subject.execute }.not_to change { spam_log.submitted_as_ham }
end
context 'if spam log record has already been marked as spam' do
before do
target.update_attribute(:submitted_as_ham, true)
spam_log.update_attribute(:submitted_as_ham, true)
end
it 'does not update the record' do
expect { subject.execute }.not_to change { target.submitted_as_ham }
expect { subject.execute }.not_to change { spam_log.submitted_as_ham }
end
end
end
context 'Akismet ham submission is successful' do
before do
target.update_attribute(:submitted_as_ham, false)
spam_log.update_attribute(:submitted_as_ham, false)
allow(fake_akismet_service).to receive(:submit_ham).and_return true
end
......@@ -49,7 +49,7 @@ describe Spam::HamService do
end
it 'updates the record' do
expect { subject.execute }.to change { target.submitted_as_ham }.from(false).to(true)
expect { subject.execute }.to change { spam_log.submitted_as_ham }.from(false).to(true)
end
end
end
......
......@@ -4,19 +4,19 @@ require 'spec_helper'
describe Spam::MarkAsSpamService do
let(:user_agent_detail) { build(:user_agent_detail) }
let(:target) { build(:issue, user_agent_detail: user_agent_detail) }
let(:spammable) { build(:issue, user_agent_detail: user_agent_detail) }
let(:fake_akismet_service) { double(:akismet_service, submit_spam: true) }
subject { described_class.new(target: target) }
subject { described_class.new(spammable: spammable) }
describe '#execute' do
before do
allow(subject).to receive(:akismet).and_return(fake_akismet_service)
end
context 'when the target object is not submittable' do
context 'when the spammable object is not submittable' do
before do
allow(target).to receive(:submittable_as_spam?).and_return false
allow(spammable).to receive(:submittable_as_spam?).and_return false
end
it 'does not submit as spam' do
......@@ -26,7 +26,7 @@ describe Spam::MarkAsSpamService do
context 'spam is submitted successfully' do
before do
allow(target).to receive(:submittable_as_spam?).and_return true
allow(spammable).to receive(:submittable_as_spam?).and_return true
allow(fake_akismet_service).to receive(:submit_spam).and_return true
end
......@@ -34,14 +34,14 @@ describe Spam::MarkAsSpamService do
expect(subject.execute).to be_truthy
end
it "updates the target object's user agent detail as being submitted as spam" do
it "updates the spammable object's user agent detail as being submitted as spam" do
expect(user_agent_detail).to receive(:update_attribute)
subject.execute
end
context 'when Akismet does not consider it spam' do
it 'does not update the target object as spam' do
it 'does not update the spammable object as spam' do
allow(fake_akismet_service).to receive(:submit_spam).and_return false
expect(subject.execute).to be_falsey
......
......@@ -22,12 +22,12 @@ describe Spam::SpamCheckService do
end
describe '#initialize' do
subject { described_class.new(target: issue, request: request) }
subject { described_class.new(spammable: issue, request: request) }
context 'when the request is nil' do
let(:request) { nil }
it 'assembles the options with information from the target' do
it 'assembles the options with information from the spammable' do
aggregate_failures do
expect(subject.options[:ip_address]).to eq(issue.ip_address)
expect(subject.options[:user_agent]).to eq(issue.user_agent)
......@@ -39,7 +39,7 @@ describe Spam::SpamCheckService do
context 'when the request is present' do
let(:request) { double(:request, env: env) }
it 'assembles the options with information from the target' do
it 'assembles the options with information from the spammable' do
aggregate_failures do
expect(subject.options[:ip_address]).to eq(fake_ip)
expect(subject.options[:user_agent]).to eq(fake_user_agent)
......@@ -55,7 +55,7 @@ describe Spam::SpamCheckService do
let_it_be(:existing_spam_log) { create(:spam_log, user: user, recaptcha_verified: false) }
subject do
described_service = described_class.new(target: issue, request: request)
described_service = described_class.new(spammable: issue, request: request)
described_service.execute(user_id: user.id, api: nil, recaptcha_verified: recaptcha_verified, spam_log_id: existing_spam_log.id)
end
......@@ -81,7 +81,7 @@ describe Spam::SpamCheckService do
context 'when recaptcha was not verified' do
let(:recaptcha_verified) { false }
context 'when target attributes have not changed' do
context 'when spammable attributes have not changed' do
before do
issue.closed_at = Time.zone.now
......@@ -98,7 +98,7 @@ describe Spam::SpamCheckService do
end
end
context 'when target attributes have changed' do
context 'when spammable attributes have changed' do
before do
issue.description = 'SPAM!'
end
......
......@@ -2,7 +2,7 @@
shared_examples 'akismet spam' do
context 'when request is missing' do
subject { described_class.new(target: issue, request: nil) }
subject { described_class.new(spammable: issue, request: nil) }
it "doesn't check as spam" do
subject
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment