Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRuben Davila <rdavila84@gmail.com>2016-08-16 03:17:26 +0300
committerRuben Davila <rdavila84@gmail.com>2016-08-16 03:17:26 +0300
commit84efe260173874132c69af46fd26b6f19ce89745 (patch)
tree63b53b196f2b64be2e181fa30f85e15551cced67
parent5cc14b56338e915583ae618b20e45c6b0b71ff2b (diff)
parent5a4ecb9825f34011e9e021af70a3ff0a696ec3f7 (diff)
Merge branch 'master' into 8-11-stable
-rw-r--r--CHANGELOG2
-rw-r--r--Gemfile.lock2
-rw-r--r--app/assets/stylesheets/framework/buttons.scss4
-rw-r--r--app/controllers/admin/spam_logs_controller.rb10
-rw-r--r--app/controllers/concerns/spammable_actions.rb25
-rw-r--r--app/controllers/projects/blob_controller.rb14
-rw-r--r--app/controllers/projects/issues_controller.rb2
-rw-r--r--app/helpers/sorting_helper.rb8
-rw-r--r--app/models/concerns/spammable.rb52
-rw-r--r--app/models/issue.rb8
-rw-r--r--app/models/spam_log.rb4
-rw-r--r--app/models/user_agent_detail.rb9
-rw-r--r--app/services/akismet_service.rb79
-rw-r--r--app/services/create_spam_log_service.rb13
-rw-r--r--app/services/files/base_service.rb1
-rw-r--r--app/services/files/update_service.rb23
-rw-r--r--app/services/ham_service.rb26
-rw-r--r--app/services/issues/create_service.rb35
-rw-r--r--app/services/spam_check_service.rb38
-rw-r--r--app/services/spam_service.rb78
-rw-r--r--app/services/user_agent_detail_service.rb13
-rw-r--r--app/views/admin/spam_logs/_spam_log.html.haml5
-rw-r--r--app/views/projects/blob/edit.html.haml9
-rw-r--r--app/views/projects/issues/show.html.haml9
-rw-r--r--config/routes.rb7
-rw-r--r--db/migrate/20160727163552_create_user_agent_details.rb18
-rw-r--r--db/migrate/20160729173930_remove_project_id_from_spam_logs.rb29
-rw-r--r--db/migrate/20160801163709_add_submitted_as_ham_to_spam_logs.rb20
-rw-r--r--db/schema.rb12
-rw-r--r--doc/integration/akismet.md23
-rw-r--r--doc/integration/img/spam_log.pngbin0 -> 187190 bytes
-rw-r--r--doc/integration/img/submit_issue.pngbin0 -> 176450 bytes
-rw-r--r--features/project/merge_requests.feature10
-rw-r--r--lib/api/issues.rb2
-rw-r--r--lib/gitlab/akismet_helper.rb47
-rw-r--r--spec/controllers/admin/groups_controller_spec.rb5
-rw-r--r--spec/controllers/admin/spam_logs_controller_spec.rb12
-rw-r--r--spec/controllers/groups_controller_spec.rb5
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb50
-rw-r--r--spec/factories/user_agent_details.rb7
-rw-r--r--spec/features/merge_requests/diff_notes_spec.rb159
-rw-r--r--spec/features/projects/files/editing_a_file_spec.rb34
-rw-r--r--spec/lib/gitlab/akismet_helper_spec.rb35
-rw-r--r--spec/models/concerns/spammable_spec.rb33
-rw-r--r--spec/models/project_services/irker_service_spec.rb7
-rw-r--r--spec/models/user_agent_detail_spec.rb31
-rw-r--r--spec/requests/api/issues_spec.rb5
-rw-r--r--spec/services/files/update_service_spec.rb84
48 files changed, 922 insertions, 182 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 993b229f975..fc9291eefd5 100644
--- a/CHANGELOG
+++ b/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
@@ -114,6 +115,7 @@ v 8.11.0 (unreleased)
- Add auto-completition in pipeline (Katarzyna Kobierska Ula Budziszewska)
- Fix a memory leak caused by Banzai::Filter::SanitizationFilter
- Speed up todos queries by limiting the projects set we join with
+ - Ensure file editing in UI does not overwrite commited changes without warning user
v 8.10.5
- Add a data migration to fix some missing timestamps in the members table. !5670
diff --git a/Gemfile.lock b/Gemfile.lock
index 3ba6048143c..2244c20203b 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -338,7 +338,7 @@ GEM
httparty (0.13.7)
json (~> 1.8)
multi_xml (>= 0.5.2)
- httpclient (2.7.0.1)
+ httpclient (2.8.2)
i18n (0.7.0)
ice_nine (0.11.1)
influxdb (0.2.3)
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/admin/spam_logs_controller.rb b/app/controllers/admin/spam_logs_controller.rb
index 3a2f0185315..2abfa22712d 100644
--- a/app/controllers/admin/spam_logs_controller.rb
+++ b/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
diff --git a/app/controllers/concerns/spammable_actions.rb b/app/controllers/concerns/spammable_actions.rb
new file mode 100644
index 00000000000..29e243c66a3
--- /dev/null
+++ b/app/controllers/concerns/spammable_actions.rb
@@ -0,0 +1,25 @@
+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
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 19d051720e9..cdf9a04bacf 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -17,6 +17,7 @@ class Projects::BlobController < Projects::ApplicationController
before_action :require_branch_head, only: [:edit, :update]
before_action :editor_variables, except: [:show, :preview, :diff]
before_action :validate_diff_params, only: :diff
+ before_action :set_last_commit_sha, only: [:edit, :update]
def new
commit unless @repository.empty?
@@ -33,7 +34,6 @@ class Projects::BlobController < Projects::ApplicationController
end
def edit
- @last_commit = Gitlab::Git::Commit.last_for_path(@repository, @ref, @path).sha
blob.load_all_data!(@repository)
end
@@ -55,6 +55,10 @@ class Projects::BlobController < Projects::ApplicationController
create_commit(Files::UpdateService, success_path: after_edit_path,
failure_view: :edit,
failure_path: namespace_project_blob_path(@project.namespace, @project, @id))
+
+ rescue Files::UpdateService::FileChangedError
+ @conflict = true
+ render :edit
end
def preview
@@ -152,7 +156,8 @@ class Projects::BlobController < Projects::ApplicationController
file_path: @file_path,
commit_message: params[:commit_message],
file_content: params[:content],
- file_content_encoding: params[:encoding]
+ file_content_encoding: params[:encoding],
+ last_commit_sha: params[:last_commit_sha]
}
end
@@ -161,4 +166,9 @@ class Projects::BlobController < Projects::ApplicationController
render nothing: true
end
end
+
+ def set_last_commit_sha
+ @last_commit_sha = Gitlab::Git::Commit.
+ last_for_path(@repository, @ref, @path).sha
+ 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/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb
index e1c0b497550..8b138a8e69f 100644
--- a/app/helpers/sorting_helper.rb
+++ b/app/helpers/sorting_helper.rb
@@ -20,13 +20,19 @@ module SortingHelper
end
def projects_sort_options_hash
- {
+ options = {
sort_value_name => sort_title_name,
sort_value_recently_updated => sort_title_recently_updated,
sort_value_oldest_updated => sort_title_oldest_updated,
sort_value_recently_created => sort_title_recently_created,
sort_value_oldest_created => sort_title_oldest_created,
}
+
+ if current_controller?('admin/projects')
+ options.merge!(sort_value_largest_repo => sort_title_largest_repo)
+ end
+
+ options
end
def sort_title_priority
diff --git a/app/models/concerns/spammable.rb b/app/models/concerns/spammable.rb
index 3b8e6df2da9..ce54fe5d3bf 100644
--- a/app/models/concerns/spammable.rb
+++ b/app/models/concerns/spammable.rb
@@ -1,9 +1,32 @@
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
diff --git a/app/models/issue.rb b/app/models/issue.rb
index d62ffb21467..788611305fe 100644
--- a/app/models/issue.rb
+++ b/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
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/models/user_agent_detail.rb b/app/models/user_agent_detail.rb
new file mode 100644
index 00000000000..0949c6ef083
--- /dev/null
+++ b/app/models/user_agent_detail.rb
@@ -0,0 +1,9 @@
+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
diff --git a/app/services/akismet_service.rb b/app/services/akismet_service.rb
new file mode 100644
index 00000000000..5c60addbe7c
--- /dev/null
+++ b/app/services/akismet_service.rb
@@ -0,0 +1,79 @@
+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
diff --git a/app/services/create_spam_log_service.rb b/app/services/create_spam_log_service.rb
deleted file mode 100644
index 59a66fde47a..00000000000
--- a/app/services/create_spam_log_service.rb
+++ /dev/null
@@ -1,13 +0,0 @@
-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
diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb
index c4a206f785e..ea94818713b 100644
--- a/app/services/files/base_service.rb
+++ b/app/services/files/base_service.rb
@@ -15,6 +15,7 @@ module Files
else
params[:file_content]
end
+ @last_commit_sha = params[:last_commit_sha]
# Validate parameters
validate
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index 8d2b5083179..4fc3b640799 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -2,11 +2,34 @@ require_relative "base_service"
module Files
class UpdateService < Files::BaseService
+ class FileChangedError < StandardError; end
+
def commit
repository.update_file(current_user, @file_path, @file_content,
branch: @target_branch,
previous_path: @previous_path,
message: @commit_message)
end
+
+ private
+
+ def validate
+ super
+
+ if file_has_changed?
+ raise FileChangedError.new("You are attempting to update a file that has changed since you started editing it.")
+ end
+ end
+
+ def file_has_changed?
+ return false unless @last_commit_sha && last_commit
+
+ @last_commit_sha != last_commit.sha
+ end
+
+ def last_commit
+ @last_commit ||= Gitlab::Git::Commit.
+ last_for_path(@source_project.repository, @source_branch, @file_path)
+ end
end
end
diff --git a/app/services/ham_service.rb b/app/services/ham_service.rb
new file mode 100644
index 00000000000..b0e1799b489
--- /dev/null
+++ b/app/services/ham_service.rb
@@ -0,0 +1,26 @@
+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
diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb
index 5e2de2ccf64..65550ab8ec6 100644
--- a/app/services/issues/create_service.rb
+++ b/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
diff --git a/app/services/spam_check_service.rb b/app/services/spam_check_service.rb
deleted file mode 100644
index 7c3e692bde9..00000000000
--- a/app/services/spam_check_service.rb
+++ /dev/null
@@ -1,38 +0,0 @@
-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
diff --git a/app/services/spam_service.rb b/app/services/spam_service.rb
new file mode 100644
index 00000000000..48903291799
--- /dev/null
+++ b/app/services/spam_service.rb
@@ -0,0 +1,78 @@
+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
diff --git a/app/services/user_agent_detail_service.rb b/app/services/user_agent_detail_service.rb
new file mode 100644
index 00000000000..a1ee3df5fe1
--- /dev/null
+++ b/app/services/user_agent_detail_service.rb
@@ -0,0 +1,13 @@
+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
diff --git a/app/views/admin/spam_logs/_spam_log.html.haml b/app/views/admin/spam_logs/_spam_log.html.haml
index 8aea67f4497..4ce4eab8753 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/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml
index b1c9895f43e..7b0621f9401 100644
--- a/app/views/projects/blob/edit.html.haml
+++ b/app/views/projects/blob/edit.html.haml
@@ -1,5 +1,11 @@
- page_title "Edit", @blob.path, @ref
+- if @conflict
+ .alert.alert-danger
+ Someone edited the file the same time you did. Please check out
+ = link_to "the file", namespace_project_blob_path(@project.namespace, @project, tree_join(@target_branch, @file_path)), target: "_blank"
+ and make sure your changes will not unintentionally remove theirs.
+
.file-editor
%ul.nav-links.no-bottom.js-edit-mode
%li.active
@@ -13,8 +19,7 @@
= form_tag(namespace_project_update_blob_path(@project.namespace, @project, @id), method: :put, class: 'form-horizontal js-quick-submit js-requires-input js-edit-blob-form') do
= render 'projects/blob/editor', ref: @ref, path: @path, blob_data: @blob.data
= render 'shared/new_commit_form', placeholder: "Update #{@blob.name}"
-
- = hidden_field_tag 'last_commit', @last_commit
+ = hidden_field_tag 'last_commit_sha', @last_commit_sha
= hidden_field_tag 'content', '', id: "file-content"
= hidden_field_tag 'from_merge_request_id', params[:from_merge_request_id]
= render 'projects/commit_button', ref: @ref, cancel_path: namespace_project_blob_path(@project.namespace, @project, @id)
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index e5cce16a171..9f1a046ea74 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/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
diff --git a/config/routes.rb b/config/routes.rb
index cf6773917cf..1d2db91344f 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
@@ -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
diff --git a/db/migrate/20160727163552_create_user_agent_details.rb b/db/migrate/20160727163552_create_user_agent_details.rb
new file mode 100644
index 00000000000..ed4ccfedc0a
--- /dev/null
+++ b/db/migrate/20160727163552_create_user_agent_details.rb
@@ -0,0 +1,18 @@
+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
diff --git a/db/migrate/20160729173930_remove_project_id_from_spam_logs.rb b/db/migrate/20160729173930_remove_project_id_from_spam_logs.rb
new file mode 100644
index 00000000000..e28ab31d629
--- /dev/null
+++ b/db/migrate/20160729173930_remove_project_id_from_spam_logs.rb
@@ -0,0 +1,29 @@
+# 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
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 f008a6bd7a7..52ba60ace11 100644
--- a/db/schema.rb
+++ b/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
diff --git a/doc/integration/akismet.md b/doc/integration/akismet.md
index c222d21612f..06c787cfcc7 100644
--- a/doc/integration/akismet.md
+++ b/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.
diff --git a/doc/integration/img/spam_log.png b/doc/integration/img/spam_log.png
new file mode 100644
index 00000000000..8d574448690
--- /dev/null
+++ b/doc/integration/img/spam_log.png
Binary files differ
diff --git a/doc/integration/img/submit_issue.png b/doc/integration/img/submit_issue.png
new file mode 100644
index 00000000000..9fd9c20f6b3
--- /dev/null
+++ b/doc/integration/img/submit_issue.png
Binary files differ
diff --git a/features/project/merge_requests.feature b/features/project/merge_requests.feature
index 6bac6011467..1b8e4262e40 100644
--- a/features/project/merge_requests.feature
+++ b/features/project/merge_requests.feature
@@ -107,16 +107,6 @@ Feature: Project Merge Requests
Then The list should be sorted by "Least popular"
@javascript
- Scenario: I comment on a merge request diff
- Given project "Shop" have "Bug NS-05" open merge request with diffs inside
- And I visit merge request page "Bug NS-05"
- And I click on the Changes tab
- And I leave a comment like "Line is wrong" on diff
- And I switch to the merge request's comments tab
- Then I should see a discussion has started on diff
- And I should see a badge of "1" next to the discussion link
-
- @javascript
Scenario: I see a new comment on merge request diff from another user in the discussion tab
Given project "Shop" have "Bug NS-05" open merge request with diffs inside
And I visit merge request page "Bug NS-05"
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index c4d3134da6c..077258faee1 100644
--- a/lib/api/issues.rb
+++ b/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
diff --git a/lib/gitlab/akismet_helper.rb b/lib/gitlab/akismet_helper.rb
deleted file mode 100644
index 207736b59db..00000000000
--- a/lib/gitlab/akismet_helper.rb
+++ /dev/null
@@ -1,47 +0,0 @@
-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
diff --git a/spec/controllers/admin/groups_controller_spec.rb b/spec/controllers/admin/groups_controller_spec.rb
index 0239aea47fb..602de72d23f 100644
--- a/spec/controllers/admin/groups_controller_spec.rb
+++ b/spec/controllers/admin/groups_controller_spec.rb
@@ -7,12 +7,13 @@ describe Admin::GroupsController do
before do
sign_in(admin)
- Sidekiq::Testing.fake!
end
describe 'DELETE #destroy' do
it 'schedules a group destroy' do
- expect { delete :destroy, id: project.group.path }.to change(GroupDestroyWorker.jobs, :size).by(1)
+ Sidekiq::Testing.fake! do
+ expect { delete :destroy, id: project.group.path }.to change(GroupDestroyWorker.jobs, :size).by(1)
+ end
end
it 'redirects to the admin group path' do
diff --git a/spec/controllers/admin/spam_logs_controller_spec.rb b/spec/controllers/admin/spam_logs_controller_spec.rb
index 520a4f6f9c5..585ca31389d 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(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
diff --git a/spec/controllers/groups_controller_spec.rb b/spec/controllers/groups_controller_spec.rb
index 4ae6364207b..a763e2c5ba8 100644
--- a/spec/controllers/groups_controller_spec.rb
+++ b/spec/controllers/groups_controller_spec.rb
@@ -89,12 +89,13 @@ describe GroupsController do
context 'as the group owner' do
before do
- Sidekiq::Testing.fake!
sign_in(user)
end
it 'schedules a group destroy' do
- expect { delete :destroy, id: group.path }.to change(GroupDestroyWorker.jobs, :size).by(1)
+ Sidekiq::Testing.fake! do
+ expect { delete :destroy, id: group.path }.to change(GroupDestroyWorker.jobs, :size).by(1)
+ end
end
it 'redirects to the root path' do
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index b6a0276846c..0836b71056c 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/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
diff --git a/spec/factories/user_agent_details.rb b/spec/factories/user_agent_details.rb
new file mode 100644
index 00000000000..9763cc0cf15
--- /dev/null
+++ b/spec/factories/user_agent_details.rb
@@ -0,0 +1,7 @@
+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
diff --git a/spec/features/merge_requests/diff_notes_spec.rb b/spec/features/merge_requests/diff_notes_spec.rb
new file mode 100644
index 00000000000..12e89742b79
--- /dev/null
+++ b/spec/features/merge_requests/diff_notes_spec.rb
@@ -0,0 +1,159 @@
+require 'spec_helper'
+
+feature 'Diff notes', js: true, feature: true do
+ include WaitForAjax
+
+ before do
+ login_as :admin
+ @merge_request = create(:merge_request)
+ @project = @merge_request.source_project
+ end
+
+ context 'merge request diffs' do
+ let(:comment_button_class) { '.add-diff-note' }
+ let(:notes_holder_input_class) { 'js-temp-notes-holder' }
+ let(:notes_holder_input_xpath) { './following-sibling::*[contains(concat(" ", @class, " "), " notes_holder ")]' }
+ let(:test_note_comment) { 'this is a test note!' }
+
+ context 'when hovering over the parallel view diff file' do
+ before(:each) do
+ visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
+ click_link 'Side-by-side'
+ end
+
+ context 'with an old line on the left and no line on the right' do
+ it 'should allow commenting on the left side' do
+ should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'left')
+ end
+
+ it 'should not allow commenting on the right side' do
+ should_not_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_23_22"]').find(:xpath, '..'), 'right')
+ end
+ end
+
+ context 'with no line on the left and a new line on the right' do
+ it 'should not allow commenting on the left side' do
+ should_not_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'left')
+ end
+
+ it 'should allow commenting on the right side' do
+ should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_15_15"]').find(:xpath, '..'), 'right')
+ end
+ end
+
+ context 'with an old line on the left and a new line on the right' do
+ it 'should allow commenting on the left side' do
+ should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'left')
+ end
+
+ it 'should allow commenting on the right side' do
+ should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_9_9"]').find(:xpath, '..'), 'right')
+ end
+ end
+
+ context 'with an unchanged line on the left and an unchanged line on the right' do
+ it 'should allow commenting on the left side' do
+ should_allow_commenting(first('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]').find(:xpath, '..'), 'left')
+ end
+
+ it 'should allow commenting on the right side' do
+ should_allow_commenting(first('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]').find(:xpath, '..'), 'right')
+ end
+ end
+
+ context 'with a match line' do
+ it 'should not allow commenting on the left side' do
+ should_not_allow_commenting(first('.match').find(:xpath, '..'), 'left')
+ end
+
+ it 'should not allow commenting on the right side' do
+ should_not_allow_commenting(first('.match').find(:xpath, '..'), 'right')
+ end
+ end
+ end
+
+ context 'when hovering over the inline view diff file' do
+ before do
+ visit diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request)
+ click_link 'Inline'
+ end
+
+ context 'with a new line' do
+ it 'should allow commenting' do
+ should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_10_9"]'))
+ end
+ end
+
+ context 'with an old line' do
+ it 'should allow commenting' do
+ should_allow_commenting(find('[id="6eb14e00385d2fb284765eb1cd8d420d33d63fc9_22_22"]'))
+ end
+ end
+
+ context 'with an unchanged line' do
+ it 'should allow commenting' do
+ should_allow_commenting(find('[id="2f6fcd96b88b36ce98c38da085c795a27d92a3dd_7_7"]'))
+ end
+ end
+
+ context 'with a match line' do
+ it 'should not allow commenting' do
+ should_not_allow_commenting(first('.match'))
+ end
+ end
+ end
+
+ def should_allow_commenting(line_holder, diff_side = nil)
+ line = get_line_components(line_holder, diff_side)
+ line[:content].hover
+ expect(line[:num]).to have_css comment_button_class
+
+ comment_on_line(line_holder, line)
+ wait_for_ajax
+
+ assert_comment_persistence(line_holder)
+ end
+
+ def should_not_allow_commenting(line_holder, diff_side = nil)
+ line = get_line_components(line_holder, diff_side)
+ line[:content].hover
+ expect(line[:num]).not_to have_css comment_button_class
+ end
+
+ def get_line_components(line_holder, diff_side = nil)
+ if diff_side.nil?
+ get_inline_line_components(line_holder)
+ else
+ get_parallel_line_components(line_holder, diff_side)
+ end
+ end
+
+ def get_inline_line_components(line_holder)
+ { content: line_holder.first('.line_content'), num: line_holder.first('.diff-line-num') }
+ end
+
+ def get_parallel_line_components(line_holder, diff_side = nil)
+ side_index = diff_side == 'left' ? 0 : 1
+ { content: line_holder.all('.line_content')[side_index], num: line_holder.all('.diff-line-num')[side_index] }
+ end
+
+ def comment_on_line(line_holder, line)
+ line[:num].find(comment_button_class).trigger 'click'
+ expect(line_holder).to have_xpath notes_holder_input_xpath
+
+ notes_holder_input = line_holder.find(:xpath, notes_holder_input_xpath)
+ expect(notes_holder_input[:class]).to include(notes_holder_input_class)
+
+ notes_holder_input.fill_in 'note[note]', with: test_note_comment
+ click_button 'Comment'
+ end
+
+ def assert_comment_persistence(line_holder)
+ expect(line_holder).to have_xpath notes_holder_input_xpath
+
+ notes_holder_saved = line_holder.find(:xpath, notes_holder_input_xpath)
+ expect(notes_holder_saved[:class]).not_to include(notes_holder_input_class)
+ expect(notes_holder_saved).to have_content test_note_comment
+ end
+ end
+end
diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb
new file mode 100644
index 00000000000..fe047e00409
--- /dev/null
+++ b/spec/features/projects/files/editing_a_file_spec.rb
@@ -0,0 +1,34 @@
+require 'spec_helper'
+
+feature 'User wants to edit a file', feature: true do
+ include WaitForAjax
+
+ let(:project) { create(:project) }
+ let(:user) { create(:user) }
+ let(:commit_params) do
+ {
+ source_branch: project.default_branch,
+ target_branch: project.default_branch,
+ commit_message: "Committing First Update",
+ file_path: ".gitignore",
+ file_content: "First Update",
+ last_commit_sha: Gitlab::Git::Commit.last_for_path(project.repository, project.default_branch,
+ ".gitignore").sha
+ }
+ end
+
+ background do
+ project.team << [user, :master]
+ login_as user
+ visit namespace_project_edit_blob_path(project.namespace, project,
+ File.join(project.default_branch, '.gitignore'))
+ end
+
+ scenario 'file has been updated since the user opened the edit page' do
+ Files::UpdateService.new(project, user, commit_params).execute
+
+ click_button 'Commit Changes'
+
+ expect(page).to have_content 'Someone edited the file the same time you did.'
+ end
+end
diff --git a/spec/lib/gitlab/akismet_helper_spec.rb b/spec/lib/gitlab/akismet_helper_spec.rb
deleted file mode 100644
index b08396da4d2..00000000000
--- a/spec/lib/gitlab/akismet_helper_spec.rb
+++ /dev/null
@@ -1,35 +0,0 @@
-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
diff --git a/spec/models/concerns/spammable_spec.rb b/spec/models/concerns/spammable_spec.rb
new file mode 100644
index 00000000000..32935bc0b09
--- /dev/null
+++ b/spec/models/concerns/spammable_spec.rb
@@ -0,0 +1,33 @@
+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
diff --git a/spec/models/project_services/irker_service_spec.rb b/spec/models/project_services/irker_service_spec.rb
index b528baaf15c..ea718232255 100644
--- a/spec/models/project_services/irker_service_spec.rb
+++ b/spec/models/project_services/irker_service_spec.rb
@@ -52,19 +52,20 @@ describe IrkerService, models: true do
let(:colorize_messages) { '1' }
before do
+ @irker_server = TCPServer.new 'localhost', 0
+
allow(irker).to receive_messages(
active: true,
project: project,
project_id: project.id,
service_hook: true,
- server_host: 'localhost',
- server_port: 6659,
+ server_host: @irker_server.addr[2],
+ server_port: @irker_server.addr[1],
default_irc_uri: 'irc://chat.freenode.net/',
recipients: recipients,
colorize_messages: colorize_messages)
irker.valid?
- @irker_server = TCPServer.new 'localhost', 6659
end
after do
diff --git a/spec/models/user_agent_detail_spec.rb b/spec/models/user_agent_detail_spec.rb
new file mode 100644
index 00000000000..a8c25766e73
--- /dev/null
+++ b/spec/models/user_agent_detail_spec.rb
@@ -0,0 +1,31 @@
+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
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 3cd4e981fb2..a40e1a93b71 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/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
diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb
new file mode 100644
index 00000000000..d019e50649f
--- /dev/null
+++ b/spec/services/files/update_service_spec.rb
@@ -0,0 +1,84 @@
+require "spec_helper"
+
+describe Files::UpdateService do
+ subject { described_class.new(project, user, commit_params) }
+
+ let(:project) { create(:project) }
+ let(:user) { create(:user) }
+ let(:file_path) { 'files/ruby/popen.rb' }
+ let(:new_contents) { "New Content" }
+ let(:commit_params) do
+ {
+ file_path: file_path,
+ commit_message: "Update File",
+ file_content: new_contents,
+ file_content_encoding: "text",
+ last_commit_sha: last_commit_sha,
+ source_project: project,
+ source_branch: project.default_branch,
+ target_branch: project.default_branch,
+ }
+ end
+
+ before do
+ project.team << [user, :master]
+ end
+
+ describe "#execute" do
+ context "when the file's last commit sha does not match the supplied last_commit_sha" do
+ let(:last_commit_sha) { "foo" }
+
+ it "returns a hash with the correct error message and a :error status " do
+ expect { subject.execute }.
+ to raise_error(Files::UpdateService::FileChangedError,
+ "You are attempting to update a file that has changed since you started editing it.")
+ end
+ end
+
+ context "when the file's last commit sha does match the supplied last_commit_sha" do
+ let(:last_commit_sha) { Gitlab::Git::Commit.last_for_path(project.repository, project.default_branch, file_path).sha }
+
+ it "returns a hash with the :success status " do
+ results = subject.execute
+
+ expect(results).to match({ status: :success })
+ end
+
+ it "updates the file with the new contents" do
+ subject.execute
+
+ results = project.repository.blob_at_branch(project.default_branch, file_path)
+
+ expect(results.data).to eq(new_contents)
+ end
+ end
+
+ context "when the last_commit_sha is not supplied" do
+ let(:commit_params) do
+ {
+ file_path: file_path,
+ commit_message: "Update File",
+ file_content: new_contents,
+ file_content_encoding: "text",
+ source_project: project,
+ source_branch: project.default_branch,
+ target_branch: project.default_branch,
+ }
+ end
+
+ it "returns a hash with the :success status " do
+ results = subject.execute
+
+ expect(results).to match({ status: :success })
+ end
+
+ it "updates the file with the new contents" do
+ subject.execute
+
+ results = project.repository.blob_at_branch(project.default_branch, file_path)
+
+ expect(results.data).to eq(new_contents)
+ end
+ end
+ end
+end