From c1c0c0c21b075ee9ee2eddb75ad9681f29765bb2 Mon Sep 17 00:00:00 2001 From: Constance Okoghenun Date: Thu, 17 Jan 2019 12:15:33 +0100 Subject: Display "commented" only for commit discussions on merge requests Add commit prop to NoteableNote component and pass it from NoteableDiscussion --- .../notes/components/noteable_discussion.vue | 11 ++++++++ .../javascripts/notes/components/noteable_note.vue | 30 ++++++++++++++++++++-- app/helpers/notes_helper.rb | 2 +- .../merge_request/user_sees_discussions_spec.rb | 10 ++++++++ spec/helpers/notes_helper_spec.rb | 4 +-- 5 files changed, 52 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index 4480ec74182..aeb7171068a 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -212,6 +212,16 @@ export default { return this.line; }, + commit() { + if (!this.discussion.for_commit) { + return null; + } + + return { + id: this.discussion.commit_id, + url: this.discussion.discussion_path, + }; + }, }, watch: { isReplying() { @@ -376,6 +386,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..c39dcc71aa6 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -2,6 +2,8 @@ import $ from 'jquery'; import { mapGetters, mapActions } from 'vuex'; import { escape } from 'underscore'; +import { truncateSha } from '~/lib/utils/text_utility'; +import { s__, sprintf } from '~/locale'; import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; import Flash from '../../flash'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; @@ -37,6 +39,11 @@ export default { required: false, default: '', }, + commit: { + type: Object, + required: false, + default: () => null, + }, }, data() { return { @@ -73,6 +80,24 @@ export default { isTarget() { return this.targetNoteHash === this.noteAnchorId; }, + actionText() { + if (this.commit) { + const { id, url } = this.commit; + const linkStart = ``; + const linkEnd = ''; + return sprintf( + s__('MergeRequests|commented on commit %{linkStart}%{commitId}%{linkEnd}'), + { + commitId: truncateSha(id), + linkStart, + linkEnd, + }, + false, + ); + } + + return '·'; + }, }, created() { @@ -205,8 +230,9 @@ export default { :author="author" :created-at="note.created_at" :note-id="note.id" - action-text="commented" - /> + > + + 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 + 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 -- cgit v1.2.1 From b3d748b8ad77855839737e99241d05b5007b6d70 Mon Sep 17 00:00:00 2001 From: Constance Okoghenun Date: Fri, 18 Jan 2019 21:23:47 +0100 Subject: Added reloading to commit non-diff discussion note spec --- spec/features/merge_request/user_sees_discussions_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/features/merge_request/user_sees_discussions_spec.rb b/spec/features/merge_request/user_sees_discussions_spec.rb index 409dd41405b..d2b909aee11 100644 --- a/spec/features/merge_request/user_sees_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_discussions_spec.rb @@ -94,6 +94,8 @@ describe 'Merge request > User sees discussions', :js do 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 -- cgit v1.2.1 From 682a43932d084e9f920412899b38ccc64c7cbd29 Mon Sep 17 00:00:00 2001 From: Constance Okoghenun Date: Fri, 18 Jan 2019 22:00:17 +0100 Subject: Refactored implementation of discussion note header - Removed use of v-html - Removed HTML content from the computed property --- .../javascripts/notes/components/noteable_note.vue | 33 +++++++--------------- 1 file changed, 10 insertions(+), 23 deletions(-) diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index c39dcc71aa6..ba2c967de4a 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -3,7 +3,6 @@ import $ from 'jquery'; import { mapGetters, mapActions } from 'vuex'; import { escape } from 'underscore'; import { truncateSha } from '~/lib/utils/text_utility'; -import { s__, sprintf } from '~/locale'; import TimelineEntryItem from '~/vue_shared/components/notes/timeline_entry_item.vue'; import Flash from '../../flash'; import userAvatarLink from '../../vue_shared/components/user_avatar/user_avatar_link.vue'; @@ -80,23 +79,12 @@ export default { isTarget() { return this.targetNoteHash === this.noteAnchorId; }, - actionText() { - if (this.commit) { - const { id, url } = this.commit; - const linkStart = ``; - const linkEnd = ''; - return sprintf( - s__('MergeRequests|commented on commit %{linkStart}%{commitId}%{linkEnd}'), - { - commitId: truncateSha(id), - linkStart, - linkEnd, - }, - false, - ); + truncatedHash() { + if (!this.commit) { + return null; } - return '·'; + return truncateSha(this.commit.id); }, }, @@ -225,13 +213,12 @@ export default {
- - + + + {{ s__('MergeRequests|commented on commit ') + }}{{ truncatedHash }} + + · Date: Mon, 21 Jan 2019 05:46:27 +0100 Subject: Resolve commit comments displayed on a merge request - Added missing i18n strings - Added changelog entry - Fixed lint errors --- .../53950-commit-comments-displayed-on-a-merge-request.yml | 5 +++++ locale/gitlab.pot | 3 +++ spec/features/merge_request/user_sees_discussions_spec.rb | 2 +- 3 files changed, 9 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/53950-commit-comments-displayed-on-a-merge-request.yml 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 404cb079371..137050e1a36 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4294,6 +4294,9 @@ msgstr "" msgid "MergeRequests|View replaced file @ %{commitId}" msgstr "" +msgid "MergeRequests|commented on commit " +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 d2b909aee11..57be1d06708 100644 --- a/spec/features/merge_request/user_sees_discussions_spec.rb +++ b/spec/features/merge_request/user_sees_discussions_spec.rb @@ -91,7 +91,7 @@ describe 'Merge request > User sees discussions', :js do 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. -- cgit v1.2.1 From 993616a6c47c4e3066072c27f87625ce9eb0cb06 Mon Sep 17 00:00:00 2001 From: Constance Okoghenun Date: Tue, 22 Jan 2019 13:05:23 +0100 Subject: Added i18n to discussion note commit SHA --- app/assets/javascripts/notes/components/noteable_note.vue | 5 +++-- locale/gitlab.pot | 3 +++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index ba2c967de4a..30d8f2c091e 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -4,6 +4,7 @@ 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'; @@ -81,10 +82,10 @@ export default { }, truncatedHash() { if (!this.commit) { - return null; + return ''; } - return truncateSha(this.commit.id); + return sprintf(s__('MergeRequests|%{commitSha}'), { commitSha: truncateSha(this.commit.id) }); }, }, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 137050e1a36..a8d8890d033 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4279,6 +4279,9 @@ msgstr "" msgid "Merge requests are a place to propose changes you've made to a project and discuss those changes with others" msgstr "" +msgid "MergeRequests|%{commitSha}" +msgstr "" + msgid "MergeRequests|Resolve this discussion in a new issue" msgstr "" -- cgit v1.2.1 From 8322495027182c5119771f0733b4a0dffb4b2fde Mon Sep 17 00:00:00 2001 From: Constance Okoghenun Date: Wed, 23 Jan 2019 07:14:21 +0100 Subject: Updated i18n for discussion note commit SHA Ensured that sentence order is preserved when translating --- .../javascripts/notes/components/noteable_note.vue | 24 ++++++++++++++++------ locale/gitlab.pot | 5 +---- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 30d8f2c091e..0fd5d561387 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -80,12 +80,27 @@ export default { isTarget() { return this.targetNoteHash === this.noteAnchorId; }, - truncatedHash() { + actionText() { if (!this.commit) { return ''; } - return sprintf(s__('MergeRequests|%{commitSha}'), { commitSha: truncateSha(this.commit.id) }); + // 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 linkStart = ``; + const linkEnd = ''; + return sprintf( + s__('MergeRequests|commented on commit %{linkStart}%{commitId}%{linkEnd}'), + { + commitId: truncateSha(id), + linkStart, + linkEnd, + }, + false, + ); }, }, @@ -215,10 +230,7 @@ export default {
- - {{ s__('MergeRequests|commented on commit ') - }}{{ truncatedHash }} - + · Date: Thu, 24 Jan 2019 19:48:41 +0100 Subject: Updated i18n format for note header commit link Unified commit link in note header action text --- app/assets/javascripts/notes/components/noteable_note.vue | 15 ++++----------- locale/gitlab.pot | 2 +- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/app/assets/javascripts/notes/components/noteable_note.vue b/app/assets/javascripts/notes/components/noteable_note.vue index 0fd5d561387..3c48d81ed05 100644 --- a/app/assets/javascripts/notes/components/noteable_note.vue +++ b/app/assets/javascripts/notes/components/noteable_note.vue @@ -90,17 +90,10 @@ export default { // language to the next. See: // https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/24427#note_133713771 const { id, url } = this.commit; - const linkStart = ``; - const linkEnd = ''; - return sprintf( - s__('MergeRequests|commented on commit %{linkStart}%{commitId}%{linkEnd}'), - { - commitId: truncateSha(id), - linkStart, - linkEnd, - }, - false, - ); + const commitLink = `${truncateSha( + id, + )}`; + return sprintf(s__('MergeRequests|commented on commit %{commitLink}'), { commitLink }, false); }, }, diff --git a/locale/gitlab.pot b/locale/gitlab.pot index c471fd2396f..dc897098bc7 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4294,7 +4294,7 @@ msgstr "" msgid "MergeRequests|View replaced file @ %{commitId}" msgstr "" -msgid "MergeRequests|commented on commit %{linkStart}%{commitId}%{linkEnd}" +msgid "MergeRequests|commented on commit %{commitLink}" msgstr "" msgid "MergeRequests|started a discussion" -- cgit v1.2.1