diff options
46 files changed, 371 insertions, 150 deletions
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/javascripts/monitoring/components/dashboard.vue b/app/assets/javascripts/monitoring/components/dashboard.vue index b596c4f383f..5d96b193fce 100644 --- a/app/assets/javascripts/monitoring/components/dashboard.vue +++ b/app/assets/javascripts/monitoring/components/dashboard.vue @@ -1,7 +1,6 @@ <script> /* global Flash */ import _ from 'underscore'; - import statusCodes from '../../lib/utils/http_status'; import MonitoringService from '../services/monitoring_service'; import GraphGroup from './graph_group.vue'; import Graph from './graph.vue'; @@ -21,10 +20,9 @@ hasMetrics: gl.utils.convertPermissionToBoolean(metricsData.hasMetrics), documentationPath: metricsData.documentationPath, settingsPath: metricsData.settingsPath, - endpoint: metricsData.additionalMetrics, + metricsEndpoint: metricsData.additionalMetrics, deploymentEndpoint: metricsData.deploymentEndpoint, showEmptyState: true, - backOffRequestCounter: 0, updateAspectRatio: false, updatedAspectRatios: 0, resizeThrottled: {}, @@ -39,50 +37,16 @@ methods: { getGraphsData() { - const maxNumberOfRequests = 3; this.state = 'loading'; - gl.utils.backOff((next, stop) => { - this.service.get().then((resp) => { - if (resp.status === statusCodes.NO_CONTENT) { - this.backOffRequestCounter = this.backOffRequestCounter += 1; - if (this.backOffRequestCounter < maxNumberOfRequests) { - next(); - } else { - stop(new Error('Failed to connect to the prometheus server')); - } - } else { - stop(resp); - } - }).catch(stop); - }) - .then((resp) => { - if (resp.status === statusCodes.NO_CONTENT) { - this.state = 'unableToConnect'; - return false; - } - return resp.json(); - }) - .then((metricGroupsData) => { - if (!metricGroupsData) return false; - this.store.storeMetrics(metricGroupsData.data); - return this.getDeploymentData(); - }) - .then((deploymentData) => { - if (deploymentData !== false) { - this.store.storeDeploymentData(deploymentData.deployments); - this.showEmptyState = false; - } - return {}; - }) - .catch(() => { - this.state = 'unableToConnect'; - }); - }, - - getDeploymentData() { - return this.service.getDeploymentData(this.deploymentEndpoint) - .then(resp => resp.json()) - .catch(() => new Flash('Error getting deployment information.')); + Promise.all([ + this.service.getGraphsData() + .then(data => this.store.storeMetrics(data)), + this.service.getDeploymentData() + .then(data => this.store.storeDeploymentData(data)) + .catch(() => new Flash('Error getting deployment information.')), + ]) + .then(() => { this.showEmptyState = false; }) + .catch(() => { this.state = 'unableToConnect'; }); }, resize() { @@ -99,7 +63,10 @@ }, created() { - this.service = new MonitoringService(this.endpoint); + this.service = new MonitoringService({ + metricsEndpoint: this.metricsEndpoint, + deploymentEndpoint: this.deploymentEndpoint, + }); eventHub.$on('toggleAspectRatio', this.toggleAspectRatio); }, diff --git a/app/assets/javascripts/monitoring/services/monitoring_service.js b/app/assets/javascripts/monitoring/services/monitoring_service.js index 1e9ae934853..4ed651d5740 100644 --- a/app/assets/javascripts/monitoring/services/monitoring_service.js +++ b/app/assets/javascripts/monitoring/services/monitoring_service.js @@ -1,19 +1,54 @@ import Vue from 'vue'; import VueResource from 'vue-resource'; +import statusCodes from '../../lib/utils/http_status'; Vue.use(VueResource); +const MAX_REQUESTS = 3; + +function backOffRequest(makeRequestCallback) { + let requestCounter = 0; + return gl.utils.backOff((next, stop) => { + makeRequestCallback().then((resp) => { + if (resp.status === statusCodes.NO_CONTENT) { + requestCounter += 1; + if (requestCounter < MAX_REQUESTS) { + next(); + } else { + stop(new Error('Failed to connect to the prometheus server')); + } + } else { + stop(resp); + } + }).catch(stop); + }); +} + export default class MonitoringService { - constructor(endpoint) { - this.graphs = Vue.resource(endpoint); + constructor({ metricsEndpoint, deploymentEndpoint }) { + this.metricsEndpoint = metricsEndpoint; + this.deploymentEndpoint = deploymentEndpoint; } - get() { - return this.graphs.get(); + getGraphsData() { + return backOffRequest(() => Vue.http.get(this.metricsEndpoint)) + .then(resp => resp.json()) + .then((response) => { + if (!response || !response.data) { + throw new Error('Unexpected metrics data response from prometheus endpoint'); + } + return response.data; + }); } - // eslint-disable-next-line class-methods-use-this - getDeploymentData(endpoint) { - return Vue.http.get(endpoint); + getDeploymentData() { + return backOffRequest(() => Vue.http.get(this.deploymentEndpoint)) + .then(resp => resp.json()) + .then((response) => { + if (!response || !response.deployments) { + throw new Error('Unexpected deployment data response from prometheus endpoint'); + } + return response.deployments; + }); } } 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/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 0c8cb9ba235..ddeff490d3a 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -320,7 +320,7 @@ module ProjectsHelper def git_user_name if current_user - current_user.name + current_user.name.gsub('"', '\"') else _("Your name") end diff --git a/app/models/issue.rb b/app/models/issue.rb index 8c7d492e605..cd5056aae5e 100644 --- a/app/models/issue.rb +++ b/app/models/issue.rb @@ -30,9 +30,6 @@ class Issue < ActiveRecord::Base has_many :issue_assignees has_many :assignees, class_name: "User", through: :issue_assignees - has_many :issue_assignees - has_many :assignees, class_name: "User", through: :issue_assignees - validates :project, presence: true scope :in_projects, ->(project_ids) { where(project_id: project_ids) } 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/repository.rb b/app/models/repository.rb index 035f85a0b46..6ed33e0c268 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/services/ci/pipeline_trigger_service.rb b/app/services/ci/pipeline_trigger_service.rb index 1e5ad28ba57..120af8c1e61 100644 --- a/app/services/ci/pipeline_trigger_service.rb +++ b/app/services/ci/pipeline_trigger_service.rb @@ -14,7 +14,7 @@ module Ci pipeline = Ci::CreatePipelineService.new(project, trigger.owner, ref: params[:ref]) .execute(:trigger, ignore_skip_ci: true) do |pipeline| - trigger.trigger_requests.create!(pipeline: pipeline) + pipeline.trigger_requests.create!(trigger: trigger) create_pipeline_variables!(pipeline) 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/18308-escape-characters.yml b/changelogs/unreleased/18308-escape-characters.yml new file mode 100644 index 00000000000..8766e971490 --- /dev/null +++ b/changelogs/unreleased/18308-escape-characters.yml @@ -0,0 +1,5 @@ +--- +title: Escape quotes in git username +merge_request: 14020 +author: Brandon Everett +type: fixed 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/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml b/changelogs/unreleased/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml new file mode 100644 index 00000000000..8aae0f6f5b6 --- /dev/null +++ b/changelogs/unreleased/fix-sm-37559-pipeline-triggered-through-api-not-showing-trigger-variables.yml @@ -0,0 +1,6 @@ +--- +title: Fix Pipeline Triggers to show triggered label and predefined variables (e.g. + CI_PIPELINE_TRIGGERED) +merge_request: 14244 +author: +type: fixed diff --git a/changelogs/unreleased/refactor-monitoring-service.yml b/changelogs/unreleased/refactor-monitoring-service.yml new file mode 100644 index 00000000000..685397cadb8 --- /dev/null +++ b/changelogs/unreleased/refactor-monitoring-service.yml @@ -0,0 +1,5 @@ +--- +title: Perform prometheus data endpoint requests in parallel +merge_request: 14003 +author: +type: fixed 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/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/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/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb index 1437479831e..a76c75e0c08 100644 --- a/spec/helpers/projects_helper_spec.rb +++ b/spec/helpers/projects_helper_spec.rb @@ -469,4 +469,15 @@ describe ProjectsHelper do expect(recorder.count).to eq(1) end end + + describe '#git_user_name' do + let(:user) { double(:user, name: 'John "A" Doe53') } + before do + allow(helper).to receive(:current_user).and_return(user) + end + + it 'parses quotes in name' do + expect(helper.send(:git_user_name)).to eq('John \"A\" Doe53') + end + end end 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/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/pipeline_trigger_service_spec.rb b/spec/services/ci/pipeline_trigger_service_spec.rb index 9a6875e448c..f4ff818c479 100644 --- a/spec/services/ci/pipeline_trigger_service_spec.rb +++ b/spec/services/ci/pipeline_trigger_service_spec.rb @@ -34,6 +34,8 @@ describe Ci::PipelineTriggerService do expect(result[:pipeline].ref).to eq('master') expect(result[:pipeline].project).to eq(project) expect(result[:pipeline].user).to eq(trigger.owner) + expect(result[:pipeline].trigger_requests.to_a) + .to eq(result[:pipeline].builds.map(&:trigger_request).uniq) expect(result[:status]).to eq(:success) end 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 |