diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index d9dd08a7a6b..7c3f5d00308 100644 --- a/app/assets/javascripts/notes/components/noteable_discussion.vue +++ b/app/assets/javascripts/notes/components/noteable_discussion.vue @@ -178,31 +178,32 @@ export default { commitId = `${truncateSha(commitId)}`; } - let text = s__('MergeRequests|started a discussion'); + const { + for_commit: isForCommit, + diff_discussion: isDiffDiscussion, + active: isActive, + } = this.discussion; - if (this.discussion.for_commit) { + let text = s__('MergeRequests|started a discussion'); + if (isForCommit) { 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}', - ); - } + } else if (isDiffDiscussion && commitId) { + text = isActive + ? s__('MergeRequests|started a discussion on commit %{linkStart}%{commitId}%{linkEnd}') + : s__( + 'MergeRequests|started a discussion on an outdated change in commit %{linkStart}%{commitId}%{linkEnd}', + ); + } else if (isDiffDiscussion) { + text = isActive + ? s__('MergeRequests|started a discussion on %{linkStart}the diff%{linkEnd}') + : s__( + 'MergeRequests|started a discussion on %{linkStart}an old version of the diff%{linkEnd}', + ); } - return sprintf( - text, - { - commitId, - linkStart, - linkEnd, - }, - false, - ); + return sprintf(text, { commitId, linkStart, linkEnd }, false); }, diffLine() { if (this.discussion.diff_discussion && this.discussion.truncated_diff_lines) { diff --git a/changelogs/unreleased/winh-merge-request-commit-context.yml b/changelogs/unreleased/winh-merge-request-commit-context.yml new file mode 100644 index 00000000000..9e12a926af4 --- /dev/null +++ b/changelogs/unreleased/winh-merge-request-commit-context.yml @@ -0,0 +1,5 @@ +--- +title: Display commit ID for discussions made on merge request commits +merge_request: 23837 +author: +type: fixed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fed490309cc..260f581446b 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4135,6 +4135,9 @@ msgstr "" msgid "MergeRequests|started a discussion on %{linkStart}the diff%{linkEnd}" msgstr "" +msgid "MergeRequests|started a discussion on an outdated change in commit %{linkStart}%{commitId}%{linkEnd}" +msgstr "" + msgid "MergeRequests|started a discussion on commit %{linkStart}%{commitId}%{linkEnd}" msgstr "" diff --git a/spec/javascripts/notes/components/noteable_discussion_spec.js b/spec/javascripts/notes/components/noteable_discussion_spec.js index 106a4ac2546..3aff2dd0641 100644 --- a/spec/javascripts/notes/components/noteable_discussion_spec.js +++ b/spec/javascripts/notes/components/noteable_discussion_spec.js @@ -133,8 +133,10 @@ describe('noteable_discussion component', () => { }); }); - describe('commit discussion', () => { + describe('action text', () => { const commitId = 'razupaltuff'; + const truncatedCommitId = commitId.substr(0, 8); + let commitElement; beforeEach(() => { vm.$destroy(); @@ -143,7 +145,6 @@ describe('noteable_discussion component', () => { projectPath: 'something', }; - vm.$destroy(); vm = new Component({ propsData: { discussion: { @@ -159,17 +160,73 @@ describe('noteable_discussion component', () => { }, store, }).$mount(); + + commitElement = vm.$el.querySelector('.commit-sha'); }); - it('displays a monospace started a discussion on commit', () => { - const truncatedCommitId = commitId.substr(0, 8); + describe('for commit discussions', () => { + it('should display a monospace started a discussion on commit', () => { + expect(vm.$el).toContainText(`started a discussion on commit ${truncatedCommitId}`); + expect(commitElement).not.toBe(null); + expect(commitElement).toHaveText(truncatedCommitId); + }); + }); - expect(vm.$el).toContainText(`started a discussion on commit ${truncatedCommitId}`); + describe('for diff discussion with a commit id', () => { + it('should display started discussion on commit header', done => { + vm.discussion.for_commit = false; - const commitElement = vm.$el.querySelector('.commit-sha'); + vm.$nextTick(() => { + expect(vm.$el).toContainText(`started a discussion on commit ${truncatedCommitId}`); + expect(commitElement).not.toBe(null); - expect(commitElement).not.toBe(null); - expect(commitElement).toHaveText(truncatedCommitId); + done(); + }); + }); + + it('should display outdated change on commit header', done => { + vm.discussion.for_commit = false; + vm.discussion.active = false; + + vm.$nextTick(() => { + expect(vm.$el).toContainText( + `started a discussion on an outdated change in commit ${truncatedCommitId}`, + ); + + expect(commitElement).not.toBe(null); + + done(); + }); + }); + }); + + describe('for diff discussions without a commit id', () => { + it('should show started a discussion on the diff text', done => { + Object.assign(vm.discussion, { + for_commit: false, + commit_id: null, + }); + + vm.$nextTick(() => { + expect(vm.$el).toContainText('started a discussion on the diff'); + + done(); + }); + }); + + it('should show discussion on older version text', done => { + Object.assign(vm.discussion, { + for_commit: false, + commit_id: null, + active: false, + }); + + vm.$nextTick(() => { + expect(vm.$el).toContainText('started a discussion on an old version of the diff'); + + done(); + }); + }); }); }); });