diff options
author | Patricio Cano <suprnova32@gmail.com> | 2016-08-02 16:21:57 -0500 |
---|---|---|
committer | Patricio Cano <suprnova32@gmail.com> | 2016-08-15 13:18:15 -0500 |
commit | 96399a81cbb2e8a0f666241eeaff7cc784c26983 (patch) | |
tree | 7123e352717a846300c031cb884028fbd0a7f1d3 | |
parent | abf2dcd25c4a176801314872733ede91297d1ab0 (diff) | |
download | gitlab-ce-96399a81cbb2e8a0f666241eeaff7cc784c26983.tar.gz |
Allow `Issue` to be submitted as spam
- Added controller actions as reusable concerns
- Added controller tests
-rw-r--r-- | app/assets/stylesheets/framework/buttons.scss | 4 | ||||
-rw-r--r-- | app/controllers/concerns/spammable_actions.rb | 32 | ||||
-rw-r--r-- | app/controllers/projects/issues_controller.rb | 2 | ||||
-rw-r--r-- | app/models/concerns/spammable.rb | 18 | ||||
-rw-r--r-- | app/services/system_note_service.rb | 17 | ||||
-rw-r--r-- | app/views/admin/spam_logs/_spam_log.html.haml | 2 | ||||
-rw-r--r-- | app/views/projects/issues/show.html.haml | 11 | ||||
-rw-r--r-- | config/routes.rb | 1 | ||||
-rw-r--r-- | db/migrate/20160727163552_create_user_agent_details.rb | 1 | ||||
-rw-r--r-- | db/schema.rb | 1 | ||||
-rw-r--r-- | spec/controllers/admin/spam_logs_controller_spec.rb | 12 | ||||
-rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 29 | ||||
-rw-r--r-- | spec/models/concerns/spammable_spec.rb | 9 |
13 files changed, 129 insertions, 10 deletions
diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index 473530cf094..f1fe1697d30 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/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 { diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb new file mode 100644 index 00000000000..85be25d84cc --- /dev/null +++ b/app/controllers/concerns/spammable_actions.rb @@ -0,0 +1,32 @@ +module SpammableActions + extend ActiveSupport::Concern + + included do + before_action :authorize_submit_spammable!, only: :mark_as_spam + end + + def mark_as_spam + if spammable.submit_spam + spammable.user_agent_detail.update_attribute(:submitted, true) + + if spammable.is_a?(Issuable) + SystemNoteService.submit_spam(spammable, spammable.project, current_user) + end + + redirect_to spammable, notice: 'Issue was submitted to Akismet successfully.' + else + flash[:error] = 'Error with Akismet. Please check the logs for more info.' + redirect_to spammable + end + end + + private + + def spammable + raise NotImplementedError + end + + def authorize_submit_spammable! + access_denied! unless current_user.admin? + end +end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 660e0eba06f..e9fb11e8f94 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/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) diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb index 5c75275b6e2..f272e7c5a55 100644 --- a/app/models/concerns/spammable.rb +++ b/app/models/concerns/spammable.rb @@ -22,7 +22,7 @@ module Spammable def can_be_submitted? if user_agent_detail - user_agent_detail.submittable? + user_agent_detail.submittable? && akismet_enabled? else false end @@ -41,6 +41,14 @@ module Spammable @spam end + def submitted? + if user_agent_detail + user_agent_detail.submitted + else + false + end + end + 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 @@ -53,17 +61,21 @@ module Spammable end end + def to_ability_name + self.class.to_s.underscore + end + # Override this method if an additional check is needed before calling Akismet def check_for_spam? akismet_enabled? end def spam_title - raise 'Implement in included model!' + raise NotImplementedError end def spam_description - raise 'Implement in included model!' + raise NotImplementedError end private diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index e13dc9265b8..56d3329f5bd 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -395,6 +395,23 @@ module SystemNoteService create_note(noteable: noteable, project: project, author: author, note: body) end + # Called when the status of a Issuable is submitted as spam + # + # noteable - Noteable object + # project - Project owning noteable + # author - User performing the change + # + # Example Note text: + # + # "Issue submitted as spam." + # + # Returns the created Note object + def submit_spam(noteable, project, author) + body = "Submitted #{noteable.class.to_s.downcase} as spam" + + create_note(noteable: noteable, project: project, author: author, note: body) + end + private def notes_for_mentioner(mentioner, noteable, notes) diff --git a/app/views/admin/spam_logs/_spam_log.html.haml b/app/views/admin/spam_logs/_spam_log.html.haml index f2b6106fb4a..4ce4eab8753 100644 --- a/app/views/admin/spam_logs/_spam_log.html.haml +++ b/app/views/admin/spam_logs/_spam_log.html.haml @@ -26,7 +26,7 @@ %td - if spam_log.submitted_as_ham? .btn.btn-xs.disabled - Submitted as Ham + 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? diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index e5cce16a171..30e6a35db53 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -37,14 +37,21 @@ = 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.can_be_submitted? && current_user.admin? + - unless @issue.submitted? + %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 + = link_to 'Edit', edit_namespace_project_issue_path(@project.namespace, @project, @issue), class: 'hidden-xs hidden-sm btn btn-grouped issuable-edit' + - if @issue.can_be_submitted? && current_user.admin? + - unless @issue.submitted? + = 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' .issue-details.issuable-details diff --git a/config/routes.rb b/config/routes.rb index 8214bc26d59..2cf2d111920 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -817,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 diff --git a/db/migrate/20160727163552_create_user_agent_details.rb b/db/migrate/20160727163552_create_user_agent_details.rb index 05c21a476fa..f9a02f310da 100644 --- a/db/migrate/20160727163552_create_user_agent_details.rb +++ b/db/migrate/20160727163552_create_user_agent_details.rb @@ -5,6 +5,7 @@ class CreateUserAgentDetails < ActiveRecord::Migration t.string :ip_address, null: false t.integer :subject_id, null: false t.string :subject_type, null: false + t.boolean :submitted, default: false t.timestamps null: false end diff --git a/db/schema.rb b/db/schema.rb index 355eed13b68..5ac08099e90 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1004,6 +1004,7 @@ ActiveRecord::Schema.define(version: 20160810142633) do t.string "ip_address", null: false t.integer "subject_id", null: false t.string "subject_type", null: false + t.boolean "submitted", default: false t.datetime "created_at", null: false t.datetime "updated_at", null: false end diff --git a/spec/controllers/admin/spam_logs_controller_spec.rb b/spec/controllers/admin/spam_logs_controller_spec.rb index 520a4f6f9c5..f94afd1139d 100644 --- a/spec/controllers/admin/spam_logs_controller_spec.rb +++ b/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(Gitlab::AkismetHelper).to receive(: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 diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index efca838613f..8fcde9a38bc 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -322,6 +322,35 @@ describe Projects::IssuesController do end end + describe 'POST #mark_as_spam' do + context 'properly submits to Akismet' do + before do + allow_any_instance_of(Spammable).to receive_messages(can_be_submitted?: true, submit_spam: 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 'creates a system note' do + expect{ post_spam }.to change(Note, :count) + end + + it 'updates issue' do + post_spam + expect(issue.submitted?).to be_truthy + end + end + end + describe "DELETE #destroy" do context "when the user is a developer" do before { sign_in(user) } diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb index 7492c42f71e..4e52d05918f 100644 --- a/spec/models/concerns/spammable_spec.rb +++ b/spec/models/concerns/spammable_spec.rb @@ -14,6 +14,10 @@ describe Issue, 'Spammable' do end describe 'InstanceMethods' do + before do + allow_any_instance_of(Gitlab::AkismetHelper).to receive(:akismet_enabled?).and_return(true) + end + it 'should return the correct creator' do expect(issue.send(:owner).id).to eq(issue.author_id) end @@ -24,14 +28,11 @@ describe Issue, 'Spammable' do end it 'should be submittable' do - create(:user_agent_detail, subject_id: issue.id, subject_type: issue.class.to_s) + create(:user_agent_detail, subject: issue) expect(issue.can_be_submitted?).to be_truthy end describe '#check_for_spam?' do - before do - allow_any_instance_of(Gitlab::AkismetHelper).to receive(:akismet_enabled?).and_return(true) - end it 'returns true for public project' do issue.project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) expect(issue.check_for_spam?).to eq(true) |