diff options
author | Kushal Pandya <kushalspandya@gmail.com> | 2018-12-21 05:28:57 +0000 |
---|---|---|
committer | Kushal Pandya <kushalspandya@gmail.com> | 2018-12-21 05:28:57 +0000 |
commit | 80ff142c1cb8b11e6e1eda27dfc09cf0253e824b (patch) | |
tree | 864a957247abecef5f5ef91566dac1751770ce20 | |
parent | 5a3bff5d83d14aeaa590304d6b13d1a8320645c1 (diff) | |
parent | 583859806262ca78557893853f9c619c917b14ee (diff) | |
download | gitlab-ce-80ff142c1cb8b11e6e1eda27dfc09cf0253e824b.tar.gz |
Merge branch 'winh-merge-request-commit-context' into 'master'
Display commit ID for discussions made on merge request commits
Closes #53953
See merge request gitlab-org/gitlab-ce!23837
4 files changed, 93 insertions, 27 deletions
diff --git a/app/assets/javascripts/notes/components/noteable_discussion.vue b/app/assets/javascripts/notes/components/noteable_discussion.vue index bba215852b7..be451401e97 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 = `<span class="commit-sha">${truncateSha(commitId)}</span>`; } - 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 c79df09229f..776cd9d03d4 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -4138,6 +4138,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'); + }); + + 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); + }); }); - it('displays a monospace started a discussion on commit', () => { - const truncatedCommitId = commitId.substr(0, 8); + describe('for diff discussion with a commit id', () => { + it('should display started discussion on commit header', done => { + vm.discussion.for_commit = false; - expect(vm.$el).toContainText(`started a discussion on commit ${truncatedCommitId}`); + vm.$nextTick(() => { + expect(vm.$el).toContainText(`started a discussion on commit ${truncatedCommitId}`); + expect(commitElement).not.toBe(null); - const commitElement = vm.$el.querySelector('.commit-sha'); + done(); + }); + }); - expect(commitElement).not.toBe(null); - expect(commitElement).toHaveText(truncatedCommitId); + 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(); + }); + }); }); }); }); |