diff options
47 files changed, 330 insertions, 171 deletions
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f27d809af3c..0520f3b1c37 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -191,6 +191,9 @@ review-docs-deploy: stage: build environment: name: review-docs/$CI_COMMIT_REF_NAME + # DOCS_REVIEW_APPS_DOMAIN and DOCS_GITLAB_REPO_SUFFIX are secret variables + # Discussion: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/14236/diffs#note_40140693 + url: http://$CI_COMMIT_REF_SLUG-built-from-ce-ee.$DOCS_REVIEW_APPS_DOMAIN/$DOCS_GITLAB_REPO_SUFFIX on_stop: review-docs-cleanup script: - gem install gitlab --no-doc diff --git a/app/assets/javascripts/labels_select.js b/app/assets/javascripts/labels_select.js index 7d7f91227f9..2538d9c2093 100644 --- a/app/assets/javascripts/labels_select.js +++ b/app/assets/javascripts/labels_select.js @@ -127,13 +127,6 @@ import DropdownUtils from './filtered_search/dropdown_utils'; $('.has-tooltip', $value).tooltip({ container: 'body' }); - return $value.find('a').each(function(i) { - return setTimeout((function(_this) { - return function() { - return gl.animate.animate($(_this), 'pulse'); - }; - })(this), 200 * i); - }); }); }; $dropdown.glDropdown({ diff --git a/app/assets/javascripts/lib/utils/animate.js b/app/assets/javascripts/lib/utils/animate.js deleted file mode 100644 index d93c1d0da59..00000000000 --- a/app/assets/javascripts/lib/utils/animate.js +++ /dev/null @@ -1,49 +0,0 @@ -/* eslint-disable func-names, space-before-function-paren, wrap-iife, no-param-reassign, no-void, prefer-template, no-var, new-cap, prefer-arrow-callback, consistent-return, max-len */ -(function() { - (function(w) { - if (w.gl == null) { - w.gl = {}; - } - if (gl.animate == null) { - gl.animate = {}; - } - gl.animate.animate = function($el, animation, options, done) { - if ((options != null ? options.cssStart : void 0) != null) { - $el.css(options.cssStart); - } - $el.removeClass(animation + ' animated').addClass(animation + ' animated').one('webkitAnimationEnd mozAnimationEnd MSAnimationEnd oanimationend animationend', function() { - $(this).removeClass(animation + ' animated'); - if (done != null) { - done(); - } - if ((options != null ? options.cssEnd : void 0) != null) { - $el.css(options.cssEnd); - } - }); - }; - gl.animate.animateEach = function($els, animation, time, options, done) { - var dfd; - dfd = $.Deferred(); - if (!$els.length) { - dfd.resolve(); - } - $els.each(function(i) { - setTimeout((function(_this) { - return function() { - var $this; - $this = $(_this); - return gl.animate.animate($this, animation, options, function() { - if (i === $els.length - 1) { - dfd.resolve(); - if (done != null) { - return done(); - } - } - }); - }; - })(this), time * i); - }); - return dfd.promise(); - }; - })(window); -}).call(window); diff --git a/app/assets/javascripts/main.js b/app/assets/javascripts/main.js index 0bc31a56684..0f84470828a 100644 --- a/app/assets/javascripts/main.js +++ b/app/assets/javascripts/main.js @@ -39,7 +39,6 @@ import './commit/file'; import './commit/image_file'; // lib/utils -import './lib/utils/animate'; import './lib/utils/bootstrap_linked_tabs'; import './lib/utils/common_utils'; import './lib/utils/datetime_utility'; diff --git a/app/assets/javascripts/milestone_select.js b/app/assets/javascripts/milestone_select.js index 04579058688..4675b1fcb8f 100644 --- a/app/assets/javascripts/milestone_select.js +++ b/app/assets/javascripts/milestone_select.js @@ -45,7 +45,7 @@ import _ from 'underscore'; if (issueUpdateURL) { milestoneLinkTemplate = _.template('<a href="/<%- full_path %>/milestones/<%- iid %>" class="bold has-tooltip" data-container="body" title="<%- remaining %>"><%- title %></a>'); milestoneLinkNoneTemplate = '<span class="no-value">None</span>'; - collapsedSidebarLabelTemplate = _.template('<span class="has-tooltip" data-container="body" title="<%- remaining %>" data-placement="left"> <%- title %> </span>'); + collapsedSidebarLabelTemplate = _.template('<span class="has-tooltip" data-container="body" title="<%- name %><br /><%- remaining %>" data-placement="left" data-html="true"> <%- title %> </span>'); } return $dropdown.glDropdown({ showMenuAbove: showMenuAbove, @@ -208,6 +208,7 @@ import _ from 'underscore'; if (data.milestone != null) { data.milestone.full_path = _this.currentProject.full_path; data.milestone.remaining = gl.utils.timeFor(data.milestone.due_date); + data.milestone.name = data.milestone.title; $value.html(milestoneLinkTemplate(data.milestone)); return $sidebarCollapsedValue.find('span').html(collapsedSidebarLabelTemplate(data.milestone)); } else { diff --git a/app/assets/stylesheets/framework/gitlab-theme.scss b/app/assets/stylesheets/framework/gitlab-theme.scss index 71f764923ff..f844d6f1d5a 100644 --- a/app/assets/stylesheets/framework/gitlab-theme.scss +++ b/app/assets/stylesheets/framework/gitlab-theme.scss @@ -158,11 +158,23 @@ box-shadow: inset 4px 0 0 $color-700; > a { - color: $color-900; + color: $color-800; } svg { - fill: $color-900; + fill: $color-800; + } + } + + .sidebar-top-level-items > li.active .badge { + color: $color-800; + } + + .nav-links li.active a { + border-bottom-color: $color-500; + + .badge { + font-weight: $gl-font-weight-bold; } } } @@ -261,5 +273,9 @@ body { fill: $theme-gray-900; } } + + .sidebar-top-level-items > li.active .badge { + color: $theme-gray-900; + } } } diff --git a/app/assets/stylesheets/new_sidebar.scss b/app/assets/stylesheets/new_sidebar.scss index e4c12e46056..952002c83d1 100644 --- a/app/assets/stylesheets/new_sidebar.scss +++ b/app/assets/stylesheets/new_sidebar.scss @@ -3,8 +3,6 @@ @import "bootstrap/variables"; $active-background: rgba(0, 0, 0, .04); -$active-border: $indigo-500; -$active-color: $indigo-700; $active-hover-background: $active-background; $active-hover-color: $gl-text-color; $inactive-badge-background: rgba(0, 0, 0, .08); @@ -217,7 +215,6 @@ $new-sidebar-collapsed-width: 50px; &:hover, &:focus { background: $active-background; - color: $active-color; } } } @@ -317,7 +314,6 @@ $new-sidebar-collapsed-width: 50px; } .badge { - color: $active-color; font-weight: $gl-font-weight-bold; } @@ -494,13 +490,3 @@ $new-sidebar-collapsed-width: 50px; .with-performance-bar .boards-list { height: calc(100vh - #{$new-navbar-height} - #{$performance-bar-height}); } - - -// Change color of all horizontal tabs to match the new indigo color -.nav-links li.active a { - border-bottom-color: $active-border; - - .badge { - font-weight: $gl-font-weight-bold; - } -} diff --git a/app/assets/stylesheets/pages/issuable.scss b/app/assets/stylesheets/pages/issuable.scss index d8a15faf7e9..d01ee4b033c 100644 --- a/app/assets/stylesheets/pages/issuable.scss +++ b/app/assets/stylesheets/pages/issuable.scss @@ -449,6 +449,12 @@ } } } + + .milestone-title span { + @include str-truncated(100%); + display: block; + margin: 0 4px; + } } a { diff --git a/app/assets/stylesheets/pages/repo.scss b/app/assets/stylesheets/pages/repo.scss index 69abb13add4..7dfcf7b7d9c 100644 --- a/app/assets/stylesheets/pages/repo.scss +++ b/app/assets/stylesheets/pages/repo.scss @@ -71,6 +71,11 @@ height: 100%; .monaco-editor.vs { + .current-line { + border: none; + background: $well-light-border; + } + .line-numbers { cursor: pointer; @@ -84,6 +89,13 @@ } } + .blob-no-preview { + .vertical-center { + justify-content: center; + width: 100%; + } + } + &.edit-mode { .blob-viewer-container { overflow: hidden; @@ -103,7 +115,7 @@ overflow: auto; > div, - .file-content { + .file-content:not(.wiki) { display: flex; } diff --git a/app/controllers/boards/issues_controller.rb b/app/controllers/boards/issues_controller.rb index 8d4ec2d6d9d..0d74078645a 100644 --- a/app/controllers/boards/issues_controller.rb +++ b/app/controllers/boards/issues_controller.rb @@ -11,9 +11,15 @@ module Boards issues = Boards::Issues::ListService.new(board_parent, current_user, filter_params).execute issues = issues.page(params[:page]).per(params[:per] || 20) make_sure_position_is_set(issues) + issues = issues.preload(:project, + :milestone, + :assignees, + labels: [:priorities], + notes: [:award_emoji, :author] + ) render json: { - issues: serialize_as_json(issues.preload(:project)), + issues: serialize_as_json(issues), size: issues.total_count } end @@ -76,14 +82,13 @@ module Boards def serialize_as_json(resource) resource.as_json( - labels: true, only: [:id, :iid, :project_id, :title, :confidential, :due_date, :relative_position], + labels: true, include: { project: { only: [:id, :path] }, assignees: { only: [:id, :name, :username], methods: [:avatar_url] }, milestone: { only: [:id, :title] } - }, - user: current_user + } ) end end diff --git a/app/helpers/boards_helper.rb b/app/helpers/boards_helper.rb index 4bd61aa8f86..62ac208f16a 100644 --- a/app/helpers/boards_helper.rb +++ b/app/helpers/boards_helper.rb @@ -77,4 +77,8 @@ module BoardsHelper 'max-select': dropdown_options[:data][:'max-select'] } end + + def boards_link_text + _("Board") + end end diff --git a/app/helpers/groups_helper.rb b/app/helpers/groups_helper.rb index 36b79da1bde..e8efe8fab27 100644 --- a/app/helpers/groups_helper.rb +++ b/app/helpers/groups_helper.rb @@ -21,7 +21,7 @@ module GroupsHelper group.ancestors.reverse.each_with_index do |parent, index| if index > 0 - add_to_breadcrumb_dropdown(group_title_link(parent, hidable: false, show_avatar: true), location: :before) + add_to_breadcrumb_dropdown(group_title_link(parent, hidable: false, show_avatar: true, for_dropdown: true), location: :before) else full_title += breadcrumb_list_item group_title_link(parent, hidable: false) end @@ -85,8 +85,8 @@ module GroupsHelper private - def group_title_link(group, hidable: false, show_avatar: false) - link_to(group_path(group), class: "group-path breadcrumb-item-text js-breadcrumb-item-text #{'hidable' if hidable}") do + def group_title_link(group, hidable: false, show_avatar: false, for_dropdown: false) + link_to(group_path(group), class: "group-path #{'breadcrumb-item-text' unless for_dropdown} js-breadcrumb-item-text #{'hidable' if hidable}") do output = if (group.try(:avatar_url) || show_avatar) && !Rails.env.test? image_tag(group_icon(group), class: "avatar-tile", width: 15, height: 15) diff --git a/app/models/label.rb b/app/models/label.rb index 958141a7358..899028a01a0 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -127,7 +127,12 @@ class Label < ActiveRecord::Base end def priority(project) - priorities.find_by(project: project).try(:priority) + priority = if priorities.loaded? + priorities.first { |p| p.project == project } + else + priorities.find_by(project: project) + end + priority.try(:priority) end def template? diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb index 9d37184be2c..6a3118a11b8 100644 --- a/app/models/project_services/pipelines_email_service.rb +++ b/app/models/project_services/pipelines_email_service.rb @@ -80,6 +80,6 @@ class PipelinesEmailService < Service end def retrieve_recipients(data) - recipients.to_s.split(',').reject(&:blank?) + recipients.to_s.split(/[,(?:\r?\n) ]+/).reject(&:empty?) end end diff --git a/app/models/repository.rb b/app/models/repository.rb index cb479777fa3..f2b54705e7b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -90,6 +90,12 @@ class Repository ) end + # we need to have this method here because it is not cached in ::Git and + # the method is called multiple times for every request + def has_visible_content? + branch_count > 0 + end + def inspect "#<#{self.class.name}:#{@disk_path}>" end diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index 9589e81c750..5f7a2d86c0f 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -114,9 +114,9 @@ List = nav_link(controller: :boards) do - = link_to project_boards_path(@project), title: 'Board' do + = link_to project_boards_path(@project), title: boards_link_text do %span - Board + = boards_link_text .feature-highlight.js-feature-highlight{ disabled: true, data: { trigger: 'manual', container: 'body', toggle: 'popover', placement: 'right', highlight: 'issue-boards' } } .feature-highlight-popover-content = render 'feature_highlight/issue_boards.svg' diff --git a/app/views/profiles/preferences/show.html.haml b/app/views/profiles/preferences/show.html.haml index 69885008ecd..66d1d1e8d44 100644 --- a/app/views/profiles/preferences/show.html.haml +++ b/app/views/profiles/preferences/show.html.haml @@ -17,7 +17,7 @@ .preview-row .quadrant.three .quadrant.four - = f.radio_button :theme_id, theme.id + = f.radio_button :theme_id, theme.id, checked: Gitlab::Themes.for_user(@user).id == theme.id = theme.name .col-sm-12 diff --git a/app/views/projects/blob/viewers/_download.html.haml b/app/views/projects/blob/viewers/_download.html.haml index 6d1138f7959..253566c43be 100644 --- a/app/views/projects/blob/viewers/_download.html.haml +++ b/app/views/projects/blob/viewers/_download.html.haml @@ -1,5 +1,5 @@ .file-content.blob_file.blob-no-preview - .center + .center.render-error.vertical-center = link_to blob_raw_path do %h1.light = icon('download') diff --git a/app/views/shared/issuable/_sidebar.html.haml b/app/views/shared/issuable/_sidebar.html.haml index 0afa48b392c..9cae3f51825 100644 --- a/app/views/shared/issuable/_sidebar.html.haml +++ b/app/views/shared/issuable/_sidebar.html.haml @@ -24,9 +24,9 @@ .block.milestone .sidebar-collapsed-icon = icon('clock-o', 'aria-hidden': 'true') - %span + %span.milestone-title - if issuable.milestone - %span.has-tooltip{ title: milestone_remaining_days(issuable.milestone), data: { container: 'body', html: 1, placement: 'left' } } + %span.has-tooltip{ title: "#{issuable.milestone.title}<br>#{milestone_remaining_days(issuable.milestone)}", data: { container: 'body', html: 1, placement: 'left' } } = issuable.milestone.title - else None diff --git a/changelogs/unreleased/34510-board-issues-sql-speedup.yml b/changelogs/unreleased/34510-board-issues-sql-speedup.yml new file mode 100644 index 00000000000..244ff7e9dfa --- /dev/null +++ b/changelogs/unreleased/34510-board-issues-sql-speedup.yml @@ -0,0 +1,5 @@ +--- +title: Optimize the boards' issues fetching. +merge_request: 14198 +author: +type: other diff --git a/changelogs/unreleased/35978-milestone-title.yml b/changelogs/unreleased/35978-milestone-title.yml new file mode 100644 index 00000000000..1a4b71328c1 --- /dev/null +++ b/changelogs/unreleased/35978-milestone-title.yml @@ -0,0 +1,5 @@ +--- +title: Truncate milestone title if sidebar is collapsed +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/37576-renamed-files-have-escaped-html-for-the-inline-diff-in-the-header.yml b/changelogs/unreleased/37576-renamed-files-have-escaped-html-for-the-inline-diff-in-the-header.yml new file mode 100644 index 00000000000..8c328eb0950 --- /dev/null +++ b/changelogs/unreleased/37576-renamed-files-have-escaped-html-for-the-inline-diff-in-the-header.yml @@ -0,0 +1,5 @@ +--- +title: Fix the diff file header from being html escaped for renamed files. +merge_request: 14121 +author: +type: fixed diff --git a/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml b/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml new file mode 100644 index 00000000000..6894e650c11 --- /dev/null +++ b/changelogs/unreleased/37759-also-treat-newlines-as-separator.yml @@ -0,0 +1,5 @@ +--- +title: Allow using newlines in pipeline email service recipients +merge_request: 14250 +author: +type: fixed diff --git a/changelogs/unreleased/refactor-animate-js.yml b/changelogs/unreleased/refactor-animate-js.yml new file mode 100644 index 00000000000..ec32d68bbdd --- /dev/null +++ b/changelogs/unreleased/refactor-animate-js.yml @@ -0,0 +1,5 @@ +--- +title: Remove animate.js and label animation. +merge_request: +author: +type: removed diff --git a/db/migrate/20170828135939_migrate_user_external_mail_data.rb b/db/migrate/20170828135939_migrate_user_external_mail_data.rb new file mode 100644 index 00000000000..592e141b7e6 --- /dev/null +++ b/db/migrate/20170828135939_migrate_user_external_mail_data.rb @@ -0,0 +1,57 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class MigrateUserExternalMailData < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + disable_ddl_transaction! + + class User < ActiveRecord::Base + self.table_name = 'users' + + include EachBatch + end + + class UserSyncedAttributesMetadata < ActiveRecord::Base + self.table_name = 'user_synced_attributes_metadata' + + include EachBatch + end + + def up + User.each_batch do |batch| + start_id, end_id = batch.pluck('MIN(id), MAX(id)').first + + execute <<-EOF + INSERT INTO user_synced_attributes_metadata (user_id, provider, email_synced) + SELECT id, email_provider, external_email + FROM users + WHERE external_email = TRUE + AND NOT EXISTS ( + SELECT true + FROM user_synced_attributes_metadata + WHERE user_id = users.id + AND provider = users.email_provider + ) + AND id BETWEEN #{start_id} AND #{end_id} + EOF + end + end + + def down + UserSyncedAttributesMetadata.each_batch do |batch| + start_id, end_id = batch.pluck('MIN(id), MAX(id)').first + + execute <<-EOF + UPDATE users + SET users.email_provider = metadata.provider, users.external_email = metadata.email_synced + FROM user_synced_attributes_metadata as metadata, users + WHERE metadata.email_synced = TRUE + AND metadata.user_id = users.id + AND id BETWEEN #{start_id} AND #{end_id} + EOF + end + end +end diff --git a/doc/administration/gitaly/index.md b/doc/administration/gitaly/index.md index 5732b6a1ca4..40099dcc967 100644 --- a/doc/administration/gitaly/index.md +++ b/doc/administration/gitaly/index.md @@ -145,8 +145,8 @@ Omnibus installations: ```ruby # /etc/gitlab/gitlab.rb git_data_dirs({ - { 'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tcp://gitlab.internal:9999' } }, - { 'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tcp://gitlab.internal:9999' } }, + 'default' => { 'path' => '/mnt/gitlab/default', 'gitaly_address' => 'tcp://gitlab.internal:9999' }, + 'storage1' => { 'path' => '/mnt/gitlab/storage1', 'gitaly_address' => 'tcp://gitlab.internal:9999' }, }) gitlab_rails['gitaly_token'] = 'abc123secret' diff --git a/doc/user/project/integrations/prometheus_library/nginx_ingress.md b/doc/user/project/integrations/prometheus_library/nginx_ingress.md index 2a37cbd160b..17a47cfa646 100644 --- a/doc/user/project/integrations/prometheus_library/nginx_ingress.md +++ b/doc/user/project/integrations/prometheus_library/nginx_ingress.md @@ -11,7 +11,7 @@ GitLab has support for automatically detecting and monitoring the Kubernetes NGI | Latency (ms) | avg(nginx_upstream_response_msecs_avg{upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}) | | HTTP Error Rate (HTTP Errors / sec) | sum(rate(nginx_upstream_responses_total{status_code="5xx", upstream=~"%{kube_namespace}-%{ci_environment_slug}-.*"}[2m])) | -## Configuring Prometheus to monitor for NGINX ingress metrics +## Configuring NGINX ingress monitoring If you have deployed with the [gitlab-omnibus](https://docs.gitlab.com/ee/install/kubernetes/gitlab_omnibus.md) Helm chart, and your application is running in the same cluster, no further action is required. The ingress metrics will be automatically enabled and annotated for Prometheus monitoring. Simply ensure Prometheus monitoring is [enabled for your project](../prometheus.md), which is on by default. @@ -20,14 +20,14 @@ For other deployments, there is some configuration required depending on your in * NGINX Ingress should be annotated for Prometheus monitoring * Prometheus should be configured to monitor annotated pods -### Configuring NGINX Ingress for Prometheus monitoring +### Setting up NGINX Ingress for Prometheus monitoring Version 0.9.0 and above of [NGINX ingress](https://github.com/kubernetes/ingress/tree/master/controllers/nginx) have built-in support for exporting Prometheus metrics. To enable, a ConfigMap setting must be passed: `enable-vts-status: "true"`. Once enabled, a Prometheus metrics endpoint will start running on port 10254. -With metric data now available, Prometheus needs to be configured to collect it. The easiest way to do this is to leverage Prometheus' [built-in Kubernetes service discovery](https://prometheus.io/docs/operating/configuration/#kubernetes_sd_config), which automatically detects a variety of Kubernetes components and makes them available for monitoring. NGINX ingress metrics are exposed per pod, a sample scrape configuration [is available](https://github.com/prometheus/prometheus/blob/master/documentation/examples/prometheus-kubernetes.yml#L248). This configuration will detect pods and enable collection of metrics **only if** they have been specifically annotated for monitoring. +With metric data now available, Prometheus needs to be configured to collect it. The easiest way to do this is to leverage Prometheus' [built-in Kubernetes service discovery](https://prometheus.io/docs/operating/configuration/#kubernetes_sd_config), which automatically detects a variety of Kubernetes components and makes them available for monitoring. Since NGINX ingress metrics are exposed per pod, a scrape job for Kubernetes pods is required. A sample pod scraping configuration [is available](https://github.com/prometheus/prometheus/blob/master/documentation/examples/prometheus-kubernetes.yml#L248). This configuration will detect pods and enable collection of metrics **only if** they have been specifically annotated for monitoring. Depending on how NGINX ingress was deployed, typically a DaemonSet or Deployment, edit the corresponding YML spec. Two new annotations need to be added: -* `prometheus.io/port: "true"` +* `prometheus.io/scrape: "true"` * `prometheus.io/port: "10254"` Prometheus should now be collecting NGINX ingress metrics. To validate view the Prometheus Targets, available under `Status > Targets` on the Prometheus dashboard. New entries for NGINX should be listed in the kubernetes pod monitoring job, `kubernetes-pods`. diff --git a/lib/api/api.rb b/lib/api/api.rb index ee4e1688e12..79e55a2f4f7 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -8,7 +8,6 @@ module API logger: Logger.new(LOG_FILENAME), formatter: Gitlab::GrapeLogging::Formatters::LogrageWithTimestamp.new, include: [ - GrapeLogging::Loggers::Response.new, GrapeLogging::Loggers::FilterParameters.new, GrapeLogging::Loggers::ClientEnv.new ] diff --git a/lib/gitlab/diff/inline_diff_marker.rb b/lib/gitlab/diff/inline_diff_marker.rb index 919965100ae..010b4be7b40 100644 --- a/lib/gitlab/diff/inline_diff_marker.rb +++ b/lib/gitlab/diff/inline_diff_marker.rb @@ -2,9 +2,10 @@ module Gitlab module Diff class InlineDiffMarker < Gitlab::StringRangeMarker def mark(line_inline_diffs, mode: nil) - super(line_inline_diffs) do |text, left:, right:| + mark = super(line_inline_diffs) do |text, left:, right:| %{<span class="#{html_class_names(left, right, mode)}">#{text}</span>} end + mark.html_safe end private diff --git a/lib/gitlab/o_auth/auth_hash.rb b/lib/gitlab/o_auth/auth_hash.rb index 1f331b1e91d..5b5ed449f94 100644 --- a/lib/gitlab/o_auth/auth_hash.rb +++ b/lib/gitlab/o_auth/auth_hash.rb @@ -13,7 +13,7 @@ module Gitlab end def provider - @provider ||= Gitlab::Utils.force_utf8(auth_hash.provider.to_s) + @provider ||= auth_hash.provider.to_s end def name diff --git a/lib/gitlab/url_sanitizer.rb b/lib/gitlab/url_sanitizer.rb index 703adae12cb..4e1ec1402ea 100644 --- a/lib/gitlab/url_sanitizer.rb +++ b/lib/gitlab/url_sanitizer.rb @@ -19,13 +19,12 @@ module Gitlab end def initialize(url, credentials: nil) - @url = Addressable::URI.parse(url.to_s.strip) - %i[user password].each do |symbol| credentials[symbol] = credentials[symbol].presence if credentials&.key?(symbol) end @credentials = credentials + @url = parse_url(url) end def sanitized_url @@ -49,12 +48,30 @@ module Gitlab private + def parse_url(url) + url = url.to_s.strip + match = url.match(%r{\A(?:git|ssh|http(?:s?))\://(?:(.+)(?:@))?(.+)}) + raw_credentials = match[1] if match + + if raw_credentials.present? + url.sub!("#{raw_credentials}@", '') + + user, password = raw_credentials.split(':') + @credentials ||= { user: user.presence, password: password.presence } + end + + url = Addressable::URI.parse(url) + url.password = password if password.present? + url.user = user if user.present? + url + end + def generate_full_url return @url unless valid_credentials? @full_url = @url.dup - @full_url.password = credentials[:password] - @full_url.user = credentials[:user] + @full_url.password = credentials[:password] if credentials[:password].present? + @full_url.user = credentials[:user] if credentials[:user].present? @full_url end diff --git a/spec/controllers/boards/issues_controller_spec.rb b/spec/controllers/boards/issues_controller_spec.rb index dfa06c78d46..5163099cd98 100644 --- a/spec/controllers/boards/issues_controller_spec.rb +++ b/spec/controllers/boards/issues_controller_spec.rb @@ -45,6 +45,17 @@ describe Boards::IssuesController do expect(parsed_response.length).to eq 2 expect(development.issues.map(&:relative_position)).not_to include(nil) end + + it 'avoids N+1 database queries' do + create(:labeled_issue, project: project, labels: [development]) + control_count = ActiveRecord::QueryRecorder.new { list_issues(user: user, board: board, list: list2) }.count + + # 25 issues is bigger than the page size + # the relative position will ignore the `#make_sure_position_set` queries + create_list(:labeled_issue, 25, project: project, labels: [development], assignees: [johndoe], relative_position: 1) + + expect { list_issues(user: user, board: board, list: list2) }.not_to exceed_query_limit(control_count) + end end context 'with invalid list id' do diff --git a/spec/features/projects/diffs/diff_show_spec.rb b/spec/features/projects/diffs/diff_show_spec.rb index bc102895aaf..a6f52c9ef58 100644 --- a/spec/features/projects/diffs/diff_show_spec.rb +++ b/spec/features/projects/diffs/diff_show_spec.rb @@ -108,6 +108,19 @@ feature 'Diff file viewer', :js do end end + context 'renamed file' do + before do + visit_commit('6907208d755b60ebeacb2e9dfea74c92c3449a1f') + end + + it 'shows the filename with diff highlight' do + within('.file-header-content') do + expect(page).to have_css('.idiff.left.right.deletion') + expect(page).to have_content('files/js/commit.coffee') + end + end + end + context 'binary file that appears to be text in the first 1024 bytes' do before do # The file we're visiting is smaller than 10 KB and we want it collapsed diff --git a/spec/helpers/diff_helper_spec.rb b/spec/helpers/diff_helper_spec.rb index 0deea0ff6a3..f9c31ac61d8 100644 --- a/spec/helpers/diff_helper_spec.rb +++ b/spec/helpers/diff_helper_spec.rb @@ -136,9 +136,9 @@ describe DiffHelper do marked_old_line, marked_new_line = mark_inline_diffs(old_line, new_line) expect(marked_old_line).to eq(%q{abc <span class="idiff left right deletion">'def'</span>}) - expect(marked_old_line).not_to be_html_safe + expect(marked_old_line).to be_html_safe expect(marked_new_line).to eq(%q{abc <span class="idiff left right addition">"def"</span>}) - expect(marked_new_line).not_to be_html_safe + expect(marked_new_line).to be_html_safe end end diff --git a/spec/lib/gitlab/o_auth/auth_hash_spec.rb b/spec/lib/gitlab/o_auth/auth_hash_spec.rb index d5f4da3ce36..dbcc200b90b 100644 --- a/spec/lib/gitlab/o_auth/auth_hash_spec.rb +++ b/spec/lib/gitlab/o_auth/auth_hash_spec.rb @@ -1,10 +1,11 @@ require 'spec_helper' describe Gitlab::OAuth::AuthHash do + let(:provider) { 'ldap'.freeze } let(:auth_hash) do described_class.new( OmniAuth::AuthHash.new( - provider: provider_ascii, + provider: provider, uid: uid_ascii, info: info_hash ) @@ -20,7 +21,6 @@ describe Gitlab::OAuth::AuthHash do let(:last_name_raw) { "K\xC3\xBC\xC3\xA7\xC3\xBCk" } let(:name_raw) { "Onur K\xC3\xBC\xC3\xA7\xC3\xBCk" } - let(:provider_ascii) { 'ldap'.force_encoding(Encoding::ASCII_8BIT) } let(:uid_ascii) { uid_raw.force_encoding(Encoding::ASCII_8BIT) } let(:email_ascii) { email_raw.force_encoding(Encoding::ASCII_8BIT) } let(:nickname_ascii) { nickname_raw.force_encoding(Encoding::ASCII_8BIT) } @@ -28,7 +28,6 @@ describe Gitlab::OAuth::AuthHash do let(:last_name_ascii) { last_name_raw.force_encoding(Encoding::ASCII_8BIT) } let(:name_ascii) { name_raw.force_encoding(Encoding::ASCII_8BIT) } - let(:provider_utf8) { provider_ascii.force_encoding(Encoding::UTF_8) } let(:uid_utf8) { uid_ascii.force_encoding(Encoding::UTF_8) } let(:email_utf8) { email_ascii.force_encoding(Encoding::UTF_8) } let(:nickname_utf8) { nickname_ascii.force_encoding(Encoding::UTF_8) } @@ -46,7 +45,7 @@ describe Gitlab::OAuth::AuthHash do end context 'defaults' do - it { expect(auth_hash.provider).to eql provider_utf8 } + it { expect(auth_hash.provider).to eq provider } it { expect(auth_hash.uid).to eql uid_utf8 } it { expect(auth_hash.email).to eql email_utf8 } it { expect(auth_hash.username).to eql nickname_utf8 } diff --git a/spec/lib/gitlab/url_sanitizer_spec.rb b/spec/lib/gitlab/url_sanitizer_spec.rb index fdc3990132a..59c28431e1e 100644 --- a/spec/lib/gitlab/url_sanitizer_spec.rb +++ b/spec/lib/gitlab/url_sanitizer_spec.rb @@ -174,4 +174,13 @@ describe Gitlab::UrlSanitizer do end end end + + context 'when credentials contains special chars' do + it 'should parse the URL without errors' do + url_sanitizer = described_class.new("https://foo:b?r@github.com/me/project.git") + + expect(url_sanitizer.sanitized_url).to eq("https://github.com/me/project.git") + expect(url_sanitizer.full_url).to eq("https://foo:b?r@github.com/me/project.git") + end + end end diff --git a/spec/models/abuse_report_spec.rb b/spec/models/abuse_report_spec.rb index d4da30b1641..f49a61062c1 100644 --- a/spec/models/abuse_report_spec.rb +++ b/spec/models/abuse_report_spec.rb @@ -1,8 +1,9 @@ require 'rails_helper' -RSpec.describe AbuseReport do - subject { create(:abuse_report) } - let(:user) { create(:admin) } +describe AbuseReport do + set(:report) { create(:abuse_report) } + set(:user) { create(:admin) } + subject { report } it { expect(subject).to be_valid } diff --git a/spec/models/appearance_spec.rb b/spec/models/appearance_spec.rb index b5d5d58697b..49f44525b29 100644 --- a/spec/models/appearance_spec.rb +++ b/spec/models/appearance_spec.rb @@ -1,6 +1,6 @@ require 'rails_helper' -RSpec.describe Appearance do +describe Appearance do subject { build(:appearance) } it { is_expected.to be_valid } diff --git a/spec/models/chat_name_spec.rb b/spec/models/chat_name_spec.rb index 8581bcbb08b..e89e534d914 100644 --- a/spec/models/chat_name_spec.rb +++ b/spec/models/chat_name_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe ChatName do - subject { create(:chat_name) } + set(:chat_name) { create(:chat_name) } + subject { chat_name } it { is_expected.to belong_to(:service) } it { is_expected.to belong_to(:user) } diff --git a/spec/models/chat_team_spec.rb b/spec/models/chat_team_spec.rb index e0e5f73e6fe..70a9a206faa 100644 --- a/spec/models/chat_team_spec.rb +++ b/spec/models/chat_team_spec.rb @@ -1,7 +1,8 @@ require 'spec_helper' describe ChatTeam do - subject { create(:chat_team) } + set(:chat_team) { create(:chat_team) } + subject { chat_team } # Associations it { is_expected.to belong_to(:namespace) } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c2c9f1c12d1..451968c7342 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -1,18 +1,19 @@ require 'spec_helper' describe Ci::Build do - let(:user) { create(:user) } - let(:project) { create(:project, :repository) } - let(:build) { create(:ci_build, pipeline: pipeline) } - let(:test_trace) { 'This is a test' } + set(:user) { create(:user) } + set(:group) { create(:group, :access_requestable) } + set(:project) { create(:project, :repository, group: group) } - let(:pipeline) do + set(:pipeline) do create(:ci_pipeline, project: project, sha: project.commit.id, ref: project.default_branch, status: 'success') end + let(:build) { create(:ci_build, pipeline: pipeline) } + it { is_expected.to belong_to(:runner) } it { is_expected.to belong_to(:trigger_request) } it { is_expected.to belong_to(:erased_by) } @@ -282,7 +283,7 @@ describe Ci::Build do let(:project_regex) { '\(\d+\.\d+\) covered' } before do - project.build_coverage_regex = project_regex + project.update_column(:build_coverage_regex, project_regex) end context 'and coverage_regex attribute is not set' do @@ -1096,9 +1097,6 @@ describe Ci::Build do end describe '#repo_url' do - let(:build) { create(:ci_build) } - let(:project) { build.project } - subject { build.repo_url } it { is_expected.to be_a(String) } @@ -1199,6 +1197,8 @@ describe Ci::Build do end context 'use from gitlab-ci.yml' do + let(:pipeline) { create(:ci_pipeline) } + before do stub_ci_pipeline_yaml_file(config) end @@ -1442,11 +1442,7 @@ describe Ci::Build do { key: 'SECRET_KEY', value: 'secret_value', public: false } end - let(:group) { create(:group, :access_requestable) } - before do - build.project.update(group: group) - create(:ci_group_variable, secret_variable.slice(:key, :value).merge(group: group)) end @@ -1459,11 +1455,7 @@ describe Ci::Build do { key: 'PROTECTED_KEY', value: 'protected_value', public: false } end - let(:group) { create(:group, :access_requestable) } - before do - build.project.update(group: group) - create(:ci_group_variable, :protected, protected_variable.slice(:key, :value).merge(group: group)) @@ -1486,6 +1478,10 @@ describe Ci::Build do end context 'when the ref is not protected' do + before do + build.update_column(:ref, 'some/feature') + end + it { is_expected.not_to include(protected_variable) } end end @@ -1552,6 +1548,8 @@ describe Ci::Build do end context 'when yaml_variables are undefined' do + let(:pipeline) { create(:ci_pipeline, project: project) } + before do build.yaml_variables = nil end @@ -1645,7 +1643,10 @@ describe Ci::Build do before do build.environment = 'production' - allow(project).to receive(:deployment_variables).and_return([deployment_variable]) + + allow_any_instance_of(Project) + .to receive(:deployment_variables) + .and_return([deployment_variable]) end it { is_expected.to include(deployment_variable) } @@ -1669,14 +1670,19 @@ describe Ci::Build do before do allow(build).to receive(:predefined_variables) { [build_pre_var] } - allow(project).to receive(:predefined_variables) { [project_pre_var] } - allow(pipeline).to receive(:predefined_variables) { [pipeline_pre_var] } allow(build).to receive(:yaml_variables) { [build_yaml_var] } - allow(project).to receive(:secret_variables_for) + allow_any_instance_of(Project) + .to receive(:predefined_variables) { [project_pre_var] } + + allow_any_instance_of(Project) + .to receive(:secret_variables_for) .with(ref: 'master', environment: nil) do [create(:ci_variable, key: 'secret', value: 'value')] end + + allow_any_instance_of(Ci::Pipeline) + .to receive(:predefined_variables) { [pipeline_pre_var] } end it do diff --git a/spec/models/lfs_objects_project_spec.rb b/spec/models/lfs_objects_project_spec.rb index d24d4cf7695..0a3180f43e8 100644 --- a/spec/models/lfs_objects_project_spec.rb +++ b/spec/models/lfs_objects_project_spec.rb @@ -1,8 +1,11 @@ require 'spec_helper' describe LfsObjectsProject do - subject { create(:lfs_objects_project, project: project) } - let(:project) { create(:project) } + set(:project) { create(:project) } + + subject do + create(:lfs_objects_project, project: project) + end describe 'associations' do it { is_expected.to belong_to(:project) } @@ -11,9 +14,13 @@ describe LfsObjectsProject do describe 'validation' do it { is_expected.to validate_presence_of(:lfs_object_id) } - it { is_expected.to validate_uniqueness_of(:lfs_object_id).scoped_to(:project_id).with_message("already exists in project") } - it { is_expected.to validate_presence_of(:project_id) } + + it 'validates object id' do + is_expected.to validate_uniqueness_of(:lfs_object_id) + .scoped_to(:project_id) + .with_message("already exists in project") + end end describe '#update_project_statistics' do diff --git a/spec/models/project_services/pipelines_email_service_spec.rb b/spec/models/project_services/pipelines_email_service_spec.rb index 5faab9ba38b..be07ca2d945 100644 --- a/spec/models/project_services/pipelines_email_service_spec.rb +++ b/spec/models/project_services/pipelines_email_service_spec.rb @@ -6,7 +6,8 @@ describe PipelinesEmailService, :mailer do end let(:project) { create(:project, :repository) } - let(:recipient) { 'test@gitlab.com' } + let(:recipients) { 'test@gitlab.com' } + let(:receivers) { [recipients] } let(:data) do Gitlab::DataBuilder::Pipeline.build(pipeline) @@ -48,18 +49,24 @@ describe PipelinesEmailService, :mailer do shared_examples 'sending email' do before do + subject.recipients = recipients + perform_enqueued_jobs do run end end it 'sends email' do - should_only_email(double(notification_email: recipient), kind: :bcc) + emails = receivers.map { |r| double(notification_email: r) } + + should_only_email(*emails, kind: :bcc) end end shared_examples 'not sending email' do before do + subject.recipients = recipients + perform_enqueued_jobs do run end @@ -75,10 +82,6 @@ describe PipelinesEmailService, :mailer do subject.test(data) end - before do - subject.recipients = recipient - end - context 'when pipeline is failed' do before do data[:object_attributes][:status] = 'failed' @@ -104,10 +107,6 @@ describe PipelinesEmailService, :mailer do end context 'with recipients' do - before do - subject.recipients = recipient - end - context 'with failed pipeline' do before do data[:object_attributes][:status] = 'failed' @@ -152,9 +151,7 @@ describe PipelinesEmailService, :mailer do end context 'with empty recipients list' do - before do - subject.recipients = ' ,, ' - end + let(:recipients) { ' ,, ' } context 'with failed pipeline' do before do @@ -165,5 +162,19 @@ describe PipelinesEmailService, :mailer do it_behaves_like 'not sending email' end end + + context 'with recipients list separating with newlines' do + let(:recipients) { "\ntest@gitlab.com, \r\nexample@gitlab.com" } + let(:receivers) { %w[test@gitlab.com example@gitlab.com] } + + context 'with failed pipeline' do + before do + data[:object_attributes][:status] = 'failed' + pipeline.update(status: 'failed') + end + + it_behaves_like 'sending email' + end + end end end diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 48d99841385..7e174903918 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -1,10 +1,13 @@ require "spec_helper" describe API::Services do - let(:user) { create(:user) } - let(:admin) { create(:admin) } - let(:user2) { create(:user) } - let(:project) { create(:project, creator_id: user.id, namespace: user.namespace) } + set(:user) { create(:user) } + set(:admin) { create(:admin) } + set(:user2) { create(:user) } + + set(:project) do + create(:project, creator_id: user.id, namespace: user.namespace) + end Service.available_services_names.each do |service| describe "PUT /projects/:id/services/#{service.dasherize}" do @@ -98,8 +101,6 @@ describe API::Services do end describe 'POST /projects/:id/services/:slug/trigger' do - let!(:project) { create(:project) } - describe 'Mattermost Service' do let(:service_name) { 'mattermost_slash_commands' } diff --git a/spec/services/ci/retry_build_service_spec.rb b/spec/services/ci/retry_build_service_spec.rb index bbc3a8c79f5..fbb3213f42b 100644 --- a/spec/services/ci/retry_build_service_spec.rb +++ b/spec/services/ci/retry_build_service_spec.rb @@ -1,9 +1,10 @@ require 'spec_helper' describe Ci::RetryBuildService do - let(:user) { create(:user) } - let(:project) { create(:project) } - let(:pipeline) { create(:ci_pipeline, project: project) } + set(:user) { create(:user) } + set(:project) { create(:project) } + set(:pipeline) { create(:ci_pipeline, project: project) } + let(:build) { create(:ci_build, pipeline: pipeline) } let(:service) do @@ -37,7 +38,7 @@ describe Ci::RetryBuildService do :queued, :coverage, :tags, :allowed_to_fail, :on_tag, :triggered, :trace, :teardown_environment, description: 'my-job', stage: 'test', pipeline: pipeline, - auto_canceled_by: create(:ci_empty_pipeline)) do |build| + auto_canceled_by: create(:ci_empty_pipeline, project: project)) do |build| ## # TODO, workaround for FactoryGirl limitation when having both # stage (text) and stage_id (integer) columns in the table. diff --git a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb index b17bc6692f3..c5f455b8948 100644 --- a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb +++ b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb @@ -1,16 +1,28 @@ require 'spec_helper' describe 'layouts/nav/sidebar/_project' do + let(:project) { create(:project, :repository) } + + before do + assign(:project, project) + assign(:repository, project.repository) + allow(view).to receive(:current_ref).and_return('master') + + allow(view).to receive(:can?).and_return(true) + end + + describe 'issue boards' do + it 'has boards tab when multiple issue boards available' do + render + + expect(rendered).to have_css('a[title="Board"]') + end + end + describe 'container registry tab' do before do - project = create(:project, :repository) stub_container_registry_config(enabled: true) - assign(:project, project) - assign(:repository, project.repository) - allow(view).to receive(:current_ref).and_return('master') - - allow(view).to receive(:can?).and_return(true) allow(controller).to receive(:controller_name) .and_return('repositories') allow(controller).to receive(:controller_path) diff --git a/spec/workers/git_garbage_collect_worker_spec.rb b/spec/workers/git_garbage_collect_worker_spec.rb index 6f9ddb6c63c..f7b67b8efc6 100644 --- a/spec/workers/git_garbage_collect_worker_spec.rb +++ b/spec/workers/git_garbage_collect_worker_spec.rb @@ -31,8 +31,8 @@ describe GitGarbageCollectWorker do expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original + expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original - expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original subject.perform(project.id, :gc, lease_key, lease_uuid) end @@ -77,8 +77,8 @@ describe GitGarbageCollectWorker do expect_any_instance_of(Repository).to receive(:after_create_branch).and_call_original expect_any_instance_of(Repository).to receive(:branch_names).and_call_original + expect_any_instance_of(Repository).to receive(:has_visible_content?).and_call_original expect_any_instance_of(Gitlab::Git::Repository).to receive(:branch_count).and_call_original - expect_any_instance_of(Gitlab::Git::Repository).to receive(:has_visible_content?).and_call_original subject.perform(project.id) end |