diff options
author | Sean McGivern <sean@mcgivern.me.uk> | 2017-03-22 11:53:24 +0000 |
---|---|---|
committer | Sean McGivern <sean@mcgivern.me.uk> | 2017-03-22 11:53:24 +0000 |
commit | 85ffd85a2bde09691784adb32395ad2c5a3ff909 (patch) | |
tree | a781997b91e5aa1dd770b63712ed21ed8ddfa042 /spec | |
parent | 78840333d7e1432fc6235b59e90040d0f9e43b41 (diff) | |
parent | d730b69eb26ab5917b773a242c21f5967661d964 (diff) | |
download | gitlab-ce-85ffd85a2bde09691784adb32395ad2c5a3ff909.tar.gz |
Merge branch '29483-spam-check-only-title-and-description' into 'master'
Spam check only when Snippet / Issue "title" or "description" are changed
Closes #29483 and #29645
See merge request !10104
Diffstat (limited to 'spec')
-rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 23 | ||||
-rw-r--r-- | spec/models/issue_spec.rb | 37 | ||||
-rw-r--r-- | spec/models/snippet_spec.rb | 43 | ||||
-rw-r--r-- | spec/services/spam_service_spec.rb | 71 |
4 files changed, 148 insertions, 26 deletions
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 57a921e3676..c467ab9fb8a 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -241,10 +241,27 @@ describe Projects::IssuesController do expect(spam_logs.first.recaptcha_verified).to be_falsey end - it 'renders verify template' do - update_spam_issue + context 'as HTML' do + it 'renders verify template' do + update_spam_issue + + expect(response).to render_template(:verify) + end + end + + context 'as JSON' do + before do + update_issue({ title: 'Spam Title', description: 'Spam lives here' }, format: :json) + end + + it 'renders json errors' do + expect(json_response) + .to eql("errors" => ["Your issue has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed."]) + end - expect(response).to render_template(:verify) + it 'returns 422 status' do + expect(response).to have_http_status(422) + end end end diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb index 73977d031f9..b8584301baa 100644 --- a/spec/models/issue_spec.rb +++ b/spec/models/issue_spec.rb @@ -670,4 +670,41 @@ describe Issue, models: true do expect(attrs_hash).to include('time_estimate') end end + + describe '#check_for_spam' do + let(:project) { create :project, visibility_level: visibility_level } + let(:issue) { create :issue, project: project } + + subject do + issue.assign_attributes(description: description) + issue.check_for_spam? + end + + context 'when project is public and spammable attributes changed' do + let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + let(:description) { 'woo' } + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when project is private' do + let(:visibility_level) { Gitlab::VisibilityLevel::PRIVATE } + let(:description) { issue.description } + + it 'returns false' do + is_expected.to be_falsey + end + end + + context 'when spammable attributes have not changed' do + let(:visibility_level) { Gitlab::VisibilityLevel::PUBLIC } + let(:description) { issue.description } + + it 'returns false' do + is_expected.to be_falsey + end + end + end end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index 219ab1989ea..8095d01b69e 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -198,4 +198,47 @@ describe Snippet, models: true do expect(snippet.participants).to include(note1.author, note2.author) end end + + describe '#check_for_spam' do + let(:snippet) { create :snippet, visibility_level: visibility_level } + + subject do + snippet.assign_attributes(title: title) + snippet.check_for_spam? + end + + context 'when public and spammable attributes changed' do + let(:visibility_level) { Snippet::PUBLIC } + let(:title) { 'woo' } + + it 'returns true' do + is_expected.to be_truthy + end + end + + context 'when private' do + let(:visibility_level) { Snippet::PRIVATE } + let(:title) { snippet.title } + + it 'returns false' do + is_expected.to be_falsey + end + + it 'returns true when switching to public' do + snippet.save! + snippet.visibility_level = Snippet::PUBLIC + + expect(snippet.check_for_spam?).to be_truthy + end + end + + context 'when spammable attributes have not changed' do + let(:visibility_level) { Snippet::PUBLIC } + let(:title) { snippet.title } + + it 'returns false' do + is_expected.to be_falsey + end + end + end end diff --git a/spec/services/spam_service_spec.rb b/spec/services/spam_service_spec.rb index 4ce3b95aa87..e09c05ccf32 100644 --- a/spec/services/spam_service_spec.rb +++ b/spec/services/spam_service_spec.rb @@ -19,42 +19,67 @@ describe SpamService, services: true do let(:issue) { create(:issue, project: project) } let(:request) { double(:request, env: {}) } - context 'when indicated as spam by akismet' do - before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: true)) } + context 'when spammable attributes have not changed' do + before do + issue.closed_at = Time.zone.now - it 'doesnt check as spam when request is missing' do - check_spam(issue, nil, false) - - expect(issue.spam).to be_falsey + allow(AkismetService).to receive(:new).and_return(double(is_spam?: true)) end - it 'checks as spam' do - check_spam(issue, request, false) - - expect(issue.spam).to be_truthy + it 'returns false' do + expect(check_spam(issue, request, false)).to be_falsey end - it 'creates a spam log' do + it 'does not create a spam log' do expect { check_spam(issue, request, false) } - .to change { SpamLog.count }.from(0).to(1) + .not_to change { SpamLog.count } end + end - it 'doesnt yield block' do - expect(check_spam(issue, request, false)) - .to eql(SpamLog.last) + context 'when spammable attributes have changed' do + before do + issue.description = 'SPAM!' end - end - context 'when not indicated as spam by akismet' do - before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: false)) } + context 'when indicated as spam by akismet' do + before do + allow(AkismetService).to receive(:new).and_return(double(is_spam?: true)) + end - it 'returns false' do - expect(check_spam(issue, request, false)).to be_falsey + it 'doesnt check as spam when request is missing' do + check_spam(issue, nil, false) + + expect(issue.spam).to be_falsey + end + + it 'checks as spam' do + check_spam(issue, request, false) + + expect(issue.spam).to be_truthy + end + + it 'creates a spam log' do + expect { check_spam(issue, request, false) } + .to change { SpamLog.count }.from(0).to(1) + end + + it 'doesnt yield block' do + expect(check_spam(issue, request, false)) + .to eql(SpamLog.last) + end end - it 'does not create a spam log' do - expect { check_spam(issue, request, false) } - .not_to change { SpamLog.count } + context 'when not indicated as spam by akismet' do + before { allow(AkismetService).to receive(:new).and_return(double(is_spam?: false)) } + + it 'returns false' do + expect(check_spam(issue, request, false)).to be_falsey + end + + it 'does not create a spam log' do + expect { check_spam(issue, request, false) } + .not_to change { SpamLog.count } + end end end end |