Merge branch 'winh-merge-request-commit-discussion' into 'master'

Display commit ID for commit diff discussion on merge request

See merge request gitlab-org/gitlab-ce!23370

4 files changed, 63 insertions, 28 deletions

diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue
index 36388e4b3cf..da78c6e21a1 100644
--- a/app/assets/javascripts/notes/components/noteable_discussion.vue
+++ b/app/assets/javascripts/notes/components/noteable_discussion.vue
@@ -1,8 +1,9 @@
 <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 Flash from '../../flash';
@@ -156,6 +157,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() {
@@ -291,24 +323,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"
diff --git a/changelogs/unreleased/winh-merge-request-commit-discussion.yml b/changelogs/unreleased/winh-merge-request-commit-discussion.yml
new file mode 100644
index 00000000000..b0c6264369b
--- /dev/null
+++ b/changelogs/unreleased/winh-merge-request-commit-discussion.yml
@@ -0,0 +1,5 @@
+---
+title: Display commit ID for commit diff discussion on merge request
+merge_request: 23370
+author:
+type: fixed
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 5672df5f965..076996e2231 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -4016,6 +4016,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 ""
 
diff --git a/spec/features/merge_request/user_sees_discussions_spec.rb b/spec/features/merge_request/user_sees_discussions_spec.rb
index 7b8c3bacfe2..4ab9a87ad4b 100644
--- a/spec/features/merge_request/user_sees_discussions_spec.rb
+++ b/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