From 461d2672a495b7990dc578fa0bcba8bfdd781fd2 Mon Sep 17 00:00:00 2001 From: James Edwards-Jones Date: Tue, 14 Mar 2017 01:14:44 +0000 Subject: [PATCH 01/38] Set 'config.assets.compile = false' in production --- config/environments/production.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config/environments/production.rb b/config/environments/production.rb index a9d8ac4b6d4..82a19085b1d 100644 --- a/config/environments/production.rb +++ b/config/environments/production.rb @@ -16,7 +16,7 @@ Rails.application.configure do # config.assets.css_compressor = :sass # Don't fallback to assets pipeline if a precompiled asset is missed - config.assets.compile = true + config.assets.compile = false # Generate digests for assets URLs config.assets.digest = true From 739e797575d47ec796206865c4d82917cb2ad93d Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 2 May 2017 22:14:20 +0800 Subject: [PATCH 02/38] Add indices for auto_canceled_by_id for PostgreSQL --- ...dd-index-for-auto_canceled_by_id-mysql.yml | 4 ++++ ..._index_ci_pipelines_auto_canceled_by_id.rb | 21 +++++++++++++++++++ ...ate_index_ci_builds_auto_canceled_by_id.rb | 21 +++++++++++++++++++ db/schema.rb | 4 +++- 4 files changed, 49 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/add-index-for-auto_canceled_by_id-mysql.yml create mode 100644 db/migrate/20170502135553_create_index_ci_pipelines_auto_canceled_by_id.rb create mode 100644 db/migrate/20170502140503_create_index_ci_builds_auto_canceled_by_id.rb diff --git a/changelogs/unreleased/add-index-for-auto_canceled_by_id-mysql.yml b/changelogs/unreleased/add-index-for-auto_canceled_by_id-mysql.yml new file mode 100644 index 00000000000..eac78e9ee1f --- /dev/null +++ b/changelogs/unreleased/add-index-for-auto_canceled_by_id-mysql.yml @@ -0,0 +1,4 @@ +--- +title: Add indices for auto_canceled_by_id for ci_pipelines and ci_builds on PostgreSQL +merge_request: 11034 +author: diff --git a/db/migrate/20170502135553_create_index_ci_pipelines_auto_canceled_by_id.rb b/db/migrate/20170502135553_create_index_ci_pipelines_auto_canceled_by_id.rb new file mode 100644 index 00000000000..b64d7e0e3f6 --- /dev/null +++ b/db/migrate/20170502135553_create_index_ci_pipelines_auto_canceled_by_id.rb @@ -0,0 +1,21 @@ +class CreateIndexCiPipelinesAutoCanceledById < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + # MySQL would already have the index + unless index_exists?(:ci_pipelines, :auto_canceled_by_id) + add_concurrent_index(:ci_pipelines, :auto_canceled_by_id) + end + end + + def down + # We cannot remove index for MySQL because it's needed for foreign key + if Gitlab::Database.postgresql? + remove_concurrent_index(:ci_pipelines, :auto_canceled_by_id) + end + end +end diff --git a/db/migrate/20170502140503_create_index_ci_builds_auto_canceled_by_id.rb b/db/migrate/20170502140503_create_index_ci_builds_auto_canceled_by_id.rb new file mode 100644 index 00000000000..0a8d2c8ff61 --- /dev/null +++ b/db/migrate/20170502140503_create_index_ci_builds_auto_canceled_by_id.rb @@ -0,0 +1,21 @@ +class CreateIndexCiBuildsAutoCanceledById < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + # MySQL would already have the index + unless index_exists?(:ci_builds, :auto_canceled_by_id) + add_concurrent_index(:ci_builds, :auto_canceled_by_id) + end + end + + def down + # We cannot remove index for MySQL because it's needed for foreign key + if Gitlab::Database.postgresql? + remove_concurrent_index(:ci_builds, :auto_canceled_by_id) + end + end +end diff --git a/db/schema.rb b/db/schema.rb index b938657a186..d411a137b70 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170426181740) do +ActiveRecord::Schema.define(version: 20170502140503) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -232,6 +232,7 @@ ActiveRecord::Schema.define(version: 20170426181740) do t.integer "auto_canceled_by_id" end + add_index "ci_builds", ["auto_canceled_by_id"], name: "index_ci_builds_on_auto_canceled_by_id", using: :btree add_index "ci_builds", ["commit_id", "stage_idx", "created_at"], name: "index_ci_builds_on_commit_id_and_stage_idx_and_created_at", using: :btree add_index "ci_builds", ["commit_id", "status", "type"], name: "index_ci_builds_on_commit_id_and_status_and_type", using: :btree add_index "ci_builds", ["commit_id", "type", "name", "ref"], name: "index_ci_builds_on_commit_id_and_type_and_name_and_ref", using: :btree @@ -263,6 +264,7 @@ ActiveRecord::Schema.define(version: 20170426181740) do t.integer "auto_canceled_by_id" end + add_index "ci_pipelines", ["auto_canceled_by_id"], name: "index_ci_pipelines_on_auto_canceled_by_id", using: :btree add_index "ci_pipelines", ["project_id", "ref", "status"], name: "index_ci_pipelines_on_project_id_and_ref_and_status", using: :btree add_index "ci_pipelines", ["project_id", "sha"], name: "index_ci_pipelines_on_project_id_and_sha", using: :btree add_index "ci_pipelines", ["project_id"], name: "index_ci_pipelines_on_project_id", using: :btree From 6d46e51031e3d27044f692eba2fe332f26f59478 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 11 May 2017 16:48:59 -0300 Subject: [PATCH 03/38] Use gitaly_migrate helper on all current migrations in repository --- lib/gitlab/git/repository.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 256318cb833..639240b83cc 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -114,7 +114,7 @@ module Gitlab # Returns the number of valid branches def branch_count - Gitlab::GitalyClient.migrate(:branch_names) do |is_enabled| + gitaly_migrate(:branch_names) do |is_enabled| if is_enabled gitaly_ref_client.count_branch_names else @@ -133,7 +133,7 @@ module Gitlab # Returns the number of valid tags def tag_count - Gitlab::GitalyClient.migrate(:tag_names) do |is_enabled| + gitaly_migrate(:tag_names) do |is_enabled| if is_enabled gitaly_ref_client.count_tag_names else @@ -471,7 +471,7 @@ module Gitlab def ref_name_for_sha(ref_path, sha) # NOTE: This feature is intentionally disabled until # https://gitlab.com/gitlab-org/gitaly/issues/180 is resolved - # Gitlab::GitalyClient.migrate(:find_ref_name) do |is_enabled| + # gitaly_migrate(:find_ref_name) do |is_enabled| # if is_enabled # gitaly_ref_client.find_ref_name(sha, ref_path) # else From c5e4e676e3b1dc5aa0b71fd9c026c0e172cda808 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 11 May 2017 17:29:42 -0300 Subject: [PATCH 04/38] Fix token interpolation when setting the Github remote --- lib/github/import.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/github/import.rb b/lib/github/import.rb index 06beb607a3e..452aebc66cb 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -1,4 +1,5 @@ require_relative 'error' + module Github class Import include Gitlab::ShellAdapter @@ -79,7 +80,7 @@ module Github def fetch_repository begin project.create_repository unless project.repository.exists? - project.repository.add_remote('github', "https://{options.fetch(:token)}@github.com/#{repo}.git") + project.repository.add_remote('github', "https://#{options.fetch(:token)}@github.com/#{repo}.git") project.repository.set_remote_as_mirror('github') project.repository.fetch_remote('github', forced: true) rescue Gitlab::Shell::Error => e From e901d1209e77e2ff0d85bb14c4a8df6da135b333 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 11 May 2017 17:30:56 -0300 Subject: [PATCH 05/38] Reset create callbacks for Issues/MRs while importing GitHub projects --- lib/github/import.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/github/import.rb b/lib/github/import.rb index 452aebc66cb..9c7eb965f93 100644 --- a/lib/github/import.rb +++ b/lib/github/import.rb @@ -7,6 +7,7 @@ module Github class MergeRequest < ::MergeRequest self.table_name = 'merge_requests' + self.reset_callbacks :create self.reset_callbacks :save self.reset_callbacks :commit self.reset_callbacks :update @@ -17,6 +18,7 @@ module Github self.table_name = 'issues' self.reset_callbacks :save + self.reset_callbacks :create self.reset_callbacks :commit self.reset_callbacks :update self.reset_callbacks :validate From 851a8b1f739d045076767497db5f89d99034183e Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Thu, 11 May 2017 17:33:16 -0300 Subject: [PATCH 06/38] Add CHANGELOG --- changelogs/unreleased/fix-github-import.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/fix-github-import.yml diff --git a/changelogs/unreleased/fix-github-import.yml b/changelogs/unreleased/fix-github-import.yml new file mode 100644 index 00000000000..3a57152f7a8 --- /dev/null +++ b/changelogs/unreleased/fix-github-import.yml @@ -0,0 +1,4 @@ +--- +title: Fix token interpolation when setting the Github remote +merge_request: +author: From 6b2e73b994c86eeb3a9ac69a2ce0e7f0171275ff Mon Sep 17 00:00:00 2001 From: Ben Bodenmiller Date: Fri, 12 May 2017 09:50:14 +0000 Subject: [PATCH 07/38] Runners tab is now CI/CD Pipelines --- doc/ci/quick_start/README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/ci/quick_start/README.md b/doc/ci/quick_start/README.md index 30f209f80eb..41cae58782d 100644 --- a/doc/ci/quick_start/README.md +++ b/doc/ci/quick_start/README.md @@ -155,7 +155,7 @@ Find more information about different Runners in the [Runners](../runners/README.md) documentation. You can find whether any Runners are assigned to your project by going to -**Settings ➔ Runners**. Setting up a Runner is easy and straightforward. The +**Settings ➔ CI/CD Pipelines**. Setting up a Runner is easy and straightforward. The official Runner supported by GitLab is written in Go and its documentation can be found at . @@ -168,7 +168,7 @@ Follow the links above to set up your own Runner or use a Shared Runner as described in the next section. Once the Runner has been set up, you should see it on the Runners page of your -project, following **Settings ➔ Runners**. +project, following **Settings ➔ CI/CD Pipelines**. ![Activated runners](img/runners_activated.png) @@ -181,7 +181,7 @@ These are special virtual machines that run on GitLab's infrastructure and can build any project. To enable the **Shared Runners** you have to go to your project's -**Settings ➔ Runners** and click **Enable shared runners**. +**Settings ➔ CI/CD Pipelines** and click **Enable shared runners**. [Read more on Shared Runners](../runners/README.md). From 9923a630af7990c7fccc39f3e6584fb6b8bb2267 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Gray Date: Fri, 12 May 2017 09:58:22 -0500 Subject: [PATCH 08/38] Use fa-refresh on retried jobs --- app/assets/stylesheets/pages/builds.scss | 2 +- app/views/projects/builds/_sidebar.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/builds.scss b/app/assets/stylesheets/pages/builds.scss index 724b4080ee0..14a62b6cbf0 100644 --- a/app/assets/stylesheets/pages/builds.scss +++ b/app/assets/stylesheets/pages/builds.scss @@ -378,7 +378,7 @@ background-color: $row-hover; } - .fa-spinner { + .fa-refresh { font-size: 13px; margin-left: 3px; } diff --git a/app/views/projects/builds/_sidebar.html.haml b/app/views/projects/builds/_sidebar.html.haml index 43191fae9e6..26c892d0fd2 100644 --- a/app/views/projects/builds/_sidebar.html.haml +++ b/app/views/projects/builds/_sidebar.html.haml @@ -136,7 +136,7 @@ - else = build.id - if build.retried? - %i.fa.fa-spinner.has-tooltip{ data: { container: 'body', placement: 'bottom' }, title: 'Job was retried' } + %i.fa.fa-refresh.has-tooltip{ data: { container: 'body', placement: 'bottom' }, title: 'Job was retried' } :javascript new Sidebar(); From d567b6474ab0b2560878d76ae0c53a41ab8dba4f Mon Sep 17 00:00:00 2001 From: tauriedavis Date: Fri, 12 May 2017 10:08:50 -0700 Subject: [PATCH 09/38] 31886 Remove spinner from loading comment --- app/assets/javascripts/notes.js | 3 --- changelogs/unreleased/31886-remover-comment-load-spinner.yml | 4 ++++ 2 files changed, 4 insertions(+), 3 deletions(-) create mode 100644 changelogs/unreleased/31886-remover-comment-load-spinner.yml diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 7ba44835741..bce5379cbb9 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -1183,9 +1183,6 @@ const normalizeNewlines = function(str) { @${currentUsername} - - -
diff --git a/changelogs/unreleased/31886-remover-comment-load-spinner.yml b/changelogs/unreleased/31886-remover-comment-load-spinner.yml new file mode 100644 index 00000000000..4b36538064a --- /dev/null +++ b/changelogs/unreleased/31886-remover-comment-load-spinner.yml @@ -0,0 +1,4 @@ +--- +title: Remove spinner from loading comment +merge_request: +author: From 38d84f83600d890f4227fc510a484554f009a1c0 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 12 May 2017 14:31:05 -0500 Subject: [PATCH 10/38] Select dependency linker based on file type --- lib/gitlab/dependency_linker/base_linker.rb | 10 ++++++++-- lib/gitlab/dependency_linker/gemfile_linker.rb | 4 +--- lib/gitlab/file_detector.rb | 1 + 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/gitlab/dependency_linker/base_linker.rb b/lib/gitlab/dependency_linker/base_linker.rb index 40a4ad11372..5f4027e7e81 100644 --- a/lib/gitlab/dependency_linker/base_linker.rb +++ b/lib/gitlab/dependency_linker/base_linker.rb @@ -1,8 +1,14 @@ module Gitlab module DependencyLinker class BaseLinker - def self.link(plain_text, highlighted_text) - new(plain_text, highlighted_text).link + class_attribute :file_type + + def self.support?(blob_name) + Gitlab::FileDetector.type_of(blob_name) == file_type + end + + def self.link(*args) + new(*args).link end attr_accessor :plain_text, :highlighted_text diff --git a/lib/gitlab/dependency_linker/gemfile_linker.rb b/lib/gitlab/dependency_linker/gemfile_linker.rb index 45be760d89e..9b82e126528 100644 --- a/lib/gitlab/dependency_linker/gemfile_linker.rb +++ b/lib/gitlab/dependency_linker/gemfile_linker.rb @@ -1,9 +1,7 @@ module Gitlab module DependencyLinker class GemfileLinker < BaseLinker - def self.support?(blob_name) - blob_name == 'Gemfile' || blob_name == 'gems.rb' - end + self.file_type = :gemfile private diff --git a/lib/gitlab/file_detector.rb b/lib/gitlab/file_detector.rb index f8b3d0b4965..c6a89597b23 100644 --- a/lib/gitlab/file_detector.rb +++ b/lib/gitlab/file_detector.rb @@ -12,6 +12,7 @@ module Gitlab version: 'version', gitignore: '.gitignore', koding: '.koding.yml', + gemfile: /\A(Gemfile|gems\.rb)\z/, gitlab_ci: '.gitlab-ci.yml', avatar: /\Alogo\.(png|jpg|gif)\z/, route_map: 'route-map.yml' From 2ac9fcbd09c4a3efc1b8c3e943c078eefb0e479e Mon Sep 17 00:00:00 2001 From: Simon Knox Date: Wed, 19 Apr 2017 11:56:24 +1000 Subject: [PATCH 11/38] fix Resolved Discussions counter wrapping to next line change all .nav-links to use flex place Resolve Discussions above tabs on small screens, and to the right on large --- app/assets/stylesheets/framework/nav.scss | 22 +++--- app/assets/stylesheets/pages/issuable.scss | 10 --- .../stylesheets/pages/merge_requests.scss | 20 ++++++ app/assets/stylesheets/pages/notes.scss | 9 +++ .../projects/merge_requests/_show.html.haml | 70 ++++++++++--------- .../unreleased/31106-tabs-alignment.yml | 4 ++ ...e_for_discussions_in_merge_request_spec.rb | 2 +- 7 files changed, 82 insertions(+), 55 deletions(-) create mode 100644 changelogs/unreleased/31106-tabs-alignment.yml diff --git a/app/assets/stylesheets/framework/nav.scss b/app/assets/stylesheets/framework/nav.scss index b6cf5101d60..a30364314cf 100644 --- a/app/assets/stylesheets/framework/nav.scss +++ b/app/assets/stylesheets/framework/nav.scss @@ -24,10 +24,10 @@ } @mixin scrolling-links() { - white-space: nowrap; overflow-x: auto; overflow-y: hidden; -webkit-overflow-scrolling: touch; + display: flex; &::-webkit-scrollbar { display: none; @@ -35,6 +35,7 @@ } .nav-links { + display: flex; padding: 0; margin: 0; list-style: none; @@ -42,17 +43,16 @@ border-bottom: 1px solid $border-color; li { - display: inline-block; + display: flex; a { - display: inline-block; padding: $gl-btn-padding; padding-bottom: 11px; - margin-bottom: -1px; font-size: 14px; line-height: 28px; color: $gl-text-color-secondary; border-bottom: 2px solid transparent; + white-space: nowrap; &:hover, &:active, @@ -85,10 +85,10 @@ .container-fluid { background-color: $gray-normal; margin-bottom: 0; + display: flex; } li { - &.active a { border-bottom: none; color: $link-underline-blue; @@ -137,9 +137,9 @@ } .nav-links { - display: inline-block; margin-bottom: 0; border-bottom: none; + float: left; &.wide { width: 100%; @@ -336,6 +336,10 @@ border-bottom: none; height: 51px; + @media (min-width: $screen-sm-min) { + justify-content: center; + } + li { a { padding-top: 10px; @@ -346,6 +350,7 @@ .scrolling-tabs-container { position: relative; + overflow: hidden; .nav-links { @include scrolling-links(); @@ -483,10 +488,7 @@ .inner-page-scroll-tabs { position: relative; - - .nav-links { - padding-bottom: 1px; - } + overflow: hidden; .fade-right { @include fade(left, $white-light); diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index c4210ffd823..291f629c000 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -23,16 +23,6 @@ .merge-manually { @extend .fixed-width-container; } - - .merge-request-tabs-holder { - &.affix { - border-bottom: 1px solid $border-color; - - .nav-links { - border: 0; - } - } - } } .merge-request-details { diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 87592926930..5b9bf7c7a5a 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -696,6 +696,7 @@ top: 0; z-index: 10; background-color: $white-light; + border-bottom: 1px solid $border-color; @media(min-width: $screen-sm-min) { position: sticky; @@ -715,6 +716,16 @@ padding-right: $gl-padding; } } + + .nav-links { + border: 0; + } +} + +.merge-request-tabs { + display: flex; + margin-bottom: 0; + padding: 0; } .limit-container-width { @@ -725,6 +736,15 @@ } } +.merge-request-tabs-container { + display: flex; + justify-content: space-between; + + @media (max-width: $screen-xs-max) { + flex-direction: column-reverse; + } +} + .limit-container-width:not(.container-limited) { .merge-request-tabs-holder:not(.affix) { .merge-request-tabs-container { diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 6d7b7031c30..c613f9f9930 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -614,6 +614,15 @@ ul.notes { } .line-resolve-all-container { + @media (min-width: $screen-sm-min) { + margin-right: 0; + padding-left: $gl-padding; + } + + > div { + white-space: nowrap; + } + .btn-group { margin-left: -4px; } diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 25b8567b78f..b7515e1d91f 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -27,40 +27,42 @@ = render 'award_emoji/awards_block', awardable: @merge_request, inline: true .merge-request-tabs-holder{ class: ("js-tabs-affix" unless ENV['RAILS_ENV'] == 'test') } - .merge-request-tabs-container.scrolling-tabs-container.inner-page-scroll-tabs - .fade-left= icon('angle-left') - .fade-right= icon('angle-right') - %ul.merge-request-tabs.nav-links.scrolling-tabs - %li.notes-tab - = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do - Discussion - %span.badge= @merge_request.related_notes.user.count - - if @merge_request.source_project - %li.commits-tab - = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do - Commits - %span.badge= @commits_count - - if @pipelines.any? - %li.pipelines-tab - = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do - Pipelines - %span.badge= @pipelines.size - %li.diffs-tab - = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do - Changes - %span.badge= @merge_request.diff_size - %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true } - %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } - %div - .line-resolve-all{ "v-show" => "discussionCount > 0", - ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } - %span.line-resolve-btn.is-disabled{ type: "button", - ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } - = render "shared/icons/icon_status_success.svg" - %span.line-resolve-text - {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved - = render "discussions/new_issue_for_all_discussions", merge_request: @merge_request - = render "discussions/jump_to_next" + .merge-request-tabs-container + .scrolling-tabs-container.inner-page-scroll-tabs.is-smaller + .fade-left= icon('angle-left') + .fade-right= icon('angle-right') + .nav-links.scrolling-tabs + %ul.merge-request-tabs + %li.notes-tab + = link_to namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#notes', action: 'notes', toggle: 'tab' } do + Discussion + %span.badge= @merge_request.related_notes.user.count + - if @merge_request.source_project + %li.commits-tab + = link_to commits_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#commits', action: 'commits', toggle: 'tab' } do + Commits + %span.badge= @commits_count + - if @pipelines.any? + %li.pipelines-tab + = link_to pipelines_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: '#pipelines', action: 'pipelines', toggle: 'tab' } do + Pipelines + %span.badge= @pipelines.size + %li.diffs-tab + = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do + Changes + %span.badge= @merge_request.diff_size + #resolve-count-app.line-resolve-all-container.prepend-top-10{ "v-cloak" => true } + %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } + %div + .line-resolve-all{ "v-show" => "discussionCount > 0", + ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } + %span.line-resolve-btn.is-disabled{ type: "button", + ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } + = render "shared/icons/icon_status_success.svg" + %span.line-resolve-text + {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ resolvedCountText }} resolved + = render "discussions/new_issue_for_all_discussions", merge_request: @merge_request + = render "discussions/jump_to_next" .tab-content#diff-notes-app #notes.notes.tab-pane.voting_notes diff --git a/changelogs/unreleased/31106-tabs-alignment.yml b/changelogs/unreleased/31106-tabs-alignment.yml new file mode 100644 index 00000000000..53da08cc32d --- /dev/null +++ b/changelogs/unreleased/31106-tabs-alignment.yml @@ -0,0 +1,4 @@ +--- +title: prevent nav tabs from wrapping to new line +merge_request: +author: diff --git a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb index dc13cab2cd1..24e2419b5ce 100644 --- a/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb +++ b/spec/features/issues/create_issue_for_discussions_in_merge_request_spec.rb @@ -14,7 +14,7 @@ feature 'Resolving all open discussions in a merge request from an issue', featu end it 'shows a button to resolve all discussions by creating a new issue' do - within('li#resolve-count-app') do + within('#resolve-count-app') do expect(page).to have_link "Resolve all discussions in new issue", href: new_namespace_project_issue_path(project.namespace, project, merge_request_to_resolve_discussions_of: merge_request.iid) end end From 6bc48d0e25668a6cd6810178e68adab6fca58dae Mon Sep 17 00:00:00 2001 From: Adam Niedzielski Date: Sat, 13 May 2017 18:00:49 +0300 Subject: [PATCH 12/38] Allow GitLab instance to start when InfluxDB hostname cannot be resolved Closes #24438 --- changelogs/unreleased/adam-influxdb-hostname.yml | 4 ++++ lib/gitlab/metrics.rb | 3 +++ 2 files changed, 7 insertions(+) create mode 100644 changelogs/unreleased/adam-influxdb-hostname.yml diff --git a/changelogs/unreleased/adam-influxdb-hostname.yml b/changelogs/unreleased/adam-influxdb-hostname.yml new file mode 100644 index 00000000000..ab201ae7894 --- /dev/null +++ b/changelogs/unreleased/adam-influxdb-hostname.yml @@ -0,0 +1,4 @@ +--- +title: Allow GitLab instance to start when InfluxDB hostname cannot be resolved +merge_request: 11356 +author: diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb index c6dfa4ad9bd..cb8db2f1e9f 100644 --- a/lib/gitlab/metrics.rb +++ b/lib/gitlab/metrics.rb @@ -49,6 +49,9 @@ module Gitlab end end end + rescue Errno::EADDRNOTAVAIL, SocketError => ex + Gitlab::EnvironmentLogger.error('Cannot resolve InfluxDB address. GitLab Performance Monitoring will not work.') + Gitlab::EnvironmentLogger.error(ex) end def self.prepare_metrics(metrics) From 3efb60642a1803adbe8dccfc684ce1771765b750 Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Sat, 13 May 2017 21:19:49 +0100 Subject: [PATCH 13/38] Rename all references to rendered_title to realtime_changes --- app/controllers/projects/issues_controller.rb | 6 +++--- app/models/issue.rb | 2 +- app/views/projects/issues/show.html.haml | 2 +- config/routes/project.rb | 2 +- lib/gitlab/etag_caching/router.rb | 6 +++--- spec/javascripts/issue_show/components/app_spec.js | 2 +- spec/lib/gitlab/etag_caching/router_spec.rb | 2 +- 7 files changed, 11 insertions(+), 11 deletions(-) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 760ba246e3e..46438e68d54 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -11,10 +11,10 @@ class Projects::IssuesController < Projects::ApplicationController before_action :redirect_to_external_issue_tracker, only: [:index, :new] before_action :module_enabled before_action :issue, only: [:edit, :update, :show, :referenced_merge_requests, - :related_branches, :can_create_branch, :rendered_title, :create_merge_request] + :related_branches, :can_create_branch, :realtime_changes, :create_merge_request] # Allow read any issue - before_action :authorize_read_issue!, only: [:show, :rendered_title] + before_action :authorize_read_issue!, only: [:show, :realtime_changes] # Allow write(create) issue before_action :authorize_create_issue!, only: [:new, :create] @@ -199,7 +199,7 @@ class Projects::IssuesController < Projects::ApplicationController end end - def rendered_title + def realtime_changes Gitlab::PollingInterval.set_header(response, interval: 3_000) render json: { diff --git a/app/models/issue.rb b/app/models/issue.rb index ecfc33ec1a1..a88dbb3e065 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -292,7 +292,7 @@ class Issue < ActiveRecord::Base end def expire_etag_cache - key = Gitlab::Routing.url_helpers.rendered_title_namespace_project_issue_path( + key = Gitlab::Routing.url_helpers.realtime_changes_namespace_project_issue_path( project.namespace, project, self diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml index f66724900de..b2401442620 100644 --- a/app/views/projects/issues/show.html.haml +++ b/app/views/projects/issues/show.html.haml @@ -51,7 +51,7 @@ .issue-details.issuable-details .detail-page-description.content-block - #js-issuable-app{ "data" => { "endpoint" => rendered_title_namespace_project_issue_path(@project.namespace, @project, @issue), + #js-issuable-app{ "data" => { "endpoint" => realtime_changes_namespace_project_issue_path(@project.namespace, @project, @issue), "can-update" => can?(current_user, :update_issue, @issue).to_s, "issuable-ref" => @issue.to_reference, } } diff --git a/config/routes/project.rb b/config/routes/project.rb index a6c104c2d3f..c786cbdee1e 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -244,7 +244,7 @@ constraints(ProjectUrlConstrainer.new) do get :referenced_merge_requests get :related_branches get :can_create_branch - get :rendered_title + get :realtime_changes post :create_merge_request end collection do diff --git a/lib/gitlab/etag_caching/router.rb b/lib/gitlab/etag_caching/router.rb index 31a5b9d108b..ba31041d0c1 100644 --- a/lib/gitlab/etag_caching/router.rb +++ b/lib/gitlab/etag_caching/router.rb @@ -7,8 +7,8 @@ module Gitlab # - Don't contain a reserved word (expect for the words used in the # regex itself) # - Ending in `noteable/issue//notes` for the `issue_notes` route - # - Ending in `issues/id`/rendered_title` for the `issue_title` route - USED_IN_ROUTES = %w[noteable issue notes issues rendered_title + # - Ending in `issues/id`/realtime_changes` for the `issue_title` route + USED_IN_ROUTES = %w[noteable issue notes issues realtime_changes commit pipelines merge_requests new].freeze RESERVED_WORDS = DynamicPathValidator::WILDCARD_ROUTES - USED_IN_ROUTES RESERVED_WORDS_REGEX = Regexp.union(*RESERVED_WORDS) @@ -18,7 +18,7 @@ module Gitlab 'issue_notes' ), Gitlab::EtagCaching::Router::Route.new( - %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/issues/\d+/rendered_title\z), + %r(^(?!.*(#{RESERVED_WORDS_REGEX})).*/issues/\d+/realtime_changes\z), 'issue_title' ), Gitlab::EtagCaching::Router::Route.new( diff --git a/spec/javascripts/issue_show/components/app_spec.js b/spec/javascripts/issue_show/components/app_spec.js index 09bca2c3680..ee456869c53 100644 --- a/spec/javascripts/issue_show/components/app_spec.js +++ b/spec/javascripts/issue_show/components/app_spec.js @@ -25,7 +25,7 @@ describe('Issuable output', () => { vm = new IssuableDescriptionComponent({ propsData: { canUpdate: true, - endpoint: '/gitlab-org/gitlab-shell/issues/9/rendered_title', + endpoint: '/gitlab-org/gitlab-shell/issues/9/realtime_changes', issuableRef: '#1', initialTitle: '', initialDescriptionHtml: '', diff --git a/spec/lib/gitlab/etag_caching/router_spec.rb b/spec/lib/gitlab/etag_caching/router_spec.rb index f3dacb4ef04..5ae4a19263c 100644 --- a/spec/lib/gitlab/etag_caching/router_spec.rb +++ b/spec/lib/gitlab/etag_caching/router_spec.rb @@ -14,7 +14,7 @@ describe Gitlab::EtagCaching::Router do it 'matches issue title endpoint' do env = build_env( - '/my-group/my-project/issues/123/rendered_title' + '/my-group/my-project/issues/123/realtime_changes' ) result = described_class.match(env) From 787b314e38a2002f5e7a2aeb9596530270c0e76a Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 15 May 2017 09:42:30 +0300 Subject: [PATCH 14/38] Ruby files that are not meant to be executable should be 644 not 755 Signed-off-by: Dmitriy Zaporozhets --- app/controllers/projects/merge_requests_controller.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) mode change 100755 => 100644 app/controllers/projects/merge_requests_controller.rb diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb old mode 100755 new mode 100644 From 45f66c8021704276cb5ddf8831606ced71e955e9 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 2 May 2017 07:50:52 +0000 Subject: [PATCH 15/38] Make auto-cancelling pending pipelines on by default --- .../enable-auto-cancelling-by-default.yml | 4 ++++ ..._auto_cancel_pending_pipelines_on_by_default.rb | 13 +++++++++++++ ...enable_auto_cancel_pending_pipelines_for_all.rb | 14 ++++++++++++++ db/schema.rb | 2 +- 4 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/enable-auto-cancelling-by-default.yml create mode 100644 db/migrate/20170502065653_make_auto_cancel_pending_pipelines_on_by_default.rb create mode 100644 db/migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb diff --git a/changelogs/unreleased/enable-auto-cancelling-by-default.yml b/changelogs/unreleased/enable-auto-cancelling-by-default.yml new file mode 100644 index 00000000000..8b1659bf38b --- /dev/null +++ b/changelogs/unreleased/enable-auto-cancelling-by-default.yml @@ -0,0 +1,4 @@ +--- +title: Enable cancelling non-HEAD pending pipelines by default for all projects +merge_request: 11023 +author: diff --git a/db/migrate/20170502065653_make_auto_cancel_pending_pipelines_on_by_default.rb b/db/migrate/20170502065653_make_auto_cancel_pending_pipelines_on_by_default.rb new file mode 100644 index 00000000000..03bf626a08a --- /dev/null +++ b/db/migrate/20170502065653_make_auto_cancel_pending_pipelines_on_by_default.rb @@ -0,0 +1,13 @@ +class MakeAutoCancelPendingPipelinesOnByDefault < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + change_column_default(:projects, :auto_cancel_pending_pipelines, 1) + end + + def down + change_column_default(:projects, :auto_cancel_pending_pipelines, 0) + end +end diff --git a/db/migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb b/db/migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb new file mode 100644 index 00000000000..bc43cd34c4d --- /dev/null +++ b/db/migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb @@ -0,0 +1,14 @@ +class EnableAutoCancelPendingPipelinesForAll < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + connection.execute( + 'UPDATE projects SET auto_cancel_pending_pipelines = 1') + end + + def down + # Nothing we can do! + end +end diff --git a/db/schema.rb b/db/schema.rb index 65eaccf766a..e164347a054 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -986,7 +986,7 @@ ActiveRecord::Schema.define(version: 20170508190732) do t.text "description_html" t.boolean "only_allow_merge_if_all_discussions_are_resolved" t.boolean "printing_merge_request_link_enabled", default: true, null: false - t.integer "auto_cancel_pending_pipelines", default: 0, null: false + t.integer "auto_cancel_pending_pipelines", default: 1, null: false t.string "import_jid" t.integer "cached_markdown_version" t.datetime "last_repository_updated_at" From dbd034b7a20c5b8f25df253d9be82c986346dc5a Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Tue, 2 May 2017 21:27:24 +0800 Subject: [PATCH 16/38] Disable auto_cancel_pending_pipelines in PostReceive test --- spec/workers/post_receive_spec.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/spec/workers/post_receive_spec.rb b/spec/workers/post_receive_spec.rb index 3289c2df1fb..f4bc63bcc6a 100644 --- a/spec/workers/post_receive_spec.rb +++ b/spec/workers/post_receive_spec.rb @@ -4,13 +4,16 @@ describe PostReceive do let(:changes) { "123456 789012 refs/heads/tést\n654321 210987 refs/tags/tag" } let(:wrongly_encoded_changes) { changes.encode("ISO-8859-1").force_encoding("UTF-8") } let(:base64_changes) { Base64.encode64(wrongly_encoded_changes) } - let(:project) { create(:project, :repository) } let(:project_identifier) { "project-#{project.id}" } let(:key) { create(:key, user: project.owner) } let(:key_id) { key.shell_id } + let(:project) do + create(:project, :repository, auto_cancel_pending_pipelines: 'disabled') + end + context "as a sidekiq worker" do - it "reponds to #perform" do + it "responds to #perform" do expect(described_class.new).to respond_to(:perform) end end From b5bffbd4adc5005b8c9f7d82d68a01743fbf2f28 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 15 May 2017 14:27:30 +0800 Subject: [PATCH 17/38] Move to post_migrate and use update_column_in_batches --- ...70502070007_enable_auto_cancel_pending_pipelines_for_all.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) rename db/{migrate => post_migrate}/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb (69%) diff --git a/db/migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb b/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb similarity index 69% rename from db/migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb rename to db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb index bc43cd34c4d..0abc75a3ed6 100644 --- a/db/migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb +++ b/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb @@ -4,8 +4,7 @@ class EnableAutoCancelPendingPipelinesForAll < ActiveRecord::Migration DOWNTIME = false def up - connection.execute( - 'UPDATE projects SET auto_cancel_pending_pipelines = 1') + update_column_in_batches(:projects, :auto_cancel_pending_pipelines, 1) end def down From e386d28b89f800f9ab7f0afa073b9891f7c0d977 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 15 May 2017 16:20:26 +0800 Subject: [PATCH 18/38] Add missing newline in schema --- db/schema.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/schema.rb b/db/schema.rb index d76432fa1e7..4aaf3642173 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1450,4 +1450,4 @@ ActiveRecord::Schema.define(version: 20170508190732) do add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade add_foreign_key "trending_projects", "projects", on_delete: :cascade add_foreign_key "u2f_registrations", "users" -end \ No newline at end of file +end From 20ece0aac37ab38c6207bd3d0a4c6276275fb049 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Mon, 15 May 2017 16:22:29 +0800 Subject: [PATCH 19/38] Disable transaction for updating the table --- ...170502070007_enable_auto_cancel_pending_pipelines_for_all.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb b/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb index 0abc75a3ed6..a19b73fc114 100644 --- a/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb +++ b/db/post_migrate/20170502070007_enable_auto_cancel_pending_pipelines_for_all.rb @@ -1,6 +1,8 @@ class EnableAutoCancelPendingPipelinesForAll < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers + disable_ddl_transaction! + DOWNTIME = false def up From 43f037c903605b55ca1c34a24ab1ea1a522816fb Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 10 May 2017 14:18:59 +0200 Subject: [PATCH 20/38] Don't reuse gRPC channels It seems that bad things happen when two gRPC stubs share one gRPC channel so let's stop doing that. The downside of this is that we create more gRPC connections; one per stub. --- app/models/repository.rb | 2 - config/initializers/8_gitaly.rb | 6 ++- lib/gitlab/git/repository.rb | 14 +++-- lib/gitlab/gitaly_client.rb | 66 +++++++++-------------- lib/gitlab/gitaly_client/commit.rb | 4 +- lib/gitlab/gitaly_client/notifications.rb | 2 +- lib/gitlab/gitaly_client/ref.rb | 2 +- lib/gitlab/workhorse.rb | 2 +- spec/lib/gitlab/gitaly_client_spec.rb | 21 +++++--- spec/lib/gitlab/workhorse_spec.rb | 2 +- spec/support/test_env.rb | 2 +- spec/tasks/gitlab/backup_rake_spec.rb | 1 - 12 files changed, 58 insertions(+), 66 deletions(-) diff --git a/app/models/repository.rb b/app/models/repository.rb index b1563bfba8b..9153e5ae5a8 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1163,8 +1163,6 @@ class Repository @project.repository_storage_path end - delegate :gitaly_channel, :gitaly_repository, to: :raw_repository - def initialize_raw_repository Gitlab::Git::Repository.new(project.repository_storage, path_with_namespace + '.git') end diff --git a/config/initializers/8_gitaly.rb b/config/initializers/8_gitaly.rb index 42ec7240b0f..31c7c91d78f 100644 --- a/config/initializers/8_gitaly.rb +++ b/config/initializers/8_gitaly.rb @@ -1,6 +1,8 @@ require 'uri' -# Make sure we initialize our Gitaly channels before Sidekiq starts multi-threaded execution. if Gitlab.config.gitaly.enabled || Rails.env.test? - Gitlab::GitalyClient.configure_channels + Gitlab.config.repositories.storages.keys.each do |storage| + # Force validation of each address + Gitlab::GitalyClient.address(storage) + end end diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 256318cb833..7a051444603 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -27,13 +27,15 @@ module Gitlab # Rugged repo object attr_reader :rugged + attr_reader :storage + # 'path' must be the path to a _bare_ git repository, e.g. # /path/to/my-repo.git - def initialize(repository_storage, relative_path) - @repository_storage = repository_storage + def initialize(storage, relative_path) + @storage = storage @relative_path = relative_path - storage_path = Gitlab.config.repositories.storages[@repository_storage]['path'] + storage_path = Gitlab.config.repositories.storages[@storage]['path'] @path = File.join(storage_path, @relative_path) @name = @relative_path.split("/").last @attributes = Gitlab::Git::Attributes.new(path) @@ -965,11 +967,7 @@ module Gitlab end def gitaly_repository - Gitlab::GitalyClient::Util.repository(@repository_storage, @relative_path) - end - - def gitaly_channel - Gitlab::GitalyClient.get_channel(@repository_storage) + Gitlab::GitalyClient::Util.repository(@storage, @relative_path) end private diff --git a/lib/gitlab/gitaly_client.rb b/lib/gitlab/gitaly_client.rb index c69676a1dac..72466700c05 100644 --- a/lib/gitlab/gitaly_client.rb +++ b/lib/gitlab/gitaly_client.rb @@ -4,56 +4,42 @@ module Gitlab module GitalyClient SERVER_VERSION_FILE = 'GITALY_SERVER_VERSION'.freeze - # This function is not thread-safe because it updates Hashes in instance variables. - def self.configure_channels - @addresses = {} - @channels = {} - Gitlab.config.repositories.storages.each do |name, params| - address = params['gitaly_address'] - unless address.present? - raise "storage #{name.inspect} is missing a gitaly_address" + MUTEX = Mutex.new + private_constant :MUTEX + + def self.stub(name, storage) + MUTEX.synchronize do + @stubs ||= {} + @stubs[storage] ||= {} + @stubs[storage][name] ||= begin + klass = Gitaly.const_get(name.to_s.camelcase.to_sym).const_get(:Stub) + addr = address(storage) + addr = addr.sub(%r{^tcp://}, '') if URI(addr).scheme == 'tcp' + klass.new(addr, :this_channel_is_insecure) end - - unless URI(address).scheme.in?(%w(tcp unix)) - raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix'" - end - - @addresses[name] = address - @channels[name] = new_channel(address) end end - def self.new_channel(address) - address = address.sub(%r{^tcp://}, '') if URI(address).scheme == 'tcp' - # NOTE: When Gitaly runs on a Unix socket, permissions are - # handled using the file system and no additional authentication is - # required (therefore the :this_channel_is_insecure flag) - # TODO: Add authentication support when Gitaly is running on a TCP socket. - GRPC::Core::Channel.new(address, {}, :this_channel_is_insecure) + def self.clear_stubs! + MUTEX.synchronize do + @stubs = nil + end end - def self.get_channel(storage) - if !Rails.env.production? && @channels.nil? - # In development mode the Rails auto-loader may reset the instance - # variables of this class. What we do here is not thread-safe. In normal - # circumstances (including production) these instance variables have - # been initialized from config/initializers. - configure_channels + def self.address(storage) + params = Gitlab.config.repositories.storages[storage] + raise "storage not found: #{storage.inspect}" if params.nil? + + address = params['gitaly_address'] + unless address.present? + raise "storage #{storage.inspect} is missing a gitaly_address" end - @channels[storage] - end - - def self.get_address(storage) - if !Rails.env.production? && @addresses.nil? - # In development mode the Rails auto-loader may reset the instance - # variables of this class. What we do here is not thread-safe. In normal - # circumstances (including development) these instance variables have - # been initialized from config/initializers. - configure_channels + unless URI(address).scheme.in?(%w(tcp unix)) + raise "Unsupported Gitaly address: #{address.inspect} does not use URL scheme 'tcp' or 'unix'" end - @addresses[storage] + address end def self.enabled? diff --git a/lib/gitlab/gitaly_client/commit.rb b/lib/gitlab/gitaly_client/commit.rb index 15c57420fb2..4491903d788 100644 --- a/lib/gitlab/gitaly_client/commit.rb +++ b/lib/gitlab/gitaly_client/commit.rb @@ -11,7 +11,7 @@ module Gitlab end def is_ancestor(ancestor_id, child_id) - stub = Gitaly::Commit::Stub.new(nil, nil, channel_override: @repository.gitaly_channel) + stub = GitalyClient.stub(:commit, @repository.storage) request = Gitaly::CommitIsAncestorRequest.new( repository: @gitaly_repo, ancestor_id: ancestor_id, @@ -52,7 +52,7 @@ module Gitlab end def diff_service_stub - Gitaly::Diff::Stub.new(nil, nil, channel_override: @repository.gitaly_channel) + GitalyClient.stub(:diff, @repository.storage) end end end diff --git a/lib/gitlab/gitaly_client/notifications.rb b/lib/gitlab/gitaly_client/notifications.rb index a94a54883db..719554eac52 100644 --- a/lib/gitlab/gitaly_client/notifications.rb +++ b/lib/gitlab/gitaly_client/notifications.rb @@ -6,7 +6,7 @@ module Gitlab # 'repository' is a Gitlab::Git::Repository def initialize(repository) @gitaly_repo = repository.gitaly_repository - @stub = Gitaly::Notifications::Stub.new(nil, nil, channel_override: repository.gitaly_channel) + @stub = GitalyClient.stub(:notifications, repository.storage) end def post_receive diff --git a/lib/gitlab/gitaly_client/ref.rb b/lib/gitlab/gitaly_client/ref.rb index f6c77ef1a3e..bf04e1fa50b 100644 --- a/lib/gitlab/gitaly_client/ref.rb +++ b/lib/gitlab/gitaly_client/ref.rb @@ -6,7 +6,7 @@ module Gitlab # 'repository' is a Gitlab::Git::Repository def initialize(repository) @gitaly_repo = repository.gitaly_repository - @stub = Gitaly::Ref::Stub.new(nil, nil, channel_override: repository.gitaly_channel) + @stub = GitalyClient.stub(:ref, repository.storage) end def default_branch_name diff --git a/lib/gitlab/workhorse.rb b/lib/gitlab/workhorse.rb index 351e2b10595..fe37e4da94f 100644 --- a/lib/gitlab/workhorse.rb +++ b/lib/gitlab/workhorse.rb @@ -26,7 +26,7 @@ module Gitlab } if Gitlab.config.gitaly.enabled - address = Gitlab::GitalyClient.get_address(project.repository_storage) + address = Gitlab::GitalyClient.address(project.repository_storage) params[:Repository] = repository.gitaly_repository.to_h feature_enabled = case action.to_s diff --git a/spec/lib/gitlab/gitaly_client_spec.rb b/spec/lib/gitlab/gitaly_client_spec.rb index 55fcf91fb6e..08ee0dff6b2 100644 --- a/spec/lib/gitlab/gitaly_client_spec.rb +++ b/spec/lib/gitlab/gitaly_client_spec.rb @@ -1,14 +1,19 @@ require 'spec_helper' describe Gitlab::GitalyClient, lib: true do - describe '.new_channel' do + describe '.stub' do + before { described_class.clear_stubs! } + context 'when passed a UNIX socket address' do - it 'passes the address as-is to GRPC::Core::Channel initializer' do + it 'passes the address as-is to GRPC' do address = 'unix:/tmp/gitaly.sock' + allow(Gitlab.config.repositories).to receive(:storages).and_return({ + 'default' => { 'gitaly_address' => address } + }) - expect(GRPC::Core::Channel).to receive(:new).with(address, any_args) + expect(Gitaly::Commit::Stub).to receive(:new).with(address, any_args) - described_class.new_channel(address) + described_class.stub(:commit, 'default') end end @@ -17,9 +22,13 @@ describe Gitlab::GitalyClient, lib: true do address = 'localhost:9876' prefixed_address = "tcp://#{address}" - expect(GRPC::Core::Channel).to receive(:new).with(address, any_args) + allow(Gitlab.config.repositories).to receive(:storages).and_return({ + 'default' => { 'gitaly_address' => prefixed_address } + }) - described_class.new_channel(prefixed_address) + expect(Gitaly::Commit::Stub).to receive(:new).with(address, any_args) + + described_class.stub(:commit, 'default') end end end diff --git a/spec/lib/gitlab/workhorse_spec.rb b/spec/lib/gitlab/workhorse_spec.rb index 67b759f7dcd..fdbb55fc874 100644 --- a/spec/lib/gitlab/workhorse_spec.rb +++ b/spec/lib/gitlab/workhorse_spec.rb @@ -202,7 +202,7 @@ describe Gitlab::Workhorse, lib: true do context 'when Gitaly is enabled' do let(:gitaly_params) do { - GitalyAddress: Gitlab::GitalyClient.get_address('default') + GitalyAddress: Gitlab::GitalyClient.address('default') } end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 8e31c26591b..f8ad0ccdb41 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -120,7 +120,7 @@ module TestEnv end def setup_gitaly - socket_path = Gitlab::GitalyClient.get_address('default').sub(/\Aunix:/, '') + socket_path = Gitlab::GitalyClient.address('default').sub(/\Aunix:/, '') gitaly_dir = File.dirname(socket_path) unless File.directory?(gitaly_dir) || system('rake', "gitlab:gitaly:install[#{gitaly_dir}]") diff --git a/spec/tasks/gitlab/backup_rake_spec.rb b/spec/tasks/gitlab/backup_rake_spec.rb index 4def113dd77..0ff1a988a9e 100644 --- a/spec/tasks/gitlab/backup_rake_spec.rb +++ b/spec/tasks/gitlab/backup_rake_spec.rb @@ -236,7 +236,6 @@ describe 'gitlab:app namespace rake task' do 'custom' => { 'path' => Settings.absolute('tmp/tests/custom_storage'), 'gitaly_address' => gitaly_address } } allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) - Gitlab::GitalyClient.configure_channels # Create the projects now, after mocking the settings but before doing the backup project_a From 60106c1e1ecdb0bb8ed37e1137c846f1a415dabf Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Wed, 10 May 2017 15:06:56 +0200 Subject: [PATCH 21/38] Log gitaly output during testing --- spec/support/test_env.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index f8ad0ccdb41..9bf9dc5d4b2 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -133,7 +133,8 @@ module TestEnv def start_gitaly(gitaly_dir) gitaly_exec = File.join(gitaly_dir, 'gitaly') gitaly_config = File.join(gitaly_dir, 'config.toml') - @gitaly_pid = spawn(gitaly_exec, gitaly_config, [:out, :err] => '/dev/null') + log_file = Rails.root.join('log/gitaly-test.log').to_s + @gitaly_pid = spawn(gitaly_exec, gitaly_config, [:out, :err] => log_file) end def stop_gitaly From 6c6d2bf047f8694e8b58db0feea6a3f320c372de Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 11 May 2017 11:33:15 +0200 Subject: [PATCH 22/38] Rename `build_events` columns to `job_events` --- ...ame_web_hooks_build_events_to_job_events.rb | 18 ++++++++++++++++++ ...name_services_build_events_to_job_events.rb | 18 ++++++++++++++++++ ...ame_web_hooks_build_events_to_job_events.rb | 18 ++++++++++++++++++ ...name_services_build_events_to_job_events.rb | 18 ++++++++++++++++++ db/schema.rb | 6 +++--- 5 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20170511082759_rename_web_hooks_build_events_to_job_events.rb create mode 100644 db/migrate/20170511083824_rename_services_build_events_to_job_events.rb create mode 100644 db/post_migrate/20170511100900_cleanup_rename_web_hooks_build_events_to_job_events.rb create mode 100644 db/post_migrate/20170511101000_cleanup_rename_services_build_events_to_job_events.rb diff --git a/db/migrate/20170511082759_rename_web_hooks_build_events_to_job_events.rb b/db/migrate/20170511082759_rename_web_hooks_build_events_to_job_events.rb new file mode 100644 index 00000000000..a2320a911b7 --- /dev/null +++ b/db/migrate/20170511082759_rename_web_hooks_build_events_to_job_events.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RenameWebHooksBuildEventsToJobEvents < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column_concurrently :web_hooks, :build_events, :job_events + end + + def down + cleanup_concurrent_column_rename :web_hooks, :job_events, :build_events + end +end diff --git a/db/migrate/20170511083824_rename_services_build_events_to_job_events.rb b/db/migrate/20170511083824_rename_services_build_events_to_job_events.rb new file mode 100644 index 00000000000..303d47078e7 --- /dev/null +++ b/db/migrate/20170511083824_rename_services_build_events_to_job_events.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RenameServicesBuildEventsToJobEvents < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + rename_column_concurrently :services, :build_events, :job_events + end + + def down + cleanup_concurrent_column_rename :services, :job_events, :build_events + end +end diff --git a/db/post_migrate/20170511100900_cleanup_rename_web_hooks_build_events_to_job_events.rb b/db/post_migrate/20170511100900_cleanup_rename_web_hooks_build_events_to_job_events.rb new file mode 100644 index 00000000000..281be90163a --- /dev/null +++ b/db/post_migrate/20170511100900_cleanup_rename_web_hooks_build_events_to_job_events.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CleanupRenameWebHooksBuildEventsToJobEvents < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :web_hooks, :build_events, :job_events + end + + def down + rename_column_concurrently :web_hooks, :job_events, :build_events + end +end diff --git a/db/post_migrate/20170511101000_cleanup_rename_services_build_events_to_job_events.rb b/db/post_migrate/20170511101000_cleanup_rename_services_build_events_to_job_events.rb new file mode 100644 index 00000000000..5d26df5688f --- /dev/null +++ b/db/post_migrate/20170511101000_cleanup_rename_services_build_events_to_job_events.rb @@ -0,0 +1,18 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class CleanupRenameServicesBuildEventsToJobEvents < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + def up + cleanup_concurrent_column_rename :services, :build_events, :job_events + end + + def down + rename_column_concurrently :services, :job_events, :build_events + end +end diff --git a/db/schema.rb b/db/schema.rb index 65eaccf766a..ca364064d4c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170508190732) do +ActiveRecord::Schema.define(version: 20170511101000) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1123,13 +1123,13 @@ ActiveRecord::Schema.define(version: 20170508190732) do t.boolean "merge_requests_events", default: true t.boolean "tag_push_events", default: true t.boolean "note_events", default: true, null: false - t.boolean "build_events", default: false, null: false t.string "category", default: "common", null: false t.boolean "default", default: false t.boolean "wiki_page_events", default: true t.boolean "pipeline_events", default: false, null: false t.boolean "confidential_issues_events", default: true, null: false t.boolean "commit_events", default: true, null: false + t.boolean "job_events", default: false, null: false end add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree @@ -1399,11 +1399,11 @@ ActiveRecord::Schema.define(version: 20170508190732) do t.boolean "tag_push_events", default: false t.boolean "note_events", default: false, null: false t.boolean "enable_ssl_verification", default: true - t.boolean "build_events", default: false, null: false t.boolean "wiki_page_events", default: false, null: false t.string "token" t.boolean "pipeline_events", default: false, null: false t.boolean "confidential_issues_events", default: false, null: false + t.boolean "job_events", default: false, null: false t.boolean "repository_update_events", default: false, null: false end From da9d8d3813afd94ea29c2e18c385107ef8ec7e45 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 15 May 2017 12:50:55 +0300 Subject: [PATCH 23/38] Minor spec improvement --- spec/features/auto_deploy_spec.rb | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/spec/features/auto_deploy_spec.rb b/spec/features/auto_deploy_spec.rb index eba1bca83a8..6c7423e4922 100644 --- a/spec/features/auto_deploy_spec.rb +++ b/spec/features/auto_deploy_spec.rb @@ -5,14 +5,7 @@ describe 'Auto deploy' do let(:project) { create(:project, :repository) } before do - project.create_kubernetes_service( - active: true, - properties: { - namespace: project.path, - api_url: 'https://kubernetes.example.com', - token: 'a' * 40 - } - ) + create :kubernetes_service, project: project project.team << [user, :master] login_as user end From cac7e03fa1cb39a685bcde87d4d67fb17c4f48f2 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 11 May 2017 11:41:36 +0200 Subject: [PATCH 24/38] Rename `build_events` to `job_events` in code --- app/models/ci/build.rb | 4 +-- app/models/hooks/project_hook.rb | 2 +- app/models/hooks/web_hook.rb | 2 +- app/models/service.rb | 4 +-- app/views/admin/hooks/index.html.haml | 2 +- .../integrations/_project_hook.html.haml | 2 +- app/views/shared/web_hooks/_form.html.haml | 6 ++-- lib/api/entities.rb | 4 +-- lib/api/project_hooks.rb | 2 -- lib/api/v3/entities.rb | 6 ++-- spec/factories/project_hooks.rb | 2 +- .../settings/integration_settings_spec.rb | 2 ++ spec/lib/gitlab/import_export/project.json | 36 +++++++++---------- .../import_export/relation_factory_spec.rb | 2 +- .../import_export/safe_model_attributes.yml | 4 +-- spec/requests/api/project_hooks_spec.rb | 4 +-- spec/requests/api/v3/project_hooks_spec.rb | 4 +-- 17 files changed, 45 insertions(+), 43 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 3c4a4d93349..760ec8e5919 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -300,8 +300,8 @@ module Ci def execute_hooks return unless project build_data = Gitlab::DataBuilder::Build.build(self) - project.execute_hooks(build_data.dup, :build_hooks) - project.execute_services(build_data.dup, :build_hooks) + project.execute_hooks(build_data.dup, :job_hooks) + project.execute_services(build_data.dup, :job_hooks) PagesService.new(build_data).execute project.running_or_pending_build_count(force: true) end diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index c631e7a7df5..ee6165fd32d 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -5,7 +5,7 @@ class ProjectHook < WebHook scope :confidential_issue_hooks, -> { where(confidential_issues_events: true) } scope :note_hooks, -> { where(note_events: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true) } - scope :build_hooks, -> { where(build_events: true) } + scope :job_hooks, -> { where(job_events: true) } scope :pipeline_hooks, -> { where(pipeline_events: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true) } end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index 7cf03aabd6f..a165fdc312f 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -8,7 +8,7 @@ class WebHook < ActiveRecord::Base default_value_for :note_events, false default_value_for :merge_requests_events, false default_value_for :tag_push_events, false - default_value_for :build_events, false + default_value_for :job_events, false default_value_for :pipeline_events, false default_value_for :repository_update_events, false default_value_for :enable_ssl_verification, true diff --git a/app/models/service.rb b/app/models/service.rb index c71a7d169ec..8916f88076e 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -12,7 +12,7 @@ class Service < ActiveRecord::Base default_value_for :merge_requests_events, true default_value_for :tag_push_events, true default_value_for :note_events, true - default_value_for :build_events, true + default_value_for :job_events, true default_value_for :pipeline_events, true default_value_for :wiki_page_events, true @@ -40,7 +40,7 @@ class Service < ActiveRecord::Base scope :confidential_issue_hooks, -> { where(confidential_issues_events: true, active: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) } scope :note_hooks, -> { where(note_events: true, active: true) } - scope :build_hooks, -> { where(build_events: true, active: true) } + scope :job_hooks, -> { where(job_events: true, active: true) } scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) } scope :external_issue_trackers, -> { issue_trackers.active.without_defaults } diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml index 3338b677bf5..e92b8bc39f4 100644 --- a/app/views/admin/hooks/index.html.haml +++ b/app/views/admin/hooks/index.html.haml @@ -27,7 +27,7 @@ = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm' .monospace= hook.url %div - - %w(repository_update_events push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger| + - %w(repository_update_events push_events tag_push_events issues_events note_events merge_requests_events job_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray= trigger.titleize %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'} diff --git a/app/views/projects/settings/integrations/_project_hook.html.haml b/app/views/projects/settings/integrations/_project_hook.html.haml index 8dc276a3bec..a6640592dba 100644 --- a/app/views/projects/settings/integrations/_project_hook.html.haml +++ b/app/views/projects/settings/integrations/_project_hook.html.haml @@ -3,7 +3,7 @@ .col-md-8.col-lg-7 %strong.light-header= hook.url %div - - %w(push_events tag_push_events issues_events confidential_issues_events note_events merge_requests_events build_events pipeline_events wiki_page_events).each do |trigger| + - %w(push_events tag_push_events issues_events confidential_issues_events note_events merge_requests_events job_events pipeline_events wiki_page_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray.deploy-project-label= trigger.titleize .col-md-4.col-lg-5.text-right-lg.prepend-top-5 diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index 37c3e61912c..1f0e7629fb4 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -54,10 +54,10 @@ %p.light This URL will be triggered when a merge request is created/updated/merged %li - = form.check_box :build_events, class: 'pull-left' + = form.check_box :job_events, class: 'pull-left' .prepend-left-20 - = form.label :build_events, class: 'list-label' do - %strong Jobs events + = form.label :job_events, class: 'list-label' do + %strong Job events %p.light This URL will be triggered when the job status changes %li diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 3fc2b453eb6..01cc8e8e1ca 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -60,7 +60,7 @@ module API class ProjectHook < Hook expose :project_id, :issues_events, :merge_requests_events expose :note_events, :pipeline_events, :wiki_page_events - expose :build_events, as: :job_events + expose :job_events end class BasicProjectDetails < Grape::Entity @@ -470,7 +470,7 @@ module API expose :id, :title, :created_at, :updated_at, :active expose :push_events, :issues_events, :merge_requests_events expose :tag_push_events, :note_events, :pipeline_events - expose :build_events, as: :job_events + expose :job_events # Expose serialized properties expose :properties do |service, options| field_names = service.fields. diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 87dfd1573a4..7a345289617 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -54,7 +54,6 @@ module API end post ":id/hooks" do hook_params = declared_params(include_missing: false) - hook_params[:build_events] = hook_params.delete(:job_events) { false } hook = user_project.hooks.new(hook_params) @@ -78,7 +77,6 @@ module API hook = user_project.hooks.find(params.delete(:hook_id)) update_params = declared_params(include_missing: false) - update_params[:build_events] = update_params.delete(:job_events) if update_params[:job_events] if hook.update_attributes(update_params) present hook, with: Entities::ProjectHook diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 56a9b019f1b..332f233bf5e 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -238,7 +238,8 @@ module API class ProjectService < Grape::Entity expose :id, :title, :created_at, :updated_at, :active expose :push_events, :issues_events, :merge_requests_events - expose :tag_push_events, :note_events, :build_events, :pipeline_events + expose :tag_push_events, :note_events, :pipeline_events + expose :job_events, as: :build_events # Expose serialized properties expose :properties do |service, options| field_names = service.fields. @@ -250,7 +251,8 @@ module API class ProjectHook < ::API::Entities::Hook expose :project_id, :issues_events, :merge_requests_events - expose :note_events, :build_events, :pipeline_events, :wiki_page_events + expose :note_events, :pipeline_events, :wiki_page_events + expose :job_events, as: :build_events end class Issue < ::API::Entities::Issue diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb index 0210e871a63..cd754ea235f 100644 --- a/spec/factories/project_hooks.rb +++ b/spec/factories/project_hooks.rb @@ -14,7 +14,7 @@ FactoryGirl.define do issues_events true confidential_issues_events true note_events true - build_events true + job_events true pipeline_events true wiki_page_events true end diff --git a/spec/features/projects/settings/integration_settings_spec.rb b/spec/features/projects/settings/integration_settings_spec.rb index 7909234556e..d3232f0cc16 100644 --- a/spec/features/projects/settings/integration_settings_spec.rb +++ b/spec/features/projects/settings/integration_settings_spec.rb @@ -52,6 +52,7 @@ feature 'Integration settings', feature: true do fill_in 'hook_url', with: url check 'Tag push events' check 'Enable SSL verification' + check 'Job events' click_button 'Add webhook' @@ -59,6 +60,7 @@ feature 'Integration settings', feature: true do expect(page).to have_content('SSL Verification: enabled') expect(page).to have_content('Push Events') expect(page).to have_content('Tag Push Events') + expect(page).to have_content('Job events') end scenario 'edit existing webhook' do diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index fdbb6a0556d..e3599d6fe59 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -6997,7 +6997,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "TeamcityService", "category": "ci", "default": false, @@ -7041,7 +7041,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "RedmineService", "category": "issue_tracker", "default": false, @@ -7063,7 +7063,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "PushoverService", "category": "common", "default": false, @@ -7085,7 +7085,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "PivotalTrackerService", "category": "common", "default": false, @@ -7108,7 +7108,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "JiraService", "category": "issue_tracker", "default": false, @@ -7130,7 +7130,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "IrkerService", "category": "common", "default": false, @@ -7174,7 +7174,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "GemnasiumService", "category": "common", "default": false, @@ -7196,7 +7196,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "FlowdockService", "category": "common", "default": false, @@ -7218,7 +7218,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "ExternalWikiService", "category": "common", "default": false, @@ -7240,7 +7240,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "EmailsOnPushService", "category": "common", "default": false, @@ -7262,7 +7262,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "DroneCiService", "category": "ci", "default": false, @@ -7284,7 +7284,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "CustomIssueTrackerService", "category": "issue_tracker", "default": false, @@ -7306,7 +7306,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "CampfireService", "category": "common", "default": false, @@ -7328,7 +7328,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "BuildkiteService", "category": "ci", "default": false, @@ -7350,7 +7350,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "BambooService", "category": "ci", "default": false, @@ -7372,7 +7372,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "AssemblaService", "category": "common", "default": false, @@ -7394,7 +7394,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "type": "AssemblaService", "category": "common", "default": false, @@ -7416,7 +7416,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "category": "common", "default": false, "wiki_page_events": true, diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index 744fed44925..5417c7534ea 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::ImportExport::RelationFactory, lib: true do 'tag_push_events' => false, 'note_events' => true, 'enable_ssl_verification' => true, - 'build_events' => false, + 'job_events' => false, 'wiki_page_events' => true, 'token' => token } diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 2b95f76e045..c22fba11225 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -292,7 +292,7 @@ Service: - tag_push_events - note_events - pipeline_events -- build_events +- job_events - category - default - wiki_page_events @@ -312,7 +312,7 @@ ProjectHook: - note_events - pipeline_events - enable_ssl_verification -- build_events +- job_events - wiki_page_events - token - group_id diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index aee0e17a153..0f9330b062d 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -60,7 +60,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) - expect(json_response['job_events']).to eq(hook.build_events) + expect(json_response['job_events']).to eq(hook.job_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) @@ -148,7 +148,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) - expect(json_response['job_events']).to eq(hook.build_events) + expect(json_response['job_events']).to eq(hook.job_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) diff --git a/spec/requests/api/v3/project_hooks_spec.rb b/spec/requests/api/v3/project_hooks_spec.rb index a3a4c77d09d..1969d1c7f2b 100644 --- a/spec/requests/api/v3/project_hooks_spec.rb +++ b/spec/requests/api/v3/project_hooks_spec.rb @@ -58,7 +58,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) - expect(json_response['build_events']).to eq(hook.build_events) + expect(json_response['build_events']).to eq(hook.job_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) @@ -143,7 +143,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) - expect(json_response['build_events']).to eq(hook.build_events) + expect(json_response['build_events']).to eq(hook.job_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) From 1e31a6df54d83b879f2c13229cb8db84653224d6 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Thu, 11 May 2017 14:44:27 +0200 Subject: [PATCH 25/38] Add `build_events` to project service in the API For backwards compatibility --- .../unreleased/bvl-rename-build-events-to-job-events.yml | 4 ++++ doc/api/services.md | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/bvl-rename-build-events-to-job-events.yml diff --git a/changelogs/unreleased/bvl-rename-build-events-to-job-events.yml b/changelogs/unreleased/bvl-rename-build-events-to-job-events.yml new file mode 100644 index 00000000000..2ce01a71361 --- /dev/null +++ b/changelogs/unreleased/bvl-rename-build-events-to-job-events.yml @@ -0,0 +1,4 @@ +--- +title: Rename build_events to job_events +merge_request: 11287 +author: diff --git a/doc/api/services.md b/doc/api/services.md index 0f42c256099..f77d15c2ea1 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -516,7 +516,7 @@ Example response: "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "build_events": true, + "job_events": true, "pipeline_events": true, "properties": { "token": "9koXpg98eAheJpvBs5tK" From 93a373fd0921b6c9912be30099f8fdb981421cd0 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Fri, 12 May 2017 08:52:34 +0200 Subject: [PATCH 26/38] Remove old migration spec. From this point, the `services` table doesn't have the `build_events` flag anymore. So instead of updating the spec for this, I removed it. It should have been executed at this point. --- ...te_build_events_to_pipeline_events_spec.rb | 74 ------------------- 1 file changed, 74 deletions(-) delete mode 100644 spec/migrations/migrate_build_events_to_pipeline_events_spec.rb diff --git a/spec/migrations/migrate_build_events_to_pipeline_events_spec.rb b/spec/migrations/migrate_build_events_to_pipeline_events_spec.rb deleted file mode 100644 index 57eb03e3c80..00000000000 --- a/spec/migrations/migrate_build_events_to_pipeline_events_spec.rb +++ /dev/null @@ -1,74 +0,0 @@ -require 'spec_helper' -require Rails.root.join('db', 'post_migrate', '20170301205640_migrate_build_events_to_pipeline_events.rb') - -# This migration uses multiple threads, and thus different transactions. This -# means data created in this spec may not be visible to some threads. To work -# around this we use the TRUNCATE cleaning strategy. -describe MigrateBuildEventsToPipelineEvents, truncate: true do - let(:migration) { described_class.new } - let(:project_with_pipeline_service) { create(:empty_project) } - let(:project_with_build_service) { create(:empty_project) } - - before do - ActiveRecord::Base.connection.execute <<-SQL - INSERT INTO services (properties, build_events, pipeline_events, type) - VALUES - ('{"notify_only_broken_builds":true}', true, false, 'SlackService') - , ('{"notify_only_broken_builds":true}', true, false, 'MattermostService') - , ('{"notify_only_broken_builds":true}', true, false, 'HipchatService') - ; - SQL - - ActiveRecord::Base.connection.execute <<-SQL - INSERT INTO services - (properties, build_events, pipeline_events, type, project_id) - VALUES - ('{"notify_only_broken_builds":true}', true, false, - 'BuildsEmailService', #{project_with_pipeline_service.id}) - , ('{"notify_only_broken_pipelines":true}', false, true, - 'PipelinesEmailService', #{project_with_pipeline_service.id}) - , ('{"notify_only_broken_builds":true}', true, false, - 'BuildsEmailService', #{project_with_build_service.id}) - ; - SQL - end - - describe '#up' do - before do - silence_migration = Module.new do - # rubocop:disable Rails/Delegate - def execute(query) - connection.execute(query) - end - end - - migration.extend(silence_migration) - migration.up - end - - it 'migrates chat service properly' do - [SlackService, MattermostService, HipchatService].each do |service| - expect(service.count).to eq(1) - - verify_service_record(service.first) - end - end - - it 'migrates pipelines email service only if it has none before' do - Project.find_each do |project| - pipeline_service_count = - project.services.where(type: 'PipelinesEmailService').count - - expect(pipeline_service_count).to eq(1) - - verify_service_record(project.pipelines_email_service) - end - end - - def verify_service_record(service) - expect(service.notify_only_broken_pipelines).to be(true) - expect(service.build_events).to be(false) - expect(service.pipeline_events).to be(true) - end - end -end From 78f0d50f4bce63c1f14b8910d88747ba6283fbc9 Mon Sep 17 00:00:00 2001 From: "Jacob Vosmaer (GitLab)" Date: Mon, 15 May 2017 12:53:47 +0000 Subject: [PATCH 27/38] Add script to regenerate seed_repo.rb for Gitlab::Git tests --- spec/support/generate-seed-repo-rb | 162 +++++++++++++++++++++++++++++ spec/support/seed_repo.rb | 11 +- 2 files changed, 172 insertions(+), 1 deletion(-) create mode 100755 spec/support/generate-seed-repo-rb diff --git a/spec/support/generate-seed-repo-rb b/spec/support/generate-seed-repo-rb new file mode 100755 index 00000000000..7335f74c0e9 --- /dev/null +++ b/spec/support/generate-seed-repo-rb @@ -0,0 +1,162 @@ +#!/usr/bin/env ruby +# +# # generate-seed-repo-rb +# +# This script generates the seed_repo.rb file used by lib/gitlab/git +# tests. The seed_repo.rb file needs to be updated anytime there is a +# Git push to https://gitlab.com/gitlab-org/gitlab-git-test. +# +# Usage: +# +# ./spec/support/generate-seed-repo-rb > spec/support/seed_repo.rb +# +# + +require 'erb' +require 'tempfile' + +SOURCE = 'https://gitlab.com/gitlab-org/gitlab-git-test.git'.freeze +SCRIPT_NAME = 'generate-seed-repo-rb'.freeze +REPO_NAME = 'gitlab-git-test.git'.freeze + +def main + Dir.mktmpdir do |dir| + unless system(*%W[git clone --bare #{SOURCE} #{REPO_NAME}], chdir: dir) + abort "git clone failed" + end + repo = File.join(dir, REPO_NAME) + erb = ERB.new(DATA.read) + erb.run(binding) + end +end + +def capture!(cmd, dir) + output = IO.popen(cmd, 'r', chdir: dir) { |io| io.read } + raise "command failed with #{$?}: #{cmd.join(' ')}" unless $?.success? + output.chomp +end + +main + +__END__ +# This file is generated by <%= SCRIPT_NAME %>. Do not edit this file manually. +# +# Seed repo: +<%= capture!(%w{git log --format=#\ %H\ %s}, repo) %> + +module SeedRepo + module BigCommit + ID = "913c66a37b4a45b9769037c55c2d238bd0942d2e".freeze + PARENT_ID = "cfe32cf61b73a0d5e9f13e774abde7ff789b1660".freeze + MESSAGE = "Files, encoding and much more".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES_COUNT = 2 + end + + module Commit + ID = "570e7b2abdd848b95f2f578043fc23bd6f6fd24d".freeze + PARENT_ID = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9".freeze + MESSAGE = "Change some files\n\nSigned-off-by: Dmitriy Zaporozhets \n".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES = ["files/ruby/popen.rb", "files/ruby/regex.rb"].freeze + FILES_COUNT = 2 + C_FILE_PATH = "files/ruby".freeze + C_FILES = ["popen.rb", "regex.rb", "version_info.rb"].freeze + BLOB_FILE = %{%h3= @key.title\n%hr\n%pre= @key.key\n.actions\n = link_to 'Remove', @key, :confirm => 'Are you sure?', :method => :delete, :class => \"btn danger delete-key\"\n\n\n}.freeze + BLOB_FILE_PATH = "app/views/keys/show.html.haml".freeze + end + + module EmptyCommit + ID = "b0e52af38d7ea43cf41d8a6f2471351ac036d6c9".freeze + PARENT_ID = "40f4a7a617393735a95a0bb67b08385bc1e7c66d".freeze + MESSAGE = "Empty commit".freeze + AUTHOR_FULL_NAME = "Rémy Coutable".freeze + FILES = [].freeze + FILES_COUNT = FILES.count + end + + module EncodingCommit + ID = "40f4a7a617393735a95a0bb67b08385bc1e7c66d".freeze + PARENT_ID = "66028349a123e695b589e09a36634d976edcc5e8".freeze + MESSAGE = "Add ISO-8859-encoded file".freeze + AUTHOR_FULL_NAME = "Stan Hu".freeze + FILES = ["encoding/iso8859.txt"].freeze + FILES_COUNT = FILES.count + end + + module FirstCommit + ID = "1a0b36b3cdad1d2ee32457c102a8c0b7056fa863".freeze + PARENT_ID = nil + MESSAGE = "Initial commit".freeze + AUTHOR_FULL_NAME = "Dmitriy Zaporozhets".freeze + FILES = ["LICENSE", ".gitignore", "README.md"].freeze + FILES_COUNT = 3 + end + + module LastCommit + ID = <%= capture!(%w[git show -s --format=%H HEAD], repo).inspect %>.freeze + PARENT_ID = <%= capture!(%w[git show -s --format=%P HEAD], repo).split.last.inspect %>.freeze + MESSAGE = <%= capture!(%w[git show -s --format=%s HEAD], repo).inspect %>.freeze + AUTHOR_FULL_NAME = <%= capture!(%w[git show -s --format=%an HEAD], repo).inspect %>.freeze + FILES = <%= + parents = capture!(%w[git show -s --format=%P HEAD], repo).split + merge_base = parents.size > 1 ? capture!(%w[git merge-base] + parents, repo) : parents.first + capture!( %W[git diff --name-only #{merge_base}..HEAD --], repo).split("\n").inspect + %>.freeze + FILES_COUNT = FILES.count + end + + module Repo + HEAD = "master".freeze + BRANCHES = %w[ +<%= capture!(%W[git for-each-ref --format=#{' ' * 3}%(refname:strip=2) refs/heads/], repo) %> + ].freeze + TAGS = %w[ +<%= capture!(%W[git for-each-ref --format=#{' ' * 3}%(refname:strip=2) refs/tags/], repo) %> + ].freeze + end + + module RubyBlob + ID = "7e3e39ebb9b2bf433b4ad17313770fbe4051649c".freeze + NAME = "popen.rb".freeze + CONTENT = <<-eos.freeze +require 'fileutils' +require 'open3' + +module Popen + extend self + + def popen(cmd, path=nil) + unless cmd.is_a?(Array) + raise RuntimeError, "System commands must be given as an array of strings" + end + + path ||= Dir.pwd + + vars = { + "PWD" => path + } + + options = { + chdir: path + } + + unless File.directory?(path) + FileUtils.mkdir_p(path) + end + + @cmd_output = "" + @cmd_status = 0 + + Open3.popen3(vars, *cmd, options) do |stdin, stdout, stderr, wait_thr| + @cmd_output << stdout.read + @cmd_output << stderr.read + @cmd_status = wait_thr.value.exitstatus + end + + return @cmd_output, @cmd_status + end +end + eos + end +end diff --git a/spec/support/seed_repo.rb b/spec/support/seed_repo.rb index 99a500bbbb1..cfe7fc980a8 100644 --- a/spec/support/seed_repo.rb +++ b/spec/support/seed_repo.rb @@ -1,4 +1,8 @@ +# This file is generated by generate-seed-repo-rb. Do not edit this file manually. +# # Seed repo: +# 4b4918a572fa86f9771e5ba40fbd48e1eb03e2c6 Merge branch 'master' into 'master' +# 0e1b353b348f8477bdbec1ef47087171c5032cd9 adds an executable with different permissions # 0e50ec4d3c7ce42ab74dda1d422cb2cbffe1e326 Merge branch 'lfs_pointers' into 'master' # 33bcff41c232a11727ac6d660bd4b0c2ba86d63d Add valid and invalid lfs pointers # 732401c65e924df81435deb12891ef570167d2e2 Update year in license file @@ -94,7 +98,12 @@ module SeedRepo master merge-test ].freeze - TAGS = %w[v1.0.0 v1.1.0 v1.2.0 v1.2.1].freeze + TAGS = %w[ + v1.0.0 + v1.1.0 + v1.2.0 + v1.2.1 + ].freeze end module RubyBlob From 872e7b7efe923192d4ef90b01672038518ba66fc Mon Sep 17 00:00:00 2001 From: George Andrinopoulos Date: Mon, 15 May 2017 13:53:12 +0000 Subject: [PATCH 28/38] Create a Users Finder --- app/finders/users_finder.rb | 74 +++++++++++++++++++++++++++++++ lib/api/users.rb | 11 +---- spec/finders/users_finder_spec.rb | 66 +++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 10 deletions(-) create mode 100644 app/finders/users_finder.rb create mode 100644 spec/finders/users_finder_spec.rb diff --git a/app/finders/users_finder.rb b/app/finders/users_finder.rb new file mode 100644 index 00000000000..dbd50d1db7c --- /dev/null +++ b/app/finders/users_finder.rb @@ -0,0 +1,74 @@ +# UsersFinder +# +# Used to filter users by set of params +# +# Arguments: +# current_user - which user use +# params: +# username: string +# extern_uid: string +# provider: string +# search: string +# active: boolean +# blocked: boolean +# external: boolean +# +class UsersFinder + attr_accessor :current_user, :params + + def initialize(current_user, params = {}) + @current_user = current_user + @params = params + end + + def execute + users = User.all + users = by_username(users) + users = by_search(users) + users = by_blocked(users) + users = by_active(users) + users = by_external_identity(users) + users = by_external(users) + + users + end + + private + + def by_username(users) + return users unless params[:username] + + users.where(username: params[:username]) + end + + def by_search(users) + return users unless params[:search].present? + + users.search(params[:search]) + end + + def by_blocked(users) + return users unless params[:blocked] + + users.blocked + end + + def by_active(users) + return users unless params[:active] + + users.active + end + + def by_external_identity(users) + return users unless current_user.admin? && params[:extern_uid] && params[:provider] + + users.joins(:identities).merge(Identity.with_extern_uid(params[:provider], params[:extern_uid])) + end + + def by_external(users) + return users = users.where.not(external: true) unless current_user.admin? + return users unless params[:external] + + users.external + end +end diff --git a/lib/api/users.rb b/lib/api/users.rb index 40acaebf670..3d83720b7b9 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -56,16 +56,7 @@ module API authenticated_as_admin! if params[:external].present? || (params[:extern_uid].present? && params[:provider].present?) - users = User.all - users = User.where(username: params[:username]) if params[:username] - users = users.active if params[:active] - users = users.search(params[:search]) if params[:search].present? - users = users.blocked if params[:blocked] - - if current_user.admin? - users = users.joins(:identities).merge(Identity.with_extern_uid(params[:provider], params[:extern_uid])) if params[:extern_uid] && params[:provider] - users = users.external if params[:external] - end + users = UsersFinder.new(current_user, params).execute entity = current_user.admin? ? Entities::UserPublic : Entities::UserBasic present paginate(users), with: entity diff --git a/spec/finders/users_finder_spec.rb b/spec/finders/users_finder_spec.rb new file mode 100644 index 00000000000..34a5440d651 --- /dev/null +++ b/spec/finders/users_finder_spec.rb @@ -0,0 +1,66 @@ +require 'spec_helper' + +describe UsersFinder do + describe '#execute' do + let!(:user1) { create(:user, username: 'johndoe') } + let!(:user2) { create(:user, :blocked, username: 'notsorandom', ) } + let!(:external_user) { create(:user, :external) } + let!(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') } + + context 'with a normal user' do + let(:user) { create(:user) } + + it 'returns all users' do + users = described_class.new(user).execute + + expect(users).to contain_exactly(user, user1, user2, omniauth_user) + end + + it 'filters by username' do + users = described_class.new(user, username: 'johndoe').execute + + expect(users).to contain_exactly(user1) + end + + it 'filters by search' do + users = described_class.new(user, search: 'orando').execute + + expect(users).to contain_exactly(user2) + end + + it 'filters by blocked users' do + users = described_class.new(user, blocked: true).execute + + expect(users).to contain_exactly(user2) + end + + it 'filters by active users' do + users = described_class.new(user, active: true).execute + + expect(users).to contain_exactly(user, user1, omniauth_user) + end + + it 'returns no external users' do + users = described_class.new(user, external: true).execute + + expect(users).to contain_exactly(user, user1, user2, omniauth_user) + end + end + + context 'with an admin user' do + let(:admin) { create(:admin) } + + it 'filters by external users' do + users = described_class.new(admin, external: true).execute + + expect(users).to contain_exactly(external_user) + end + + it 'returns all users' do + users = described_class.new(admin).execute + + expect(users).to contain_exactly(admin, user1, user2, external_user, omniauth_user) + end + end + end +end From 13176add18907a7f969a49b4b3094c74b7fdcd9f Mon Sep 17 00:00:00 2001 From: "Luke \"Jared\" Bennett" Date: Mon, 15 May 2017 14:01:38 +0000 Subject: [PATCH 29/38] MR zindex regression --- app/assets/stylesheets/framework/sidebar.scss | 1 + app/assets/stylesheets/pages/issuable.scss | 2 +- app/assets/stylesheets/pages/merge_requests.scss | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/framework/sidebar.scss b/app/assets/stylesheets/framework/sidebar.scss index 2b5ab539955..018f61ca3a8 100644 --- a/app/assets/stylesheets/framework/sidebar.scss +++ b/app/assets/stylesheets/framework/sidebar.scss @@ -53,6 +53,7 @@ .right-sidebar-expanded { padding-right: 0; + z-index: 300; @media (min-width: $screen-sm-min) and (max-width: $screen-sm-max) { &:not(.wiki-sidebar):not(.build-sidebar) .content-wrapper { diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index c4210ffd823..2b9a7e43f0f 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -206,7 +206,7 @@ transition: width .3s; background: $gray-light; padding: 10px 20px; - z-index: 2; + z-index: 200; &.right-sidebar-expanded { width: $gutter_width; diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 0173a05b403..61b13745f63 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -121,6 +121,7 @@ .dropdown-menu { margin-top: 11px; + z-index: 200; } .ci-action-icon-wrapper { @@ -690,7 +691,7 @@ .merge-request-tabs-holder { top: $header-height; - z-index: 10; + z-index: 100; background-color: $white-light; @media(min-width: $screen-sm-min) { From 2c45d07a06b3d070b2a6b92a48dac364311c411e Mon Sep 17 00:00:00 2001 From: Joshua Lambert Date: Mon, 15 May 2017 14:39:55 +0000 Subject: [PATCH 30/38] Fix typo --- doc/install/kubernetes/gitlab_chart.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/install/kubernetes/gitlab_chart.md b/doc/install/kubernetes/gitlab_chart.md index 35d395af024..2d7edbe16e4 100644 --- a/doc/install/kubernetes/gitlab_chart.md +++ b/doc/install/kubernetes/gitlab_chart.md @@ -392,7 +392,7 @@ Once you [have configured](#configuration) GitLab in your `values.yml` file, run the following: ```bash -helm install --namepace --name gitlab -f gitlab/gitlab +helm install --namespace --name gitlab -f gitlab/gitlab ``` where: @@ -407,7 +407,7 @@ Once your GitLab Chart is installed, configuration changes and chart updates should we done using `helm upgrade` ```bash -helm upgrade --namepace -f gitlab/gitlab +helm upgrade --namespace -f gitlab/gitlab ``` where: From a302ab159285c69553e88c10c6bfde4cd5fb26e1 Mon Sep 17 00:00:00 2001 From: Marcia Ramos Date: Mon, 15 May 2017 11:55:19 -0300 Subject: [PATCH 31/38] add article - how to install git --- doc/articles/how_to_install_git/index.md | 66 ++++++++++++++++++++++++ doc/articles/index.md | 4 ++ doc/topics/git/index.md | 1 + 3 files changed, 71 insertions(+) create mode 100644 doc/articles/how_to_install_git/index.md diff --git a/doc/articles/how_to_install_git/index.md b/doc/articles/how_to_install_git/index.md new file mode 100644 index 00000000000..2f504416358 --- /dev/null +++ b/doc/articles/how_to_install_git/index.md @@ -0,0 +1,66 @@ +# Installing Git + +> **Article [Type](../../development/writing_documentation.html#types-of-technical-articles):** user guide || +> **Level:** beginner || +> **Author:** [Sean Packham](https://gitlab.com/SeanPackham) || +> **Publication date:** 2017/05/15 + +To begin contributing to GitLab projects +you will need to install the Git client on your computer. +This article will show you how to install Git on macOS, Ubuntu Linux and Windows. + +## Install Git on macOS using the Homebrew package manager + +Although it is easy to use the version of Git shipped with macOS +or install the latest version of Git on macOS by downloading it from the project website, +we recommend installing it via Homebrew to get access to +an extensive selection of dependancy managed libraries and applications. + +If you are sure you don't need access to any additional development libraries +or don't have approximately 15gb of available disk space for Xcode and Homebrew +use one of the the aforementioned methods. + +### Installing Xcode + +Xcode is needed by Homebrew to build dependencies. +You can install [XCode](https://developer.apple.com/xcode/) +through the macOS App Store. + +### Installing Homebrew + +Once Xcode is installed browse to the [Homebrew website](http://brew.sh/index.html) +for the official Homebrew installation instructions. + +### Installing Git via Homebrew + +With Homebrew installed you are now ready to install Git. +Open a Terminal and enter in the following command: + +```bash +brew install git +``` + +Congratulations you should now have Git installed via Homebrew. +Next read our article on [adding an SSH key to GitLab](../ssh/README.md). + +## Install Git on Ubuntu Linux + +On Ubuntu and other Linux operating systems +it is recommended to use the built in package manager to install Git. + +Open a Terminal and enter in the following commands +to install the latest Git from the official Git maintained package archives: + +```bash +sudo apt-add-repository ppa:git-core/ppa +sudo apt-get update +sudo apt-get install git +``` + +Congratulations you should now have Git installed via the Ubuntu package manager. +Next read our article on [adding an SSH key to GitLab](../../ssh/README.md). + +## Installing Git on Windows from the Git website + +Browse to the [Git website](https://git-scm.com/) and download and install Git for Windows. +Next read our article on [adding an SSH key to GitLab](../../ssh/README.md). diff --git a/doc/articles/index.md b/doc/articles/index.md index 49db64134f5..342fa88e80f 100644 --- a/doc/articles/index.md +++ b/doc/articles/index.md @@ -12,6 +12,10 @@ They are written by members of the GitLab Team and by - **LDAP** - [How to configure LDAP with GitLab CE](how_to_configure_ldap_gitlab_ce/index.md) +## Git + +- [How to install Git](how_to_install_git/index.md) + ## GitLab Pages - **GitLab Pages from A to Z** diff --git a/doc/topics/git/index.md b/doc/topics/git/index.md index d13066c9015..604f9375714 100644 --- a/doc/topics/git/index.md +++ b/doc/topics/git/index.md @@ -22,6 +22,7 @@ We've gathered some resources to help you to get the best from Git with GitLab. - [Cherry-picking a commit](../../user/project/merge_requests/cherry_pick_changes.md#cherry-picking-a-commit) - [Squashing commits](../../workflow/gitlab_flow.md#squashing-commits-with-rebase) - **Articles:** + - [How to install Git](../../articles/how_to_install_git/index.md) - [Git Tips & Tricks](https://about.gitlab.com/2016/12/08/git-tips-and-tricks/) - [Eight Tips to help you work better with Git](https://about.gitlab.com/2015/02/19/8-tips-to-help-you-work-better-with-git/) - **Presentations:** From 0fd196acbb30ffa944907913d50d31629c637ac1 Mon Sep 17 00:00:00 2001 From: Marcia Ramos Date: Mon, 15 May 2017 12:03:34 -0300 Subject: [PATCH 32/38] fix broken link --- doc/articles/how_to_install_git/index.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/articles/how_to_install_git/index.md b/doc/articles/how_to_install_git/index.md index 2f504416358..66d866b2d09 100644 --- a/doc/articles/how_to_install_git/index.md +++ b/doc/articles/how_to_install_git/index.md @@ -41,7 +41,7 @@ brew install git ``` Congratulations you should now have Git installed via Homebrew. -Next read our article on [adding an SSH key to GitLab](../ssh/README.md). +Next read our article on [adding an SSH key to GitLab](../../ssh/README.md). ## Install Git on Ubuntu Linux From 55294ce60aeb416da285c273ef2d25572679a6c1 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Mon, 15 May 2017 15:07:43 +0000 Subject: [PATCH 33/38] Improve slash command stripping, escape temporary note contents --- app/assets/javascripts/notes.js | 5 ++-- spec/javascripts/notes_spec.js | 46 ++++++++++++++++++++++++++++++--- 2 files changed, 45 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index bce5379cbb9..f143bfbfc29 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -24,7 +24,7 @@ const normalizeNewlines = function(str) { (function() { this.Notes = (function() { const MAX_VISIBLE_COMMIT_LIST_COUNT = 3; - const REGEX_SLASH_COMMANDS = /^\/\w+/gm; + const REGEX_SLASH_COMMANDS = /^\/\w+.*$/gm; Notes.interval = null; @@ -1170,6 +1170,7 @@ const normalizeNewlines = function(str) { */ Notes.prototype.createPlaceholderNote = function({ formContent, uniqueId, isDiscussionNote, currentUsername, currentUserFullname }) { const discussionClass = isDiscussionNote ? 'discussion' : ''; + const escapedFormContent = _.escape(formContent); const $tempNote = $( `
  • @@ -1187,7 +1188,7 @@ const normalizeNewlines = function(str) {
    -

    ${formContent}

    +

    ${escapedFormContent}

  • diff --git a/spec/javascripts/notes_spec.js b/spec/javascripts/notes_spec.js index be4605a5b89..8243a9c991a 100644 --- a/spec/javascripts/notes_spec.js +++ b/spec/javascripts/notes_spec.js @@ -377,7 +377,7 @@ import '~/notes'; }); it('should return true when comment begins with a slash command', () => { - const sampleComment = '/wip \n/milestone %1.0 \n/merge \n/unassign Merging this'; + const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign Merging this'; const hasSlashCommands = this.notes.hasSlashCommands(sampleComment); expect(hasSlashCommands).toBeTruthy(); @@ -401,10 +401,18 @@ import '~/notes'; describe('stripSlashCommands', () => { it('should strip slash commands from the comment which begins with a slash command', () => { this.notes = new Notes(); - const sampleComment = '/wip \n/milestone %1.0 \n/merge \n/unassign Merging this'; + const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign Merging this'; const stripedComment = this.notes.stripSlashCommands(sampleComment); - expect(stripedComment).not.toBe(sampleComment); + expect(stripedComment).toBe(''); + }); + + it('should strip slash commands from the comment but leaves plain comment if it is present', () => { + this.notes = new Notes(); + const sampleComment = '/wip\n/milestone %1.0\n/merge\n/unassign\nMerging this'; + const stripedComment = this.notes.stripSlashCommands(sampleComment); + + expect(stripedComment).toBe('Merging this'); }); it('should NOT strip string that has slashes within', () => { @@ -424,6 +432,22 @@ import '~/notes'; beforeEach(() => { this.notes = new Notes('', []); + spyOn(_, 'escape').and.callFake((comment) => { + const escapedString = comment.replace(/["&'<>]/g, (a) => { + const escapedToken = { + '&': '&', + '<': '<', + '>': '>', + '"': '"', + "'": ''', + '`': '`' + }[a]; + + return escapedToken; + }); + + return escapedString; + }); }); it('should return constructed placeholder element for regular note based on form contents', () => { @@ -444,7 +468,21 @@ import '~/notes'; expect($tempNote.find('.timeline-content').hasClass('discussion')).toBeFalsy(); expect($tempNoteHeader.find('.hidden-xs').text().trim()).toEqual(currentUserFullname); expect($tempNoteHeader.find('.note-headline-light').text().trim()).toEqual(`@${currentUsername}`); - expect($tempNote.find('.note-body .note-text').text().trim()).toEqual(sampleComment); + expect($tempNote.find('.note-body .note-text p').text().trim()).toEqual(sampleComment); + }); + + it('should escape HTML characters from note based on form contents', () => { + const commentWithHtml = ''; + const $tempNote = this.notes.createPlaceholderNote({ + formContent: commentWithHtml, + uniqueId, + isDiscussionNote: false, + currentUsername, + currentUserFullname + }); + + expect(_.escape).toHaveBeenCalledWith(commentWithHtml); + expect($tempNote.find('.note-body .note-text p').html()).toEqual('<script>alert("Boom!");</script>'); }); it('should return constructed placeholder element for discussion note based on form contents', () => { From af67b6638aee659aa406dab166c499cc544903b5 Mon Sep 17 00:00:00 2001 From: Bob Van Landuyt Date: Mon, 15 May 2017 17:24:43 +0200 Subject: [PATCH 34/38] Fix trailing ',' in hash. --- spec/finders/users_finder_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/finders/users_finder_spec.rb b/spec/finders/users_finder_spec.rb index 34a5440d651..780b309b45e 100644 --- a/spec/finders/users_finder_spec.rb +++ b/spec/finders/users_finder_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' describe UsersFinder do describe '#execute' do let!(:user1) { create(:user, username: 'johndoe') } - let!(:user2) { create(:user, :blocked, username: 'notsorandom', ) } + let!(:user2) { create(:user, :blocked, username: 'notsorandom') } let!(:external_user) { create(:user, :external) } let!(:omniauth_user) { create(:omniauth_user, provider: 'twitter', extern_uid: '123456') } From cae2274fad39ae0933114cfcde1313e1f1ec4209 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Fri, 5 May 2017 21:33:21 -0500 Subject: [PATCH 35/38] Scope recent searches to project Fix https://gitlab.com/gitlab-org/gitlab-ce/issues/31902 --- .../filtered_search_manager.js | 10 ++-- .../stores/recent_searches_store.js | 1 + .../shared/issuable/_search_bar.html.haml | 2 +- ...2-namespace-recent-searches-to-project.yml | 4 ++ .../filtered_search/recent_searches_spec.rb | 54 ++++++++++++------- spec/support/filtered_search_helpers.rb | 6 +-- 6 files changed, 52 insertions(+), 25 deletions(-) create mode 100644 changelogs/unreleased/31902-namespace-recent-searches-to-project.yml diff --git a/app/assets/javascripts/filtered_search/filtered_search_manager.js b/app/assets/javascripts/filtered_search/filtered_search_manager.js index 9fea563370f..57d247e11a9 100644 --- a/app/assets/javascripts/filtered_search/filtered_search_manager.js +++ b/app/assets/javascripts/filtered_search/filtered_search_manager.js @@ -16,10 +16,14 @@ class FilteredSearchManager { this.recentSearchesStore = new RecentSearchesStore({ isLocalStorageAvailable: RecentSearchesService.isAvailable(), }); - let recentSearchesKey = 'issue-recent-searches'; + const searchHistoryDropdownElement = document.querySelector('.js-filtered-search-history-dropdown'); + const projectPath = searchHistoryDropdownElement ? + searchHistoryDropdownElement.dataset.projectFullPath : 'project'; + let recentSearchesPagePrefix = 'issue-recent-searches'; if (page === 'merge_requests') { - recentSearchesKey = 'merge-request-recent-searches'; + recentSearchesPagePrefix = 'merge-request-recent-searches'; } + const recentSearchesKey = `${projectPath}-${recentSearchesPagePrefix}`; this.recentSearchesService = new RecentSearchesService(recentSearchesKey); // Fetch recent searches from localStorage @@ -47,7 +51,7 @@ class FilteredSearchManager { this.recentSearchesRoot = new RecentSearchesRoot( this.recentSearchesStore, this.recentSearchesService, - document.querySelector('.js-filtered-search-history-dropdown'), + searchHistoryDropdownElement, ); this.recentSearchesRoot.init(); diff --git a/app/assets/javascripts/filtered_search/stores/recent_searches_store.js b/app/assets/javascripts/filtered_search/stores/recent_searches_store.js index 066be69766a..35fc15e4c87 100644 --- a/app/assets/javascripts/filtered_search/stores/recent_searches_store.js +++ b/app/assets/javascripts/filtered_search/stores/recent_searches_store.js @@ -3,6 +3,7 @@ import _ from 'underscore'; class RecentSearchesStore { constructor(initialState = {}) { this.state = Object.assign({ + isLocalStorageAvailable: true, recentSearches: [], }, initialState); } diff --git a/app/views/shared/issuable/_search_bar.html.haml b/app/views/shared/issuable/_search_bar.html.haml index f7b87171573..411d08bd8f8 100644 --- a/app/views/shared/issuable/_search_bar.html.haml +++ b/app/views/shared/issuable/_search_bar.html.haml @@ -19,7 +19,7 @@ dropdown_class: "filtered-search-history-dropdown", content_class: "filtered-search-history-dropdown-content", title: "Recent searches" }) do - .js-filtered-search-history-dropdown + .js-filtered-search-history-dropdown{ data: { project_full_path: @project.full_path } } .filtered-search-box-input-container .scroll-container %ul.tokens-container.list-unstyled diff --git a/changelogs/unreleased/31902-namespace-recent-searches-to-project.yml b/changelogs/unreleased/31902-namespace-recent-searches-to-project.yml new file mode 100644 index 00000000000..e00eb6d8f72 --- /dev/null +++ b/changelogs/unreleased/31902-namespace-recent-searches-to-project.yml @@ -0,0 +1,4 @@ +--- +title: Scope issue/merge request recent searches to project +merge_request: +author: diff --git a/spec/features/issues/filtered_search/recent_searches_spec.rb b/spec/features/issues/filtered_search/recent_searches_spec.rb index 08fe3b4553b..09f228bcf49 100644 --- a/spec/features/issues/filtered_search/recent_searches_spec.rb +++ b/spec/features/issues/filtered_search/recent_searches_spec.rb @@ -3,17 +3,17 @@ require 'spec_helper' describe 'Recent searches', js: true, feature: true do include FilteredSearchHelpers - let!(:group) { create(:group) } - let!(:project) { create(:project, group: group) } - let!(:user) { create(:user) } + let(:project_1) { create(:empty_project, :public) } + let(:project_2) { create(:empty_project, :public) } + let(:project_1_local_storage_key) { "#{project_1.full_path}-issue-recent-searches" } before do Capybara.ignore_hidden_elements = false - project.add_master(user) - group.add_developer(user) - create(:issue, project: project) - login_as(user) + create(:issue, project: project_1) + create(:issue, project: project_2) + # Visit any fast-loading page so we can clear local storage without a DOM exception + visit '/404' remove_recent_searches end @@ -22,7 +22,7 @@ describe 'Recent searches', js: true, feature: true do end it 'searching adds to recent searches' do - visit namespace_project_issues_path(project.namespace, project) + visit namespace_project_issues_path(project_1.namespace, project_1) input_filtered_search('foo', submit: true) input_filtered_search('bar', submit: true) @@ -35,8 +35,8 @@ describe 'Recent searches', js: true, feature: true do end it 'visiting URL with search params adds to recent searches' do - visit namespace_project_issues_path(project.namespace, project, label_name: 'foo', search: 'bar') - visit namespace_project_issues_path(project.namespace, project, label_name: 'qux', search: 'garply') + visit namespace_project_issues_path(project_1.namespace, project_1, label_name: 'foo', search: 'bar') + visit namespace_project_issues_path(project_1.namespace, project_1, label_name: 'qux', search: 'garply') items = all('.filtered-search-history-dropdown-item', visible: false) @@ -46,9 +46,9 @@ describe 'Recent searches', js: true, feature: true do end it 'saved recent searches are restored last on the list' do - set_recent_searches('["saved1", "saved2"]') + set_recent_searches(project_1_local_storage_key, '["saved1", "saved2"]') - visit namespace_project_issues_path(project.namespace, project, search: 'foo') + visit namespace_project_issues_path(project_1.namespace, project_1, search: 'foo') items = all('.filtered-search-history-dropdown-item', visible: false) @@ -58,9 +58,27 @@ describe 'Recent searches', js: true, feature: true do expect(items[2].text).to eq('saved2') end + it 'searches are scoped to projects' do + visit namespace_project_issues_path(project_1.namespace, project_1) + + input_filtered_search('foo', submit: true) + input_filtered_search('bar', submit: true) + + visit namespace_project_issues_path(project_2.namespace, project_2) + + input_filtered_search('more', submit: true) + input_filtered_search('things', submit: true) + + items = all('.filtered-search-history-dropdown-item', visible: false) + + expect(items.count).to eq(2) + expect(items[0].text).to eq('things') + expect(items[1].text).to eq('more') + end + it 'clicking item fills search input' do - set_recent_searches('["foo", "bar"]') - visit namespace_project_issues_path(project.namespace, project) + set_recent_searches(project_1_local_storage_key, '["foo", "bar"]') + visit namespace_project_issues_path(project_1.namespace, project_1) all('.filtered-search-history-dropdown-item', visible: false)[0].trigger('click') wait_for_filtered_search('foo') @@ -69,8 +87,8 @@ describe 'Recent searches', js: true, feature: true do end it 'clear recent searches button, clears recent searches' do - set_recent_searches('["foo"]') - visit namespace_project_issues_path(project.namespace, project) + set_recent_searches(project_1_local_storage_key, '["foo"]') + visit namespace_project_issues_path(project_1.namespace, project_1) items_before = all('.filtered-search-history-dropdown-item', visible: false) @@ -83,8 +101,8 @@ describe 'Recent searches', js: true, feature: true do end it 'shows flash error when failed to parse saved history' do - set_recent_searches('fail') - visit namespace_project_issues_path(project.namespace, project) + set_recent_searches(project_1_local_storage_key, 'fail') + visit namespace_project_issues_path(project_1.namespace, project_1) expect(find('.flash-alert')).to have_text('An error occured while parsing recent searches') end diff --git a/spec/support/filtered_search_helpers.rb b/spec/support/filtered_search_helpers.rb index 36be0bb6bf8..37cc308e613 100644 --- a/spec/support/filtered_search_helpers.rb +++ b/spec/support/filtered_search_helpers.rb @@ -73,11 +73,11 @@ module FilteredSearchHelpers end def remove_recent_searches - execute_script('window.localStorage.removeItem(\'issue-recent-searches\');') + execute_script('window.localStorage.clear();') end - def set_recent_searches(input) - execute_script("window.localStorage.setItem('issue-recent-searches', '#{input}');") + def set_recent_searches(key, input) + execute_script("window.localStorage.setItem('#{key}', '#{input}');") end def wait_for_filtered_search(text) From 897a85a461b95ece12a1c5daaa86f55c27ddadcb Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 18 Apr 2017 01:44:32 -0400 Subject: [PATCH 36/38] submodule_links: Handle in-repository submodule urls Sometimes it is useful to store submodules in the same repository which contains links to them. Make the UI support this. See https://github.com/twosigma/git-meta/wiki/The-Omega-Repo for information about this strategy Signed-off-by: David Turner --- app/helpers/submodule_helper.rb | 4 ++++ changelogs/unreleased/omega-submodules.yml | 4 ++++ spec/helpers/submodule_helper_spec.rb | 13 +++++++++++++ 3 files changed, 21 insertions(+) create mode 100644 changelogs/unreleased/omega-submodules.yml diff --git a/app/helpers/submodule_helper.rb b/app/helpers/submodule_helper.rb index b739554a7a4..09b73eee8cf 100644 --- a/app/helpers/submodule_helper.rb +++ b/app/helpers/submodule_helper.rb @@ -7,6 +7,10 @@ module SubmoduleHelper def submodule_links(submodule_item, ref = nil, repository = @repository) url = repository.submodule_url_for(ref, submodule_item.path) + if url == '.' || url == './' + url = File.join(Gitlab.config.gitlab.url, @project.full_path) + end + if url =~ /([^\/:]+)\/([^\/]+(?:\.git)?)\Z/ namespace, project = $1, $2 project.sub!(/\.git\z/, '') diff --git a/changelogs/unreleased/omega-submodules.yml b/changelogs/unreleased/omega-submodules.yml new file mode 100644 index 00000000000..1488eb72174 --- /dev/null +++ b/changelogs/unreleased/omega-submodules.yml @@ -0,0 +1,4 @@ +--- +title: 'Repository browser: handle in-repository submodule urls' +merge_request: +author: David Turner diff --git a/spec/helpers/submodule_helper_spec.rb b/spec/helpers/submodule_helper_spec.rb index 9da33792659..18935be95c9 100644 --- a/spec/helpers/submodule_helper_spec.rb +++ b/spec/helpers/submodule_helper_spec.rb @@ -81,6 +81,19 @@ describe SubmoduleHelper do end end + context 'in-repository submodule' do + let(:group) { create(:group, name: "Master Project", path: "master-project") } + let(:project) { create(:empty_project, group: group) } + before do + self.instance_variable_set(:@project, project) + end + + it 'in-repository' do + stub_url('./') + expect(submodule_links(submodule_item)).to eq(["/master-project/#{project.path}", "/master-project/#{project.path}/tree/hash"]) + end + end + context 'submodule on gitlab.com' do it 'detects ssh' do stub_url('git@gitlab.com:gitlab-org/gitlab-ce.git') From 950fa32a7c2c25d70bf8b2d495d759ce40bf2d9b Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 15 May 2017 20:24:19 +0000 Subject: [PATCH 37/38] Revert "Merge branch 'bvl-rename-build-events-to-job-events' into 'master'" This reverts merge request !11287 --- app/models/ci/build.rb | 4 +- app/models/hooks/project_hook.rb | 2 +- app/models/hooks/web_hook.rb | 2 +- app/models/service.rb | 4 +- app/views/admin/hooks/index.html.haml | 2 +- .../integrations/_project_hook.html.haml | 2 +- app/views/shared/web_hooks/_form.html.haml | 6 +- .../bvl-rename-build-events-to-job-events.yml | 4 - ...me_web_hooks_build_events_to_job_events.rb | 18 ----- ...ame_services_build_events_to_job_events.rb | 18 ----- ...me_web_hooks_build_events_to_job_events.rb | 18 ----- ...ame_services_build_events_to_job_events.rb | 18 ----- db/schema.rb | 6 +- doc/api/services.md | 2 +- lib/api/entities.rb | 4 +- lib/api/project_hooks.rb | 2 + lib/api/v3/entities.rb | 6 +- spec/factories/project_hooks.rb | 2 +- .../settings/integration_settings_spec.rb | 2 - spec/lib/gitlab/import_export/project.json | 36 ++++----- .../import_export/relation_factory_spec.rb | 2 +- .../import_export/safe_model_attributes.yml | 4 +- ...te_build_events_to_pipeline_events_spec.rb | 74 +++++++++++++++++++ spec/requests/api/project_hooks_spec.rb | 4 +- spec/requests/api/v3/project_hooks_spec.rb | 4 +- 25 files changed, 121 insertions(+), 125 deletions(-) delete mode 100644 changelogs/unreleased/bvl-rename-build-events-to-job-events.yml delete mode 100644 db/migrate/20170511082759_rename_web_hooks_build_events_to_job_events.rb delete mode 100644 db/migrate/20170511083824_rename_services_build_events_to_job_events.rb delete mode 100644 db/post_migrate/20170511100900_cleanup_rename_web_hooks_build_events_to_job_events.rb delete mode 100644 db/post_migrate/20170511101000_cleanup_rename_services_build_events_to_job_events.rb create mode 100644 spec/migrations/migrate_build_events_to_pipeline_events_spec.rb diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 760ec8e5919..3c4a4d93349 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -300,8 +300,8 @@ module Ci def execute_hooks return unless project build_data = Gitlab::DataBuilder::Build.build(self) - project.execute_hooks(build_data.dup, :job_hooks) - project.execute_services(build_data.dup, :job_hooks) + project.execute_hooks(build_data.dup, :build_hooks) + project.execute_services(build_data.dup, :build_hooks) PagesService.new(build_data).execute project.running_or_pending_build_count(force: true) end diff --git a/app/models/hooks/project_hook.rb b/app/models/hooks/project_hook.rb index ee6165fd32d..c631e7a7df5 100644 --- a/app/models/hooks/project_hook.rb +++ b/app/models/hooks/project_hook.rb @@ -5,7 +5,7 @@ class ProjectHook < WebHook scope :confidential_issue_hooks, -> { where(confidential_issues_events: true) } scope :note_hooks, -> { where(note_events: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true) } - scope :job_hooks, -> { where(job_events: true) } + scope :build_hooks, -> { where(build_events: true) } scope :pipeline_hooks, -> { where(pipeline_events: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true) } end diff --git a/app/models/hooks/web_hook.rb b/app/models/hooks/web_hook.rb index a165fdc312f..7cf03aabd6f 100644 --- a/app/models/hooks/web_hook.rb +++ b/app/models/hooks/web_hook.rb @@ -8,7 +8,7 @@ class WebHook < ActiveRecord::Base default_value_for :note_events, false default_value_for :merge_requests_events, false default_value_for :tag_push_events, false - default_value_for :job_events, false + default_value_for :build_events, false default_value_for :pipeline_events, false default_value_for :repository_update_events, false default_value_for :enable_ssl_verification, true diff --git a/app/models/service.rb b/app/models/service.rb index 8916f88076e..c71a7d169ec 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -12,7 +12,7 @@ class Service < ActiveRecord::Base default_value_for :merge_requests_events, true default_value_for :tag_push_events, true default_value_for :note_events, true - default_value_for :job_events, true + default_value_for :build_events, true default_value_for :pipeline_events, true default_value_for :wiki_page_events, true @@ -40,7 +40,7 @@ class Service < ActiveRecord::Base scope :confidential_issue_hooks, -> { where(confidential_issues_events: true, active: true) } scope :merge_request_hooks, -> { where(merge_requests_events: true, active: true) } scope :note_hooks, -> { where(note_events: true, active: true) } - scope :job_hooks, -> { where(job_events: true, active: true) } + scope :build_hooks, -> { where(build_events: true, active: true) } scope :pipeline_hooks, -> { where(pipeline_events: true, active: true) } scope :wiki_page_hooks, -> { where(wiki_page_events: true, active: true) } scope :external_issue_trackers, -> { issue_trackers.active.without_defaults } diff --git a/app/views/admin/hooks/index.html.haml b/app/views/admin/hooks/index.html.haml index e92b8bc39f4..3338b677bf5 100644 --- a/app/views/admin/hooks/index.html.haml +++ b/app/views/admin/hooks/index.html.haml @@ -27,7 +27,7 @@ = link_to 'Remove', admin_hook_path(hook), data: { confirm: 'Are you sure?' }, method: :delete, class: 'btn btn-remove btn-sm' .monospace= hook.url %div - - %w(repository_update_events push_events tag_push_events issues_events note_events merge_requests_events job_events).each do |trigger| + - %w(repository_update_events push_events tag_push_events issues_events note_events merge_requests_events build_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray= trigger.titleize %span.label.label-gray SSL Verification: #{hook.enable_ssl_verification ? 'enabled' : 'disabled'} diff --git a/app/views/projects/settings/integrations/_project_hook.html.haml b/app/views/projects/settings/integrations/_project_hook.html.haml index a6640592dba..8dc276a3bec 100644 --- a/app/views/projects/settings/integrations/_project_hook.html.haml +++ b/app/views/projects/settings/integrations/_project_hook.html.haml @@ -3,7 +3,7 @@ .col-md-8.col-lg-7 %strong.light-header= hook.url %div - - %w(push_events tag_push_events issues_events confidential_issues_events note_events merge_requests_events job_events pipeline_events wiki_page_events).each do |trigger| + - %w(push_events tag_push_events issues_events confidential_issues_events note_events merge_requests_events build_events pipeline_events wiki_page_events).each do |trigger| - if hook.send(trigger) %span.label.label-gray.deploy-project-label= trigger.titleize .col-md-4.col-lg-5.text-right-lg.prepend-top-5 diff --git a/app/views/shared/web_hooks/_form.html.haml b/app/views/shared/web_hooks/_form.html.haml index 1f0e7629fb4..37c3e61912c 100644 --- a/app/views/shared/web_hooks/_form.html.haml +++ b/app/views/shared/web_hooks/_form.html.haml @@ -54,10 +54,10 @@ %p.light This URL will be triggered when a merge request is created/updated/merged %li - = form.check_box :job_events, class: 'pull-left' + = form.check_box :build_events, class: 'pull-left' .prepend-left-20 - = form.label :job_events, class: 'list-label' do - %strong Job events + = form.label :build_events, class: 'list-label' do + %strong Jobs events %p.light This URL will be triggered when the job status changes %li diff --git a/changelogs/unreleased/bvl-rename-build-events-to-job-events.yml b/changelogs/unreleased/bvl-rename-build-events-to-job-events.yml deleted file mode 100644 index 2ce01a71361..00000000000 --- a/changelogs/unreleased/bvl-rename-build-events-to-job-events.yml +++ /dev/null @@ -1,4 +0,0 @@ ---- -title: Rename build_events to job_events -merge_request: 11287 -author: diff --git a/db/migrate/20170511082759_rename_web_hooks_build_events_to_job_events.rb b/db/migrate/20170511082759_rename_web_hooks_build_events_to_job_events.rb deleted file mode 100644 index a2320a911b7..00000000000 --- a/db/migrate/20170511082759_rename_web_hooks_build_events_to_job_events.rb +++ /dev/null @@ -1,18 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class RenameWebHooksBuildEventsToJobEvents < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - def up - rename_column_concurrently :web_hooks, :build_events, :job_events - end - - def down - cleanup_concurrent_column_rename :web_hooks, :job_events, :build_events - end -end diff --git a/db/migrate/20170511083824_rename_services_build_events_to_job_events.rb b/db/migrate/20170511083824_rename_services_build_events_to_job_events.rb deleted file mode 100644 index 303d47078e7..00000000000 --- a/db/migrate/20170511083824_rename_services_build_events_to_job_events.rb +++ /dev/null @@ -1,18 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class RenameServicesBuildEventsToJobEvents < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - def up - rename_column_concurrently :services, :build_events, :job_events - end - - def down - cleanup_concurrent_column_rename :services, :job_events, :build_events - end -end diff --git a/db/post_migrate/20170511100900_cleanup_rename_web_hooks_build_events_to_job_events.rb b/db/post_migrate/20170511100900_cleanup_rename_web_hooks_build_events_to_job_events.rb deleted file mode 100644 index 281be90163a..00000000000 --- a/db/post_migrate/20170511100900_cleanup_rename_web_hooks_build_events_to_job_events.rb +++ /dev/null @@ -1,18 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class CleanupRenameWebHooksBuildEventsToJobEvents < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - def up - cleanup_concurrent_column_rename :web_hooks, :build_events, :job_events - end - - def down - rename_column_concurrently :web_hooks, :job_events, :build_events - end -end diff --git a/db/post_migrate/20170511101000_cleanup_rename_services_build_events_to_job_events.rb b/db/post_migrate/20170511101000_cleanup_rename_services_build_events_to_job_events.rb deleted file mode 100644 index 5d26df5688f..00000000000 --- a/db/post_migrate/20170511101000_cleanup_rename_services_build_events_to_job_events.rb +++ /dev/null @@ -1,18 +0,0 @@ -# See http://doc.gitlab.com/ce/development/migration_style_guide.html -# for more information on how to write migrations for GitLab. - -class CleanupRenameServicesBuildEventsToJobEvents < ActiveRecord::Migration - include Gitlab::Database::MigrationHelpers - - DOWNTIME = false - - disable_ddl_transaction! - - def up - cleanup_concurrent_column_rename :services, :build_events, :job_events - end - - def down - rename_column_concurrently :services, :job_events, :build_events - end -end diff --git a/db/schema.rb b/db/schema.rb index 513e3696646..a683521c84b 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20170511101000) do +ActiveRecord::Schema.define(version: 20170508190732) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1125,13 +1125,13 @@ ActiveRecord::Schema.define(version: 20170511101000) do t.boolean "merge_requests_events", default: true t.boolean "tag_push_events", default: true t.boolean "note_events", default: true, null: false + t.boolean "build_events", default: false, null: false t.string "category", default: "common", null: false t.boolean "default", default: false t.boolean "wiki_page_events", default: true t.boolean "pipeline_events", default: false, null: false t.boolean "confidential_issues_events", default: true, null: false t.boolean "commit_events", default: true, null: false - t.boolean "job_events", default: false, null: false end add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree @@ -1401,11 +1401,11 @@ ActiveRecord::Schema.define(version: 20170511101000) do t.boolean "tag_push_events", default: false t.boolean "note_events", default: false, null: false t.boolean "enable_ssl_verification", default: true + t.boolean "build_events", default: false, null: false t.boolean "wiki_page_events", default: false, null: false t.string "token" t.boolean "pipeline_events", default: false, null: false t.boolean "confidential_issues_events", default: false, null: false - t.boolean "job_events", default: false, null: false t.boolean "repository_update_events", default: false, null: false end diff --git a/doc/api/services.md b/doc/api/services.md index f77d15c2ea1..0f42c256099 100644 --- a/doc/api/services.md +++ b/doc/api/services.md @@ -516,7 +516,7 @@ Example response: "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "pipeline_events": true, "properties": { "token": "9koXpg98eAheJpvBs5tK" diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 01cc8e8e1ca..3fc2b453eb6 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -60,7 +60,7 @@ module API class ProjectHook < Hook expose :project_id, :issues_events, :merge_requests_events expose :note_events, :pipeline_events, :wiki_page_events - expose :job_events + expose :build_events, as: :job_events end class BasicProjectDetails < Grape::Entity @@ -470,7 +470,7 @@ module API expose :id, :title, :created_at, :updated_at, :active expose :push_events, :issues_events, :merge_requests_events expose :tag_push_events, :note_events, :pipeline_events - expose :job_events + expose :build_events, as: :job_events # Expose serialized properties expose :properties do |service, options| field_names = service.fields. diff --git a/lib/api/project_hooks.rb b/lib/api/project_hooks.rb index 7a345289617..87dfd1573a4 100644 --- a/lib/api/project_hooks.rb +++ b/lib/api/project_hooks.rb @@ -54,6 +54,7 @@ module API end post ":id/hooks" do hook_params = declared_params(include_missing: false) + hook_params[:build_events] = hook_params.delete(:job_events) { false } hook = user_project.hooks.new(hook_params) @@ -77,6 +78,7 @@ module API hook = user_project.hooks.find(params.delete(:hook_id)) update_params = declared_params(include_missing: false) + update_params[:build_events] = update_params.delete(:job_events) if update_params[:job_events] if hook.update_attributes(update_params) present hook, with: Entities::ProjectHook diff --git a/lib/api/v3/entities.rb b/lib/api/v3/entities.rb index 332f233bf5e..56a9b019f1b 100644 --- a/lib/api/v3/entities.rb +++ b/lib/api/v3/entities.rb @@ -238,8 +238,7 @@ module API class ProjectService < Grape::Entity expose :id, :title, :created_at, :updated_at, :active expose :push_events, :issues_events, :merge_requests_events - expose :tag_push_events, :note_events, :pipeline_events - expose :job_events, as: :build_events + expose :tag_push_events, :note_events, :build_events, :pipeline_events # Expose serialized properties expose :properties do |service, options| field_names = service.fields. @@ -251,8 +250,7 @@ module API class ProjectHook < ::API::Entities::Hook expose :project_id, :issues_events, :merge_requests_events - expose :note_events, :pipeline_events, :wiki_page_events - expose :job_events, as: :build_events + expose :note_events, :build_events, :pipeline_events, :wiki_page_events end class Issue < ::API::Entities::Issue diff --git a/spec/factories/project_hooks.rb b/spec/factories/project_hooks.rb index cd754ea235f..0210e871a63 100644 --- a/spec/factories/project_hooks.rb +++ b/spec/factories/project_hooks.rb @@ -14,7 +14,7 @@ FactoryGirl.define do issues_events true confidential_issues_events true note_events true - job_events true + build_events true pipeline_events true wiki_page_events true end diff --git a/spec/features/projects/settings/integration_settings_spec.rb b/spec/features/projects/settings/integration_settings_spec.rb index d3232f0cc16..7909234556e 100644 --- a/spec/features/projects/settings/integration_settings_spec.rb +++ b/spec/features/projects/settings/integration_settings_spec.rb @@ -52,7 +52,6 @@ feature 'Integration settings', feature: true do fill_in 'hook_url', with: url check 'Tag push events' check 'Enable SSL verification' - check 'Job events' click_button 'Add webhook' @@ -60,7 +59,6 @@ feature 'Integration settings', feature: true do expect(page).to have_content('SSL Verification: enabled') expect(page).to have_content('Push Events') expect(page).to have_content('Tag Push Events') - expect(page).to have_content('Job events') end scenario 'edit existing webhook' do diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index e3599d6fe59..fdbb6a0556d 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -6997,7 +6997,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "TeamcityService", "category": "ci", "default": false, @@ -7041,7 +7041,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "RedmineService", "category": "issue_tracker", "default": false, @@ -7063,7 +7063,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "PushoverService", "category": "common", "default": false, @@ -7085,7 +7085,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "PivotalTrackerService", "category": "common", "default": false, @@ -7108,7 +7108,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "JiraService", "category": "issue_tracker", "default": false, @@ -7130,7 +7130,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "IrkerService", "category": "common", "default": false, @@ -7174,7 +7174,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "GemnasiumService", "category": "common", "default": false, @@ -7196,7 +7196,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "FlowdockService", "category": "common", "default": false, @@ -7218,7 +7218,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "ExternalWikiService", "category": "common", "default": false, @@ -7240,7 +7240,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "EmailsOnPushService", "category": "common", "default": false, @@ -7262,7 +7262,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "DroneCiService", "category": "ci", "default": false, @@ -7284,7 +7284,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "CustomIssueTrackerService", "category": "issue_tracker", "default": false, @@ -7306,7 +7306,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "CampfireService", "category": "common", "default": false, @@ -7328,7 +7328,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "BuildkiteService", "category": "ci", "default": false, @@ -7350,7 +7350,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "BambooService", "category": "ci", "default": false, @@ -7372,7 +7372,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "AssemblaService", "category": "common", "default": false, @@ -7394,7 +7394,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "type": "AssemblaService", "category": "common", "default": false, @@ -7416,7 +7416,7 @@ "merge_requests_events": true, "tag_push_events": true, "note_events": true, - "job_events": true, + "build_events": true, "category": "common", "default": false, "wiki_page_events": true, diff --git a/spec/lib/gitlab/import_export/relation_factory_spec.rb b/spec/lib/gitlab/import_export/relation_factory_spec.rb index 5417c7534ea..744fed44925 100644 --- a/spec/lib/gitlab/import_export/relation_factory_spec.rb +++ b/spec/lib/gitlab/import_export/relation_factory_spec.rb @@ -33,7 +33,7 @@ describe Gitlab::ImportExport::RelationFactory, lib: true do 'tag_push_events' => false, 'note_events' => true, 'enable_ssl_verification' => true, - 'job_events' => false, + 'build_events' => false, 'wiki_page_events' => true, 'token' => token } diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index c22fba11225..2b95f76e045 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -292,7 +292,7 @@ Service: - tag_push_events - note_events - pipeline_events -- job_events +- build_events - category - default - wiki_page_events @@ -312,7 +312,7 @@ ProjectHook: - note_events - pipeline_events - enable_ssl_verification -- job_events +- build_events - wiki_page_events - token - group_id diff --git a/spec/migrations/migrate_build_events_to_pipeline_events_spec.rb b/spec/migrations/migrate_build_events_to_pipeline_events_spec.rb new file mode 100644 index 00000000000..57eb03e3c80 --- /dev/null +++ b/spec/migrations/migrate_build_events_to_pipeline_events_spec.rb @@ -0,0 +1,74 @@ +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20170301205640_migrate_build_events_to_pipeline_events.rb') + +# This migration uses multiple threads, and thus different transactions. This +# means data created in this spec may not be visible to some threads. To work +# around this we use the TRUNCATE cleaning strategy. +describe MigrateBuildEventsToPipelineEvents, truncate: true do + let(:migration) { described_class.new } + let(:project_with_pipeline_service) { create(:empty_project) } + let(:project_with_build_service) { create(:empty_project) } + + before do + ActiveRecord::Base.connection.execute <<-SQL + INSERT INTO services (properties, build_events, pipeline_events, type) + VALUES + ('{"notify_only_broken_builds":true}', true, false, 'SlackService') + , ('{"notify_only_broken_builds":true}', true, false, 'MattermostService') + , ('{"notify_only_broken_builds":true}', true, false, 'HipchatService') + ; + SQL + + ActiveRecord::Base.connection.execute <<-SQL + INSERT INTO services + (properties, build_events, pipeline_events, type, project_id) + VALUES + ('{"notify_only_broken_builds":true}', true, false, + 'BuildsEmailService', #{project_with_pipeline_service.id}) + , ('{"notify_only_broken_pipelines":true}', false, true, + 'PipelinesEmailService', #{project_with_pipeline_service.id}) + , ('{"notify_only_broken_builds":true}', true, false, + 'BuildsEmailService', #{project_with_build_service.id}) + ; + SQL + end + + describe '#up' do + before do + silence_migration = Module.new do + # rubocop:disable Rails/Delegate + def execute(query) + connection.execute(query) + end + end + + migration.extend(silence_migration) + migration.up + end + + it 'migrates chat service properly' do + [SlackService, MattermostService, HipchatService].each do |service| + expect(service.count).to eq(1) + + verify_service_record(service.first) + end + end + + it 'migrates pipelines email service only if it has none before' do + Project.find_each do |project| + pipeline_service_count = + project.services.where(type: 'PipelinesEmailService').count + + expect(pipeline_service_count).to eq(1) + + verify_service_record(project.pipelines_email_service) + end + end + + def verify_service_record(service) + expect(service.notify_only_broken_pipelines).to be(true) + expect(service.build_events).to be(false) + expect(service.pipeline_events).to be(true) + end + end +end diff --git a/spec/requests/api/project_hooks_spec.rb b/spec/requests/api/project_hooks_spec.rb index 0f9330b062d..aee0e17a153 100644 --- a/spec/requests/api/project_hooks_spec.rb +++ b/spec/requests/api/project_hooks_spec.rb @@ -60,7 +60,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) - expect(json_response['job_events']).to eq(hook.job_events) + expect(json_response['job_events']).to eq(hook.build_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) @@ -148,7 +148,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) - expect(json_response['job_events']).to eq(hook.job_events) + expect(json_response['job_events']).to eq(hook.build_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) diff --git a/spec/requests/api/v3/project_hooks_spec.rb b/spec/requests/api/v3/project_hooks_spec.rb index 1969d1c7f2b..a3a4c77d09d 100644 --- a/spec/requests/api/v3/project_hooks_spec.rb +++ b/spec/requests/api/v3/project_hooks_spec.rb @@ -58,7 +58,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) - expect(json_response['build_events']).to eq(hook.job_events) + expect(json_response['build_events']).to eq(hook.build_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) @@ -143,7 +143,7 @@ describe API::ProjectHooks, 'ProjectHooks' do expect(json_response['merge_requests_events']).to eq(hook.merge_requests_events) expect(json_response['tag_push_events']).to eq(hook.tag_push_events) expect(json_response['note_events']).to eq(hook.note_events) - expect(json_response['build_events']).to eq(hook.job_events) + expect(json_response['build_events']).to eq(hook.build_events) expect(json_response['pipeline_events']).to eq(hook.pipeline_events) expect(json_response['wiki_page_events']).to eq(hook.wiki_page_events) expect(json_response['enable_ssl_verification']).to eq(hook.enable_ssl_verification) From beec97f6cf5f7357238e6f27e48115d6960f3de4 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Mon, 15 May 2017 16:01:09 -0500 Subject: [PATCH 38/38] Disallow NULL on renamed column after default has been set --- lib/gitlab/database/migration_helpers.rb | 3 ++- spec/lib/gitlab/database/migration_helpers_spec.rb | 6 ++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/gitlab/database/migration_helpers.rb b/lib/gitlab/database/migration_helpers.rb index f3476dadec8..e76c9abbe04 100644 --- a/lib/gitlab/database/migration_helpers.rb +++ b/lib/gitlab/database/migration_helpers.rb @@ -283,7 +283,6 @@ module Gitlab add_column(table, new, new_type, limit: old_col.limit, - null: old_col.null, precision: old_col.precision, scale: old_col.scale) @@ -307,6 +306,8 @@ module Gitlab update_column_in_batches(table, new, Arel::Table.new(table)[old]) + change_column_null(table, new, false) unless old_col.null + copy_indexes(table, old, new) copy_foreign_keys(table, old, new) end diff --git a/spec/lib/gitlab/database/migration_helpers_spec.rb b/spec/lib/gitlab/database/migration_helpers_spec.rb index d6535f97665..dfa3ae9142e 100644 --- a/spec/lib/gitlab/database/migration_helpers_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers_spec.rb @@ -382,7 +382,6 @@ describe Gitlab::Database::MigrationHelpers, lib: true do expect(model).to receive(:add_column). with(:users, :new, :integer, limit: old_column.limit, - null: old_column.null, precision: old_column.precision, scale: old_column.scale) @@ -391,6 +390,8 @@ describe Gitlab::Database::MigrationHelpers, lib: true do expect(model).to receive(:update_column_in_batches) + expect(model).to receive(:change_column_null).with(:users, :new, false) + expect(model).to receive(:copy_indexes).with(:users, :old, :new) expect(model).to receive(:copy_foreign_keys).with(:users, :old, :new) @@ -408,7 +409,6 @@ describe Gitlab::Database::MigrationHelpers, lib: true do expect(model).to receive(:add_column). with(:users, :new, :integer, limit: old_column.limit, - null: old_column.null, precision: old_column.precision, scale: old_column.scale) @@ -417,6 +417,8 @@ describe Gitlab::Database::MigrationHelpers, lib: true do expect(model).to receive(:update_column_in_batches) + expect(model).to receive(:change_column_null).with(:users, :new, false) + expect(model).to receive(:copy_indexes).with(:users, :old, :new) expect(model).to receive(:copy_foreign_keys).with(:users, :old, :new)