Commit ede6eef1 authored by Mike Greiling's avatar Mike Greiling
Browse files

Merge branch '53950-commit-comments-displayed-on-a-merge-request' into 'master'

Resolve "Commit comments displayed on a merge request that contains that commit no longer specify which commit they come from"

Closes #53950

See merge request gitlab-org/gitlab-ce!24427
......@@ -216,6 +216,16 @@ export default {
return null;
},
commit() {
if (!this.discussion.for_commit) {
return null;
}
return {
id: this.discussion.commit_id,
url: this.discussion.discussion_path,
};
},
},
watch: {
isReplying() {
......@@ -380,6 +390,7 @@ Please check your network connection and try again.`;
:is="componentName(initialDiscussion)"
:note="componentData(initialDiscussion)"
:line="line"
:commit="commit"
:help-page-path="helpPagePath"
@handleDeleteNote="deleteNoteHandler"
>
......
......@@ -2,7 +2,9 @@
import $ from 'jquery';
import { mapGetters, mapActions } from 'vuex';
import { escape } from 'underscore';
import { truncateSha } from '~/lib/utils/text_utility';
import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue';
import { s__, sprintf } from '../../locale';
import Flash from '../../flash';
import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue';
import noteHeader from './note_header.vue';
......@@ -37,6 +39,11 @@ export default {
required: false,
default: '',
},
commit: {
type: Object,
required: false,
default: () => null,
},
},
data() {
return {
......@@ -73,6 +80,21 @@ export default {
isTarget() {
return this.targetNoteHash === this.noteAnchorId;
},
actionText() {
if (!this.commit) {
return '';
}
// We need to do this to ensure we have the currect sentence order
// when translating this as the sentence order may change from one
// language to the next. See:
// https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24427#note_133713771
const { id, url } = this.commit;
const commitLink = `<a class="commit-sha monospace" href="${escape(url)}">${truncateSha(
id,
)}</a>`;
return sprintf(s__('MergeRequests|commented on commit %{commitLink}'), { commitLink }, false);
},
},
created() {
......@@ -200,13 +222,10 @@ export default {
</div>
<div class="timeline-content">
<div class="note-header">
<note-header
v-once
:author="author"
:created-at="note.created_at"
:note-id="note.id"
action-text="commented"
/>
<note-header v-once :author="author" :created-at="note.created_at" :note-id="note.id">
<span v-if="commit" v-html="actionText"></span>
<span v-else class="d-none d-sm-inline">&middot;</span>
</note-header>
<note-actions
:author-id="author.id"
:note-id="note.id"
......
......@@ -85,7 +85,7 @@ def discussion_path(discussion)
diffs_project_merge_request_path(discussion.project, discussion.noteable, path_params)
elsif discussion.for_commit?
anchor = discussion.line_code if discussion.diff_discussion?
anchor = discussion.diff_discussion? ? discussion.line_code : "note_#{discussion.first_note.id}"
project_commit_path(discussion.project, discussion.noteable, anchor: anchor)
end
......
---
title: Display "commented" only for commit discussions on merge requests
merge_request: 24427
author:
type: changed
......@@ -4303,6 +4303,9 @@ msgstr ""
msgid "MergeRequests|View replaced file @ %{commitId}"
msgstr ""
msgid "MergeRequests|commented on commit %{commitLink}"
msgstr ""
msgid "MergeRequests|started a discussion"
msgstr ""
......
......@@ -88,5 +88,17 @@
expect(page).to have_content "started a discussion on commit #{note.commit_id[0...7]}"
end
end
context 'a commit non-diff discussion' do
let(:note) { create(:discussion_note_on_commit, project: project) }
it 'displays correct header' do
page.within(find("#note_#{note.id}", match: :first)) do
refresh # Trigger a refresh of notes.
wait_for_requests
expect(page).to have_content "commented on commit #{note.commit_id[0...7]}"
end
end
end
end
end
......@@ -185,8 +185,8 @@
context 'for a non-diff discussion' do
let(:discussion) { create(:discussion_note_on_commit, project: project).to_discussion }
it 'returns the commit path' do
expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit))
it 'returns the commit path with the note anchor' do
expect(helper.discussion_path(discussion)).to eq(project_commit_path(project, commit, anchor: "note_#{discussion.first_note.id}"))
end
end
end
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment