From 47e0a6cf429fed8d7bd527f8ab9fa53a86caedfe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 17:47:39 +0100 Subject: [PATCH 01/33] Remove environment_scope in user/gcp show partial --- app/views/projects/clusters/gcp/_show.html.haml | 4 ---- app/views/projects/clusters/user/_show.html.haml | 4 ---- 2 files changed, 8 deletions(-) diff --git a/app/views/projects/clusters/gcp/_show.html.haml b/app/views/projects/clusters/gcp/_show.html.haml index bde85aed341..f3122a1bf47 100644 --- a/app/views/projects/clusters/gcp/_show.html.haml +++ b/app/views/projects/clusters/gcp/_show.html.haml @@ -9,10 +9,6 @@ = form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field| = form_errors(@cluster) - .form-group - = field.label :environment_scope, s_('ClusterIntegration|Environment scope') - = field.text_field :environment_scope, class: 'form-control js-select-on-focus', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope') - = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| .form-group = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL') diff --git a/app/views/projects/clusters/user/_show.html.haml b/app/views/projects/clusters/user/_show.html.haml index 89595bca007..5931e0b7f17 100644 --- a/app/views/projects/clusters/user/_show.html.haml +++ b/app/views/projects/clusters/user/_show.html.haml @@ -4,10 +4,6 @@ = field.label :name, s_('ClusterIntegration|Cluster name') = field.text_field :name, class: 'form-control', placeholder: s_('ClusterIntegration|Cluster name') - .form-group - = field.label :environment_scope, s_('ClusterIntegration|Environment scope') - = field.text_field :environment_scope, class: 'form-control js-select-on-focus', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope') - = field.fields_for :platform_kubernetes, @cluster.platform_kubernetes do |platform_kubernetes_field| .form-group = platform_kubernetes_field.label :api_url, s_('ClusterIntegration|API URL') From 9812d8c283abcd2b2ab3c7fea061c77f6f4eeb99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 17:58:46 +0100 Subject: [PATCH 02/33] Add environment_scope to enabled partial --- app/views/projects/clusters/_banner.html.haml | 14 +++------- .../projects/clusters/_enabled.html.haml | 26 +++++++++++++++---- app/views/projects/clusters/show.html.haml | 2 +- 3 files changed, 25 insertions(+), 17 deletions(-) diff --git a/app/views/projects/clusters/_banner.html.haml b/app/views/projects/clusters/_banner.html.haml index 76a66fb92a2..6b9507c854f 100644 --- a/app/views/projects/clusters/_banner.html.haml +++ b/app/views/projects/clusters/_banner.html.haml @@ -1,6 +1,7 @@ -%h4= s_('ClusterIntegration|Enable cluster integration') -.settings-content +%h4= s_('ClusterIntegration|Cluster integration') +%p= s_('ClusterIntegration|Control how your cluster integrates with GitLab') +.settings-content .hidden.js-cluster-error.alert.alert-danger.alert-block.append-bottom-10{ role: 'alert' } = s_('ClusterIntegration|Something went wrong while creating your cluster on Google Kubernetes Engine') %p.js-error-reason @@ -10,12 +11,3 @@ .hidden.js-cluster-success.alert.alert-success.alert-block.append-bottom-10{ role: 'alert' } = s_('ClusterIntegration|Cluster was successfully created on Google Kubernetes Engine. Refresh the page to see cluster\'s details') - - %p - - if @cluster.enabled? - - if can?(current_user, :update_cluster, @cluster) - = s_('ClusterIntegration|Cluster integration is enabled for this project. Disabling this integration will not affect your cluster, it will only temporarily turn off GitLab\'s connection to it.') - - else - = s_('ClusterIntegration|Cluster integration is enabled for this project.') - - else - = s_('ClusterIntegration|Cluster integration is disabled for this project.') diff --git a/app/views/projects/clusters/_enabled.html.haml b/app/views/projects/clusters/_enabled.html.haml index 547b3c8446f..1eac2c9dc1f 100644 --- a/app/views/projects/clusters/_enabled.html.haml +++ b/app/views/projects/clusters/_enabled.html.haml @@ -1,7 +1,16 @@ = form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field| = form_errors(@cluster) - .form-group.append-bottom-20 - %label.append-bottom-10 + .form-group + %h5= s_('ClusterIntegration|Integration status') + %p + - if @cluster.enabled? + - if can?(current_user, :update_cluster, @cluster) + = s_('ClusterIntegration|Cluster integration is enabled for this project. Disabling this integration will not affect your cluster, it will only temporarily turn off GitLab\'s connection to it.') + - else + = s_('ClusterIntegration|Cluster integration is enabled for this project.') + - else + = s_('ClusterIntegration|Cluster integration is disabled for this project.') + %label = field.hidden_field :enabled, { class: 'js-toggle-input'} %button{ type: 'button', @@ -12,6 +21,13 @@ = sprite_icon('status_success_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-checked') = sprite_icon('status_failed_borderless', size: 16, css_class: 'toggle-icon-svg toggle-status-unchecked') - - if can?(current_user, :update_cluster, @cluster) - .form-group - = field.submit _('Save'), class: 'btn btn-success' + .form-group + %h5= s_('ClusterIntegration|Environment scope') + %p + = s_("ClusterIntegration|Choose which of your project's environments will use this cluster.") + = link_to s_("ClusterIntegration|Learn more about environments"), help_page_path('ci/environments') + = field.text_field :environment_scope, class: 'form-control js-select-on-focus', readonly: !has_multiple_clusters?(@project), placeholder: s_('ClusterIntegration|Environment scope') + + - if can?(current_user, :update_cluster, @cluster) + .form-group + = field.submit _('Save changes'), class: 'btn btn-success' diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml index fe6dacf1f0d..dfaef8716de 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -17,7 +17,7 @@ .js-cluster-application-notice .flash-container - %section.settings.no-animate.expanded + %section.settings.no-animate.expanded#cluster-integration = render 'banner' = render 'enabled' From d2d494196da1f9665034845c19bc5b87c3c67ebd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Mon, 18 Dec 2017 19:22:53 +0100 Subject: [PATCH 03/33] Match updated clusters/show in feature specs --- spec/features/projects/clusters/gcp_spec.rb | 6 +++--- spec/features/projects/clusters/user_spec.rb | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/spec/features/projects/clusters/gcp_spec.rb b/spec/features/projects/clusters/gcp_spec.rb index 67b8901f8fb..882a2756b72 100644 --- a/spec/features/projects/clusters/gcp_spec.rb +++ b/spec/features/projects/clusters/gcp_spec.rb @@ -81,14 +81,14 @@ feature 'Gcp Cluster', :js do end it 'user sees a cluster details page' do - expect(page).to have_button('Save') + expect(page).to have_button('Save changes') expect(page.find(:css, '.cluster-name').value).to eq(cluster.name) end context 'when user disables the cluster' do before do page.find(:css, '.js-toggle-cluster').click - click_button 'Save' + page.within('#cluster-integration') { click_button 'Save changes' } end it 'user sees the successful message' do @@ -99,7 +99,7 @@ feature 'Gcp Cluster', :js do context 'when user changes cluster parameters' do before do fill_in 'cluster_platform_kubernetes_attributes_namespace', with: 'my-namespace' - click_button 'Save changes' + page.within('#js-cluster-details') { click_button 'Save changes' } end it 'user sees the successful message' do diff --git a/spec/features/projects/clusters/user_spec.rb b/spec/features/projects/clusters/user_spec.rb index 414f4acba86..a519b9f9c7e 100644 --- a/spec/features/projects/clusters/user_spec.rb +++ b/spec/features/projects/clusters/user_spec.rb @@ -29,7 +29,7 @@ feature 'User Cluster', :js do end it 'user sees a cluster details page' do - expect(page).to have_content('Enable cluster integration') + expect(page).to have_content('Cluster integration') expect(page.find_field('cluster[name]').value).to eq('dev-cluster') expect(page.find_field('cluster[platform_kubernetes_attributes][api_url]').value) .to have_content('http://example.com') @@ -57,14 +57,14 @@ feature 'User Cluster', :js do end it 'user sees a cluster details page' do - expect(page).to have_button('Save') + expect(page).to have_button('Save changes') end context 'when user disables the cluster' do before do page.find(:css, '.js-toggle-cluster').click fill_in 'cluster_name', with: 'dev-cluster' - click_button 'Save' + page.within('#cluster-integration') { click_button 'Save changes' } end it 'user sees the successful message' do @@ -76,7 +76,7 @@ feature 'User Cluster', :js do before do fill_in 'cluster_name', with: 'my-dev-cluster' fill_in 'cluster_platform_kubernetes_attributes_namespace', with: 'my-namespace' - click_button 'Save changes' + page.within('#js-cluster-details') { click_button 'Save changes' } end it 'user sees the successful message' do From 9e2febf82fa63ff07e513cd400c7a41e4a4fe4bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Wed, 20 Dec 2017 18:19:34 +0100 Subject: [PATCH 04/33] Environment pattern -> Environment scope --- app/views/projects/clusters/_cluster.html.haml | 2 +- app/views/projects/clusters/index.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/projects/clusters/_cluster.html.haml b/app/views/projects/clusters/_cluster.html.haml index ad696daa259..3943dfc0856 100644 --- a/app/views/projects/clusters/_cluster.html.haml +++ b/app/views/projects/clusters/_cluster.html.haml @@ -4,7 +4,7 @@ .table-mobile-content = link_to cluster.name, namespace_project_cluster_path(@project.namespace, @project, cluster) .table-section.section-30 - .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Environment pattern") + .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Environment scope") .table-mobile-content= cluster.environment_scope .table-section.section-30 .table-mobile-header{ role: "rowheader" }= s_("ClusterIntegration|Project namespace") diff --git a/app/views/projects/clusters/index.html.haml b/app/views/projects/clusters/index.html.haml index bec512be91c..74dbe859eea 100644 --- a/app/views/projects/clusters/index.html.haml +++ b/app/views/projects/clusters/index.html.haml @@ -13,7 +13,7 @@ .table-section.section-30{ role: "rowheader" } = s_("ClusterIntegration|Cluster") .table-section.section-30{ role: "rowheader" } - = s_("ClusterIntegration|Environment pattern") + = s_("ClusterIntegration|Environment scope") .table-section.section-30{ role: "rowheader" } = s_("ClusterIntegration|Project namespace") .table-section.section-10{ role: "rowheader" } From fea009bfdacad62f03d0cf3ea02d54ec9989390c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 21 Dec 2017 18:24:38 +0100 Subject: [PATCH 05/33] Rename enabled partial to integration_form --- app/views/projects/clusters/_banner.html.haml | 3 ++- .../{_enabled.html.haml => _integration_form.html.haml} | 0 app/views/projects/clusters/show.html.haml | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) rename app/views/projects/clusters/{_enabled.html.haml => _integration_form.html.haml} (100%) diff --git a/app/views/projects/clusters/_banner.html.haml b/app/views/projects/clusters/_banner.html.haml index 6b9507c854f..26ca3307a4a 100644 --- a/app/views/projects/clusters/_banner.html.haml +++ b/app/views/projects/clusters/_banner.html.haml @@ -1,5 +1,4 @@ %h4= s_('ClusterIntegration|Cluster integration') -%p= s_('ClusterIntegration|Control how your cluster integrates with GitLab') .settings-content .hidden.js-cluster-error.alert.alert-danger.alert-block.append-bottom-10{ role: 'alert' } @@ -11,3 +10,5 @@ .hidden.js-cluster-success.alert.alert-success.alert-block.append-bottom-10{ role: 'alert' } = s_('ClusterIntegration|Cluster was successfully created on Google Kubernetes Engine. Refresh the page to see cluster\'s details') + + %p= s_('ClusterIntegration|Control how your cluster integrates with GitLab') diff --git a/app/views/projects/clusters/_enabled.html.haml b/app/views/projects/clusters/_integration_form.html.haml similarity index 100% rename from app/views/projects/clusters/_enabled.html.haml rename to app/views/projects/clusters/_integration_form.html.haml diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml index dfaef8716de..c15785806b9 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -19,7 +19,7 @@ %section.settings.no-animate.expanded#cluster-integration = render 'banner' - = render 'enabled' + = render 'integration_form' .cluster-applications-table#js-cluster-applications From 59e50e33b3203bfe450f82d2ee2cc83b87f9e5fb Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Thu, 21 Dec 2017 15:05:35 +0100 Subject: [PATCH 06/33] Reroute batch blobs to single blob RPC Given the priorities shifted for the Gitaly team, this endpoint does not get a dedicated endpoint yet. To make it work in a cloud native environment the request needs to go to Gitaly, not rugged. This is achieved by rerouting to the generic TreeEntry endpoint. --- lib/gitlab/git/blob.rb | 31 +++++++++++++++++++++++++------ spec/lib/gitlab/git/blob_spec.rb | 15 ++++----------- 2 files changed, 29 insertions(+), 17 deletions(-) diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index 228d97a87ab..bd91125d3b6 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -50,10 +50,19 @@ module Gitlab # to the caller to limit the number of blobs and blob_size_limit. # # Gitaly migration issue: https://gitlab.com/gitlab-org/gitaly/issues/798 - def batch(repository, blob_references, blob_size_limit: nil) - blob_size_limit ||= MAX_DATA_DISPLAY_SIZE - blob_references.map do |sha, path| - find_by_rugged(repository, sha, path, limit: blob_size_limit) + def batch(repository, blob_references, blob_size_limit: MAX_DATA_DISPLAY_SIZE) + Gitlab::GitalyClient.migrate(:list_blobs_by_sha_path) do |is_enabled| + if is_enabled + Gitlab::GitalyClient.allow_n_plus_1_calls do + blob_references.map do |sha, path| + find_by_gitaly(repository, sha, path, limit: blob_size_limit) + end + end + else + blob_references.map do |sha, path| + find_by_rugged(repository, sha, path, limit: blob_size_limit) + end + end end end @@ -122,13 +131,23 @@ module Gitlab ) end - def find_by_gitaly(repository, sha, path) + def find_by_gitaly(repository, sha, path, limit: MAX_DATA_DISPLAY_SIZE) path = path.sub(/\A\/*/, '') path = '/' if path.empty? name = File.basename(path) - entry = Gitlab::GitalyClient::CommitService.new(repository).tree_entry(sha, path, MAX_DATA_DISPLAY_SIZE) + + # Gitaly will think that setting the limit to 0 means unlimited, while + # the client might only need the metadata and thus set the limit to 0. + # In this method we'll than set the limit to 1, but clear the byte of data + # that we got back so fot the outside world it looks like the limit was + # actually 0. + req_limit = limit == 0 ? 1 : limit + + entry = Gitlab::GitalyClient::CommitService.new(repository).tree_entry(sha, path, req_limit) return unless entry + entry.data = "" if limit == 0 + case entry.type when :COMMIT new( diff --git a/spec/lib/gitlab/git/blob_spec.rb b/spec/lib/gitlab/git/blob_spec.rb index c04a9688503..7f5946b1658 100644 --- a/spec/lib/gitlab/git/blob_spec.rb +++ b/spec/lib/gitlab/git/blob_spec.rb @@ -202,16 +202,6 @@ describe Gitlab::Git::Blob, seed_helper: true do context 'limiting' do subject { described_class.batch(repository, blob_references, blob_size_limit: blob_size_limit) } - context 'default' do - let(:blob_size_limit) { nil } - - it 'limits to MAX_DATA_DISPLAY_SIZE' do - stub_const('Gitlab::Git::Blob::MAX_DATA_DISPLAY_SIZE', 100) - - expect(subject.first.data.size).to eq(100) - end - end - context 'positive' do let(:blob_size_limit) { 10 } @@ -221,7 +211,10 @@ describe Gitlab::Git::Blob, seed_helper: true do context 'zero' do let(:blob_size_limit) { 0 } - it { expect(subject.first.data).to eq('') } + it 'only loads the metadata' do + expect(subject.first.size).not_to be(0) + expect(subject.first.data).to eq('') + end end context 'negative' do From 5e0143a84bca7fd8b2dccd175e0f50c87dea4b98 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Sat, 27 May 2017 15:23:27 +0200 Subject: [PATCH 07/33] Add online attribute to runner api entity --- .../unreleased/feature-api_runners_online.yml | 4 +++ doc/api/runners.md | 29 +++++++++++++------ lib/api/entities.rb | 1 + 3 files changed, 25 insertions(+), 9 deletions(-) create mode 100644 changelogs/unreleased/feature-api_runners_online.yml diff --git a/changelogs/unreleased/feature-api_runners_online.yml b/changelogs/unreleased/feature-api_runners_online.yml new file mode 100644 index 00000000000..f5077507e5b --- /dev/null +++ b/changelogs/unreleased/feature-api_runners_online.yml @@ -0,0 +1,4 @@ +--- +title: Add online attribute to runner api entity +merge_request: 11750 +author: Alessio Caiazza diff --git a/doc/api/runners.md b/doc/api/runners.md index 015b09a745e..50981ed96bc 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -30,14 +30,16 @@ Example response: "description": "test-1-20150125", "id": 6, "is_shared": false, - "name": null + "name": null, + "online": true }, { "active": true, "description": "test-2-20150125", "id": 8, "is_shared": false, - "name": null + "name": null, + "online": false } ] ``` @@ -69,28 +71,32 @@ Example response: "description": "shared-runner-1", "id": 1, "is_shared": true, - "name": null + "name": null, + "online": true }, { "active": true, "description": "shared-runner-2", "id": 3, "is_shared": true, - "name": null + "name": null, + "online": false }, { "active": true, "description": "test-1-20150125", "id": 6, "is_shared": false, - "name": null + "name": null, + "online": true }, { "active": true, "description": "test-2-20150125", "id": 8, "is_shared": false, - "name": null + "name": null, + "online": false } ] ``` @@ -122,6 +128,7 @@ Example response: "is_shared": false, "contacted_at": "2016-01-25T16:39:48.066Z", "name": null, + "online": true, "platform": null, "projects": [ { @@ -176,6 +183,7 @@ Example response: "is_shared": false, "contacted_at": "2016-01-25T16:39:48.066Z", "name": null, + "online": true, "platform": null, "projects": [ { @@ -327,14 +335,16 @@ Example response: "description": "test-2-20150125", "id": 8, "is_shared": false, - "name": null + "name": null, + "online": false }, { "active": true, "description": "development_runner", "id": 5, "is_shared": true, - "name": null + "name": null, + "online": true } ] ``` @@ -364,7 +374,8 @@ Example response: "description": "test-2016-02-01", "id": 9, "is_shared": false, - "name": null + "name": null, + "online": true } ``` diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 4ad4a1f7867..c612dde7f73 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -862,6 +862,7 @@ module API expose :active expose :is_shared expose :name + expose :online?, as: :online end class RunnerDetails < Runner From 0d6b9e30cb5c3b76ee97cd14dea1dae12a74e8d6 Mon Sep 17 00:00:00 2001 From: Jose Ivan Vargas Date: Fri, 22 Dec 2017 12:25:24 -0600 Subject: [PATCH 08/33] Fix import project url not updating project name --- .../javascripts/projects/project_new.js | 7 ++-- .../jivl-fix-import-project-url-bug.yml | 5 +++ spec/javascripts/projects/project_new_spec.js | 32 +++++++++++-------- 3 files changed, 27 insertions(+), 17 deletions(-) create mode 100644 changelogs/unreleased/jivl-fix-import-project-url-bug.yml diff --git a/app/assets/javascripts/projects/project_new.js b/app/assets/javascripts/projects/project_new.js index 3ecc0c2a6e5..4710e70d619 100644 --- a/app/assets/javascripts/projects/project_new.js +++ b/app/assets/javascripts/projects/project_new.js @@ -1,6 +1,7 @@ let hasUserDefinedProjectPath = false; -const deriveProjectPathFromUrl = ($projectImportUrl, $projectPath) => { +const deriveProjectPathFromUrl = ($projectImportUrl) => { + const $currentProjectPath = $projectImportUrl.parents('.toggle-import-form').find('#project_path'); if (hasUserDefinedProjectPath) { return; } @@ -21,7 +22,7 @@ const deriveProjectPathFromUrl = ($projectImportUrl, $projectPath) => { // extract everything after the last slash const pathMatch = /\/([^/]+)$/.exec(importUrl); if (pathMatch) { - $projectPath.val(pathMatch[1]); + $currentProjectPath.val(pathMatch[1]); } }; @@ -96,7 +97,7 @@ const bindEvents = () => { hasUserDefinedProjectPath = $projectPath.val().trim().length > 0; }); - $projectImportUrl.keyup(() => deriveProjectPathFromUrl($projectImportUrl, $projectPath)); + $projectImportUrl.keyup(() => deriveProjectPathFromUrl($projectImportUrl)); }; document.addEventListener('DOMContentLoaded', bindEvents); diff --git a/changelogs/unreleased/jivl-fix-import-project-url-bug.yml b/changelogs/unreleased/jivl-fix-import-project-url-bug.yml new file mode 100644 index 00000000000..0d97b9c9a53 --- /dev/null +++ b/changelogs/unreleased/jivl-fix-import-project-url-bug.yml @@ -0,0 +1,5 @@ +--- +title: Fix import project url not updating project name +merge_request: 16120 +author: +type: fixed diff --git a/spec/javascripts/projects/project_new_spec.js b/spec/javascripts/projects/project_new_spec.js index 850768f0e4f..c314ca8ab72 100644 --- a/spec/javascripts/projects/project_new_spec.js +++ b/spec/javascripts/projects/project_new_spec.js @@ -6,8 +6,12 @@ describe('New Project', () => { beforeEach(() => { setFixtures(` - - +
+
+ + +
+
`); $projectImportUrl = $('#project_import_url'); @@ -25,7 +29,7 @@ describe('New Project', () => { it('does not change project path for disabled $projectImportUrl', () => { $projectImportUrl.attr('disabled', true); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual(dummyImportUrl); }); @@ -38,7 +42,7 @@ describe('New Project', () => { it('does not change project path if it is set by user', () => { $projectPath.keyup(); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual(dummyImportUrl); }); @@ -46,7 +50,7 @@ describe('New Project', () => { it('does not change project path for empty $projectImportUrl', () => { $projectImportUrl.val(''); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual(dummyImportUrl); }); @@ -54,7 +58,7 @@ describe('New Project', () => { it('does not change project path for whitespace $projectImportUrl', () => { $projectImportUrl.val(' '); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual(dummyImportUrl); }); @@ -62,7 +66,7 @@ describe('New Project', () => { it('does not change project path for $projectImportUrl without slashes', () => { $projectImportUrl.val('has-no-slash'); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual(dummyImportUrl); }); @@ -70,7 +74,7 @@ describe('New Project', () => { it('changes project path to last $projectImportUrl component', () => { $projectImportUrl.val('/this/is/last'); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual('last'); }); @@ -78,7 +82,7 @@ describe('New Project', () => { it('ignores trailing slashes in $projectImportUrl', () => { $projectImportUrl.val('/has/trailing/slash/'); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual('slash'); }); @@ -86,7 +90,7 @@ describe('New Project', () => { it('ignores fragment identifier in $projectImportUrl', () => { $projectImportUrl.val('/this/has/a#fragment-identifier/'); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual('a'); }); @@ -94,7 +98,7 @@ describe('New Project', () => { it('ignores query string in $projectImportUrl', () => { $projectImportUrl.val('/url/with?query=string'); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual('with'); }); @@ -102,7 +106,7 @@ describe('New Project', () => { it('ignores trailing .git in $projectImportUrl', () => { $projectImportUrl.val('/repository.git'); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual('repository'); }); @@ -110,7 +114,7 @@ describe('New Project', () => { it('changes project path for HTTPS URL in $projectImportUrl', () => { $projectImportUrl.val('https://username:password@gitlab.company.com/group/project.git'); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual('project'); }); @@ -118,7 +122,7 @@ describe('New Project', () => { it('changes project path for SSH URL in $projectImportUrl', () => { $projectImportUrl.val('git@gitlab.com:gitlab-org/gitlab-ce.git'); - projectNew.deriveProjectPathFromUrl($projectImportUrl, $projectPath); + projectNew.deriveProjectPathFromUrl($projectImportUrl); expect($projectPath.val()).toEqual('gitlab-ce'); }); From 5d3ade5cebfaefa38f888d2b2c3ae85c131ada7f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 4 Jan 2018 01:55:18 +0100 Subject: [PATCH 09/33] Update Advanced cluster settings subtitle --- app/views/projects/clusters/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/clusters/show.html.haml b/app/views/projects/clusters/show.html.haml index c15785806b9..1f5426dcfa5 100644 --- a/app/views/projects/clusters/show.html.haml +++ b/app/views/projects/clusters/show.html.haml @@ -40,6 +40,6 @@ %h4= _('Advanced settings') %button.btn.js-settings-toggle = expanded ? 'Collapse' : 'Expand' - %p= s_('ClusterIntegration|Manage cluster integration on your GitLab project') + %p= s_("ClusterIntegration|Advanced options on this cluster's integration") .settings-content = render 'advanced_settings' From ab7382f90a8a59d1dcd4445eadb0a3adb0eda7c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Thu, 4 Jan 2018 01:58:28 +0100 Subject: [PATCH 10/33] Update Remove cluster subtitle and alert --- app/views/projects/clusters/_advanced_settings.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/projects/clusters/_advanced_settings.html.haml b/app/views/projects/clusters/_advanced_settings.html.haml index 7032b892029..8a13713ae02 100644 --- a/app/views/projects/clusters/_advanced_settings.html.haml +++ b/app/views/projects/clusters/_advanced_settings.html.haml @@ -11,5 +11,5 @@ %label.text-danger = s_('ClusterIntegration|Remove cluster integration') %p - = s_('ClusterIntegration|Removing cluster integration will remove the cluster configuration you have added to this project. It will not delete your cluster on Google Kubernetes Engine.') - = link_to(s_('ClusterIntegration|Remove integration'), namespace_project_cluster_path(@project.namespace, @project, @cluster.id), method: :delete, class: 'btn btn-danger', data: { confirm: "Are you sure you want to remove cluster integration from this project? This will not delete your cluster on Google Kubernetes Engine"}) + = s_("ClusterIntegration|Remove this cluster's configuration from this project. This will not delete your actual cluster.") + = link_to(s_('ClusterIntegration|Remove integration'), namespace_project_cluster_path(@project.namespace, @project, @cluster.id), method: :delete, class: 'btn btn-danger', data: { confirm: s_("ClusterIntegration|Are you sure you want to remove this cluster's integration? This will not delete your actual cluster.")}) From 260935868acfb7c0cb720088d4f8c4c1c1088ddb Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 14 Dec 2017 11:49:35 +0100 Subject: [PATCH 11/33] add new git fsck rake task and spec --- lib/tasks/gitlab/git.rake | 10 ++++++++++ spec/tasks/gitlab/git_rake_spec.rb | 27 +++++++++++++++++++++++++++ 2 files changed, 37 insertions(+) create mode 100644 spec/tasks/gitlab/git_rake_spec.rb diff --git a/lib/tasks/gitlab/git.rake b/lib/tasks/gitlab/git.rake index cf82134d97e..f3ffff43726 100644 --- a/lib/tasks/gitlab/git.rake +++ b/lib/tasks/gitlab/git.rake @@ -30,6 +30,16 @@ namespace :gitlab do end end + desc 'GitLab | Git | Check all repos integrity' + task fsck: :environment do + failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} fsck --name-objects --no-progress), "Checking integrity") + if failures.empty? + puts "Done".color(:green) + else + output_failures(failures) + end + end + def perform_git_cmd(cmd, message) puts "Starting #{message} on all repositories" diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb new file mode 100644 index 00000000000..63a7f7efe73 --- /dev/null +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -0,0 +1,27 @@ +require 'rake_helper' + +describe 'gitlab:git rake tasks' do + before do + Rake.application.rake_require 'tasks/gitlab/git' + + stub_warn_user_is_not_gitlab + + FileUtils.mkdir(Settings.absolute('tmp/tests/default_storage')) + end + + after do + FileUtils.rm_rf(Settings.absolute('tmp/tests/default_storage')) + end + + describe 'fsck' do + let(:storages) do + { 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage') } } + end + + it 'outputs the right git command' do + expect(Kernel).to receive(:system).with('').and_return(true) + + run_rake_task('gitlab:git:fsck') + end + end +end From 7721e8dfca9d272376f58dcb03ff277aef0a9c31 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 14 Dec 2017 14:53:34 +0100 Subject: [PATCH 12/33] fix spec --- spec/tasks/gitlab/git_rake_spec.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb index 63a7f7efe73..60b51186ceb 100644 --- a/spec/tasks/gitlab/git_rake_spec.rb +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -4,9 +4,11 @@ describe 'gitlab:git rake tasks' do before do Rake.application.rake_require 'tasks/gitlab/git' - stub_warn_user_is_not_gitlab + storages = { 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage') } } - FileUtils.mkdir(Settings.absolute('tmp/tests/default_storage')) + FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/repo/test.git')) + allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) + stub_warn_user_is_not_gitlab end after do @@ -14,14 +16,8 @@ describe 'gitlab:git rake tasks' do end describe 'fsck' do - let(:storages) do - { 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage') } } - end - it 'outputs the right git command' do - expect(Kernel).to receive(:system).with('').and_return(true) - - run_rake_task('gitlab:git:fsck') + expect { run_rake_task('gitlab:git:fsck') }.to output(/Performed Checking integrity/).to_stdout end end end From bc46c822fc94cfa54a190cfb0e89afeae799f57a Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 21 Dec 2017 15:42:25 +0100 Subject: [PATCH 13/33] remove max-depth flag so it works with subgroups --- lib/tasks/gitlab/task_helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/tasks/gitlab/task_helpers.rb b/lib/tasks/gitlab/task_helpers.rb index 6723662703c..c1182af1014 100644 --- a/lib/tasks/gitlab/task_helpers.rb +++ b/lib/tasks/gitlab/task_helpers.rb @@ -130,7 +130,7 @@ module Gitlab def all_repos Gitlab.config.repositories.storages.each_value do |repository_storage| - IO.popen(%W(find #{repository_storage['path']} -mindepth 2 -maxdepth 2 -type d -name *.git)) do |find| + IO.popen(%W(find #{repository_storage['path']} -mindepth 2 -type d -name *.git)) do |find| find.each_line do |path| yield path.chomp end From f8e1b44dc5d2a78676672dfc7d44c17e6defeda6 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 3 Jan 2018 14:51:04 +0100 Subject: [PATCH 14/33] add locks chek --- lib/tasks/gitlab/git.rake | 26 +++++++++++++++++++++++++- spec/tasks/gitlab/git_rake_spec.rb | 4 +++- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/lib/tasks/gitlab/git.rake b/lib/tasks/gitlab/git.rake index f3ffff43726..5c1b19860f0 100644 --- a/lib/tasks/gitlab/git.rake +++ b/lib/tasks/gitlab/git.rake @@ -32,7 +32,10 @@ namespace :gitlab do desc 'GitLab | Git | Check all repos integrity' task fsck: :environment do - failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} fsck --name-objects --no-progress), "Checking integrity") + failures = perform_git_cmd(%W(#{Gitlab.config.git.bin_path} fsck --name-objects --no-progress), "Checking integrity") do |repo| + check_config_lock(repo) + check_ref_locks(repo) + end if failures.empty? puts "Done".color(:green) else @@ -50,6 +53,8 @@ namespace :gitlab do else failures << repo end + + yield(repo) if block_given? end failures @@ -59,5 +64,24 @@ namespace :gitlab do puts "The following repositories reported errors:".color(:red) failures.each { |f| puts "- #{f}" } end + + def check_config_lock(repo_dir) + config_exists = File.exist?(File.join(repo_dir, 'config.lock')) + config_output = config_exists ? 'yes'.color(:red) : 'no'.color(:green) + + puts "'config.lock' file exists?".color(:yellow) + " ... #{config_output}" + end + + def check_ref_locks(repo_dir) + lock_files = Dir.glob(File.join(repo_dir, 'refs/heads/*.lock')) + + if lock_files.present? + puts "Ref lock files exist:".color(:red) + + lock_files.each { |lock_file| puts " #{lock_file}" } + else + puts "No ref lock files exist".color(:green) + end + end end end diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb index 60b51186ceb..19d298fb36d 100644 --- a/spec/tasks/gitlab/git_rake_spec.rb +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -1,3 +1,5 @@ + + require 'rake_helper' describe 'gitlab:git rake tasks' do @@ -6,7 +8,7 @@ describe 'gitlab:git rake tasks' do storages = { 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage') } } - FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/repo/test.git')) + FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@repo/1/2/test.git')) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) stub_warn_user_is_not_gitlab end From 5b9e7773766eebbe73bb400025de002962532a7c Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 3 Jan 2018 15:32:16 +0100 Subject: [PATCH 15/33] add lock specs --- spec/tasks/gitlab/git_rake_spec.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb index 19d298fb36d..44a2607bea2 100644 --- a/spec/tasks/gitlab/git_rake_spec.rb +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -21,5 +21,18 @@ describe 'gitlab:git rake tasks' do it 'outputs the right git command' do expect { run_rake_task('gitlab:git:fsck') }.to output(/Performed Checking integrity/).to_stdout end + + it 'errors out about config.lock issues' do + FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@repo/1/2/test.git/config.lock')) + + expect { run_rake_task('gitlab:git:fsck') }.to output(/file exists\? ... yes/).to_stdout + end + + it 'errors out about ref lock issues' do + FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@repo/1/2/test.git/refs/heads')) + FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@repo/1/2/test.git/refs/heads/blah.lock')) + + expect { run_rake_task('gitlab:git:fsck') }.to output(/Ref lock files exist:/).to_stdout + end end end From 6ee122c04ee8263dc1cb9dfddd010c5c0b587e8e Mon Sep 17 00:00:00 2001 From: James Lopez Date: Wed, 3 Jan 2018 16:11:17 +0100 Subject: [PATCH 16/33] deprecate check integrity task --- lib/tasks/gitlab/check.rake | 41 ++----------------------------------- 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/lib/tasks/gitlab/check.rake b/lib/tasks/gitlab/check.rake index dfade1f3885..903e84359cd 100644 --- a/lib/tasks/gitlab/check.rake +++ b/lib/tasks/gitlab/check.rake @@ -387,14 +387,8 @@ namespace :gitlab do namespace :repo do desc "GitLab | Check the integrity of the repositories managed by GitLab" task check: :environment do - Gitlab.config.repositories.storages.each do |name, repository_storage| - namespace_dirs = Dir.glob(File.join(repository_storage['path'], '*')) - - namespace_dirs.each do |namespace_dir| - repo_dirs = Dir.glob(File.join(namespace_dir, '*')) - repo_dirs.each { |repo_dir| check_repo_integrity(repo_dir) } - end - end + puts "This task is deprecated. Please use gitlab:git:fsck instead".color(:red) + Rake::Task["gitlab:git:fsck"].execute end end @@ -461,35 +455,4 @@ namespace :gitlab do puts "FAIL. Please update gitlab-shell to #{required_version} from #{current_version}".color(:red) end end - - def check_repo_integrity(repo_dir) - puts "\nChecking repo at #{repo_dir.color(:yellow)}" - - git_fsck(repo_dir) - check_config_lock(repo_dir) - check_ref_locks(repo_dir) - end - - def git_fsck(repo_dir) - puts "Running `git fsck`".color(:yellow) - system(*%W(#{Gitlab.config.git.bin_path} fsck), chdir: repo_dir) - end - - def check_config_lock(repo_dir) - config_exists = File.exist?(File.join(repo_dir, 'config.lock')) - config_output = config_exists ? 'yes'.color(:red) : 'no'.color(:green) - puts "'config.lock' file exists?".color(:yellow) + " ... #{config_output}" - end - - def check_ref_locks(repo_dir) - lock_files = Dir.glob(File.join(repo_dir, 'refs/heads/*.lock')) - if lock_files.present? - puts "Ref lock files exist:".color(:red) - lock_files.each do |lock_file| - puts " #{lock_file}" - end - else - puts "No ref lock files exist".color(:green) - end - end end From de36a8e27961d4c2af43d0ac2d700a391c245353 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 4 Jan 2018 11:02:43 +0100 Subject: [PATCH 17/33] refactor spec, add docs --- doc/administration/raketasks/check.md | 4 ++-- lib/tasks/gitlab/git.rake | 1 + spec/tasks/gitlab/git_rake_spec.rb | 16 ++++++++-------- 3 files changed, 11 insertions(+), 10 deletions(-) diff --git a/doc/administration/raketasks/check.md b/doc/administration/raketasks/check.md index c8b5434c068..7dabc014bad 100644 --- a/doc/administration/raketasks/check.md +++ b/doc/administration/raketasks/check.md @@ -34,13 +34,13 @@ This task loops through all repositories on the GitLab server and runs the **Omnibus Installation** ``` -sudo gitlab-rake gitlab:repo:check +sudo gitlab-rake gitlab:git:fsck ``` **Source Installation** ```bash -sudo -u git -H bundle exec rake gitlab:repo:check RAILS_ENV=production +sudo -u git -H bundle exec rake gitlab:git:fsck RAILS_ENV=production ``` ### Check repositories for a specific user diff --git a/lib/tasks/gitlab/git.rake b/lib/tasks/gitlab/git.rake index 5c1b19860f0..3f5dd2ae3b3 100644 --- a/lib/tasks/gitlab/git.rake +++ b/lib/tasks/gitlab/git.rake @@ -36,6 +36,7 @@ namespace :gitlab do check_config_lock(repo) check_ref_locks(repo) end + if failures.empty? puts "Done".color(:green) else diff --git a/spec/tasks/gitlab/git_rake_spec.rb b/spec/tasks/gitlab/git_rake_spec.rb index 44a2607bea2..dacc5dc5ae7 100644 --- a/spec/tasks/gitlab/git_rake_spec.rb +++ b/spec/tasks/gitlab/git_rake_spec.rb @@ -1,5 +1,3 @@ - - require 'rake_helper' describe 'gitlab:git rake tasks' do @@ -8,8 +6,10 @@ describe 'gitlab:git rake tasks' do storages = { 'default' => { 'path' => Settings.absolute('tmp/tests/default_storage') } } - FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@repo/1/2/test.git')) + FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git')) allow(Gitlab.config.repositories).to receive(:storages).and_return(storages) + allow_any_instance_of(String).to receive(:color) { |string, _color| string } + stub_warn_user_is_not_gitlab end @@ -18,19 +18,19 @@ describe 'gitlab:git rake tasks' do end describe 'fsck' do - it 'outputs the right git command' do - expect { run_rake_task('gitlab:git:fsck') }.to output(/Performed Checking integrity/).to_stdout + it 'outputs the integrity check for a repo' do + expect { run_rake_task('gitlab:git:fsck') }.to output(/Performed Checking integrity at .*@hashed\/1\/2\/test.git/).to_stdout end it 'errors out about config.lock issues' do - FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@repo/1/2/test.git/config.lock')) + FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/config.lock')) expect { run_rake_task('gitlab:git:fsck') }.to output(/file exists\? ... yes/).to_stdout end it 'errors out about ref lock issues' do - FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@repo/1/2/test.git/refs/heads')) - FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@repo/1/2/test.git/refs/heads/blah.lock')) + FileUtils.mkdir_p(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/refs/heads')) + FileUtils.touch(Settings.absolute('tmp/tests/default_storage/@hashed/1/2/test.git/refs/heads/blah.lock')) expect { run_rake_task('gitlab:git:fsck') }.to output(/Ref lock files exist:/).to_stdout end From 21d0a3a6c4ee78724e084f355da9e40c4243b036 Mon Sep 17 00:00:00 2001 From: James Lopez Date: Thu, 4 Jan 2018 11:19:11 +0100 Subject: [PATCH 18/33] add missing changelog --- .../unreleased/40228-verify-integrity-of-repositories.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/40228-verify-integrity-of-repositories.yml diff --git a/changelogs/unreleased/40228-verify-integrity-of-repositories.yml b/changelogs/unreleased/40228-verify-integrity-of-repositories.yml new file mode 100644 index 00000000000..261d48652db --- /dev/null +++ b/changelogs/unreleased/40228-verify-integrity-of-repositories.yml @@ -0,0 +1,5 @@ +--- +title: Fix gitlab-rake gitlab:import:repos import schedule +merge_request: 15931 +author: +type: fixed From 0ba0f9de08eb3d5113f4557b925506167484950a Mon Sep 17 00:00:00 2001 From: Ahmad Sherif Date: Wed, 3 Jan 2018 13:31:06 +0100 Subject: [PATCH 19/33] Prepare Gitlab::Git::Repository#rebase for Gitaly migration --- lib/gitlab/git/operation_service.rb | 5 +++++ lib/gitlab/git/repository.rb | 9 ++++++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb index ef5bdbaf819..3fb0e2eed93 100644 --- a/lib/gitlab/git/operation_service.rb +++ b/lib/gitlab/git/operation_service.rb @@ -97,6 +97,11 @@ module Gitlab end end + def update_branch(branch_name, newrev, oldrev) + ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name + update_ref_in_hooks(ref, newrev, oldrev) + end + private # Returns [newrev, should_run_after_create, should_run_after_create_branch] diff --git a/lib/gitlab/git/repository.rb b/lib/gitlab/git/repository.rb index 176bd953ca1..7c6349f4e84 100644 --- a/lib/gitlab/git/repository.rb +++ b/lib/gitlab/git/repository.rb @@ -1212,9 +1212,16 @@ module Gitlab rebase_path = worktree_path(REBASE_WORKTREE_PREFIX, rebase_id) env = git_env_for_user(user) + if remote_repository.is_a?(RemoteRepository) + env.merge!(remote_repository.fetch_env) + remote_repo_path = GITALY_INTERNAL_URL + else + remote_repo_path = remote_repository.path + end + with_worktree(rebase_path, branch, env: env) do run_git!( - %W(pull --rebase #{remote_repository.path} #{remote_branch}), + %W(pull --rebase #{remote_repo_path} #{remote_branch}), chdir: rebase_path, env: env ) From b5fe3916752aafd5c79b2aa7a770fbd51f1b4bef Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 4 Jan 2018 15:44:00 +0100 Subject: [PATCH 20/33] Update some Gitaly annotations in Gitlab::Shell --- lib/gitlab/shell.rb | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/lib/gitlab/shell.rb b/lib/gitlab/shell.rb index 9cdd3d22f18..18da242e1cb 100644 --- a/lib/gitlab/shell.rb +++ b/lib/gitlab/shell.rb @@ -71,7 +71,6 @@ module Gitlab # Ex. # add_repository("/path/to/storage", "gitlab/gitlab-ci") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def add_repository(storage, name) relative_path = name.dup relative_path << '.git' unless relative_path.end_with?('.git') @@ -100,7 +99,7 @@ module Gitlab # Ex. # import_repository("/path/to/storage", "gitlab/gitlab-ci", "https://gitlab.com/gitlab-org/gitlab-test.git") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/874 def import_repository(storage, name, url) # The timeout ensures the subprocess won't hang forever cmd = gitlab_projects(storage, "#{name}.git") @@ -122,7 +121,6 @@ module Gitlab # Ex. # fetch_remote(my_repo, "upstream") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 def fetch_remote(repository, remote, ssh_auth: nil, forced: false, no_tags: false) gitaly_migrate(:fetch_remote) do |is_enabled| if is_enabled @@ -142,7 +140,7 @@ module Gitlab # Ex. # mv_repository("/path/to/storage", "gitlab/gitlab-ci", "randx/gitlab-ci-new") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873 def mv_repository(storage, path, new_path) gitlab_projects(storage, "#{path}.git").mv_project("#{new_path}.git") end @@ -156,7 +154,7 @@ module Gitlab # Ex. # fork_repository("/path/to/forked_from/storage", "gitlab/gitlab-ci", "/path/to/forked_to/storage", "new-namespace/gitlab-ci") # - # Gitaly note: JV: not easy to migrate because this involves two Gitaly servers, not one. + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/817 def fork_repository(forked_from_storage, forked_from_disk_path, forked_to_storage, forked_to_disk_path) gitlab_projects(forked_from_storage, "#{forked_from_disk_path}.git") .fork_repository(forked_to_storage, "#{forked_to_disk_path}.git") @@ -170,7 +168,7 @@ module Gitlab # Ex. # remove_repository("/path/to/storage", "gitlab/gitlab-ci") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/387 + # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/873 def remove_repository(storage, name) gitlab_projects(storage, "#{name}.git").rm_project end @@ -221,7 +219,6 @@ module Gitlab # Ex. # add_namespace("/path/to/storage", "gitlab") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def add_namespace(storage, name) Gitlab::GitalyClient.migrate(:add_namespace) do |enabled| if enabled @@ -243,7 +240,6 @@ module Gitlab # Ex. # rm_namespace("/path/to/storage", "gitlab") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def rm_namespace(storage, name) Gitlab::GitalyClient.migrate(:remove_namespace) do |enabled| if enabled @@ -261,7 +257,6 @@ module Gitlab # Ex. # mv_namespace("/path/to/storage", "gitlab", "gitlabhq") # - # Gitaly migration: https://gitlab.com/gitlab-org/gitaly/issues/385 def mv_namespace(storage, old_name, new_name) Gitlab::GitalyClient.migrate(:rename_namespace) do |enabled| if enabled From 3df6fa6c05ad86f1bc861a8f38d8096098110e37 Mon Sep 17 00:00:00 2001 From: Kushal Pandya Date: Thu, 4 Jan 2018 21:33:25 +0530 Subject: [PATCH 21/33] Enclose props in quotes --- app/assets/javascripts/groups/components/item_stats.vue | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/groups/components/item_stats.vue b/app/assets/javascripts/groups/components/item_stats.vue index 803dc63d39c..3a94e57a028 100644 --- a/app/assets/javascripts/groups/components/item_stats.vue +++ b/app/assets/javascripts/groups/components/item_stats.vue @@ -43,27 +43,27 @@ export default { css-class="number-subgroups" icon-name="folder" :title="s__('Subgroups')" - :value=item.subgroupCount + :value="item.subgroupCount" /> Date: Thu, 4 Jan 2018 21:47:40 +0530 Subject: [PATCH 22/33] Use `__` instead of `s__` when context is not required --- app/assets/javascripts/groups/components/item_stats.vue | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/assets/javascripts/groups/components/item_stats.vue b/app/assets/javascripts/groups/components/item_stats.vue index 3a94e57a028..2e42fb6c9a6 100644 --- a/app/assets/javascripts/groups/components/item_stats.vue +++ b/app/assets/javascripts/groups/components/item_stats.vue @@ -42,21 +42,21 @@ export default { v-if="isGroup" css-class="number-subgroups" icon-name="folder" - :title="s__('Subgroups')" + :title="__('Subgroups')" :value="item.subgroupCount" /> Date: Thu, 4 Jan 2018 22:36:11 +0100 Subject: [PATCH 23/33] Ignore the Migration/Datetime cop in a migration that fix a column type to datetime_with_timezone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- .../20171221140220_schedule_issues_closed_at_type_change.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/post_migrate/20171221140220_schedule_issues_closed_at_type_change.rb b/db/post_migrate/20171221140220_schedule_issues_closed_at_type_change.rb index be18c5866ae..eeecc7b1de0 100644 --- a/db/post_migrate/20171221140220_schedule_issues_closed_at_type_change.rb +++ b/db/post_migrate/20171221140220_schedule_issues_closed_at_type_change.rb @@ -1,6 +1,6 @@ # See http://doc.gitlab.com/ce/development/migration_style_guide.html # for more information on how to write migrations for GitLab. - +# rubocop:disable Migration/Datetime class ScheduleIssuesClosedAtTypeChange < ActiveRecord::Migration include Gitlab::Database::MigrationHelpers From 5152cc3bfb8d60814063e86c3776030aa8891e0b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Rodr=C3=ADguez?= Date: Thu, 4 Jan 2018 19:27:37 -0300 Subject: [PATCH 24/33] Fix a bug where charlock_holmes was used needlessly to encode strings --- lib/gitlab/encoding_helper.rb | 26 +++++++++++++++---------- lib/gitlab/git.rb | 2 +- spec/lib/gitlab/encoding_helper_spec.rb | 18 +++++++++++++++++ 3 files changed, 35 insertions(+), 11 deletions(-) diff --git a/lib/gitlab/encoding_helper.rb b/lib/gitlab/encoding_helper.rb index 6b53eb4533d..c0edcabc6fd 100644 --- a/lib/gitlab/encoding_helper.rb +++ b/lib/gitlab/encoding_helper.rb @@ -14,14 +14,7 @@ module Gitlab ENCODING_CONFIDENCE_THRESHOLD = 50 def encode!(message) - return nil unless message.respond_to?(:force_encoding) - return message if message.encoding == Encoding::UTF_8 && message.valid_encoding? - - if message.respond_to?(:frozen?) && message.frozen? - message = message.dup - end - - message.force_encoding("UTF-8") + message = force_encode_utf8(message) return message if message.valid_encoding? # return message if message type is binary @@ -35,6 +28,8 @@ module Gitlab # encode and clean the bad chars message.replace clean(message) + rescue ArgumentError + return nil rescue encoding = detect ? detect[:encoding] : "unknown" "--broken encoding: #{encoding}" @@ -54,8 +49,8 @@ module Gitlab end def encode_utf8(message) - return nil unless message.is_a?(String) - return message if message.encoding == Encoding::UTF_8 && message.valid_encoding? + message = force_encode_utf8(message) + return message if message.valid_encoding? detect = CharlockHolmes::EncodingDetector.detect(message) if detect && detect[:encoding] @@ -69,6 +64,8 @@ module Gitlab else clean(message) end + rescue ArgumentError + return nil end def encode_binary(s) @@ -83,6 +80,15 @@ module Gitlab private + def force_encode_utf8(message) + raise ArgumentError unless message.respond_to?(:force_encoding) + return message if message.encoding == Encoding::UTF_8 && message.valid_encoding? + + message = message.dup if message.respond_to?(:frozen?) && message.frozen? + + message.force_encoding("UTF-8") + end + def clean(message) message.encode("UTF-16BE", undef: :replace, invalid: :replace, replace: "") .encode("UTF-8") diff --git a/lib/gitlab/git.rb b/lib/gitlab/git.rb index 1f7c35cafaa..71647099f83 100644 --- a/lib/gitlab/git.rb +++ b/lib/gitlab/git.rb @@ -11,7 +11,7 @@ module Gitlab include Gitlab::EncodingHelper def ref_name(ref) - encode_utf8(ref).sub(/\Arefs\/(tags|heads|remotes)\//, '') + encode!(ref).sub(/\Arefs\/(tags|heads|remotes)\//, '') end def branch_name(ref) diff --git a/spec/lib/gitlab/encoding_helper_spec.rb b/spec/lib/gitlab/encoding_helper_spec.rb index 87ec2698fc1..4e9367323cb 100644 --- a/spec/lib/gitlab/encoding_helper_spec.rb +++ b/spec/lib/gitlab/encoding_helper_spec.rb @@ -120,6 +120,24 @@ describe Gitlab::EncodingHelper do it 'returns empty string on conversion errors' do expect { ext_class.encode_utf8('') }.not_to raise_error(ArgumentError) end + + context 'with strings that can be forcefully encoded into utf8' do + let(:test_string) do + "refs/heads/FixSymbolsTitleDropdown".encode("ASCII-8BIT") + end + let(:expected_string) do + "refs/heads/FixSymbolsTitleDropdown".encode("UTF-8") + end + + subject { ext_class.encode_utf8(test_string) } + + it "doesn't use CharlockHolmes if the encoding can be forced into utf_8" do + expect(CharlockHolmes::EncodingDetector).not_to receive(:detect) + + expect(subject).to eq(expected_string) + expect(subject.encoding.name).to eq('UTF-8') + end + end end describe '#clean' do From 1859dc867092c1ba31335cf67047c9b6141466b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Matija=20=C4=8Cupi=C4=87?= Date: Fri, 5 Jan 2018 00:02:26 +0100 Subject: [PATCH 25/33] Add back bottom margins for integration form --- app/views/projects/clusters/_integration_form.html.haml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/projects/clusters/_integration_form.html.haml b/app/views/projects/clusters/_integration_form.html.haml index 1eac2c9dc1f..9d593ffc021 100644 --- a/app/views/projects/clusters/_integration_form.html.haml +++ b/app/views/projects/clusters/_integration_form.html.haml @@ -1,6 +1,6 @@ = form_for @cluster, url: namespace_project_cluster_path(@project.namespace, @project, @cluster), as: :cluster do |field| = form_errors(@cluster) - .form-group + .form-group.append-bottom-20 %h5= s_('ClusterIntegration|Integration status') %p - if @cluster.enabled? @@ -10,7 +10,7 @@ = s_('ClusterIntegration|Cluster integration is enabled for this project.') - else = s_('ClusterIntegration|Cluster integration is disabled for this project.') - %label + %label.append-bottom-10 = field.hidden_field :enabled, { class: 'js-toggle-input'} %button{ type: 'button', From 7d1fdcdc836921d2f2324265752107519f47a6de Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Wed, 8 Nov 2017 15:32:12 -0600 Subject: [PATCH 26/33] Modify `LDAP::Person` to return username value based on attributes `Gitlab::LDAP::Person` did not respect the LDAP attributes username configuration and would simply return the uid value. There are cases where users would like to specify a different username field to allow more friendly GitLab usernames. For example, it's common in AD to have sAMAccountName be an employee ID like `A12345` while the local part of the email address is more human-friendly. --- .../unreleased/ldap_username_attributes.yml | 5 ++ lib/gitlab/ldap/adapter.rb | 2 +- lib/gitlab/ldap/config.rb | 2 +- lib/gitlab/ldap/person.rb | 36 +++++++-- spec/lib/gitlab/ldap/adapter_spec.rb | 10 ++- spec/lib/gitlab/ldap/person_spec.rb | 73 ++++++++++++++++++- spec/lib/gitlab/o_auth/user_spec.rb | 20 +++++ 7 files changed, 135 insertions(+), 13 deletions(-) create mode 100644 changelogs/unreleased/ldap_username_attributes.yml diff --git a/changelogs/unreleased/ldap_username_attributes.yml b/changelogs/unreleased/ldap_username_attributes.yml new file mode 100644 index 00000000000..89bbca58fc9 --- /dev/null +++ b/changelogs/unreleased/ldap_username_attributes.yml @@ -0,0 +1,5 @@ +--- +title: Modify `LDAP::Person` to return username value based on attributes +merge_request: +author: +type: fixed diff --git a/lib/gitlab/ldap/adapter.rb b/lib/gitlab/ldap/adapter.rb index 0afaa2306b5..76863e77dc3 100644 --- a/lib/gitlab/ldap/adapter.rb +++ b/lib/gitlab/ldap/adapter.rb @@ -74,7 +74,7 @@ module Gitlab def user_options(fields, value, limit) options = { - attributes: Gitlab::LDAP::Person.ldap_attributes(config).compact.uniq, + attributes: Gitlab::LDAP::Person.ldap_attributes(config), base: config.base } diff --git a/lib/gitlab/ldap/config.rb b/lib/gitlab/ldap/config.rb index c8f19cd52d5..0d9a554fc18 100644 --- a/lib/gitlab/ldap/config.rb +++ b/lib/gitlab/ldap/config.rb @@ -148,7 +148,7 @@ module Gitlab def default_attributes { - 'username' => %w(uid userid sAMAccountName), + 'username' => %w(uid sAMAccountName userid), 'email' => %w(mail email userPrincipalName), 'name' => 'cn', 'first_name' => 'givenName', diff --git a/lib/gitlab/ldap/person.rb b/lib/gitlab/ldap/person.rb index 38d7a9ba2f5..e81cec6ba1a 100644 --- a/lib/gitlab/ldap/person.rb +++ b/lib/gitlab/ldap/person.rb @@ -6,6 +6,8 @@ module Gitlab # Source: http://ctogonewild.com/2009/09/03/bitmask-searches-in-ldap/ AD_USER_DISABLED = Net::LDAP::Filter.ex("userAccountControl:1.2.840.113556.1.4.803", "2") + InvalidEntryError = Class.new(StandardError) + attr_accessor :entry, :provider def self.find_by_uid(uid, adapter) @@ -29,11 +31,12 @@ module Gitlab def self.ldap_attributes(config) [ - 'dn', # Used in `dn` - config.uid, # Used in `uid` - *config.attributes['name'], # Used in `name` - *config.attributes['email'] # Used in `email` - ] + 'dn', + config.uid, + *config.attributes['name'], + *config.attributes['email'], + *config.attributes['username'] + ].compact.uniq end def self.normalize_dn(dn) @@ -60,6 +63,8 @@ module Gitlab Rails.logger.debug { "Instantiating #{self.class.name} with LDIF:\n#{entry.to_ldif}" } @entry = entry @provider = provider + + validate_entry end def name @@ -71,7 +76,13 @@ module Gitlab end def username - uid + username = attribute_value(:username) + + # Depending on the attribute, multiple values may + # be returned. We need only one for username. + # Ex. `uid` returns only one value but `mail` may + # return an array of multiple email addresses. + [username].flatten.first end def email @@ -104,6 +115,19 @@ module Gitlab entry.public_send(selected_attr) # rubocop:disable GitlabSecurity/PublicSend end + + def validate_entry + allowed_attrs = self.class.ldap_attributes(config).map(&:downcase) + + # Net::LDAP::Entry transforms keys to symbols. Change to strings to compare. + entry_attrs = entry.attribute_names.map { |n| n.to_s.downcase } + invalid_attrs = entry_attrs - allowed_attrs + + if invalid_attrs.any? + raise InvalidEntryError, + "#{self.class.name} initialized with Net::LDAP::Entry containing invalid attributes(s): #{invalid_attrs}" + end + end end end end diff --git a/spec/lib/gitlab/ldap/adapter_spec.rb b/spec/lib/gitlab/ldap/adapter_spec.rb index d9ddb4326be..6132abd9b35 100644 --- a/spec/lib/gitlab/ldap/adapter_spec.rb +++ b/spec/lib/gitlab/ldap/adapter_spec.rb @@ -16,7 +16,7 @@ describe Gitlab::LDAP::Adapter do expect(adapter).to receive(:ldap_search) do |arg| expect(arg[:filter].to_s).to eq('(uid=johndoe)') expect(arg[:base]).to eq('dc=example,dc=com') - expect(arg[:attributes]).to match(%w{dn uid cn mail email userPrincipalName}) + expect(arg[:attributes]).to match(ldap_attributes) end.and_return({}) adapter.users('uid', 'johndoe') @@ -26,7 +26,7 @@ describe Gitlab::LDAP::Adapter do expect(adapter).to receive(:ldap_search).with( base: 'uid=johndoe,ou=users,dc=example,dc=com', scope: Net::LDAP::SearchScope_BaseObject, - attributes: %w{dn uid cn mail email userPrincipalName}, + attributes: ldap_attributes, filter: nil ).and_return({}) @@ -63,7 +63,7 @@ describe Gitlab::LDAP::Adapter do it 'uses the right uid attribute when non-default' do stub_ldap_config(uid: 'sAMAccountName') expect(adapter).to receive(:ldap_search).with( - hash_including(attributes: %w{dn sAMAccountName cn mail email userPrincipalName}) + hash_including(attributes: ldap_attributes) ).and_return({}) adapter.users('sAMAccountName', 'johndoe') @@ -137,4 +137,8 @@ describe Gitlab::LDAP::Adapter do end end end + + def ldap_attributes + Gitlab::LDAP::Person.ldap_attributes(Gitlab::LDAP::Config.new('ldapmain')) + end end diff --git a/spec/lib/gitlab/ldap/person_spec.rb b/spec/lib/gitlab/ldap/person_spec.rb index d204050ef66..ff29d9aa5be 100644 --- a/spec/lib/gitlab/ldap/person_spec.rb +++ b/spec/lib/gitlab/ldap/person_spec.rb @@ -8,13 +8,16 @@ describe Gitlab::LDAP::Person do before do stub_ldap_config( options: { + 'uid' => 'uid', 'attributes' => { - 'name' => 'cn', - 'email' => %w(mail email userPrincipalName) + 'name' => 'cn', + 'email' => %w(mail email userPrincipalName), + 'username' => username_attribute } } ) end + let(:username_attribute) { %w(uid sAMAccountName userid) } describe '.normalize_dn' do subject { described_class.normalize_dn(given) } @@ -44,6 +47,34 @@ describe Gitlab::LDAP::Person do end end + describe '.ldap_attributes' do + it 'returns a compact and unique array' do + stub_ldap_config( + options: { + 'uid' => nil, + 'attributes' => { + 'name' => 'cn', + 'email' => 'mail', + 'username' => %w(uid mail memberof) + } + } + ) + config = Gitlab::LDAP::Config.new('ldapmain') + ldap_attributes = described_class.ldap_attributes(config) + + expect(ldap_attributes).to match_array(%w(dn uid cn mail memberof)) + end + end + + describe '.validate_entry' do + it 'raises InvalidEntryError' do + entry['foo'] = 'bar' + + expect { described_class.new(entry, 'ldapmain') } + .to raise_error(Gitlab::LDAP::Person::InvalidEntryError) + end + end + describe '#name' do it 'uses the configured name attribute and handles values as an array' do name = 'John Doe' @@ -72,6 +103,44 @@ describe Gitlab::LDAP::Person do end end + describe '#username' do + context 'with default uid username attribute' do + let(:username_attribute) { 'uid' } + + it 'returns the proper username value' do + attr_value = 'johndoe' + entry[username_attribute] = attr_value + person = described_class.new(entry, 'ldapmain') + + expect(person.username).to eq(attr_value) + end + end + + context 'with a different username attribute' do + let(:username_attribute) { 'sAMAccountName' } + + it 'returns the proper username value' do + attr_value = 'johndoe' + entry[username_attribute] = attr_value + person = described_class.new(entry, 'ldapmain') + + expect(person.username).to eq(attr_value) + end + end + + context 'with a non-standard username attribute' do + let(:username_attribute) { 'mail' } + + it 'returns the proper username value' do + attr_value = 'john.doe@example.com' + entry[username_attribute] = attr_value + person = described_class.new(entry, 'ldapmain') + + expect(person.username).to eq(attr_value) + end + end + end + def assert_generic_test(test_description, got, expected) test_failure_message = "Failed test description: '#{test_description}'\n\n expected: #{expected}\n got: #{got}" expect(got).to eq(expected), test_failure_message diff --git a/spec/lib/gitlab/o_auth/user_spec.rb b/spec/lib/gitlab/o_auth/user_spec.rb index 6334bcd0156..45fff4c5787 100644 --- a/spec/lib/gitlab/o_auth/user_spec.rb +++ b/spec/lib/gitlab/o_auth/user_spec.rb @@ -275,6 +275,26 @@ describe Gitlab::OAuth::User do end end + context 'and a corresponding LDAP person with a non-default username' do + before do + allow(ldap_user).to receive(:uid) { uid } + allow(ldap_user).to receive(:username) { 'johndoe@example.com' } + allow(ldap_user).to receive(:email) { %w(johndoe@example.com john2@example.com) } + allow(ldap_user).to receive(:dn) { dn } + end + + context 'and no account for the LDAP user' do + it 'creates a user favoring the LDAP username and strips email domain' do + allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(ldap_user) + + oauth_user.save + + expect(gl_user).to be_valid + expect(gl_user.username).to eql 'johndoe' + end + end + end + context "and no corresponding LDAP person" do before do allow(Gitlab::LDAP::Person).to receive(:find_by_uid).and_return(nil) From 8b3b28b8d81acc719701a3c2bfc05b6f7c22c4f2 Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Fri, 5 Jan 2018 15:32:31 +0800 Subject: [PATCH 27/33] Just try to detect and assign once --- app/models/concerns/deployment_platform.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/app/models/concerns/deployment_platform.rb b/app/models/concerns/deployment_platform.rb index e1373455e98..89d0474a596 100644 --- a/app/models/concerns/deployment_platform.rb +++ b/app/models/concerns/deployment_platform.rb @@ -1,8 +1,9 @@ module DeploymentPlatform def deployment_platform - @deployment_platform ||= find_cluster_platform_kubernetes - @deployment_platform ||= find_kubernetes_service_integration - @deployment_platform ||= build_cluster_and_deployment_platform + @deployment_platform ||= + find_cluster_platform_kubernetes || + find_kubernetes_service_integration || + build_cluster_and_deployment_platform end private From 6b15784ce7d893ed509c00d9f51a4702787799ed Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Fri, 5 Jan 2018 09:41:05 +0000 Subject: [PATCH 28/33] Fix typos in a code comment --- lib/gitlab/git/blob.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/gitlab/git/blob.rb b/lib/gitlab/git/blob.rb index bd91125d3b6..a1755143abe 100644 --- a/lib/gitlab/git/blob.rb +++ b/lib/gitlab/git/blob.rb @@ -138,8 +138,8 @@ module Gitlab # Gitaly will think that setting the limit to 0 means unlimited, while # the client might only need the metadata and thus set the limit to 0. - # In this method we'll than set the limit to 1, but clear the byte of data - # that we got back so fot the outside world it looks like the limit was + # In this method we'll then set the limit to 1, but clear the byte of data + # that we got back so for the outside world it looks like the limit was # actually 0. req_limit = limit == 0 ? 1 : limit From 3514b7248cf00bcee8a6b3133e4e157f656d30c6 Mon Sep 17 00:00:00 2001 From: Alessio Caiazza Date: Tue, 13 Jun 2017 22:03:34 +0200 Subject: [PATCH 29/33] Add status attribute to runner api entity --- .../unreleased/feature-api_runners_online.yml | 5 ++-- doc/api/runners.md | 23 ++++++++++++++----- lib/api/entities.rb | 1 + 3 files changed, 21 insertions(+), 8 deletions(-) diff --git a/changelogs/unreleased/feature-api_runners_online.yml b/changelogs/unreleased/feature-api_runners_online.yml index f5077507e5b..08f4dd16f28 100644 --- a/changelogs/unreleased/feature-api_runners_online.yml +++ b/changelogs/unreleased/feature-api_runners_online.yml @@ -1,4 +1,5 @@ --- -title: Add online attribute to runner api entity +title: Add online and status attribute to runner api entity merge_request: 11750 -author: Alessio Caiazza +author: +type: added diff --git a/doc/api/runners.md b/doc/api/runners.md index 50981ed96bc..7495c6cdedb 100644 --- a/doc/api/runners.md +++ b/doc/api/runners.md @@ -31,7 +31,8 @@ Example response: "id": 6, "is_shared": false, "name": null, - "online": true + "online": true, + "status": "online" }, { "active": true, @@ -39,7 +40,8 @@ Example response: "id": 8, "is_shared": false, "name": null, - "online": false + "online": false, + "status": "offline" } ] ``` @@ -72,7 +74,8 @@ Example response: "id": 1, "is_shared": true, "name": null, - "online": true + "online": true, + "status": "online" }, { "active": true, @@ -81,6 +84,7 @@ Example response: "is_shared": true, "name": null, "online": false + "status": "offline" }, { "active": true, @@ -89,6 +93,7 @@ Example response: "is_shared": false, "name": null, "online": true + "status": "paused" }, { "active": true, @@ -96,7 +101,8 @@ Example response: "id": 8, "is_shared": false, "name": null, - "online": false + "online": false, + "status": "offline" } ] ``` @@ -129,6 +135,7 @@ Example response: "contacted_at": "2016-01-25T16:39:48.066Z", "name": null, "online": true, + "status": "online", "platform": null, "projects": [ { @@ -184,6 +191,7 @@ Example response: "contacted_at": "2016-01-25T16:39:48.066Z", "name": null, "online": true, + "status": "online", "platform": null, "projects": [ { @@ -336,7 +344,8 @@ Example response: "id": 8, "is_shared": false, "name": null, - "online": false + "online": false, + "status": "offline" }, { "active": true, @@ -345,6 +354,7 @@ Example response: "is_shared": true, "name": null, "online": true + "status": "paused" } ] ``` @@ -375,7 +385,8 @@ Example response: "id": 9, "is_shared": false, "name": null, - "online": true + "online": true, + "status": "online" } ``` diff --git a/lib/api/entities.rb b/lib/api/entities.rb index c612dde7f73..f5fa5fef389 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -863,6 +863,7 @@ module API expose :is_shared expose :name expose :online?, as: :online + expose :status end class RunnerDetails < Runner From 13926246020fb41599e1a7b908912bb2e61f114f Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 5 Jan 2018 11:16:18 +0100 Subject: [PATCH 30/33] add deprecation and removal issue to docs --- doc/administration/raketasks/check.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/doc/administration/raketasks/check.md b/doc/administration/raketasks/check.md index 7dabc014bad..831b73237b6 100644 --- a/doc/administration/raketasks/check.md +++ b/doc/administration/raketasks/check.md @@ -28,6 +28,12 @@ exactly which repositories are causing the trouble. ### Check all GitLab repositories +>**Note:** +> +> - `gitlab:repo:check` has been deprecated in favour of `gitlab:git:fsck` +> - [Deprecated][ce-15931] in GitLab 10.4. +> - `gitlab:repo:check` will be removed in the future. [Removal issue][ce-41699] + This task loops through all repositories on the GitLab server and runs the 3 integrity checks described previously. @@ -76,3 +82,6 @@ The LDAP check Rake task will test the bind_dn and password credentials (if configured) and will list a sample of LDAP users. This task is also executed as part of the `gitlab:check` task, but can run independently. See [LDAP Rake Tasks - LDAP Check](ldap.md#check) for details. + +[ce-15931]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/15931 +[ce-41699]: https://gitlab.com/gitlab-org/gitlab-ce/issues/41699 From 0feb2437e1318fa1b3157bf322d6759bb32bc01a Mon Sep 17 00:00:00 2001 From: James Lopez Date: Fri, 5 Jan 2018 10:30:57 +0000 Subject: [PATCH 31/33] Update check.md --- doc/administration/raketasks/check.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/administration/raketasks/check.md b/doc/administration/raketasks/check.md index 831b73237b6..c39cb49b1c6 100644 --- a/doc/administration/raketasks/check.md +++ b/doc/administration/raketasks/check.md @@ -30,7 +30,7 @@ exactly which repositories are causing the trouble. >**Note:** > -> - `gitlab:repo:check` has been deprecated in favour of `gitlab:git:fsck` +> - `gitlab:repo:check` has been deprecated in favor of `gitlab:git:fsck` > - [Deprecated][ce-15931] in GitLab 10.4. > - `gitlab:repo:check` will be removed in the future. [Removal issue][ce-41699] From c5e2c0665fe7e4937689cfedaa064aa64f538c8b Mon Sep 17 00:00:00 2001 From: "Jacob Vosmaer (GitLab)" Date: Fri, 5 Jan 2018 11:31:12 +0000 Subject: [PATCH 32/33] Allow local tests to use a modified Gitaly --- doc/development/gitaly.md | 23 ++++++++++++++ lib/gitlab/setup_helper.rb | 61 ++++++++++++++++++++++++++++++++++++ lib/tasks/gitlab/gitaly.rake | 57 ++------------------------------- spec/support/test_env.rb | 7 +++++ 4 files changed, 93 insertions(+), 55 deletions(-) create mode 100644 lib/gitlab/setup_helper.rb diff --git a/doc/development/gitaly.md b/doc/development/gitaly.md index ca2048c7019..26abf967dcf 100644 --- a/doc/development/gitaly.md +++ b/doc/development/gitaly.md @@ -97,6 +97,29 @@ describe 'Gitaly Request count tests' do end ``` +## Running tests with a locally modified version of Gitaly + +Normally, gitlab-ce/ee tests use a local clone of Gitaly in `tmp/tests/gitaly` +pinned at the version specified in GITALY_SERVER_VERSION. If you want +to run tests locally against a modified version of Gitaly you can +replace `tmp/tests/gitaly` with a symlink. + +```shell +rm -rf tmp/tests/gitaly +ln -s /path/to/gitaly tmp/tests/gitaly +``` + +Make sure you run `make` in your local Gitaly directory before running +tests. Otherwise, Gitaly will fail to boot. + +If you make changes to your local Gitaly in between test runs you need +to manually run `make` again. + +Note that CI tests will not use your locally modified version of +Gitaly. To use a custom Gitaly version in CI you need to update +GITALY_SERVER_VERSION. You can use the format `=revision` to use a +non-tagged commit from https://gitlab.com/gitlab-org/gitaly in CI. + --- [Return to Development documentation](README.md) diff --git a/lib/gitlab/setup_helper.rb b/lib/gitlab/setup_helper.rb new file mode 100644 index 00000000000..d01213bb6e0 --- /dev/null +++ b/lib/gitlab/setup_helper.rb @@ -0,0 +1,61 @@ +module Gitlab + module SetupHelper + class << self + # We cannot create config.toml files for all possible Gitaly configuations. + # For instance, if Gitaly is running on another machine then it makes no + # sense to write a config.toml file on the current machine. This method will + # only generate a configuration for the most common and simplest case: when + # we have exactly one Gitaly process and we are sure it is running locally + # because it uses a Unix socket. + # For development and testing purposes, an extra storage is added to gitaly, + # which is not known to Rails, but must be explicitly stubbed. + def gitaly_configuration_toml(gitaly_dir, gitaly_ruby: true) + storages = [] + address = nil + + Gitlab.config.repositories.storages.each do |key, val| + if address + if address != val['gitaly_address'] + raise ArgumentError, "Your gitlab.yml contains more than one gitaly_address." + end + elsif URI(val['gitaly_address']).scheme != 'unix' + raise ArgumentError, "Automatic config.toml generation only supports 'unix:' addresses." + else + address = val['gitaly_address'] + end + + storages << { name: key, path: val['path'] } + end + + if Rails.env.test? + storages << { name: 'test_second_storage', path: Rails.root.join('tmp', 'tests', 'second_storage').to_s } + end + + config = { socket_path: address.sub(%r{\Aunix:}, ''), storage: storages } + config[:auth] = { token: 'secret' } if Rails.env.test? + config[:'gitaly-ruby'] = { dir: File.join(gitaly_dir, 'ruby') } if gitaly_ruby + config[:'gitlab-shell'] = { dir: Gitlab.config.gitlab_shell.path } + config[:bin_dir] = Gitlab.config.gitaly.client_path + + TOML.dump(config) + end + + # rubocop:disable Rails/Output + def create_gitaly_configuration(dir, force: false) + config_path = File.join(dir, 'config.toml') + FileUtils.rm_f(config_path) if force + + File.open(config_path, File::WRONLY | File::CREAT | File::EXCL) do |f| + f.puts gitaly_configuration_toml(dir) + end + rescue Errno::EEXIST + puts "Skipping config.toml generation:" + puts "A configuration file already exists." + rescue ArgumentError => e + puts "Skipping config.toml generation:" + puts e.message + end + # rubocop:enable Rails/Output + end + end +end diff --git a/lib/tasks/gitlab/gitaly.rake b/lib/tasks/gitlab/gitaly.rake index 4d880c05f99..4507b841964 100644 --- a/lib/tasks/gitlab/gitaly.rake +++ b/lib/tasks/gitlab/gitaly.rake @@ -21,8 +21,8 @@ namespace :gitlab do command << 'BUNDLE_FLAGS=--no-deployment' if Rails.env.test? + Gitlab::SetupHelper.create_gitaly_configuration(args.dir) Dir.chdir(args.dir) do - create_gitaly_configuration # In CI we run scripts/gitaly-test-build instead of this command unless ENV['CI'].present? Bundler.with_original_env { run_command!(command) } @@ -39,60 +39,7 @@ namespace :gitlab do # Exclude gitaly-ruby configuration because that depends on the gitaly # installation directory. - puts gitaly_configuration_toml(gitaly_ruby: false) - end - - private - - # We cannot create config.toml files for all possible Gitaly configuations. - # For instance, if Gitaly is running on another machine then it makes no - # sense to write a config.toml file on the current machine. This method will - # only generate a configuration for the most common and simplest case: when - # we have exactly one Gitaly process and we are sure it is running locally - # because it uses a Unix socket. - # For development and testing purposes, an extra storage is added to gitaly, - # which is not known to Rails, but must be explicitly stubbed. - def gitaly_configuration_toml(gitaly_ruby: true) - storages = [] - address = nil - - Gitlab.config.repositories.storages.each do |key, val| - if address - if address != val['gitaly_address'] - raise ArgumentError, "Your gitlab.yml contains more than one gitaly_address." - end - elsif URI(val['gitaly_address']).scheme != 'unix' - raise ArgumentError, "Automatic config.toml generation only supports 'unix:' addresses." - else - address = val['gitaly_address'] - end - - storages << { name: key, path: val['path'] } - end - - if Rails.env.test? - storages << { name: 'test_second_storage', path: Rails.root.join('tmp', 'tests', 'second_storage').to_s } - end - - config = { socket_path: address.sub(%r{\Aunix:}, ''), storage: storages } - config[:auth] = { token: 'secret' } if Rails.env.test? - config[:'gitaly-ruby'] = { dir: File.join(Dir.pwd, 'ruby') } if gitaly_ruby - config[:'gitlab-shell'] = { dir: Gitlab.config.gitlab_shell.path } - config[:bin_dir] = Gitlab.config.gitaly.client_path - - TOML.dump(config) - end - - def create_gitaly_configuration - File.open("config.toml", File::WRONLY | File::CREAT | File::EXCL) do |f| - f.puts gitaly_configuration_toml - end - rescue Errno::EEXIST - puts "Skipping config.toml generation:" - puts "A configuration file already exists." - rescue ArgumentError => e - puts "Skipping config.toml generation:" - puts e.message + puts Gitlab::SetupHelper.gitaly_configuration_toml('', gitaly_ruby: false) end end end diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb index 1d99746b09f..664698fcbaf 100644 --- a/spec/support/test_env.rb +++ b/spec/support/test_env.rb @@ -1,4 +1,5 @@ require 'rspec/mocks' +require 'toml' module TestEnv extend self @@ -147,6 +148,9 @@ module TestEnv version: Gitlab::GitalyClient.expected_server_version, task: "gitlab:gitaly:install[#{gitaly_dir}]") do + # Always re-create config, in case it's outdated. This is fast anyway. + Gitlab::SetupHelper.create_gitaly_configuration(gitaly_dir, force: true) + start_gitaly(gitaly_dir) end end @@ -347,6 +351,9 @@ module TestEnv end def component_needs_update?(component_folder, expected_version) + # Allow local overrides of the component for tests during development + return false if Rails.env.test? && File.symlink?(component_folder) + version = File.read(File.join(component_folder, 'VERSION')).strip # Notice that this will always yield true when using branch versions From 2c47f0924fc5534035905746046ab0f5e9c99f23 Mon Sep 17 00:00:00 2001 From: Winnie Hellmann Date: Wed, 3 Jan 2018 10:21:17 +0100 Subject: [PATCH 33/33] Add id to modal.vue to support data-toggle="modal" --- .../groups/components/item_actions.vue | 6 +- .../ide/components/new_dropdown/index.vue | 8 +- .../ide/components/new_dropdown/modal.vue | 8 +- .../ide/components/repo_commit_section.vue | 2 +- .../ide/components/repo_edit_button.vue | 2 +- .../components/delete_account_modal.vue | 111 +++++++----------- .../javascripts/profile/account/index.js | 8 ++ .../vue_shared/components/modal.vue | 30 +++-- .../vue_shared/components/recaptcha_modal.vue | 2 +- app/views/profiles/accounts/show.html.haml | 6 +- .../unreleased/winh-modal-target-id.yml | 5 + .../groups/components/item_actions_spec.js | 12 +- .../components/delete_account_modal_spec.js | 8 +- .../components/new_dropdown/index_spec.js | 13 +- .../vue_shared/components/modal_spec.js | 64 +++++++++- 15 files changed, 169 insertions(+), 116 deletions(-) create mode 100644 changelogs/unreleased/winh-modal-target-id.yml diff --git a/app/assets/javascripts/groups/components/item_actions.vue b/app/assets/javascripts/groups/components/item_actions.vue index 58ba5aff7cf..b98cfcf7563 100644 --- a/app/assets/javascripts/groups/components/item_actions.vue +++ b/app/assets/javascripts/groups/components/item_actions.vue @@ -45,11 +45,9 @@ export default { onLeaveGroup() { this.modalStatus = true; }, - leaveGroup(leaveConfirmed) { + leaveGroup() { this.modalStatus = false; - if (leaveConfirmed) { - eventHub.$emit('leaveGroup', this.group, this.parentGroup); - } + eventHub.$emit('leaveGroup', this.group, this.parentGroup); }, }, }; diff --git a/app/assets/javascripts/ide/components/new_dropdown/index.vue b/app/assets/javascripts/ide/components/new_dropdown/index.vue index 6e67e99a70f..d475813c4f7 100644 --- a/app/assets/javascripts/ide/components/new_dropdown/index.vue +++ b/app/assets/javascripts/ide/components/new_dropdown/index.vue @@ -32,10 +32,10 @@ methods: { createNewItem(type) { this.modalType = type; - this.toggleModalOpen(); + this.openModal = true; }, - toggleModalOpen() { - this.openModal = !this.openModal; + hideModal() { + this.openModal = false; }, }, }; @@ -95,7 +95,7 @@ :branch-id="branch" :path="path" :parent="parent" - @toggle="toggleModalOpen" + @hide="hideModal" /> diff --git a/app/assets/javascripts/ide/components/new_dropdown/modal.vue b/app/assets/javascripts/ide/components/new_dropdown/modal.vue index a0650d37690..0312f56efbd 100644 --- a/app/assets/javascripts/ide/components/new_dropdown/modal.vue +++ b/app/assets/javascripts/ide/components/new_dropdown/modal.vue @@ -43,10 +43,10 @@ type: this.type, }); - this.toggleModalOpen(); + this.hideModal(); }, - toggleModalOpen() { - this.$emit('toggle'); + hideModal() { + this.$emit('hide'); }, }, computed: { @@ -86,7 +86,7 @@ :title="modalTitle" :primary-button-label="buttonLabel" kind="success" - @toggle="toggleModalOpen" + @cancel="hideModal" @submit="createEntryInStore" >
diff --git a/app/assets/javascripts/profile/account/components/delete_account_modal.vue b/app/assets/javascripts/profile/account/components/delete_account_modal.vue index 78be6b6e884..36ad618aa46 100644 --- a/app/assets/javascripts/profile/account/components/delete_account_modal.vue +++ b/app/assets/javascripts/profile/account/components/delete_account_modal.vue @@ -1,7 +1,7 @@ diff --git a/app/assets/javascripts/profile/account/index.js b/app/assets/javascripts/profile/account/index.js index 635056e0eeb..a93bc935dd0 100644 --- a/app/assets/javascripts/profile/account/index.js +++ b/app/assets/javascripts/profile/account/index.js @@ -1,7 +1,12 @@ import Vue from 'vue'; +import Translate from '~/vue_shared/translate'; + import deleteAccountModal from './components/delete_account_modal.vue'; +Vue.use(Translate); + +const deleteAccountButton = document.getElementById('delete-account-button'); const deleteAccountModalEl = document.getElementById('delete-account-modal'); // eslint-disable-next-line no-new new Vue({ @@ -9,6 +14,9 @@ new Vue({ components: { deleteAccountModal, }, + mounted() { + deleteAccountButton.classList.remove('disabled'); + }, render(createElement) { return createElement('delete-account-modal', { props: { diff --git a/app/assets/javascripts/vue_shared/components/modal.vue b/app/assets/javascripts/vue_shared/components/modal.vue index 55f466b7b41..00089dfef38 100644 --- a/app/assets/javascripts/vue_shared/components/modal.vue +++ b/app/assets/javascripts/vue_shared/components/modal.vue @@ -3,6 +3,10 @@ export default { name: 'modal', props: { + id: { + type: String, + required: false, + }, title: { type: String, required: false, @@ -62,11 +66,11 @@ export default { }, methods: { - close() { - this.$emit('toggle', false); + emitCancel(event) { + this.$emit('cancel', event); }, - emitSubmit(status) { - this.$emit('submit', status); + emitSubmit(event) { + this.$emit('submit', event); }, }, }; @@ -75,7 +79,9 @@ export default { diff --git a/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue b/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue index 8053c65d498..16d60bb2876 100644 --- a/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue +++ b/app/assets/javascripts/vue_shared/components/recaptcha_modal.vue @@ -70,7 +70,7 @@ export default { class="recaptcha-modal js-recaptcha-modal" :hide-footer="true" :title="__('Please solve the reCAPTCHA')" - @toggle="close" + @cancel="close" >

diff --git a/app/views/profiles/accounts/show.html.haml b/app/views/profiles/accounts/show.html.haml index f1313b79589..79e197ad08b 100644 --- a/app/views/profiles/accounts/show.html.haml +++ b/app/views/profiles/accounts/show.html.haml @@ -84,11 +84,13 @@ = s_('Profiles|Deleting an account has the following effects:') = render 'users/deletion_guidance', user: current_user + %button#delete-account-button.btn.btn-danger.disabled{ data: { toggle: 'modal', + target: '#delete-account-modal' } } + = s_('Profiles|Delete account') + #delete-account-modal{ data: { action_url: user_registration_path, confirm_with_password: ('true' if current_user.confirm_deletion_with_password?), username: current_user.username } } - %button.btn.btn-danger.disabled - = s_('Profiles|Delete account') - else - if @user.solo_owned_groups.present? %p diff --git a/changelogs/unreleased/winh-modal-target-id.yml b/changelogs/unreleased/winh-modal-target-id.yml new file mode 100644 index 00000000000..f8d5b72be50 --- /dev/null +++ b/changelogs/unreleased/winh-modal-target-id.yml @@ -0,0 +1,5 @@ +--- +title: Add id to modal.vue to support data-toggle="modal" +merge_request: 16189 +author: +type: other diff --git a/spec/javascripts/groups/components/item_actions_spec.js b/spec/javascripts/groups/components/item_actions_spec.js index 7a5c1da4d1d..6d6fb410859 100644 --- a/spec/javascripts/groups/components/item_actions_spec.js +++ b/spec/javascripts/groups/components/item_actions_spec.js @@ -47,18 +47,12 @@ describe('ItemActionsComponent', () => { it('should change `modalStatus` prop to `false` and emit `leaveGroup` event with required params when called with `leaveConfirmed` as `true`', () => { spyOn(eventHub, '$emit'); vm.modalStatus = true; - vm.leaveGroup(true); + + vm.leaveGroup(); + expect(vm.modalStatus).toBeFalsy(); expect(eventHub.$emit).toHaveBeenCalledWith('leaveGroup', vm.group, vm.parentGroup); }); - - it('should change `modalStatus` prop to `false` and should NOT emit `leaveGroup` event when called with `leaveConfirmed` as `false`', () => { - spyOn(eventHub, '$emit'); - vm.modalStatus = true; - vm.leaveGroup(false); - expect(vm.modalStatus).toBeFalsy(); - expect(eventHub.$emit).not.toHaveBeenCalled(); - }); }); }); diff --git a/spec/javascripts/profile/account/components/delete_account_modal_spec.js b/spec/javascripts/profile/account/components/delete_account_modal_spec.js index 2e94948cfb2..588b61196a5 100644 --- a/spec/javascripts/profile/account/components/delete_account_modal_spec.js +++ b/spec/javascripts/profile/account/components/delete_account_modal_spec.js @@ -51,7 +51,7 @@ describe('DeleteAccountModal component', () => { Vue.nextTick() .then(() => { expect(vm.enteredPassword).toBe(input.value); - expect(submitButton).toHaveClass('disabled'); + expect(submitButton).toHaveAttr('disabled', 'disabled'); submitButton.click(); expect(form.submit).not.toHaveBeenCalled(); }) @@ -68,7 +68,7 @@ describe('DeleteAccountModal component', () => { Vue.nextTick() .then(() => { expect(vm.enteredPassword).toBe(input.value); - expect(submitButton).not.toHaveClass('disabled'); + expect(submitButton).not.toHaveAttr('disabled', 'disabled'); submitButton.click(); expect(form.submit).toHaveBeenCalled(); }) @@ -101,7 +101,7 @@ describe('DeleteAccountModal component', () => { Vue.nextTick() .then(() => { expect(vm.enteredUsername).toBe(input.value); - expect(submitButton).toHaveClass('disabled'); + expect(submitButton).toHaveAttr('disabled', 'disabled'); submitButton.click(); expect(form.submit).not.toHaveBeenCalled(); }) @@ -118,7 +118,7 @@ describe('DeleteAccountModal component', () => { Vue.nextTick() .then(() => { expect(vm.enteredUsername).toBe(input.value); - expect(submitButton).not.toHaveClass('disabled'); + expect(submitButton).not.toHaveAttr('disabled', 'disabled'); submitButton.click(); expect(form.submit).toHaveBeenCalled(); }) diff --git a/spec/javascripts/repo/components/new_dropdown/index_spec.js b/spec/javascripts/repo/components/new_dropdown/index_spec.js index b001c1655b4..6efbbf6d75e 100644 --- a/spec/javascripts/repo/components/new_dropdown/index_spec.js +++ b/spec/javascripts/repo/components/new_dropdown/index_spec.js @@ -57,15 +57,16 @@ describe('new dropdown component', () => { }); }); - describe('toggleModalOpen', () => { + describe('hideModal', () => { + beforeAll((done) => { + vm.openModal = true; + Vue.nextTick(done); + }); + it('closes modal after toggling', (done) => { - vm.toggleModalOpen(); + vm.hideModal(); Vue.nextTick() - .then(() => { - expect(vm.$el.querySelector('.modal')).not.toBeNull(); - }) - .then(vm.toggleModalOpen) .then(() => { expect(vm.$el.querySelector('.modal')).toBeNull(); }) diff --git a/spec/javascripts/vue_shared/components/modal_spec.js b/spec/javascripts/vue_shared/components/modal_spec.js index 721f4044659..fe75a86cac8 100644 --- a/spec/javascripts/vue_shared/components/modal_spec.js +++ b/spec/javascripts/vue_shared/components/modal_spec.js @@ -2,11 +2,65 @@ import Vue from 'vue'; import modal from '~/vue_shared/components/modal.vue'; import mountComponent from '../../helpers/vue_mount_component_helper'; -describe('Modal', () => { - it('does not render a primary button if no primaryButtonLabel', () => { - const modalComponent = Vue.extend(modal); - const vm = mountComponent(modalComponent); +const modalComponent = Vue.extend(modal); - expect(vm.$el.querySelector('.js-primary-button')).toBeNull(); +describe('Modal', () => { + let vm; + + afterEach(() => { + vm.$destroy(); + }); + + describe('props', () => { + describe('without primaryButtonLabel', () => { + beforeEach(() => { + vm = mountComponent(modalComponent, { + primaryButtonLabel: null, + }); + }); + + it('does not render a primary button', () => { + expect(vm.$el.querySelector('.js-primary-button')).toBeNull(); + }); + }); + + describe('with id', () => { + it('does not render a primary button', () => { + beforeEach(() => { + vm = mountComponent(modalComponent, { + id: 'my-modal', + }); + }); + + it('assigns the id to the modal', () => { + expect(vm.$el.querySelector('#my-modal.modal')).not.toBeNull(); + }); + + it('does not show the modal immediately', () => { + expect(vm.$el.querySelector('#my-modal.modal')).not.toHaveClass('show'); + }); + + it('does not show a backdrop', () => { + expect(vm.$el.querySelector('modal-backdrop')).toBeNull(); + }); + }); + }); + + it('works with data-toggle="modal"', (done) => { + setFixtures(` + +

+ `); + + const modalContainer = document.getElementById('modal-container'); + const modalButton = document.getElementById('modal-button'); + vm = mountComponent(modalComponent, { + id: 'my-modal', + }, modalContainer); + const modalElement = vm.$el.querySelector('#my-modal'); + $(modalElement).on('shown.bs.modal', () => done()); + + modalButton.click(); + }); }); });