From 51266f41b3150b3a198bf8316d093a86d4d38cd1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 15 Jan 2016 15:28:16 +0100 Subject: [PATCH 01/43] Consider re-assign as a mention from a notification point of view This will ensure new assignee gets an email even if his notif level is "on mention". --- app/services/notification_service.rb | 16 +++++++++++----- spec/services/notification_service_spec.rb | 18 +++++++++++++++++- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index e4edc55bf6..60b86914a3 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -379,7 +379,7 @@ class NotificationService previous_assignee_id = previous_record(target, "assignee_id") previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id - recipients = build_recipients(target, project, current_user, [previous_assignee]) + recipients = build_recipients(target, project, current_user, action: :reassign, extra_recipients: [previous_assignee]) recipients.each do |recipient| mailer.send( @@ -400,22 +400,28 @@ class NotificationService end end - def build_recipients(target, project, current_user, extra_recipients = nil) + def build_recipients(target, project, current_user, action: nil, extra_recipients: nil) recipients = target.participants(current_user) recipients = recipients.concat(extra_recipients).compact.uniq if extra_recipients recipients = add_project_watchers(recipients, project) recipients = reject_mention_users(recipients, project) - recipients = reject_muted_users(recipients, project) + # Re-assign is considered as a mention of the new assignee so we add the + # new assignee to the list of recipients after we rejected users with + # the "on mention" notification level + if action == :reassign + recipients << target.assignee + end + + recipients = reject_muted_users(recipients, project) recipients = add_subscribed_users(recipients, target) recipients = reject_unsubscribed_users(recipients, target) recipients.delete(current_user) - recipients = recipients.uniq - recipients + recipients.uniq end def mailer diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 6d219f3589..b63903af41 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -227,9 +227,11 @@ describe NotificationService, services: true do end describe :reassigned_issue do - it 'should email new assignee' do + it 'emails new assignee' do + issue.update_attribute(:assignee, @u_mentioned) notification.reassigned_issue(issue, @u_disabled) + expect(issue.assignee).to be @u_mentioned should_email(issue.assignee) should_email(@u_watcher) should_email(@u_participant_mentioned) @@ -238,6 +240,20 @@ describe NotificationService, services: true do should_not_email(@u_participating) should_not_email(@u_disabled) end + + it 'does not email new assignee if they are the current user' do + issue.update_attribute(:assignee, @u_mentioned) + notification.reassigned_issue(issue, @u_mentioned) + + expect(issue.assignee).to be @u_mentioned + should_email(@u_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_not_email(issue.assignee) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + end end describe :close_issue do From 55ae5394a05fbde53067336a17ff2f03733775a0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 15 Jan 2016 16:38:18 +0100 Subject: [PATCH 02/43] Add changelog entry for fix-consider-re-assign-as-a-mention --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index d01576be67..f3916c39ec 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.4.0 (unreleased) + - Consider re-assign as a mention from a notification point of view - Add pagination headers to already paginated API resources - Properly generate diff of orphan commits, like the first commit in a repository - Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages From 30a4f4c9e0cc44b67ea6041b8aef01196649b8d3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Fri, 15 Jan 2016 17:57:45 +0100 Subject: [PATCH 03/43] This will ensure previous assignee gets an email even if his notif level is "on mention" --- app/services/notification_service.rb | 9 +++-- spec/services/notification_service_spec.rb | 40 ++++++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 60b86914a3..ca8a41d93b 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -376,10 +376,10 @@ class NotificationService end def reassign_resource_email(target, project, current_user, method) - previous_assignee_id = previous_record(target, "assignee_id") + previous_assignee_id = previous_record(target, 'assignee_id') previous_assignee = User.find_by(id: previous_assignee_id) if previous_assignee_id - recipients = build_recipients(target, project, current_user, action: :reassign, extra_recipients: [previous_assignee]) + recipients = build_recipients(target, project, current_user, action: :reassign, previous_assignee: previous_assignee) recipients.each do |recipient| mailer.send( @@ -400,11 +400,9 @@ class NotificationService end end - def build_recipients(target, project, current_user, action: nil, extra_recipients: nil) + def build_recipients(target, project, current_user, action: nil, previous_assignee: nil) recipients = target.participants(current_user) - recipients = recipients.concat(extra_recipients).compact.uniq if extra_recipients - recipients = add_project_watchers(recipients, project) recipients = reject_mention_users(recipients, project) @@ -412,6 +410,7 @@ class NotificationService # new assignee to the list of recipients after we rejected users with # the "on mention" notification level if action == :reassign + recipients << previous_assignee if previous_assignee recipients << target.assignee end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index b63903af41..2d0b5df422 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -227,6 +227,46 @@ describe NotificationService, services: true do end describe :reassigned_issue do + it 'emails new assignee' do + notification.reassigned_issue(issue, @u_disabled) + + should_email(issue.assignee) + should_email(@u_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + end + + it 'emails previous assignee even if he has the "on mention" notif level' do + issue.update_attribute(:assignee, @u_mentioned) + issue.update_attributes(assignee: @u_watcher) + notification.reassigned_issue(issue, @u_disabled) + + should_email(@u_mentioned) + should_email(@u_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + end + + it 'emails new assignee even if he has the "on mention" notif level' do + issue.update_attributes(assignee: @u_mentioned) + notification.reassigned_issue(issue, @u_disabled) + + expect(issue.assignee).to be @u_mentioned + should_email(issue.assignee) + should_email(@u_watcher) + should_email(@u_participant_mentioned) + should_email(@subscriber) + should_not_email(@unsubscriber) + should_not_email(@u_participating) + should_not_email(@u_disabled) + end + it 'emails new assignee' do issue.update_attribute(:assignee, @u_mentioned) notification.reassigned_issue(issue, @u_disabled) From c12bd5474e7c6caa9a2af4287cf25e84b47a6b74 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Sat, 16 Jan 2016 10:14:13 -0500 Subject: [PATCH 04/43] Search title was getting clipped horizontally. Fixed by adding height to search title css. --- app/assets/stylesheets/pages/search.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/pages/search.scss b/app/assets/stylesheets/pages/search.scss index 3aaa96da60..bdcf189752 100644 --- a/app/assets/stylesheets/pages/search.scss +++ b/app/assets/stylesheets/pages/search.scss @@ -3,6 +3,10 @@ border-bottom: 1px solid #DDD; padding-bottom: 15px; margin-bottom: 15px; + + .term { + height: 22px; + } } } From adcab296fd48074daf9cac903d90270391f3e9d1 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Fri, 15 Jan 2016 10:33:42 +0100 Subject: [PATCH 05/43] Expose link to CI Lint Tool on builds page --- app/views/projects/builds/index.html.haml | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml index 5d18c0d803..36b308b7b8 100644 --- a/app/views/projects/builds/index.html.haml +++ b/app/views/projects/builds/index.html.haml @@ -6,7 +6,19 @@ - if can?(current_user, :manage_builds, @project) .pull-left.hidden-xs - if @all_builds.running_or_pending.any? - = link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post + = link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project), + data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post + + .pull-left.hidden-xs.hidden-sm{ style: 'margin-left: 7px;' } + .dropdown + %button.btn.btn-default.dropdown-toggle{ type: 'button', id: 'tools-dropdown', 'data-toggle' => 'dropdown' } + Tools + %span.caret + %ul.dropdown-menu.dropdown-menu-right + %li + = link_to ci_lint_path do + = icon('wrench') + %span CI Lint Tool %ul.nav-links %li{class: ('active' if @scope.nil?)} From 59ffe978fd5e89b55c737a030314a0bc3fdb85bd Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sun, 17 Jan 2016 16:33:53 +0100 Subject: [PATCH 06/43] Reuse existing CSS classes when exposing CI Lint tool --- app/views/projects/builds/index.html.haml | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml index 36b308b7b8..747d94cc9e 100644 --- a/app/views/projects/builds/index.html.haml +++ b/app/views/projects/builds/index.html.haml @@ -9,16 +9,15 @@ = link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post - .pull-left.hidden-xs.hidden-sm{ style: 'margin-left: 7px;' } - .dropdown - %button.btn.btn-default.dropdown-toggle{ type: 'button', id: 'tools-dropdown', 'data-toggle' => 'dropdown' } - Tools - %span.caret - %ul.dropdown-menu.dropdown-menu-right - %li - = link_to ci_lint_path do - = icon('wrench') - %span CI Lint Tool + .dropdown.inline + %button.btn.btn-default.dropdown-toggle{ type: 'button', id: 'tools-dropdown', 'data-toggle' => 'dropdown' } + Tools + %span.caret + %ul.dropdown-menu.dropdown-menu-right + %li + = link_to ci_lint_path do + = icon('wrench') + %span CI Lint Tool %ul.nav-links %li{class: ('active' if @scope.nil?)} From d9629a0cf03b80c8074b2ffa003d541193ca7097 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sun, 17 Jan 2016 16:57:42 +0100 Subject: [PATCH 07/43] Add specs for CI Lint button exposed on a builds page --- features/project/builds/summary.feature | 10 +++++++--- features/steps/project/builds/summary.rb | 9 ++++++++- features/steps/shared/builds.rb | 6 +++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/features/project/builds/summary.feature b/features/project/builds/summary.feature index e90ea592aa..9f006fcaca 100644 --- a/features/project/builds/summary.feature +++ b/features/project/builds/summary.feature @@ -5,7 +5,11 @@ Feature: Project Builds Summary And project has CI enabled And project has a recent build - Scenario: I browse build summary page - When I visit recent build summary page - Then I see summary for build + Scenario: I browse build details page + When I visit recent build details page + Then I see details of a build And I see build trace + + Scenario: I browse project builds page + When I visit project builds page + Then I see button to CI Lint Tool diff --git a/features/steps/project/builds/summary.rb b/features/steps/project/builds/summary.rb index 2439d48fbe..67986b958e 100644 --- a/features/steps/project/builds/summary.rb +++ b/features/steps/project/builds/summary.rb @@ -4,11 +4,18 @@ class Spinach::Features::ProjectBuildsSummary < Spinach::FeatureSteps include SharedBuilds include RepoHelpers - step 'I see summary for build' do + step 'I see details of a build' do expect(page).to have_content "Build ##{@build.id}" end step 'I see build trace' do expect(page).to have_css '#build-trace' end + + step 'I see button to CI Lint Tool' do + page.within('.controls') do + ci_lint_tool_link = page.find_link('CI Lint Tool') + expect(ci_lint_tool_link[:href]).to eq ci_lint_path + end + end end diff --git a/features/steps/shared/builds.rb b/features/steps/shared/builds.rb index f88b01af84..92bf362879 100644 --- a/features/steps/shared/builds.rb +++ b/features/steps/shared/builds.rb @@ -10,10 +10,14 @@ module SharedBuilds @build = create :ci_build, commit: ci_commit end - step 'I visit recent build summary page' do + step 'I visit recent build details page' do visit namespace_project_build_path(@project.namespace, @project, @build) end + step 'I visit project builds page' do + visit namespace_project_builds_path(@project.namespace, @project) + end + step 'recent build has artifacts available' do artifacts = Rails.root + 'spec/fixtures/ci_build_artifacts.zip' archive = fixture_file_upload(artifacts, 'application/zip') From 968998a9d18cd6e108abafc6ba4606ce7249fd0a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sun, 17 Jan 2016 17:04:00 +0100 Subject: [PATCH 08/43] Add Changelog entry for exposing CI Lint tool on builds page --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index a15bbfbc49..102382b02f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -58,6 +58,7 @@ v 8.4.0 (unreleased) - Import GitHub wiki into GitLab - Add reporters ability to download and browse build artifacts (Andrew Johnson) - Autofill referring url in message box when reporting user abuse. (Josh Frye) + - Expose button to CI Lint tool on project builds page v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) From 7506afe14bad7bb4f93e26a539b3ffe75303e180 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Sun, 17 Jan 2016 18:13:45 +0100 Subject: [PATCH 09/43] Update build artifacts feature tests --- features/project/builds/artifacts.feature | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/features/project/builds/artifacts.feature b/features/project/builds/artifacts.feature index 7a7dbb71b1..d17c8aa89e 100644 --- a/features/project/builds/artifacts.feature +++ b/features/project/builds/artifacts.feature @@ -7,21 +7,21 @@ Feature: Project Builds Artifacts Scenario: I download build artifacts Given recent build has artifacts available - When I visit recent build summary page + When I visit recent build details page And I click artifacts download button Then download of build artifacts archive starts Scenario: I browse build artifacts Given recent build has artifacts available And recent build has artifacts metadata available - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button Then I should see content of artifacts archive Scenario: I browse subdirectory of build artifacts Given recent build has artifacts available And recent build has artifacts metadata available - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button And I click link to subdirectory within build artifacts Then I should see content of subdirectory within artifacts archive @@ -30,7 +30,7 @@ Feature: Project Builds Artifacts Given recent build has artifacts available And recent build has artifacts metadata available And recent build artifacts contain directory with UTF-8 characters - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button And I navigate to directory with UTF-8 characters in name Then I should see content of directory with UTF-8 characters in name @@ -39,7 +39,7 @@ Feature: Project Builds Artifacts Given recent build has artifacts available And recent build has artifacts metadata available And recent build artifacts contain directory with invalid UTF-8 characters - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button And I navigate to parent directory of directory with invalid name Then I should not see directory with invalid name on the list @@ -47,7 +47,7 @@ Feature: Project Builds Artifacts Scenario: I download a single file from build artifacts Given recent build has artifacts available And recent build has artifacts metadata available - When I visit recent build summary page + When I visit recent build details page And I click artifacts browse button And I click download button for a file within build artifacts Then download of a file extracted from build artifacts should start From 182cc843b02a6773b8650b4e0f2e42a099bef0bd Mon Sep 17 00:00:00 2001 From: Zeger-Jan van de Weg Date: Fri, 15 Jan 2016 16:04:48 +0100 Subject: [PATCH 10/43] Remove leading comma when user is the only one to award that emoji Fixes #9073 --- CHANGELOG | 1 + app/assets/javascripts/awards_handler.coffee | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index a15bbfbc49..15b7f6b0d6 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -58,6 +58,7 @@ v 8.4.0 (unreleased) - Import GitHub wiki into GitLab - Add reporters ability to download and browse build artifacts (Andrew Johnson) - Autofill referring url in message box when reporting user abuse. (Josh Frye) + - Remove leading comma on award emoji when the user is the first to award the emoji (Zeger-Jan van de Weg) v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) diff --git a/app/assets/javascripts/awards_handler.coffee b/app/assets/javascripts/awards_handler.coffee index 619abb1fb0..8b7e0723f6 100644 --- a/app/assets/javascripts/awards_handler.coffee +++ b/app/assets/javascripts/awards_handler.coffee @@ -19,7 +19,7 @@ class @AwardsHandler @addAwardToEmojiBar(emoji) $(".emoji-menu").hide() - + addAwardToEmojiBar: (emoji) -> @addEmojiToFrequentlyUsedList(emoji) @@ -66,9 +66,14 @@ class @AwardsHandler addMeToAuthorList: (emoji) -> award_block = @findEmojiIcon(emoji).parent() - authors = award_block.attr("data-original-title").split(", ") + authors = _.compact(award_block.attr("data-original-title").split(", ")) authors.push("me") - award_block.attr("title", authors.join(", ")) + + if authors.length == 1 + award_block.attr("title", "me") + else + award_block.attr("title", authors.join(", ")) + @resetTooltip(award_block) resetTooltip: (award) -> @@ -78,7 +83,7 @@ class @AwardsHandler setTimeout (-> award.tooltip() ), 200 - + createEmoji: (emoji) -> emojiCssClass = @resolveNameToCssClass(emoji) From 6fe33804dc080522242d9ea3639548c2b246a56b Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 18 Jan 2016 08:12:31 +0100 Subject: [PATCH 11/43] Remove dropdown containing CI Lint button on builds page Link to CI Lint on builds page will be a separate button from now on. --- app/views/projects/builds/index.html.haml | 12 +++--------- features/project/builds/summary.feature | 2 +- features/steps/project/builds/summary.rb | 4 ++-- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml index 747d94cc9e..bbb6944a65 100644 --- a/app/views/projects/builds/index.html.haml +++ b/app/views/projects/builds/index.html.haml @@ -9,15 +9,9 @@ = link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project), data: { confirm: 'Are you sure?' }, class: 'btn btn-danger', method: :post - .dropdown.inline - %button.btn.btn-default.dropdown-toggle{ type: 'button', id: 'tools-dropdown', 'data-toggle' => 'dropdown' } - Tools - %span.caret - %ul.dropdown-menu.dropdown-menu-right - %li - = link_to ci_lint_path do - = icon('wrench') - %span CI Lint Tool + = link_to ci_lint_path, class: 'btn btn-default' do + = icon('wrench') + %span CI Lint %ul.nav-links %li{class: ('active' if @scope.nil?)} diff --git a/features/project/builds/summary.feature b/features/project/builds/summary.feature index 9f006fcaca..b69d279517 100644 --- a/features/project/builds/summary.feature +++ b/features/project/builds/summary.feature @@ -12,4 +12,4 @@ Feature: Project Builds Summary Scenario: I browse project builds page When I visit project builds page - Then I see button to CI Lint Tool + Then I see button to CI Lint diff --git a/features/steps/project/builds/summary.rb b/features/steps/project/builds/summary.rb index 67986b958e..036bc0a499 100644 --- a/features/steps/project/builds/summary.rb +++ b/features/steps/project/builds/summary.rb @@ -12,9 +12,9 @@ class Spinach::Features::ProjectBuildsSummary < Spinach::FeatureSteps expect(page).to have_css '#build-trace' end - step 'I see button to CI Lint Tool' do + step 'I see button to CI Lint' do page.within('.controls') do - ci_lint_tool_link = page.find_link('CI Lint Tool') + ci_lint_tool_link = page.find_link('CI Lint') expect(ci_lint_tool_link[:href]).to eq ci_lint_path end end From 2723dea6673b061dae2c266318727f9fd5aed8d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9my=20Coutable?= Date: Mon, 18 Jan 2016 12:35:30 +0100 Subject: [PATCH 12/43] Ensure Gravatar host looks like an actual host --- CHANGELOG | 1 + config/initializers/1_settings.rb | 22 +++++++++------ spec/initializers/settings_spec.rb | 44 ++++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 spec/initializers/settings_spec.rb diff --git a/CHANGELOG b/CHANGELOG index a15bbfbc49..e3a1bc5d9f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.4.0 (unreleased) + - Ensure Gravatar host looks like an actual host - Add pagination headers to already paginated API resources - Properly generate diff of orphan commits, like the first commit in a repository - Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index d625a909bf..04a7c16ebd 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -9,13 +9,8 @@ class Settings < Settingslogic gitlab.port.to_i == (gitlab.https ? 443 : 80) end - # get host without www, thanks to http://stackoverflow.com/a/6674363/1233435 - def get_host_without_www(url) - url = CGI.escape(url) - uri = URI.parse(url) - uri = URI.parse("http://#{url}") if uri.scheme.nil? - host = uri.host.downcase - host.start_with?('www.') ? host[4..-1] : host + def host_without_www(url) + host(url).sub('www.', '') end def build_gitlab_ci_url @@ -87,6 +82,17 @@ class Settings < Settingslogic custom_port ] end + + # Extract the host part of the given +url+. + def host(url) + url = url.downcase + url = "http://#{url}" unless url.start_with?('http') + + # Get rid of the path so that we don't even have to encode it + url_without_path = url.sub(%r{(https?://[^\/]+)/?.*}, '\1') + + URI.parse(url_without_path).host + end end end @@ -228,7 +234,7 @@ Settings['gravatar'] ||= Settingslogic.new({}) Settings.gravatar['enabled'] = true if Settings.gravatar['enabled'].nil? Settings.gravatar['plain_url'] ||= 'http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon' Settings.gravatar['ssl_url'] ||= 'https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon' -Settings.gravatar['host'] = Settings.get_host_without_www(Settings.gravatar['plain_url']) +Settings.gravatar['host'] = Settings.host_without_www(Settings.gravatar['plain_url']) # # Cron Jobs diff --git a/spec/initializers/settings_spec.rb b/spec/initializers/settings_spec.rb new file mode 100644 index 0000000000..e58f2c80e9 --- /dev/null +++ b/spec/initializers/settings_spec.rb @@ -0,0 +1,44 @@ +require_relative '../../config/initializers/1_settings' + +describe Settings, lib: true do + + describe '#host_without_www' do + context 'URL with protocol' do + it 'returns the host' do + expect(Settings.host_without_www('http://foo.com')).to eq 'foo.com' + expect(Settings.host_without_www('http://www.foo.com')).to eq 'foo.com' + expect(Settings.host_without_www('http://secure.foo.com')).to eq 'secure.foo.com' + expect(Settings.host_without_www('http://www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com' + + expect(Settings.host_without_www('https://foo.com')).to eq 'foo.com' + expect(Settings.host_without_www('https://www.foo.com')).to eq 'foo.com' + expect(Settings.host_without_www('https://secure.foo.com')).to eq 'secure.foo.com' + expect(Settings.host_without_www('https://secure.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'secure.gravatar.com' + end + end + + context 'URL without protocol' do + it 'returns the host' do + expect(Settings.host_without_www('foo.com')).to eq 'foo.com' + expect(Settings.host_without_www('www.foo.com')).to eq 'foo.com' + expect(Settings.host_without_www('secure.foo.com')).to eq 'secure.foo.com' + expect(Settings.host_without_www('www.gravatar.com/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com' + end + + context 'URL with user/port' do + it 'returns the host' do + expect(Settings.host_without_www('bob:pass@foo.com:8080')).to eq 'foo.com' + expect(Settings.host_without_www('bob:pass@www.foo.com:8080')).to eq 'foo.com' + expect(Settings.host_without_www('bob:pass@secure.foo.com:8080')).to eq 'secure.foo.com' + expect(Settings.host_without_www('bob:pass@www.gravatar.com:8080/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com' + + expect(Settings.host_without_www('http://bob:pass@foo.com:8080')).to eq 'foo.com' + expect(Settings.host_without_www('http://bob:pass@www.foo.com:8080')).to eq 'foo.com' + expect(Settings.host_without_www('http://bob:pass@secure.foo.com:8080')).to eq 'secure.foo.com' + expect(Settings.host_without_www('http://bob:pass@www.gravatar.com:8080/avatar/%{hash}?s=%{size}&d=identicon')).to eq 'gravatar.com' + end + end + end + end + +end From 4472c07f8149251a755138a9d2910e819bb555c4 Mon Sep 17 00:00:00 2001 From: Jacob Vosmaer Date: Mon, 18 Jan 2016 16:06:03 +0100 Subject: [PATCH 13/43] Use gitlab-workhorse 0.6.0 --- GITLAB_WORKHORSE_VERSION | 2 +- doc/update/8.3-to-8.4.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index 7d8568351b..a918a2aa18 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -0.5.4 +0.6.0 diff --git a/doc/update/8.3-to-8.4.md b/doc/update/8.3-to-8.4.md index 1cbeab3eca..604939cd73 100644 --- a/doc/update/8.3-to-8.4.md +++ b/doc/update/8.3-to-8.4.md @@ -48,7 +48,7 @@ which should already be on your system from GitLab 8.1. ```bash cd /home/git/gitlab-workhorse sudo -u git -H git fetch --all -sudo -u git -H git checkout 0.5.4 +sudo -u git -H git checkout 0.6.0 sudo -u git -H make ``` From 9f03ae8c91e2026c65346ab68c3cd737ab2e3c94 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 18 Jan 2016 12:38:21 -0500 Subject: [PATCH 14/43] Update CHANGELOG and VERSION for 8.5.0-pre --- CHANGELOG | 2 ++ VERSION | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index e3a1bc5d9f..96da0cde9e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. +v 8.5.0 (unreleased) + v 8.4.0 (unreleased) - Ensure Gravatar host looks like an actual host - Add pagination headers to already paginated API resources diff --git a/VERSION b/VERSION index ce66973011..ffec98087c 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -8.4.0.pre +8.5.0-pre From 13f0ea12e3b99a59c3387aa2646a8b84220ba4a4 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 18 Jan 2016 10:18:16 +0100 Subject: [PATCH 15/43] Improve ux in builds artifacts browser --- .../artifacts/_tree_directory.html.haml | 6 +++-- .../projects/artifacts/_tree_file.html.haml | 10 +++---- app/views/projects/artifacts/browse.html.haml | 26 +++++++++++-------- features/project/builds/artifacts.feature | 2 +- features/steps/project/builds/artifacts.rb | 4 +-- 5 files changed, 27 insertions(+), 21 deletions(-) diff --git a/app/views/projects/artifacts/_tree_directory.html.haml b/app/views/projects/artifacts/_tree_directory.html.haml index 5b87d55efd..e4b7979949 100644 --- a/app/views/projects/artifacts/_tree_directory.html.haml +++ b/app/views/projects/artifacts/_tree_directory.html.haml @@ -1,7 +1,9 @@ -%tr{ class: 'tree-item' } +- path_to_directory = browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build, path: directory.path) + +%tr.tree-item{ 'data-link' => path_to_directory} %td.tree-item-file-name = tree_icon('folder', '755', directory.name) %span.str-truncated - = link_to directory.name, browse_namespace_project_build_artifacts_path(@project.namespace, @project, @build, path: directory.path) + = link_to directory.name, path_to_directory %td %td diff --git a/app/views/projects/artifacts/_tree_file.html.haml b/app/views/projects/artifacts/_tree_file.html.haml index 92c1648f72..3dfc09cc49 100644 --- a/app/views/projects/artifacts/_tree_file.html.haml +++ b/app/views/projects/artifacts/_tree_file.html.haml @@ -1,11 +1,11 @@ -%tr{ class: 'tree-item' } +- path_to_file = file_namespace_project_build_artifacts_path(@project.namespace, @project, @build, path: file.path) + +%tr.tree-item{ 'data-link' => path_to_file } %td.tree-item-file-name = tree_icon('file', '664', file.name) %span.str-truncated - = file.name + = link_to file.name, path_to_file %td = number_to_human_size(file.metadata[:size], precision: 2) %td - = link_to file_namespace_project_build_artifacts_path(@project.namespace, @project, @build, path: file.path), - class: 'btn btn-xs btn-default artifact-download' do - = icon('download') + = number_to_human_size(file.metadata[:zipped], precision: 2) diff --git a/app/views/projects/artifacts/browse.html.haml b/app/views/projects/artifacts/browse.html.haml index 1add7ef6bf..9dc7b2c2e1 100644 --- a/app/views/projects/artifacts/browse.html.haml +++ b/app/views/projects/artifacts/browse.html.haml @@ -1,24 +1,28 @@ - page_title 'Artifacts', "#{@build.name} (##{@build.id})", 'Builds' = render 'projects/builds/header_title' -#tree-holder.tree-holder - .gray-content-block.top-block.clearfix - .pull-right - = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build), - class: 'btn btn-default' do - = icon('download') - Download artifacts archive +.top-block.gray-content-block.clearfix + .pull-right + = link_to download_namespace_project_build_artifacts_path(@project.namespace, @project, @build), + class: 'btn btn-default' do + = icon('download') + Download artifacts archive -%div.tree-content-holder - .table-holder - %table.table.tree-table.table-striped +.tree-holder + %div.tree-content-holder + %table.table.tree-table %thead %tr %th Name %th Size - %th Download + %th Compressed to = render partial: 'tree_directory', collection: @entry.directories(parent: true), as: :directory = render partial: 'tree_file', collection: @entry.files, as: :file - if @entry.empty? .center Empty + +:javascript + $(document).on('click', 'tr[data-link]', function(e) { + window.location = this.dataset.link; + }); diff --git a/features/project/builds/artifacts.feature b/features/project/builds/artifacts.feature index 7a7dbb71b1..4f68e44fd7 100644 --- a/features/project/builds/artifacts.feature +++ b/features/project/builds/artifacts.feature @@ -49,5 +49,5 @@ Feature: Project Builds Artifacts And recent build has artifacts metadata available When I visit recent build summary page And I click artifacts browse button - And I click download button for a file within build artifacts + And I click a link to file within build artifacts Then download of a file extracted from build artifacts should start diff --git a/features/steps/project/builds/artifacts.rb b/features/steps/project/builds/artifacts.rb index f2c87da471..25f2f4e837 100644 --- a/features/steps/project/builds/artifacts.rb +++ b/features/steps/project/builds/artifacts.rb @@ -63,8 +63,8 @@ class Spinach::Features::ProjectBuildsArtifacts < Spinach::FeatureSteps end end - step 'I click download button for a file within build artifacts' do - page.within('.tree-table') { first('.artifact-download').click } + step 'I click a link to file within build artifacts' do + page.within('.tree-table') { find_link('ci_artifacts.txt').click } end step 'download of a file extracted from build artifacts should start' do From c7fe3c846ac98daa5057c4299d2d414b1d95df49 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Mon, 18 Jan 2016 10:18:47 +0100 Subject: [PATCH 16/43] Update build artifacts metadata fixture to 0.0.2 --- spec/fixtures/ci_build_artifacts_metadata.gz | Bin 415 -> 461 bytes 1 file changed, 0 insertions(+), 0 deletions(-) diff --git a/spec/fixtures/ci_build_artifacts_metadata.gz b/spec/fixtures/ci_build_artifacts_metadata.gz index fe9d4c8c661ebd27c55c589aafd8ff1a0e5ecbdc..e6d17e4595d61ff48dcd459f48183fa7f7c75757 100644 GIT binary patch literal 461 zcmV;;0W$s{iwFP!32ul0|D2Q0Zqq;z#v_hg=!qjT7H}a+clNKhaDfZrpOAO~Yh$m& zBFDDuwM1L>DY)_qycAEu7`m9oPD@dw$QsXl-}hV3E*6U?ubh8pHqp!8m3j1{^{z0P z@1pnCo7{L4ku_PvUF^Jbeu4i-nM+ORTKg{^Yj4lvU7fqa**rc0E`Vw+7+J-eEF~FK0hs1O|(@G-p5$u#}bd23Vss;|0!le^Yt zzHUzsAxX-oNY8JfyD=610_xTkIREilREpn-@C29+s0zvT$f|$?nbJPHMJl^%{Av1s z^!L2&P~BD;wpZJ^St>M9bF&ZkW_w>G(NK#pOZ4O0*F$Prkg3}7jcTpk_N8gl!j(3q zIPzwy4MfSXWkhJiCS*MNzm9!+>!q%M0QSG01JU84 zGbROaM&!S79JR)kU0Bl3&GyD6WuPQw9C#8O7)B7I3j+;u&*^6X00960^T;u!any42q`V3?i1{}iz z8HU8R*{AU8EBI1Ai65i{+wNjFBpkl^{rzS(o6YOb)_>94;L~91BKYXMEp_hu;H&Yv z&|U{9K?yzs)jV{s*yT$o}u8NE4dq?>6T_rtcZ&g8yzS1WPfouIPS4SaKr%9g=j^8H}?4e1bo} zey(sY{>ycy8fxvb($*#;K#>oW=9@=c5v34XR`$=jCc78+?3%WTN6Q%8;@>Mdqjln) zkQngdSV&4!ju}Pi(TwL#+q$2h$@i{WdnOc%lu(A5ob@PHI9)U;C!Fuk{ssU5|Nq@y J Date: Mon, 18 Jan 2016 10:21:15 +0100 Subject: [PATCH 17/43] Add Changelog entry for improvements in builds artifacts browser --- CHANGELOG | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 9c242cb44a..18ccd56dec 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -62,10 +62,11 @@ v 8.4.0 (unreleased) - Add reporters ability to download and browse build artifacts (Andrew Johnson) - Autofill referring url in message box when reporting user abuse. (Josh Frye) - Remove leading comma on award emoji when the user is the first to award the emoji (Zeger-Jan van de Weg) + - Add build artifacts browser + - Improve UX in builds artifacts browser v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) - - Add build artifacts browser v 8.3.3 - Preserve CE behavior with JIRA integration by only calling API if URL is set From ab33b93daee12b1e33bf6b961de0860e5be465b3 Mon Sep 17 00:00:00 2001 From: Drew Blessing Date: Mon, 18 Jan 2016 19:18:35 +0000 Subject: [PATCH 18/43] Move entry `Fix: As an admin, cannot add oneself as a member to a group/project` --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 9c242cb44a..c0b2b75b7e 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -113,6 +113,7 @@ v 8.3.0 - Fix: Assignee selector is empty when 'Unassigned' is selected (Jose Corcuera) - Add rake tasks for git repository maintainance (Zeger-Jan van de Weg) - Fix 500 error when update group member permission + - Fix: As an admin, cannot add oneself as a member to a group/project - Trim leading and trailing whitespace of milestone and issueable titles (Jose Corcuera) - Recognize issue/MR/snippet/commit links as references - Backport JIRA features from EE to CE @@ -174,7 +175,6 @@ v 8.2.2 - Fix Error 500 when viewing user's personal projects from admin page (Stan Hu) - Fix: Raw private snippets access workflow - Prevent "413 Request entity too large" errors when pushing large files with LFS - - Fix: As an admin, cannot add oneself as a member to a group/project - Fix invalid links within projects dashboard header - Make current user the first user in assignee dropdown in issues detail page (Stan Hu) - Fix: duplicate email notifications on issue comments From b76bfe35c76084f17a7c017192049b793f70b9b1 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 18 Jan 2016 14:13:21 +0100 Subject: [PATCH 19/43] Add limit change for 'data' column in 'events' when using MySQL --- db/migrate/limits_to_mysql.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/migrate/limits_to_mysql.rb b/db/migrate/limits_to_mysql.rb index 2b7afae6d7..14d7e84d85 100644 --- a/db/migrate/limits_to_mysql.rb +++ b/db/migrate/limits_to_mysql.rb @@ -6,5 +6,6 @@ class LimitsToMysql < ActiveRecord::Migration change_column :merge_request_diffs, :st_diffs, :text, limit: 2147483647 change_column :snippets, :content, :text, limit: 2147483647 change_column :notes, :st_diff, :text, limit: 2147483647 + change_column :events, :data, :text, limit: 2147483647 end end From d1c474ae6aa98e281cdc46da183a0c29a324ba31 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 18 Jan 2016 15:52:26 +0100 Subject: [PATCH 20/43] Add information about PostgreSQL<>MySQL 'text' type differences into to installation docs --- doc/install/database_mysql.md | 19 +++++++++++++++++++ doc/update/README.md | 1 + 2 files changed, 20 insertions(+) diff --git a/doc/install/database_mysql.md b/doc/install/database_mysql.md index 513ad69ec2..4a2c699129 100644 --- a/doc/install/database_mysql.md +++ b/doc/install/database_mysql.md @@ -52,3 +52,22 @@ We do not recommend using MySQL due to various issues. For example, case [(in)se mysql> \q # You are done installing the database and can go back to the rest of the installation. + +## MySQL strings limits + +After installation or upgrade is done, remember to run `add_limits_mysql` rake task: + +``` +bundle exec rake add_limits_mysql +``` + +The `text` type in MySQL have different size limit than `text` type in PostgreSQL. In MySQL `text` columns are limited +to ~65kB, where in PostgreSQL `text` columns are limited up to ~1GB! + +`add_limits_mysql` rake task converts some important `text` columns in GitLab database to `longtext` which can persist +up to 4GB characters (may be less if the value contains multibyte characters). + +Details can be found in [PostgreSQL manual][postgres-text-type] and in [MySQL manual][mysql-text-types]. + +[postgres-text-type]: http://www.postgresql.org/docs/9.1/static/datatype-character.html +[mysql-text-types]: http://dev.mysql.com/doc/refman/5.7/en/string-type-overview.html diff --git a/doc/update/README.md b/doc/update/README.md index 0472537eeb..46aec4885e 100644 --- a/doc/update/README.md +++ b/doc/update/README.md @@ -14,3 +14,4 @@ Depending on the installation method and your GitLab version, there are multiple ## 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 informations about configuring GitLab to work with MySQL database. From f386a4bd158964a70c6a68531d75cdc74bd32881 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 18 Jan 2016 15:54:16 +0100 Subject: [PATCH 21/43] Update CHANGELOG --- CHANGELOG | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG b/CHANGELOG index 18ccd56dec..7f1ce16938 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -64,6 +64,7 @@ v 8.4.0 (unreleased) - Remove leading comma on award emoji when the user is the first to award the emoji (Zeger-Jan van de Weg) - Add build artifacts browser - Improve UX in builds artifacts browser + - Add MySQL limit migration for `data` column in `events` table v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) From a7daf33bc5d33d496456adb017b55094b917e718 Mon Sep 17 00:00:00 2001 From: Tomasz Maczukin Date: Mon, 18 Jan 2016 19:10:16 +0100 Subject: [PATCH 22/43] Modify CHANGELOG entry [ci skip] --- CHANGELOG | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 7f1ce16938..d1b5bef6b3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -64,7 +64,7 @@ v 8.4.0 (unreleased) - Remove leading comma on award emoji when the user is the first to award the emoji (Zeger-Jan van de Weg) - Add build artifacts browser - Improve UX in builds artifacts browser - - Add MySQL limit migration for `data` column in `events` table + - Change type of `data` column in `events` table when using MySQL v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) From ea44742b47bcb9dec75ba73793b1235863f04728 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Mon, 18 Jan 2016 15:58:15 -0500 Subject: [PATCH 23/43] Fix positioning issues with browse files button Issue is that in Firefox clipboard is blocking the browse files from moving all the way to the right. Move clipboard up 1px into -1px to unblock browse files --- app/assets/stylesheets/pages/commits.scss | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/assets/stylesheets/pages/commits.scss b/app/assets/stylesheets/pages/commits.scss index 800df95cff..818fd03e2a 100644 --- a/app/assets/stylesheets/pages/commits.scss +++ b/app/assets/stylesheets/pages/commits.scss @@ -36,6 +36,10 @@ li.commit { line-height: 20px; margin-bottom: 2px; + .btn-clipboard { + margin-top: -1px; + } + .notes_count { float: right; margin-right: 10px; From a936a1021ffbea04e363d3890a0eb34b470dfef3 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 18 Jan 2016 19:39:31 -0200 Subject: [PATCH 24/43] Update devise to version 3.5.4 --- Gemfile | 2 +- Gemfile.lock | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Gemfile b/Gemfile index a9a8bed106..072f7a9fcc 100644 --- a/Gemfile +++ b/Gemfile @@ -18,7 +18,7 @@ gem "mysql2", '~> 0.3.16', group: :mysql gem "pg", '~> 0.18.2', group: :postgres # Authentication libraries -gem 'devise', '~> 3.5.3' +gem 'devise', '~> 3.5.4' gem 'devise-async', '~> 0.9.0' gem 'doorkeeper', '~> 2.2.0' gem 'omniauth', '~> 1.2.2' diff --git a/Gemfile.lock b/Gemfile.lock index f1bba7f437..a14fdbeed2 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -157,7 +157,7 @@ GEM activerecord (>= 3.2.0, < 5.0) descendants_tracker (0.0.4) thread_safe (~> 0.3, >= 0.3.1) - devise (3.5.3) + devise (3.5.4) bcrypt (~> 3.0) orm_adapter (~> 0.1) railties (>= 3.2.6, < 5) @@ -614,7 +614,7 @@ GEM thor (>= 0.18.1, < 2.0) rainbow (2.0.0) raindrops (0.15.0) - rake (10.4.2) + rake (10.5.0) raphael-rails (2.1.2) rb-fsevent (0.9.6) rb-inotify (0.9.5) @@ -648,8 +648,8 @@ GEM request_store (1.2.1) rerun (0.11.0) listen (~> 3.0) - responders (2.1.0) - railties (>= 4.2.0, < 5) + responders (2.1.1) + railties (>= 4.2.0, < 5.1) rest-client (1.8.0) http-cookie (>= 1.0.2, < 2.0) mime-types (>= 1.16, < 3.0) @@ -911,7 +911,7 @@ DEPENDENCIES d3_rails (~> 3.5.0) database_cleaner (~> 1.4.0) default_value_for (~> 3.0.0) - devise (~> 3.5.3) + devise (~> 3.5.4) devise-async (~> 0.9.0) devise-two-factor (~> 2.0.0) diffy (~> 3.0.3) From 402feec6ddd96880921ad37a931db7c96fd48be0 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 18 Jan 2016 16:53:33 -0500 Subject: [PATCH 25/43] Update MySQL docs [ci skip] --- CHANGELOG | 2 +- doc/install/database_mysql.md | 19 +++++++++++-------- doc/update/README.md | 2 +- 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index d1b5bef6b3..9e9d66fb7f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -64,7 +64,7 @@ v 8.4.0 (unreleased) - Remove leading comma on award emoji when the user is the first to award the emoji (Zeger-Jan van de Weg) - Add build artifacts browser - Improve UX in builds artifacts browser - - Change type of `data` column in `events` table when using MySQL + - Increase default size of `data` column in `events` table when using MySQL v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) diff --git a/doc/install/database_mysql.md b/doc/install/database_mysql.md index 4a2c699129..e51ff5a5de 100644 --- a/doc/install/database_mysql.md +++ b/doc/install/database_mysql.md @@ -8,7 +8,7 @@ We do not recommend using MySQL due to various issues. For example, case [(in)se # Install the database packages sudo apt-get install -y mysql-server mysql-client libmysqlclient-dev - + # Ensure you have MySQL version 5.5.14 or later mysql --version @@ -31,7 +31,7 @@ We do not recommend using MySQL due to various issues. For example, case [(in)se # Ensure you can use the InnoDB engine which is necessary to support long indexes # If this fails, check your MySQL config files (e.g. `/etc/mysql/*.cnf`, `/etc/mysql/conf.d/*`) for the setting "innodb = off" mysql> SET storage_engine=INNODB; - + # Create the GitLab production database mysql> CREATE DATABASE IF NOT EXISTS `gitlabhq_production` DEFAULT CHARACTER SET `utf8` COLLATE `utf8_unicode_ci`; @@ -55,19 +55,22 @@ We do not recommend using MySQL due to various issues. For example, case [(in)se ## MySQL strings limits -After installation or upgrade is done, remember to run `add_limits_mysql` rake task: +After installation or upgrade, remember to run the `add_limits_mysql` Rake task: ``` bundle exec rake add_limits_mysql ``` -The `text` type in MySQL have different size limit than `text` type in PostgreSQL. In MySQL `text` columns are limited -to ~65kB, where in PostgreSQL `text` columns are limited up to ~1GB! +The `text` type in MySQL has a different size limit than the `text` type in +PostgreSQL. In MySQL `text` columns are limited to ~65kB, whereas in PostgreSQL +`text` columns are limited up to ~1GB! -`add_limits_mysql` rake task converts some important `text` columns in GitLab database to `longtext` which can persist -up to 4GB characters (may be less if the value contains multibyte characters). +The `add_limits_mysql` Rake task converts some important `text` columns in the +GitLab database to `longtext` columns, which can persist values of up to 4GB +(sometimes less if the value contains multibyte characters). -Details can be found in [PostgreSQL manual][postgres-text-type] and in [MySQL manual][mysql-text-types]. +Details can be found in the [PostgreSQL][postgres-text-type] and +[MySQL][mysql-text-types] manuals. [postgres-text-type]: http://www.postgresql.org/docs/9.1/static/datatype-character.html [mysql-text-types]: http://dev.mysql.com/doc/refman/5.7/en/string-type-overview.html diff --git a/doc/update/README.md b/doc/update/README.md index 46aec4885e..109d5de3fa 100644 --- a/doc/update/README.md +++ b/doc/update/README.md @@ -14,4 +14,4 @@ Depending on the installation method and your GitLab version, there are multiple ## 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 informations about configuring GitLab to work with MySQL database. +- [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. From 1133251f0883f137ddd2154507fe5f83c5894117 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Mon, 18 Jan 2016 19:51:48 -0500 Subject: [PATCH 26/43] Reduce margin to fit dropdowns Fixes #10747 --- app/assets/stylesheets/framework/filters.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index 8e6922c923..b7638c86bf 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -1,5 +1,5 @@ .filter-item { - margin-right: 15px; + margin-right: 6px; } @media (min-width: 800px) { From 3c078cef41305d2c4126501f5de571143c19e147 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Wed, 13 Jan 2016 08:59:56 -0500 Subject: [PATCH 27/43] adds exclamation point when assigned user cannot merge --- app/assets/stylesheets/pages/projects.scss | 5 +++++ app/views/shared/issuable/_sidebar.html.haml | 3 +++ 2 files changed, 8 insertions(+) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index 3a87b71078..b14033720b 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -558,3 +558,8 @@ pre.light-well { width: 101%; } } + +.cannot-be-merged, .cannot-be-merged:hover { + color: #E62958; + margin-top: 2px; +} diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 9f4a7098ea..1eb14b5153 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -10,6 +10,9 @@ .value - if issuable.assignee %strong= link_to_member(@project, issuable.assignee, size: 24) + - if issuable.instance_of?(MergeRequest) && !@merge_request.can_be_merged_by?(issuable.assignee) + %a.pull-right.cannot-be-merged{href:'#', data:{toggle:'tooltip'}, title:'Not allowed to merge'} + = icon('exclamation-triangle') - else .light None From 8b3d6793454b2a955c88d9dcd65a130133996646 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Wed, 13 Jan 2016 23:31:30 -0500 Subject: [PATCH 28/43] color variable for error for exclamation point --- app/assets/stylesheets/framework/variables.scss | 2 ++ app/assets/stylesheets/pages/projects.scss | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 85ecdddda7..70239a74c8 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -26,6 +26,8 @@ $gl-vert-padding: 6px; $gl-padding-top:10px; $gl-avatar-size: 46px; $secondary-text: #7f8fa4; +$error-exclamation-point: #E62958; + /* * Color schema diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index b14033720b..b715819e30 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -560,6 +560,6 @@ pre.light-well { } .cannot-be-merged, .cannot-be-merged:hover { - color: #E62958; + color: $error-exclamation-point; margin-top: 2px; -} +} \ No newline at end of file From fbb27a4244994103ed94c4921fc47902a767b860 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Mon, 18 Jan 2016 20:17:29 -0500 Subject: [PATCH 29/43] Fix spacing issues, fix naming inconsistencies. Fix conflicts. Used `issuable` instead --- app/assets/stylesheets/pages/projects.scss | 4 ++-- app/views/shared/issuable/_sidebar.html.haml | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index b715819e30..b14033720b 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -560,6 +560,6 @@ pre.light-well { } .cannot-be-merged, .cannot-be-merged:hover { - color: $error-exclamation-point; + color: #E62958; margin-top: 2px; -} \ 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 1eb14b5153..3092ff5424 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -10,8 +10,8 @@ .value - if issuable.assignee %strong= link_to_member(@project, issuable.assignee, size: 24) - - if issuable.instance_of?(MergeRequest) && !@merge_request.can_be_merged_by?(issuable.assignee) - %a.pull-right.cannot-be-merged{href:'#', data:{toggle:'tooltip'}, title:'Not allowed to merge'} + - if issuable.instance_of?(MergeRequest) && !issuable.can_be_merged_by?(issuable.assignee) + %a.pull-right.cannot-be-merged{href: '#', data: {toggle: 'tooltip'}, title: 'Not allowed to merge'} = icon('exclamation-triangle') - else .light None From bddef293d666c4b3933b93b017ae49bdf18b234f Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 18 Jan 2016 22:43:04 -0200 Subject: [PATCH 30/43] Ensure that doesn't have pending migrations when running the specs --- .gitlab-ci.yml | 4 ++-- lib/gitlab/current_settings.rb | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index c23a7a3bf0..ac8390074f 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -8,7 +8,7 @@ before_script: - touch log/application.log - touch log/test.log - bundle install --without postgres production --jobs $(nproc) "${FLAGS[@]}" - - bundle exec rake db:reset db:create RAILS_ENV=test + - RAILS_ENV=test bundle exec rake db:drop db:create db:schema:load db:migrate spec:feature: script: @@ -118,7 +118,7 @@ flay: - mysql bundler:audit: - script: + script: - "bundle exec bundle-audit update" - "bundle exec bundle-audit check" tags: diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb index 7f938780ab..ea05425582 100644 --- a/lib/gitlab/current_settings.rb +++ b/lib/gitlab/current_settings.rb @@ -39,7 +39,6 @@ module Gitlab end use_db && ActiveRecord::Base.connection.active? && - !ActiveRecord::Migrator.needs_migration? && ActiveRecord::Base.connection.table_exists?('application_settings') rescue ActiveRecord::NoDatabaseError From 96440042a904274b88afc25eddd46c2722482427 Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Mon, 18 Jan 2016 22:44:15 -0200 Subject: [PATCH 31/43] Raise an error on page load if there are pending migrations on dev env --- config/environments/development.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/config/environments/development.rb b/config/environments/development.rb index 257c163720..689694a348 100644 --- a/config/environments/development.rb +++ b/config/environments/development.rb @@ -16,6 +16,9 @@ Rails.application.configure do # Print deprecation notices to the Rails logger config.active_support.deprecation = :log + # Raise an error on page load if there are pending migrations + config.active_record.migration_error = :page_load + # Only use best-standards-support built into browsers config.action_dispatch.best_standards_support = :builtin From 19608d3ee027efb27ecf83671a85487040287619 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Tue, 19 Jan 2016 12:00:38 +0100 Subject: [PATCH 32/43] Fix double request issue in artifacts browser --- app/views/projects/artifacts/browse.html.haml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/app/views/projects/artifacts/browse.html.haml b/app/views/projects/artifacts/browse.html.haml index 9dc7b2c2e1..b70c776a2b 100644 --- a/app/views/projects/artifacts/browse.html.haml +++ b/app/views/projects/artifacts/browse.html.haml @@ -23,6 +23,10 @@ .center Empty :javascript - $(document).on('click', 'tr[data-link]', function(e) { + $('.tree-holder').on('click', 'tr[data-link] a', function(e) { + e.stopImmediatePropagation(); + }); + + $('.tree-holder').on('click', 'tr[data-link]', function(e) { window.location = this.dataset.link; }); From b4ee6f57b9322401d1439eb21f9160ae2cb91d14 Mon Sep 17 00:00:00 2001 From: Yorick Peterse Date: Tue, 19 Jan 2016 13:48:07 +0100 Subject: [PATCH 33/43] Greatly improve external_issue_tracker performance This greatly improves the performance of Project#external_issue_tracker by moving most of the fields queried in Ruby to the database and letting the database handle all logic. Prior to this change the process of finding an external issue tracker was along the lines of the following: 1. Load all project services into memory. 2. Reduce the list to only services where "issue_tracker?" returns true 3. Reduce the list from step 2 to service where "default?" returns false 4. Find the first service where "activated?" returns true This has to two big problems: 1. Loading all services into memory only to reduce the list down to a single item later on is a waste of memory (and slow timing wise). 2. Calling Array#select followed by Array#reject followed by Array#find allocates extra objects when this really isn't needed. To work around this the following service fields have been moved to the database (instead of being hardcoded): * category * default This in turn means we can get the external issue tracker using the following query: SELECT * FROM services WHERE active IS TRUE AND default IS FALSE AND category = 'issue_tracker' AND project_id = XXX LIMIT 1 This coupled with memoizing the result (just as before this commit) greatly reduces the time it takes for Project#external_issue_tracker to complete. The exact reduction depends on one's environment, but locally the execution time is reduced from roughly 230 ms to only 2 ms (= a reduction of almost 180x). Fixes gitlab-org/gitlab-ce#10771 --- app/models/project.rb | 7 +--- app/models/project_services/ci_service.rb | 8 ++-- .../gitlab_issue_tracker_service.rb | 4 +- .../project_services/issue_tracker_service.rb | 6 +-- app/models/service.rb | 11 ++++-- .../20160119111158_add_services_category.rb | 39 +++++++++++++++++++ .../20160119112418_add_services_default.rb | 20 ++++++++++ db/schema.rb | 26 +++++++------ 8 files changed, 90 insertions(+), 31 deletions(-) create mode 100644 db/migrate/20160119111158_add_services_category.rb create mode 100644 db/migrate/20160119112418_add_services_default.rb diff --git a/app/models/project.rb b/app/models/project.rb index 7e13115151..cb668e0c2f 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -468,12 +468,9 @@ class Project < ActiveRecord::Base !external_issue_tracker end - def external_issues_trackers - services.select(&:issue_tracker?).reject(&:default?) - end - def external_issue_tracker - @external_issues_tracker ||= external_issues_trackers.find(&:activated?) + @external_issue_tracker ||= + services.issue_trackers.active.without_defaults.first end def can_have_issues_tracker_id? diff --git a/app/models/project_services/ci_service.rb b/app/models/project_services/ci_service.rb index c3f70d1f97..e10b5529b4 100644 --- a/app/models/project_services/ci_service.rb +++ b/app/models/project_services/ci_service.rb @@ -23,14 +23,12 @@ # List methods you need to implement to get your CI service # working with GitLab Merge Requests class CiService < Service - def category - :ci - end - + default_value_for :category, 'ci' + def valid_token?(token) self.respond_to?(:token) && self.token.present? && self.token == token end - + def supported_events %w(push) end diff --git a/app/models/project_services/gitlab_issue_tracker_service.rb b/app/models/project_services/gitlab_issue_tracker_service.rb index 7aa04309f5..05436cd0f7 100644 --- a/app/models/project_services/gitlab_issue_tracker_service.rb +++ b/app/models/project_services/gitlab_issue_tracker_service.rb @@ -24,9 +24,7 @@ class GitlabIssueTrackerService < IssueTrackerService prop_accessor :title, :description, :project_url, :issues_url, :new_issue_url - def default? - true - end + default_value_for :default, true def to_param 'gitlab' diff --git a/app/models/project_services/issue_tracker_service.rb b/app/models/project_services/issue_tracker_service.rb index ed201979d3..25045224ce 100644 --- a/app/models/project_services/issue_tracker_service.rb +++ b/app/models/project_services/issue_tracker_service.rb @@ -23,12 +23,10 @@ class IssueTrackerService < Service validates :project_url, :issues_url, :new_issue_url, presence: true, if: :activated? - def category - :issue_tracker - end + default_value_for :category, 'issue_tracker' def default? - false + default end def issue_url(iid) diff --git a/app/models/service.rb b/app/models/service.rb index 24f4bf7646..721273250e 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -43,6 +43,9 @@ class Service < ActiveRecord::Base validates :project_id, presence: true, unless: Proc.new { |service| service.template? } scope :visible, -> { where.not(type: ['GitlabIssueTrackerService', 'GitlabCiService']) } + scope :issue_trackers, -> { where(category: 'issue_tracker') } + scope :active, -> { where(active: true) } + scope :without_defaults, -> { where(default: false) } scope :push_hooks, -> { where(push_events: true, active: true) } scope :tag_push_hooks, -> { where(tag_push_events: true, active: true) } @@ -51,6 +54,8 @@ class Service < ActiveRecord::Base scope :note_hooks, -> { where(note_events: true, active: true) } scope :build_hooks, -> { where(build_events: true, active: true) } + default_value_for :category, 'common' + def activated? active end @@ -60,7 +65,7 @@ class Service < ActiveRecord::Base end def category - :common + read_attribute(:category).to_sym end def initialize_properties @@ -153,7 +158,7 @@ class Service < ActiveRecord::Base # Returns a hash of the properties that have been assigned a new value since last save, # indicating their original values (attr => original value). - # ActiveRecord does not provide a mechanism to track changes in serialized keys, + # ActiveRecord does not provide a mechanism to track changes in serialized keys, # so we need a specific implementation for service properties. # This allows to track changes to properties set with the accessor methods, # but not direct manipulation of properties hash. @@ -164,7 +169,7 @@ class Service < ActiveRecord::Base def reset_updated_properties @updated_properties = nil end - + def async_execute(data) return unless supported_events.include?(data[:object_kind]) diff --git a/db/migrate/20160119111158_add_services_category.rb b/db/migrate/20160119111158_add_services_category.rb new file mode 100644 index 0000000000..a9110a8418 --- /dev/null +++ b/db/migrate/20160119111158_add_services_category.rb @@ -0,0 +1,39 @@ +class AddServicesCategory < ActiveRecord::Migration + def up + add_column :services, :category, :string, default: 'common', null: false + + category = quote_column_name('category') + type = quote_column_name('type') + + execute <<-EOF +UPDATE services +SET #{category} = 'issue_tracker' +WHERE #{type} IN ( + 'CustomIssueTrackerService', + 'GitlabIssueTrackerService', + 'IssueTrackerService', + 'JiraService', + 'RedmineService' +); +EOF + + execute <<-EOF +UPDATE services +SET #{category} = 'ci' +WHERE #{type} IN ( + 'BambooService', + 'BuildkiteService', + 'CiService', + 'DroneCiService', + 'GitlabCiService', + 'TeamcityService' +); + EOF + + add_index :services, :category + end + + def down + remove_column :services, :category + end +end diff --git a/db/migrate/20160119112418_add_services_default.rb b/db/migrate/20160119112418_add_services_default.rb new file mode 100644 index 0000000000..69a42d7b87 --- /dev/null +++ b/db/migrate/20160119112418_add_services_default.rb @@ -0,0 +1,20 @@ +class AddServicesDefault < ActiveRecord::Migration + def up + add_column :services, :default, :boolean, default: false + + default = quote_column_name('default') + type = quote_column_name('type') + + execute <<-EOF +UPDATE services +SET #{default} = true +WHERE #{type} = 'GitlabIssueTrackerService' +EOF + + add_index :services, :default + end + + def down + remove_column :services, :default + end +end diff --git a/db/schema.rb b/db/schema.rb index 9045135dd9..a08181b910 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: 20160113111034) do +ActiveRecord::Schema.define(version: 20160119112418) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -725,20 +725,24 @@ ActiveRecord::Schema.define(version: 20160113111034) do t.string "type" t.string "title" t.integer "project_id" - t.datetime "created_at" - t.datetime "updated_at" - t.boolean "active", default: false, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.boolean "active", null: false t.text "properties" - t.boolean "template", default: false - t.boolean "push_events", default: true - t.boolean "issues_events", default: true - t.boolean "merge_requests_events", default: true - t.boolean "tag_push_events", default: true - t.boolean "note_events", default: true, null: false - t.boolean "build_events", default: false, null: false + t.boolean "template", default: false + t.boolean "push_events", default: true + t.boolean "issues_events", default: true + t.boolean "merge_requests_events", default: true + t.boolean "tag_push_events", default: true + t.boolean "note_events", default: true, null: false + t.boolean "build_events", default: false, null: false + t.string "category", default: "common", null: false + t.boolean "default", default: false end + add_index "services", ["category"], name: "index_services_on_category", using: :btree add_index "services", ["created_at", "id"], name: "index_services_on_created_at_and_id", using: :btree + add_index "services", ["default"], name: "index_services_on_default", using: :btree add_index "services", ["project_id"], name: "index_services_on_project_id", using: :btree add_index "services", ["template"], name: "index_services_on_template", using: :btree From f5562355f4580ea049d33991aa0eaf76f46b17f7 Mon Sep 17 00:00:00 2001 From: Jacob Schatz Date: Tue, 19 Jan 2016 08:44:16 -0500 Subject: [PATCH 34/43] Move multiple css rules to multiple lines. Removes extra line breaks. --- app/assets/stylesheets/framework/variables.scss | 1 - app/assets/stylesheets/pages/projects.scss | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/stylesheets/framework/variables.scss b/app/assets/stylesheets/framework/variables.scss index 70239a74c8..3ec48da9a4 100644 --- a/app/assets/stylesheets/framework/variables.scss +++ b/app/assets/stylesheets/framework/variables.scss @@ -28,7 +28,6 @@ $gl-avatar-size: 46px; $secondary-text: #7f8fa4; $error-exclamation-point: #E62958; - /* * Color schema */ diff --git a/app/assets/stylesheets/pages/projects.scss b/app/assets/stylesheets/pages/projects.scss index b14033720b..003a4c22f2 100644 --- a/app/assets/stylesheets/pages/projects.scss +++ b/app/assets/stylesheets/pages/projects.scss @@ -559,7 +559,8 @@ pre.light-well { } } -.cannot-be-merged, .cannot-be-merged:hover { +.cannot-be-merged, +.cannot-be-merged:hover { color: #E62958; margin-top: 2px; } From 10669097fa7621a175755d144c61ecb80a5ea9fb Mon Sep 17 00:00:00 2001 From: Josh Frye Date: Wed, 13 Jan 2016 09:07:21 -0500 Subject: [PATCH 35/43] Add public params to GET /projects api. Closes #3788 --- CHANGELOG | 1 + doc/api/projects.md | 1 + lib/api/helpers.rb | 4 ++++ 3 files changed, 6 insertions(+) diff --git a/CHANGELOG b/CHANGELOG index 207ddf1dd6..5bccb80a64 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,6 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.5.0 (unreleased) + - Add "public" flag to GET /projects api endpoint v 8.4.0 (unreleased) - Ensure Gravatar host looks like an actual host diff --git a/doc/api/projects.md b/doc/api/projects.md index 241229221d..29ed327d98 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -29,6 +29,7 @@ GET /projects Parameters: - `archived` (optional) - if passed, limit by archived status +- `public` (optional) - if passed, limit by public visibility - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 6d2380cf47..68d0aa671b 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -264,6 +264,10 @@ module API projects = projects.search(params[:search]) end + if params[:public].present? && parse_boolean(params[:public]) + projects = projects.public_only + end + projects.reorder(project_order_by => project_sort) end From 6951396dbeddc72821c3beddf5aa13d24eeb0712 Mon Sep 17 00:00:00 2001 From: Josh Frye Date: Wed, 13 Jan 2016 09:53:41 -0500 Subject: [PATCH 36/43] Update all docs for public param --- doc/api/groups.md | 1 + doc/api/projects.md | 3 +++ 2 files changed, 4 insertions(+) diff --git a/doc/api/groups.md b/doc/api/groups.md index 808675d860..c6f1975bda 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -33,6 +33,7 @@ GET /groups/:id/projects Parameters: - `archived` (optional) - if passed, limit by archived status +- `public` (optional) - if passed, limit by public visibility - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria diff --git a/doc/api/projects.md b/doc/api/projects.md index 29ed327d98..8b9dec46f3 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -153,6 +153,7 @@ GET /projects/owned Parameters: - `archived` (optional) - if passed, limit by archived status +- `public` (optional) - if passed, limit by public visibility - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria @@ -168,6 +169,7 @@ GET /projects/starred Parameters: - `archived` (optional) - if passed, limit by archived status +- `public` (optional) - if passed, limit by public visibility - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria @@ -183,6 +185,7 @@ GET /projects/all Parameters: - `archived` (optional) - if passed, limit by archived status +- `public` (optional) - if passed, limit by public visibility - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria From 11797df1af483156b8cf11290c49c3f4d6089d99 Mon Sep 17 00:00:00 2001 From: Josh Frye Date: Mon, 18 Jan 2016 12:41:49 -0500 Subject: [PATCH 37/43] Change 'public' flag to more robust 'visibility' query. --- CHANGELOG | 2 +- app/models/project.rb | 4 ++++ doc/api/groups.md | 2 +- doc/api/projects.md | 8 ++++---- lib/api/helpers.rb | 4 ++-- 5 files changed, 12 insertions(+), 8 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 5bccb80a64..965b88fc11 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,7 +1,7 @@ Please view this file on the master branch, on stable branches it's out of date. v 8.5.0 (unreleased) - - Add "public" flag to GET /projects api endpoint + - Add "visibility" flag to GET /projects api endpoint v 8.4.0 (unreleased) - Ensure Gravatar host looks like an actual host diff --git a/app/models/project.rb b/app/models/project.rb index 7e13115151..56db0ce6cf 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -272,6 +272,10 @@ class Project < ActiveRecord::Base query: "%#{query.try(:downcase)}%") end + def search_by_visibility(level) + where(visibility_level: visibility_levels[level.capitalize]) + end + def search_by_title(query) where('projects.archived = ?', false).where('LOWER(projects.name) LIKE :query', query: "%#{query.downcase}%") end diff --git a/doc/api/groups.md b/doc/api/groups.md index c6f1975bda..d47e79ba47 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -33,7 +33,7 @@ GET /groups/:id/projects Parameters: - `archived` (optional) - if passed, limit by archived status -- `public` (optional) - if passed, limit by public visibility +- `visibility` (optional) - if passed, limit by visibility `public`, `internal`, `private` - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria diff --git a/doc/api/projects.md b/doc/api/projects.md index 8b9dec46f3..3f372c955d 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -29,7 +29,7 @@ GET /projects Parameters: - `archived` (optional) - if passed, limit by archived status -- `public` (optional) - if passed, limit by public visibility +- `visibility` (optional) - if passed, limit by visibility `public`, `internal`, `private` - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria @@ -153,7 +153,7 @@ GET /projects/owned Parameters: - `archived` (optional) - if passed, limit by archived status -- `public` (optional) - if passed, limit by public visibility +- `visibility` (optional) - if passed, limit by visibility `public`, `internal`, `private` - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria @@ -169,7 +169,7 @@ GET /projects/starred Parameters: - `archived` (optional) - if passed, limit by archived status -- `public` (optional) - if passed, limit by public visibility +- `visibility` (optional) - if passed, limit by visibility `public`, `internal`, `private` - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria @@ -185,7 +185,7 @@ GET /projects/all Parameters: - `archived` (optional) - if passed, limit by archived status -- `public` (optional) - if passed, limit by public visibility +- `visibility` (optional) - if passed, limit by visibility `public`, `internal`, `private` - `order_by` (optional) - Return requests ordered by `id`, `name`, `path`, `created_at`, `updated_at` or `last_activity_at` fields. Default is `created_at` - `sort` (optional) - Return requests sorted in `asc` or `desc` order. Default is `desc` - `search` (optional) - Return list of authorized projects according to a search criteria diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 68d0aa671b..3f528b9f7c 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -264,8 +264,8 @@ module API projects = projects.search(params[:search]) end - if params[:public].present? && parse_boolean(params[:public]) - projects = projects.public_only + if params[:visibility].present? + projects = projects.search_by_visibility(params[:visibility]) end projects.reorder(project_order_by => project_sort) From ae6080c791ee8b6aedd7091e2a321f5fe41f0978 Mon Sep 17 00:00:00 2001 From: Josh Frye Date: Tue, 19 Jan 2016 09:53:40 -0500 Subject: [PATCH 38/43] Add specs. Adjust filter. --- app/models/project.rb | 2 +- spec/requests/api/projects_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/app/models/project.rb b/app/models/project.rb index 56db0ce6cf..60729d9318 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -273,7 +273,7 @@ class Project < ActiveRecord::Base end def search_by_visibility(level) - where(visibility_level: visibility_levels[level.capitalize]) + where(visibility_level: Gitlab::VisibilityLevel.const_get(level.upcase)) end def search_by_title(query) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 6f4c336b66..2a310f3834 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -90,6 +90,29 @@ describe API::API, api: true do end end + context 'and using the visibility filter' do + it 'should filter based on private visibility param' do + get api('/projects', user), { visibility: 'private' } + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(user.namespace.projects.where(visibility_level: Gitlab::VisibilityLevel::PRIVATE).count) + end + + it 'should filter based on internal visibility param' do + get api('/projects', user), { visibility: 'internal' } + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(user.namespace.projects.where(visibility_level: Gitlab::VisibilityLevel::INTERNAL).count) + end + + it 'should filter based on public visibility param' do + get api('/projects', user), { visibility: 'public' } + expect(response.status).to eq(200) + expect(json_response).to be_an Array + expect(json_response.length).to eq(user.namespace.projects.where(visibility_level: Gitlab::VisibilityLevel::PUBLIC).count) + end + end + context 'and using sorting' do before do project2 From 2ece1b774aac36f4a92801c665355e7fe1c4d48f Mon Sep 17 00:00:00 2001 From: Douglas Barbosa Alexandre Date: Tue, 19 Jan 2016 13:24:42 -0200 Subject: [PATCH 39/43] Fix creator should be added as a master of the project on creation --- CHANGELOG | 1 + app/services/projects/create_service.rb | 2 +- spec/services/projects/create_service_spec.rb | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG b/CHANGELOG index 207ddf1dd6..adda5373c3 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -67,6 +67,7 @@ v 8.4.0 (unreleased) - Improve UX in builds artifacts browser - Increase default size of `data` column in `events` table when using MySQL - Expose button to CI Lint tool on project builds page + - Fix: Creator should be added as a master of the project on creation v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index a6820183be..c94d7ab710 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -95,7 +95,7 @@ module Projects system_hook_service.execute_hooks_for(@project, :create) unless @project.group - @project.team << [current_user, :master, current_user] + @project.team << [current_user, :master] end @project.import_start if @project.import? diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 5d0b18558b..e43903dbd3 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -32,6 +32,7 @@ describe Projects::CreateService, services: true do it { expect(@project).to be_valid } it { expect(@project.owner).to eq(@user) } + it { expect(@project.team.masters).to include(@user) } it { expect(@project.namespace).to eq(@user.namespace) } end From 98e1a5b63424b6912de98ee5055d3f9e57e63899 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Tue, 19 Jan 2016 16:25:38 +0100 Subject: [PATCH 40/43] Allow LDAP users to change their email if it was not set by the LDAP server --- CHANGELOG | 1 + app/models/user.rb | 5 +++- app/views/profiles/show.html.haml | 4 +-- .../20160119145451_add_ldap_email_to_users.rb | 30 +++++++++++++++++++ db/schema.rb | 3 +- lib/gitlab/ldap/user.rb | 29 ++++++++++-------- lib/gitlab/o_auth/auth_hash.rb | 8 +++-- lib/gitlab/o_auth/user.rb | 14 ++++----- spec/lib/gitlab/ldap/user_spec.rb | 28 ++++++++++++++++- 9 files changed, 95 insertions(+), 27 deletions(-) create mode 100644 db/migrate/20160119145451_add_ldap_email_to_users.rb diff --git a/CHANGELOG b/CHANGELOG index 207ddf1dd6..dce909089f 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.5.0 (unreleased) v 8.4.0 (unreleased) + - Allow LDAP users to change their email if it was not set by the LDAP server - Ensure Gravatar host looks like an actual host - Consider re-assign as a mention from a notification point of view - Add pagination headers to already paginated API resources diff --git a/app/models/user.rb b/app/models/user.rb index 592468933e..4214f01f6a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -664,7 +664,10 @@ class User < ActiveRecord::Base end def all_emails - [self.email, *self.emails.map(&:email)] + all_emails = [] + all_emails << self.email unless self.temp_oauth_email? + all_emails.concat(self.emails.map(&:email)) + all_emails end def hook_attrs diff --git a/app/views/profiles/show.html.haml b/app/views/profiles/show.html.haml index 9459d8a629..add9a00138 100644 --- a/app/views/profiles/show.html.haml +++ b/app/views/profiles/show.html.haml @@ -21,10 +21,10 @@ .form-group = f.label :email, class: "control-label" .col-sm-10 - - if @user.ldap_user? + - if @user.ldap_user? && @user.ldap_email? = f.text_field :email, class: "form-control", required: true, readonly: true %span.help-block.light - Email is read-only for LDAP user + Your email address was automatically set based on the LDAP server. - else - if @user.temp_oauth_email? = f.text_field :email, class: "form-control", required: true, value: nil diff --git a/db/migrate/20160119145451_add_ldap_email_to_users.rb b/db/migrate/20160119145451_add_ldap_email_to_users.rb new file mode 100644 index 0000000000..654d31ab15 --- /dev/null +++ b/db/migrate/20160119145451_add_ldap_email_to_users.rb @@ -0,0 +1,30 @@ +class AddLdapEmailToUsers < ActiveRecord::Migration + def up + add_column :users, :ldap_email, :boolean, default: false, null: false + + if Gitlab::Database.mysql? + execute %{ + UPDATE users, identities + SET users.ldap_email = TRUE + WHERE identities.user_id = users.id + AND users.email LIKE 'temp-email-for-oauth%' + AND identities.provider LIKE 'ldap%' + AND identities.extern_uid IS NOT NULL + } + else + execute %{ + UPDATE users + SET ldap_email = TRUE + FROM identities + WHERE identities.user_id = users.id + AND users.email LIKE 'temp-email-for-oauth%' + AND identities.provider LIKE 'ldap%' + AND identities.extern_uid IS NOT NULL + } + end + end + + def down + remove_column :users, :ldap_email + end +end diff --git a/db/schema.rb b/db/schema.rb index 9045135dd9..4681190402 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: 20160113111034) do +ActiveRecord::Schema.define(version: 20160119145451) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -850,6 +850,7 @@ ActiveRecord::Schema.define(version: 20160113111034) do t.boolean "hide_project_limit", default: false t.string "unlock_token" t.datetime "otp_grace_period_started_at" + t.boolean "ldap_email", default: false, null: false end add_index "users", ["admin"], name: "index_users_on_admin", using: :btree diff --git a/lib/gitlab/ldap/user.rb b/lib/gitlab/ldap/user.rb index aef08c97d1..e044f0ecc6 100644 --- a/lib/gitlab/ldap/user.rb +++ b/lib/gitlab/ldap/user.rb @@ -30,28 +30,31 @@ module Gitlab end def find_by_uid_and_provider - self.class.find_by_uid_and_provider( - auth_hash.uid, auth_hash.provider) + self.class.find_by_uid_and_provider(auth_hash.uid, auth_hash.provider) end def find_by_email - ::User.find_by(email: auth_hash.email.downcase) + ::User.find_by(email: auth_hash.email.downcase) if auth_hash.has_email? end def update_user_attributes - return unless persisted? + if persisted? + if auth_hash.has_email? + gl_user.skip_reconfirmation! + gl_user.email = auth_hash.email + end - gl_user.skip_reconfirmation! - gl_user.email = auth_hash.email + # find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved. + identity = gl_user.identities.find { |identity| identity.provider == auth_hash.provider } + identity ||= gl_user.identities.build(provider: auth_hash.provider) - # find_or_initialize_by doesn't update `gl_user.identities`, and isn't autosaved. - identity = gl_user.identities.find { |identity| identity.provider == auth_hash.provider } - identity ||= gl_user.identities.build(provider: auth_hash.provider) + # For a new identity set extern_uid to the LDAP DN + # For an existing identity with matching email but changed DN, update the DN. + # For an existing identity with no change in DN, this line changes nothing. + identity.extern_uid = auth_hash.uid + end - # For a new user set extern_uid to the LDAP DN - # For an existing user with matching email but changed DN, update the DN. - # For an existing user with no change in DN, this line changes nothing. - identity.extern_uid = auth_hash.uid + gl_user.ldap_email = auth_hash.has_email? gl_user end diff --git a/lib/gitlab/o_auth/auth_hash.rb b/lib/gitlab/o_auth/auth_hash.rb index ba31599432..36e5c2670b 100644 --- a/lib/gitlab/o_auth/auth_hash.rb +++ b/lib/gitlab/o_auth/auth_hash.rb @@ -32,6 +32,10 @@ module Gitlab @password ||= Gitlab::Utils.force_utf8(Devise.friendly_token[0, 8].downcase) end + def has_email? + get_info(:email).present? + end + private def info @@ -46,8 +50,8 @@ module Gitlab def username_and_email @username_and_email ||= begin - username = get_info(:username) || get_info(:nickname) - email = get_info(:email) + username = get_info(:username).presence || get_info(:nickname).presence + email = get_info(:email).presence username ||= generate_username(email) if email email ||= generate_temporarily_email(username) if username diff --git a/lib/gitlab/o_auth/user.rb b/lib/gitlab/o_auth/user.rb index e3d2cc65a8..d87a72f7ba 100644 --- a/lib/gitlab/o_auth/user.rb +++ b/lib/gitlab/o_auth/user.rb @@ -111,7 +111,7 @@ module Gitlab def block_after_signup? if creating_linked_ldap_user? ldap_config.block_auto_created_users - else + else Gitlab.config.omniauth.block_auto_created_users end end @@ -135,16 +135,16 @@ module Gitlab def user_attributes # Give preference to LDAP for sensitive information when creating a linked account if creating_linked_ldap_user? - username = ldap_person.username - email = ldap_person.email.first - else - username = auth_hash.username - email = auth_hash.email + username = ldap_person.username.presence + email = ldap_person.email.first.presence end + username ||= auth_hash.username + email ||= auth_hash.email + name = auth_hash.name name = ::Namespace.clean_path(username) if name.strip.empty? - + { name: name, username: ::Namespace.clean_path(username), diff --git a/spec/lib/gitlab/ldap/user_spec.rb b/spec/lib/gitlab/ldap/user_spec.rb index 1e755259da..03199a2523 100644 --- a/spec/lib/gitlab/ldap/user_spec.rb +++ b/spec/lib/gitlab/ldap/user_spec.rb @@ -37,7 +37,7 @@ describe Gitlab::LDAP::User, lib: true do end it "dont marks existing ldap user as changed" do - create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain') + create(:omniauth_user, email: 'john@example.com', extern_uid: 'my-uid', provider: 'ldapmain', ldap_email: true) expect(ldap_user.changed?).to be_falsey end end @@ -110,6 +110,32 @@ describe Gitlab::LDAP::User, lib: true do end end + describe 'updating email' do + context "when LDAP sets an email" do + it "has a real email" do + expect(ldap_user.gl_user.email).to eq(info[:email]) + end + + it "has ldap_email set to true" do + expect(ldap_user.gl_user.ldap_email?).to be(true) + end + end + + context "when LDAP doesn't set an email" do + before do + info.delete(:email) + end + + it "has a temp email" do + expect(ldap_user.gl_user.temp_oauth_email?).to be(true) + end + + it "has ldap_email set to false" do + expect(ldap_user.gl_user.ldap_email?).to be(false) + end + end + end + describe 'blocking' do def configure_block(value) allow_any_instance_of(Gitlab::LDAP::Config). From 2821306a740d62153b1a9192cb912b04b42b9279 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 19 Jan 2016 16:48:43 +0000 Subject: [PATCH 41/43] when starring a project it correctly increase the star count This is instead of increasing star & fork count Fixed #11396 --- app/assets/javascripts/star.js.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/star.js.coffee b/app/assets/javascripts/star.js.coffee index d849b2e795..f27780dda9 100644 --- a/app/assets/javascripts/star.js.coffee +++ b/app/assets/javascripts/star.js.coffee @@ -6,7 +6,7 @@ class @Star $starIcon = $this.find('i') toggleStar = (isStarred) -> - $this.parent().find('span.count').text data.star_count + $this.parent().find('.star-count').text data.star_count if isStarred $starSpan.removeClass('starred').text 'Star' $starIcon.removeClass('fa-star').addClass 'fa-star-o' @@ -19,4 +19,4 @@ class @Star return ).on 'ajax:error', (e, xhr, status, error) -> new Flash('Star toggle failed. Try again later.', 'alert') - return \ No newline at end of file + return From 84124380e815e549f50d82051b2b546c12d9a724 Mon Sep 17 00:00:00 2001 From: Anton Baklanov Date: Sat, 19 Dec 2015 21:04:40 +0200 Subject: [PATCH 42/43] Added X-GitLab-... headers to emails from CI and Email On Push services Fixes #2098 --- CHANGELOG | 1 + app/mailers/emails/builds.rb | 13 +++++ app/mailers/emails/projects.rb | 6 ++- app/mailers/notify.rb | 15 +++--- lib/gitlab/email/message/repository_push.rb | 1 + spec/mailers/notify_spec.rb | 57 ++++++++++++++++++++- 6 files changed, 84 insertions(+), 9 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index adda5373c3..48f820d858 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -68,6 +68,7 @@ v 8.4.0 (unreleased) - Increase default size of `data` column in `events` table when using MySQL - Expose button to CI Lint tool on project builds page - Fix: Creator should be added as a master of the project on creation + - Added X-GitLab-... headers to emails from CI and Email On Push services v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug) diff --git a/app/mailers/emails/builds.rb b/app/mailers/emails/builds.rb index d58609a2de..64c1ce8cfa 100644 --- a/app/mailers/emails/builds.rb +++ b/app/mailers/emails/builds.rb @@ -3,13 +3,26 @@ module Emails def build_fail_email(build_id, to) @build = Ci::Build.find(build_id) @project = @build.project + add_project_headers + add_build_headers + headers['X-GitLab-Build-Status'] = "failed" mail(to: to, subject: subject("Build failed for #{@project.name}", @build.short_sha)) end def build_success_email(build_id, to) @build = Ci::Build.find(build_id) @project = @build.project + add_project_headers + add_build_headers + headers['X-GitLab-Build-Status'] = "success" mail(to: to, subject: subject("Build success for #{@project.name}", @build.short_sha)) end + + private + def add_build_headers + headers['X-GitLab-Build-Id'] = @build.id + headers['X-GitLab-Build-Ref'] = @build.ref + end + end end diff --git a/app/mailers/emails/projects.rb b/app/mailers/emails/projects.rb index b96418679b..377c2999d6 100644 --- a/app/mailers/emails/projects.rb +++ b/app/mailers/emails/projects.rb @@ -43,7 +43,7 @@ module Emails @current_user = @created_by = User.find(created_by_id) @access_level = access_level @invite_email = invite_email - + @target_url = namespace_project_url(@project.namespace, @project) mail(to: @created_by.notification_email, @@ -65,6 +65,10 @@ module Emails # used in notify layout @target_url = @message.target_url + @project = Project.find project_id + + add_project_headers + headers['X-GitLab-Author'] = @message.author_username mail(from: sender(@message.author_id, @message.send_from_committer_email?), reply_to: @message.reply_to, diff --git a/app/mailers/notify.rb b/app/mailers/notify.rb index e1cd075a97..8cbc9eefc7 100644 --- a/app/mailers/notify.rb +++ b/app/mailers/notify.rb @@ -100,12 +100,7 @@ class Notify < BaseMailer end def mail_thread(model, headers = {}) - if @project - headers['X-GitLab-Project'] = @project.name - headers['X-GitLab-Project-Id'] = @project.id - headers['X-GitLab-Project-Path'] = @project.path_with_namespace - end - + add_project_headers headers["X-GitLab-#{model.class.name}-ID"] = model.id headers['X-GitLab-Reply-Key'] = reply_key @@ -152,4 +147,12 @@ class Notify < BaseMailer def reply_key @reply_key ||= SentNotification.reply_key end + + def add_project_headers + return unless @project + + headers['X-GitLab-Project'] = @project.name + headers['X-GitLab-Project-Id'] = @project.id + headers['X-GitLab-Project-Path'] = @project.path_with_namespace + end end diff --git a/lib/gitlab/email/message/repository_push.rb b/lib/gitlab/email/message/repository_push.rb index a2eb7a70bd..a05ffeb9cd 100644 --- a/lib/gitlab/email/message/repository_push.rb +++ b/lib/gitlab/email/message/repository_push.rb @@ -9,6 +9,7 @@ module Gitlab delegate :namespace, :name_with_namespace, to: :project, prefix: :project delegate :name, to: :author, prefix: :author + delegate :username, to: :author, prefix: :author def initialize(notify, project_id, recipient, opts = {}) raise ArgumentError, 'Missing options: author_id, ref, action' unless diff --git a/spec/mailers/notify_spec.rb b/spec/mailers/notify_spec.rb index 8f86c491d3..7289e596ef 100644 --- a/spec/mailers/notify_spec.rb +++ b/spec/mailers/notify_spec.rb @@ -40,14 +40,38 @@ describe Notify do end end + shared_examples 'an email with X-GitLab headers containing project details' do + it 'has X-GitLab-Project* headers' do + is_expected.to have_header 'X-GitLab-Project', /#{project.name}/ + is_expected.to have_header 'X-GitLab-Project-Id', /#{project.id}/ + is_expected.to have_header 'X-GitLab-Project-Path', /#{project.path_with_namespace}/ + end + end + + shared_examples 'an email with X-GitLab headers containing build details' do + it 'has X-GitLab-Build* headers' do + is_expected.to have_header 'X-GitLab-Build-Id', /#{build.id}/ + is_expected.to have_header 'X-GitLab-Build-Ref', /#{build.ref}/ + end + end + + shared_examples 'an email that contains a header with author username' do + it 'has X-GitLab-Author header containing author\'s username' do + is_expected.to have_header 'X-GitLab-Author', user.username + end + end + shared_examples 'an email starting a new thread' do |message_id_prefix| + include_examples 'an email with X-GitLab headers containing project details' + it 'has a discussion identifier' do is_expected.to have_header 'Message-ID', /<#{message_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/ - is_expected.to have_header 'X-GitLab-Project', /#{project.name}/ end end shared_examples 'an answer to an existing thread' do |thread_id_prefix| + include_examples 'an email with X-GitLab headers containing project details' + it 'has a subject that begins with Re: ' do is_expected.to have_subject /^Re: / end @@ -56,7 +80,6 @@ describe Notify do is_expected.to have_header 'Message-ID', /<(.*)@#{Gitlab.config.gitlab.host}>/ is_expected.to have_header 'References', /<#{thread_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/ is_expected.to have_header 'In-Reply-To', /<#{thread_id_prefix}(.*)@#{Gitlab.config.gitlab.host}>/ - is_expected.to have_header 'X-GitLab-Project', /#{project.name}/ end end @@ -656,6 +679,8 @@ describe Notify do it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'an email with X-GitLab headers containing project details' + it_behaves_like 'an email that contains a header with author username' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -685,6 +710,8 @@ describe Notify do it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'an email with X-GitLab headers containing project details' + it_behaves_like 'an email that contains a header with author username' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -713,6 +740,8 @@ describe Notify do it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'an email with X-GitLab headers containing project details' + it_behaves_like 'an email that contains a header with author username' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -737,6 +766,8 @@ describe Notify do it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'an email with X-GitLab headers containing project details' + it_behaves_like 'an email that contains a header with author username' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -765,6 +796,8 @@ describe Notify do it_behaves_like 'it should not have Gmail Actions links' it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'an email with X-GitLab headers containing project details' + it_behaves_like 'an email that contains a header with author username' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -871,6 +904,8 @@ describe Notify do it_behaves_like 'it should show Gmail Actions View Commit link' it_behaves_like "a user cannot unsubscribe through footer link" + it_behaves_like 'an email with X-GitLab headers containing project details' + it_behaves_like 'an email that contains a header with author username' it 'is sent as the author' do sender = subject.header[:from].addrs[0] @@ -904,6 +939,15 @@ describe Notify do subject { Notify.build_success_email(build.id, 'wow@example.com') } + it_behaves_like 'an email with X-GitLab headers containing build details' + it_behaves_like 'an email with X-GitLab headers containing project details' do + let(:project) { build.project } + end + + it 'has header indicating build status' do + is_expected.to have_header 'X-GitLab-Build-Status', 'success' + end + it 'has the correct subject' do should have_subject /Build success for/ end @@ -918,6 +962,15 @@ describe Notify do subject { Notify.build_fail_email(build.id, 'wow@example.com') } + it_behaves_like 'an email with X-GitLab headers containing build details' + it_behaves_like 'an email with X-GitLab headers containing project details' do + let(:project) { build.project } + end + + it 'has header indicating build status' do + is_expected.to have_header 'X-GitLab-Build-Status', 'failed' + end + it 'has the correct subject' do should have_subject /Build failed for/ end From a382ad99efd4c792f70705d386b2be688b667f24 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Tue, 19 Jan 2016 16:45:11 -0500 Subject: [PATCH 43/43] Update CHANGELOG - Add attribution for Anton Baklanov - Remove attributions for employees [ci skip] --- CHANGELOG | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index ca759a263b..2e0eee52a5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,9 +10,9 @@ v 8.4.0 (unreleased) - Add pagination headers to already paginated API resources - Properly generate diff of orphan commits, like the first commit in a repository - Improve the consistency of commit titles, branch names, tag names, issue/MR titles, on their respective project pages - - Autocomplete data is now always loaded, instead of when focusing a comment text area (Yorick Peterse) - - Improved performance of finding issues for an entire group (Yorick Peterse) - - Added custom application performance measuring system powered by InfluxDB (Yorick Peterse) + - Autocomplete data is now always loaded, instead of when focusing a comment text area + - Improved performance of finding issues for an entire group + - Added custom application performance measuring system powered by InfluxDB - Bump fog to 1.36.0 (Stan Hu) - Add user's last used IP addresses to admin page (Stan Hu) - Add housekeeping function to project settings page @@ -63,14 +63,14 @@ v 8.4.0 (unreleased) - Autosize Markdown textareas - Import GitHub wiki into GitLab - Add reporters ability to download and browse build artifacts (Andrew Johnson) - - Autofill referring url in message box when reporting user abuse. (Josh Frye) + - Autofill referring url in message box when reporting user abuse. - Remove leading comma on award emoji when the user is the first to award the emoji (Zeger-Jan van de Weg) - Add build artifacts browser - Improve UX in builds artifacts browser - Increase default size of `data` column in `events` table when using MySQL - Expose button to CI Lint tool on project builds page - Fix: Creator should be added as a master of the project on creation - - Added X-GitLab-... headers to emails from CI and Email On Push services + - Added X-GitLab-... headers to emails from CI and Email On Push services (Anton Baklanov) v 8.3.4 - Use gitlab-workhorse 0.5.4 (fixes API routing bug)