From ccc929d75d1f34accc4c7a6604c26cac6113a18c Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 11:50:00 -0500 Subject: [PATCH 01/28] CSS tweaks - Use colors according to design - Fix search input' width --- app/assets/stylesheets/framework/variables.scss | 8 ++++---- app/assets/stylesheets/pages/search.scss | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index bb49ae396c..b86dbcb04b 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -199,11 +199,11 @@ $award-emoji-new-btn-icon-color: #dcdcdc; /* * Search Box */ -$search-input-border-color: $dropdown-input-focus-border; +$search-input-border-color: rgba(#4688f1, .8); $search-input-focus-shadow-color: $dropdown-input-focus-shadow; -$search-input-width: $dropdown-width; +$search-input-width: 244px; $location-badge-color: #aaa; $location-badge-bg: $gray-normal; +$location-badge-active-bg: #4f91f8; $location-icon-color: #e7e9ed; -$location-active-color: $gl-text-color; -$location-active-bg: $search-input-border-color; +$location-icon-active-color: #807e7e; diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 3c74d25beb..9274796233 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -135,13 +135,13 @@ .location-badge { @include transition(all .15s); - background-color: $location-active-bg; + background-color: $location-badge-active-bg; color: $white-light; } .search-input-wrap { i { - color: $location-active-color; + color: $location-icon-active-color; } } From e3a983050714aa54a3c7911666cdd0abbc36e57a Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 15:28:45 -0500 Subject: [PATCH 02/28] Skip default behaviour if we are clicking a result for the same location --- app/assets/javascripts/gl_dropdown.js.coffee | 2 +- app/assets/javascripts/search_autocomplete.js.coffee | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 0fea2a69cb..e4df7d46d5 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -174,7 +174,7 @@ class GitLabDropdown selected = self.rowClicked $(@) if self.options.clicked - self.options.clicked(selected) + self.options.clicked(selected, e) # Finds an element inside wrapper element getElement: (selector) -> diff --git a/app/assets/javascripts/search_autocomplete.js.coffee b/app/assets/javascripts/search_autocomplete.js.coffee index 030655491b..2656c6e30a 100644 --- a/app/assets/javascripts/search_autocomplete.js.coffee +++ b/app/assets/javascripts/search_autocomplete.js.coffee @@ -62,6 +62,8 @@ class @SearchAutocomplete search: fields: ['text'] data: @getData.bind(@) + selectable: true + clicked: @onClick.bind(@) getData: (term, callback) -> _this = @ @@ -268,3 +270,9 @@ class @SearchAutocomplete
  • Loading...
  • " @dropdownContent.html(html) + + onClick: (item, e) -> + if location.pathname.indexOf(item.url) isnt -1 + e.preventDefault() + @disableAutocomplete() + @searchInput.val('') From ff676333d0d257869a917a5e48e9717d219b8311 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 19:28:17 -0500 Subject: [PATCH 03/28] Add current element and event as params to clicked callback --- app/assets/javascripts/gl_dropdown.js.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index e4df7d46d5..d447013f63 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -171,10 +171,11 @@ class GitLabDropdown selector = ".dropdown-page-one .dropdown-content a" @dropdown.on "click", selector, (e) -> - selected = self.rowClicked $(@) + $el = $(@) + selected = self.rowClicked $el if self.options.clicked - self.options.clicked(selected, e) + self.options.clicked(selected, $el, e) # Finds an element inside wrapper element getElement: (selector) -> From ab42560ea6c2fade09b8436a893bf4a269e67c39 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 19:28:41 -0500 Subject: [PATCH 04/28] Return selected object if toggleLabel option is not defined --- app/assets/javascripts/gl_dropdown.js.coffee | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index d447013f63..ca8d9a1694 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -352,6 +352,8 @@ class GitLabDropdown # Toggle the dropdown label if @options.toggleLabel $(@el).find(".dropdown-toggle-text").text @options.toggleLabel + else + selectedObject else if !value? field.remove() From 824fecb728ae7534b8ef56aaa6679814f3924cd5 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Thu, 31 Mar 2016 19:31:01 -0500 Subject: [PATCH 05/28] Bring back search context when chosing the same project/group --- .../javascripts/search_autocomplete.js.coffee | 25 +++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/search_autocomplete.js.coffee b/app/assets/javascripts/search_autocomplete.js.coffee index 2656c6e30a..564fb265b9 100644 --- a/app/assets/javascripts/search_autocomplete.js.coffee +++ b/app/assets/javascripts/search_autocomplete.js.coffee @@ -104,6 +104,8 @@ class @SearchAutocomplete lastCategory = suggestion.category data.push + id: "#{suggestion.category.toLowerCase()}-#{suggestion.id}" + category: suggestion.category text: suggestion.label url: suggestion.url @@ -271,8 +273,27 @@ class @SearchAutocomplete " @dropdownContent.html(html) - onClick: (item, e) -> + onClick: (item, $el, e) -> if location.pathname.indexOf(item.url) isnt -1 e.preventDefault() + if not @badgePresent + if item.category is 'Projects' + @projectInputEl.val(item.id) + @addLocationBadge( + value: 'This project' + ) + + if item.category is 'Groups' + @groupInputEl.val(item.id) + @addLocationBadge( + value: 'This group' + ) + + $el.removeClass('is-active') @disableAutocomplete() - @searchInput.val('') + @searchInput.val('').focus() + + # We need to wait because of @skipBlurEvent + setTimeout( => + @onSearchInputFocus() + , 200) From deca9fc6266dfd5588b23f64157ddb05e5713412 Mon Sep 17 00:00:00 2001 From: Alessio Biancalana Date: Sat, 2 Apr 2016 14:44:50 +0200 Subject: [PATCH 06/28] Added bottom margin to CLI instructions --- app/assets/stylesheets/pages/projects.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 4e6aa8cd1a..61150746c7 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -315,7 +315,7 @@ pre.light-well { } .git-empty { - margin: 0 7px; + margin: 0 7px 7px; h5 { color: #5c5d5e; From 261c8e765f5b13dd627fcda5ca1ae263ecfad0c8 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Thu, 31 Mar 2016 23:33:32 +0100 Subject: [PATCH 07/28] fix missing filters on status tab when user swithches to another state --- app/assets/javascripts/issues.js.coffee | 15 +++++++++++ app/assets/javascripts/lib/url_utility.js | 33 +++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 app/assets/javascripts/lib/url_utility.js diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index b1479bfb44..8d069a209d 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -26,6 +26,20 @@ $(".selected_issue").bind "change", Issues.checkChanged + # Update state filters if present in page + updateStateFilters: -> + stateFilters = $('.issues-state-filters') + newParams = {} + paramKeys = ['author_id', 'label_name', 'milestone_title', 'assignee_id', 'issue_search'] + + for paramKey in paramKeys + newParams[paramKey] = getUrlParameter(paramKey) or '' + + if stateFilters.length + stateFilters.find('a').each -> + initialUrl = $(this).attr 'href' + $(this).attr 'href', mergeUrlParams(newParams, initialUrl) + # Make sure we trigger ajax request only after user stop typing initSearch: -> @timer = null @@ -54,6 +68,7 @@ # Change url so if user reload a page - search results are saved history.replaceState {page: issuesUrl}, document.title, issuesUrl Issues.reload() + Issues.updateStateFilters() dataType: "json" checkChanged: -> diff --git a/app/assets/javascripts/lib/url_utility.js b/app/assets/javascripts/lib/url_utility.js new file mode 100644 index 0000000000..5fa3a4c69d --- /dev/null +++ b/app/assets/javascripts/lib/url_utility.js @@ -0,0 +1,33 @@ +function getUrlParameter(sParam) { + var sPageURL = decodeURIComponent(window.location.search.substring(1)), + sURLVariables = sPageURL.split('&'), + sParameterName, + i; + + for (i = 0; i < sURLVariables.length; i++) { + sParameterName = sURLVariables[i].split('='); + + if (sParameterName[0] === sParam) { + return sParameterName[1] === undefined ? true : sParameterName[1]; + } + } +} + +/** + * @param {Object} params - url keys and value to merge + * @param {String} url + */ +function mergeUrlParams(params, url){ + var newUrl = decodeURIComponent(url); + + Object.keys(params).forEach(function(paramName) { + var pattern = new RegExp('\\b('+paramName+'=).*?(&|$)') + if (url.search(pattern) >= 0){ + newUrl = newUrl.replace(pattern,'$1' + params[paramName] + '$2'); + } else { + newUrl = newUrl + (newUrl.indexOf('?') > 0 ? '&' : '?') + paramName + '=' + params[paramName] + } + }); + + return newUrl; +} \ No newline at end of file From db8836ca84ab86dc39b4d8b3282603c724a78e67 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Sat, 2 Apr 2016 16:56:31 +0100 Subject: [PATCH 08/28] attach the utitlity function to the global scope with some changes --- app/assets/javascripts/issues.js.coffee | 4 +-- app/assets/javascripts/lib/url_utility.js | 33 ------------------- .../javascripts/lib/url_utility.js.coffee | 31 +++++++++++++++++ 3 files changed, 33 insertions(+), 35 deletions(-) delete mode 100644 app/assets/javascripts/lib/url_utility.js create mode 100644 app/assets/javascripts/lib/url_utility.js.coffee diff --git a/app/assets/javascripts/issues.js.coffee b/app/assets/javascripts/issues.js.coffee index 8d069a209d..0d9f2094c2 100644 --- a/app/assets/javascripts/issues.js.coffee +++ b/app/assets/javascripts/issues.js.coffee @@ -33,12 +33,12 @@ paramKeys = ['author_id', 'label_name', 'milestone_title', 'assignee_id', 'issue_search'] for paramKey in paramKeys - newParams[paramKey] = getUrlParameter(paramKey) or '' + newParams[paramKey] = gl.utils.getUrlParameter(paramKey) or '' if stateFilters.length stateFilters.find('a').each -> initialUrl = $(this).attr 'href' - $(this).attr 'href', mergeUrlParams(newParams, initialUrl) + $(this).attr 'href', gl.utils.mergeUrlParams(newParams, initialUrl) # Make sure we trigger ajax request only after user stop typing initSearch: -> diff --git a/app/assets/javascripts/lib/url_utility.js b/app/assets/javascripts/lib/url_utility.js deleted file mode 100644 index 5fa3a4c69d..0000000000 --- a/app/assets/javascripts/lib/url_utility.js +++ /dev/null @@ -1,33 +0,0 @@ -function getUrlParameter(sParam) { - var sPageURL = decodeURIComponent(window.location.search.substring(1)), - sURLVariables = sPageURL.split('&'), - sParameterName, - i; - - for (i = 0; i < sURLVariables.length; i++) { - sParameterName = sURLVariables[i].split('='); - - if (sParameterName[0] === sParam) { - return sParameterName[1] === undefined ? true : sParameterName[1]; - } - } -} - -/** - * @param {Object} params - url keys and value to merge - * @param {String} url - */ -function mergeUrlParams(params, url){ - var newUrl = decodeURIComponent(url); - - Object.keys(params).forEach(function(paramName) { - var pattern = new RegExp('\\b('+paramName+'=).*?(&|$)') - if (url.search(pattern) >= 0){ - newUrl = newUrl.replace(pattern,'$1' + params[paramName] + '$2'); - } else { - newUrl = newUrl + (newUrl.indexOf('?') > 0 ? '&' : '?') + paramName + '=' + params[paramName] - } - }); - - return newUrl; -} \ No newline at end of file diff --git a/app/assets/javascripts/lib/url_utility.js.coffee b/app/assets/javascripts/lib/url_utility.js.coffee new file mode 100644 index 0000000000..abd556e0b4 --- /dev/null +++ b/app/assets/javascripts/lib/url_utility.js.coffee @@ -0,0 +1,31 @@ +((w) -> + + w.gl ?= {} + w.gl.utils ?= {} + + w.gl.utils.getUrlParameter = (sParam) -> + sPageURL = decodeURIComponent(window.location.search.substring(1)) + sURLVariables = sPageURL.split('&') + sParameterName = undefined + i = 0 + while i < sURLVariables.length + sParameterName = sURLVariables[i].split('=') + if sParameterName[0] is sParam + return if sParameterName[1] is undefined then true else sParameterName[1] + i++ + + # # + # @param {Object} params - url keys and value to merge + # @param {String} url + # # + w.gl.utils.mergeUrlParams = (params, url) -> + newUrl = decodeURIComponent(url) + for paramName, paramValue of params + pattern = new RegExp "\\b(#{paramName}=).*?(&|$)" + if url.search(pattern) >= 0 + newUrl = newUrl.replace pattern, "$1#{paramValue}$2" + else + newUrl = "#{newUrl}#{(if newUrl.indexOf('?') > 0 then '&' else '?')}#{paramName}=#{paramValue}" + newUrl + +) window From 0ab6b82a234f6dcc208591724036f152eca0e1c4 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 5 Apr 2016 12:09:19 -0700 Subject: [PATCH 09/28] Update shades of red --- app/assets/stylesheets/framework/variables.scss | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 98fe794d36..dd189a2cb7 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -104,9 +104,9 @@ $orange-light: rgba(252, 109, 38, 0.80); $orange-normal: #e75e40; $orange-dark: #ce5237; -$red-light: #f06559; -$red-normal: #e52c5a; -$red-dark: #d22852; +$red-light: #e52c5a; +$red-normal: #d22852; +$red-dark: darken($red-normal, 5%); $border-white-light: #f1f2f4; $border-white-normal: #d6dae2; @@ -128,9 +128,9 @@ $border-orange-light: #fc6d26; $border-orange-normal: #ce5237; $border-orange-dark: #c14e35; -$border-red-light: #f24f41; -$border-red-normal: #d22852; -$border-red-dark: #ca264f; +$border-red-light: #d22852; +$border-red-normal: #ca264f; +$border-red-dark: darken($border-red-normal, 5%); $help-well-bg: #fafafa; $help-well-border: #e5e5e5; From b23e67bce5ae5f3bed8f27feba7eb5f833dcdca5 Mon Sep 17 00:00:00 2001 From: Annabel Dunstone Date: Tue, 5 Apr 2016 14:12:47 -0700 Subject: [PATCH 10/28] Add CI styling to all CI updates --- app/assets/stylesheets/pages/projects.scss | 2 +- app/assets/stylesheets/pages/status.scss | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 4e6aa8cd1a..a0ac06008b 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -401,7 +401,7 @@ pre.light-well { } .commit_short_id { - margin-right: 5px; + margin: 0 5px; color: $gl-link-color; font-weight: 600; } diff --git a/app/assets/stylesheets/pages/status.scss b/app/assets/stylesheets/pages/status.scss index 5e5e38a0ba..dbb6daf0d7 100644 --- a/app/assets/stylesheets/pages/status.scss +++ b/app/assets/stylesheets/pages/status.scss @@ -1,4 +1,4 @@ -.container-fluid .content { +.container-fluid { .ci-status { padding: 2px 7px; margin-right: 5px; From 7baebd322008e0ca912c447792f1d7cffd74f8e2 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Tue, 5 Apr 2016 19:18:34 -0500 Subject: [PATCH 11/28] Do not create input if fieldName is not defined --- app/assets/javascripts/gl_dropdown.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 6b55917ec8..362ddda359 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -369,7 +369,7 @@ class GitLabDropdown if @options.toggleLabel $(@el).find(".dropdown-toggle-text").text @options.toggleLabel(selectedObject) if value? - if !field.length + if !field.length and fieldName # Create hidden input for form input = "" if @options.inputId? From 55b2fae327854be6ab0789dbdf92f4265207680f Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Wed, 6 Apr 2016 02:50:01 +0100 Subject: [PATCH 12/28] set up test for preventing this issue from reoccuring --- spec/features/issues/filter_issues_spec.rb | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) create mode 100644 spec/features/issues/filter_issues_spec.rb diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb new file mode 100644 index 0000000000..7884121353 --- /dev/null +++ b/spec/features/issues/filter_issues_spec.rb @@ -0,0 +1,45 @@ +require 'rails_helper' + +describe 'Filter issues', feature: true do + + let!(:project) { create(:project) } + let!(:issue) { create(:issue, project: project) } + let!(:user) { create(:user)} + + before do + project.team << [user, :master] + login_as(user) + end + + describe 'Filter issues for assignee from issues#index' do + + before do + visit namespace_project_issues_path(project.namespace, project) + + find('.js-assignee-search').click + + find('.dropdown-menu-user-link', text: user.username).click + + sleep 2 + end + + context 'assignee', js: true do + it 'should update to current user' do + expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name) + end + + it 'should not change when closed link is clicked' do + find('.issues-state-filters a', text: "Closed").click + + expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name) + end + + + it 'should not change when all link is clicked' do + find('.issues-state-filters a', text: "All").click + + expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name) + end + end + end +end From 1e87679702d57a0c01f6352851fd144fd213ea56 Mon Sep 17 00:00:00 2001 From: Arinde Eniola Date: Wed, 6 Apr 2016 03:42:27 +0100 Subject: [PATCH 13/28] complete the tests --- spec/features/issues/filter_issues_spec.rb | 76 +++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/spec/features/issues/filter_issues_spec.rb b/spec/features/issues/filter_issues_spec.rb index 7884121353..90822a8c12 100644 --- a/spec/features/issues/filter_issues_spec.rb +++ b/spec/features/issues/filter_issues_spec.rb @@ -3,8 +3,9 @@ require 'rails_helper' describe 'Filter issues', feature: true do let!(:project) { create(:project) } - let!(:issue) { create(:issue, project: project) } let!(:user) { create(:user)} + let!(:milestone) { create(:milestone, project: project) } + let!(:label) { create(:label, project: project) } before do project.team << [user, :master] @@ -42,4 +43,77 @@ describe 'Filter issues', feature: true do end end end + + describe 'Filter issues for milestone from issues#index' do + + before do + visit namespace_project_issues_path(project.namespace, project) + + find('.js-milestone-select').click + + find('.milestone-filter .dropdown-content a', text: milestone.title).click + + sleep 2 + end + + context 'milestone', js: true do + it 'should update to current milestone' do + expect(find('.js-milestone-select .dropdown-toggle-text')).to have_content(milestone.title) + end + + it 'should not change when closed link is clicked' do + find('.issues-state-filters a', text: "Closed").click + + expect(find('.js-milestone-select .dropdown-toggle-text')).to have_content(milestone.title) + end + + + it 'should not change when all link is clicked' do + find('.issues-state-filters a', text: "All").click + + expect(find('.js-milestone-select .dropdown-toggle-text')).to have_content(milestone.title) + end + end + end + + describe 'Filter issues for assignee and label from issues#index' do + + before do + visit namespace_project_issues_path(project.namespace, project) + + find('.js-assignee-search').click + + find('.dropdown-menu-user-link', text: user.username).click + + sleep 2 + + find('.js-label-select').click + + find('.dropdown-menu-labels .dropdown-content a', text: label.title).click + + sleep 2 + end + + context 'assignee and label', js: true do + it 'should update to current assignee and label' do + expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name) + expect(find('.js-label-select .dropdown-toggle-text')).to have_content(label.title) + end + + it 'should not change when closed link is clicked' do + find('.issues-state-filters a', text: "Closed").click + + expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name) + expect(find('.js-label-select .dropdown-toggle-text')).to have_content(label.title) + end + + + it 'should not change when all link is clicked' do + find('.issues-state-filters a', text: "All").click + + expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name) + expect(find('.js-label-select .dropdown-toggle-text')).to have_content(label.title) + end + end + end end From 4d463c3b0f05fa8873b76afce621eaaf44c19836 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 6 Apr 2016 09:54:49 +0100 Subject: [PATCH 14/28] Fixed delete comment button color --- app/views/projects/notes/_note.html.haml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 34fe1743f4..a681d6dece 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -18,7 +18,7 @@ = access = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do = icon('pencil-square-o') - = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete danger' do + = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button js-note-delete' do = icon('trash-o') .note-body{class: note_editable?(note) ? 'js-task-list-container' : ''} .note-text From d6f276eacfdb7e8287df69dfc1dbf756288bade8 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 6 Apr 2016 10:05:39 +0100 Subject: [PATCH 15/28] Fixed issue with enter key not selecting correct value in dropdown --- app/assets/javascripts/gl_dropdown.js.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/javascripts/gl_dropdown.js.coffee b/app/assets/javascripts/gl_dropdown.js.coffee index 2a4811b876..33c5126087 100644 --- a/app/assets/javascripts/gl_dropdown.js.coffee +++ b/app/assets/javascripts/gl_dropdown.js.coffee @@ -394,7 +394,7 @@ class GitLabDropdown selector = ".dropdown-page-one #{selector}" # simulate a click on the first link - $(selector).trigger "click" + $(selector, @dropdown).trigger "click" addArrowKeyEvent: -> ARROW_KEY_CODES = [38, 40] From 7f287c9136d5d1cdda8df170c6e772ca82aad1e9 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Wed, 6 Apr 2016 13:59:50 +0200 Subject: [PATCH 16/28] API: Ability to retrieve a single tag --- CHANGELOG | 1 + doc/api/tags.md | 34 +++++++++++++++++++++++++++++++++- lib/api/tags.rb | 14 ++++++++++++++ spec/requests/api/tags_spec.rb | 17 +++++++++++++++++ 4 files changed, 65 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 8db9a9b0d1..e0ae6b7975 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -22,6 +22,7 @@ v 8.7.0 (unreleased) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) - Improved UX of the navigation sidebar - Build status notifications + - API: Ability to retrieve a specific tag (Robert Schilling) v 8.6.5 (unreleased) - Check permissions when user attempts to import members from another project diff --git a/doc/api/tags.md b/doc/api/tags.md index 17d12e9cc6..c451a42b72 100644 --- a/doc/api/tags.md +++ b/doc/api/tags.md @@ -38,6 +38,38 @@ Parameters: ] ``` +## Get a single repository tag + +Get a specific repository tag determined by its name. It returns 200 together +with the tag information if the tag exists. It returns 404 if the tag does not +exist. + +Parameters: + +- `id` (required) - The ID of a project +- `tag_name` (required) - The name of the tag + +```json +{ + "name": "v5.0.0", + "message": null, + "commit": { + "id": "60a8ff033665e1207714d6670fcd7b65304ec02f", + "message": "v5.0.0\n", + "parent_ids": [ + "f61c062ff8bcbdb00e0a1b3317a91aed6ceee06b" + ], + "authored_date": "2015-02-01T21:56:31.000+01:00", + "author_name": "Arthur Verschaeve", + "author_email": "contact@arthurverschaeve.be", + "committed_date": "2015-02-01T21:56:31.000+01:00", + "committer_name": "Arthur Verschaeve", + "committer_email": "contact@arthurverschaeve.be" + }, + "release": null +} +``` + ## Create a new tag Creates a new tag in the repository that points to the supplied ref. @@ -148,4 +180,4 @@ Parameters: "tag_name": "1.0.0", "description": "Amazing release. Wow" } -``` \ No newline at end of file +``` diff --git a/lib/api/tags.rb b/lib/api/tags.rb index 2d8a9e51bb..731a68082b 100644 --- a/lib/api/tags.rb +++ b/lib/api/tags.rb @@ -16,6 +16,20 @@ module API with: Entities::RepoTag, project: user_project end + # Get a single repository tag + # + # Parameters: + # id (required) - The ID of a project + # tag_name (required) - The name of the tag + # Example Request: + # GET /projects/:id/repository/tags/:tag_name + get ":id/repository/tags/:tag_name", requirements: { tag_name: /.*/ } do + tag = user_project.repository.find_tag(params[:tag_name]) + not_found!('Tag') unless tag + + present tag, with: Entities::RepoTag, project: user_project + end + # Create tag # # Parameters: diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index a15be07ed5..acbd9c3e33 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -40,6 +40,23 @@ describe API::API, api: true do end end + describe "GET /projects/:id/repository/tags/:tag_name" do + let(:tag_name) { project.repository.tag_names.sort.reverse.first } + + it 'should return a specific tag' do + get api("/projects/#{project.id}/repository/tags/#{tag_name}", user) + + expect(response.status).to eq(200) + expect(json_response['name']).to eq(tag_name) + end + + it 'should return 404 for an invalid tag name' do + get api("/projects/#{project.id}/repository/tags/foobar", user) + + expect(response.status).to eq(404) + end + end + describe 'POST /projects/:id/repository/tags' do context 'lightweight tags' do it 'should create a new tag' do From 507cbca339444eabdfdd5d6dff1237bd00ee636a Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Wed, 6 Apr 2016 14:24:30 +0200 Subject: [PATCH 17/28] Fix header link rendering when containing numbers This fixes the problem where Markdown such as: ### 31st Would get rendered as a link tag pointing to issue number 31 inside a header tag. See gitlab-org/gitlab-ce#14936 for more information. --- lib/banzai/filter/abstract_reference_filter.rb | 2 +- spec/lib/banzai/filter/issue_reference_filter_spec.rb | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index f21dbef216..b8962379cb 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -119,7 +119,7 @@ module Banzai elsif element_node?(node) yield_valid_link(node) do |link, text| - if ref_pattern && link =~ /\A#{ref_pattern}/ + if ref_pattern && link =~ /\A#{ref_pattern}\z/ replace_link_node_with_href(node, link) do object_link_filter(link, ref_pattern, link_text: text) end diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb index 5a0d3d577a..266ebef33d 100644 --- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb @@ -95,6 +95,14 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do result = reference_pipeline_result("Fixed #{reference}") expect(result[:references][:issue]).to eq [issue] end + + it 'does not process links containing issue numbers followed by text' do + href = "#{reference}st" + doc = reference_filter("") + link = doc.css('a').first.attr('href') + + expect(link).to eq(href) + end end context 'cross-project reference' do From a3cf3d19826ea1e67381592f2d91f7041cccc222 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Apr 2016 12:47:10 +0200 Subject: [PATCH 18/28] Corrected some spec headers for Gitlab::Metrics --- spec/lib/gitlab/metrics_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index 0ec8a6dc5c..93dd958168 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -13,7 +13,7 @@ describe Gitlab::Metrics do end end - describe '#submit_metrics' do + describe '.submit_metrics' do it 'prepares and writes the metrics to InfluxDB' do connection = double(:connection) pool = double(:pool) @@ -26,7 +26,7 @@ describe Gitlab::Metrics do end end - describe '#prepare_metrics' do + describe '.prepare_metrics' do it 'returns a Hash with the keys as Symbols' do metrics = described_class. prepare_metrics([{ 'values' => {}, 'tags' => {} }]) @@ -51,7 +51,7 @@ describe Gitlab::Metrics do end end - describe '#escape_value' do + describe '.escape_value' do it 'escapes an equals sign' do expect(described_class.escape_value('foo=')).to eq('foo\\=') end From 1af6cf28c031cec7813d3fc090476c088de57173 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Mon, 4 Apr 2016 14:00:35 +0200 Subject: [PATCH 19/28] Measure Ruby blocks using Gitlab::Metrics This allows measuring of timings of arbitrary Ruby blocks, this allows for more fine grained performance monitoring. Custom values and tags can also be attached to a block. --- lib/gitlab/metrics.rb | 26 +++++++++++++++++++++ spec/lib/gitlab/metrics_spec.rb | 41 +++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/lib/gitlab/metrics.rb b/lib/gitlab/metrics.rb index 88a265c6af..4a3f47b5a9 100644 --- a/lib/gitlab/metrics.rb +++ b/lib/gitlab/metrics.rb @@ -70,6 +70,32 @@ module Gitlab value.to_s.gsub('=', '\\=') end + # Measures the execution time of a block. + # + # Example: + # + # Gitlab::Metrics.measure(:find_by_username_timings) do + # User.find_by_username(some_username) + # end + # + # series - The name of the series to store the data in. + # values - A Hash containing extra values to add to the metric. + # tags - A Hash containing extra tags to add to the metric. + # + # Returns the value yielded by the supplied block. + def self.measure(series, values = {}, tags = {}) + return yield unless Transaction.current + + start = Time.now.to_f + retval = yield + duration = (Time.now.to_f - start) * 1000.0 + values = values.merge(duration: duration) + + Transaction.current.add_metric(series, values, tags) + + retval + end + # When enabled this should be set before being used as the usual pattern # "@foo ||= bar" is _not_ thread-safe. if enabled? diff --git a/spec/lib/gitlab/metrics_spec.rb b/spec/lib/gitlab/metrics_spec.rb index 93dd958168..8f63a5f204 100644 --- a/spec/lib/gitlab/metrics_spec.rb +++ b/spec/lib/gitlab/metrics_spec.rb @@ -60,4 +60,45 @@ describe Gitlab::Metrics do expect(described_class.escape_value(10)).to eq('10') end end + + describe '.measure' do + context 'without a transaction' do + it 'returns the return value of the block' do + val = Gitlab::Metrics.measure(:foo) { 10 } + + expect(val).to eq(10) + end + end + + context 'with a transaction' do + let(:transaction) { Gitlab::Metrics::Transaction.new } + + before do + allow(Gitlab::Metrics::Transaction).to receive(:current). + and_return(transaction) + end + + it 'adds a metric to the current transaction' do + expect(transaction).to receive(:add_metric). + with(:foo, { duration: a_kind_of(Numeric) }, { tag: 'value' }) + + Gitlab::Metrics.measure(:foo, {}, tag: 'value') { 10 } + end + + it 'supports adding of custom values' do + values = { duration: a_kind_of(Numeric), number: 10 } + + expect(transaction).to receive(:add_metric). + with(:foo, values, { tag: 'value' }) + + Gitlab::Metrics.measure(:foo, { number: 10 }, tag: 'value') { 10 } + end + + it 'returns the return value of the block' do + val = Gitlab::Metrics.measure(:foo) { 10 } + + expect(val).to eq(10) + end + end + end end From 352bc7d2b9a4ed5725e4f0e9f2e63d71678de9bc Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 5 Apr 2016 12:37:41 +0200 Subject: [PATCH 20/28] Added dev guide for measuring Ruby blocks --- doc/development/README.md | 1 + doc/development/instrumentation.md | 37 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) create mode 100644 doc/development/instrumentation.md diff --git a/doc/development/README.md b/doc/development/README.md index 1b281809af..8940b558fb 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -4,6 +4,7 @@ - [CI setup](ci_setup.md) for testing GitLab - [Gotchas](gotchas.md) to avoid - [How to dump production data to staging](db_dump.md) +- [Instrumentation](instrumentation.md) - [Migration Style Guide](migration_style_guide.md) for creating safe migrations - [Rake tasks](rake_tasks.md) for development - [Shell commands](shell_commands.md) in the GitLab codebase diff --git a/doc/development/instrumentation.md b/doc/development/instrumentation.md new file mode 100644 index 0000000000..c0192bd670 --- /dev/null +++ b/doc/development/instrumentation.md @@ -0,0 +1,37 @@ +# Instrumenting Ruby Code + +GitLab Performance Monitoring allows instrumenting of custom blocks of Ruby +code. This can be used to measure the time spent in a specific part of a larger +chunk of code. The resulting data is written to a separate series. + +To start measuring a block of Ruby code you should use +`Gitlab::Metrics.measure` and give it a name for the series to store the data +in: + +```ruby +Gitlab::Metrics.measure(:user_logins) do + ... +end +``` + +The first argument of this method is the series name and should be plural. This +name will be prefixed with `rails_` or `sidekiq_` depending on whether the code +was run in the Rails application or one of the Sidekiq workers. In the +above example the final series names would be as follows: + +- rails_user_logins +- sidekiq_user_logins + +Series names should be plural as this keeps the naming style in line with the +other series names. + +By default metrics measured using a block contain a single value, "duration", +which contains the number of milliseconds it took to execute the block. Custom +values can be added by passing a Hash as the 2nd argument. Custom tags can be +added by passing a Hash as the 3rd argument. A simple example is as follows: + +```ruby +Gitlab::Metrics.measure(:example_series, { number: 10 }, { class: self.class.to_s }) do + ... +end +``` From 62b6963255696192a015bbc28cb21de159572c1d Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Wed, 6 Apr 2016 01:57:21 +0200 Subject: [PATCH 21/28] Expose user location in API --- CHANGELOG | 1 + doc/api/users.md | 6 ++++++ lib/api/entities.rb | 2 +- lib/api/users.rb | 6 ++++-- 4 files changed, 12 insertions(+), 3 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e0ae6b7975..cba1a97cd1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -23,6 +23,7 @@ v 8.7.0 (unreleased) - Improved UX of the navigation sidebar - Build status notifications - API: Ability to retrieve a specific tag (Robert Schilling) + - API: Expose user location (Robert Schilling) v 8.6.5 (unreleased) - Check permissions when user attempts to import members from another project diff --git a/doc/api/users.md b/doc/api/users.md index 383e7c76ab..7d2b4897cf 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -69,6 +69,7 @@ GET /users "state": "blocked", "created_at": "2012-05-23T08:01:01Z", "bio": null, + "location": null, "skype": "", "linkedin": "", "twitter": "", @@ -126,6 +127,7 @@ Parameters: "created_at": "2012-05-23T08:00:58Z", "is_admin": false, "bio": null, + "location": null, "skype": "", "linkedin": "", "twitter": "", @@ -154,6 +156,7 @@ Parameters: "confirmed_at": "2012-05-23T08:00:58Z", "last_sign_in_at": "2015-03-23T08:00:58Z", "bio": null, + "location": null, "skype": "", "linkedin": "", "twitter": "", @@ -191,6 +194,7 @@ Parameters: - `extern_uid` (optional) - External UID - `provider` (optional) - External provider name - `bio` (optional) - User's biography +- `location` (optional) - User's location - `admin` (optional) - User is admin - true or false (default) - `can_create_group` (optional) - User can create groups - true or false - `confirm` (optional) - Require confirmation - true (default) or false @@ -218,6 +222,7 @@ Parameters: - `extern_uid` - External UID - `provider` - External provider name - `bio` - User's biography +- `location` (optional) - User's location - `admin` (optional) - User is admin - true or false (default) - `can_create_group` (optional) - User can create groups - true or false - `external` (optional) - Flags the user as external - true or false(default) @@ -260,6 +265,7 @@ GET /user "state": "active", "created_at": "2012-05-23T08:00:58Z", "bio": null, + "location": null, "skype": "", "linkedin": "", "twitter": "", diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 340fc5452a..4c49442bf8 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -15,7 +15,7 @@ module API class User < UserBasic expose :created_at expose :is_admin?, as: :is_admin - expose :bio, :skype, :linkedin, :twitter, :website_url + expose :bio, :location, :skype, :linkedin, :twitter, :website_url end class Identity < Grape::Entity diff --git a/lib/api/users.rb b/lib/api/users.rb index 13ab17c690..0a14bac07c 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -58,6 +58,7 @@ module API # extern_uid - External authentication provider UID # provider - External provider # bio - Bio + # location - Location of the user # admin - User is admin - true or false (default) # can_create_group - User can create groups - true or false # confirm - Require user confirmation - true (default) or false @@ -67,7 +68,7 @@ module API post do authenticated_as_admin! required_attributes! [:email, :password, :name, :username] - attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :bio, :can_create_group, :admin, :confirm, :external] + attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :projects_limit, :username, :bio, :location, :can_create_group, :admin, :confirm, :external] admin = attrs.delete(:admin) confirm = !(attrs.delete(:confirm) =~ (/(false|f|no|0)$/i)) user = User.build_user(attrs) @@ -106,6 +107,7 @@ module API # website_url - Website url # projects_limit - Limit projects each user can create # bio - Bio + # location - Location of the user # admin - User is admin - true or false (default) # can_create_group - User can create groups - true or false # external - Flags the user as external - true or false(default) @@ -114,7 +116,7 @@ module API put ":id" do authenticated_as_admin! - attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :website_url, :projects_limit, :username, :bio, :can_create_group, :admin, :external] + attrs = attributes_for_keys [:email, :name, :password, :skype, :linkedin, :twitter, :website_url, :projects_limit, :username, :bio, :location, :can_create_group, :admin, :external] user = User.find(params[:id]) not_found!('User') unless user From b818e74e0c111426be48df151c8e521120e4929c Mon Sep 17 00:00:00 2001 From: Kamil Trzcinski Date: Tue, 5 Apr 2016 16:16:34 +0200 Subject: [PATCH 22/28] Optimise .gitlab-ci.yml --- .gitlab-ci.yml | 76 -------------------------------------------------- 1 file changed, 76 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 336ceb3102..1dc49ca336 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -2,7 +2,6 @@ image: "ruby:2.1" services: - mysql:latest - - postgres:latest - redis:latest cache: @@ -35,123 +34,79 @@ spec:feature: script: - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spec:feature - tags: - - ruby - - mysql spec:api: stage: test script: - RAILS_ENV=test SIMPLECOV=true bundle exec rake spec:api - tags: - - ruby - - mysql spec:models: stage: test script: - RAILS_ENV=test SIMPLECOV=true bundle exec rake spec:models - tags: - - ruby - - mysql spec:lib: stage: test script: - RAILS_ENV=test SIMPLECOV=true bundle exec rake spec:lib - tags: - - ruby - - mysql spec:services: stage: test script: - RAILS_ENV=test SIMPLECOV=true bundle exec rake spec:services - tags: - - ruby - - mysql spec:other: stage: test script: - RAILS_ENV=test SIMPLECOV=true bundle exec rake spec:other - tags: - - ruby - - mysql spinach:project:half: stage: test script: - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:project:half - tags: - - ruby - - mysql spinach:project:rest: stage: test script: - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:project:rest - tags: - - ruby - - mysql spinach:other: stage: test script: - RAILS_ENV=test bundle exec rake assets:precompile 2>/dev/null - RAILS_ENV=test SIMPLECOV=true bundle exec rake spinach:other - tags: - - ruby - - mysql teaspoon: stage: test script: - RAILS_ENV=test bundle exec teaspoon - tags: - - ruby - - mysql rubocop: stage: test script: - bundle exec rubocop - tags: - - ruby - - mysql scss-lint: stage: test script: - bundle exec rake scss_lint - tags: - - ruby brakeman: stage: test script: - bundle exec rake brakeman - tags: - - ruby - - mysql flog: stage: test script: - bundle exec rake flog - tags: - - ruby - - mysql flay: stage: test script: - bundle exec rake flay - tags: - - ruby - - mysql bundler:audit: stage: test @@ -159,9 +114,6 @@ bundler:audit: - master script: - "bundle exec bundle-audit check --update --ignore OSVDB-115941" - tags: - - ruby - - mysql # Ruby 2.2 jobs @@ -177,9 +129,6 @@ spec:feature:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql spec:api:ruby22: stage: test @@ -192,9 +141,6 @@ spec:api:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql spec:models:ruby22: stage: test @@ -207,9 +153,6 @@ spec:models:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql spec:lib:ruby22: stage: test @@ -222,9 +165,6 @@ spec:lib:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql spec:services:ruby22: stage: test @@ -237,9 +177,6 @@ spec:services:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql spec:other:ruby22: stage: test @@ -252,9 +189,6 @@ spec:other:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql spinach:project:half:ruby22: stage: test @@ -268,9 +202,6 @@ spinach:project:half:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql spinach:project:rest:ruby22: stage: test @@ -284,9 +215,6 @@ spinach:project:rest:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql spinach:other:ruby22: stage: test @@ -300,10 +228,6 @@ spinach:other:ruby22: key: "ruby22" paths: - vendor - tags: - - ruby - - mysql - notify:slack: stage: notifications From e79b867d2b24f97cdf91315abd9dbbb22dc2d0e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Tue, 5 Apr 2016 18:15:13 +0200 Subject: [PATCH 23/28] Ensure empty recipients are rejected in BuildsEmailService MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Rémy Coutable --- CHANGELOG | 1 + .../project_services/builds_email_service.rb | 11 +++++---- .../builds_email_service_spec.rb | 24 +++++++++++++++++-- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index e0ae6b7975..3febc65da6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,6 +14,7 @@ v 8.7.0 (unreleased) - Add links to CI setup documentation from project settings and builds pages - Handle nil descriptions in Slack issue messages (Stan Hu) - Add default scope to projects to exclude projects pending deletion + - Ensure empty recipients are rejected in BuildsEmailService - Implement 'Groups View' as an option for dashboard preferences !3379 (Elias W.) - Implement 'TODOs View' as an option for dashboard preferences !3379 (Elias W.) - Gracefully handle notes on deleted commits in merge requests (Stan Hu) diff --git a/app/models/project_services/builds_email_service.rb b/app/models/project_services/builds_email_service.rb index f6313255cb..f9f0483876 100644 --- a/app/models/project_services/builds_email_service.rb +++ b/app/models/project_services/builds_email_service.rb @@ -50,12 +50,15 @@ class BuildsEmailService < Service def execute(push_data) return unless supported_events.include?(push_data[:object_kind]) + return unless should_build_be_notified?(push_data) - if should_build_be_notified?(push_data) + recipients = all_recipients(push_data) + + if recipients.any? BuildEmailWorker.perform_async( push_data[:build_id], - all_recipients(push_data), - push_data, + recipients, + push_data ) end end @@ -84,7 +87,7 @@ class BuildsEmailService < Service end def all_recipients(data) - all_recipients = recipients.split(',') + all_recipients = recipients.split(',').compact.reject(&:blank?) if add_pusher? && data[:user][:email] all_recipients << "#{data[:user][:email]}" diff --git a/spec/models/project_services/builds_email_service_spec.rb b/spec/models/project_services/builds_email_service_spec.rb index 905379a64e..2ccbff553f 100644 --- a/spec/models/project_services/builds_email_service_spec.rb +++ b/spec/models/project_services/builds_email_service_spec.rb @@ -6,18 +6,38 @@ describe BuildsEmailService do let(:service) { BuildsEmailService.new } describe :execute do - it "sends email" do + it 'sends email' do service.recipients = 'test@gitlab.com' data[:build_status] = 'failed' expect(BuildEmailWorker).to receive(:perform_async) service.execute(data) end - it "does not sends email with failed build and allowed_failure on" do + it 'does not send email with succeeded build and notify_only_broken_builds on' do + expect(service).to receive(:notify_only_broken_builds).and_return(true) + data[:build_status] = 'success' + expect(BuildEmailWorker).not_to receive(:perform_async) + service.execute(data) + end + + it 'does not send email with failed build and build_allow_failure is true' do data[:build_status] = 'failed' data[:build_allow_failure] = true expect(BuildEmailWorker).not_to receive(:perform_async) service.execute(data) end + + it 'does not send email with unknown build status' do + data[:build_status] = 'foo' + expect(BuildEmailWorker).not_to receive(:perform_async) + service.execute(data) + end + + it 'does not send email when recipients list is empty' do + service.recipients = ' ,, ' + data[:build_status] = 'failed' + expect(BuildEmailWorker).not_to receive(:perform_async) + service.execute(data) + end end end From ee87d15cf443e66f47e20b92bb442b9e1874fee7 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Tue, 5 Apr 2016 19:19:00 -0500 Subject: [PATCH 24/28] Change approach, do not use timeouts --- .../javascripts/search_autocomplete.js.coffee | 30 ++++++++----------- app/views/layouts/_search.html.haml | 4 +-- 2 files changed, 14 insertions(+), 20 deletions(-) diff --git a/app/assets/javascripts/search_autocomplete.js.coffee b/app/assets/javascripts/search_autocomplete.js.coffee index 564fb265b9..5a0df7817c 100644 --- a/app/assets/javascripts/search_autocomplete.js.coffee +++ b/app/assets/javascripts/search_autocomplete.js.coffee @@ -137,13 +137,17 @@ class @SearchAutocomplete } bindEvents: -> + $(document).on 'click', @onDocumentClick @searchInput.on 'keydown', @onSearchInputKeyDown @searchInput.on 'keyup', @onSearchInputKeyUp @searchInput.on 'click', @onSearchInputClick @searchInput.on 'focus', @onSearchInputFocus - @searchInput.on 'blur', @onSearchInputBlur @clearInput.on 'click', @onRemoveLocationClick + onDocumentClick: (e) => + if not $.contains(@dropdown[0], e.target) and @isFocused + @onSearchInputBlur() + enableAutocomplete: -> # No need to enable anything if user is not logged in return if !gon.current_user_id @@ -193,27 +197,21 @@ class @SearchAutocomplete e.stopImmediatePropagation() onSearchInputFocus: => + @isFocused = true @wrap.addClass('search-active') onRemoveLocationClick: (e) => e.preventDefault() @removeLocationBadge() @searchInput.val('').focus() - @skipBlurEvent = true onSearchInputBlur: (e) => - @skipBlurEvent = false + @isFocused = false + @wrap.removeClass('search-active') - # We should wait to make sure we are not clearing the input instead - setTimeout( => - return if @skipBlurEvent - - @wrap.removeClass('search-active') - - # If input is blank then restore state - if @searchInput.val() is '' - @restoreOriginalState() - , 150) + # If input is blank then restore state + if @searchInput.val() is '' + @restoreOriginalState() addLocationBadge: (item) -> category = if item.category? then "#{item.category}: " else '' @@ -291,9 +289,5 @@ class @SearchAutocomplete $el.removeClass('is-active') @disableAutocomplete() + @onSearchInputFocus() @searchInput.val('').focus() - - # We need to wait because of @skipBlurEvent - setTimeout( => - @onSearchInputFocus() - , 200) diff --git a/app/views/layouts/_search.html.haml b/app/views/layouts/_search.html.haml index 9d4ab9847a..ba04bd74b8 100644 --- a/app/views/layouts/_search.html.haml +++ b/app/views/layouts/_search.html.haml @@ -21,8 +21,8 @@ %a.is-focused.dropdown-menu-empty-link Loading... = dropdown_loading - %i.search-icon - %i.clear-icon.js-clear-input + %i.search-icon + %i.clear-icon.js-clear-input = hidden_field_tag :group_id, @group.try(:id) = hidden_field_tag :project_id, @project && @project.persisted? ? @project.id : '', id: 'search_project_id' From 5f4ce722c0f5fbf9129dddf20de879f00db4a311 Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 6 Apr 2016 13:55:49 -0500 Subject: [PATCH 25/28] Display clear button only if input has a value --- .../javascripts/search_autocomplete.js.coffee | 7 ++++--- app/assets/stylesheets/pages/search.scss | 16 ++++++++-------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/app/assets/javascripts/search_autocomplete.js.coffee b/app/assets/javascripts/search_autocomplete.js.coffee index 5a0df7817c..9c85ce1633 100644 --- a/app/assets/javascripts/search_autocomplete.js.coffee +++ b/app/assets/javascripts/search_autocomplete.js.coffee @@ -142,7 +142,7 @@ class @SearchAutocomplete @searchInput.on 'keyup', @onSearchInputKeyUp @searchInput.on 'click', @onSearchInputClick @searchInput.on 'focus', @onSearchInputFocus - @clearInput.on 'click', @onRemoveLocationClick + @clearInput.on 'click', @onClearInputClick onDocumentClick: (e) => if not $.contains(@dropdown[0], e.target) and @isFocused @@ -189,6 +189,8 @@ class @SearchAutocomplete # We should display the menu only when input is not empty @enableAutocomplete() + @wrap.toggleClass 'has-value', !!e.target.value + # Avoid falsy value to be returned return @@ -200,9 +202,8 @@ class @SearchAutocomplete @isFocused = true @wrap.addClass('search-active') - onRemoveLocationClick: (e) => + onClearInputClick: (e) => e.preventDefault() - @removeLocationBadge() @searchInput.val('').focus() onSearchInputBlur: (e) => diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 9274796233..f0f3744c6f 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -144,16 +144,16 @@ color: $location-icon-active-color; } } + } - &.has-location-badge { - .search-icon { - display: none; - } + &.has-value { + .search-icon { + display: none; + } - .clear-icon { - cursor: pointer; - display: block; - } + .clear-icon { + cursor: pointer; + display: block; } } From dc3272dccbf2462ad8a7ee8f677086ffabae3afb Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Wed, 6 Apr 2016 21:03:24 +0200 Subject: [PATCH 26/28] Revert "API: Ability to retrieve a single tag" This reverts commit 7f287c9136d5d1cdda8df170c6e772ca82aad1e9. --- doc/api/tags.md | 34 +--------------------------------- lib/api/tags.rb | 14 -------------- spec/requests/api/tags_spec.rb | 17 ----------------- 3 files changed, 1 insertion(+), 64 deletions(-) diff --git a/doc/api/tags.md b/doc/api/tags.md index c451a42b72..17d12e9cc6 100644 --- a/doc/api/tags.md +++ b/doc/api/tags.md @@ -38,38 +38,6 @@ Parameters: ] ``` -## Get a single repository tag - -Get a specific repository tag determined by its name. It returns 200 together -with the tag information if the tag exists. It returns 404 if the tag does not -exist. - -Parameters: - -- `id` (required) - The ID of a project -- `tag_name` (required) - The name of the tag - -```json -{ - "name": "v5.0.0", - "message": null, - "commit": { - "id": "60a8ff033665e1207714d6670fcd7b65304ec02f", - "message": "v5.0.0\n", - "parent_ids": [ - "f61c062ff8bcbdb00e0a1b3317a91aed6ceee06b" - ], - "authored_date": "2015-02-01T21:56:31.000+01:00", - "author_name": "Arthur Verschaeve", - "author_email": "contact@arthurverschaeve.be", - "committed_date": "2015-02-01T21:56:31.000+01:00", - "committer_name": "Arthur Verschaeve", - "committer_email": "contact@arthurverschaeve.be" - }, - "release": null -} -``` - ## Create a new tag Creates a new tag in the repository that points to the supplied ref. @@ -180,4 +148,4 @@ Parameters: "tag_name": "1.0.0", "description": "Amazing release. Wow" } -``` +``` \ No newline at end of file diff --git a/lib/api/tags.rb b/lib/api/tags.rb index 731a68082b..2d8a9e51bb 100644 --- a/lib/api/tags.rb +++ b/lib/api/tags.rb @@ -16,20 +16,6 @@ module API with: Entities::RepoTag, project: user_project end - # Get a single repository tag - # - # Parameters: - # id (required) - The ID of a project - # tag_name (required) - The name of the tag - # Example Request: - # GET /projects/:id/repository/tags/:tag_name - get ":id/repository/tags/:tag_name", requirements: { tag_name: /.*/ } do - tag = user_project.repository.find_tag(params[:tag_name]) - not_found!('Tag') unless tag - - present tag, with: Entities::RepoTag, project: user_project - end - # Create tag # # Parameters: diff --git a/spec/requests/api/tags_spec.rb b/spec/requests/api/tags_spec.rb index acbd9c3e33..a15be07ed5 100644 --- a/spec/requests/api/tags_spec.rb +++ b/spec/requests/api/tags_spec.rb @@ -40,23 +40,6 @@ describe API::API, api: true do end end - describe "GET /projects/:id/repository/tags/:tag_name" do - let(:tag_name) { project.repository.tag_names.sort.reverse.first } - - it 'should return a specific tag' do - get api("/projects/#{project.id}/repository/tags/#{tag_name}", user) - - expect(response.status).to eq(200) - expect(json_response['name']).to eq(tag_name) - end - - it 'should return 404 for an invalid tag name' do - get api("/projects/#{project.id}/repository/tags/foobar", user) - - expect(response.status).to eq(404) - end - end - describe 'POST /projects/:id/repository/tags' do context 'lightweight tags' do it 'should create a new tag' do From d1ad639010ecb8619db08f412babc6fb9e23458e Mon Sep 17 00:00:00 2001 From: Alfredo Sumaran Date: Wed, 6 Apr 2016 14:28:15 -0500 Subject: [PATCH 27/28] Do not fire blur event when clicking a suggestion --- app/assets/javascripts/search_autocomplete.js.coffee | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/search_autocomplete.js.coffee b/app/assets/javascripts/search_autocomplete.js.coffee index 9c85ce1633..6a7b4ad1db 100644 --- a/app/assets/javascripts/search_autocomplete.js.coffee +++ b/app/assets/javascripts/search_autocomplete.js.coffee @@ -145,7 +145,10 @@ class @SearchAutocomplete @clearInput.on 'click', @onClearInputClick onDocumentClick: (e) => - if not $.contains(@dropdown[0], e.target) and @isFocused + # If clicking outside the search box + # And search input is not focused + # And we are not clicking inside a suggestion + if not $.contains(@dropdown[0], e.target) and @isFocused and not $(e.target).parents('ul').length @onSearchInputBlur() enableAutocomplete: -> @@ -290,5 +293,4 @@ class @SearchAutocomplete $el.removeClass('is-active') @disableAutocomplete() - @onSearchInputFocus() @searchInput.val('').focus() From b3dd470854f83616bb6bd36bd73680bfda869d81 Mon Sep 17 00:00:00 2001 From: Robert Schilling Date: Wed, 6 Apr 2016 22:46:07 +0200 Subject: [PATCH 28/28] Fix wrong changelog item --- CHANGELOG | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index a23c8a54af..92a0d8547e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -23,7 +23,6 @@ v 8.7.0 (unreleased) - Remove "Congratulations!" tweet button on newly-created project. (Connor Shea) - Improved UX of the navigation sidebar - Build status notifications - - API: Ability to retrieve a specific tag (Robert Schilling) - API: Expose user location (Robert Schilling) v 8.6.5 (unreleased)