diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-03 15:07:07 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2019-10-03 15:07:07 +0000 |
commit | 9a1c5456747a7b5b218b8b44e4b43396bf7fd705 (patch) | |
tree | dc5873f33437c897389e923a59365fb192d87fb8 | |
parent | 927cfbfe63dd3dc64df9d341d7c4328a2fe3597f (diff) | |
download | gitlab-ce-9a1c5456747a7b5b218b8b44e4b43396bf7fd705.tar.gz |
Add latest changes from gitlab-org/gitlab@master
38 files changed, 380 insertions, 176 deletions
diff --git a/.gitlab/ci/notifications.gitlab-ci.yml b/.gitlab/ci/notifications.gitlab-ci.yml index 5d7dd752fd5..21f7312063a 100644 --- a/.gitlab/ci/notifications.gitlab-ci.yml +++ b/.gitlab/ci/notifications.gitlab-ci.yml @@ -10,8 +10,10 @@ schedule:package-and-qa:notify-success: extends: - .only-canonical-schedules - .notify + before_script: + - export COMMIT_NOTES_URL="https://$CI_SERVER_HOST/$CI_PROJECT_PATH/commit/$CI_COMMIT_SHA#notes-list" script: - - 'scripts/notify-slack qa-master ":tada: Scheduled QA against master passed! :tada: See $CI_PIPELINE_URL." ci_passing' + - 'scripts/notify-slack qa-master ":tada: Scheduled QA against master passed! :tada: See $CI_PIPELINE_URL. For downstream pipelines, see $COMMIT_NOTES_URL" ci_passing' needs: ["schedule:package-and-qa"] when: on_success @@ -19,7 +21,9 @@ schedule:package-and-qa:notify-failure: extends: - .only-canonical-schedules - .notify + before_script: + - export COMMIT_NOTES_URL="https://$CI_SERVER_HOST/$CI_PROJECT_PATH/commit/$CI_COMMIT_SHA#notes-list" script: - - 'scripts/notify-slack qa-master ":skull_and_crossbones: Scheduled QA against master failed! :skull_and_crossbones: See $CI_PIPELINE_URL." ci_failing' + - 'scripts/notify-slack qa-master ":skull_and_crossbones: Scheduled QA against master failed! :skull_and_crossbones: See $CI_PIPELINE_URL. For downstream pipelines, see $COMMIT_NOTES_URL" ci_failing' needs: ["schedule:package-and-qa"] when: on_failure diff --git a/app/services/concerns/git/change_params.rb b/app/services/concerns/git/change_params.rb new file mode 100644 index 00000000000..32faf805b5e --- /dev/null +++ b/app/services/concerns/git/change_params.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Git + module ChangeParams + private + + %i[oldrev newrev ref].each do |method| + define_method method do + change[method] + end + end + + def change + @change ||= params.fetch(:change, {}) + end + end +end diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb index 0d320e96b2e..97047d96de1 100644 --- a/app/services/git/base_hooks_service.rb +++ b/app/services/git/base_hooks_service.rb @@ -3,6 +3,7 @@ module Git class BaseHooksService < ::BaseService include Gitlab::Utils::StrongMemoize + include ChangeParams # The N most recent commits to process in a single push payload. PROCESS_COMMIT_LIMIT = 100 @@ -77,20 +78,20 @@ module Git def pipeline_params { - before: params[:oldrev], - after: params[:newrev], - ref: params[:ref], + before: oldrev, + after: newrev, + ref: ref, push_options: params[:push_options] || {}, checkout_sha: Gitlab::DataBuilder::Push.checkout_sha( - project.repository, params[:newrev], params[:ref]) + project.repository, newrev, ref) } end def push_data_params(commits:, with_changed_files: true) { - oldrev: params[:oldrev], - newrev: params[:newrev], - ref: params[:ref], + oldrev: oldrev, + newrev: newrev, + ref: ref, project: project, user: current_user, commits: commits, diff --git a/app/services/git/branch_hooks_service.rb b/app/services/git/branch_hooks_service.rb index c633cff2822..69f1f9eb31f 100644 --- a/app/services/git/branch_hooks_service.rb +++ b/app/services/git/branch_hooks_service.rb @@ -20,15 +20,15 @@ module Git strong_memoize(:commits) do if creating_default_branch? # The most recent PROCESS_COMMIT_LIMIT commits in the default branch - project.repository.commits(params[:newrev], limit: PROCESS_COMMIT_LIMIT) + project.repository.commits(newrev, limit: PROCESS_COMMIT_LIMIT) elsif creating_branch? # Use the pushed commits that aren't reachable by the default branch # as a heuristic. This may include more commits than are actually # pushed, but that shouldn't matter because we check for existing # cross-references later. - project.repository.commits_between(project.default_branch, params[:newrev]) + project.repository.commits_between(project.default_branch, newrev) elsif updating_branch? - project.repository.commits_between(params[:oldrev], params[:newrev]) + project.repository.commits_between(oldrev, newrev) else # removing branch [] end @@ -70,7 +70,7 @@ module Git def branch_update_hooks # Update the bare repositories info/attributes file using the contents of # the default branch's .gitattributes file - project.repository.copy_gitattributes(params[:ref]) if default_branch? + project.repository.copy_gitattributes(ref) if default_branch? end def branch_change_hooks @@ -118,7 +118,7 @@ module Git # https://gitlab.com/gitlab-org/gitlab-foss/issues/59257 def creating_branch? strong_memoize(:creating_branch) do - Gitlab::Git.blank_ref?(params[:oldrev]) || + Gitlab::Git.blank_ref?(oldrev) || !project.repository.branch_exists?(branch_name) end end @@ -128,7 +128,7 @@ module Git end def removing_branch? - Gitlab::Git.blank_ref?(params[:newrev]) + Gitlab::Git.blank_ref?(newrev) end def creating_default_branch? @@ -137,7 +137,7 @@ module Git def count_commits_in_branch strong_memoize(:count_commits_in_branch) do - project.repository.commit_count_for_ref(params[:ref]) + project.repository.commit_count_for_ref(ref) end end @@ -148,7 +148,7 @@ module Git end def branch_name - strong_memoize(:branch_name) { Gitlab::Git.ref_name(params[:ref]) } + strong_memoize(:branch_name) { Gitlab::Git.ref_name(ref) } end def upstream_commit_ids(commits) diff --git a/app/services/git/branch_push_service.rb b/app/services/git/branch_push_service.rb index 7adc3320e06..da45bcc7eaa 100644 --- a/app/services/git/branch_push_service.rb +++ b/app/services/git/branch_push_service.rb @@ -4,6 +4,7 @@ module Git class BranchPushService < ::BaseService include Gitlab::Access include Gitlab::Utils::StrongMemoize + include ChangeParams # This method will be called after each git update # and only if the provided user and project are present in GitLab. @@ -19,7 +20,7 @@ module Git # 6. Checks if the project's main language has changed # def execute - return unless Gitlab::Git.branch_ref?(params[:ref]) + return unless Gitlab::Git.branch_ref?(ref) enqueue_update_mrs enqueue_detect_repository_languages @@ -38,9 +39,9 @@ module Git UpdateMergeRequestsWorker.perform_async( project.id, current_user.id, - params[:oldrev], - params[:newrev], - params[:ref] + oldrev, + newrev, + ref ) end @@ -69,11 +70,11 @@ module Git end def removing_branch? - Gitlab::Git.blank_ref?(params[:newrev]) + Gitlab::Git.blank_ref?(newrev) end def branch_name - strong_memoize(:branch_name) { Gitlab::Git.ref_name(params[:ref]) } + strong_memoize(:branch_name) { Gitlab::Git.ref_name(ref) } end def default_branch? diff --git a/app/services/git/tag_hooks_service.rb b/app/services/git/tag_hooks_service.rb index e5b109c79d6..0e5e1bbc992 100644 --- a/app/services/git/tag_hooks_service.rb +++ b/app/services/git/tag_hooks_service.rb @@ -18,12 +18,12 @@ module Git def tag strong_memoize(:tag) do - next if Gitlab::Git.blank_ref?(params[:newrev]) + next if Gitlab::Git.blank_ref?(newrev) - tag_name = Gitlab::Git.ref_name(params[:ref]) + tag_name = Gitlab::Git.ref_name(ref) tag = project.repository.find_tag(tag_name) - tag if tag && tag.target == params[:newrev] + tag if tag && tag.target == newrev end end diff --git a/app/services/git/tag_push_service.rb b/app/services/git/tag_push_service.rb index ee4166dccd0..9a266f7d74c 100644 --- a/app/services/git/tag_push_service.rb +++ b/app/services/git/tag_push_service.rb @@ -2,8 +2,10 @@ module Git class TagPushService < ::BaseService + include ChangeParams + def execute - return unless Gitlab::Git.tag_ref?(params[:ref]) + return unless Gitlab::Git.tag_ref?(ref) project.repository.before_push_tag TagHooksService.new(project, current_user, params).execute diff --git a/app/services/issues/close_service.rb b/app/services/issues/close_service.rb index 965351b5b6c..14585c2850b 100644 --- a/app/services/issues/close_service.rb +++ b/app/services/issues/close_service.rb @@ -4,7 +4,7 @@ module Issues class CloseService < Issues::BaseService # Closes the supplied issue if the current user is able to do so. def execute(issue, commit: nil, notifications: true, system_note: true) - return issue unless can?(current_user, :update_issue, issue) + return issue unless can?(current_user, :update_issue, issue) || issue.is_a?(ExternalIssue) close_issue(issue, closed_via: commit, diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index fbe6c48ac28..0364c0dd479 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -29,9 +29,7 @@ module MergeRequests closed_issues = merge_request.visible_closing_issues_for(current_user) closed_issues.each do |issue| - if can?(current_user, :update_issue, issue) - Issues::CloseService.new(project, current_user, {}).execute(issue, commit: merge_request) - end + Issues::CloseService.new(project, current_user).execute(issue, commit: merge_request) end end diff --git a/app/views/projects/cycle_analytics/_overview.html.haml b/app/views/projects/cycle_analytics/_overview.html.haml index 5b0d73b8c68..ea94b637f89 100644 --- a/app/views/projects/cycle_analytics/_overview.html.haml +++ b/app/views/projects/cycle_analytics/_overview.html.haml @@ -9,7 +9,7 @@ Cycle Analytics gives an overview of how much time it takes to go from idea to production in your project. To set up CA, you must first define a production environment by setting up your CI and then deploy to production. %p - %a.btn{ href: help_page_path('user/project/cycle_analytics'), target: '_blank' } Read more + %a.btn{ href: help_page_path('user/analytics/cycle_analytics.md'), target: '_blank' } Read more .col-md-6.overview-image %span.overview-icon = custom_icon ('icon_cycle_analytics_overview') diff --git a/app/views/projects/cycle_analytics/show.html.haml b/app/views/projects/cycle_analytics/show.html.haml index 6b56a4ee7ab..7fedd1ab785 100644 --- a/app/views/projects/cycle_analytics/show.html.haml +++ b/app/views/projects/cycle_analytics/show.html.haml @@ -3,7 +3,7 @@ #cycle-analytics{ "v-cloak" => "true", data: { request_path: project_cycle_analytics_path(@project) } } - if @cycle_analytics_no_data %banner{ "v-if" => "!isOverviewDialogDismissed", - "documentation-link": help_page_path('user/project/cycle_analytics'), + "documentation-link": help_page_path('user/analytics/cycle_analytics.md'), "v-on:dismiss-overview-dialog" => "dismissOverviewDialog()" } = icon("spinner spin", "v-show" => "isLoading") .wrapper{ "v-show" => "!isLoading && !hasError" } diff --git a/app/workers/post_receive.rb b/app/workers/post_receive.rb index cae3cb45c45..a4b9ef18a3b 100644 --- a/app/workers/post_receive.rb +++ b/app/workers/post_receive.rb @@ -37,41 +37,22 @@ class PostReceive end def process_project_changes(post_received) - changes = [] - refs = Set.new user = identify_user(post_received) + return false unless user + project = post_received.project + push_options = post_received.push_options + changes = post_received.changes + # We only need to expire certain caches once per push expire_caches(post_received, post_received.project.repository) enqueue_repository_cache_update(post_received) - post_received.enum_for(:changes_refs).with_index do |(oldrev, newrev, ref), index| - service_klass = - if Gitlab::Git.tag_ref?(ref) - Git::TagPushService - elsif Gitlab::Git.branch_ref?(ref) - Git::BranchPushService - end - - if service_klass - service_klass.new( - post_received.project, - user, - oldrev: oldrev, - newrev: newrev, - ref: ref, - push_options: post_received.push_options, - create_pipelines: index < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, post_received.project) - ).execute - end - - changes << Gitlab::DataBuilder::Repository.single_change(oldrev, newrev, ref) - refs << ref - end - + process_changes(Git::BranchPushService, project, user, push_options, changes.branch_changes) + process_changes(Git::TagPushService, project, user, push_options, changes.tag_changes) update_remote_mirrors(post_received) - after_project_changes_hooks(post_received, user, refs.to_a, changes) + after_project_changes_hooks(project, user, changes.refs, changes.repository_data) end # Expire the repository status, branch, and tag cache once per push. @@ -94,6 +75,20 @@ class PostReceive ) end + def process_changes(service_class, project, user, push_options, changes) + return if changes.empty? + + changes.each do |change| + service_class.new( + project, + user, + change: change, + push_options: push_options, + create_pipelines: change[:index] < PIPELINE_PROCESS_LIMIT || Feature.enabled?(:git_push_create_all_pipelines, project) + ).execute + end + end + def update_remote_mirrors(post_received) return unless post_received.includes_branches? || post_received.includes_tags? @@ -104,9 +99,9 @@ class PostReceive project.update_remote_mirrors end - def after_project_changes_hooks(post_received, user, refs, changes) - hook_data = Gitlab::DataBuilder::Repository.update(post_received.project, user, changes, refs) - SystemHooksService.new.execute_hooks(hook_data, :repository_update_hooks) + def after_project_changes_hooks(project, user, refs, changes) + repository_update_hook_data = Gitlab::DataBuilder::Repository.update(project, user, changes, refs) + SystemHooksService.new.execute_hooks(repository_update_hook_data, :repository_update_hooks) Gitlab::UsageDataCounters::SourceCodeCounter.count(:pushes) end @@ -121,7 +116,7 @@ class PostReceive # We only need to expire certain caches once per push expire_caches(post_received, post_received.project.wiki.repository) - ::Git::WikiPushService.new(post_received.project, user, changes: post_received.enum_for(:changes_refs)).execute + ::Git::WikiPushService.new(post_received.project, user, changes: post_received.changes).execute end def log(message) diff --git a/changelogs/unreleased/22879-close-jira-issues-with-issues-disabled.yml b/changelogs/unreleased/22879-close-jira-issues-with-issues-disabled.yml new file mode 100644 index 00000000000..c8f32d2226e --- /dev/null +++ b/changelogs/unreleased/22879-close-jira-issues-with-issues-disabled.yml @@ -0,0 +1,5 @@ +--- +title: 'Merge Request: Close JIRA issues when issues are disabled' +merge_request: 17743 +author: +type: fixed diff --git a/changelogs/unreleased/id-merge-request-dependencies.yml b/changelogs/unreleased/id-merge-request-dependencies.yml new file mode 100644 index 00000000000..7532979a9f6 --- /dev/null +++ b/changelogs/unreleased/id-merge-request-dependencies.yml @@ -0,0 +1,5 @@ +--- +title: Allow intra-project MR dependencies +merge_request: 16799 +author: +type: changed diff --git a/db/fixtures/development/17_cycle_analytics.rb b/db/fixtures/development/17_cycle_analytics.rb index 9d293f425e6..b7ddeef95b8 100644 --- a/db/fixtures/development/17_cycle_analytics.rb +++ b/db/fixtures/development/17_cycle_analytics.rb @@ -150,9 +150,11 @@ class Gitlab::Seeder::CycleAnalytics ::Git::BranchPushService.new( issue.project, @user, - oldrev: issue.project.repository.commit("master").sha, - newrev: commit_sha, - ref: 'refs/heads/master' + change: { + oldrev: issue.project.repository.commit("master").sha, + newrev: commit_sha, + ref: 'refs/heads/master' + } ).execute branch_name diff --git a/doc/user/admin_area/monitoring/health_check.md b/doc/user/admin_area/monitoring/health_check.md index 3fbfba2f839..6439607de33 100644 --- a/doc/user/admin_area/monitoring/health_check.md +++ b/doc/user/admin_area/monitoring/health_check.md @@ -102,6 +102,9 @@ Example response: ## Liveness +DANGER: **Warning:** +In Gitlab [12.4](https://about.gitlab.com/upcoming-releases/) the response body of the Liveness check will change to match the example below. + The liveness probe checks whether the application server is alive. Unlike the [`health`](#health) check, this check hits the database. ```text diff --git a/doc/user/project/merge_requests/img/cross_project_dependencies_edit_inaccessible_v12_2.png b/doc/user/project/merge_requests/img/cross_project_dependencies_edit_inaccessible_v12_2.png Binary files differdeleted file mode 100644 index 2dc02634fd8..00000000000 --- a/doc/user/project/merge_requests/img/cross_project_dependencies_edit_inaccessible_v12_2.png +++ /dev/null diff --git a/doc/user/project/merge_requests/img/cross_project_dependencies_edit_v12_2.png b/doc/user/project/merge_requests/img/cross_project_dependencies_edit_v12_2.png Binary files differdeleted file mode 100644 index 362e7e0ead2..00000000000 --- a/doc/user/project/merge_requests/img/cross_project_dependencies_edit_v12_2.png +++ /dev/null diff --git a/doc/user/project/merge_requests/img/dependencies_edit_inaccessible_v12_4.png b/doc/user/project/merge_requests/img/dependencies_edit_inaccessible_v12_4.png Binary files differnew file mode 100644 index 00000000000..3699ffd16b4 --- /dev/null +++ b/doc/user/project/merge_requests/img/dependencies_edit_inaccessible_v12_4.png diff --git a/doc/user/project/merge_requests/img/dependencies_edit_v12_4.png b/doc/user/project/merge_requests/img/dependencies_edit_v12_4.png Binary files differnew file mode 100644 index 00000000000..beb452e80cf --- /dev/null +++ b/doc/user/project/merge_requests/img/dependencies_edit_v12_4.png diff --git a/doc/user/project/merge_requests/img/cross_project_dependencies_view_v12_2.png b/doc/user/project/merge_requests/img/dependencies_view_v12_2.png Binary files differindex e00231c839b..e00231c839b 100644 --- a/doc/user/project/merge_requests/img/cross_project_dependencies_view_v12_2.png +++ b/doc/user/project/merge_requests/img/dependencies_view_v12_2.png diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md index 3b327472e0f..52912927e07 100644 --- a/doc/user/project/merge_requests/index.md +++ b/doc/user/project/merge_requests/index.md @@ -47,7 +47,7 @@ With **[GitLab Enterprise Edition][ee]**, you can also: - Analyze your dependencies for vulnerabilities with [Dependency Scanning](../../application_security/dependency_scanning/index.md) **(ULTIMATE)** - Analyze your Docker images for vulnerabilities with [Container Scanning](../../application_security/container_scanning/index.md) **(ULTIMATE)** - Determine the performance impact of changes with [Browser Performance Testing](#browser-performance-testing-premium) **(PREMIUM)** -- Specify merge order dependencies with [Cross-project Merge Request Dependencies](#cross-project-merge-request-dependencies-premium) **(PREMIUM)** +- Specify merge order dependencies with [Merge Request Dependencies](#merge-request-dependencies-premium) **(PREMIUM)** ## Use cases @@ -509,7 +509,7 @@ GitLab runs the [Sitespeed.io container][sitespeed-container] and displays the d [Read more about Browser Performance Testing.](browser_performance_testing.md) -## Cross-project Merge Request Dependencies **(PREMIUM)** +## Merge Request Dependencies **(PREMIUM)** > Introduced in [GitLab Premium][products] 12.2. @@ -522,7 +522,7 @@ this relationship in place, the merge request cannot be merged until all of its dependencies have also been merged, helping to maintain the consistency of a single logical change. -[Read more about cross-project merge request dependencies.](merge_request_dependencies.md) +[Read more about merge request dependencies.](merge_request_dependencies.md) ## Security reports **(ULTIMATE)** diff --git a/doc/user/project/merge_requests/merge_request_dependencies.md b/doc/user/project/merge_requests/merge_request_dependencies.md index b9f229783f1..f1a02e837a5 100644 --- a/doc/user/project/merge_requests/merge_request_dependencies.md +++ b/doc/user/project/merge_requests/merge_request_dependencies.md @@ -2,14 +2,13 @@ type: reference, concepts --- -# Cross-project Merge Request dependencies **(PREMIUM)** +# Merge Request dependencies **(PREMIUM)** > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/9688) in [GitLab Premium](https://about.gitlab.com/pricing/) 12.2. -Cross-project merge request dependencies allows a required order of merging -between merge requests in different projects to be expressed. If a -merge request "depends on" another, then it cannot be merged until its -dependency is itself merged. +Merge request dependencies allows a required order of merging +between merge requests to be expressed. If a merge request "depends on" another, +then it cannot be merged until its dependency is itself merged. NOTE: **Note:** Merge requests dependencies are a **PREMIUM** feature, but this restriction is @@ -58,20 +57,20 @@ instead. To continue the above example, you can configure a dependency when creating the new merge request in `awesome-project` (or by editing it, if it already exists). The dependency needs to be configured on the **dependent** merge -request. There is a "Cross-project dependencies" section in the form: +request. There is a **Merge request dependencies** section in the form: -![Cross-project dependencies form control](img/cross_project_dependencies_edit_v12_2.png) +![Merge request dependencies form control](img/dependencies_edit_v12_4.png) Anyone who can edit a merge request can change the list of dependencies. New dependencies can be added by reference, or by URL. To remove a dependency, press the **X** by its reference. -As dependencies are specified across projects, it's possible that someone else +As dependencies can be specified across projects, it's possible that someone else has added a dependency for a merge request in a project you don't have access to. These are shown as a simple count: -![Cross-project dependencies form control with inaccessible merge requests](img/cross_project_dependencies_edit_inaccessible_v12_2.png) +![Merge request dependencies form control with inaccessible merge requests](img/dependencies_edit_inaccessible_v12_4.png) If necessary, you can remove all the dependencies like this by pressing the **X**, just as you would for a single, visible dependency. @@ -82,7 +81,7 @@ or **Cancel** to return without making any changes. The list of configured dependencies, and the status of each one, is shown in the merge request widget: -![Cross-project dependencies in merge request widget](img/cross_project_dependencies_view_v12_2.png) +![Dependencies in merge request widget](img/dependencies_view_v12_2.png) Until all dependencies have, themselves, been merged, the **Merge** button will be disabled for the dependent merge request. In diff --git a/lib/gitlab/git/changes.rb b/lib/gitlab/git/changes.rb new file mode 100644 index 00000000000..4e888eec44f --- /dev/null +++ b/lib/gitlab/git/changes.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true + +module Gitlab + module Git + class Changes + include Enumerable + + attr_reader :repository_data + + def initialize + @refs = Set.new + @items = [] + @branches_index = [] + @tags_index = [] + @repository_data = [] + end + + def includes_branches? + branches_index.any? + end + + def includes_tags? + tags_index.any? + end + + def add_branch_change(change) + @branches_index << add_change(change) + self + end + + def add_tag_change(change) + @tags_index << add_change(change) + self + end + + def each + items.each do |item| + yield item + end + end + + def refs + @refs.to_a + end + + def branch_changes + items.values_at(*branches_index) + end + + def tag_changes + items.values_at(*tags_index) + end + + private + + attr_reader :items, :branches_index, :tags_index + + def add_change(change) + # refs and repository_data are being cached when a change is added to + # the collection to remove the need to iterate through changes multiple + # times. + @refs << change[:ref] + @repository_data << build_change_repository_data(change) + @items << change + + @items.size - 1 + end + + def build_change_repository_data(change) + DataBuilder::Repository.single_change(change[:oldrev], change[:newrev], change[:ref]) + end + end + end +end diff --git a/lib/gitlab/git_post_receive.rb b/lib/gitlab/git_post_receive.rb index 2a8bcd015a8..5264bae47a1 100644 --- a/lib/gitlab/git_post_receive.rb +++ b/lib/gitlab/git_post_receive.rb @@ -8,7 +8,7 @@ module Gitlab def initialize(project, identifier, changes, push_options = {}) @project = project @identifier = identifier - @changes = deserialize_changes(changes) + @changes = parse_changes(changes) @push_options = push_options end @@ -16,27 +16,12 @@ module Gitlab super(identifier) end - def changes_refs - return changes unless block_given? - - changes.each do |change| - change.strip! - oldrev, newrev, ref = change.split(' ') - - yield oldrev, newrev, ref - end - end - def includes_branches? - enum_for(:changes_refs).any? do |_oldrev, _newrev, ref| - Gitlab::Git.branch_ref?(ref) - end + changes.includes_branches? end def includes_tags? - enum_for(:changes_refs).any? do |_oldrev, _newrev, ref| - Gitlab::Git.tag_ref?(ref) - end + changes.includes_tags? end def includes_default_branch? @@ -44,16 +29,28 @@ module Gitlab # first branch pushed will be the default. return true unless project.default_branch.present? - enum_for(:changes_refs).any? do |_oldrev, _newrev, ref| - Gitlab::Git.branch_ref?(ref) && - Gitlab::Git.branch_name(ref) == project.default_branch + changes.branch_changes.any? do |change| + Gitlab::Git.branch_name(change[:ref]) == project.default_branch end end private - def deserialize_changes(changes) - utf8_encode_changes(changes).each_line + def parse_changes(changes) + deserialized_changes = utf8_encode_changes(changes).each_line + + Git::Changes.new.tap do |collection| + deserialized_changes.each_with_index do |raw_change, index| + oldrev, newrev, ref = raw_change.strip.split(' ') + change = { index: index, oldrev: oldrev, newrev: newrev, ref: ref } + + if Git.branch_ref?(ref) + collection.add_branch_change(change) + elsif Git.tag_ref?(ref) + collection.add_tag_change(change) + end + end + end end def utf8_encode_changes(changes) diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 2616a19fdaa..f1e31a615a4 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -42,6 +42,9 @@ module Gitlab # Initialize gon.features with any flags that should be # made globally available to the frontend push_frontend_feature_flag(:suppress_ajax_navigation_errors, default_enabled: true) + + # Flag controls a GFM feature used across many routes. + push_frontend_feature_flag(:gfm_grafana_integration) end # Exposes the state of a feature flag to the frontend code. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2b4f75d0a58..e57e4d668a5 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -1260,9 +1260,6 @@ msgstr "" msgid "All changes are committed" msgstr "" -msgid "All cross-project dependencies have merged" -msgstr "" - msgid "All email addresses will be used to identify your commits." msgstr "" @@ -1278,6 +1275,9 @@ msgstr "" msgid "All merge conflicts were resolved. The merge request can now be merged." msgstr "" +msgid "All merge request dependencies have been merged" +msgstr "" + msgid "All paths are relative to the GitLab URL. Do not include %{relative_url_link_start}relative URL%{relative_url_link_end}." msgstr "" @@ -4606,9 +4606,6 @@ msgstr "" msgid "Cron syntax" msgstr "" -msgid "Cross-project dependencies" -msgstr "" - msgid "Current Branch" msgstr "" @@ -9839,6 +9836,9 @@ msgstr "" msgid "Merge request approvals allow you to set the number of necessary approvals and predefine a list of approvers that will need to approve every merge request in a project." msgstr "" +msgid "Merge request dependencies" +msgstr "" + msgid "Merge requests" msgstr "" @@ -18696,9 +18696,6 @@ msgstr "" msgid "cannot be enabled unless all domains have TLS certificates" msgstr "" -msgid "cannot be in the same project" -msgstr "" - msgid "cannot be modified" msgstr "" diff --git a/scripts/rspec_helpers.sh b/scripts/rspec_helpers.sh index 9c9d33235c9..1c75958aef1 100644 --- a/scripts/rspec_helpers.sh +++ b/scripts/rspec_helpers.sh @@ -44,7 +44,7 @@ function rspec_simple_job() { scripts/gitaly-test-spawn - bin/rspec --color --format documentation --format RspecJunitFormatter --out junit_rspec.xml "${rspec_opts}" + bin/rspec --color --format documentation --format RspecJunitFormatter --out junit_rspec.xml ${rspec_opts} } function rspec_paralellized_job() { diff --git a/spec/features/projects/environments/environment_spec.rb b/spec/features/projects/environments/environment_spec.rb index 497880a7835..25823b75d18 100644 --- a/spec/features/projects/environments/environment_spec.rb +++ b/spec/features/projects/environments/environment_spec.rb @@ -304,9 +304,11 @@ describe 'Environment' do # def remove_branch_with_hooks(project, user, branch) params = { - oldrev: project.commit(branch).id, - newrev: Gitlab::Git::BLANK_SHA, - ref: "refs/heads/#{branch}" + change: { + oldrev: project.commit(branch).id, + newrev: Gitlab::Git::BLANK_SHA, + ref: "refs/heads/#{branch}" + } } yield diff --git a/spec/lib/gitlab/git/changes_spec.rb b/spec/lib/gitlab/git/changes_spec.rb new file mode 100644 index 00000000000..7f56d30cb48 --- /dev/null +++ b/spec/lib/gitlab/git/changes_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::Git::Changes do + let(:changes) { described_class.new } + + describe '#includes_branches?' do + subject { changes.includes_branches? } + + context 'has changes for branches' do + before do + changes.add_branch_change(oldrev: 'abc123', newrev: 'def456', ref: 'branch') + end + + it { is_expected.to be_truthy } + end + + context 'has no changes for branches' do + before do + changes.add_tag_change(oldrev: 'abc123', newrev: 'def456', ref: 'tag') + end + + it { is_expected.to be_falsey } + end + end + + describe '#includes_tags?' do + subject { changes.includes_tags? } + + context 'has changes for tags' do + before do + changes.add_tag_change(oldrev: 'abc123', newrev: 'def456', ref: 'tag') + end + + it { is_expected.to be_truthy } + end + + context 'has no changes for tags' do + before do + changes.add_branch_change(oldrev: 'abc123', newrev: 'def456', ref: 'branch') + end + + it { is_expected.to be_falsey } + end + end + + describe '#add_branch_change' do + let(:change) { { oldrev: 'abc123', newrev: 'def456', ref: 'branch' } } + + subject { changes.add_branch_change(change) } + + it 'adds the branch change to the collection' do + expect(subject).to include(change) + expect(subject.refs).to include(change[:ref]) + expect(subject.repository_data).to include(before: change[:oldrev], after: change[:newrev], ref: change[:ref]) + expect(subject.branch_changes).to include(change) + end + + it 'does not add the change as a tag change' do + expect(subject.tag_changes).not_to include(change) + end + end + + describe '#add_tag_change' do + let(:change) { { oldrev: 'abc123', newrev: 'def456', ref: 'tag' } } + + subject { changes.add_tag_change(change) } + + it 'adds the tag change to the collection' do + expect(subject).to include(change) + expect(subject.refs).to include(change[:ref]) + expect(subject.repository_data).to include(before: change[:oldrev], after: change[:newrev], ref: change[:ref]) + expect(subject.tag_changes).to include(change) + end + + it 'does not add the change as a branch change' do + expect(subject.branch_changes).not_to include(change) + end + end +end diff --git a/spec/services/git/base_hooks_service_spec.rb b/spec/services/git/base_hooks_service_spec.rb index 6dc0353299b..e71900e3c0d 100644 --- a/spec/services/git/base_hooks_service_spec.rb +++ b/spec/services/git/base_hooks_service_spec.rb @@ -8,7 +8,6 @@ describe Git::BaseHooksService do let(:user) { create(:user) } let(:project) { create(:project, :repository) } - let(:service) { described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) } let(:oldrev) { Gitlab::Git::BLANK_SHA } let(:newrev) { "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b" } # gitlab-test: git rev-parse refs/tags/v1.1.0 @@ -27,7 +26,7 @@ describe Git::BaseHooksService do let(:project) { create(:project, :repository) } - subject { TestService.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) } + subject { TestService.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }) } context '#execute_hooks' do before do diff --git a/spec/services/git/branch_hooks_service_spec.rb b/spec/services/git/branch_hooks_service_spec.rb index 206b87fb889..085b49f31ab 100644 --- a/spec/services/git/branch_hooks_service_spec.rb +++ b/spec/services/git/branch_hooks_service_spec.rb @@ -16,7 +16,7 @@ describe Git::BranchHooksService do let(:newrev) { commit.id } let(:service) do - described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) + described_class.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }) end describe "Git Push Data" do @@ -350,7 +350,7 @@ describe Git::BranchHooksService do let(:forked_project) { fork_project(upstream_project, user, repository: true) } let!(:forked_service) do - described_class.new(forked_project, user, oldrev: oldrev, newrev: newrev, ref: ref) + described_class.new(forked_project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }) end context 'when commits already exists in the upstream project' do diff --git a/spec/services/git/branch_push_service_spec.rb b/spec/services/git/branch_push_service_spec.rb index f4d1a1e34cd..bf68eb0af20 100644 --- a/spec/services/git/branch_push_service_spec.rb +++ b/spec/services/git/branch_push_service_spec.rb @@ -19,7 +19,7 @@ describe Git::BranchPushService, services: true do describe 'Push branches' do subject do - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end context 'new branch' do @@ -70,7 +70,7 @@ describe Git::BranchPushService, services: true do end describe "Pipelines" do - subject { execute_service(project, user, oldrev, newrev, ref) } + subject { execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) } before do stub_ci_pipeline_to_return_yaml_file @@ -121,7 +121,7 @@ describe Git::BranchPushService, services: true do .to receive(:perform_async) .with(project.id, user.id, blankrev, 'newrev', ref) - execute_service(project, user, blankrev, 'newrev', ref ) + execute_service(project, user, oldrev: blankrev, newrev: 'newrev', ref: ref) end end @@ -130,13 +130,13 @@ describe Git::BranchPushService, services: true do it "calls the copy attributes method for the first push to the default branch" do expect(project.repository).to receive(:copy_gitattributes).with('master') - execute_service(project, user, blankrev, 'newrev', ref) + execute_service(project, user, oldrev: blankrev, newrev: 'newrev', ref: ref) end it "calls the copy attributes method for changes to the default branch" do expect(project.repository).to receive(:copy_gitattributes).with(ref) - execute_service(project, user, 'oldrev', 'newrev', ref) + execute_service(project, user, oldrev: 'oldrev', newrev: 'newrev', ref: ref) end end @@ -149,7 +149,7 @@ describe Git::BranchPushService, services: true do it "does not call copy attributes method" do expect(project.repository).not_to receive(:copy_gitattributes) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end end end @@ -163,7 +163,7 @@ describe Git::BranchPushService, services: true do it "when pushing a branch for the first time" do expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") - execute_service(project, user, blankrev, 'newrev', ref) + execute_service(project, user, oldrev: blankrev, newrev: 'newrev', ref: ref) expect(project.protected_branches).not_to be_empty expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) @@ -174,7 +174,7 @@ describe Git::BranchPushService, services: true do expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") - execute_service(project, user, blankrev, 'newrev', ref) + execute_service(project, user, oldrev: blankrev, newrev: 'newrev', ref: ref) expect(project.protected_branches).to be_empty end @@ -184,7 +184,7 @@ describe Git::BranchPushService, services: true do expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") - execute_service(project, user, blankrev, 'newrev', ref) + execute_service(project, user, oldrev: blankrev, newrev: 'newrev', ref: ref) expect(project.protected_branches).not_to be_empty expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) @@ -199,7 +199,7 @@ describe Git::BranchPushService, services: true do expect(project.default_branch).to eq("master") expect(ProtectedBranches::CreateService).not_to receive(:new) - execute_service(project, user, blankrev, 'newrev', ref) + execute_service(project, user, oldrev: blankrev, newrev: 'newrev', ref: ref) expect(project.protected_branches).not_to be_empty expect(project.protected_branches.last.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::NO_ACCESS]) @@ -211,7 +211,7 @@ describe Git::BranchPushService, services: true do expect(project).to receive(:execute_hooks) expect(project.default_branch).to eq("master") - execute_service(project, user, blankrev, 'newrev', ref) + execute_service(project, user, oldrev: blankrev, newrev: 'newrev', ref: ref) expect(project.protected_branches).not_to be_empty expect(project.protected_branches.first.push_access_levels.map(&:access_level)).to eq([Gitlab::Access::MAINTAINER]) expect(project.protected_branches.first.merge_access_levels.map(&:access_level)).to eq([Gitlab::Access::DEVELOPER]) @@ -219,7 +219,7 @@ describe Git::BranchPushService, services: true do it "when pushing new commits to existing branch" do expect(project).to receive(:execute_hooks) - execute_service(project, user, 'oldrev', 'newrev', ref) + execute_service(project, user, oldrev: 'oldrev', newrev: 'newrev', ref: ref) end end end @@ -249,7 +249,7 @@ describe Git::BranchPushService, services: true do it "creates a note if a pushed commit mentions an issue" do expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end it "only creates a cross-reference note if one doesn't already exist" do @@ -257,7 +257,7 @@ describe Git::BranchPushService, services: true do expect(SystemNoteService).not_to receive(:cross_reference).with(issue, commit, commit_author) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end it "defaults to the pushing user if the commit's author is not known" do @@ -267,7 +267,7 @@ describe Git::BranchPushService, services: true do ) expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, user) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end it "finds references in the first push to a non-default branch" do @@ -276,7 +276,7 @@ describe Git::BranchPushService, services: true do expect(SystemNoteService).to receive(:cross_reference).with(issue, commit, commit_author) - execute_service(project, user, blankrev, newrev, 'refs/heads/other') + execute_service(project, user, oldrev: blankrev, newrev: newrev, ref: 'refs/heads/other') end end @@ -306,14 +306,14 @@ describe Git::BranchPushService, services: true do context "while saving the 'first_mentioned_in_commit_at' metric for an issue" do it 'sets the metric for referenced issues' do - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) expect(issue.reload.metrics.first_mentioned_in_commit_at).to be_like_time(commit_time) end it 'does not set the metric for non-referenced issues' do non_referenced_issue = create(:issue, project: project) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) expect(non_referenced_issue.reload.metrics.first_mentioned_in_commit_at).to be_nil end @@ -345,18 +345,18 @@ describe Git::BranchPushService, services: true do context "to default branches" do it "closes issues" do - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) expect(Issue.find(issue.id)).to be_closed end it "adds a note indicating that the issue is now closed" do expect(SystemNoteService).to receive(:change_status).with(issue, project, commit_author, "closed", closing_commit) - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) end it "doesn't create additional cross-reference notes" do expect(SystemNoteService).not_to receive(:cross_reference) - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) end end @@ -368,11 +368,11 @@ describe Git::BranchPushService, services: true do it "creates cross-reference notes" do expect(SystemNoteService).to receive(:cross_reference).with(issue, closing_commit, commit_author) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end it "doesn't close issues" do - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) expect(Issue.find(issue.id)).to be_opened end end @@ -408,7 +408,7 @@ describe Git::BranchPushService, services: true do let(:message) { "this is some work.\n\nrelated to JIRA-1" } it "initiates one api call to jira server to mention the issue" do - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( body: /mentioned this issue in/ @@ -436,13 +436,13 @@ describe Git::BranchPushService, services: true do context "using right markdown" do it "initiates one api call to jira server to close the issue" do - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) expect(WebMock).to have_requested(:post, jira_api_transition_url('JIRA-1')).once end it "initiates one api call to jira server to comment on the issue" do - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( body: comment_body @@ -459,13 +459,13 @@ describe Git::BranchPushService, services: true do let(:message) { "this is some work.\n\ncloses #1" } it "does not initiates one api call to jira server to close the issue" do - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) expect(WebMock).not_to have_requested(:post, jira_api_transition_url('JIRA-1')) end it "does not initiates one api call to jira server to comment on the issue" do - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) expect(WebMock).not_to have_requested(:post, jira_api_comment_url('JIRA-1')).with( body: comment_body @@ -478,13 +478,13 @@ describe Git::BranchPushService, services: true do let(:message) { "this is some work.\n\ncloses JIRA-1 \n\n closes #{issue.to_reference}" } it "initiates one api call to jira server to close the jira issue" do - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) expect(WebMock).to have_requested(:post, jira_api_transition_url('JIRA-1')).once end it "initiates one api call to jira server to comment on the jira issue" do - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) expect(WebMock).to have_requested(:post, jira_api_comment_url('JIRA-1')).with( body: comment_body @@ -492,14 +492,14 @@ describe Git::BranchPushService, services: true do end it "closes the internal issue" do - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) expect(issue.reload).to be_closed end it "adds a note indicating that the issue is now closed" do expect(SystemNoteService).to receive(:change_status) .with(issue, project, commit_author, "closed", closing_commit) - execute_service(project, commit_author, oldrev, newrev, ref) + execute_service(project, commit_author, oldrev: oldrev, newrev: newrev, ref: ref) end end end @@ -517,7 +517,7 @@ describe Git::BranchPushService, services: true do end it 'push to first branch updates HEAD' do - execute_service(project, user, blankrev, newrev, new_ref) + execute_service(project, user, oldrev: blankrev, newrev: newrev, ref: new_ref) end end @@ -542,7 +542,7 @@ describe Git::BranchPushService, services: true do it 'does not perform housekeeping when not needed' do expect(housekeeping).not_to receive(:execute) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end context 'when housekeeping is needed' do @@ -553,20 +553,20 @@ describe Git::BranchPushService, services: true do it 'performs housekeeping' do expect(housekeeping).to receive(:execute) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end it 'does not raise an exception' do allow(housekeeping).to receive(:try_obtain_lease).and_return(false) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end end it 'increments the push counter' do expect(housekeeping).to receive(:increment!) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end end @@ -577,7 +577,7 @@ describe Git::BranchPushService, services: true do it 'does nothing' do expect(::Ci::StopEnvironmentsService).not_to receive(:new) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end end @@ -585,7 +585,7 @@ describe Git::BranchPushService, services: true do it 'does nothing' do expect(::Ci::StopEnvironmentsService).not_to receive(:new) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end end @@ -599,7 +599,7 @@ describe Git::BranchPushService, services: true do expect(stop_service).to receive(:execute).with(branch) end - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end end end @@ -611,15 +611,17 @@ describe Git::BranchPushService, services: true do expect(hooks_service.project).to eq(project) expect(hooks_service.current_user).to eq(user) expect(hooks_service.params).to include( - oldrev: oldrev, - newrev: newrev, - ref: ref + change: { + oldrev: oldrev, + newrev: newrev, + ref: ref + } ) expect(hooks_service).to receive(:execute) end - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end end @@ -629,13 +631,13 @@ describe Git::BranchPushService, services: true do it 'does nothing' do expect(::Git::BranchHooksService).not_to receive(:new) - execute_service(project, user, oldrev, newrev, ref) + execute_service(project, user, oldrev: oldrev, newrev: newrev, ref: ref) end end end - def execute_service(project, user, oldrev, newrev, ref) - service = described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) + def execute_service(project, user, change) + service = described_class.new(project, user, change: change) service.execute service end diff --git a/spec/services/git/tag_hooks_service_spec.rb b/spec/services/git/tag_hooks_service_spec.rb index 1b8e0675f1b..c97d4d38b1c 100644 --- a/spec/services/git/tag_hooks_service_spec.rb +++ b/spec/services/git/tag_hooks_service_spec.rb @@ -15,7 +15,7 @@ describe Git::TagHooksService, :service do let(:commit) { tag.dereferenced_target } let(:service) do - described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) + described_class.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }) end describe 'System hooks' do diff --git a/spec/services/git/tag_push_service_spec.rb b/spec/services/git/tag_push_service_spec.rb index 7e008637182..9688041c08c 100644 --- a/spec/services/git/tag_push_service_spec.rb +++ b/spec/services/git/tag_push_service_spec.rb @@ -8,7 +8,7 @@ describe Git::TagPushService do let(:user) { create(:user) } let(:project) { create(:project, :repository) } - let(:service) { described_class.new(project, user, oldrev: oldrev, newrev: newrev, ref: ref) } + let(:service) { described_class.new(project, user, change: { oldrev: oldrev, newrev: newrev, ref: ref }) } let(:oldrev) { Gitlab::Git::BLANK_SHA } let(:newrev) { "8a2a6eb295bb170b34c24c76c49ed0e9b2eaf34b" } # gitlab-test: git rev-parse refs/tags/v1.1.0 diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 642a49d57d5..1f7d564b6ec 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -8,6 +8,7 @@ describe Issues::CloseService do let(:user2) { create(:user, email: "user2@example.com") } let(:guest) { create(:user) } let(:issue) { create(:issue, title: "My issue", project: project, assignees: [user2], author: create(:user)) } + let(:external_issue) { ExternalIssue.new('JIRA-123', project) } let(:closing_merge_request) { create(:merge_request, source_project: project) } let(:closing_commit) { create(:commit, project: project) } let!(:todo) { create(:todo, :assigned, user: user, project: project, target: issue, author: user2) } @@ -36,6 +37,16 @@ describe Issues::CloseService do expect(service.execute(issue)).to eq(issue) end + it 'closes the external issue even when the user is not authorized to do so' do + allow(service).to receive(:can?).with(user, :update_issue, external_issue) + .and_return(false) + + expect(service).to receive(:close_issue) + .with(external_issue, closed_via: nil, notifications: true, system_note: true) + + service.execute(external_issue) + end + it 'closes the issue when the user is authorized to do so' do allow(service).to receive(:can?).with(user, :update_issue, issue) .and_return(true) diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index ffc86f68469..fff6ddf3928 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -56,9 +56,11 @@ describe MergeRequests::PostMergeService do issue = create(:issue, project: project) allow(merge_request).to receive(:visible_closing_issues_for).and_return([issue]) - allow_any_instance_of(Issues::CloseService).to receive(:execute).with(issue, commit: merge_request).and_raise + expect_next_instance_of(Issues::CloseService) do |service| + allow(service).to receive(:execute).with(issue, commit: merge_request).and_raise(RuntimeError) + end - expect { described_class.new(project, user, {}).execute(merge_request) }.to raise_error + expect { described_class.new(project, user).execute(merge_request) }.to raise_error(RuntimeError) expect(merge_request.reload).to be_merged end diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index 575b2e779c5..b2817f9c14a 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -25,11 +25,15 @@ module CycleAnalyticsHelpers return if skip_push_handler - Git::BranchPushService.new(project, - user, - oldrev: oldrev, - newrev: commit_shas.last, - ref: 'refs/heads/master').execute + Git::BranchPushService.new( + project, + user, + change: { + oldrev: oldrev, + newrev: commit_shas.last, + ref: 'refs/heads/master' + } + ).execute end def create_cycle(user, project, issue, mr, milestone, pipeline) |