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:
authorGitLab Bot <gitlab-bot@gitlab.com>2019-11-15 18:06:12 +0300
committerGitLab Bot <gitlab-bot@gitlab.com>2019-11-15 18:06:12 +0300
commit6e81d7f6283fae1b22f66b9d9b133243921cbd9e (patch)
tree8cf8052ef6734ceeb49314f15ff07d2720511f0d
parent3fc9a8e6957ddf75576dc63069c4c0249514499f (diff)
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--Gemfile2
-rw-r--r--Gemfile.lock6
-rw-r--r--app/assets/javascripts/lib/utils/text_utility.js15
-rw-r--r--app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue8
-rw-r--r--app/assets/javascripts/new_branch_form.js18
-rw-r--r--app/controllers/concerns/renders_commits.rb13
-rw-r--r--app/controllers/projects/merge_requests/creations_controller.rb8
-rw-r--r--app/controllers/projects/merge_requests_controller.rb9
-rw-r--r--app/finders/todos_finder.rb8
-rw-r--r--app/models/merge_request.rb11
-rw-r--r--app/models/merge_request_diff.rb11
-rw-r--r--app/models/project.rb7
-rw-r--r--app/models/todo.rb3
-rw-r--r--app/services/merge_requests/merge_base_service.rb28
-rw-r--r--app/services/merge_requests/merge_service.rb16
-rw-r--r--app/services/merge_requests/update_service.rb4
-rw-r--r--changelogs/unreleased/30182-checkbox-with-no-text-causing-following-checkboxes-not-to-be-saved.yml5
-rw-r--r--changelogs/unreleased/32534-gitlab-rake-gitlab-cleanup-orphan_job_artifact_files-dry_run-false-is-not-removing-artifacts.yml5
-rw-r--r--changelogs/unreleased/34610-Remove-IIFEs-from-new_branch_form-js.yml5
-rw-r--r--changelogs/unreleased/36460-metrics-dashboard-fails-to-load-script-doesn-t-stop.yml5
-rw-r--r--changelogs/unreleased/chore-slugify-duplication-removal.yml5
-rw-r--r--db/migrate/20191114173508_add_resolved_attributes_to_vulnerabilities.rb15
-rw-r--r--db/migrate/20191114173602_add_foreign_key_on_resolved_by_id_to_vulnerabilities.rb19
-rw-r--r--db/post_migrate/20191114173624_set_resolved_state_on_vulnerabilities.rb31
-rw-r--r--db/schema.rb6
-rw-r--r--doc/development/documentation/styleguide.md16
-rw-r--r--doc/development/geo.md18
-rw-r--r--lib/api/merge_requests.rb3
-rw-r--r--lib/gitlab/cleanup/orphan_job_artifact_files.rb11
-rw-r--r--package.json2
-rw-r--r--spec/controllers/concerns/renders_commits_spec.rb60
-rw-r--r--spec/controllers/projects/merge_requests_controller_spec.rb13
-rw-r--r--spec/factories/merge_requests.rb1
-rw-r--r--spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb2
-rw-r--r--spec/finders/todos_finder_spec.rb22
-rw-r--r--spec/frontend/fixtures/merge_requests.rb18
-rw-r--r--spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js10
-rw-r--r--spec/javascripts/merge_request_spec.js29
-rw-r--r--spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb9
-rw-r--r--spec/models/merge_request_spec.rb65
-rw-r--r--spec/models/project_spec.rb14
-rw-r--r--spec/models/todo_spec.rb34
-rw-r--r--spec/services/merge_requests/ff_merge_service_spec.rb7
-rw-r--r--spec/services/merge_requests/merge_service_spec.rb106
-rw-r--r--spec/services/merge_requests/merge_to_ref_service_spec.rb8
-rw-r--r--spec/services/merge_requests/update_service_spec.rb4
-rw-r--r--spec/support/helpers/cycle_analytics_helpers.rb2
-rw-r--r--spec/workers/merge_worker_spec.rb1
-rw-r--r--spec/workers/process_commit_worker_spec.rb3
-rw-r--r--yarn.lock8
50 files changed, 577 insertions, 152 deletions
diff --git a/Gemfile b/Gemfile
index d3689e4a211..5a721364084 100644
--- a/Gemfile
+++ b/Gemfile
@@ -136,7 +136,7 @@ gem 'faraday_middleware-aws-signers-v4'
# Markdown and HTML processing
gem 'html-pipeline', '~> 2.8'
-gem 'deckar01-task_list', '2.2.0'
+gem 'deckar01-task_list', '2.2.1'
gem 'gitlab-markup', '~> 1.7.0'
gem 'github-markup', '~> 1.7.0', require: 'github/markup'
gem 'commonmarker', '~> 0.17'
diff --git a/Gemfile.lock b/Gemfile.lock
index 5951eb4bf71..55fc902584c 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -194,7 +194,7 @@ GEM
database_cleaner (1.7.0)
debug_inspector (0.0.3)
debugger-ruby_core_source (1.3.8)
- deckar01-task_list (2.2.0)
+ deckar01-task_list (2.2.1)
html-pipeline
declarative (0.0.10)
declarative-option (0.1.0)
@@ -604,7 +604,7 @@ GEM
netrc (0.11.0)
nio4r (2.3.1)
no_proxy_fix (0.1.2)
- nokogiri (1.10.4)
+ nokogiri (1.10.5)
mini_portile2 (~> 2.4.0)
nokogumbo (1.5.0)
nokogiri
@@ -1128,7 +1128,7 @@ DEPENDENCIES
creole (~> 0.5.0)
danger (~> 6.0)
database_cleaner (~> 1.7.0)
- deckar01-task_list (= 2.2.0)
+ deckar01-task_list (= 2.2.1)
default_value_for (~> 3.3.0)
derailed_benchmarks
device_detector
diff --git a/app/assets/javascripts/lib/utils/text_utility.js b/app/assets/javascripts/lib/utils/text_utility.js
index b839a994455..0c194d67bce 100644
--- a/app/assets/javascripts/lib/utils/text_utility.js
+++ b/app/assets/javascripts/lib/utils/text_utility.js
@@ -36,25 +36,26 @@ export const humanize = string =>
export const dasherize = str => str.replace(/[_\s]+/g, '-');
/**
- * Replaces whitespaces with hyphens, convert to lower case and remove non-allowed special characters
- * @param {String} str
+ * Replaces whitespace and non-sluggish characters with a given separator
+ * @param {String} str - The string to slugify
+ * @param {String=} separator - The separator used to separate words (defaults to "-")
* @returns {String}
*/
-export const slugify = str => {
+export const slugify = (str, separator = '-') => {
const slug = str
.trim()
.toLowerCase()
- .replace(/[^a-zA-Z0-9_.-]+/g, '-');
+ .replace(/[^a-zA-Z0-9_.-]+/g, separator);
- return slug === '-' ? '' : slug;
+ return slug === separator ? '' : slug;
};
/**
- * Replaces whitespaces with underscore and converts to lower case
+ * Replaces whitespace and non-sluggish characters with underscores
* @param {String} str
* @returns {String}
*/
-export const slugifyWithUnderscore = str => str.toLowerCase().replace(/\s+/g, '_');
+export const slugifyWithUnderscore = str => slugify(str, '_');
/**
* Truncates given text
diff --git a/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue b/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue
index 46d8f61cab6..8749019c5cd 100644
--- a/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue
+++ b/app/assets/javascripts/monitoring/components/date_time_picker/date_time_picker.vue
@@ -55,10 +55,12 @@ export default {
};
},
},
- mounted() {
- this.verifyTimeRange();
+ watch: {
+ selectedTimeWindow() {
+ this.verifyTimeRange();
+ },
},
- updated() {
+ mounted() {
this.verifyTimeRange();
},
methods: {
diff --git a/app/assets/javascripts/new_branch_form.js b/app/assets/javascripts/new_branch_form.js
index 52361e963bc..918c6e408a2 100644
--- a/app/assets/javascripts/new_branch_form.js
+++ b/app/assets/javascripts/new_branch_form.js
@@ -72,16 +72,14 @@ export default class NewBranchForm {
});
return `${restriction.prefix} ${formatted.join(restriction.conjunction)}`;
};
- const validator = (function(_this) {
- return function(errors, restriction) {
- const matched = _this.name.val().match(restriction.pattern);
- if (matched) {
- return errors.concat(formatter(matched.reduce(unique, []), restriction));
- } else {
- return errors;
- }
- };
- })(this);
+ const validator = (errors, restriction) => {
+ const matched = this.name.val().match(restriction.pattern);
+ if (matched) {
+ return errors.concat(formatter(matched.reduce(unique, []), restriction));
+ } else {
+ return errors;
+ }
+ };
const errors = this.restrictions.reduce(validator, []);
if (errors.length > 0) {
const errorMessage = $('<span/>').text(errors.join(', '));
diff --git a/app/controllers/concerns/renders_commits.rb b/app/controllers/concerns/renders_commits.rb
index ed9b898a2a3..826fae834fa 100644
--- a/app/controllers/concerns/renders_commits.rb
+++ b/app/controllers/concerns/renders_commits.rb
@@ -1,11 +1,11 @@
# frozen_string_literal: true
module RendersCommits
- def limited_commits(commits)
- if commits.size > MergeRequestDiff::COMMITS_SAFE_SIZE
+ def limited_commits(commits, commits_count)
+ if commits_count > MergeRequestDiff::COMMITS_SAFE_SIZE
[
commits.first(MergeRequestDiff::COMMITS_SAFE_SIZE),
- commits.size - MergeRequestDiff::COMMITS_SAFE_SIZE
+ commits_count - MergeRequestDiff::COMMITS_SAFE_SIZE
]
else
[commits, 0]
@@ -14,9 +14,10 @@ module RendersCommits
# This is used as a helper method in a controller.
# rubocop: disable Gitlab/ModuleWithInstanceVariables
- def set_commits_for_rendering(commits)
- @total_commit_count = commits.size
- limited, @hidden_commit_count = limited_commits(commits)
+ def set_commits_for_rendering(commits, commits_count: nil)
+ @total_commit_count = commits_count || commits.size
+ limited, @hidden_commit_count = limited_commits(commits, @total_commit_count)
+ commits.each(&:lazy_author) # preload authors
prepare_commits_for_rendering(limited)
end
# rubocop: enable Gitlab/ModuleWithInstanceVariables
diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb
index 808265634da..78dc196b08e 100644
--- a/app/controllers/projects/merge_requests/creations_controller.rb
+++ b/app/controllers/projects/merge_requests/creations_controller.rb
@@ -109,7 +109,13 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap
@target_project = @merge_request.target_project
@source_project = @merge_request.source_project
- @commits = set_commits_for_rendering(@merge_request.commits)
+
+ @commits =
+ set_commits_for_rendering(
+ @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch),
+ commits_count: @merge_request.commits_count
+ )
+
@commit = @merge_request.diff_head_commit
# FIXME: We have to assign a presenter to another instance variable
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 9f6f6621bf4..59987da21ec 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -90,7 +90,10 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
# Get commits from repository
# or from cache if already merged
@commits =
- set_commits_for_rendering(@merge_request.commits.with_latest_pipeline)
+ set_commits_for_rendering(
+ @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch),
+ commits_count: @merge_request.commits_count
+ )
render json: { html: view_to_html_string('projects/merge_requests/_commits') }
end
@@ -252,7 +255,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end
def merge_params_attributes
- [:should_remove_source_branch, :commit_message, :squash_commit_message, :squash, :auto_merge_strategy]
+ MergeRequest::KNOWN_MERGE_PARAMS
end
def auto_merge_requested?
@@ -292,7 +295,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
return :sha_mismatch if params[:sha] != @merge_request.diff_head_sha
- @merge_request.update(merge_error: nil, squash: merge_params.fetch(:squash, false))
+ @merge_request.update(merge_error: nil, squash: params.fetch(:squash, false))
if auto_merge_requested?
if merge_request.auto_merge_enabled?
diff --git a/app/finders/todos_finder.rb b/app/finders/todos_finder.rb
index cc71ea8e916..e56009be33d 100644
--- a/app/finders/todos_finder.rb
+++ b/app/finders/todos_finder.rb
@@ -117,12 +117,6 @@ class TodosFinder
params[:group_id].present?
end
- def project
- strong_memoize(:project) do
- Project.find_without_deleted(params[:project_id]) if project?
- end
- end
-
def group
strong_memoize(:group) do
Group.find(params[:group_id])
@@ -181,7 +175,7 @@ class TodosFinder
def by_project(items)
if project?
- items.for_project(project)
+ items.for_undeleted_projects.for_project(params[:project_id])
else
items
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index df516009397..7e1898e7142 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -374,11 +374,12 @@ class MergeRequest < ApplicationRecord
"#{project.to_reference(from, full: full)}#{reference}"
end
- def commits
- return merge_request_diff.commits if persisted?
+ def commits(limit: nil)
+ return merge_request_diff.commits(limit: limit) if persisted?
commits_arr = if compare_commits
- compare_commits.reverse
+ reversed_commits = compare_commits.reverse
+ limit ? reversed_commits.take(limit) : reversed_commits
else
[]
end
@@ -386,6 +387,10 @@ class MergeRequest < ApplicationRecord
CommitCollection.new(source_project, commits_arr, source_branch)
end
+ def recent_commits
+ commits(limit: MergeRequestDiff::COMMITS_SAFE_SIZE)
+ end
+
def commits_count
if persisted?
merge_request_diff.commits_count
diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb
index 5fe97a13a42..70ce4df5678 100644
--- a/app/models/merge_request_diff.rb
+++ b/app/models/merge_request_diff.rb
@@ -213,8 +213,10 @@ class MergeRequestDiff < ApplicationRecord
end
end
- def commits
- @commits ||= load_commits
+ def commits(limit: nil)
+ strong_memoize(:"commits_#{limit || 'all'}") do
+ load_commits(limit: limit)
+ end
end
def last_commit_sha
@@ -529,8 +531,9 @@ class MergeRequestDiff < ApplicationRecord
end
end
- def load_commits
- commits = merge_request_diff_commits.map { |commit| Commit.from_hash(commit.to_hash, project) }
+ def load_commits(limit: nil)
+ commits = merge_request_diff_commits.limit(limit)
+ .map { |commit| Commit.from_hash(commit.to_hash, project) }
CommitCollection
.new(merge_request.source_project, commits, merge_request.source_branch)
diff --git a/app/models/project.rb b/app/models/project.rb
index 1df2a981658..f4aa336fbcd 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -464,13 +464,6 @@ class Project < ApplicationRecord
# Used by Projects::CleanupService to hold a map of rewritten object IDs
mount_uploader :bfg_object_map, AttachmentUploader
- # Returns a project, if it is not about to be removed.
- #
- # id - The ID of the project to retrieve.
- def self.find_without_deleted(id)
- without_deleted.find_by_id(id)
- end
-
def self.eager_load_namespace_and_owner
includes(namespace: :owner)
end
diff --git a/app/models/todo.rb b/app/models/todo.rb
index 4e48fb3b782..f217c942e8e 100644
--- a/app/models/todo.rb
+++ b/app/models/todo.rb
@@ -55,7 +55,8 @@ class Todo < ApplicationRecord
scope :done, -> { with_state(:done) }
scope :for_action, -> (action) { where(action: action) }
scope :for_author, -> (author) { where(author: author) }
- scope :for_project, -> (project) { where(project: project) }
+ scope :for_project, -> (projects) { where(project: projects) }
+ scope :for_undeleted_projects, -> { joins(:project).merge(Project.without_deleted) }
scope :for_group, -> (group) { where(group: group) }
scope :for_type, -> (type) { where(target_type: type) }
scope :for_target, -> (id) { where(target_id: id) }
diff --git a/app/services/merge_requests/merge_base_service.rb b/app/services/merge_requests/merge_base_service.rb
index 3f7f8bcdcbf..27b5e31faab 100644
--- a/app/services/merge_requests/merge_base_service.rb
+++ b/app/services/merge_requests/merge_base_service.rb
@@ -19,10 +19,12 @@ module MergeRequests
end
def source
- if merge_request.squash
- squash_sha!
- else
- merge_request.diff_head_sha
+ strong_memoize(:source) do
+ if merge_request.squash
+ squash_sha!
+ else
+ merge_request.diff_head_sha
+ end
end
end
@@ -58,16 +60,14 @@ module MergeRequests
end
def squash_sha!
- strong_memoize(:squash_sha) do
- params[:merge_request] = merge_request
- squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute
-
- case squash_result[:status]
- when :success
- squash_result[:squash_sha]
- when :error
- raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
- end
+ params[:merge_request] = merge_request
+ squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute
+
+ case squash_result[:status]
+ when :success
+ squash_result[:squash_sha]
+ when :error
+ raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
end
end
end
diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb
index 6309052244d..a45b4f1142e 100644
--- a/app/services/merge_requests/merge_service.rb
+++ b/app/services/merge_requests/merge_service.rb
@@ -37,6 +37,7 @@ module MergeRequests
def validate!
authorization_check!
error_check!
+ updated_check!
end
def authorization_check!
@@ -60,6 +61,15 @@ module MergeRequests
raise_error(error) if error
end
+ def updated_check!
+ return unless Feature.enabled?(:validate_merge_sha, merge_request.target_project, default_enabled: false)
+
+ unless source_matches?
+ raise_error('Branch has been updated since the merge was requested. '\
+ 'Please review the changes.')
+ end
+ end
+
def commit
log_info("Git merge started on JID #{merge_jid}")
commit_id = try_merge
@@ -125,5 +135,11 @@ module MergeRequests
def merge_request_info
merge_request.to_reference(full: true)
end
+
+ def source_matches?
+ # params-keys are symbols coming from the controller, but when they get
+ # loaded from the database they're strings
+ params.with_indifferent_access[:sha] == merge_request.diff_head_sha
+ end
end
end
diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb
index 7c9abb12b6e..8a6a7119508 100644
--- a/app/services/merge_requests/update_service.rb
+++ b/app/services/merge_requests/update_service.rb
@@ -88,9 +88,9 @@ module MergeRequests
merge_request.update(merge_error: nil)
if merge_request.head_pipeline && merge_request.head_pipeline.active?
- AutoMergeService.new(project, current_user).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
+ AutoMergeService.new(project, current_user, { sha: last_diff_sha }).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
else
- merge_request.merge_async(current_user.id, {})
+ merge_request.merge_async(current_user.id, { sha: last_diff_sha })
end
end
diff --git a/changelogs/unreleased/30182-checkbox-with-no-text-causing-following-checkboxes-not-to-be-saved.yml b/changelogs/unreleased/30182-checkbox-with-no-text-causing-following-checkboxes-not-to-be-saved.yml
new file mode 100644
index 00000000000..5ea21bbd983
--- /dev/null
+++ b/changelogs/unreleased/30182-checkbox-with-no-text-causing-following-checkboxes-not-to-be-saved.yml
@@ -0,0 +1,5 @@
+---
+title: Fix checking task item when previous tasks contain only spaces
+merge_request: 19724
+author:
+type: fixed
diff --git a/changelogs/unreleased/32534-gitlab-rake-gitlab-cleanup-orphan_job_artifact_files-dry_run-false-is-not-removing-artifacts.yml b/changelogs/unreleased/32534-gitlab-rake-gitlab-cleanup-orphan_job_artifact_files-dry_run-false-is-not-removing-artifacts.yml
new file mode 100644
index 00000000000..aa0e5fee47a
--- /dev/null
+++ b/changelogs/unreleased/32534-gitlab-rake-gitlab-cleanup-orphan_job_artifact_files-dry_run-false-is-not-removing-artifacts.yml
@@ -0,0 +1,5 @@
+---
+title: Correctly cleanup orphan job artifacts
+merge_request: 17679
+author: Adam Mulvany
+type: fixed
diff --git a/changelogs/unreleased/34610-Remove-IIFEs-from-new_branch_form-js.yml b/changelogs/unreleased/34610-Remove-IIFEs-from-new_branch_form-js.yml
new file mode 100644
index 00000000000..7875772222f
--- /dev/null
+++ b/changelogs/unreleased/34610-Remove-IIFEs-from-new_branch_form-js.yml
@@ -0,0 +1,5 @@
+---
+title: Remove IIFEs from new_branch_form.js
+merge_request: 20009
+author: minghuan lei
+type: other
diff --git a/changelogs/unreleased/36460-metrics-dashboard-fails-to-load-script-doesn-t-stop.yml b/changelogs/unreleased/36460-metrics-dashboard-fails-to-load-script-doesn-t-stop.yml
new file mode 100644
index 00000000000..313df8ba4fb
--- /dev/null
+++ b/changelogs/unreleased/36460-metrics-dashboard-fails-to-load-script-doesn-t-stop.yml
@@ -0,0 +1,5 @@
+---
+title: Remove update hook from date filter to prevent js from getting stuck
+merge_request: 20215
+author:
+type: fixed
diff --git a/changelogs/unreleased/chore-slugify-duplication-removal.yml b/changelogs/unreleased/chore-slugify-duplication-removal.yml
new file mode 100644
index 00000000000..a076e7f5429
--- /dev/null
+++ b/changelogs/unreleased/chore-slugify-duplication-removal.yml
@@ -0,0 +1,5 @@
+---
+title: Remove duplication from slugifyWithUnderscore function
+merge_request: 20016
+author: Arun Kumar Mohan
+type: other
diff --git a/db/migrate/20191114173508_add_resolved_attributes_to_vulnerabilities.rb b/db/migrate/20191114173508_add_resolved_attributes_to_vulnerabilities.rb
new file mode 100644
index 00000000000..ec45a729ebb
--- /dev/null
+++ b/db/migrate/20191114173508_add_resolved_attributes_to_vulnerabilities.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class AddResolvedAttributesToVulnerabilities < ActiveRecord::Migration[5.2]
+ DOWNTIME = false
+
+ def up
+ add_column :vulnerabilities, :resolved_by_id, :bigint
+ add_column :vulnerabilities, :resolved_at, :datetime_with_timezone
+ end
+
+ def down
+ remove_column :vulnerabilities, :resolved_at
+ remove_column :vulnerabilities, :resolved_by_id
+ end
+end
diff --git a/db/migrate/20191114173602_add_foreign_key_on_resolved_by_id_to_vulnerabilities.rb b/db/migrate/20191114173602_add_foreign_key_on_resolved_by_id_to_vulnerabilities.rb
new file mode 100644
index 00000000000..e0a125ca756
--- /dev/null
+++ b/db/migrate/20191114173602_add_foreign_key_on_resolved_by_id_to_vulnerabilities.rb
@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class AddForeignKeyOnResolvedByIdToVulnerabilities < ActiveRecord::Migration[5.2]
+ include Gitlab::Database::MigrationHelpers
+
+ DOWNTIME = false
+
+ disable_ddl_transaction!
+
+ def up
+ add_concurrent_index :vulnerabilities, :resolved_by_id
+ add_concurrent_foreign_key :vulnerabilities, :users, column: :resolved_by_id, on_delete: :nullify
+ end
+
+ def down
+ remove_foreign_key :vulnerabilities, column: :resolved_by_id
+ remove_concurrent_index :vulnerabilities, :resolved_by_id
+ end
+end
diff --git a/db/post_migrate/20191114173624_set_resolved_state_on_vulnerabilities.rb b/db/post_migrate/20191114173624_set_resolved_state_on_vulnerabilities.rb
new file mode 100644
index 00000000000..b28aecdc0a3
--- /dev/null
+++ b/db/post_migrate/20191114173624_set_resolved_state_on_vulnerabilities.rb
@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+class SetResolvedStateOnVulnerabilities < ActiveRecord::Migration[5.2]
+ DOWNTIME = false
+
+ def up
+ execute <<~SQL
+ -- selecting IDs for all non-orphan Findings that either have no feedback or it's a non-dismissal feedback
+ WITH resolved_vulnerability_ids AS (
+ SELECT DISTINCT vulnerability_id AS id
+ FROM vulnerability_occurrences
+ LEFT JOIN vulnerability_feedback ON vulnerability_feedback.project_fingerprint = ENCODE(vulnerability_occurrences.project_fingerprint::bytea, 'HEX')
+ WHERE vulnerability_id IS NOT NULL
+ AND (vulnerability_feedback.id IS NULL OR vulnerability_feedback.feedback_type <> 0)
+ )
+ UPDATE vulnerabilities
+ SET state = 3, resolved_by_id = closed_by_id, resolved_at = NOW()
+ FROM resolved_vulnerability_ids
+ WHERE vulnerabilities.id IN (resolved_vulnerability_ids.id)
+ AND state = 2 -- only 'closed' Vulnerabilities become 'resolved'
+ SQL
+ end
+
+ def down
+ execute <<~SQL
+ UPDATE vulnerabilities
+ SET state = 2, resolved_by_id = NULL, resolved_at = NULL -- state = 'closed'
+ WHERE state = 3 -- 'resolved'
+ SQL
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 72d1d957d6b..ebf1eb41499 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 2019_11_12_232338) do
+ActiveRecord::Schema.define(version: 2019_11_14_173624) do
# These are extensions that must be enabled in order to support this database
enable_extension "pg_trgm"
@@ -3952,6 +3952,8 @@ ActiveRecord::Schema.define(version: 2019_11_12_232338) do
t.boolean "severity_overridden", default: false
t.integer "confidence", limit: 2, null: false
t.boolean "confidence_overridden", default: false
+ t.bigint "resolved_by_id"
+ t.datetime_with_timezone "resolved_at"
t.integer "report_type", limit: 2, null: false
t.integer "cached_markdown_version"
t.index ["author_id"], name: "index_vulnerabilities_on_author_id"
@@ -3961,6 +3963,7 @@ ActiveRecord::Schema.define(version: 2019_11_12_232338) do
t.index ["last_edited_by_id"], name: "index_vulnerabilities_on_last_edited_by_id"
t.index ["milestone_id"], name: "index_vulnerabilities_on_milestone_id"
t.index ["project_id"], name: "index_vulnerabilities_on_project_id"
+ t.index ["resolved_by_id"], name: "index_vulnerabilities_on_resolved_by_id"
t.index ["start_date_sourcing_milestone_id"], name: "index_vulnerabilities_on_start_date_sourcing_milestone_id"
t.index ["updated_by_id"], name: "index_vulnerabilities_on_updated_by_id"
end
@@ -4518,6 +4521,7 @@ ActiveRecord::Schema.define(version: 2019_11_12_232338) do
add_foreign_key "vulnerabilities", "users", column: "author_id", name: "fk_b1de915a15", on_delete: :nullify
add_foreign_key "vulnerabilities", "users", column: "closed_by_id", name: "fk_cf5c60acbf", on_delete: :nullify
add_foreign_key "vulnerabilities", "users", column: "last_edited_by_id", name: "fk_1302949740", on_delete: :nullify
+ add_foreign_key "vulnerabilities", "users", column: "resolved_by_id", name: "fk_76bc5f5455", on_delete: :nullify
add_foreign_key "vulnerabilities", "users", column: "updated_by_id", name: "fk_7ac31eacb9", on_delete: :nullify
add_foreign_key "vulnerability_feedback", "ci_pipelines", column: "pipeline_id", on_delete: :nullify
add_foreign_key "vulnerability_feedback", "issues", on_delete: :nullify
diff --git a/doc/development/documentation/styleguide.md b/doc/development/documentation/styleguide.md
index 8d68079963f..e6d666473c3 100644
--- a/doc/development/documentation/styleguide.md
+++ b/doc/development/documentation/styleguide.md
@@ -1179,12 +1179,12 @@ Rendered example:
- Prefer to use examples using the personal access token and don't pass data of
username and password.
-| Methods | Description |
-|:-------------------------------------------|:------------------------------------------------------|
-| `-H "PRIVATE-TOKEN: <your_access_token>"` | Use this method as is, whenever authentication needed |
-| `-X POST` | Use this method when creating new objects |
-| `-X PUT` | Use this method when updating existing objects |
-| `-X DELETE` | Use this method when removing existing objects |
+| Methods | Description |
+|:------------------------------------------- |:------------------------------------------------------|
+| `--header "PRIVATE-TOKEN: <your_access_token>"` | Use this method as is, whenever authentication needed |
+| `--request POST` | Use this method when creating new objects |
+| `--request PUT` | Use this method when updating existing objects |
+| `--request DELETE` | Use this method when removing existing objects |
### cURL Examples
@@ -1206,9 +1206,9 @@ Create a new project under the authenticated user's namespace:
curl --request POST --header "PRIVATE-TOKEN: <your_access_token>" "https://gitlab.example.com/api/v4/projects?name=foo"
```
-#### Post data using cURL's --data
+#### Post data using cURL's `--data`
-Instead of using `-X POST` and appending the parameters to the URI, you can use
+Instead of using `--request POST` and appending the parameters to the URI, you can use
cURL's `--data` option. The example below will create a new project `foo` under
the authenticated user's namespace.
diff --git a/doc/development/geo.md b/doc/development/geo.md
index 2fb4cc710ff..5010e44e826 100644
--- a/doc/development/geo.md
+++ b/doc/development/geo.md
@@ -491,6 +491,24 @@ When some write actions are not allowed because the node is a
The database itself will already be read-only in a replicated setup,
so we don't need to take any extra step for that.
+## Steps needed to replicate a new data type
+
+As GitLab evolves, we constantly need to add new resources to the Geo replication system.
+The implementation depends on resource specifics, but there are several things
+that need to be taken care of:
+
+- Event generation on the primary site. Whenever a new resource is changed/updated, we need to
+ create a task for the Log Cursor.
+- Event handling. The Log Cursor needs to have a handler for every event type generated by the primary site.
+- Dispatch worker (cron job). Make sure the backfill condition works well.
+- Sync worker.
+- Registry with all possible states.
+- Verification.
+- Cleaner. When sync settings are changed for the secondary site, some resources need to be cleaned up.
+- Geo Node Status. We need to provide API endpoints as well as some presentation in the GitLab Admin Area.
+- Health Check. If we can perform some pre-cheсks and make node unhealthy if something is wrong, we should do that.
+ The `rake gitlab:geo:check` command has to be updated too.
+
## History of communication channel
The communication channel has changed since first iteration, you can
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 47ee81c7080..6e10414def4 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -407,7 +407,8 @@ module API
merge_params = HashWithIndifferentAccess.new(
commit_message: params[:merge_commit_message],
squash_commit_message: params[:squash_commit_message],
- should_remove_source_branch: params[:should_remove_source_branch]
+ should_remove_source_branch: params[:should_remove_source_branch],
+ sha: params[:sha] || merge_request.diff_head_sha
)
if merge_when_pipeline_succeeds && merge_request.head_pipeline && merge_request.head_pipeline.active?
diff --git a/lib/gitlab/cleanup/orphan_job_artifact_files.rb b/lib/gitlab/cleanup/orphan_job_artifact_files.rb
index 1b01ca25559..020de45e5bf 100644
--- a/lib/gitlab/cleanup/orphan_job_artifact_files.rb
+++ b/lib/gitlab/cleanup/orphan_job_artifact_files.rb
@@ -8,7 +8,8 @@ module Gitlab
ABSOLUTE_ARTIFACT_DIR = ::JobArtifactUploader.root.freeze
LOST_AND_FOUND = File.join(ABSOLUTE_ARTIFACT_DIR, '-', 'lost+found').freeze
BATCH_SIZE = 500
- DEFAULT_NICENESS = 'Best-effort'
+ DEFAULT_NICENESS = 'best-effort'
+ VALID_NICENESS_LEVELS = %w{none realtime best-effort idle}.freeze
attr_accessor :batch, :total_found, :total_cleaned
attr_reader :limit, :dry_run, :niceness, :logger
@@ -16,7 +17,7 @@ module Gitlab
def initialize(limit: nil, dry_run: true, niceness: nil, logger: nil)
@limit = limit
@dry_run = dry_run
- @niceness = niceness || DEFAULT_NICENESS
+ @niceness = (niceness || DEFAULT_NICENESS).downcase
@logger = logger || Rails.logger # rubocop:disable Gitlab/RailsLogger
@total_found = @total_cleaned = 0
@@ -35,7 +36,7 @@ module Gitlab
clean_batch!
- log_info("Processed #{total_found} job artifacts to find and clean #{total_cleaned} orphans.")
+ log_info("Processed #{total_found} job artifact(s) to find and cleaned #{total_cleaned} orphan(s).")
end
private
@@ -75,7 +76,7 @@ module Gitlab
def find_artifacts
Open3.popen3(*find_command) do |stdin, stdout, stderr, status_thread|
stdout.each_line do |line|
- yield line
+ yield line.chomp
end
log_error(stderr.read.color(:red)) unless status_thread.value.success?
@@ -99,7 +100,7 @@ module Gitlab
cmd += %w[-type d]
if ionice
- raise ArgumentError, 'Invalid niceness' unless niceness.match?(/^\w[\w\-]*$/)
+ raise ArgumentError, 'Invalid niceness' unless VALID_NICENESS_LEVELS.include?(niceness)
cmd.unshift(*%W[#{ionice} --class #{niceness}])
end
diff --git a/package.json b/package.json
index f247b972bd2..2a439be9514 100644
--- a/package.json
+++ b/package.json
@@ -75,7 +75,7 @@
"d3-time-format": "^2.1.1",
"d3-transition": "^1.1.1",
"dateformat": "^3.0.3",
- "deckar01-task_list": "^2.2.0",
+ "deckar01-task_list": "^2.2.1",
"diff": "^3.4.0",
"document-register-element": "1.13.1",
"dropzone": "^4.2.0",
diff --git a/spec/controllers/concerns/renders_commits_spec.rb b/spec/controllers/concerns/renders_commits_spec.rb
new file mode 100644
index 00000000000..79350847383
--- /dev/null
+++ b/spec/controllers/concerns/renders_commits_spec.rb
@@ -0,0 +1,60 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe RendersCommits do
+ let_it_be(:project) { create(:project, :public, :repository) }
+ let_it_be(:merge_request) { create(:merge_request, source_project: project) }
+ let_it_be(:user) { create(:user) }
+
+ controller(ApplicationController) do
+ # `described_class` is not available in this context
+ include RendersCommits # rubocop:disable RSpec/DescribedClass
+
+ def index
+ @merge_request = MergeRequest.find(params[:id])
+ @commits = set_commits_for_rendering(
+ @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch),
+ commits_count: @merge_request.commits_count
+ )
+
+ render json: { html: view_to_html_string('projects/merge_requests/_commits') }
+ end
+ end
+
+ before do
+ sign_in(user)
+ end
+
+ def go
+ get :index, params: { id: merge_request.id }
+ end
+
+ it 'sets instance variables for counts' do
+ stub_const("MergeRequestDiff::COMMITS_SAFE_SIZE", 10)
+
+ go
+
+ expect(assigns[:total_commit_count]).to eq(29)
+ expect(assigns[:hidden_commit_count]).to eq(19)
+ expect(assigns[:commits].size).to eq(10)
+ end
+
+ context 'rendering commits' do
+ render_views
+
+ it 'avoids N + 1' do
+ stub_const("MergeRequestDiff::COMMITS_SAFE_SIZE", 5)
+
+ control_count = ActiveRecord::QueryRecorder.new(skip_cached: false) do
+ go
+ end.count
+
+ stub_const("MergeRequestDiff::COMMITS_SAFE_SIZE", 15)
+
+ expect do
+ go
+ end.not_to exceed_all_query_limit(control_count)
+ end
+ end
+end
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index e49c0f60eeb..9f7fde2f0da 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -405,7 +405,7 @@ describe Projects::MergeRequestsController do
end
it 'starts the merge immediately with permitted params' do
- expect(MergeWorker).to receive(:perform_async).with(merge_request.id, anything, { 'squash' => false })
+ expect(MergeWorker).to receive(:perform_async).with(merge_request.id, anything, { 'sha' => merge_request.diff_head_sha })
merge_with_sha
end
@@ -432,9 +432,14 @@ describe Projects::MergeRequestsController do
let(:message) { 'My custom squash commit message' }
it 'passes the same message to SquashService', :sidekiq_might_not_need_inline do
- params = { squash: '1', squash_commit_message: message }
-
- expect_next_instance_of(MergeRequests::SquashService, project, user, params.merge(merge_request: merge_request)) do |squash_service|
+ params = { squash: '1',
+ squash_commit_message: message,
+ sha: merge_request.diff_head_sha }
+ expected_squash_params = { squash_commit_message: message,
+ sha: merge_request.diff_head_sha,
+ merge_request: merge_request }
+
+ expect_next_instance_of(MergeRequests::SquashService, project, user, expected_squash_params) do |squash_service|
expect(squash_service).to receive(:execute).and_return({
status: :success,
squash_sha: SecureRandom.hex(20)
diff --git a/spec/factories/merge_requests.rb b/spec/factories/merge_requests.rb
index 13612214e72..42248dc1165 100644
--- a/spec/factories/merge_requests.rb
+++ b/spec/factories/merge_requests.rb
@@ -100,6 +100,7 @@ FactoryBot.define do
auto_merge_enabled { true }
auto_merge_strategy { AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS }
merge_user { author }
+ merge_params { { sha: diff_head_sha } }
end
trait :remove_source_branch do
diff --git a/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb b/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb
index 1d62f7f0702..d7675cd06a8 100644
--- a/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb
+++ b/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb
@@ -15,7 +15,7 @@ describe 'Merge request > User cherry-picks', :js do
context 'Viewing a merged merge request' do
before do
- service = MergeRequests::MergeService.new(project, user)
+ service = MergeRequests::MergeService.new(project, user, sha: merge_request.diff_head_sha)
perform_enqueued_jobs do
service.execute(merge_request)
diff --git a/spec/finders/todos_finder_spec.rb b/spec/finders/todos_finder_spec.rb
index 75aa10c66a2..a837e7af251 100644
--- a/spec/finders/todos_finder_spec.rb
+++ b/spec/finders/todos_finder_spec.rb
@@ -163,6 +163,28 @@ describe TodosFinder do
expect(todos).to match_array([todo1, todo2])
end
end
+
+ context 'by project' do
+ let_it_be(:project1) { create(:project) }
+ let_it_be(:project2) { create(:project) }
+ let_it_be(:project3) { create(:project) }
+
+ let!(:todo1) { create(:todo, user: user, project: project1, state: :pending) }
+ let!(:todo2) { create(:todo, user: user, project: project2, state: :pending) }
+ let!(:todo3) { create(:todo, user: user, project: project3, state: :pending) }
+
+ it 'returns the expected todos for one project' do
+ todos = finder.new(user, { project_id: project2.id }).execute
+
+ expect(todos).to match_array([todo2])
+ end
+
+ it 'returns the expected todos for many projects' do
+ todos = finder.new(user, { project_id: [project2.id, project1.id] }).execute
+
+ expect(todos).to match_array([todo2, todo1])
+ end
+ end
end
context 'external authorization' do
diff --git a/spec/frontend/fixtures/merge_requests.rb b/spec/frontend/fixtures/merge_requests.rb
index 8fbdb534b3d..f20c0aa3540 100644
--- a/spec/frontend/fixtures/merge_requests.rb
+++ b/spec/frontend/fixtures/merge_requests.rb
@@ -8,7 +8,23 @@ describe Projects::MergeRequestsController, '(JavaScript fixtures)', type: :cont
let(:admin) { create(:admin) }
let(:namespace) { create(:namespace, name: 'frontend-fixtures' )}
let(:project) { create(:project, :repository, namespace: namespace, path: 'merge-requests-project') }
- let(:merge_request) { create(:merge_request, :with_diffs, source_project: project, target_project: project, description: '- [ ] Task List Item') }
+
+ # rubocop: disable Layout/TrailingWhitespace
+ let(:merge_request) do
+ create(
+ :merge_request,
+ :with_diffs,
+ source_project: project,
+ target_project: project,
+ description: <<~MARKDOWN.strip_heredoc
+ - [ ] Task List Item
+ - [ ]
+ - [ ] Task List Item 2
+ MARKDOWN
+ )
+ end
+ # rubocop: enable Layout/TrailingWhitespace
+
let(:merged_merge_request) { create(:merge_request, :merged, source_project: project, target_project: project) }
let(:pipeline) do
create(
diff --git a/spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js b/spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js
index be544435671..ca05461c8cf 100644
--- a/spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js
+++ b/spec/frontend/monitoring/components/date_time_picker/date_time_picker_spec.js
@@ -51,6 +51,16 @@ describe('DateTimePicker', () => {
});
});
+ it('renders dropdown without a selectedTimeWindow set', done => {
+ createComponent({
+ selectedTimeWindow: {},
+ });
+ dateTimePicker.vm.$nextTick(() => {
+ expect(dateTimePicker.findAll('input').length).toBe(2);
+ done();
+ });
+ });
+
it('renders inputs with h/m/s truncated if its all 0s', done => {
createComponent({
selectedTimeWindow: {
diff --git a/spec/javascripts/merge_request_spec.js b/spec/javascripts/merge_request_spec.js
index 998637ef595..54071ccc5c2 100644
--- a/spec/javascripts/merge_request_spec.js
+++ b/spec/javascripts/merge_request_spec.js
@@ -1,5 +1,3 @@
-/* eslint-disable no-return-assign */
-
import $ from 'jquery';
import MockAdapter from 'axios-mock-adapter';
import axios from '~/lib/utils/axios_utils';
@@ -22,7 +20,8 @@ describe('MergeRequest', function() {
.onPatch(`${gl.TEST_HOST}/frontend-fixtures/merge-requests-project/merge_requests/1.json`)
.reply(200, {});
- return (this.merge = new MergeRequest());
+ this.merge = new MergeRequest();
+ return this.merge;
});
afterEach(() => {
@@ -34,10 +33,30 @@ describe('MergeRequest', function() {
const changeEvent = document.createEvent('HTMLEvents');
changeEvent.initEvent('change', true, true);
$('input[type=checkbox]')
+ .first()
+ .attr('checked', true)[0]
+ .dispatchEvent(changeEvent);
+ setTimeout(() => {
+ expect($('.js-task-list-field').val()).toBe(
+ '- [x] Task List Item\n- [ ] \n- [ ] Task List Item 2\n',
+ );
+ done();
+ });
+ });
+
+ it('ensure that task with only spaces does not get checked incorrectly', done => {
+ // fixed in 'deckar01-task_list', '2.2.1' gem
+ spyOn($, 'ajax').and.stub();
+ const changeEvent = document.createEvent('HTMLEvents');
+ changeEvent.initEvent('change', true, true);
+ $('input[type=checkbox]')
+ .last()
.attr('checked', true)[0]
.dispatchEvent(changeEvent);
setTimeout(() => {
- expect($('.js-task-list-field').val()).toBe('- [x] Task List Item');
+ expect($('.js-task-list-field').val()).toBe(
+ '- [ ] Task List Item\n- [ ] \n- [x] Task List Item 2\n',
+ );
done();
});
});
@@ -59,7 +78,7 @@ describe('MergeRequest', function() {
`${gl.TEST_HOST}/frontend-fixtures/merge-requests-project/merge_requests/1.json`,
{
merge_request: {
- description: '- [ ] Task List Item',
+ description: '- [ ] Task List Item\n- [ ] \n- [ ] Task List Item 2\n',
lock_version: 0,
update_task: { line_number: lineNumber, line_source: lineSource, index, checked },
},
diff --git a/spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb b/spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb
index 974cc2c4660..fc9792e16d7 100644
--- a/spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb
+++ b/spec/lib/gitlab/cleanup/orphan_job_artifact_files_spec.rb
@@ -21,11 +21,10 @@ describe Gitlab::Cleanup::OrphanJobArtifactFiles do
end
it 'errors when invalid niceness is given' do
+ allow(Gitlab::Utils).to receive(:which).with('ionice').and_return('/fake/ionice')
cleanup = described_class.new(logger: null_logger, niceness: 'FooBar')
- expect(null_logger).to receive(:error).with(/FooBar/)
-
- cleanup.run!
+ expect { cleanup.run! }.to raise_error('Invalid niceness')
end
it 'finds artifacts on disk' do
@@ -63,6 +62,8 @@ describe Gitlab::Cleanup::OrphanJobArtifactFiles do
def mock_artifacts_found(cleanup, *files)
mock = allow(cleanup).to receive(:find_artifacts)
- files.each { |file| mock.and_yield(file) }
+ # Because we shell out to run `find -L ...`, each file actually
+ # contains a trailing newline
+ files.each { |file| mock.and_yield("#{file}\n") }
end
end
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index b19f7a80d63..b5aa05fd8b4 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1934,7 +1934,7 @@ describe MergeRequest do
context 'when the MR has been merged' do
before do
MergeRequests::MergeService
- .new(subject.target_project, subject.author)
+ .new(subject.target_project, subject.author, { sha: subject.diff_head_sha })
.execute(subject)
end
@@ -3484,4 +3484,67 @@ describe MergeRequest do
end
it_behaves_like 'versioned description'
+
+ describe '#commits' do
+ context 'persisted merge request' do
+ context 'with a limit' do
+ it 'returns a limited number of commits' do
+ expect(subject.commits(limit: 2).map(&:sha)).to eq(%w[
+ b83d6e391c22777fca1ed3012fce84f633d7fed0
+ 498214de67004b1da3d820901307bed2a68a8ef6
+ ])
+ expect(subject.commits(limit: 3).map(&:sha)).to eq(%w[
+ b83d6e391c22777fca1ed3012fce84f633d7fed0
+ 498214de67004b1da3d820901307bed2a68a8ef6
+ 1b12f15a11fc6e62177bef08f47bc7b5ce50b141
+ ])
+ end
+ end
+
+ context 'without a limit' do
+ it 'returns all commits of the merge request diff' do
+ expect(subject.commits.size).to eq(29)
+ end
+ end
+ end
+
+ context 'new merge request' do
+ subject { build(:merge_request) }
+
+ context 'compare commits' do
+ let(:first_commit) { double }
+ let(:second_commit) { double }
+
+ before do
+ subject.compare_commits = [
+ first_commit, second_commit
+ ]
+ end
+
+ context 'without a limit' do
+ it 'returns all the compare commits' do
+ expect(subject.commits.to_a).to eq([second_commit, first_commit])
+ end
+ end
+
+ context 'with a limit' do
+ it 'returns a limited number of commits' do
+ expect(subject.commits(limit: 1).to_a).to eq([second_commit])
+ end
+ end
+ end
+ end
+ end
+
+ describe '#recent_commits' do
+ before do
+ stub_const("#{MergeRequestDiff}::COMMITS_SAFE_SIZE", 2)
+ end
+
+ it 'returns the safe number of commits' do
+ expect(subject.recent_commits.map(&:sha)).to eq(%w[
+ b83d6e391c22777fca1ed3012fce84f633d7fed0 498214de67004b1da3d820901307bed2a68a8ef6
+ ])
+ end
+ end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 0a64c70dccb..815ab7aa166 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -4899,20 +4899,6 @@ describe Project do
end
end
- describe '.find_without_deleted' do
- it 'returns nil if the project is about to be removed' do
- project = create(:project, pending_delete: true)
-
- expect(described_class.find_without_deleted(project.id)).to be_nil
- end
-
- it 'returns a project when it is not about to be removed' do
- project = create(:project)
-
- expect(described_class.find_without_deleted(project.id)).to eq(project)
- end
- end
-
describe '.for_group' do
it 'returns the projects for a given group' do
group = create(:group)
diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb
index e79a2bc325b..ea09c6caed3 100644
--- a/spec/models/todo_spec.rb
+++ b/spec/models/todo_spec.rb
@@ -221,6 +221,40 @@ describe Todo do
expect(described_class.for_project(project1)).to eq([todo])
end
+
+ it 'returns the todos for many projects' do
+ project1 = create(:project)
+ project2 = create(:project)
+ project3 = create(:project)
+
+ todo1 = create(:todo, project: project1)
+ todo2 = create(:todo, project: project2)
+ create(:todo, project: project3)
+
+ expect(described_class.for_project([project2, project1])).to contain_exactly(todo2, todo1)
+ end
+ end
+
+ describe '.for_undeleted_projects' do
+ let(:project1) { create(:project) }
+ let(:project2) { create(:project) }
+ let(:project3) { create(:project) }
+
+ let!(:todo1) { create(:todo, project: project1) }
+ let!(:todo2) { create(:todo, project: project2) }
+ let!(:todo3) { create(:todo, project: project3) }
+
+ it 'returns the todos for a given project' do
+ expect(described_class.for_undeleted_projects).to contain_exactly(todo1, todo2, todo3)
+ end
+
+ context 'when todo belongs to deleted project' do
+ let(:project2) { create(:project, pending_delete: true) }
+
+ it 'excludes todos of deleted projects' do
+ expect(described_class.for_undeleted_projects).to contain_exactly(todo1, todo3)
+ end
+ end
end
describe '.for_group' do
diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb
index 3b1096c51cb..c724a1a47b4 100644
--- a/spec/services/merge_requests/ff_merge_service_spec.rb
+++ b/spec/services/merge_requests/ff_merge_service_spec.rb
@@ -13,6 +13,7 @@ describe MergeRequests::FfMergeService do
author: create(:user))
end
let(:project) { merge_request.project }
+ let(:valid_merge_params) { { sha: merge_request.diff_head_sha } }
before do
project.add_maintainer(user)
@@ -21,7 +22,7 @@ describe MergeRequests::FfMergeService do
describe '#execute' do
context 'valid params' do
- let(:service) { described_class.new(project, user, {}) }
+ let(:service) { described_class.new(project, user, valid_merge_params) }
before do
allow(service).to receive(:execute_hooks)
@@ -52,8 +53,8 @@ describe MergeRequests::FfMergeService do
end
end
- context "error handling" do
- let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
+ context 'error handling' do
+ let(:service) { described_class.new(project, user, valid_merge_params.merge(commit_message: 'Awesome message')) }
before do
allow(Rails.logger).to receive(:error)
diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb
index 22578436c18..c938dd1cb0b 100644
--- a/spec/services/merge_requests/merge_service_spec.rb
+++ b/spec/services/merge_requests/merge_service_spec.rb
@@ -14,9 +14,12 @@ describe MergeRequests::MergeService do
end
describe '#execute' do
- context 'valid params' do
- let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
+ let(:service) { described_class.new(project, user, merge_params) }
+ let(:merge_params) do
+ { commit_message: 'Awesome message', sha: merge_request.diff_head_sha }
+ end
+ context 'valid params' do
before do
allow(service).to receive(:execute_hooks)
@@ -38,11 +41,80 @@ describe MergeRequests::MergeService do
note = merge_request.notes.last
expect(note.note).to include 'merged'
end
+
+ context 'when squashing' do
+ let(:merge_params) do
+ { commit_message: 'Merge commit message',
+ squash_commit_message: 'Squash commit message',
+ sha: merge_request.diff_head_sha }
+ end
+
+ let(:merge_request) do
+ # A merge reqeust with 5 commits
+ create(:merge_request, :simple,
+ author: user2,
+ assignees: [user2],
+ squash: true,
+ source_branch: 'improve/awesome',
+ target_branch: 'fix')
+ end
+
+ it 'merges the merge request with squashed commits' do
+ expect(merge_request).to be_merged
+
+ merge_commit = merge_request.merge_commit
+ squash_commit = merge_request.merge_commit.parents.last
+
+ expect(merge_commit.message).to eq('Merge commit message')
+ expect(squash_commit.message).to eq("Squash commit message\n")
+ end
+ end
end
- context 'closes related issues' do
- let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
+ context 'when an invalid sha is passed' do
+ let(:merge_request) do
+ create(:merge_request, :simple,
+ author: user2,
+ assignees: [user2],
+ squash: true,
+ source_branch: 'improve/awesome',
+ target_branch: 'fix')
+ end
+
+ let(:merge_params) do
+ { sha: merge_request.commits.second.sha }
+ end
+
+ it 'does not merge the MR' do
+ service.execute(merge_request)
+
+ expect(merge_request).not_to be_merged
+ expect(merge_request.merge_error).to match(/Branch has been updated/)
+ end
+ end
+
+ context 'when the `sha` param is missing' do
+ let(:merge_params) { {} }
+
+ it 'returns the error' do
+ merge_error = 'Branch has been updated since the merge was requested. '\
+ 'Please review the changes.'
+
+ expect { service.execute(merge_request) }
+ .to change { merge_request.merge_error }
+ .from(nil).to(merge_error)
+ end
+
+ it 'merges the MR when the feature is disabled' do
+ stub_feature_flags(validate_merge_sha: false)
+ service.execute(merge_request)
+
+ expect(merge_request).to be_merged
+ end
+ end
+
+ context 'closes related issues' do
before do
allow(project).to receive(:default_branch).and_return(merge_request.target_branch)
end
@@ -83,12 +155,12 @@ describe MergeRequests::MergeService do
service.execute(merge_request)
end
- context "when jira_issue_transition_id is not present" do
+ context 'when jira_issue_transition_id is not present' do
before do
allow_any_instance_of(JIRA::Resource::Issue).to receive(:resolution).and_return(nil)
end
- it "does not close issue" do
+ it 'does not close issue' do
allow(jira_tracker).to receive_messages(jira_issue_transition_id: nil)
expect_any_instance_of(JiraService).not_to receive(:transition_issue)
@@ -97,7 +169,7 @@ describe MergeRequests::MergeService do
end
end
- context "wrong issue markdown" do
+ context 'wrong issue markdown' do
it 'does not close issues on Jira issue tracker' do
jira_issue = ExternalIssue.new('#JIRA-123', project)
stub_jira_urls(jira_issue)
@@ -115,7 +187,7 @@ describe MergeRequests::MergeService do
context 'closes related todos' do
let(:merge_request) { create(:merge_request, assignees: [user], author: user) }
let(:project) { merge_request.project }
- let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
+
let!(:todo) do
create(:todo, :assigned,
project: project,
@@ -139,7 +211,7 @@ describe MergeRequests::MergeService do
context 'source branch removal' do
context 'when the source branch is protected' do
let(:service) do
- described_class.new(project, user, 'should_remove_source_branch' => true)
+ described_class.new(project, user, merge_params.merge('should_remove_source_branch' => true))
end
before do
@@ -154,7 +226,7 @@ describe MergeRequests::MergeService do
context 'when the source branch is the default branch' do
let(:service) do
- described_class.new(project, user, 'should_remove_source_branch' => true)
+ described_class.new(project, user, merge_params.merge('should_remove_source_branch' => true))
end
before do
@@ -169,8 +241,6 @@ describe MergeRequests::MergeService do
context 'when the source branch can be removed' do
context 'when MR author set the source branch to be removed' do
- let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
-
before do
merge_request.update_attribute(:merge_params, { 'force_remove_source_branch' => '1' })
end
@@ -183,7 +253,7 @@ describe MergeRequests::MergeService do
end
context 'when the merger set the source branch not to be removed' do
- let(:service) { described_class.new(project, user, commit_message: 'Awesome message', 'should_remove_source_branch' => false) }
+ let(:service) { described_class.new(project, user, merge_params.merge('should_remove_source_branch' => false)) }
it 'does not delete the source branch' do
expect(DeleteBranchService).not_to receive(:new)
@@ -194,7 +264,7 @@ describe MergeRequests::MergeService do
context 'when MR merger set the source branch to be removed' do
let(:service) do
- described_class.new(project, user, commit_message: 'Awesome message', 'should_remove_source_branch' => true)
+ described_class.new(project, user, merge_params.merge('should_remove_source_branch' => true))
end
it 'removes the source branch using the current user' do
@@ -207,9 +277,7 @@ describe MergeRequests::MergeService do
end
end
- context "error handling" do
- let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
-
+ context 'error handling' do
before do
allow(Rails.logger).to receive(:error)
end
@@ -230,7 +298,7 @@ describe MergeRequests::MergeService do
it 'logs and saves error if there is an exception' do
error_message = 'error message'
- allow(service).to receive(:repository).and_raise("error message")
+ allow(service).to receive(:repository).and_raise('error message')
allow(service).to receive(:execute_hooks)
service.execute(merge_request)
@@ -310,7 +378,7 @@ describe MergeRequests::MergeService do
expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
end
- context "when fast-forward merge is not allowed" do
+ context 'when fast-forward merge is not allowed' do
before do
allow_any_instance_of(Repository).to receive(:ancestor?).and_return(nil)
end
diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb
index 758679edc45..cccafddc450 100644
--- a/spec/services/merge_requests/merge_to_ref_service_spec.rb
+++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb
@@ -76,7 +76,7 @@ describe MergeRequests::MergeToRefService do
described_class.new(project, user, **params)
end
- let(:params) { { commit_message: 'Awesome message', should_remove_source_branch: true } }
+ let(:params) { { commit_message: 'Awesome message', should_remove_source_branch: true, sha: merge_request.diff_head_sha } }
def process_merge_to_ref
perform_enqueued_jobs do
@@ -103,7 +103,7 @@ describe MergeRequests::MergeToRefService do
end
let(:merge_service) do
- MergeRequests::MergeService.new(project, user, {})
+ MergeRequests::MergeService.new(project, user, { sha: merge_request.diff_head_sha })
end
context 'when merge commit' do
@@ -205,7 +205,7 @@ describe MergeRequests::MergeToRefService do
end
context 'when target ref is passed as a parameter' do
- let(:params) { { commit_message: 'merge train', target_ref: target_ref } }
+ let(:params) { { commit_message: 'merge train', target_ref: target_ref, sha: merge_request.diff_head_sha } }
it_behaves_like 'successfully merges to ref with merge method' do
let(:first_parent_ref) { 'refs/heads/master' }
@@ -215,7 +215,7 @@ describe MergeRequests::MergeToRefService do
describe 'cascading merge refs' do
set(:project) { create(:project, :repository) }
- let(:params) { { commit_message: 'Cascading merge', first_parent_ref: first_parent_ref, target_ref: target_ref } }
+ let(:params) { { commit_message: 'Cascading merge', first_parent_ref: first_parent_ref, target_ref: target_ref, sha: merge_request.diff_head_sha } }
context 'when first merge happens' do
let(:merge_request) do
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 0cfcae1b006..baa0ecf27e3 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -219,7 +219,7 @@ describe MergeRequests::UpdateService, :mailer do
head_pipeline_of: merge_request
)
- expect(AutoMerge::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user, {})
+ expect(AutoMerge::MergeWhenPipelineSucceedsService).to receive(:new).with(project, user, { sha: merge_request.diff_head_sha })
.and_return(service_mock)
allow(service_mock).to receive(:available_for?) { true }
expect(service_mock).to receive(:execute).with(merge_request)
@@ -411,7 +411,7 @@ describe MergeRequests::UpdateService, :mailer do
context 'when auto merge is enabled and target branch changed' do
before do
- AutoMergeService.new(project, user).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
+ AutoMergeService.new(project, user, { sha: merge_request.diff_head_sha }).execute(merge_request, AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS)
update_merge_request({ target_branch: 'target' })
end
diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb
index a604359942f..d101b092e7d 100644
--- a/spec/support/helpers/cycle_analytics_helpers.rb
+++ b/spec/support/helpers/cycle_analytics_helpers.rb
@@ -77,7 +77,7 @@ module CycleAnalyticsHelpers
.new(project, user)
.closed_by_merge_requests(issue)
- merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user).execute(merge_request) }
+ merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user, sha: merge_request.diff_head_sha).execute(merge_request) }
end
def deploy_master(user, project, environment: 'production')
diff --git a/spec/workers/merge_worker_spec.rb b/spec/workers/merge_worker_spec.rb
index 138a99abde6..dc98c9836fa 100644
--- a/spec/workers/merge_worker_spec.rb
+++ b/spec/workers/merge_worker_spec.rb
@@ -20,6 +20,7 @@ describe MergeWorker do
described_class.new.perform(
merge_request.id, merge_request.author_id,
commit_message: 'wow such merge',
+ sha: merge_request.diff_head_sha,
should_remove_source_branch: true)
merge_request.reload
diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb
index eb1d3c364ac..99800135075 100644
--- a/spec/workers/process_commit_worker_spec.rb
+++ b/spec/workers/process_commit_worker_spec.rb
@@ -81,9 +81,10 @@ describe ProcessCommitWorker do
let(:commit) do
project.repository.create_branch('feature-merged', 'feature')
+ project.repository.after_create_branch
MergeRequests::MergeService
- .new(project, merge_request.author)
+ .new(project, merge_request.author, { sha: merge_request.diff_head_sha })
.execute(merge_request)
merge_request.reload.merge_commit
diff --git a/yarn.lock b/yarn.lock
index ba6a14cbba0..61173c11bf3 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -3614,10 +3614,10 @@ decamelize@^1.1.0, decamelize@^1.1.1, decamelize@^1.1.2, decamelize@^1.2.0:
resolved "https://registry.yarnpkg.com/decamelize/-/decamelize-1.2.0.tgz#f6534d15148269b20352e7bee26f501f9a191290"
integrity sha1-9lNNFRSCabIDUue+4m9QH5oZEpA=
-deckar01-task_list@^2.2.0:
- version "2.2.0"
- resolved "https://registry.yarnpkg.com/deckar01-task_list/-/deckar01-task_list-2.2.0.tgz#5cc3ea06f01d3d786b1a667064a462eb5d069bd3"
- integrity sha512-NUfu5ARoD9SC2k+fBT5cBer59iKfEdawPrmfqp5+zAahTECb8z9dsuS1Xnx7jzFAmCCLnEs3z/aYucYXzNrKkQ==
+deckar01-task_list@^2.2.1:
+ version "2.2.1"
+ resolved "https://registry.yarnpkg.com/deckar01-task_list/-/deckar01-task_list-2.2.1.tgz#e1e8a16c4fd6e153e51fd9258fdbee067ebcd86b"
+ integrity sha512-aNAVYAYwONXezSQy2p5M67wjZE+U7JpPotdegbyy1Wq35V6jDhF3qndJYA1rYnY3aI9ifCep6EMGPav/UQaBZw==
decode-uri-component@^0.2.0:
version "0.2.0"