Merge branch 'ce-to-ee-2018-11-29' into 'master'

CE upstream - 2018-11-29 19:21 UTC

See merge request gitlab-org/gitlab-ee!8645

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

 <script>
+import _ from 'underscore';
 import { mapActions, mapGetters } from 'vuex';
 import { GlTooltipDirective } from '@gitlab/ui';
 import { truncateSha } from '~/lib/utils/text_utility';
-import { s__, __ } from '~/locale';
+import { s__, __, sprintf } from '~/locale';
 import systemNote from '~/vue_shared/components/notes/system_note.vue';
 import icon from '~/vue_shared/components/icon.vue';
 import batchCommentsDiffLineNoteFormMixin from 'ee/batch_comments/mixins/diff_line_note_form';
@@ -166,6 +167,37 @@ export default {
         (!discussion.diff_discussion && resolved && hasReplies && !isRepliesToggledByUser) || false
       );
     },
+    actionText() {
+      const commitId = this.discussion.commit_id ? truncateSha(this.discussion.commit_id) : '';
+      const linkStart = `<a href="${_.escape(this.discussion.discussion_path)}">`;
+      const linkEnd = '</a>';
+
+      let text = s__('MergeRequests|started a discussion');
+
+      if (this.discussion.for_commit) {
+        text = s__(
+          'MergeRequests|started a discussion on commit %{linkStart}%{commitId}%{linkEnd}',
+        );
+      } else if (this.discussion.diff_discussion) {
+        if (this.discussion.active) {
+          text = s__('MergeRequests|started a discussion on %{linkStart}the diff%{linkEnd}');
+        } else {
+          text = s__(
+            'MergeRequests|started a discussion on %{linkStart}an old version of the diff%{linkEnd}',
+          );
+        }
+      }
+
+      return sprintf(
+        text,
+        {
+          commitId,
+          linkStart,
+          linkEnd,
+        },
+        false,
+      );
+    },
   },
   watch: {
     isReplying() {
@@ -301,24 +333,7 @@ Please check your network connection and try again.`;
               :expanded="discussion.expanded"
               @toggleHandler="toggleDiscussionHandler"
             >
-              <template v-if="discussion.diff_discussion">
-                started a discussion on
-                <a :href="discussion.discussion_path">
-                  <template v-if="discussion.active"
-                    >the diff</template
-                  >
-                  <template v-else
-                    >an old version of the diff</template
-                  >
-                </a>
-              </template>
-              <template v-else-if="discussion.for_commit">
-                started a discussion on commit
-                <a :href="discussion.discussion_path">{{ truncateSha(discussion.commit_id) }}</a>
-              </template>
-              <template v-else
-                >started a discussion</template
-              >
+              <span v-html="actionText"></span>
             </note-header>
             <note-edited-text
               v-if="discussion.resolved"

app/assets/stylesheets/framework/variables.scss

@@ -244,6 +244,7 @@ $gl-padding-top: 10px;
 $gl-sidebar-padding: 22px;
 $gl-bar-padding: 3px;
 $input-horizontal-padding: 12px;
+$browserScrollbarSize: 10px;
 
 /*
  * Misc

app/assets/stylesheets/pages/projects.scss

@@ -724,7 +724,8 @@
   .scrolling-tabs-container {
     .scrolling-tabs {
       margin-top: $gl-padding-8;
-      margin-bottom: $gl-padding-8;
+      margin-bottom: $gl-padding-8 - $browserScrollbarSize;
+      padding-bottom: $browserScrollbarSize;
       flex-wrap: wrap;
       border-bottom: 0;
     }
@@ -732,7 +733,7 @@
     .fade-left,
     .fade-right {
       top: 0;
-      height: 100%;
+      height: calc(100% - #{$browserScrollbarSize});
 
       .fa {
         top: 50%;

changelogs/unreleased/non-webkit-scrollbar-fixing.yml

+---
+title: Fix horizontal scrollbar overlapping on horizontal scrolling-tabs
+merge_request: 23167
+author: Harry Kiselev
+type: other

changelogs/unreleased/winh-merge-request-commit-discussion.yml

+---
+title: Display commit ID for commit diff discussion on merge request
+merge_request: 23370
+author:
+type: fixed

locale/gitlab.pot

@@ -5247,6 +5247,18 @@ msgstr ""
 msgid "MergeRequests|View replaced file @ %{commitId}"
 msgstr ""
 
+msgid "MergeRequests|started a discussion"
+msgstr ""
+
+msgid "MergeRequests|started a discussion on %{linkStart}an old version of the diff%{linkEnd}"
+msgstr ""
+
+msgid "MergeRequests|started a discussion on %{linkStart}the diff%{linkEnd}"
+msgstr ""
+
+msgid "MergeRequests|started a discussion on commit %{linkStart}%{commitId}%{linkEnd}"
+msgstr ""
+
 msgid "MergeRequest| %{paragraphStart}changed the description %{descriptionChangedTimes} times %{timeDifferenceMinutes}%{paragraphEnd}"
 msgstr ""
 

scripts/trigger-build

@@ -68,6 +68,7 @@ module Trigger
 
     def base_variables
       {
+        'GITLAB_REF_SLUG' => ref_slug,
         'TRIGGERED_USER' => ENV['TRIGGERED_USER'] || ENV['GITLAB_USER_NAME'],
         'TRIGGER_SOURCE' => ENV['CI_JOB_URL'],
         'TOP_UPSTREAM_SOURCE_PROJECT' => ENV['CI_PROJECT_PATH'],
@@ -76,6 +77,12 @@ module Trigger
       }
     end
 
+    def ref_slug
+      return 'master' if ENV['CI_COMMIT_REF_SLUG'] =~ %r{(\Aqa[/-]|-qa\z)}
+
+      ENV['CI_COMMIT_REF_SLUG']
+    end
+
     # Read version files from all components
     def version_file_variables
       Dir.glob("*_VERSION").each_with_object({}) do |version_file, params|
@@ -106,18 +113,11 @@ module Trigger
     def extra_variables
       {
         'GITLAB_VERSION' => ENV['CI_COMMIT_SHA'],
-        'GITLAB_REF_SLUG' => ref_slug,
         'ALTERNATIVE_SOURCES' => 'true',
         'ee' => Trigger.ee? ? 'true' : 'false',
         'QA_BRANCH' => ENV['QA_BRANCH'] || 'master'
       }
     end
-
-    def ref_slug
-      return 'master' if ENV['CI_COMMIT_REF_SLUG'] =~ %r{(\Aqa[/-]|-qa\z)}
-
-      ENV['CI_COMMIT_REF_SLUG']
-    end
   end
 
   class CNG < Base

spec/features/merge_request/user_sees_discussions_spec.rb

@@ -53,13 +53,11 @@ describe 'Merge request > User sees discussions', :js do
     shared_examples 'a functional discussion' do
       let(:discussion_id) { note.discussion_id(merge_request) }
 
-      # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034
-      xit 'is displayed' do
+      it 'is displayed' do
         expect(page).to have_css(".discussion[data-discussion-id='#{discussion_id}']")
       end
 
-      # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034
-      xit 'can be replied to' do
+      it 'can be replied to' do
         within(".discussion[data-discussion-id='#{discussion_id}']") do
           click_button 'Reply...'
           fill_in 'note[note]', with: 'Test!'
@@ -74,16 +72,21 @@ describe 'Merge request > User sees discussions', :js do
       visit project_merge_request_path(project, merge_request)
     end
 
-    context 'a regular commit comment' do
-      let(:note) { create(:note_on_commit, project: project) }
-
-      it_behaves_like 'a functional discussion'
-    end
+    # TODO: https://gitlab.com/gitlab-org/gitlab-ce/issues/48034
+    # context 'a regular commit comment' do
+    #   let(:note) { create(:note_on_commit, project: project) }
+    #
+    #   it_behaves_like 'a functional discussion'
+    # end
 
     context 'a commit diff comment' do
       let(:note) { create(:diff_note_on_commit, project: project) }
 
       it_behaves_like 'a functional discussion'
+
+      it 'displays correct header' do
+        expect(page).to have_content "started a discussion on commit #{note.commit_id[0...7]}"
+      end
     end
   end
 end