diff options
author | Mike Greiling <mike@pixelcog.com> | 2019-01-24 21:54:04 +0000 |
---|---|---|
committer | Mike Greiling <mike@pixelcog.com> | 2019-01-24 21:54:04 +0000 |
commit | d6239b88b25e64aefaee0b272f082f24c5a97881 (patch) | |
tree | c13d6c699e8eafb157bc3d40772abb32383278e2 | |
parent | 99e363debddbc75e1bc401dcecd41ea748b001e0 (diff) | |
parent | a8dcc181da5fdd564360cd6291522e89488def3e (diff) | |
download | gitlab-ce-d6239b88b25e64aefaee0b272f082f24c5a97881.tar.gz |
Merge branch '53950-commit-comments-displayed-on-a-merge-request' into 'master'56221-spec-features-projects-clusters-gcp_spec-rb-appears-to-be-making-real-google-api-requests
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
-rw-r--r-- | app/assets/javascripts/notes/components/noteable_discussion.vue | 11 | ||||
-rw-r--r-- | app/assets/javascripts/notes/components/noteable_note.vue | 33 | ||||
-rw-r--r-- | app/helpers/notes_helper.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/53950-commit-comments-displayed-on-a-merge-request.yml | 5 | ||||
-rw-r--r-- | locale/gitlab.pot | 3 | ||||
-rw-r--r-- | spec/features/merge_request/user_sees_discussions_spec.rb | 12 | ||||
-rw-r--r-- | spec/helpers/notes_helper_spec.rb | 4 |
7 files changed, 60 insertions, 10 deletions
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 1a116161e3c..1a9723de856 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -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" > diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 4c02588127e..3c48d81ed05 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -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">·</span> + </note-header> <note-actions :author-id="author.id" :note-id="note.id" diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 033686823a2..293dd20ad49 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -85,7 +85,7 @@ module NotesHelper 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 diff --git a/changelogs/unreleased/53950-commit-comments-displayed-on-a-merge-request.yml b/changelogs/unreleased/53950-commit-comments-displayed-on-a-merge-request.yml new file mode 100644 index 00000000000..adaaed7f1aa --- /dev/null +++ b/changelogs/unreleased/53950-commit-comments-displayed-on-a-merge-request.yml @@ -0,0 +1,5 @@ +--- +title: Display "commented" only for commit discussions on merge requests +merge_request: 24427 +author: +type: changed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index f0316234dc6..0ff03e7fb2b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -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 "" diff --git a/spec/features/merge_request/user_sees_discussions_spec.rb b/spec/features/merge_request/user_sees_discussions_spec.rb index 4ab9a87ad4b..57be1d06708 100644 --- a/spec/features/merge_request/user_sees_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_discussions_spec.rb @@ -88,5 +88,17 @@ describe 'Merge request > User sees discussions', :js do 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 diff --git a/spec/helpers/notes_helper_spec.rb b/spec/helpers/notes_helper_spec.rb index 21461e46cf4..0715f34dafe 100644 --- a/spec/helpers/notes_helper_spec.rb +++ b/spec/helpers/notes_helper_spec.rb @@ -185,8 +185,8 @@ describe NotesHelper do 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 |