From 90a67a76d5b852c62b59dd52b9dafd58722f2237 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 14 Apr 2016 17:32:46 -0400 Subject: [PATCH 01/78] Always read diff_view setting from the cookie Prior, when the user had their view set to "parallel" and then visited a merge request's changes tab _without_ passing the `view` parameter via query string, the view would be parallel but the `Notes` class was always instantiated with the default value from `diff_view` ("inline"), resulting in broken markup when the form to add a line note was dynamically inserted. The cookie is set whenever the view is changed, so this value should always be up-to-date. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/14557 --- app/helpers/diff_helper.rb | 8 +++++++- .../projects/merge_requests/_show.html.haml | 2 +- spec/helpers/diff_helper_spec.rb | 20 +++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 6a3ec83b8c..97466d532f 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -9,7 +9,13 @@ module DiffHelper end def diff_view - params[:view] == 'parallel' ? 'parallel' : 'inline' + diff_views = %w(inline parallel) + + if diff_views.include?(cookies[:diff_view]) + cookies[:diff_view] + else + diff_views.first + end end def diff_hard_limit_enabled? diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 1dd8f721f7..1733622e9b 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -4,7 +4,7 @@ = render "header_title" -- if params[:view] == 'parallel' +- if diff_view == 'parallel' - fluid_layout true .merge-request{'data-url' => merge_request_path(@merge_request)} diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 982c113e84..b7810185d1 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -11,6 +11,26 @@ describe DiffHelper do let(:diff_refs) { [commit.parent, commit] } let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs) } + describe 'diff_view' do + it 'returns a valid value when cookie is set' do + helper.request.cookies[:diff_view] = 'parallel' + + expect(helper.diff_view).to eq 'parallel' + end + + it 'returns a default value when cookie is invalid' do + helper.request.cookies[:diff_view] = 'invalid' + + expect(helper.diff_view).to eq 'inline' + end + + it 'returns a default value when cookie is nil' do + expect(helper.request.cookies).to be_empty + + expect(helper.diff_view).to eq 'inline' + end + end + describe 'diff_hard_limit_enabled?' do it 'should return true if param is provided' do allow(controller).to receive(:params) { { force_show_diff: true } } From 50998e69bf743bda5d053262237145bd240bca8b Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 11 Apr 2016 23:05:49 -0500 Subject: [PATCH 02/78] Restore broken code --- app/assets/javascripts/labels_select.js.coffee | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index bc80980acb..c1fcd764a6 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -20,15 +20,6 @@ class @LabelsSelect $value = $block.find('.value') $loading = $block.find('.block-loading').fadeOut() - if newLabelField.length - $newLabelCreateButton = $('.js-new-label-btn') - $colorPreview = $('.js-dropdown-label-color-preview') - $newLabelError = $dropdown.parent().find('.js-label-error') - $newLabelError.hide() - - # Suggested colors in the dropdown to chose from pre-chosen colors - $('.suggest-colors-dropdown a').on 'click', (e) -> - issueURLSplit = issueUpdateURL.split('/') if issueUpdateURL? if issueUpdateURL labelHTMLTemplate = _.template( @@ -43,6 +34,12 @@ class @LabelsSelect labelNoneHTMLTemplate = _.template('
None
') if newLabelField.length and $dropdown.hasClass 'js-extra-options' + $newLabelCreateButton = $('.js-new-label-btn') + $colorPreview = $('.js-dropdown-label-color-preview') + $newLabelError = $dropdown.parent().find('.js-label-error') + $newLabelError.hide() + + # Suggested colors in the dropdown to chose from pre-chosen colors $('.suggest-colors-dropdown a').on "click", (e) -> e.preventDefault() e.stopPropagation() From f396b2e6d09fbfe47201ceb0f897c508dabb1b1a Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 11 Apr 2016 23:19:47 -0500 Subject: [PATCH 03/78] Reuse partial for Create Label page --- .../shared/issuable/_label_dropdown.html.haml | 18 +----------------- .../issuable/_label_page_create.html.haml | 17 +++++++++++++++++ app/views/shared/issuable/_sidebar.html.haml | 4 +++- 3 files changed, 21 insertions(+), 18 deletions(-) create mode 100644 app/views/shared/issuable/_label_page_create.html.haml diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index f722e61eea..371acc46c5 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -24,21 +24,5 @@ - else View labels - if can? current_user, :admin_label, @project and @project - .dropdown-page-two.dropdown-new-label - = dropdown_title("Create new label", back: true) - = dropdown_content do - .dropdown-labels-error.js-label-error - %input#new_label_name.dropdown-input-field{type: "text", placeholder: "Name new label"} - .suggest-colors.suggest-colors-dropdown - - suggested_colors.each do |color| - = link_to '#', style: "background-color: #{color}", data: { color: color } do -   - .dropdown-label-color-input - .dropdown-label-color-preview.js-dropdown-label-color-preview - %input#new_label_color.dropdown-input-field{ type: "text" } - .clearfix - %button.btn.btn-primary.pull-left.js-new-label-btn{type: "button"} - Create - %button.btn.btn-default.pull-right.js-cancel-label-btn{type: "button"} - Cancel + = render partial: "shared/issuable/label_page_create" = dropdown_loading diff --git a/app/views/shared/issuable/_label_page_create.html.haml b/app/views/shared/issuable/_label_page_create.html.haml new file mode 100644 index 0000000000..89bc836826 --- /dev/null +++ b/app/views/shared/issuable/_label_page_create.html.haml @@ -0,0 +1,17 @@ +.dropdown-page-two.dropdown-new-label + = dropdown_title("Create new label", back: true) + = dropdown_content do + .dropdown-labels-error.js-label-error + %input#new_label_name.dropdown-input-field{type: "text", placeholder: "Name new label"} + .suggest-colors.suggest-colors-dropdown + - suggested_colors.each do |color| + = link_to '#', style: "background-color: #{color}", data: { color: color } do +   + .dropdown-label-color-input + .dropdown-label-color-preview.js-dropdown-label-color-preview + %input#new_label_color.dropdown-input-field{ type: "text" } + .clearfix + %button.btn.btn-primary.pull-left.js-new-label-btn{type: "button"} + Create + %button.btn.btn-default.pull-right.js-cancel-label-btn{type: "button"} + Cancel diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 08bfd93f4e..2dc5c0c19e 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -96,7 +96,7 @@ - issuable.labels.each do |label| = hidden_field_tag "#{issuable.to_ability_name}[label_names][]", label.id, id: nil .dropdown - %button.dropdown-menu-toggle.js-label-select.js-multiselect{type: "button", data: {toggle: "dropdown", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", project_id: (@project.id if @project), issue_update: issuable_json_path(issuable), labels: (namespace_project_labels_path(@project.namespace, @project, :json) if @project)}} + %button.dropdown-menu-toggle.js-label-select.js-multiselect.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", project_id: (@project.id if @project), issue_update: issuable_json_path(issuable), labels: (namespace_project_labels_path(@project.namespace, @project, :json) if @project)}} %span.dropdown-toggle-text Label = icon('chevron-down') @@ -118,6 +118,8 @@ Manage labels - else View labels + - if can? current_user, :admin_label, @project and @project + = render "shared/issuable/milestone_dropdown" = render "shared/issuable/participants", participants: issuable.participants(current_user) - if current_user From a2550392100422a00db8f9baabfded7e2e71d046 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 13 Apr 2016 14:12:40 -0500 Subject: [PATCH 04/78] Add spaces --- app/views/shared/issuable/_label_page_create.html.haml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/shared/issuable/_label_page_create.html.haml b/app/views/shared/issuable/_label_page_create.html.haml index 89bc836826..9179069f78 100644 --- a/app/views/shared/issuable/_label_page_create.html.haml +++ b/app/views/shared/issuable/_label_page_create.html.haml @@ -2,7 +2,7 @@ = dropdown_title("Create new label", back: true) = dropdown_content do .dropdown-labels-error.js-label-error - %input#new_label_name.dropdown-input-field{type: "text", placeholder: "Name new label"} + %input#new_label_name.dropdown-input-field{ type: "text", placeholder: "Name new label"} .suggest-colors.suggest-colors-dropdown - suggested_colors.each do |color| = link_to '#', style: "background-color: #{color}", data: { color: color } do @@ -11,7 +11,7 @@ .dropdown-label-color-preview.js-dropdown-label-color-preview %input#new_label_color.dropdown-input-field{ type: "text" } .clearfix - %button.btn.btn-primary.pull-left.js-new-label-btn{type: "button"} + %button.btn.btn-primary.pull-left.js-new-label-btn{ type: "button" } Create - %button.btn.btn-default.pull-right.js-cancel-label-btn{type: "button"} + %button.btn.btn-default.pull-right.js-cancel-label-btn{ type: "button" } Cancel From 88ddce257d90114c3981c5c07d621331a683bd05 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 14 Apr 2016 12:57:54 -0500 Subject: [PATCH 05/78] Render proper partial --- app/views/shared/issuable/_label_page_create.html.haml | 2 +- app/views/shared/issuable/_sidebar.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/views/shared/issuable/_label_page_create.html.haml b/app/views/shared/issuable/_label_page_create.html.haml index 9179069f78..26ccade425 100644 --- a/app/views/shared/issuable/_label_page_create.html.haml +++ b/app/views/shared/issuable/_label_page_create.html.haml @@ -2,7 +2,7 @@ = dropdown_title("Create new label", back: true) = dropdown_content do .dropdown-labels-error.js-label-error - %input#new_label_name.dropdown-input-field{ type: "text", placeholder: "Name new label"} + %input#new_label_name.dropdown-input-field{ type: "text", placeholder: "Name new label" } .suggest-colors.suggest-colors-dropdown - suggested_colors.each do |color| = link_to '#', style: "background-color: #{color}", data: { color: color } do diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 2dc5c0c19e..6108239aed 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -119,7 +119,7 @@ - else View labels - if can? current_user, :admin_label, @project and @project - = render "shared/issuable/milestone_dropdown" + = render partial: "shared/issuable/label_page_create" = render "shared/issuable/participants", participants: issuable.participants(current_user) - if current_user From 0556d661e0027068aa7bee45a5c61017344505f3 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 14 Apr 2016 19:47:53 -0500 Subject: [PATCH 06/78] Fix Label dropdown and organize code # Conflicts: # app/assets/javascripts/labels_select.js.coffee --- app/assets/javascripts/gl_dropdown.js.coffee | 7 +++++-- app/assets/stylesheets/framework/dropdowns.scss | 2 +- app/assets/stylesheets/pages/issuable.scss | 4 ++++ app/views/shared/issuable/_label_page_create.html.haml | 4 ++-- app/views/shared/issuable/_sidebar.html.haml | 2 +- 5 files changed, 13 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 2dc37257e2..fa1f765095 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -224,6 +224,9 @@ class GitLabDropdown menu.toggleClass PAGE_TWO_CLASS + # Focus first visible input on active page + @dropdown.find('[class^="dropdown-page-"]:visible :text:visible:first').focus() + parseData: (data) -> @renderedData = data @@ -243,7 +246,8 @@ class GitLabDropdown shouldPropagate: (e) => if @options.multiSelect $target = $(e.target) - if not $target.hasClass('dropdown-menu-close') and not $target.hasClass('dropdown-menu-close-icon') + + if not $target.hasClass('dropdown-menu-close') and not $target.hasClass('dropdown-menu-close-icon') and ($target.attr('href') is '#') e.stopPropagation() return false else @@ -378,7 +382,6 @@ class GitLabDropdown selectedObject = @renderedData[selectedIndex] value = if @options.id then @options.id(selectedObject, el) else selectedObject.id field = @dropdown.parent().find("input[name='#{fieldName}'][value='#{value}']") - if el.hasClass(ACTIVE_CLASS) el.removeClass(ACTIVE_CLASS) field.remove() diff --git a/app/assets/stylesheets/framework/dropdowns.scss b/app/assets/stylesheets/framework/dropdowns.scss index ba6c7930cd..719703b1af 100644 --- a/app/assets/stylesheets/framework/dropdowns.scss +++ b/app/assets/stylesheets/framework/dropdowns.scss @@ -320,7 +320,7 @@ } } -.dropdown-input-field { +.dropdown-input-field, .default-dropdown-input { width: 100%; padding: 0 7px; color: $dropdown-input-color; diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index 6bd90a2362..152b60d1e8 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -248,6 +248,10 @@ background: $gray-dark; border: 1px solid $border-gray-dark; } + + &.btn-primary { + @extend .btn-primary + } } a:not(.btn) { diff --git a/app/views/shared/issuable/_label_page_create.html.haml b/app/views/shared/issuable/_label_page_create.html.haml index 26ccade425..3bc57d3d2a 100644 --- a/app/views/shared/issuable/_label_page_create.html.haml +++ b/app/views/shared/issuable/_label_page_create.html.haml @@ -2,14 +2,14 @@ = dropdown_title("Create new label", back: true) = dropdown_content do .dropdown-labels-error.js-label-error - %input#new_label_name.dropdown-input-field{ type: "text", placeholder: "Name new label" } + %input#new_label_name.default-dropdown-input{ type: "text", placeholder: "Name new label" } .suggest-colors.suggest-colors-dropdown - suggested_colors.each do |color| = link_to '#', style: "background-color: #{color}", data: { color: color } do   .dropdown-label-color-input .dropdown-label-color-preview.js-dropdown-label-color-preview - %input#new_label_color.dropdown-input-field{ type: "text" } + %input#new_label_color.default-dropdown-input{ type: "text" } .clearfix %button.btn.btn-primary.pull-left.js-new-label-btn{ type: "button" } Create diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 6108239aed..ef670bc9ba 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -96,7 +96,7 @@ - issuable.labels.each do |label| = hidden_field_tag "#{issuable.to_ability_name}[label_names][]", label.id, id: nil .dropdown - %button.dropdown-menu-toggle.js-label-select.js-multiselect.js-extra-options{type: "button", data: {toggle: "dropdown", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", project_id: (@project.id if @project), issue_update: issuable_json_path(issuable), labels: (namespace_project_labels_path(@project.namespace, @project, :json) if @project)}} + %button.dropdown-menu-toggle.js-label-select.js-multiselect{type: "button", data: {toggle: "dropdown", field_name: "#{issuable.to_ability_name}[label_names][]", ability_name: issuable.to_ability_name, show_no: "true", show_any: "true", project_id: (@project.id if @project), issue_update: issuable_json_path(issuable), labels: (namespace_project_labels_path(@project.namespace, @project, :json) if @project)}} %span.dropdown-toggle-text Label = icon('chevron-down') From 9066ac0da51bd2b74249185c79ca278942a40241 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 15 Apr 2016 13:01:46 -0500 Subject: [PATCH 07/78] Fix create label functionality --- .../javascripts/labels_select.js.coffee | 66 +++++++------------ 1 file changed, 25 insertions(+), 41 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index c1fcd764a6..ddf7d84a38 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -2,14 +2,15 @@ class @LabelsSelect constructor: -> $('.js-label-select').each (i, dropdown) -> $dropdown = $(dropdown) + $parent = $dropdown.parent() projectId = $dropdown.data('project-id') labelUrl = $dropdown.data('labels') issueUpdateURL = $dropdown.data('issueUpdate') selectedLabel = $dropdown.data('selected') if selectedLabel? selectedLabel = selectedLabel.split(',') - newLabelField = $('#new_label_name') - newColorField = $('#new_label_color') + newLabelField = $('#new_label_name', $parent) + newColorField = $('#new_label_color', $parent) showNo = $dropdown.data('show-no') showAny = $dropdown.data('show-any') defaultLabel = $dropdown.data('default-label') @@ -18,6 +19,11 @@ class @LabelsSelect $block = $selectbox.closest('.block') $sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon span') $value = $block.find('.value') + $newLabelError = $('.js-label-error', $parent) + $colorPreview = $('.js-dropdown-label-color-preview', $parent) + $newLabelCreateButton = $('.js-new-label-btn', $parent) + + $newLabelError.hide() $loading = $block.find('.block-loading').fadeOut() issueURLSplit = issueUpdateURL.split('/') if issueUpdateURL? @@ -33,11 +39,7 @@ class @LabelsSelect ); labelNoneHTMLTemplate = _.template('
None
') - if newLabelField.length and $dropdown.hasClass 'js-extra-options' - $newLabelCreateButton = $('.js-new-label-btn') - $colorPreview = $('.js-dropdown-label-color-preview') - $newLabelError = $dropdown.parent().find('.js-label-error') - $newLabelError.hide() + if newLabelField.length # Suggested colors in the dropdown to chose from pre-chosen colors $('.suggest-colors-dropdown a').on "click", (e) -> @@ -78,26 +80,25 @@ class @LabelsSelect enableLabelCreateButton = -> if newLabelField.val() isnt '' and newColorField.val() isnt '' $newLabelError.hide() - $('.js-new-label-btn').disable() - - # Create new label with API - Api.newLabel projectId, { - name: newLabelField.val() - color: newColorField.val() - }, (label) -> - $('.js-new-label-btn').enable() - - if label.message? - $newLabelError - .text label.message - .show() - else - $('.dropdown-menu-back', $dropdown.parent()).trigger 'click' - $newLabelCreateButton.enable() else $newLabelCreateButton.disable() + saveLabel = -> + # Create new label with API + Api.newLabel projectId, { + name: newLabelField.val() + color: newColorField.val() + }, (label) -> + $newLabelCreateButton.enable() + + if label.message? + $newLabelError + .text label.message + .show() + else + $('.dropdown-menu-back', $dropdown.parent()).trigger 'click' + newLabelField.on 'keyup change', enableLabelCreateButton newColorField.on 'keyup change', enableLabelCreateButton @@ -108,24 +109,7 @@ class @LabelsSelect .on 'click', (e) -> e.preventDefault() e.stopPropagation() - - if newLabelField.val() isnt '' and newColorField.val() isnt '' - $newLabelError.hide() - $('.js-new-label-btn').disable() - - # Create new label with API - Api.newLabel projectId, { - name: newLabelField.val() - color: newColorField.val() - }, (label) -> - $('.js-new-label-btn').enable() - - if label.message? - $newLabelError - .text label.message - .show() - else - $('.dropdown-menu-back', $dropdown.parent()).trigger 'click' + saveLabel() saveLabelData = -> selected = $dropdown From 6c29e1ef69d4ee7e656d0b3b203cf43ff68c637b Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 15 Apr 2016 14:25:36 -0500 Subject: [PATCH 08/78] Add tests to check if a new label is added --- spec/features/issues/issue_sidebar_spec.rb | 79 ++++++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 spec/features/issues/issue_sidebar_spec.rb diff --git a/spec/features/issues/issue_sidebar_spec.rb b/spec/features/issues/issue_sidebar_spec.rb new file mode 100644 index 0000000000..5739bc64df --- /dev/null +++ b/spec/features/issues/issue_sidebar_spec.rb @@ -0,0 +1,79 @@ +require 'rails_helper' + +feature 'Issue Sidebar', feature: true do + let(:project) { create(:project) } + let(:issue) { create(:issue, project: project) } + let!(:user) { create(:user)} + + before do + create(:label, project: project, title: 'bug') + login_as(user) + end + + context 'as a allowed user' do + before do + project.team << [user, :developer] + visit_issue(project, issue) + end + + describe 'when clicking on edit labels', js: true do + it 'dropdown has an option to create a new label' do + find('.block.labels .edit-link').click + + page.within('.block.labels') do + expect(page).to have_content 'Create new' + end + end + end + + context 'creating a new label', js: true do + it 'option to crate a new label is present' do + page.within('.block.labels') do + find('.edit-link').click + + expect(page).to have_content 'Create new' + end + end + + it 'dropdown switches to "create label" section' do + page.within('.block.labels') do + find('.edit-link').click + click_link 'Create new' + + expect(page).to have_content 'Create new label' + end + end + + it 'new label is added' do + page.within('.block.labels') do + find('.edit-link').click + sleep 1 + click_link 'Create new' + + fill_in 'new_label_name', with: 'wontfix' + page.find(".suggest-colors a", match: :first).click + click_button 'Create' + + page.within('.dropdown-page-one') do + expect(page).to have_content 'wontfix' + end + end + end + end + end + + context 'as a guest' do + before do + project.team << [user, :guest] + visit_issue(project, issue) + end + + it 'does not have a option to edit labels' do + expect(page).not_to have_selector('.block.labels .edit-link') + end + end + + def visit_issue(project, issue) + visit namespace_project_issue_path(project.namespace, project, issue) + end +end From 37973bc1435f78f040d1e3f2c3398a4e2668b567 Mon Sep 17 00:00:00 2001 From: Connor Shea Date: Sat, 16 Apr 2016 17:56:13 -0600 Subject: [PATCH 09/78] Enable the EmptyRule SCSS Linter. Prevents empty SCSS rules from being included in the SCSS. Also fixes the handful of lints caught by the linter. --- .scss-lint.yml | 2 +- app/assets/stylesheets/framework/files.scss | 4 ---- app/assets/stylesheets/framework/selects.scss | 3 --- app/assets/stylesheets/highlight/monokai.scss | 2 -- app/assets/stylesheets/pages/graph.scss | 3 --- 5 files changed, 1 insertion(+), 13 deletions(-) diff --git a/.scss-lint.yml b/.scss-lint.yml index 835a4a88c4..9bfc18b969 100644 --- a/.scss-lint.yml +++ b/.scss-lint.yml @@ -65,7 +65,7 @@ linters: # Reports when you have an empty rule set. EmptyRule: - enabled: false + enabled: true # Reports when you have an @extend directive. ExtendDirective: diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 789df42fb6..98556460b5 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -82,10 +82,6 @@ } } - &.blob_file { - - } - &.blob-no-preview { background: #eee; text-shadow: 0 1px 2px #fff; diff --git a/app/assets/stylesheets/framework/selects.scss b/app/assets/stylesheets/framework/selects.scss index b2fab387e1..eae5f062dd 100644 --- a/app/assets/stylesheets/framework/selects.scss +++ b/app/assets/stylesheets/framework/selects.scss @@ -121,9 +121,6 @@ } } -.select2-container-multi .select2-choices .select2-search-choice { -} - .select2-drop-active { margin-top: 6px; font-size: 14px; diff --git a/app/assets/stylesheets/highlight/monokai.scss b/app/assets/stylesheets/highlight/monokai.scss index 28253d4ccb..80a509a7c1 100644 --- a/app/assets/stylesheets/highlight/monokai.scss +++ b/app/assets/stylesheets/highlight/monokai.scss @@ -111,8 +111,6 @@ .vg { color: #f8f8f2 } /* Name.Variable.Global */ .vi { color: #f8f8f2 } /* Name.Variable.Instance */ .il { color: #ae81ff } /* Literal.Number.Integer.Long */ - - .gh { } /* Generic Heading & Diff Header */ .gu { color: #75715e; } /* Generic.Subheading & Diff Unified/Comment? */ .gd { color: #f92672; } /* Generic.Deleted & Diff Deleted */ .gi { color: #a6e22e; } /* Generic.Inserted & Diff Inserted */ diff --git a/app/assets/stylesheets/pages/graph.scss b/app/assets/stylesheets/pages/graph.scss index 4e5c4ed84b..f7f9a9bb77 100644 --- a/app/assets/stylesheets/pages/graph.scss +++ b/app/assets/stylesheets/pages/graph.scss @@ -18,9 +18,6 @@ } .graphs { - .graph-author-commits-count { - } - .graph-author-email { float: right; color: #777; From 9a99fccb9717d4f1b0221de84094bb8db4ed280f Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 18 Apr 2016 12:49:23 -0500 Subject: [PATCH 10/78] Remove $parent --- app/assets/javascripts/labels_select.js.coffee | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index ddf7d84a38..602fc4be07 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -2,15 +2,14 @@ class @LabelsSelect constructor: -> $('.js-label-select').each (i, dropdown) -> $dropdown = $(dropdown) - $parent = $dropdown.parent() projectId = $dropdown.data('project-id') labelUrl = $dropdown.data('labels') issueUpdateURL = $dropdown.data('issueUpdate') selectedLabel = $dropdown.data('selected') if selectedLabel? selectedLabel = selectedLabel.split(',') - newLabelField = $('#new_label_name', $parent) - newColorField = $('#new_label_color', $parent) + newLabelField = $('#new_label_name') + newColorField = $('#new_label_color') showNo = $dropdown.data('show-no') showAny = $dropdown.data('show-any') defaultLabel = $dropdown.data('default-label') @@ -19,9 +18,9 @@ class @LabelsSelect $block = $selectbox.closest('.block') $sidebarCollapsedValue = $block.find('.sidebar-collapsed-icon span') $value = $block.find('.value') - $newLabelError = $('.js-label-error', $parent) - $colorPreview = $('.js-dropdown-label-color-preview', $parent) - $newLabelCreateButton = $('.js-new-label-btn', $parent) + $newLabelError = $('.js-label-error') + $colorPreview = $('.js-dropdown-label-color-preview') + $newLabelCreateButton = $('.js-new-label-btn') $newLabelError.hide() $loading = $block.find('.block-loading').fadeOut() From 5c7181b51492bd3ee13c5b498ac117ba08576ea8 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 18 Apr 2016 14:37:18 -0500 Subject: [PATCH 11/78] Add is-link attribute to know if a menu item should not be stopped --- app/assets/javascripts/gl_dropdown.js.coffee | 2 +- app/views/shared/issuable/_sidebar.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index fa1f765095..398d3ba4dc 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -247,7 +247,7 @@ class GitLabDropdown if @options.multiSelect $target = $(e.target) - if not $target.hasClass('dropdown-menu-close') and not $target.hasClass('dropdown-menu-close-icon') and ($target.attr('href') is '#') + if not $target.hasClass('dropdown-menu-close') and not $target.hasClass('dropdown-menu-close-icon') and not $target.data('is-link') e.stopPropagation() return false else diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index ef670bc9ba..62b95824ba 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -113,7 +113,7 @@ %a.dropdown-toggle-page{href: "#"} Create new %li - = link_to namespace_project_labels_path(@project.namespace, @project) do + = link_to namespace_project_labels_path(@project.namespace, @project), :"data-is-link" => true do - if can? current_user, :admin_label, @project Manage labels - else From e784958cb5396d745e43797988bacf9c9a27e9f4 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 18 Apr 2016 14:56:02 -0500 Subject: [PATCH 12/78] Move shared html to a partial --- .../shared/issuable/_label_dropdown.html.haml | 18 +----------------- .../issuable/_label_page_default.html.haml | 19 +++++++++++++++++++ app/views/shared/issuable/_sidebar.html.haml | 18 +----------------- 3 files changed, 21 insertions(+), 34 deletions(-) create mode 100644 app/views/shared/issuable/_label_page_default.html.haml diff --git a/app/views/shared/issuable/_label_dropdown.html.haml b/app/views/shared/issuable/_label_dropdown.html.haml index 371acc46c5..ec0cb78fac 100644 --- a/app/views/shared/issuable/_label_dropdown.html.haml +++ b/app/views/shared/issuable/_label_dropdown.html.haml @@ -6,23 +6,7 @@ = h(params[:label_name].presence || "Label") = icon('chevron-down') .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable - .dropdown-page-one - = dropdown_title("Filter by label") - = dropdown_filter("Search labels") - = dropdown_content - - if @project - = dropdown_footer do - %ul.dropdown-footer-list - - if can? current_user, :admin_label, @project - %li - %a.dropdown-toggle-page{href: "#"} - Create new - %li - = link_to namespace_project_labels_path(@project.namespace, @project) do - - if can? current_user, :admin_label, @project - Manage labels - - else - View labels + = render partial: "shared/issuable/label_page_default", locals: { title: "Filter by label" } - if can? current_user, :admin_label, @project and @project = render partial: "shared/issuable/label_page_create" = dropdown_loading diff --git a/app/views/shared/issuable/_label_page_default.html.haml b/app/views/shared/issuable/_label_page_default.html.haml new file mode 100644 index 0000000000..80a8fb82e0 --- /dev/null +++ b/app/views/shared/issuable/_label_page_default.html.haml @@ -0,0 +1,19 @@ +- title = local_assigns.fetch(:title, 'Assign labels') +- filter_placeholder = local_assigns.fetch(:filter_placeholder, 'Search labels') +.dropdown-page-one + = dropdown_title(title) + = dropdown_filter(filter_placeholder) + = dropdown_content + - if @project + = dropdown_footer do + %ul.dropdown-footer-list + - if can? current_user, :admin_label, @project + %li + %a.dropdown-toggle-page{href: "#"} + Create new + %li + = link_to namespace_project_labels_path(@project.namespace, @project), :"data-is-link" => true do + - if can? current_user, :admin_label, @project + Manage labels + - else + View labels \ No newline at end of file diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 62b95824ba..225737b9ec 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -101,23 +101,7 @@ Label = icon('chevron-down') .dropdown-menu.dropdown-select.dropdown-menu-paging.dropdown-menu-labels.dropdown-menu-selectable - .dropdown-page-one - = dropdown_title("Assign labels") - = dropdown_filter("Search labels") - = dropdown_content - - if @project - = dropdown_footer do - %ul.dropdown-footer-list - - if can? current_user, :admin_label, @project - %li - %a.dropdown-toggle-page{href: "#"} - Create new - %li - = link_to namespace_project_labels_path(@project.namespace, @project), :"data-is-link" => true do - - if can? current_user, :admin_label, @project - Manage labels - - else - View labels + = render partial: "shared/issuable/label_page_default" - if can? current_user, :admin_label, @project and @project = render partial: "shared/issuable/label_page_create" From ded5efad1ffddb5f1bf8585802bb25e588eee6ea Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 15 Apr 2016 21:35:48 -0500 Subject: [PATCH 13/78] Allow filtering by Owned projects --- app/controllers/concerns/filter_projects.rb | 5 +++++ app/views/shared/projects/_dropdown.html.haml | 10 +++++++++- features/dashboard/dashboard.feature | 5 +++++ features/steps/shared/issuable.rb | 14 ++++++++++++++ 4 files changed, 33 insertions(+), 1 deletion(-) diff --git a/app/controllers/concerns/filter_projects.rb b/app/controllers/concerns/filter_projects.rb index f63b703d10..091412a7c9 100644 --- a/app/controllers/concerns/filter_projects.rb +++ b/app/controllers/concerns/filter_projects.rb @@ -10,6 +10,11 @@ module FilterProjects def filter_projects(projects) projects = projects.search(params[:filter_projects]) if params[:filter_projects].present? projects = projects.non_archived if params[:archived].blank? + + if params[:personal].present? and current_user + projects = projects.personal(current_user) + end + projects end end diff --git a/app/views/shared/projects/_dropdown.html.haml b/app/views/shared/projects/_dropdown.html.haml index e7e04621ff..681d182dae 100644 --- a/app/views/shared/projects/_dropdown.html.haml +++ b/app/views/shared/projects/_dropdown.html.haml @@ -1,4 +1,5 @@ - @sort ||= sort_value_recently_updated +- personal = params[:personal] - archived = params[:archived] .dropdown.inline %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'} @@ -10,7 +11,7 @@ Sort by - projects_sort_options_hash.each do |value, title| %li - = link_to filter_projects_path(sort: value, archived: archived), class: ("is-active" if @sort == value) do + = link_to filter_projects_path(sort: value, archived: archived, personal: personal), class: ("is-active" if @sort == value) do = title %li.divider @@ -20,3 +21,10 @@ %li = link_to filter_projects_path(sort: @sort, archived: true), class: ("is-active" if params[:archived].present?) do Show archived projects + %li.divider + %li + = link_to filter_projects_path(sort: @sort, personal: nil), class: ("is-active" unless personal) do + Owned by anyone + %li + = link_to filter_projects_path(sort: @sort, personal: true), class: ("is-active" if personal) do + Owned by me diff --git a/features/dashboard/dashboard.feature b/features/dashboard/dashboard.feature index db73309804..c88cd0c9c2 100644 --- a/features/dashboard/dashboard.feature +++ b/features/dashboard/dashboard.feature @@ -79,3 +79,8 @@ Feature: Dashboard And I sort the list by "Oldest updated" And I visit project "Shop" merge requests page Then The list should be sorted by "Oldest updated" + + @javascript @focus + Scenario: Sort owned Projects + Then I filter the list by "Owned by me" + Then The list should be filtered by "Owned by me" diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb index a58b3cb7e1..c09be48080 100644 --- a/features/steps/shared/issuable.rb +++ b/features/steps/shared/issuable.rb @@ -138,6 +138,20 @@ module SharedIssuable end end + step 'I filter the list by "Owned by me"' do + find('button.dropdown-toggle.btn').click + + page.within('ul.dropdown-menu.dropdown-menu-align-right') do + click_link "Owned by me" + end + end + + step 'The list should be filtered by "Owned by me"' do + page.within('ul.dropdown-menu.dropdown-menu-align-right') do + expect(page).to have_selector('a', text: 'Owned by me') + end + end + step 'I should see "1 of 1" in the sidebar' do expect_sidebar_content('1 of 1') end From 9bd29a772604d9ea9aa7f9a5e6d4482e6fc81d6b Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 18 Apr 2016 19:53:49 -0500 Subject: [PATCH 14/78] Remove Spinach feature --- features/dashboard/dashboard.feature | 5 ----- features/steps/shared/issuable.rb | 14 -------------- 2 files changed, 19 deletions(-) diff --git a/features/dashboard/dashboard.feature b/features/dashboard/dashboard.feature index c88cd0c9c2..db73309804 100644 --- a/features/dashboard/dashboard.feature +++ b/features/dashboard/dashboard.feature @@ -79,8 +79,3 @@ Feature: Dashboard And I sort the list by "Oldest updated" And I visit project "Shop" merge requests page Then The list should be sorted by "Oldest updated" - - @javascript @focus - Scenario: Sort owned Projects - Then I filter the list by "Owned by me" - Then The list should be filtered by "Owned by me" diff --git a/features/steps/shared/issuable.rb b/features/steps/shared/issuable.rb index c09be48080..a58b3cb7e1 100644 --- a/features/steps/shared/issuable.rb +++ b/features/steps/shared/issuable.rb @@ -138,20 +138,6 @@ module SharedIssuable end end - step 'I filter the list by "Owned by me"' do - find('button.dropdown-toggle.btn').click - - page.within('ul.dropdown-menu.dropdown-menu-align-right') do - click_link "Owned by me" - end - end - - step 'The list should be filtered by "Owned by me"' do - page.within('ul.dropdown-menu.dropdown-menu-align-right') do - expect(page).to have_selector('a', text: 'Owned by me') - end - end - step 'I should see "1 of 1" in the sidebar' do expect_sidebar_content('1 of 1') end From c0ce49861510e73194760a4251858437abaedc03 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 18 Apr 2016 19:54:53 -0500 Subject: [PATCH 15/78] Add tests --- spec/features/dashboard_filter_spec.rb | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 spec/features/dashboard_filter_spec.rb diff --git a/spec/features/dashboard_filter_spec.rb b/spec/features/dashboard_filter_spec.rb new file mode 100644 index 0000000000..3d20813161 --- /dev/null +++ b/spec/features/dashboard_filter_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe "Dashboard projects filters", feature: true, js: true do + + context 'filtering personal projects' do + before do + user = create(:user) + project = create(:project, name: "Victorialand", namespace: user.namespace) + project.team << [user, :master] + + login_as(user) + visit dashboard_projects_path + + open_filter_dropdown + click_link "Owned by me" + end + + it 'filters by projects "Owned by me"' do + sleep 1 + open_filter_dropdown + page.within('ul.dropdown-menu.dropdown-menu-align-right') do + expect(page).to have_css('.is-active', text: 'Owned by me') + end + end + end + + def open_filter_dropdown + find('button.dropdown-toggle.btn').click + end +end From 136887da4b1c1184630ca2242055fe35bf04a69b Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 19 Apr 2016 10:54:27 +0100 Subject: [PATCH 16/78] Removed JS update templates --- app/controllers/projects/issues_controller.rb | 1 - app/controllers/projects/merge_requests_controller.rb | 1 - app/views/projects/issues/update.js.haml | 0 app/views/projects/merge_requests/update.js.haml | 0 app/views/shared/issuable/_sidebar.html.haml | 2 +- 5 files changed, 1 insertion(+), 3 deletions(-) delete mode 100644 app/views/projects/issues/update.js.haml delete mode 100644 app/views/projects/merge_requests/update.js.haml diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 38214f0479..05f12dad6a 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -100,7 +100,6 @@ class Projects::IssuesController < Projects::ApplicationController end respond_to do |format| - format.js format.html do if @issue.valid? redirect_to issue_path(@issue) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 3e0cfc6aa6..debe0475a2 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -148,7 +148,6 @@ class Projects::MergeRequestsController < Projects::ApplicationController if @merge_request.valid? respond_to do |format| - format.js format.html do redirect_to([@merge_request.target_project.namespace.becomes(Namespace), @merge_request.target_project, @merge_request]) diff --git a/app/views/projects/issues/update.js.haml b/app/views/projects/issues/update.js.haml deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/app/views/projects/merge_requests/update.js.haml b/app/views/projects/merge_requests/update.js.haml deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 08bfd93f4e..4560667d91 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -20,7 +20,7 @@ %a.btn.btn-default.disabled{href: '#'} Next - = form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f| + = form_for [@project.namespace.becomes(Namespace), @project, issuable], remote: true, format: :json, html: {class: 'issuable-context-form inline-update js-issuable-update'} do |f| .block.assignee .sidebar-collapsed-icon.sidebar-collapsed-user{data: {toggle: "tooltip", placement: "left", container: "body"}, title: (issuable.assignee.to_reference if issuable.assignee)} - if issuable.assignee From 60b5b22241046498c53a0ddb8509b4ebbf2b9315 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Tue, 19 Apr 2016 05:44:44 -0700 Subject: [PATCH 17/78] Update Sidekiq and related gems (bundle update sidekiq) Closes #15079 --- Gemfile.lock | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 958824f7ed..1589f226d3 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -134,7 +134,7 @@ GEM execjs coffee-script-source (1.10.0) colorize (0.7.7) - concurrent-ruby (1.0.0) + concurrent-ruby (1.0.1) connection_pool (2.2.0) coveralls (0.8.13) json (~> 1.8) @@ -627,7 +627,7 @@ GEM recaptcha (1.0.2) json redcarpet (3.3.3) - redis (3.2.2) + redis (3.3.0) redis-actionpack (4.0.1) actionpack (~> 4) redis-rack (~> 1.5.0) @@ -734,10 +734,9 @@ GEM rack shoulda-matchers (2.8.0) activesupport (>= 3.0.0) - sidekiq (4.0.1) + sidekiq (4.1.1) concurrent-ruby (~> 1.0) connection_pool (~> 2.2, >= 2.2.0) - json (~> 1.0) redis (~> 3.2, >= 3.2.1) sidekiq-cron (0.4.0) redis-namespace (>= 1.5.2) From d7f1e041d9df1121b550fbdb7682d25e404b04c2 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Tue, 19 Apr 2016 19:12:12 +0300 Subject: [PATCH 18/78] Fix links in update README [ci skip] --- doc/update/README.md | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/doc/update/README.md b/doc/update/README.md index 0241f03683..d4197e2cfc 100644 --- a/doc/update/README.md +++ b/doc/update/README.md @@ -2,17 +2,20 @@ Depending on the installation method and your GitLab version, there are multiple ## Omnibus Packages -- [Omnibus update guide](https://gitlab.com/gitlab-org/omnibus-gitlab/blob/master/doc/update.md) contains the steps needed to update a GitLab [package](https://about.gitlab.com/downloads/). +- [Omnibus update guide](http://doc.gitlab.com/omnibus/update/README.html) + contains the steps needed to update a GitLab + [package](https://about.gitlab.com/downloads/). ## Installation from source - [The individual upgrade guides](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc/update) are for those who have installed GitLab from source. -- [The CE to EE update guides](https://gitlab.com/subscribers/gitlab-ee/tree/master/doc/update) are for subscribers of the Enterprise Edition only. The steps are very similar to a version upgrade: stop the server, get the code, update config files for the new functionality, install libs and do migrations, update the init script, start the application and check the application status. -- [Upgrader](upgrader.md) is an automatic ruby script that performs the update for installations from source. -- [Patch versions](patch_versions.md) guide includes the steps needed for a patch version, eg. 6.2.0 to 6.2.1. +- [The CE to EE update guides](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc/update) are for subscribers of the Enterprise Edition only. The steps are very similar to a version upgrade: stop the server, get the code, update config files for the new functionality, install libs and do migrations, update the init script, start the application and check the application status. +- [Patch versions](patch_versions.md) guide includes the steps needed for a + patch version, eg. 6.2.0 to 6.2.1. ## Miscellaneous -- [MySQL to PostgreSQL](mysql_to_postgresql.md) guides you through migrating your database from MySQL to PostgreSQL. +- [MySQL to PostgreSQL](mysql_to_postgresql.md) guides you through migrating + your database from MySQL to PostgreSQL. - [MySQL installation guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/database_mysql.md) contains additional information about configuring GitLab to work with a MySQL database. - [Restoring from backup after a failed upgrade](restore_after_failure.md) From 8530ce4c6fbc411e00cf426f3b3baca74a4370f7 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 18 Apr 2016 14:13:38 -0400 Subject: [PATCH 19/78] Clarify that the diff view setting always comes from the cookie This invalidates one test, which we've removed. --- app/controllers/projects/application_controller.rb | 3 +-- .../projects/merge_requests_controller_spec.rb | 8 -------- 2 files changed, 1 insertion(+), 10 deletions(-) diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 74150ad606..be872a93fe 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -83,8 +83,7 @@ class Projects::ApplicationController < ApplicationController end def apply_diff_view_cookie! - view = params[:view] || cookies[:diff_view] - cookies.permanent[:diff_view] = params[:view] = view if view + cookies.permanent[:diff_view] = params.delete(:view) if params[:view].present? end def builds_enabled diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index c54e83339a..c0a1f45195 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -300,14 +300,6 @@ describe Projects::MergeRequestsController do expect(response.cookies['diff_view']).to eq('parallel') end - - it 'assigns :view param based on cookie' do - request.cookies['diff_view'] = 'parallel' - - go - - expect(controller.params[:view]).to eq 'parallel' - end end describe 'GET commits' do From 7bcacafc8a601896f8750f29fcde71f8f8ffb910 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 14 Apr 2016 16:52:01 +0100 Subject: [PATCH 20/78] Fixed issue with incorrect line being highlighted if line is in discussion view --- app/assets/javascripts/merge_request_tabs.js.coffee | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee index e8613cab72..55e09c6acb 100644 --- a/app/assets/javascripts/merge_request_tabs.js.coffee +++ b/app/assets/javascripts/merge_request_tabs.js.coffee @@ -176,12 +176,12 @@ class @MergeRequestTabs if locationHash isnt '' hashClassString = ".#{locationHash.replace('#', '')}" - $diffLine = $(locationHash) + $diffLine = $("#{locationHash}:not(.match)", $('#diffs')) - if $diffLine.is ':not(tr)' - $diffLine = $("td#{locationHash}, td#{hashClassString}") + if not $diffLine.is 'tr' + $diffLine = $('#diffs').find("td#{locationHash}, td#{hashClassString}") else - $diffLine = $('td', $diffLine) + $diffLine = $diffLine.find('td') if $diffLine.length $diffLine.addClass 'hll' From 6495c13d70cfd26cb46fb8bd0b7bc138cb939ff0 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 14 Apr 2016 17:00:05 +0100 Subject: [PATCH 21/78] Fixed issue with lines not being selectable when expanding diff --- app/views/projects/blob/diff.html.haml | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/views/projects/blob/diff.html.haml b/app/views/projects/blob/diff.html.haml index 38e62c81fe..88d51b00e3 100644 --- a/app/views/projects/blob/diff.html.haml +++ b/app/views/projects/blob/diff.html.haml @@ -1,17 +1,17 @@ - if @lines.present? - if @form.unfold? && @form.since != 1 && !@form.bottom? - %tr.line_holder{ id: @form.since } + %tr.line_holder = render "projects/diffs/match_line", { line: @match_line, line_old: @form.since, line_new: @form.since, bottom: false, new_file: false } - @lines.each_with_index do |line, index| - line_new = index + @form.since - line_old = line_new - @form.offset - %tr.line_holder + %tr.line_holder{ id: line_old } %td.old_line.diff-line-num{ data: { linenumber: line_old } } - = link_to raw(line_old), "#" - %td.new_line.diff-line-num{ data: { linenumber: line_old } } - = link_to raw(line_new) , "#" + = link_to raw(line_old), "##{line_old}" + %td.new_line.diff-line-num + = link_to raw(line_new) , "##{line_old}" %td.line_content.noteable_line==#{' ' * @form.indent}#{line} - if @form.unfold? && @form.bottom? && @form.to < @blob.loc From 28faa9719d70867f6ca0b0fb6b9fbf125812eb47 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 14 Apr 2016 17:07:44 +0100 Subject: [PATCH 22/78] Fixed issue when scrolling to element and there is an expander --- app/assets/javascripts/merge_request_tabs.js.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/merge_request_tabs.js.coffee b/app/assets/javascripts/merge_request_tabs.js.coffee index 55e09c6acb..372732d0aa 100644 --- a/app/assets/javascripts/merge_request_tabs.js.coffee +++ b/app/assets/javascripts/merge_request_tabs.js.coffee @@ -87,8 +87,8 @@ class @MergeRequestTabs if window.location.hash navBarHeight = $('.navbar-gitlab').outerHeight() - $el = $("#{container} #{window.location.hash}") - $.scrollTo("#{container} #{window.location.hash}", offset: -navBarHeight) if $el.length + $el = $("#{container} #{window.location.hash}:not(.match)") + $.scrollTo("#{container} #{window.location.hash}:not(.match)", offset: -navBarHeight) if $el.length # Activate a tab based on the current action activateTab: (action) -> From 2dbfa1de27f5123b4be75b46cdebfae1f3fdbdeb Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 20 Apr 2016 17:30:21 +0100 Subject: [PATCH 23/78] Put back removed line number --- app/views/projects/blob/diff.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/blob/diff.html.haml b/app/views/projects/blob/diff.html.haml index 88d51b00e3..5926d181ba 100644 --- a/app/views/projects/blob/diff.html.haml +++ b/app/views/projects/blob/diff.html.haml @@ -10,7 +10,7 @@ %tr.line_holder{ id: line_old } %td.old_line.diff-line-num{ data: { linenumber: line_old } } = link_to raw(line_old), "##{line_old}" - %td.new_line.diff-line-num + %td.new_line.diff-line-num{ data: { linenumber: line_old } } = link_to raw(line_new) , "##{line_old}" %td.line_content.noteable_line==#{' ' * @form.indent}#{line} From e38958d1d51b5f5fd695f09600d29b9b455757a1 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 21 Apr 2016 00:20:13 +0000 Subject: [PATCH 24/78] Remove unused param --- app/assets/javascripts/commits.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/commits.js.coffee b/app/assets/javascripts/commits.js.coffee index ffd3627b1b..0acb4c1955 100644 --- a/app/assets/javascripts/commits.js.coffee +++ b/app/assets/javascripts/commits.js.coffee @@ -1,7 +1,7 @@ class @CommitsList @timer = null - @init: (ref, limit) -> + @init: (limit) -> $("body").on "click", ".day-commits-table li.commit", (event) -> if event.target.nodeName != "A" location.href = $(this).attr("url") From fd5b158765ff46617e1e00642bb266177f6d5e32 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 21 Apr 2016 00:21:28 +0000 Subject: [PATCH 25/78] Remove unnecessary provided param --- app/views/projects/commits/show.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/commits/show.html.haml b/app/views/projects/commits/show.html.haml index c52cf25d40..bcdb09208a 100644 --- a/app/views/projects/commits/show.html.haml +++ b/app/views/projects/commits/show.html.haml @@ -39,4 +39,4 @@ = spinner :javascript - CommitsList.init("#{@ref}", #{@limit}); + CommitsList.init(#{@limit}); From dbe06ac790befe2d49dbb6edecb79d82abfe46e7 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 20 Apr 2016 23:55:06 -0500 Subject: [PATCH 26/78] Add noreferrer value to rel attribute for external links --- lib/banzai/filter/external_link_filter.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/banzai/filter/external_link_filter.rb b/lib/banzai/filter/external_link_filter.rb index d179bea181..699de56fd4 100644 --- a/lib/banzai/filter/external_link_filter.rb +++ b/lib/banzai/filter/external_link_filter.rb @@ -15,7 +15,7 @@ module Banzai # Skip internal links next if link.start_with?(internal_url) - node.set_attribute('rel', 'nofollow') + node.set_attribute('rel', 'nofollow noreferrer') end doc From 7f23e0b8c051da34c8587e4096476939a57fee24 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 08:50:15 +0100 Subject: [PATCH 27/78] Fixes text color on labels in sidebar --- app/assets/javascripts/labels_select.js.coffee | 5 +++-- app/controllers/projects/issues_controller.rb | 2 +- app/controllers/projects/merge_requests_controller.rb | 2 +- app/helpers/labels_helper.rb | 2 +- 4 files changed, 6 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 021ade73d4..5cb225c9ba 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -34,8 +34,8 @@ class @LabelsSelect if issueUpdateURL labelHTMLTemplate = _.template( '<% _.each(labels, function(label){ %> - issues?label_name=<%= _.escape(label.title) %>"> - + issues?label_name[]=<%= _.escape(label.title) %>"> + <%= _.escape(label.title) %> @@ -157,6 +157,7 @@ class @LabelsSelect data.issueURLSplit = issueURLSplit labelCount = 0 if data.labels.length + console.log data.labels template = labelHTMLTemplate(data) labelCount = data.labels.length else diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index e86428147e..346b055615 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -110,7 +110,7 @@ class Projects::IssuesController < Projects::ApplicationController end end format.json do - render json: @issue.to_json(include: [:milestone, :labels, assignee: { methods: :avatar_url }]) + render json: @issue.to_json(include: [:milestone, labels: { methods: :text_color }, assignee: { methods: :avatar_url }]) end end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 1388ea9d66..d82a4290ed 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -155,7 +155,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.target_project, @merge_request]) end format.json do - render json: @merge_request.to_json(include: [:milestone, :labels, assignee: { methods: :avatar_url }]) + render json: @merge_request.to_json(include: [:milestone, labels: { methods: :text_color }, assignee: { methods: :avatar_url }]) end end else diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index 3dded7c2f2..c99b137cda 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -37,7 +37,7 @@ module LabelsHelper link = send("namespace_project_#{type.to_s.pluralize}_path", project.namespace, project, - label_name: label.name) + label_name: [label.name]) if block_given? link_to link, &block From 2e0182b27821edc3c61b3aac5e8b214f7d08d8c2 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 08:54:24 +0100 Subject: [PATCH 28/78] Fixed issue with author link color on dark diffs --- app/assets/stylesheets/pages/notes.scss | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index aacb12532c..e308f01f91 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -196,6 +196,9 @@ ul.notes { } } + .author_link { + color: $gl-gray; + } } .note-headline-light, From 15d1804d9f6a2c37255150f43c164fec0af8371f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 10:54:03 +0100 Subject: [PATCH 29/78] Updated tests --- spec/helpers/labels_helper_spec.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 39042ff7e9..501f150cfd 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -11,13 +11,13 @@ describe LabelsHelper do end it 'uses the instance variable' do - expect(link_to_label(label)).to match %r{} + expect(link_to_label(label)).to match %r{} end end context 'without @project set' do it "uses the label's project" do - expect(link_to_label(label)).to match %r{.*} + expect(link_to_label(label)).to match %r{.*} end end @@ -25,7 +25,7 @@ describe LabelsHelper do let(:another_project) { double('project', namespace: 'foo3', to_param: 'bar3') } it 'links to merge requests page' do - expect(link_to_label(label, project: another_project)).to match %r{.*} + expect(link_to_label(label, project: another_project)).to match %r{.*} end end @@ -33,7 +33,7 @@ describe LabelsHelper do ['issue', :issue, 'merge_request', :merge_request].each do |type| context "set to #{type}" do it 'links to correct page' do - expect(link_to_label(label, type: type)).to match %r{.*} + expect(link_to_label(label, type: type)).to match %r{.*} end end end From 67c990be654eafae3bcaa2229e97b38acd98f200 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Thu, 21 Apr 2016 13:07:33 +0300 Subject: [PATCH 30/78] Split into sections, port `downgrade_ee_to_ce` doc from EE [ci skip] --- doc/downgrade_ee_to_ce/README.md | 82 +++++++++++++++++++++++++++++ doc/update/README.md | 90 +++++++++++++++++++++++++++++--- 2 files changed, 164 insertions(+), 8 deletions(-) create mode 100644 doc/downgrade_ee_to_ce/README.md diff --git a/doc/downgrade_ee_to_ce/README.md b/doc/downgrade_ee_to_ce/README.md new file mode 100644 index 0000000000..3625c4191b --- /dev/null +++ b/doc/downgrade_ee_to_ce/README.md @@ -0,0 +1,82 @@ +# Downgrading from EE to CE + +If you ever decide to downgrade your Enterprise Edition back to the Community +Edition, there are a few steps you need take before installing the CE package +on top of the current EE package, or, if you are in an installation from source, +before you change remotes and fetch the latest CE code. + +## Disable Enterprise-only features + +First thing to do is to disable the following features. + +### Authentication mechanisms + +Kerberos and Atlassian Crowd are only available on the Enterprise Edition, so +you should disable these mechanisms before downgrading and you should provide +alternative authentication methods to your users. + +### Git Annex + +Git Annex is also only available on the Enterprise Edition. This means that if +you have repositories that use Git Annex to store large files, these files will +no longer be easily available via Git. You should consider migrating these +repositories to use Git LFS before downgrading to the Community Edition. + +### Remove Jenkins CI Service entries from the database + +The `JenkinsService` class is only available on the Enterprise Edition codebase, +so if you downgrade to the Community Edition, you'll come across the following +error: + +``` +Completed 500 Internal Server Error in 497ms (ActiveRecord: 32.2ms) + +ActionView::Template::Error (The single-table inheritance mechanism failed to locate the subclass: 'JenkinsService'. This +error is raised because the column 'type' is reserved for storing the class in case of inheritance. Please rename this +column if you didn't intend it to be used for storing the inheritance class or overwrite Service.inheritance_column to +use another column for that information.) +``` + +All services are created automatically for every project you have, so in order +to avoid getting this error, you need to remove all instances of the +`JenkinsService` from your database: + +**Omnibus Installation** + +``` +$ sudo gitlab-rails runner "Service.where(type: 'JenkinsService').delete_all" +``` + +**Source Installation** + +``` +$ bundle exec rails runner "Service.where(type: 'JenkinsService').delete_all" production +``` + +## Downgrade to CE + +After performing the above mentioned steps, you are now ready to downgrade your +GitLab installation to the Community Edition. + +**Omnibus Installation** + +To downgrade an Omnibus installation, it is sufficient to install the Community +Edition package on top of the currently installed one. You can do this manually, +by directly [downloading the package](https://packages.gitlab.com/gitlab/gitlab-ce) +you need, or by adding our CE package repository and following the +[CE installation instructions](https://about.gitlab.com/downloads/). + +**Source Installation** + +To downgrade a source installation, you need to replace the current remote of +your GitLab installation with the Community Edition's remote, fetch the latest +changes, and checkout the latest stable branch: + +``` +$ git remote set-url origin git@gitlab.com:gitlab-org/gitlab-ce.git +$ git fetch --all +$ git checkout 8-x-stable +``` + +Remember to follow the correct [update guides](../update/README.md) to make +sure all dependencies are up to date. diff --git a/doc/update/README.md b/doc/update/README.md index d4197e2cfc..a770633c9b 100644 --- a/doc/update/README.md +++ b/doc/update/README.md @@ -1,21 +1,95 @@ -Depending on the installation method and your GitLab version, there are multiple update guides. Choose one that fits your needs. +# Updating GitLab + +Depending on the installation method and your GitLab version, there are multiple +update guides. + +There are currently 3 official ways to install GitLab: + +- Omnibus packages +- Source installation +- Docker installation + +Based on your installation, choose a section below that fits your needs. + +--- + + + +**Table of Contents** *generated with [DocToc](https://github.com/thlorenz/doctoc)* + +- [Omnibus Packages](#omnibus-packages) +- [Installation from source](#installation-from-source) +- [Installation using Docker](#installation-using-docker) +- [Upgrading between editions](#upgrading-between-editions) + - [Community to Enterprise Edition](#community-to-enterprise-edition) + - [Enterprise to Community Edition](#enterprise-to-community-edition) +- [Miscellaneous](#miscellaneous) + + ## Omnibus Packages -- [Omnibus update guide](http://doc.gitlab.com/omnibus/update/README.html) - contains the steps needed to update a GitLab - [package](https://about.gitlab.com/downloads/). +- The [Omnibus update guide](http://doc.gitlab.com/omnibus/update/README.html) + contains the steps needed to update an Omnibus GitLab package. ## Installation from source -- [The individual upgrade guides](https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc/update) are for those who have installed GitLab from source. -- [The CE to EE update guides](https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc/update) are for subscribers of the Enterprise Edition only. The steps are very similar to a version upgrade: stop the server, get the code, update config files for the new functionality, install libs and do migrations, update the init script, start the application and check the application status. +- [Upgrading Community Edition from source][source-ce] - The individual + upgrade guides are for those who have installed GitLab CE from source. +- [Upgrading Enterprise Edition from source][source-ee] - The individual + upgrade guides are for those who have installed GitLab EE from source. - [Patch versions](patch_versions.md) guide includes the steps needed for a - patch version, eg. 6.2.0 to 6.2.1. + patch version, eg. 6.2.0 to 6.2.1, and apply to both Community and Enterprise + Editions. + +## Installation using Docker + +GitLab provides official Docker images for both Community and Enterprise +editions. They are based on the Omnibus package and instructions on how to +update them are in [a separate document][omnidocker]. + +## Upgrading between editions + +GitLab comes in two flavors: [Community Edition][ce] which is MIT licensed, +and [Enterprise Edition][ee] which builds on top of the Community Edition and +includes extra features mainly aimed at organizations with more than 100 users. + +Below you can find some guides to help you change editions easily. + +### Community to Enterprise Edition + +>**Note:** +The following guides are for subscribers of the Enterprise Edition only. + +If you wish to upgrade your GitLab installation from Community to Enterprise +Edition, follow the guides below based on the installation method: + +- [Source CE to EE update guides][source-ee] - Find your version, and follow the + `-ce-to-ee.md` guide. The steps are very similar to a version upgrade: stop + the server, get the code, update config files for the new functionality, + install libraries and do migrations, update the init script, start the + application and check its status. +- [Omnibus CE to EE][omni-ce-ee] - Follow this guide to update your Omnibus + GitLab Community Edition to the Enterprise Edition. + +### Enterprise to Community Edition + +If you need to downgrade your Enterprise Edition installation back to Community +Edition, you can follow [this guide][ee-ce] to make the process as smooth as +possible. ## Miscellaneous - [MySQL to PostgreSQL](mysql_to_postgresql.md) guides you through migrating your database from MySQL to PostgreSQL. -- [MySQL installation guide](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/install/database_mysql.md) contains additional information about configuring GitLab to work with a MySQL database. +- [MySQL installation guide](../install/database_mysql.md) contains additional + information about configuring GitLab to work with a MySQL database. - [Restoring from backup after a failed upgrade](restore_after_failure.md) + +[omnidocker]: http://doc.gitlab.com/omnibus/docker/README.html +[source-ee]: https://gitlab.com/gitlab-org/gitlab-ee/tree/master/doc/update +[source-ce]: https://gitlab.com/gitlab-org/gitlab-ce/tree/master/doc/update +[ee-ce]: ../downgrade_ee_to_ce/README.md +[ce]: https://about.gitlab.com/features/#community +[ee]: https://about.gitlab.com/features/#enterprise +[omni-ce-ee]: http://doc.gitlab.com/omnibus/update/README.html#from-community-edition-to-enterprise-edition From 1641e81971353b9f1b715716181a0f2b45d6e52a Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Thu, 21 Apr 2016 09:14:54 -0500 Subject: [PATCH 31/78] File name change appears on one line --- app/assets/stylesheets/framework/files.scss | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 789df42fb6..148a7cab6e 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -38,12 +38,14 @@ .filename { &.old { + display: inline-block; span.idiff { background-color: #f8cbcb; } } &.new { + display: inline-block; span.idiff { background-color: #a6f3a6; } From b80bf57d378aec82cc933df233a30e0390564419 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Thu, 21 Apr 2016 16:50:26 +0200 Subject: [PATCH 32/78] Disable 'repository check' feature in 8.7.0 It still causes too many false alarms. --- CHANGELOG | 2 +- .../20160421130527_disable_repository_checks.rb | 11 +++++++++++ db/schema.rb | 4 ++-- doc/administration/repository_checks.md | 3 ++- 4 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 db/migrate/20160421130527_disable_repository_checks.rb diff --git a/CHANGELOG b/CHANGELOG index 89572b9e15..c9f7859932 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -53,7 +53,7 @@ v 8.7.0 (unreleased) - Add links to CI setup documentation from project settings and builds pages - Display project members page to all members - Handle nil descriptions in Slack issue messages (Stan Hu) - - Add automated repository integrity checks + - Add automated repository integrity checks (OFF by default) - API: Expose open_issues_count, closed_issues_count, open_merge_requests_count for labels (Robert Schilling) - API: Ability to star and unstar a project (Robert Schilling) - Add default scope to projects to exclude projects pending deletion diff --git a/db/migrate/20160421130527_disable_repository_checks.rb b/db/migrate/20160421130527_disable_repository_checks.rb new file mode 100644 index 0000000000..808a4b93c7 --- /dev/null +++ b/db/migrate/20160421130527_disable_repository_checks.rb @@ -0,0 +1,11 @@ +class DisableRepositoryChecks < ActiveRecord::Migration + def up + change_column_default :application_settings, :repository_checks_enabled, false + execute 'UPDATE application_settings SET repository_checks_enabled = false' + end + + def down + change_column_default :application_settings, :repository_checks_enabled, true + execute 'UPDATE application_settings SET repository_checks_enabled = true' + end +end diff --git a/db/schema.rb b/db/schema.rb index a743e68242..42457d9235 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: 20160419120017) do +ActiveRecord::Schema.define(version: 20160421130527) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -77,7 +77,7 @@ ActiveRecord::Schema.define(version: 20160419120017) do t.string "akismet_api_key" t.boolean "email_author_in_body", default: false t.integer "default_group_visibility" - t.boolean "repository_checks_enabled", default: true + t.boolean "repository_checks_enabled", default: false t.integer "metrics_packet_size", default: 1 t.text "shared_runners_text" end diff --git a/doc/administration/repository_checks.md b/doc/administration/repository_checks.md index 61bf8ce616..3411e4af6a 100644 --- a/doc/administration/repository_checks.md +++ b/doc/administration/repository_checks.md @@ -1,7 +1,8 @@ # Repository checks >**Note:** -This feature was [introduced][ce-3232] in GitLab 8.7. +This feature was [introduced][ce-3232] in GitLab 8.7. It is OFF by +default because it still causes too many false alarms. Git has a built-in mechanism, [git fsck][git-fsck], to verify the integrity of all data commited to a repository. GitLab administrators From 639ba1b09b421a8c4c115d7e9f4e66923faf531f Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 17:22:16 +0100 Subject: [PATCH 33/78] Removed console.log --- app/assets/javascripts/labels_select.js.coffee | 1 - 1 file changed, 1 deletion(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 5cb225c9ba..0417ba823f 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -157,7 +157,6 @@ class @LabelsSelect data.issueURLSplit = issueURLSplit labelCount = 0 if data.labels.length - console.log data.labels template = labelHTMLTemplate(data) labelCount = data.labels.length else From 6b4adeb3d1c35de04af486912b5eb9f1d1d3f24c Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 21 Apr 2016 12:38:08 -0400 Subject: [PATCH 34/78] Fix undefined local variable error in Invalid MR template Closes #15408 --- app/views/projects/merge_requests/invalid.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/merge_requests/invalid.html.haml b/app/views/projects/merge_requests/invalid.html.haml index 8ac653427c..f5bf16ef3a 100644 --- a/app/views/projects/merge_requests/invalid.html.haml +++ b/app/views/projects/merge_requests/invalid.html.haml @@ -1,4 +1,4 @@ -- page_title "#{@merge_request.title} (#{merge_request.to_reference}", "Merge Requests" +- page_title "#{@merge_request.title} (#{@merge_request.to_reference}", "Merge Requests" = render "header_title" .merge-request From 5e37d02d0ebd64249186577e306b7c502953a2e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Thu, 21 Apr 2016 18:49:08 +0200 Subject: [PATCH 35/78] Remove the `.distinct` when finding issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is not needed anymore after !3815. Signed-off-by: Rémy Coutable --- app/finders/issuable_finder.rb | 4 +--- spec/features/issues_spec.rb | 13 +++++++++++++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 93aa30b325..f00f3f709e 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -278,9 +278,7 @@ class IssuableFinder end end - # When filtering by multiple labels we may end up duplicating issues (if one - # has multiple labels). This ensures we only return unique issues. - items.distinct + items end def by_due_date(items) diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index 90476ab369..b1d49875ac 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -178,6 +178,19 @@ describe 'Issues', feature: true do expect(first_issue).to include('foo') end + + context 'with a filter on labels' do + let(:label) { create(:label, project: project) } + before { create(:label_link, label: label, target: foo) } + + it 'sorts by least recently due date by excluding nil due dates' do + bar.update(due_date: nil) + + visit namespace_project_issues_path(project.namespace, project, label_names: [label.name], sort: sort_value_due_date_later) + + expect(first_issue).to include('foo') + end + end end describe 'filtering by due date' do From c5cb68fd03dfb8caea7866d1e9f73af067229604 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Thu, 21 Apr 2016 19:13:06 +0200 Subject: [PATCH 36/78] Instrument Gitlab::GitAccess/GitAccessWiki --- CHANGELOG | 1 + config/initializers/metrics.rb | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 89572b9e15..9bdd6eb6a0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.0 (unreleased) v 8.7.0 (unreleased) + - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented - The number of InfluxDB points stored per UDP packet can now be configured - Fix error when cross-project label reference used with non-existent project - Transactions for /internal/allowed now have an "action" tag set diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb index 22fe51a453..283936d0ef 100644 --- a/config/initializers/metrics.rb +++ b/config/initializers/metrics.rb @@ -107,6 +107,10 @@ if Gitlab::Metrics.enabled? config.instrument_methods(const) config.instrument_instance_methods(const) end + + # Instrument the classes used for checking if somebody has push access. + config.instrument_instance_methods(Gitlab::GitAccess) + config.instrument_instance_methods(Gitlab::GitAccessWiki) end GC::Profiler.enable From 97baf41adc6338c45cca2a274539d155b767a538 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 18:46:12 +0100 Subject: [PATCH 37/78] Reverted link changes --- app/assets/javascripts/labels_select.js.coffee | 2 +- spec/helpers/labels_helper_spec.rb | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 0417ba823f..11e9227ba9 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -34,7 +34,7 @@ class @LabelsSelect if issueUpdateURL labelHTMLTemplate = _.template( '<% _.each(labels, function(label){ %> - issues?label_name[]=<%= _.escape(label.title) %>"> + issues?label_name=<%= _.escape(label.title) %>"> <%= _.escape(label.title) %> diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 501f150cfd..39042ff7e9 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -11,13 +11,13 @@ describe LabelsHelper do end it 'uses the instance variable' do - expect(link_to_label(label)).to match %r{} + expect(link_to_label(label)).to match %r{} end end context 'without @project set' do it "uses the label's project" do - expect(link_to_label(label)).to match %r{.*} + expect(link_to_label(label)).to match %r{.*} end end @@ -25,7 +25,7 @@ describe LabelsHelper do let(:another_project) { double('project', namespace: 'foo3', to_param: 'bar3') } it 'links to merge requests page' do - expect(link_to_label(label, project: another_project)).to match %r{.*} + expect(link_to_label(label, project: another_project)).to match %r{.*} end end @@ -33,7 +33,7 @@ describe LabelsHelper do ['issue', :issue, 'merge_request', :merge_request].each do |type| context "set to #{type}" do it 'links to correct page' do - expect(link_to_label(label, type: type)).to match %r{.*} + expect(link_to_label(label, type: type)).to match %r{.*} end end end From c079ae273c3fcab6f73b000a0e89d4faf233a9d9 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 21 Apr 2016 18:47:16 +0100 Subject: [PATCH 38/78] Reverted label link helper --- app/helpers/labels_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index c99b137cda..3dded7c2f2 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -37,7 +37,7 @@ module LabelsHelper link = send("namespace_project_#{type.to_s.pluralize}_path", project.namespace, project, - label_name: [label.name]) + label_name: label.name) if block_given? link_to link, &block From ac08a9f1e12e17a4f9fb94ca8a7b3fb6dbe348f0 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Mon, 18 Apr 2016 15:32:26 +0200 Subject: [PATCH 39/78] Add db:migrate task to CI Signed-off-by: Dmitriy Zaporozhets --- .gitlab-ci.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1dc49ca336..85730e1b68 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -115,6 +115,11 @@ bundler:audit: script: - "bundle exec bundle-audit check --update --ignore OSVDB-115941" +db-migrate-reset: + stage: test + script: + - RAILS_ENV=test bundle exec rake db:migrate:reset + # Ruby 2.2 jobs spec:feature:ruby22: From 72debd840cb76a5fc0929c0caeca179c39378680 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 21 Apr 2016 13:03:37 -0500 Subject: [PATCH 40/78] Fix failing spec --- lib/banzai/filter/external_link_filter.rb | 3 +-- spec/features/markdown_spec.rb | 7 ++++++- spec/lib/banzai/filter/external_link_filter_spec.rb | 10 +++++++++- 3 files changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/banzai/filter/external_link_filter.rb b/lib/banzai/filter/external_link_filter.rb index 699de56fd4..38c4219518 100644 --- a/lib/banzai/filter/external_link_filter.rb +++ b/lib/banzai/filter/external_link_filter.rb @@ -1,7 +1,6 @@ module Banzai module Filter - # HTML Filter to add a `rel="nofollow"` attribute to external links - # + # HTML Filter to modify the attributes of external links class ExternalLinkFilter < HTML::Pipeline::Filter def call doc.search('a').each do |node| diff --git a/spec/features/markdown_spec.rb b/spec/features/markdown_spec.rb index 3d0d0e59fd..0148c87084 100644 --- a/spec/features/markdown_spec.rb +++ b/spec/features/markdown_spec.rb @@ -165,7 +165,12 @@ describe 'GitLab Markdown', feature: true do describe 'ExternalLinkFilter' do it 'adds nofollow to external link' do link = doc.at_css('a:contains("Google")') - expect(link.attr('rel')).to match 'nofollow' + expect(link.attr('rel')).to include('nofollow') + end + + it 'adds noreferrer to external link' do + link = doc.at_css('a:contains("Google")') + expect(link.attr('rel')).to include('noreferrer') end it 'ignores internal link' do diff --git a/spec/lib/banzai/filter/external_link_filter_spec.rb b/spec/lib/banzai/filter/external_link_filter_spec.rb index e3a8e15330..f4c5c621bd 100644 --- a/spec/lib/banzai/filter/external_link_filter_spec.rb +++ b/spec/lib/banzai/filter/external_link_filter_spec.rb @@ -24,6 +24,14 @@ describe Banzai::Filter::ExternalLinkFilter, lib: true do doc = filter(act) expect(doc.at_css('a')).to have_attribute('rel') - expect(doc.at_css('a')['rel']).to eq 'nofollow' + expect(doc.at_css('a')['rel']).to include 'nofollow' + end + + it 'adds rel="noreferrer" to external links' do + act = %q(Google) + doc = filter(act) + + expect(doc.at_css('a')).to have_attribute('rel') + expect(doc.at_css('a')['rel']).to include 'noreferrer' end end From 8277bd79681e18d121d54c6552e20bd4744781b3 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Thu, 21 Apr 2016 13:41:47 -0500 Subject: [PATCH 41/78] Remove float from icon --- app/assets/stylesheets/framework/files.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/assets/stylesheets/framework/files.scss b/app/assets/stylesheets/framework/files.scss index 148a7cab6e..32cc15113f 100644 --- a/app/assets/stylesheets/framework/files.scss +++ b/app/assets/stylesheets/framework/files.scss @@ -131,6 +131,11 @@ td.line-numbers { float: none; border-left: 1px solid #ddd; + + i { + float: none; + margin-right: 0; + } } td.lines { padding: 0; From 3f574c4db958f2d1ea4d6475cb027dc005fbae87 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 21 Apr 2016 16:02:00 -0500 Subject: [PATCH 42/78] Use compact syntax --- app/controllers/concerns/filter_projects.rb | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/app/controllers/concerns/filter_projects.rb b/app/controllers/concerns/filter_projects.rb index 091412a7c9..586f97c5eb 100644 --- a/app/controllers/concerns/filter_projects.rb +++ b/app/controllers/concerns/filter_projects.rb @@ -10,10 +10,7 @@ module FilterProjects def filter_projects(projects) projects = projects.search(params[:filter_projects]) if params[:filter_projects].present? projects = projects.non_archived if params[:archived].blank? - - if params[:personal].present? and current_user - projects = projects.personal(current_user) - end + projects = projects.personal(current_user) if params[:personal].present? && current_user projects end From 41cee8c38e61d3f00e1c79b2d170d6fc5f8bea0e Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 21 Apr 2016 16:03:14 -0500 Subject: [PATCH 43/78] Do not display filters if user if there's no current_user --- app/views/shared/projects/_dropdown.html.haml | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/app/views/shared/projects/_dropdown.html.haml b/app/views/shared/projects/_dropdown.html.haml index 681d182dae..1169bed038 100644 --- a/app/views/shared/projects/_dropdown.html.haml +++ b/app/views/shared/projects/_dropdown.html.haml @@ -21,10 +21,11 @@ %li = link_to filter_projects_path(sort: @sort, archived: true), class: ("is-active" if params[:archived].present?) do Show archived projects - %li.divider - %li - = link_to filter_projects_path(sort: @sort, personal: nil), class: ("is-active" unless personal) do - Owned by anyone - %li - = link_to filter_projects_path(sort: @sort, personal: true), class: ("is-active" if personal) do - Owned by me + - if current_user + %li.divider + %li + = link_to filter_projects_path(sort: @sort, personal: nil), class: ("is-active" unless personal) do + Owned by anyone + %li + = link_to filter_projects_path(sort: @sort, personal: true), class: ("is-active" if personal) do + Owned by me From 80ae3790b456b12314243c75632ed6024ac16424 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 21 Apr 2016 16:03:21 -0500 Subject: [PATCH 44/78] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 290c956814..669a26bf04 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -86,6 +86,7 @@ v 8.7.0 (unreleased) - Updated print style for issues - Use GitHub Issue/PR number as iid to keep references - Import GitHub labels + - Add option to filter by "Owned projects" on dashboard page v 8.6.6 - Expire the exists cache before deletion to ensure project dir actually exists (Stan Hu). !3413 From 50ed43e49099648fa5c242d0cee15f05e50349c3 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 11 Mar 2016 16:14:29 +0000 Subject: [PATCH 45/78] Improved confirmation UX Closes #4228 --- CHANGELOG | 1 + app/assets/stylesheets/framework/buttons.scss | 4 ++++ app/assets/stylesheets/pages/confirmation.scss | 18 ++++++++++++++++++ app/controllers/confirmations_controller.rb | 9 +++++++++ app/controllers/registrations_controller.rb | 4 ++-- .../devise/confirmations/almost_there.haml | 10 ++++++++++ app/views/layouts/devise_empty.html.haml | 17 +++++++++++++++++ config/routes.rb | 1 + spec/features/signup_spec.rb | 4 ++-- 9 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 app/assets/stylesheets/pages/confirmation.scss create mode 100644 app/views/devise/confirmations/almost_there.haml create mode 100644 app/views/layouts/devise_empty.html.haml diff --git a/CHANGELOG b/CHANGELOG index ab8b86ba92..d78833d220 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -81,6 +81,7 @@ v 8.7.0 (unreleased) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) - Fix admin/projects when using visibility levels on search (PotHix) - Build status notifications + - Update email confirmation interface - API: Expose user location (Robert Schilling) - API: Do not leak group existence via return code (Robert Schilling) - ClosingIssueExtractor regex now also works with colons. e.g. "Fixes: #1234" !3591 diff --git a/app/assets/stylesheets/framework/buttons.scss b/app/assets/stylesheets/framework/buttons.scss index e8c0172680..18a74fe21a 100644 --- a/app/assets/stylesheets/framework/buttons.scss +++ b/app/assets/stylesheets/framework/buttons.scss @@ -144,6 +144,10 @@ } } +.btn-lg { + padding: 12px 20px; +} + .btn-transparent { color: $btn-transparent-color; background-color: transparent; diff --git a/app/assets/stylesheets/pages/confirmation.scss b/app/assets/stylesheets/pages/confirmation.scss new file mode 100644 index 0000000000..125f495d6d --- /dev/null +++ b/app/assets/stylesheets/pages/confirmation.scss @@ -0,0 +1,18 @@ +.well-confirmation { + margin-bottom: 20px; + border-bottom: 1px solid #eee; + + > h1 { + font-weight: 400; + } + + .lead { + margin-bottom: 20px; + } +} + +.confirmation-content { + a { + color: $md-link-color; + } +} diff --git a/app/controllers/confirmations_controller.rb b/app/controllers/confirmations_controller.rb index af1faca93f..7b66ad3f92 100644 --- a/app/controllers/confirmations_controller.rb +++ b/app/controllers/confirmations_controller.rb @@ -1,7 +1,16 @@ class ConfirmationsController < Devise::ConfirmationsController + def almost_there + flash[:notice] = nil + render layout: "devise_empty" + end + protected + def after_resending_confirmation_instructions_path_for(resource) + users_almost_there_path + end + def after_confirmation_path_for(resource_name, resource) if signed_in?(resource_name) after_sign_in_path_for(resource) diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index c48175a4c5..059b88e225 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -31,11 +31,11 @@ class RegistrationsController < Devise::RegistrationsController end def after_sign_up_path_for(_resource) - new_user_session_path + users_almost_there_path end def after_inactive_sign_up_path_for(_resource) - new_user_session_path + users_almost_there_path end private diff --git a/app/views/devise/confirmations/almost_there.haml b/app/views/devise/confirmations/almost_there.haml new file mode 100644 index 0000000000..3c3830a3f1 --- /dev/null +++ b/app/views/devise/confirmations/almost_there.haml @@ -0,0 +1,10 @@ +.well-confirmation.text-center + %h1.prepend-top-0 + Almost there... + %p.lead + Please check your email to confirm your account +%p.confirmation-content.text-center + No confirmation email received? Please check your spam folder or +.append-bottom-20.prepend-top-20.text-center + %a.btn.btn-lg.btn-success{ href: new_user_confirmation_path } + Request new confirmation email diff --git a/app/views/layouts/devise_empty.html.haml b/app/views/layouts/devise_empty.html.haml new file mode 100644 index 0000000000..7c061dd531 --- /dev/null +++ b/app/views/layouts/devise_empty.html.haml @@ -0,0 +1,17 @@ +!!! 5 +%html{ lang: "en"} + = render "layouts/head" + %body.ui_charcoal.login-page.application.navless + = render "layouts/header/empty" + = render "layouts/broadcast" + .container.navless-container + .content + = render "layouts/flash" + = yield + + %hr + .container + .footer-links + = link_to "Explore", explore_root_path + = link_to "Help", help_path + = link_to "About GitLab", "https://about.gitlab.com/" diff --git a/config/routes.rb b/config/routes.rb index 79b62a0b1b..2f820aafed 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -418,6 +418,7 @@ Rails.application.routes.draw do devise_scope :user do get '/users/auth/:provider/omniauth_error' => 'omniauth_callbacks#omniauth_error', as: :omniauth_error + get '/users/almost_there' => 'confirmations#almost_there' end root to: "root#index" diff --git a/spec/features/signup_spec.rb b/spec/features/signup_spec.rb index 01472743b2..51b754ff85 100644 --- a/spec/features/signup_spec.rb +++ b/spec/features/signup_spec.rb @@ -13,8 +13,8 @@ feature 'Signup', feature: true do fill_in 'user_password_sign_up', with: user.password click_button "Sign up" - expect(current_path).to eq user_session_path - expect(page).to have_content("A message with a confirmation link has been sent to your email address.") + expect(current_path).to eq users_almost_there_path + expect(page).to have_content("Please check your email to confirm your account") end end From 4adfd501a5d31abd16bccf08586bf8a125b03450 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 12:20:05 +0200 Subject: [PATCH 46/78] Verify label affiliation before assigning to issue This also verify if milestone belongs to correct project before creating a new issue. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/15439 --- app/services/issuable_base_service.rb | 28 +++++++++++++++++++-- spec/services/issues/create_service_spec.rb | 28 +++++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 18f76d3f65..ab110001f9 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -37,8 +37,9 @@ class IssuableBaseService < BaseService end def filter_params(issuable_ability_name = :issue) - params[:assignee_id] = "" if params[:assignee_id] == IssuableFinder::NONE - params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE + filter_assignee + filter_milestone + filter_labels ability = :"admin_#{issuable_ability_name}" @@ -49,6 +50,29 @@ class IssuableBaseService < BaseService end end + def filter_assignee + if params[:assignee_id] == IssuableFinder::NONE + params[:assignee_id] = '' + end + end + + def filter_milestone + return unless params[:milestone_id] + + if params[:milestone_id] == IssuableFinder::NONE || + Milestone.find(params[:milestone_id]).try(:project) != project + params[:milestone_id] = '' + end + end + + def filter_labels + return if params[:label_ids].to_a.empty? + + params[:label_ids].select! do |label_id| + Label.find(label_id).try(:project) == project + end + end + def update(issuable) change_state(issuable) filter_params diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 5e7915db7e..d11c45df8f 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -37,6 +37,34 @@ describe Issues::CreateService, services: true do expect(Todo.where(attributes).count).to eq 1 end + + context 'label that belongs to different project' do + let(:issue) { Issues::CreateService.new(project, user, opts).execute } + let(:label) { create(:label) } + let(:opts) do + { title: 'Title', + description: 'Description', + label_ids: [label.id] } + end + + it 'does not assign label'do + expect(issue.labels).to_not include label + end + end + + context 'milestone that belongs to different project' do + let(:issue) { Issues::CreateService.new(project, user, opts).execute } + let(:milestone) { create(:milestone) } + let(:opts) do + { title: 'Title', + description: 'Description', + milestone_id: milestone.id } + end + + it 'does not assign label' do + expect(issue.milestone).to_not eq milestone + end + end end end end From 8df63d1513118f0b3643365e2950a68c80e46176 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 12:25:21 +0200 Subject: [PATCH 47/78] Update specs to accomodate issuable assign changes --- .../issues/bulk_update_service_spec.rb | 2 +- spec/services/issues/create_service_spec.rb | 32 +++++++++---------- spec/services/issues/update_service_spec.rb | 11 +++++-- .../merge_requests/update_service_spec.rb | 11 +++++-- 4 files changed, 33 insertions(+), 23 deletions(-) diff --git a/spec/services/issues/bulk_update_service_spec.rb b/spec/services/issues/bulk_update_service_spec.rb index 6a7ea4b2f4..e91906d0d4 100644 --- a/spec/services/issues/bulk_update_service_spec.rb +++ b/spec/services/issues/bulk_update_service_spec.rb @@ -100,7 +100,7 @@ describe Issues::BulkUpdateService, services: true do describe :update_milestone do before do - @milestone = create :milestone + @milestone = create(:milestone, project: @project) @params = { issues_ids: [issue.id], milestone_id: @milestone.id diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index d11c45df8f..64921c5f47 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -6,31 +6,31 @@ describe Issues::CreateService, services: true do let(:assignee) { create(:user) } describe :execute do + let(:issue) { Issues::CreateService.new(project, user, opts).execute } + context 'valid params' do before do project.team << [user, :master] project.team << [assignee, :master] - - opts = { - title: 'Awesome issue', - description: 'please fix', - assignee: assignee - } - - @issue = Issues::CreateService.new(project, user, opts).execute end - it { expect(@issue).to be_valid } - it { expect(@issue.title).to eq('Awesome issue') } - it { expect(@issue.assignee).to eq assignee } + let(:opts) do + { title: 'Awesome issue', + description: 'please fix', + assignee: assignee } + end + + it { expect(issue).to be_valid } + it { expect(issue.title).to eq('Awesome issue') } + it { expect(issue.assignee).to eq assignee } it 'creates a pending todo for new assignee' do attributes = { project: project, author: user, user: assignee, - target_id: @issue.id, - target_type: @issue.class.name, + target_id: issue.id, + target_type: issue.class.name, action: Todo::ASSIGNED, state: :pending } @@ -39,8 +39,8 @@ describe Issues::CreateService, services: true do end context 'label that belongs to different project' do - let(:issue) { Issues::CreateService.new(project, user, opts).execute } let(:label) { create(:label) } + let(:opts) do { title: 'Title', description: 'Description', @@ -53,15 +53,15 @@ describe Issues::CreateService, services: true do end context 'milestone that belongs to different project' do - let(:issue) { Issues::CreateService.new(project, user, opts).execute } let(:milestone) { create(:milestone) } + let(:opts) do { title: 'Title', description: 'Description', milestone_id: milestone.id } end - it 'does not assign label' do + it 'does not assign milestone' do expect(issue.milestone).to_not eq milestone end end diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 6b214a0d96..52f6930699 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -4,10 +4,15 @@ describe Issues::UpdateService, services: true do let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } - let(:issue) { create(:issue, title: 'Old title', assignee_id: user3.id) } - let(:label) { create(:label) } + let(:project) { create(:empty_project) } + let(:label) { create(:label, project: project) } let(:label2) { create(:label) } - let(:project) { issue.project } + + let(:issue) do + create(:issue, title: 'Old title', + assignee_id: user3.id, + project: project) + end before do project.team << [user, :master] diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index cb8cff2fa8..213e8c2eb3 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -1,14 +1,19 @@ require 'spec_helper' describe MergeRequests::UpdateService, services: true do + let(:project) { create(:project) } let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } - let(:merge_request) { create(:merge_request, :simple, title: 'Old title', assignee_id: user3.id) } - let(:project) { merge_request.project } - let(:label) { create(:label) } + let(:label) { create(:label, project: project) } let(:label2) { create(:label) } + let(:merge_request) do + create(:merge_request, :simple, title: 'Old title', + assignee_id: user3.id, + source_project: project) + end + before do project.team << [user, :master] project.team << [user2, :developer] From 006d8b268ca2964ca3363fae53fdff7d26a9ae08 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 12:46:48 +0200 Subject: [PATCH 48/78] Add affinity checks in issue create service --- spec/services/issues/create_service_spec.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 64921c5f47..2972c38783 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -3,12 +3,15 @@ require 'spec_helper' describe Issues::CreateService, services: true do let(:project) { create(:empty_project) } let(:user) { create(:user) } - let(:assignee) { create(:user) } describe :execute do let(:issue) { Issues::CreateService.new(project, user, opts).execute } context 'valid params' do + let(:assignee) { create(:user) } + let(:milestone) { create(:milestone, project: project) } + let(:labels) { create_list(:label, 4, project: project) } + before do project.team << [user, :master] project.team << [assignee, :master] @@ -17,12 +20,16 @@ describe Issues::CreateService, services: true do let(:opts) do { title: 'Awesome issue', description: 'please fix', - assignee: assignee } + assignee: assignee, + label_ids: labels.map(&:id), + milestone_id: milestone.id } end it { expect(issue).to be_valid } it { expect(issue.title).to eq('Awesome issue') } it { expect(issue.assignee).to eq assignee } + it { expect(issue.labels).to match_array labels } + it { expect(issue.milestone).to eq milestone } it 'creates a pending todo for new assignee' do attributes = { From 01ab6d704c36b211b5a06e290e6912346c8bf496 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 13:40:52 +0200 Subject: [PATCH 49/78] Use association search in issuable create service --- app/services/issuable_base_service.rb | 12 ++++++------ spec/services/issues/create_service_spec.rb | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index ab110001f9..2b16089df1 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -57,10 +57,11 @@ class IssuableBaseService < BaseService end def filter_milestone - return unless params[:milestone_id] + milestone_id = params[:milestone_id] + return unless milestone_id - if params[:milestone_id] == IssuableFinder::NONE || - Milestone.find(params[:milestone_id]).try(:project) != project + if milestone_id == IssuableFinder::NONE || + project.milestones.find_by(id: milestone_id).nil? params[:milestone_id] = '' end end @@ -68,9 +69,8 @@ class IssuableBaseService < BaseService def filter_labels return if params[:label_ids].to_a.empty? - params[:label_ids].select! do |label_id| - Label.find(label_id).try(:project) == project - end + params[:label_ids] = + project.labels.where(id: params[:label_ids]).pluck(:id) end def update(issuable) diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 2972c38783..ac28b6f71f 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -4,13 +4,13 @@ describe Issues::CreateService, services: true do let(:project) { create(:empty_project) } let(:user) { create(:user) } - describe :execute do - let(:issue) { Issues::CreateService.new(project, user, opts).execute } + describe '#execute' do + let(:issue) { described_class.new(project, user, opts).execute } - context 'valid params' do + context 'when params are valid' do let(:assignee) { create(:user) } let(:milestone) { create(:milestone, project: project) } - let(:labels) { create_list(:label, 4, project: project) } + let(:labels) { create_pair(:label, project: project) } before do project.team << [user, :master] @@ -45,7 +45,7 @@ describe Issues::CreateService, services: true do expect(Todo.where(attributes).count).to eq 1 end - context 'label that belongs to different project' do + context 'when label belongs to different project' do let(:label) { create(:label) } let(:opts) do @@ -59,7 +59,7 @@ describe Issues::CreateService, services: true do end end - context 'milestone that belongs to different project' do + context 'when milestone belongs to different project' do let(:milestone) { create(:milestone) } let(:opts) do From 55df95c3886b42e92b0079b4d9d5eef0011f44d5 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 21 Apr 2016 19:44:19 +0200 Subject: [PATCH 50/78] Add Changelog entry for private labels security fix --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index ab8b86ba92..e7428834c1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,7 @@ v 8.8.0 (unreleased) v 8.7.0 (unreleased) - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented + - Fix vulnerability that made it possible to gain access to private labels and milestones - The number of InfluxDB points stored per UDP packet can now be configured - Fix error when cross-project label reference used with non-existent project - Transactions for /internal/allowed now have an "action" tag set From de39959f559d8f1be73a11714abcd2782f476fea Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Fri, 22 Apr 2016 10:42:30 +0300 Subject: [PATCH 51/78] Use new Note styleguide [ci skip] --- doc/workflow/cherry_pick_changes.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/doc/workflow/cherry_pick_changes.md b/doc/workflow/cherry_pick_changes.md index b0ca087964..4a49900984 100644 --- a/doc/workflow/cherry_pick_changes.md +++ b/doc/workflow/cherry_pick_changes.md @@ -1,6 +1,7 @@ # Cherry-pick changes -_**Note:** This feature was [introduced][ce-3514] in GitLab 8.7._ +>**Note:** +This feature was [introduced][ce-3514] in GitLab 8.7. --- From 846a9ef5ad5d946fefa135ce31efefab384f760c Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Fri, 22 Apr 2016 11:20:37 +0300 Subject: [PATCH 52/78] Add newest enhancements to GH importer docs [ci skip] --- .../importing/import_projects_from_github.md | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/doc/workflow/importing/import_projects_from_github.md b/doc/workflow/importing/import_projects_from_github.md index f693f430a4..e670e415c7 100644 --- a/doc/workflow/importing/import_projects_from_github.md +++ b/doc/workflow/importing/import_projects_from_github.md @@ -1,7 +1,8 @@ # Import your project from GitHub to GitLab -_**Note:** In order to enable the GitHub import setting, you should first -enable the [GitHub integration][gh-import] in your GitLab instance._ +>**Note:** +In order to enable the GitHub import setting, you should first +enable the [GitHub integration][gh-import] in your GitLab instance. At its current state, GitHub importer can import: @@ -10,10 +11,13 @@ At its current state, GitHub importer can import: - the issues (introduced in GitLab 7.7) - the pull requests (introduced in GitLab 8.4) - the wiki pages (introduced in GitLab 8.4) +- the milestones (introduced in GitLab 8.7) +- the labels (introduced in GitLab 8.7) -It is not yet possible to import your labels, milestones and cross-repository -pull requests (those from forks). We are working on improving this in the near -future. +With GitLab 8.7+, references to pull requests and issues are preserved. + +It is not yet possible to import your cross-repository pull requests (those from +forks). We are working on improving this in the near future. The importer page is visible when you [create a new project][new-project]. Click on the **GitHub** link and you will be redirected to GitHub for From e0c07b5e3d1e14fa6efda3c13db46477d79c7c34 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Fri, 22 Apr 2016 06:19:43 -0500 Subject: [PATCH 53/78] Add tests for filtering owned projects --- .../dashboard/user_filters_projects_spec.rb | 27 +++++++++++++++++ spec/features/dashboard_filter_spec.rb | 30 ------------------- 2 files changed, 27 insertions(+), 30 deletions(-) create mode 100644 spec/features/dashboard/user_filters_projects_spec.rb delete mode 100644 spec/features/dashboard_filter_spec.rb diff --git a/spec/features/dashboard/user_filters_projects_spec.rb b/spec/features/dashboard/user_filters_projects_spec.rb new file mode 100644 index 0000000000..cf86e2c85e --- /dev/null +++ b/spec/features/dashboard/user_filters_projects_spec.rb @@ -0,0 +1,27 @@ +require 'spec_helper' + +describe "Dashboard > User filters projects", feature: true do + + describe 'filtering personal projects' do + before do + user = create(:user) + project = create(:project, name: "Victorialand", namespace: user.namespace) + project.team << [user, :master] + + user2 = create(:user) + project2 = create(:project, name: "Treasure", namespace: user2.namespace) + project2.team << [user, :developer] + + login_as(user) + visit dashboard_projects_path + end + + it 'filters by projects "Owned by me"' do + click_link "Owned by me" + + expect(page).to have_css('.is-active', text: 'Owned by me') + expect(page).to have_content('Victorialand') + expect(page).not_to have_content('Treasure') + end + end +end diff --git a/spec/features/dashboard_filter_spec.rb b/spec/features/dashboard_filter_spec.rb deleted file mode 100644 index 3d20813161..0000000000 --- a/spec/features/dashboard_filter_spec.rb +++ /dev/null @@ -1,30 +0,0 @@ -require 'spec_helper' - -describe "Dashboard projects filters", feature: true, js: true do - - context 'filtering personal projects' do - before do - user = create(:user) - project = create(:project, name: "Victorialand", namespace: user.namespace) - project.team << [user, :master] - - login_as(user) - visit dashboard_projects_path - - open_filter_dropdown - click_link "Owned by me" - end - - it 'filters by projects "Owned by me"' do - sleep 1 - open_filter_dropdown - page.within('ul.dropdown-menu.dropdown-menu-align-right') do - expect(page).to have_css('.is-active', text: 'Owned by me') - end - end - end - - def open_filter_dropdown - find('button.dropdown-toggle.btn').click - end -end From b711fc0abdd90fec4ff13cd169e9ad9e0aedb525 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 22 Apr 2016 13:34:03 +0200 Subject: [PATCH 54/78] Move cherry-pick feature specs to proper directory --- spec/features/{project => projects}/commits/cherry_pick_spec.rb | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/features/{project => projects}/commits/cherry_pick_spec.rb (100%) diff --git a/spec/features/project/commits/cherry_pick_spec.rb b/spec/features/projects/commits/cherry_pick_spec.rb similarity index 100% rename from spec/features/project/commits/cherry_pick_spec.rb rename to spec/features/projects/commits/cherry_pick_spec.rb From 32be8bd2b105885d88e1ad8d5bf1296f974031ab Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 22 Apr 2016 13:30:58 +0100 Subject: [PATCH 55/78] Fixed issue with assignee object not being returned Closes #15515 --- app/controllers/projects/issues_controller.rb | 2 +- .../projects/merge_requests_controller.rb | 2 +- spec/features/issues_spec.rb | 21 +++++++++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 18e28fc49f..7d4fc361ce 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -109,7 +109,7 @@ class Projects::IssuesController < Projects::ApplicationController end end format.json do - render json: @issue.to_json(include: [:milestone, labels: { methods: :text_color }, assignee: { methods: :avatar_url }]) + render json: @issue.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }) end end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index a57fd2d4e3..9c147b3689 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -154,7 +154,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.target_project, @merge_request]) end format.json do - render json: @merge_request.to_json(include: [:milestone, labels: { methods: :text_color }, assignee: { methods: :avatar_url }]) + render json: @merge_request.to_json(include: { milestone: {}, assignee: { methods: :avatar_url }, labels: { methods: :text_color } }) end end else diff --git a/spec/features/issues_spec.rb b/spec/features/issues_spec.rb index b1d49875ac..b57131f68d 100644 --- a/spec/features/issues_spec.rb +++ b/spec/features/issues_spec.rb @@ -317,6 +317,27 @@ describe 'Issues', feature: true do expect(issue.reload.assignee).to be_nil end + + it 'allows user to select an assignee', js: true do + issue2 = create(:issue, project: project, author: @user) + visit namespace_project_issue_path(project.namespace, project, issue2) + + page.within('.assignee') do + expect(page).to have_content "No assignee" + end + + page.within '.assignee' do + click_link 'Edit' + end + + page.within '.dropdown-menu-user' do + click_link @user.name + end + + page.within('.assignee') do + expect(page).to have_content @user.name + end + end end context 'by unauthorized user' do From 83adcf19b77898d3033500920973ed73ceb7c3c0 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 22 Apr 2016 14:32:14 +0100 Subject: [PATCH 56/78] Escapes label title in filters Fixes #15522 --- app/assets/javascripts/labels_select.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/labels_select.js.coffee b/app/assets/javascripts/labels_select.js.coffee index 516f7d1993..85517b18c5 100644 --- a/app/assets/javascripts/labels_select.js.coffee +++ b/app/assets/javascripts/labels_select.js.coffee @@ -223,7 +223,7 @@ class @LabelsSelect fieldName: $dropdown.data('field-name') id: (label) -> if $dropdown.hasClass("js-filter-submit") and not label.isAny? - label.title + _.escape label.title else label.id From 951e3459e1d7c0f0caa5371d9c5edf7d0168168f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 22 Apr 2016 15:44:48 +0200 Subject: [PATCH 57/78] Use the `can?` helper instead of `current_user.can?` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #15513. Signed-off-by: Rémy Coutable --- CHANGELOG | 5 ++++- .../_shared_group_members.html.haml | 2 +- .../anonymous_user_sees_members_spec.rb | 20 +++++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 spec/features/projects/members/anonymous_user_sees_members_spec.rb diff --git a/CHANGELOG b/CHANGELOG index 361e33eb6b..da83eab2de 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,7 +2,10 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.8.0 (unreleased) -v 8.7.0 (unreleased) +v 8.7.1 (unreleased) + - Use the `can?` helper instead of `current_user.can?` + +v 8.7.0 - Gitlab::GitAccess and Gitlab::GitAccessWiki are now instrumented - Fix vulnerability that made it possible to gain access to private labels and milestones - The number of InfluxDB points stored per UDP packet can now be configured diff --git a/app/views/projects/project_members/_shared_group_members.html.haml b/app/views/projects/project_members/_shared_group_members.html.haml index 62888e4193..ae13f8428f 100644 --- a/app/views/projects/project_members/_shared_group_members.html.haml +++ b/app/views/projects/project_members/_shared_group_members.html.haml @@ -8,7 +8,7 @@ group, members with %strong #{group_links.human_access} role (#{shared_group_users_count}) - - if current_user.can?(:admin_group, shared_group) + - if can?(current_user, :admin_group, shared_group) .panel-head-actions = link_to group_group_members_path(shared_group), class: 'btn btn-sm' do %i.fa.fa-pencil-square-o diff --git a/spec/features/projects/members/anonymous_user_sees_members_spec.rb b/spec/features/projects/members/anonymous_user_sees_members_spec.rb new file mode 100644 index 0000000000..c5e3d143d9 --- /dev/null +++ b/spec/features/projects/members/anonymous_user_sees_members_spec.rb @@ -0,0 +1,20 @@ +require 'spec_helper' + +feature 'Projects > Members > Anonymous user sees members', feature: true do + let(:user) { create(:user) } + let(:group) { create(:group, :public) } + let(:project) { create(:empty_project, :public) } + + background do + project.team << [user, :master] + create(:project_group_link, project: project, group: group) + end + + scenario "anonymous user visits the project's members page and sees the list of members" do + visit namespace_project_project_members_path(project.namespace, project) + + expect(current_path).to eq( + namespace_project_project_members_path(project.namespace, project)) + expect(page).to have_content(user.name) + end +end From f063c3184b3b6249098b27ee14f2473d79404903 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Fri, 22 Apr 2016 18:15:03 +0300 Subject: [PATCH 58/78] Bump gitlab-shell to 2.7.2 [ci skip] --- doc/update/8.6-to-8.7.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/update/8.6-to-8.7.md b/doc/update/8.6-to-8.7.md index 8599133a72..8cff613327 100644 --- a/doc/update/8.6-to-8.7.md +++ b/doc/update/8.6-to-8.7.md @@ -46,7 +46,7 @@ sudo -u git -H git checkout 8-7-stable-ee ```bash cd /home/git/gitlab-shell sudo -u git -H git fetch --all -sudo -u git -H git checkout v2.7.0 +sudo -u git -H git checkout v2.7.2 ``` ### 5. Update gitlab-workhorse From bdd844f312a7bd3d40ae1a0eb29a3c376b3dae27 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 22 Apr 2016 13:04:05 -0400 Subject: [PATCH 59/78] Refactor Todos feature spec to be faster and less brittle We now only create two Todos instead of 21 when testing pagination, and we've updated the test to be less brittle when dealing with slower CI environments. --- spec/features/todos/todos_spec.rb | 38 ++++++++++++++++--------------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb index 113d4c40cf..248e004ba6 100644 --- a/spec/features/todos/todos_spec.rb +++ b/spec/features/todos/todos_spec.rb @@ -1,12 +1,10 @@ require 'spec_helper' describe 'Dashboard Todos', feature: true do - let(:user){ create(:user) } - let(:author){ create(:user) } - let(:project){ create(:project) } - let(:issue){ create(:issue) } - let(:todos_per_page){ Todo.default_per_page } - let(:todos_total){ todos_per_page + 1 } + let(:user) { create(:user) } + let(:author) { create(:user) } + let(:project) { create(:project) } + let(:issue) { create(:issue) } describe 'GET /dashboard/todos' do context 'User does not have todos' do @@ -46,31 +44,35 @@ describe 'Dashboard Todos', feature: true do end context 'User has multiple pages of Todos' do - let(:todo_total_pages){ (todos_total.to_f/todos_per_page).ceil } - before do - todos_total.times do - create(:todo, :mentioned, user: user, project: project, target: issue, author: author) - end + allow(Todo).to receive(:default_per_page).and_return(1) + + # Create just enough records to cause us to paginate + create_list(:todo, 2, :mentioned, user: user, project: project, target: issue, author: author) login_as(user) - visit dashboard_todos_path end it 'is paginated' do + visit dashboard_todos_path + expect(page).to have_selector('.gl-pagination') end it 'is has the right number of pages' do - expect(page).to have_selector('.gl-pagination .page', count: todo_total_pages) + visit dashboard_todos_path + + expect(page).to have_selector('.gl-pagination .page', count: 2) end - describe 'deleting last todo from last page', js: true do + describe 'completing last todo from last page', js: true do it 'redirects to the previous page' do - page.within('.gl-pagination') do - click_link todo_total_pages.to_s - end - first('.done-todo').click + visit dashboard_todos_path(page: 2) + expect(page).to have_content(Todo.first.body) + + click_link('Done') + + expect(current_path).to eq dashboard_todos_path expect(page).to have_content(Todo.last.body) end end From 3e16997fd7cbefaa406e73e7b37f1c3516b2e016 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Mon, 18 Apr 2016 16:23:11 -0500 Subject: [PATCH 60/78] Add Sidekiq troubleshooting docs [ci skip] --- doc/README.md | 1 + doc/administration/troubleshooting/sidekiq.md | 162 ++++++++++++++++++ 2 files changed, 163 insertions(+) create mode 100644 doc/administration/troubleshooting/sidekiq.md diff --git a/doc/README.md b/doc/README.md index e6ac479482..45c2da7d79 100644 --- a/doc/README.md +++ b/doc/README.md @@ -41,6 +41,7 @@ - [Git LFS configuration](workflow/lfs/lfs_administration.md) - [Housekeeping](administration/housekeeping.md) Keep your Git repository tidy and fast. - [GitLab Performance Monitoring](monitoring/performance/introduction.md) Configure GitLab and InfluxDB for measuring performance metrics +- [Sidekiq Troubleshooting](administration/troubleshooting/sidekiq.md) Debug when Sidekiq appears hung and is not processing jobs ## Contributor documentation diff --git a/doc/administration/troubleshooting/sidekiq.md b/doc/administration/troubleshooting/sidekiq.md new file mode 100644 index 0000000000..134a758376 --- /dev/null +++ b/doc/administration/troubleshooting/sidekiq.md @@ -0,0 +1,162 @@ +# Troubleshooting Sidekiq + +Sidekiq is the background job processor GitLab uses to asynchronously run +tasks. When things go wrong it can be difficult to troubleshoot. These +situations also tend to be high-pressure because a production system job queue +may be filling up. Users will notice when this happens because new branches +may not show up and merge requests may not be updated. The following are some +troubleshooting steps that will help you diagnose the bottleneck. + +> **Note:** GitLab administrators/users should consider working through these +debug steps with GitLab Support so the backtraces can be analyzed by our team. +It may reveal a bug or necessary improvement in GitLab. + +> **Note:** In any of the backtraces, be weary of suspecting cases where every + thread appears to be waiting in the database, Redis, or waiting to acquire + a mutex. This **may** mean there's contention in the database, for example, + but look for one thread that is different than the rest. This other thread + may be using all available CPU, or have a Ruby Global Interpreter Lock, + preventing other threads from continuing. + +## Thread dump + +Send the Sidekiq process ID the `TTIN` signal and it will output thread +backtraces in the log file. + +``` +kill -TTIN +``` + +Check in `/var/log/gitlab/sidekiq/current` or `$GITLAB_HOME/log/sidekiq.log` for +the backtrace output. The backtraces will be lengthy and generally start with +several `WARN` level messages. Here's an example of a single thread's backtrace: + +``` +2016-04-13T06:21:20.022Z 31517 TID-orn4urby0 WARN: ActiveRecord::RecordNotFound: Couldn't find Note with 'id'=3375386 +2016-04-13T06:21:20.022Z 31517 TID-orn4urby0 WARN: /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/activerecord-4.2.5.2/lib/active_record/core.rb:155:in `find' +/opt/gitlab/embedded/service/gitlab-rails/app/workers/new_note_worker.rb:7:in `perform' +/opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/sidekiq-4.0.1/lib/sidekiq/processor.rb:150:in `execute_job' +/opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/sidekiq-4.0.1/lib/sidekiq/processor.rb:132:in `block (2 levels) in process' +/opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/sidekiq-4.0.1/lib/sidekiq/middleware/chain.rb:127:in `block in invoke' +/opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/sidekiq_middleware/memory_killer.rb:17:in `call' +/opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/sidekiq-4.0.1/lib/sidekiq/middleware/chain.rb:129:in `block in invoke' +/opt/gitlab/embedded/service/gitlab-rails/lib/gitlab/sidekiq_middleware/arguments_logger.rb:6:in `call' +... +``` + +In some cases Sidekiq may be hung and unable to respond to the `TTIN` signal. +Move on to other troubleshooting methods if this happens. + +## Process profiling with `perf` + +Linux has a process profiling tool called `perf` that is helpful when a certain +process is eating up a lot of CPU. If you see high CPU usage and Sidekiq won't +respond to the `TTIN` signal, this is a good next step. + +If `perf` is not installed on your system, install it with `apt-get` or `yum`: + +``` +# Debian +sudo apt-get install linux-tools + +# Ubuntu (may require these additional Kernel packages) +sudo apt-get install linux-tools-common linux-tools-generic linux-tools-`uname -r` + +# Red Hat/CentOS +sudo yum install perf +``` + +Run perf against the Sidekiq PID: + +``` +sudo perf record -p +``` + +Let this run for 30-60 seconds and then press Ctrl-C. Then view the perf report: + +``` +sudo perf report + +# Sample output +Samples: 348K of event 'cycles', Event count (approx.): 280908431073 + 97.69% ruby nokogiri.so [.] xmlXPathNodeSetMergeAndClear + 0.18% ruby libruby.so.2.1.0 [.] objspace_malloc_increase + 0.12% ruby libc-2.12.so [.] _int_malloc + 0.10% ruby libc-2.12.so [.] _int_free +``` + +Above you see sample output from a perf report. It shows that 97% of the CPU is +being spent inside Nokogiri and `xmlXPathNodeSetMergeAndClear`. For something +this obvious you should then go investigate what job in GitLab would use +Nokogiri and XPath. Combine with `TTIN` or `gdb` output to show the +corresponding Ruby code where this is happening. + +## The GNU Project Debugger (gdb) + +`gdb` can be another effective tool for debugging Sidekiq. It gives you a little +more interactive way to look at each thread and see what's causing problems. + +> **Note:** Attaching to a process with `gdb` will suspends the normal operation + of the process (Sidekiq will not process jobs while `gdb` is attached). + +Start by attaching to the Sidekiq PID: + +``` +gdb -p +``` + +Then gather information on all the threads: + +``` +info threads + +# Example output +30 Thread 0x7fe5fbd63700 (LWP 26060) 0x0000003f7cadf113 in poll () from /lib64/libc.so.6 +29 Thread 0x7fe5f2b3b700 (LWP 26533) 0x0000003f7ce0b68c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 +28 Thread 0x7fe5f2a3a700 (LWP 26534) 0x0000003f7ce0ba5e in pthread_cond_timedwait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 +27 Thread 0x7fe5f2939700 (LWP 26535) 0x0000003f7ce0b68c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 +26 Thread 0x7fe5f2838700 (LWP 26537) 0x0000003f7ce0b68c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 +25 Thread 0x7fe5f2737700 (LWP 26538) 0x0000003f7ce0b68c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 +24 Thread 0x7fe5f2535700 (LWP 26540) 0x0000003f7ce0b68c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 +23 Thread 0x7fe5f2434700 (LWP 26541) 0x0000003f7ce0b68c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 +22 Thread 0x7fe5f2232700 (LWP 26543) 0x0000003f7ce0b68c in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0 +21 Thread 0x7fe5f2131700 (LWP 26544) 0x00007fe5f7b570f0 in xmlXPathNodeSetMergeAndClear () +from /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/nokogiri-1.6.7.2/lib/nokogiri/nokogiri.so +... +``` + +If you see a suspicious thread, like the Nokogiri one above, you may want +to get more information: + +``` +thread 21 +bt + +# Example output +#0 0x00007ff0d6afe111 in xmlXPathNodeSetMergeAndClear () from /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/nokogiri-1.6.7.2/lib/nokogiri/nokogiri.so +#1 0x00007ff0d6b0b836 in xmlXPathNodeCollectAndTest () from /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/nokogiri-1.6.7.2/lib/nokogiri/nokogiri.so +#2 0x00007ff0d6b09037 in xmlXPathCompOpEval () from /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/nokogiri-1.6.7.2/lib/nokogiri/nokogiri.so +#3 0x00007ff0d6b09017 in xmlXPathCompOpEval () from /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/nokogiri-1.6.7.2/lib/nokogiri/nokogiri.so +#4 0x00007ff0d6b092e0 in xmlXPathCompOpEval () from /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/nokogiri-1.6.7.2/lib/nokogiri/nokogiri.so +#5 0x00007ff0d6b0bc37 in xmlXPathRunEval () from /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/nokogiri-1.6.7.2/lib/nokogiri/nokogiri.so +#6 0x00007ff0d6b0be5f in xmlXPathEvalExpression () from /opt/gitlab/embedded/service/gem/ruby/2.1.0/gems/nokogiri-1.6.7.2/lib/nokogiri/nokogiri.so +#7 0x00007ff0d6a97dc3 in evaluate (argc=2, argv=0x1022d058, self=) at xml_xpath_context.c:221 +#8 0x00007ff0daeab0ea in vm_call_cfunc_with_frame (th=0x1022a4f0, reg_cfp=0x1032b810, ci=) at vm_insnhelper.c:1510 +``` + +To output a backtrace from all threads at once: + +``` +apply all thread bt +``` + +## Check for blocking queries + +Sometimes the speed at which Sidekiq processes jobs can be so fast that it can +cause database contention. Check for blocking queries when backtraces above +show that many threads are stuck in the database adapter. + +The PostgreSQL wiki has details on the query you can run to see blocking +queries. The query is different based on PostgreSQL version. See +[Lock Monitoring](https://wiki.postgresql.org/wiki/Lock_Monitoring) for +the query details. From 34ea1d2cfe84e6625672e1ba20d69cb3afcc3008 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Fri, 22 Apr 2016 13:18:06 -0500 Subject: [PATCH 61/78] New HA docs [ci skip] --- doc/README.md | 1 + .../high_availability/README.md | 35 +++++ .../high_availability/database.md | 116 ++++++++++++++++ .../high_availability/gitlab.md | 131 ++++++++++++++++++ .../high_availability/load_balancer.md | 63 +++++++++ doc/administration/high_availability/nfs.md | 116 ++++++++++++++++ doc/administration/high_availability/redis.md | 62 +++++++++ 7 files changed, 524 insertions(+) create mode 100644 doc/administration/high_availability/README.md create mode 100644 doc/administration/high_availability/database.md create mode 100644 doc/administration/high_availability/gitlab.md create mode 100644 doc/administration/high_availability/load_balancer.md create mode 100644 doc/administration/high_availability/nfs.md create mode 100644 doc/administration/high_availability/redis.md diff --git a/doc/README.md b/doc/README.md index 45c2da7d79..f01a2a7f94 100644 --- a/doc/README.md +++ b/doc/README.md @@ -42,6 +42,7 @@ - [Housekeeping](administration/housekeeping.md) Keep your Git repository tidy and fast. - [GitLab Performance Monitoring](monitoring/performance/introduction.md) Configure GitLab and InfluxDB for measuring performance metrics - [Sidekiq Troubleshooting](administration/troubleshooting/sidekiq.md) Debug when Sidekiq appears hung and is not processing jobs +- [High Availability](administration/high-availability/README.md) Configure multiple servers for scaling or high availability ## Contributor documentation diff --git a/doc/administration/high_availability/README.md b/doc/administration/high_availability/README.md new file mode 100644 index 0000000000..43d85ffb77 --- /dev/null +++ b/doc/administration/high_availability/README.md @@ -0,0 +1,35 @@ +# High Availability + +GitLab supports several different types of clustering and high-availability. +The solution you choose will be based on the level of scalability and +availability you require. The easiest solutions are scalable, but not necessarily +highly available. + +## Architecture + +### Active/Passive + +For pure high-availability/failover with no scaling you can use an +active/passive configuration. This utilizes DRBD (Distributed Replicated +Block Device) to keep all data in sync. DRBD requires a low latency link to +remain in sync. It is not advisable to attempt to run DRBD between data centers +or in different cloud availability zones. + +Components/Servers Required: + +- 2 servers/virtual machines (one active/one passive) + +### Active/Active + +This architecture scales easily because all application servers handle +user requests simultaneously. The database, Redis, and GitLab application are +all deployed on separate servers. The configuration is **only** highly-available +if the database, Redis and storage are also configured as such. + +**Steps to configure active/active:** + +1. [Configure the database](database.md) +1. [Configure Redis](redis.md) +1. [Configure NFS](nfs.md) +1. [Configure the GitLab application servers](gitlab.md) +1. [Configure the load balancers](load_balancer.md) diff --git a/doc/administration/high_availability/database.md b/doc/administration/high_availability/database.md new file mode 100644 index 0000000000..538dada1ba --- /dev/null +++ b/doc/administration/high_availability/database.md @@ -0,0 +1,116 @@ +# Configuring a Database for GitLab HA + +You can choose to install and manage a database server (PostgreSQL/MySQL) +yourself, or you can use GitLab Omnibus packages to help. GitLab recommends +PostgreSQL. This is the database that will be installed if you use the +Omnibus package to manage your database. + +## Configure your own database server + +If you're hosting GitLab on a cloud provider, you can optionally use a +managed service for PostgreSQL. For example, AWS offers a managed Relational +Database Service (RDS) that runs PostgreSQL. + +If you use a cloud-managed service, or provide your own PostgreSQL: + +1. Set up a `gitlab` username with a password of your choice. The `gitlab` user + needs privileges to create the `gitlabhq_production` database. +1. Configure the GitLab application servers with the appropriate details. + This step is covered in [Configuring GitLab for HA](gitlab.md) + +## Configure using Omnibus + +1. Download/install GitLab Omnibus using **steps 1 and 2** from + [GitLab downloads](https://about.gitlab.com/downloads). Do not complete other + steps on the download page. +1. Create/edit `/etc/gitlab/gitlab.rb` and use the following configuration. + Be sure to change the `external_url` to match your eventual GitLab front-end + URL. + + ```ruby + external_url 'https://gitlab.example.com' + + # Disable all components except PostgreSQL + postgresql['enable'] = true + bootstrap['enable'] = false + nginx['enable'] = false + unicorn['enable'] = false + sidekiq['enable'] = false + redis['enable'] = false + gitlab_workhorse['enable'] = false + mailroom['enable'] = false + + # PostgreSQL configuration + postgresql['sql_password'] = 'DB password' + postgresql['md5_auth_cidr_addresses'] = ['0.0.0.0/0'] + postgresql['listen_address'] = '0.0.0.0' + ``` + +1. Run `sudo gitlab-ctl reconfigure` to install and configure PostgreSQL. + + > **Note**: This `reconfigure` step will result in some errors. + That's OK - don't be alarmed. + +1. Open a database prompt: + + ``` + su - gitlab-psql + /bin/bash + psql -h /var/opt/gitlab/postgresql -d template1 + + # Output: + + psql (9.2.15) + Type "help" for help. + + template1=# + ``` + +1. Run the following command at the database prompt and you will be asked to + enter the new password for the PostgreSQL superuser. + + ``` + \password + + # Output: + + Enter new password: + Enter it again: + ``` + +1. Similarly, set the password for the `gitlab` database user. Use the same + password that you specified in the `/etc/gitlab/gitlab.rb` file for + `postgresql['sql_password']`. + + ``` + \password gitlab + + # Output: + + Enter new password: + Enter it again: + ``` + +1. Enable the `pg_trgm` extension: + ``` + CREATE EXTENSION pg_trgm; + + # Output: + + CREATE EXTENSION + ``` +1. Exit the database prompt by typing `\q` and Enter. +1. Exit the `gitlab-psql` user by running `exit` twice. +1. Run `sudo gitlab-ctl reconfigure` a final time. +1. Run `touch /etc/gitlab/skip-auto-migrations` to prevent database migrations + from running on upgrade. Only the primary GitLab application server should + handle migrations. + +--- + +Read more on high-availability configuration: + +1. [Configure Redis](redis.md) +1. [Configure NFS](nfs.md) +1. [Configure the GitLab application servers](gitlab.md) +1. [Configure the load balancers](load_balancer.md) diff --git a/doc/administration/high_availability/gitlab.md b/doc/administration/high_availability/gitlab.md new file mode 100644 index 0000000000..8a881ce886 --- /dev/null +++ b/doc/administration/high_availability/gitlab.md @@ -0,0 +1,131 @@ +# Configuring GitLab for HA + +Assuming you have already configured a database, Redis, and NFS, you can +configure the GitLab application server(s) now. Complete the steps below +for each GitLab application server in your environment. + +> **Note:** There is some additional configuration near the bottom for + secondary GitLab application servers. It's important to read and understand + these additional steps before proceeding with GitLab installation. + +1. If necessary, install the NFS client utility packages using the following + commands: + + ``` + # Ubuntu/Debian + apt-get install nfs-common + + # CentOS/Red Hat + yum install nfs-utils nfs-utils-lib + ``` + +1. Specify the necessary NFS shares. Mounts are specified in + `/etc/fstab`. The exact contents of `/etc/fstab` will depend on how you chose + to configure your NFS server. See [NFS documentation](nfs.md) for the various + options. Here is an example snippet to add to `/etc/fstab`: + + ``` + 10.1.0.1:/var/opt/gitlab/.ssh /var/opt/gitlab/.ssh nfs defaults,soft,rsize=1048576,wsize=1048576,noatime,nobootwait,lookupcache=positive 0 2 + 10.1.0.1:/var/opt/gitlab/gitlab-rails/uploads /var/opt/gitlab/gitlab-rails/uploads nfs defaults,soft,rsize=1048576,wsize=1048576,noatime,nobootwait,lookupcache=positive 0 2 + 10.1.0.1:/var/opt/gitlab/gitlab-rails/shared /var/opt/gitlab/gitlab-rails/shared nfs defaults,soft,rsize=1048576,wsize=1048576,noatime,nobootwait,lookupcache=positive 0 2 + 10.1.0.1:/var/opt/gitlab/gitlab-ci/builds /var/opt/gitlab/gitlab-ci/builds nfs defaults,soft,rsize=1048576,wsize=1048576,noatime,nobootwait,lookupcache=positive 0 2 + 10.1.1.1:/var/opt/gitlab/git-data /var/opt/gitlab/git-data nfs defaults,soft,rsize=1048576,wsize=1048576,noatime,nobootwait,lookupcache=positive 0 2 + ``` + +1. Create the shared directories. These may be different depending on your NFS + mount locations. + + ``` + mkdir -p /var/opt/gitlab/.ssh /var/opt/gitlab/gitlab-rails/uploads /var/opt/gitlab/gitlab-rails/shared /var/opt/gitlab/gitlab-ci/builds /var/opt/gitlab/git-data + ``` + +1. Download/install GitLab Omnibus using **steps 1 and 2** from + [GitLab downloads](https://about.gitlab.com/downloads). Do not complete other + steps on the download page. +1. Create/edit `/etc/gitlab/gitlab.rb` and use the following configuration. + Be sure to change the `external_url` to match your eventual GitLab front-end + URL. Depending your the NFS configuration, you may need to change some GitLab + data locations. See [NFS documentation](nfs.md) for `/etc/gitlab/gitlab.rb` + configuration values for various scenarios. The example below assumes you've + added NFS mounts in the default data locations. + + ```ruby + external_url 'https://gitlab.example.com' + + # Prevent GitLab from starting if NFS data mounts are not available + high_availability['mountpoint'] = '/var/opt/gitlab/git-data' + + # Disable components that will not be on the GitLab application server + postgresql['enable'] = false + redis['enable'] = false + + # PostgreSQL connection details + gitlab_rails['db_adapter'] = 'postgresql' + gitlab_rails['db_encoding'] = 'unicode' + gitlab_rails['db_host'] = '10.1.0.5' # IP/hostname of database server + gitlab_rails['db_password'] = 'DB password' + + # Redis connection details + gitlab_rails['redis_port'] = '6379' + gitlab_rails['redis_host'] = '10.1.0.6' # IP/hostname of Redis server + gitlab_rails['redis_password'] = 'Redis Password' + ``` + +1. Run `sudo gitlab-ctl reconfigure` to compile the configuration. + +## Primary GitLab application server + +As a final step, run the setup rake task on the first GitLab application server. +It is not necessary to run this on additional application servers. + +1. Initialize the database by running `sudo gitlab-rake gitlab:setup`. + +> **WARNING:** Only run this setup task on **NEW** GitLab instances because it + will wipe any existing data. + +> **Note:** When you specify `https` in the `external_url`, as in the example + above, GitLab assumes you have SSL certificates in `/etc/gitlab/ssl/`. If + certificates are not present, Nginx will fail to start. See + [Nginx documentation](http://docs.gitlab.com/omnibus/settings/nginx.html#enable-https) + for more information. + +## Additional configuration for secondary GitLab application servers + +Secondary GitLab servers (servers configured **after** the first GitLab server) +need some additional configuration. + +1. Configure shared secrets. These values can be obtained from the primary + GitLab server in `/etc/gitlab/gitlab-secrets.json`. Add these to + `/etc/gitlab/gitlab.rb` **prior to** running the first `reconfigure` in + the steps above. + + ```ruby + gitlab_shell['secret_token'] = 'fbfb19c355066a9afb030992231c4a363357f77345edd0f2e772359e5be59b02538e1fa6cae8f93f7d23355341cea2b93600dab6d6c3edcdced558fc6d739860' + gitlab_rails['secret_token'] = 'b719fe119132c7810908bba18315259ed12888d4f5ee5430c42a776d840a396799b0a5ef0a801348c8a357f07aa72bbd58e25a84b8f247a25c72f539c7a6c5fa' + gitlab_ci['secret_key_base'] = '6e657410d57c71b4fc3ed0d694e7842b1895a8b401d812c17fe61caf95b48a6d703cb53c112bc01ebd197a85da81b18e29682040e99b4f26594772a4a2c98c6d' + gitlab_ci['db_key_base'] = 'bf2e47b68d6cafaef1d767e628b619365becf27571e10f196f98dc85e7771042b9203199d39aff91fcb6837c8ed83f2a912b278da50999bb11a2fbc0fba52964' + ``` + +1. Run `touch /etc/gitlab/skip-auto-migrations` to prevent database migrations + from running on upgrade. Only the primary GitLab application server should + handle migrations. + +## Troubleshooting + +- `mount: wrong fs type, bad option, bad superblock on` + +You have not installed the necessary NFS client utilities. See step 1 above. + +- `mount: mount point /var/opt/gitlab/... does not exist` + +This particular directory does not exist on the NFS server. Ensure +the share is exported and exists on the NFS server and try to remount. + +--- + +Read more on high-availability configuration: + +1. [Configure the database](database.md) +1. [Configure Redis](redis.md) +1. [Configure NFS](nfs.md) +1. [Configure the load balancers](load_balancer.md) diff --git a/doc/administration/high_availability/load_balancer.md b/doc/administration/high_availability/load_balancer.md new file mode 100644 index 0000000000..b1fe34ed9a --- /dev/null +++ b/doc/administration/high_availability/load_balancer.md @@ -0,0 +1,63 @@ +# Load Balancer for GitLab HA + +In an active/active GitLab configuration, you will need a load balancer to route +traffic to the application servers. The specifics on which load balancer to use +or the exact configuration is beyond the scope of GitLab documentation. We hope +that if you're managing HA systems like GitLab you have a load balancer of +choice already. Some examples including HAProxy (open-source), F5 Big-IP LTM, +and Citrix Net Scaler. This documentation will outline what ports and protocols +you need to use with GitLab. + +## Basic ports + +| LB Port | Backend Port | Protocol | +| ------- | ------------ | -------- | +| 80 | 80 | HTTP | +| 443 | 443 | HTTPS [^1] | +| 22 | 22 | TCP | + +## GitLab Pages Ports + +If you're using GitLab Pages you will need some additional port configurations. +GitLab Pages requires a separate VIP. Configure DNS to point the +`pages_external_url` from `/etc/gitlab/gitlab.rb` at the new VIP. See the +[GitLab Pages documentation][gitlab-pages] for more information. + +| LB Port | Backend Port | Protocol | +| ------- | ------------ | -------- | +| 80 | Varies [^2] | HTTP | +| 443 | Varies [^2] | TCP [^3] | + +## Alternate SSH Port + +Some organizations have policies against opening SSH port 22. In this case, +it may be helpful to configure an alternate SSH hostname that allows users +to use SSH on port 443. An alternate SSH hostname will require a new VIP +compared to the other GitLab HTTP configuration above. + +Configure DNS for an alternate SSH hostname such as altssh.gitlab.example.com. + +| LB Port | Backend Port | Protocol | +| ------- | ------------ | -------- | +| 443 | 22 | TCP | + +--- + +Read more on high-availability configuration: + +1. [Configure the database](database.md) +1. [Configure Redis](redis.md) +1. [Configure NFS](nfs.md) +1. [Configure the GitLab application servers](gitlab.md) + +[^1]: When using HTTPS protocol for port 443, you will need to add an SSL + certificate to the load balancers. If you wish to terminate SSL at the + GitLab application server instead, use TCP protocol. +[^2]: The backend port for GitLab Pages depends on the + `gitlab_pages['external_http']` and `gitlab_pages['external_https']` + setting. See [GitLab Pages documentation][gitlab-pages] for more details. +[^3]: Port 443 for GitLab Pages should always use the TCP protocol. Users can + configure custom domains with custom SSL, which would not be possible + if SSL was terminated at the load balancer. + +[gitlab-pages]: http://doc.gitlab.com/ee/pages/administration.html diff --git a/doc/administration/high_availability/nfs.md b/doc/administration/high_availability/nfs.md new file mode 100644 index 0000000000..e4e124e200 --- /dev/null +++ b/doc/administration/high_availability/nfs.md @@ -0,0 +1,116 @@ +# NFS + +## Required NFS Server features + +**File locking**: GitLab **requires** file locking which is only supported +natively in NFS version 4. NFSv3 also supports locking as long as +Linux Kernel 2.6.5+ is used. We recommend using version 4 and do not +specifically test NFSv3. + +**no_root_squash**: NFS normally changes the `root` user to `nobody`. This is +a good security measure when NFS shares will be accessed by many different +users. However, in this case only GitLab will use the NFS share so it +is safe. GitLab requires the `no_root_squash` setting because we need to +manage file permissions automatically. Without the setting you will receive +errors when the Omnibus package tries to alter permissions. Note that GitLab +and other bundled components do **not** run as `root` but as non-privileged +users. The requirement for `no_root_squash` is to allow the Omnibus package to +set ownership and permissions on files, as needed. + +### Recommended options + +When you define your NFS exports, we recommend you also add the following +options: + +- `sync` - Force synchronous behavior. Default is asynchronous and under certain + circumstances it could lead to data loss if a failure occurs before data has + synced. + +## Client mount options + +Below is an example of an NFS mount point we use on GitLab.com: + +``` +10.1.1.1:/var/opt/gitlab/git-data /var/opt/gitlab/git-data nfs4 defaults,soft,rsize=1048576,wsize=1048576,noatime,nobootwait,lookupcache=positive 0 2 +``` + +Notice several options that you should consider using: + +| Setting | Description | +| ------- | ----------- | +| `nobootwait` | Don't halt boot process waiting for this mount to become available +| `lookupcache=positive` | Tells the NFS client to honor `positive` cache results but invalidates any `negative` cache results. Negative cache results cause problems with Git. Specifically, a `git push` can fail to register uniformly across all NFS clients. The negative cache causes the clients to 'remember' that the files did not exist previously. + +## Mount locations + +When using default Omnibus configuration you will need to share 5 data locations +between all GitLab cluster nodes. No other locations should be shared. The +following are the 5 locations you need to mount: + +| Location | Description | +| -------- | ----------- | +| `/var/opt/gitlab/git-data` | Git repository data. This will account for a large portion of your data +| `/var/opt/gitlab/.ssh` | SSH `authorized_keys` file and keys used to import repositories from some other Git services +| `/var/opt/gitlab/gitlab-rails/uploads` | User uploaded attachments +| `/var/opt/gitlab/gitlab-rails/shared` | Build artifacts, GitLab Pages, LFS objects, temp files, etc. If you're using LFS this may also account for a large portion of your data +| `/var/opt/gitlab/gitlab-ci/builds` | GitLab CI build traces + +Other GitLab directories should not be shared between nodes. They contain +node-specific files and GitLab code that does not need to be shared. To ship +logs to a central location consider using remote syslog. GitLab Omnibus packages +provide configuration for [UDP log shipping][udp-log-shipping]. + +### Consolidating mount points + +If you don't want to configure 5-6 different NFS mount points, you have a few +alternative options. + +#### Change default file locations + +Omnibus allows you to configure the file locations. With custom configuration +you can specify just one main mountpoint and have all of these locations +as subdirectories. Mount `/gitlab-data` then use the following Omnibus +configuration to move each data location to a subdirectory: + +```ruby +user['home'] = '/gitlab-data/home' +git_data_dir '/gitlab-data/git-data' +gitlab_rails['shared_path'] = '/gitlab-data/shared' +gitlab_rails['uploads_directory'] = "/gitlab-data/uploads" +gitlab_ci['builds_directory'] = '/gitlab-data/builds' +``` + +To move the `git` home directory, all GitLab services must be stopped. Run +`gitlab-ctl stop && initctl stop gitlab-runsvdir`. Then continue with the +reconfigure. + +Run `sudo gitlab-ctl reconfigure` to start using the central location. Please +be aware that if you had existing data you will need to manually copy/rsync it +to these new locations and then restart GitLab. + +#### Bind mounts + +Bind mounts provide a way to specify just one NFS mount and then +bind the default GitLab data locations to the NFS mount. Start by defining your +single NFS mount point as you normally would in `/etc/fstab`. Let's assume your +NFS mount point is `/gitlab-data`. Then, add the following bind mounts in +`/etc/fstab`: + +```bash +/gitlab-data/git-data /var/opt/gitlab/git-data none bind 0 0 +/gitlab-data/.ssh /var/opt/gitlab/.ssh none bind 0 0 +/gitlab-data/uploads /var/opt/gitlab/gitlab-rails/uploads none bind 0 0 +/gitlab-data/shared /var/opt/gitlab/gitlab-rails/shared none bind 0 0 +/gitlab-data/builds /var/opt/gitlab/gitlab-ci/builds none bind 0 0 +``` + +--- + +Read more on high-availability configuration: + +1. [Configure the database](database.md) +1. [Configure Redis](redis.md) +1. [Configure the GitLab application servers](gitlab.md) +1. [Configure the load balancers](load_balancer.md) + +[udp-log-shipping]: http://doc.gitlab.com/omnibus/settings/logs.html#udp-log-shipping-gitlab-enterprise-edition-only "UDP log shipping" diff --git a/doc/administration/high_availability/redis.md b/doc/administration/high_availability/redis.md new file mode 100644 index 0000000000..d89a1e582c --- /dev/null +++ b/doc/administration/high_availability/redis.md @@ -0,0 +1,62 @@ +# Configuring Redis for GitLab HA + +You can choose to install and manage Redis yourself, or you can use GitLab +Omnibus packages to help. + +## Configure your own Redis server + +If you're hosting GitLab on a cloud provider, you can optionally use a +managed service for Redis. For example, AWS offers a managed ElastiCache service +that runs Redis. + +> **Note:** Redis does not require authentication by default. See + [Redis Security](http://redis.io/topics/security) documentation for more + information. We recommend using a combination of a Redis password and tight + firewall rules to secure your Redis service. + +## Configure using Omnibus + +1. Download/install GitLab Omnibus using **steps 1 and 2** from + [GitLab downloads](https://about.gitlab.com/downloads). Do not complete other + steps on the download page. +1. Create/edit `/etc/gitlab/gitlab.rb` and use the following configuration. + Be sure to change the `external_url` to match your eventual GitLab front-end + URL. + + ```ruby + external_url 'https://gitlab.example.com' + + # Disable all components except PostgreSQL + redis['enable'] = true + bootstrap['enable'] = false + nginx['enable'] = false + unicorn['enable'] = false + sidekiq['enable'] = false + postgresql['enable'] = false + gitlab_workhorse['enable'] = false + mailroom['enable'] = false + + # Redis configuration + redis['port'] = 6379 + redis['bind'] = '0.0.0.0' + + # If you wish to use Redis authentication (recommended) + redis['password'] = 'Redis Password' + ``` + +1. Run `sudo gitlab-ctl reconfigure` to install and configure PostgreSQL. + + > **Note**: This `reconfigure` step will result in some errors. + That's OK - don't be alarmed. +1. Run `touch /etc/gitlab/skip-auto-migrations` to prevent database migrations + from running on upgrade. Only the primary GitLab application server should + handle migrations. + +--- + +Read more on high-availability configuration: + +1. [Configure the database](database.md) +1. [Configure NFS](nfs.md) +1. [Configure the GitLab application servers](gitlab.md) +1. [Configure the load balancers](load_balancer.md) From 0bb85509c913e5cae57e51ecc3d793c7a75f6748 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Fri, 22 Apr 2016 13:24:36 -0500 Subject: [PATCH 62/78] Fix link --- doc/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/README.md b/doc/README.md index f01a2a7f94..e358da1c42 100644 --- a/doc/README.md +++ b/doc/README.md @@ -42,7 +42,7 @@ - [Housekeeping](administration/housekeeping.md) Keep your Git repository tidy and fast. - [GitLab Performance Monitoring](monitoring/performance/introduction.md) Configure GitLab and InfluxDB for measuring performance metrics - [Sidekiq Troubleshooting](administration/troubleshooting/sidekiq.md) Debug when Sidekiq appears hung and is not processing jobs -- [High Availability](administration/high-availability/README.md) Configure multiple servers for scaling or high availability +- [High Availability](administration/high_availability/README.md) Configure multiple servers for scaling or high availability ## Contributor documentation From fdc8b05ab181b809fd7d0fb3b8e8d73fac534fd0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Fri, 22 Apr 2016 12:36:47 -0400 Subject: [PATCH 63/78] Re-generate Spring binstubs for updated version of Spring Prevents output like this: Array values in the parameter are deprecated. Please use a String or nil. An Array was passed in from bin/rspec:3:in `load' --- bin/rails | 5 +++++ bin/rake | 5 +++++ bin/rspec | 5 +++-- bin/spinach | 5 +++-- bin/spring | 12 ++++++------ bin/teaspoon | 8 ++++++++ 6 files changed, 30 insertions(+), 10 deletions(-) create mode 100755 bin/teaspoon diff --git a/bin/rails b/bin/rails index 5191e6927a..0138d79b75 100755 --- a/bin/rails +++ b/bin/rails @@ -1,4 +1,9 @@ #!/usr/bin/env ruby +begin + load File.expand_path('../spring', __FILE__) +rescue LoadError => e + raise unless e.message.include?('spring') +end APP_PATH = File.expand_path('../../config/application', __FILE__) require_relative '../config/boot' require 'rails/commands' diff --git a/bin/rake b/bin/rake index 17240489f6..d87d5f5781 100755 --- a/bin/rake +++ b/bin/rake @@ -1,4 +1,9 @@ #!/usr/bin/env ruby +begin + load File.expand_path('../spring', __FILE__) +rescue LoadError => e + raise unless e.message.include?('spring') +end require_relative '../config/boot' require 'rake' Rake.application.run diff --git a/bin/rspec b/bin/rspec index 20060ebd79..6e6709219a 100755 --- a/bin/rspec +++ b/bin/rspec @@ -1,7 +1,8 @@ #!/usr/bin/env ruby begin - load File.expand_path("../spring", __FILE__) -rescue LoadError + load File.expand_path('../spring', __FILE__) +rescue LoadError => e + raise unless e.message.include?('spring') end require 'bundler/setup' load Gem.bin_path('rspec-core', 'rspec') diff --git a/bin/spinach b/bin/spinach index a080e286cf..474050e29d 100755 --- a/bin/spinach +++ b/bin/spinach @@ -1,7 +1,8 @@ #!/usr/bin/env ruby begin - load File.expand_path("../spring", __FILE__) -rescue LoadError + load File.expand_path('../spring', __FILE__) +rescue LoadError => e + raise unless e.message.include?('spring') end require 'bundler/setup' load Gem.bin_path('spinach', 'spinach') diff --git a/bin/spring b/bin/spring index 7b45d374fc..7fe232c3aa 100755 --- a/bin/spring +++ b/bin/spring @@ -4,12 +4,12 @@ # It gets overwritten when you run the `spring binstub` command. unless defined?(Spring) - require "rubygems" - require "bundler" + require 'rubygems' + require 'bundler' - if match = Bundler.default_lockfile.read.match(/^GEM$.*?^ (?: )*spring \((.*?)\)$.*?^$/m) - Gem.paths = { "GEM_PATH" => [Bundler.bundle_path.to_s, *Gem.path].uniq } - gem "spring", match[1] - require "spring/binstub" + if (match = Bundler.default_lockfile.read.match(/^GEM$.*?^ (?: )*spring \((.*?)\)$.*?^$/m)) + Gem.paths = { 'GEM_PATH' => [Bundler.bundle_path.to_s, *Gem.path].uniq.join(Gem.path_separator) } + gem 'spring', match[1] + require 'spring/binstub' end end diff --git a/bin/teaspoon b/bin/teaspoon new file mode 100755 index 0000000000..7c3b8dfc4e --- /dev/null +++ b/bin/teaspoon @@ -0,0 +1,8 @@ +#!/usr/bin/env ruby +begin + load File.expand_path('../spring', __FILE__) +rescue LoadError => e + raise unless e.message.include?('spring') +end +require 'bundler/setup' +load Gem.bin_path('teaspoon', 'teaspoon') From 8dd27b47eaa566806ae44a01f963718449f59d8d Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Thu, 21 Apr 2016 12:18:35 +0200 Subject: [PATCH 64/78] Add posibility to define a hidden job without 'script' in .gitlab-ci.yml --- lib/ci/gitlab_ci_yaml_processor.rb | 11 +- spec/lib/ci/gitlab_ci_yaml_processor_spec.rb | 161 +++++++++++++------ 2 files changed, 118 insertions(+), 54 deletions(-) diff --git a/lib/ci/gitlab_ci_yaml_processor.rb b/lib/ci/gitlab_ci_yaml_processor.rb index ff9887cba1..4319d99d52 100644 --- a/lib/ci/gitlab_ci_yaml_processor.rb +++ b/lib/ci/gitlab_ci_yaml_processor.rb @@ -63,9 +63,8 @@ module Ci @cache = @config[:cache] @config.except!(*ALLOWED_YAML_KEYS) - # anything that doesn't have script is considered as unknown @config.each do |name, param| - raise ValidationError, "Unknown parameter: #{name}" unless param.is_a?(Hash) && param.has_key?(:script) + raise ValidationError, "Unknown parameter: #{name}" unless is_a_job?(name, param) end unless @config.values.any?{|job| job.is_a?(Hash)} @@ -80,6 +79,12 @@ module Ci end end + def is_a_job?(name, value) + return true if value.is_a?(Hash) && value.has_key?(:script) + return true if name.to_s.start_with?('.') + false + end + def build_job(name, job) { stage_idx: stages.index(job[:stage]), @@ -112,8 +117,6 @@ module Ci true end - private - def validate_global! unless validate_array_of_strings(@before_script) raise ValidationError, "before_script should be an array of strings" diff --git a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb index 643acf0303..17ffe146fb 100644 --- a/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb +++ b/spec/lib/ci/gitlab_ci_yaml_processor_spec.rb @@ -648,70 +648,131 @@ module Ci end describe "Hidden jobs" do - let(:config) do - YAML.dump({ - '.hidden_job' => { script: 'test' }, - 'normal_job' => { script: 'test' } - }) - end - let(:config_processor) { GitlabCiYamlProcessor.new(config) } - subject { config_processor.builds_for_stage_and_ref("test", "master") } - it "doesn't create jobs that starts with dot" do - expect(subject.size).to eq(1) - expect(subject.first).to eq({ - except: nil, - stage: "test", - stage_idx: 1, - name: :normal_job, - only: nil, - commands: "test", - tag_list: [], - options: {}, - when: "on_success", - allow_failure: false - }) + shared_examples 'hidden_job_handling' do + it "doesn't create jobs that starts with dot" do + expect(subject.size).to eq(1) + expect(subject.first).to eq({ + except: nil, + stage: "test", + stage_idx: 1, + name: :normal_job, + only: nil, + commands: "test", + tag_list: [], + options: {}, + when: "on_success", + allow_failure: false + }) + end + end + + context 'When hidden job have a script definition' do + let(:config) do + YAML.dump({ + '.hidden_job' => { image: 'ruby:2.1', script: 'test' }, + 'normal_job' => { script: 'test' } + }) + end + + it_behaves_like 'hidden_job_handling' + end + + context "When hidden job doesn't have a script definition" do + let(:config) do + YAML.dump({ + '.hidden_job' => { image: 'ruby:2.1' }, + 'normal_job' => { script: 'test' } + }) + end + + it_behaves_like 'hidden_job_handling' end end describe "YAML Alias/Anchor" do - it "is correctly supported for jobs" do - config = <