From 90a67a76d5b852c62b59dd52b9dafd58722f2237 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Thu, 14 Apr 2016 17:32:46 -0400 Subject: [PATCH 01/51] 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/51] 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/51] 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/51] 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/51] 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/51] 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/51] 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/51] 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 9a99fccb9717d4f1b0221de84094bb8db4ed280f Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Mon, 18 Apr 2016 12:49:23 -0500 Subject: [PATCH 09/51] 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 10/51] 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 11/51] 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 12/51] 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 13/51] 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 14/51] 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 15/51] 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 d7f1e041d9df1121b550fbdb7682d25e404b04c2 Mon Sep 17 00:00:00 2001 From: Achilleas Pipinellis Date: Tue, 19 Apr 2016 19:12:12 +0300 Subject: [PATCH 16/51] 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 17/51] 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 18/51] 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 19/51] 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 20/51] 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 21/51] 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 22/51] 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 23/51] 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 24/51] 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 25/51] 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 26/51] 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 27/51] 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 28/51] 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 29/51] 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 30/51] 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 31/51] 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 32/51] 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 33/51] 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 34/51] 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 35/51] 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 36/51] 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 37/51] 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 38/51] 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 39/51] 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 40/51] 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 41/51] 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 42/51] 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 43/51] 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 44/51] 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 45/51] 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 46/51] 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 47/51] 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 48/51] 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 49/51] 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 50/51] 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 32be8bd2b105885d88e1ad8d5bf1296f974031ab Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Fri, 22 Apr 2016 13:30:58 +0100 Subject: [PATCH 51/51] 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