Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-12-11

# Conflicts:
#	doc/api/README.md

[ci skip]

app/assets/javascripts/notes/components/noteable_discussion.vue

@@ -413,7 +413,7 @@ Please check your network connection and try again.`;
                   <div class="discussion-with-resolve-btn">
                     <button
                       type="button"
-                      class="js-vue-discussion-reply btn btn-text-field mr-sm-2 qa-discussion-reply"
+                      class="js-vue-discussion-reply btn btn-text-field qa-discussion-reply"
                       title="Add a reply"
                       @click="showReplyForm"
                     >
@@ -422,7 +422,7 @@ Please check your network connection and try again.`;
                     <div v-if="discussion.resolvable">
                       <button
                         type="button"
-                        class="btn btn-default mr-sm-2"
+                        class="btn btn-default ml-sm-2"
                         @click="resolveHandler();"
                       >
                         <i v-if="isResolving" aria-hidden="true" class="fa fa-spinner fa-spin"></i>

app/assets/stylesheets/pages/note_form.scss

@@ -178,7 +178,7 @@
 
 table {
   .discussion-form-container {
-    padding: $gl-padding-top $gl-padding $gl-padding;
+    padding: $gl-padding;
   }
 }
 
@@ -237,11 +237,12 @@ table {
 }
 
 .discussion-body,
-.diff-file {
+.diff-file,
+.commit-diff {
   .discussion-reply-holder {
     background-color: $white-light;
-    padding: 10px 16px;
     border-radius: 0 0 3px 3px;
+    padding: $gl-padding;
 
     &.is-replying {
       padding-bottom: $gl-padding;
@@ -254,7 +255,6 @@ table {
     display: flex;
   }
 
-
   .discussion-actions {
     display: table;
 
@@ -275,9 +275,11 @@ table {
     }
   }
 
+  @include media-breakpoint-down(xs) {
     .btn {
       width: 100%;
     }
+  }
 
   .btn-text-field {
     @include media-breakpoint-down(xs) {

app/assets/stylesheets/pages/notes.scss

@@ -14,7 +14,7 @@ $note-form-margin-left: 72px;
 }
 
 @mixin outline-comment() {
-  margin: $gl-padding;
+  margin: $gl-padding $gl-padding 0;
   border: 1px solid $border-color;
   border-radius: $border-radius-default;
 }
@@ -27,8 +27,10 @@ $note-form-margin-left: 72px;
   }
 }
 
-.main-notes-list {
+.issuable-discussion {
+  .main-notes-list {
     @include vertical-line(36px);
+  }
 }
 
 .notes {
@@ -76,11 +78,11 @@ $note-form-margin-left: 72px;
     .card {
       border: 0;
     }
+  }
 
   li.note {
     border-bottom: 1px solid $border-color;
   }
-  }
 
   .replies-toggle {
     background-color: $gray-light;
@@ -161,20 +163,6 @@ $note-form-margin-left: 72px;
     position: relative;
     border-bottom: 0;
 
-    &:target,
-    &.target {
-      border-bottom: 1px solid $white-normal;
-
-      &:not(:first-child) {
-        border-top: 1px solid $white-normal;
-        margin-top: -1px;
-      }
-
-      .timeline-entry-inner {
-        border-bottom: 0;
-      }
-    }
-
     &.being-posted {
       pointer-events: none;
       opacity: 0.5;
@@ -462,7 +450,7 @@ $note-form-margin-left: 72px;
     font-family: $regular-font;
 
     td {
-      border: 1px solid $white-normal;
+      border: 1px solid $border-color;
       border-left: 0;
 
       &.notes_content {
@@ -504,8 +492,6 @@ $note-form-margin-left: 72px;
   }
 
   .note-wrapper {
-    @include outline-comment();
-
     &.system-note {
       border: 0;
       margin-left: 20px;
@@ -514,23 +500,14 @@ $note-form-margin-left: 72px;
 
   .discussion-reply-holder {
     border-radius: 0 0 $border-radius-default $border-radius-default;
-    border-top: 1px solid $border-color;
     position: relative;
   }
 }
 
 .commit-diff {
-  .notes {
-    @include vertical-line(52px);
-  }
-
   .notes_content {
     background-color: $white-light;
   }
-
-  .discussion-reply-holder {
-    border-top: 1px solid $border-color;
-  }
 }
 
 .discussion-header,
@@ -943,12 +920,6 @@ $note-form-margin-left: 72px;
     border-bottom: 1px solid $border-color;
   }
 
-  .note-wrapper.outlined {
-    margin: 0;
-    border: 0;
-    border-radius: 0;
-  }
-
   .discussion-form-container {
     padding: $gl-padding;
   }

app/models/ci/build.rb

@@ -744,7 +744,7 @@ module Ci
     def collect_test_reports!(test_reports)
       test_reports.get_suite(group_name).tap do |test_suite|
         each_report(Ci::JobArtifact::TEST_REPORT_FILE_TYPES) do |file_type, blob|
-          Gitlab::Ci::Parsers::Test.fabricate!(file_type).parse!(blob, test_suite)
+          Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_suite)
         end
       end
     end

app/models/merge_request.rb

@@ -1120,14 +1120,17 @@ class MergeRequest < ActiveRecord::Base
     end
   end
 
-  # rubocop: disable CodeReuse/ServiceClass
   def compare_test_reports
     unless has_test_reports?
       return { status: :error, status_reason: 'This merge request does not have test reports' }
     end
 
-    with_reactive_cache(:compare_test_results) do |data|
-      unless Ci::CompareTestReportsService.new(project)
+    compare_reports(Ci::CompareTestReportsService)
+  end
+
+  def compare_reports(service_class)
+    with_reactive_cache(service_class.name) do |data|
+      unless service_class.new(project)
         .latest?(base_pipeline, actual_head_pipeline, data)
         raise InvalidateReactiveCache
       end
@@ -1135,19 +1138,14 @@ class MergeRequest < ActiveRecord::Base
       data
     end || { status: :parsing }
   end
-  # rubocop: enable CodeReuse/ServiceClass
 
-  # rubocop: disable CodeReuse/ServiceClass
   def calculate_reactive_cache(identifier, *args)
-    case identifier.to_sym
-    when :compare_test_results
-      Ci::CompareTestReportsService.new(project).execute(
-        base_pipeline, actual_head_pipeline)
-    else
-      raise NotImplementedError, "Unknown identifier: #{identifier}"
-    end
+    service_class = identifier.constantize
+
+    raise NameError, service_class unless service_class < Ci::CompareReportsBaseService
+
+    service_class.new(project).execute(base_pipeline, actual_head_pipeline)
   end
-  # rubocop: enable CodeReuse/ServiceClass
 
   def all_commits
     # MySQL doesn't support LIMIT in a subquery.

app/services/ci/compare_reports_base_service.rb

+# frozen_string_literal: true
+
+module Ci
+  class CompareReportsBaseService < ::BaseService
+    def execute(base_pipeline, head_pipeline)
+      comparer = comparer_class.new(get_report(base_pipeline), get_report(head_pipeline))
+      {
+        status: :parsed,
+        key: key(base_pipeline, head_pipeline),
+        data: serializer_class
+          .new(project: project)
+          .represent(comparer).as_json
+      }
+    rescue Gitlab::Ci::Parsers::ParserError => e
+      {
+        status: :error,
+        key: key(base_pipeline, head_pipeline),
+        status_reason: e.message
+      }
+    end
+
+    def latest?(base_pipeline, head_pipeline, data)
+      data&.fetch(:key, nil) == key(base_pipeline, head_pipeline)
+    end
+
+    private
+
+    def key(base_pipeline, head_pipeline)
+      [
+        base_pipeline&.id, base_pipeline&.updated_at,
+        head_pipeline&.id, head_pipeline&.updated_at
+      ]
+    end
+
+    def comparer_class
+      raise NotImplementedError
+    end
+
+    def serializer_class
+      raise NotImplementedError
+    end
+
+    def get_report(pipeline)
+      raise NotImplementedError
+    end
+  end
+end

app/services/ci/compare_test_reports_service.rb

 # frozen_string_literal: true
 
 module Ci
-  class CompareTestReportsService < ::BaseService
-    def execute(base_pipeline, head_pipeline)
-      # rubocop: disable CodeReuse/Serializer
-      comparer = Gitlab::Ci::Reports::TestReportsComparer
-        .new(base_pipeline&.test_reports, head_pipeline.test_reports)
-
-      {
-        status: :parsed,
-        key: key(base_pipeline, head_pipeline),
-        data: TestReportsComparerSerializer
-          .new(project: project)
-          .represent(comparer).as_json
-      }
-    rescue => e
-      {
-        status: :error,
-        key: key(base_pipeline, head_pipeline),
-        status_reason: e.message
-      }
-      # rubocop: enable CodeReuse/Serializer
+  class CompareTestReportsService < CompareReportsBaseService
+    def comparer_class
+      Gitlab::Ci::Reports::TestReportsComparer
     end
 
-    def latest?(base_pipeline, head_pipeline, data)
-      data&.fetch(:key, nil) == key(base_pipeline, head_pipeline)
+    def serializer_class
+      TestReportsComparerSerializer
     end
 
-    private
-
-    def key(base_pipeline, head_pipeline)
-      [
-        base_pipeline&.id, base_pipeline&.updated_at,
-        head_pipeline&.id, head_pipeline&.updated_at
-      ]
+    def get_report(pipeline)
+      pipeline&.test_reports
     end
   end
 end

app/views/shared/notes/_note.html.haml

@@ -5,7 +5,7 @@
 - note_editable = can?(current_user, :admin_note, note)
 - note_counter = local_assigns.fetch(:note_counter, 0)
 
-%li.timeline-entry.note-wrapper.outlined{ id: dom_id(note),
+%li.timeline-entry.note-wrapper{ id: dom_id(note),
   class: ["note", "note-row-#{note.id}", ('system-note' if note.system)],
   data: { author_id: note.author.id,
     editable: note_editable,

changelogs/unreleased/gt-remove-unnecessary-line-before-reply-holder.yml

+---
+title: Remove unnecessary line before reply holder
+merge_request: 23092
+author: George Tsiolis
+type: changed

doc/administration/repository_storage_paths.md

@@ -11,6 +11,25 @@ storage load between several mount points.
 > - The paths are defined in key-value pairs. The key is an arbitrary name you
 >   can pick to name the file path.
 > - The target directories and any of its subpaths must not be a symlink.
+> - No target directory may be a sub-directory of another; no nesting.
+
+Example: this is OK:
+
+```
+default:
+  path: /mnt/git-storage-1
+storage2:
+  path: /mnt/git-storage-2
+```
+
+This is not OK because it nests storage paths:
+
+```
+default:
+  path: /mnt/git-storage-1
+storage2:
+  path: /mnt/git-storage-1/git-storage-2 # <- NOT OK because of nesting
+```
 
 ## Configure GitLab
 

doc/api/README.md

@@ -25,7 +25,10 @@ The following API resources are available:
 - [Epics](epics.md) **[ULTIMATE]**
 - [Events](events.md)
 - [Feature flags](features.md)
+<<<<<<< HEAD
 - [Geo Nodes](geo_nodes.md) **[PREMIUM]**
+=======
+>>>>>>> upstream/master
 - Group-related resources, including:
   - [Groups](groups.md)
   - [Group access requests](access_requests.md)
@@ -36,14 +39,20 @@ The following API resources are available:
   - [Group milestones](group_milestones.md)
 - [Issues](issues.md)
 - [Issue boards](boards.md)
+<<<<<<< HEAD
 - [Issue links](issue_links.md)
+=======
+>>>>>>> upstream/master
 - [Jobs](jobs.md)
 - [Keys](keys.md)
 - [Labels](labels.md)
 - [License](license.md)
 - [Managed licenses](managed_licenses.md) **[ULTIMATE]**
 - [Markdown](markdown.md)
+<<<<<<< HEAD
 - [Merge request approvals](merge_request_approvals.md) **[STARTER]**
+=======
+>>>>>>> upstream/master
 - [Merge requests](merge_requests.md)
 - [Namespaces](namespaces.md)
 - [Notes](notes.md) (comments)

doc/ci/caching/index.md

@@ -29,8 +29,8 @@ needed to compile the project:
   Cache was designed to be used to speed up invocations of subsequent runs of a
   given job, by keeping things like dependencies (e.g., npm packages, Go vendor
   packages, etc.) so they don't have to be re-fetched from the public internet.
-  While the cache can be abused to pass intermediate build results between stages,
-  there may be cases where artifacts are a better fit.
+  While the cache can be abused to pass intermediate build results between 
+  stages, there may be cases where artifacts are a better fit.
 - `artifacts`: **Use for stage results that will be passed between stages.**
   Artifacts were designed to upload some compiled/generated bits of the build,
   and they can be fetched by any number of concurrent Runners. They are
@@ -39,11 +39,13 @@ needed to compile the project:
   directories relative to the build directory** and specifying paths which don't
   comply to this rule trigger an unintuitive and illogical error message (an
   enhancement is discussed at
-  https://gitlab.com/gitlab-org/gitlab-ce/issues/15530). Artifacts need to be
-  uploaded to the GitLab instance (not only the GitLab runner) before the next
-  stage job(s) can start, so you need to evaluate carefully whether your
-  bandwidth allows you to profit from parallelization with stages and shared
-  artifacts before investing time in changes to the setup.
+  [https://gitlab.com/gitlab-org/gitlab-ce/issues/15530](https://gitlab.com/gitlab-org/gitlab-ce/issues/15530)
+  ). Artifacts need to be uploaded to the GitLab instance (not only the GitLab 
+  runner) before the next stage job(s) can start, so you need to evaluate 
+  carefully whether your bandwidth allows you to profit from parallelization 
+  with stages and shared artifacts before investing time in changes to the 
+  setup.
+
 
 It's sometimes confusing because the name artifact sounds like something that
 is only useful outside of the job, like for downloading a final image. But

doc/ci/yaml/README.md

@@ -571,7 +571,7 @@ osx job:
 
 `allow_failure` is used when you want to allow a job to fail without impacting
 the rest of the CI suite. Failed jobs don't contribute to the commit status.
-The default value is `false`.
+The default value is `false`, except for [manual](#whenmanual) jobs.
 
 When enabled and the job fails, the pipeline will be successful/green for all
 intents and purposes, but a "CI build passed with warnings" message  will be

doc/user/project/issues/index.md

@@ -166,3 +166,7 @@ Read through the [API documentation](../../../api/issues.md).
 ### Bulk editing issues
 
 Find out about [bulk editing issues](../../project/bulk_editing.md).
+
+### Similar issues
+
+Find out about [similar issues](similar_issues.md).

doc/user/project/issues/similar_issues.md

+# Similar issues
+
+> [Introduced][ce-22866] in GitLab 11.6.
+
+Similar issues suggests issues that are similar when new issues are being created.
+This features requires [GraphQL] to be enabled.
+
+![Similar issues](img/similar_issues.png)
+
+You can see the similar issues when typing in the title in the new issue form.
+This searches both titles and descriptions across all issues the user has access
+to in the current project. It then displays the first 5 issues sorted by most
+recently updated.
+
+[GraphQL]: ../../../api/graphql/index.md
+[ce-22866]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22866

lib/gitlab/ci/parsers/test.rb → lib/gitlab/ci/parsers.rb

@@ -3,19 +3,19 @@
 module Gitlab
   module Ci
     module Parsers
-      module Test
-        ParserNotFoundError = Class.new(StandardError)
+      ParserNotFoundError = Class.new(ParserError)
 
-        PARSERS = {
+      def self.parsers
+        {
           junit: ::Gitlab::Ci::Parsers::Test::Junit
-        }.freeze
+        }
+      end
 
       def self.fabricate!(file_type)
-          PARSERS.fetch(file_type.to_sym).new
+        parsers.fetch(file_type.to_sym).new
       rescue KeyError
         raise ParserNotFoundError, "Cannot find any parser matching file type '#{file_type}'"
       end
     end
   end
-  end
 end

lib/gitlab/ci/parsers/parser_error.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Ci
+    module Parsers
+      ParserError = Class.new(StandardError)
+    end
+  end
+end

lib/gitlab/ci/parsers/test/junit.rb

@@ -5,7 +5,7 @@ module Gitlab
     module Parsers
       module Test
         class Junit
-          JunitParserError = Class.new(StandardError)
+          JunitParserError = Class.new(Gitlab::Ci::Parsers::ParserError)
 
           def parse!(xml_data, test_suite)
             root = Hash.from_xml(xml_data)

spec/lib/gitlab/ci/parsers/test_spec.rb → spec/lib/gitlab/ci/parsers_spec.rb

+# frozen_string_literal: true
+
 require 'spec_helper'
 
-describe Gitlab::Ci::Parsers::Test do
+describe Gitlab::Ci::Parsers do
   describe '.fabricate!' do
     subject { described_class.fabricate!(file_type) }
 
@@ -8,7 +10,7 @@ describe Gitlab::Ci::Parsers::Test do
       let(:file_type) { 'junit' }
 
       it 'fabricates the class' do
-        is_expected.to be_a(described_class::Junit)
+        is_expected.to be_a(described_class::Test::Junit)
       end
     end
 
@@ -16,7 +18,7 @@ describe Gitlab::Ci::Parsers::Test do
       let(:file_type) { 'undefined' }
 
       it 'raises an error' do
-        expect { subject }.to raise_error(Gitlab::Ci::Parsers::Test::ParserNotFoundError)
+        expect { subject }.to raise_error(Gitlab::Ci::Parsers::ParserNotFoundError)
       end
     end
   end

spec/models/merge_request_spec.rb

@@ -1470,6 +1470,30 @@ describe MergeRequest do
     end
   end
 
+  describe '#calculate_reactive_cache' do
+    let(:project) { create(:project, :repository) }
+    let(:merge_request) { create(:merge_request, source_project: project) }
+    subject { merge_request.calculate_reactive_cache(service_class_name) }
+
+    context 'when given an unknown service class name' do
+      let(:service_class_name) { 'Integer' }
+
+      it 'raises a NameError exception' do
+        expect { subject }.to raise_error(NameError, service_class_name)
+      end
+    end
+
+    context 'when given a known service class name' do
+      let(:service_class_name) { 'Ci::CompareTestReportsService' }
+
+      it 'does not raises a NameError exception' do
+        allow_any_instance_of(service_class_name.constantize).to receive(:execute).and_return(nil)
+
+        expect { subject }.not_to raise_error(NameError)
+      end
+    end
+  end
+
   describe '#compare_test_reports' do
     subject { merge_request.compare_test_reports }