diff options
author | Douwe Maan <douwe@gitlab.com> | 2016-08-19 05:24:34 +0300 |
---|---|---|
committer | Ruben Davila <rdavila84@gmail.com> | 2016-08-19 09:22:27 +0300 |
commit | 78c5927031b8181e4fa3824676a552ebfaf42bab (patch) | |
tree | cb8d953af9d71e8c0ceaff43f4e5055aa722f74b /app | |
parent | d02b168979a35ca48ffe843a3adc92d58f394d9b (diff) |
Merge branch 'diff-line-comment-vuejs' into 'master'
Diff line comments resolve
## What does this MR do?
Diff line comments can be resolved.
Part of #10325
To do:
- [x] Backend (@DouweM)
- [x] Fix https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022#note_13319326. Will be made easier by https://gitlab.com/gitlab-org/gitlab-ce/issues/17237#note_13370331
- [x] System note when all discussions are resolved
- [x] Notification when all discussions are resolved
- [x] Write unit tests
- [x] Look at resolve time https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022#note_13912743 - Fixed by 4a13aa9
- [x] Frontend (@iamphill)
- [x] Fix bugs
- [x] Write more feature tests
- [x] Frontend (@connorshea)
- [x] Address frontend feedback
- [x] Feature specs for Jump feature
- [x] Documentation
- [x] Add Vue.js in a standard way
See merge request !5022
Diffstat (limited to 'app')
53 files changed, 1461 insertions, 126 deletions
diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js index 2b331546e4b..4cc5af76e47 100644 --- a/app/assets/javascripts/application.js +++ b/app/assets/javascripts/application.js @@ -224,10 +224,13 @@ }); $body.on("click", ".js-toggle-diff-comments", function(e) { var $this = $(this); - var showComments = $this.hasClass('active'); - $this.toggleClass('active'); - $this.closest(".diff-file").find(".notes_holder").toggle(showComments); + var notesHolders = $this.closest('.diff-file').find('.notes_holder'); + if ($this.hasClass('active')) { + notesHolders.show(); + } else { + notesHolders.hide(); + } return e.preventDefault(); }); $document.off("click", '.js-confirm-danger'); diff --git a/app/assets/javascripts/diff_notes/components/comment_resolve_btn.js.es6 b/app/assets/javascripts/diff_notes/components/comment_resolve_btn.js.es6 new file mode 100644 index 00000000000..48bc7d77805 --- /dev/null +++ b/app/assets/javascripts/diff_notes/components/comment_resolve_btn.js.es6 @@ -0,0 +1,49 @@ +((w) => { + w.CommentAndResolveBtn = Vue.extend({ + props: { + discussionId: String, + textareaIsEmpty: Boolean + }, + computed: { + discussion: function () { + return CommentsStore.state[this.discussionId]; + }, + showButton: function () { + if (this.discussion) { + return this.discussion.isResolvable(); + } else { + return false; + } + }, + isDiscussionResolved: function () { + return this.discussion.isResolved(); + }, + buttonText: function () { + if (this.isDiscussionResolved) { + if (this.textareaIsEmpty) { + return "Unresolve discussion"; + } else { + return "Comment & unresolve discussion"; + } + } else { + if (this.textareaIsEmpty) { + return "Resolve discussion"; + } else { + return "Comment & resolve discussion"; + } + } + } + }, + ready: function () { + const $textarea = $(`#new-discussion-note-form-${this.discussionId} .note-textarea`); + this.textareaIsEmpty = $textarea.val() === ''; + + $textarea.on('input.comment-and-resolve-btn', () => { + this.textareaIsEmpty = $textarea.val() === ''; + }); + }, + destroyed: function () { + $(`#new-discussion-note-form-${this.discussionId} .note-textarea`).off('input.comment-and-resolve-btn'); + } + }); +})(window); diff --git a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 new file mode 100644 index 00000000000..ad80d1118df --- /dev/null +++ b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 @@ -0,0 +1,188 @@ +(() => { + JumpToDiscussion = Vue.extend({ + mixins: [DiscussionMixins], + props: { + discussionId: String + }, + data: function () { + return { + discussions: CommentsStore.state, + }; + }, + computed: { + discussion: function () { + return this.discussions[this.discussionId]; + }, + allResolved: function () { + return this.unresolvedDiscussionCount === 0; + }, + showButton: function () { + if (this.discussionId) { + if (this.unresolvedDiscussionCount > 1) { + return true; + } else { + return this.discussionId !== this.lastResolvedId; + } + } else { + return this.unresolvedDiscussionCount >= 1; + } + }, + lastResolvedId: function () { + let lastId; + for (const discussionId in this.discussions) { + const discussion = this.discussions[discussionId]; + + if (!discussion.isResolved()) { + lastId = discussion.id; + } + } + return lastId; + } + }, + methods: { + jumpToNextUnresolvedDiscussion: function () { + let discussionsSelector, + discussionIdsInScope, + firstUnresolvedDiscussionId, + nextUnresolvedDiscussionId, + activeTab = window.mrTabs.currentAction, + hasDiscussionsToJumpTo = true, + jumpToFirstDiscussion = !this.discussionId; + + const discussionIdsForElements = function(elements) { + return elements.map(function() { + return $(this).attr('data-discussion-id'); + }).toArray(); + }; + + const discussions = this.discussions; + + if (activeTab === 'diffs') { + discussionsSelector = '.diffs .notes[data-discussion-id]'; + discussionIdsInScope = discussionIdsForElements($(discussionsSelector)); + + let unresolvedDiscussionCount = 0; + + for (let i = 0; i < discussionIdsInScope.length; i++) { + const discussionId = discussionIdsInScope[i]; + const discussion = discussions[discussionId]; + if (discussion && !discussion.isResolved()) { + unresolvedDiscussionCount++; + } + } + + if (this.discussionId && !this.discussion.isResolved()) { + // If this is the last unresolved discussion on the diffs tab, + // there are no discussions to jump to. + if (unresolvedDiscussionCount === 1) { + hasDiscussionsToJumpTo = false; + } + } else { + // If there are no unresolved discussions on the diffs tab at all, + // there are no discussions to jump to. + if (unresolvedDiscussionCount === 0) { + hasDiscussionsToJumpTo = false; + } + } + } else if (activeTab !== 'notes') { + // If we are on the commits or builds tabs, + // there are no discussions to jump to. + hasDiscussionsToJumpTo = false; + } + + if (!hasDiscussionsToJumpTo) { + // If there are no discussions to jump to on the current page, + // switch to the notes tab and jump to the first disucssion there. + window.mrTabs.activateTab('notes'); + activeTab = 'notes'; + jumpToFirstDiscussion = true; + } + + if (activeTab === 'notes') { + discussionsSelector = '.discussion[data-discussion-id]'; + discussionIdsInScope = discussionIdsForElements($(discussionsSelector)); + } + + let currentDiscussionFound = false; + for (let i = 0; i < discussionIdsInScope.length; i++) { + const discussionId = discussionIdsInScope[i]; + const discussion = discussions[discussionId]; + + if (!discussion) { + // Discussions for comments on commits in this MR don't have a resolved status. + continue; + } + + if (!firstUnresolvedDiscussionId && !discussion.isResolved()) { + firstUnresolvedDiscussionId = discussionId; + + if (jumpToFirstDiscussion) { + break; + } + } + + if (!jumpToFirstDiscussion) { + if (currentDiscussionFound) { + if (!discussion.isResolved()) { + nextUnresolvedDiscussionId = discussionId; + break; + } + else { + continue; + } + } + + if (discussionId === this.discussionId) { + currentDiscussionFound = true; + } + } + } + + nextUnresolvedDiscussionId = nextUnresolvedDiscussionId || firstUnresolvedDiscussionId; + + if (!nextUnresolvedDiscussionId) { + return; + } + + let $target = $(`${discussionsSelector}[data-discussion-id="${nextUnresolvedDiscussionId}"]`); + + if (activeTab === 'notes') { + $target = $target.closest('.note-discussion'); + + // If the next discussion is closed, toggle it open. + if ($target.find('.js-toggle-content').is(':hidden')) { + $target.find('.js-toggle-button i').trigger('click') + } + } else if (activeTab === 'diffs') { + // Resolved discussions are hidden in the diffs tab by default. + // If they are marked unresolved on the notes tab, they will still be hidden on the diffs tab. + // When jumping between unresolved discussions on the diffs tab, we show them. + $target.closest(".content").show(); + + $target = $target.closest("tr.notes_holder"); + $target.show(); + + // If we are on the diffs tab, we don't scroll to the discussion itself, but to + // 4 diff lines above it: the line the discussion was in response to + 3 context + let prevEl; + for (let i = 0; i < 4; i++) { + prevEl = $target.prev(); + + // If the discussion doesn't have 4 lines above it, we'll have to do with fewer. + if (!prevEl.hasClass("line_holder")) { + break; + } + + $target = prevEl; + } + } + + $.scrollTo($target, { + offset: -($('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight()) + }); + } + } + }); + + Vue.component('jump-to-discussion', JumpToDiscussion); +})(); diff --git a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 new file mode 100644 index 00000000000..be6ebc77947 --- /dev/null +++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 @@ -0,0 +1,107 @@ +((w) => { + w.ResolveBtn = Vue.extend({ + mixins: [ + ButtonMixins + ], + props: { + noteId: Number, + discussionId: String, + resolved: Boolean, + namespacePath: String, + projectPath: String, + canResolve: Boolean, + resolvedBy: String + }, + data: function () { + return { + discussions: CommentsStore.state, + loading: false + }; + }, + watch: { + 'discussions': { + handler: 'updateTooltip', + deep: true + } + }, + computed: { + discussion: function () { + return this.discussions[this.discussionId]; + }, + note: function () { + if (this.discussion) { + return this.discussion.getNote(this.noteId); + } else { + return undefined; + } + }, + buttonText: function () { + if (this.isResolved) { + return `Resolved by ${this.resolvedByName}`; + } else if (this.canResolve) { + return 'Mark as resolved'; + } else { + return 'Unable to resolve'; + } + }, + isResolved: function () { + if (this.note) { + return this.note.resolved; + } else { + return false; + } + }, + resolvedByName: function () { + return this.note.resolved_by; + }, + }, + methods: { + updateTooltip: function () { + $(this.$els.button) + .tooltip('hide') + .tooltip('fixTitle'); + }, + resolve: function () { + if (!this.canResolve) return; + + let promise; + this.loading = true; + + if (this.isResolved) { + promise = ResolveService + .unresolve(this.namespace, this.noteId); + } else { + promise = ResolveService + .resolve(this.namespace, this.noteId); + } + + promise.then((response) => { + this.loading = false; + + if (response.status === 200) { + const data = response.json(); + const resolved_by = data ? data.resolved_by : null; + + CommentsStore.update(this.discussionId, this.noteId, !this.isResolved, resolved_by); + this.discussion.updateHeadline(data); + } else { + new Flash('An error occurred when trying to resolve a comment. Please try again.', 'alert'); + } + + this.$nextTick(this.updateTooltip); + }); + } + }, + compiled: function () { + $(this.$els.button).tooltip({ + container: 'body' + }); + }, + beforeDestroy: function () { + CommentsStore.delete(this.discussionId, this.noteId); + }, + created: function () { + CommentsStore.create(this.discussionId, this.noteId, this.canResolve, this.resolved, this.resolvedBy); + } + }); +})(window); diff --git a/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 new file mode 100644 index 00000000000..9e383b14a3e --- /dev/null +++ b/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 @@ -0,0 +1,18 @@ +((w) => { + w.ResolveCount = Vue.extend({ + mixins: [DiscussionMixins], + props: { + loggedOut: Boolean + }, + data: function () { + return { + discussions: CommentsStore.state + }; + }, + computed: { + allResolved: function () { + return this.resolvedDiscussionCount === this.discussionCount; + } + } + }); +})(window); diff --git a/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6 new file mode 100644 index 00000000000..e373b06b1eb --- /dev/null +++ b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6 @@ -0,0 +1,60 @@ +((w) => { + w.ResolveDiscussionBtn = Vue.extend({ + mixins: [ + ButtonMixins + ], + props: { + discussionId: String, + mergeRequestId: Number, + namespacePath: String, + projectPath: String, + canResolve: Boolean, + }, + data: function() { + return { + discussions: CommentsStore.state + }; + }, + computed: { + discussion: function () { + return this.discussions[this.discussionId]; + }, + showButton: function () { + if (this.discussion) { + return this.discussion.isResolvable(); + } else { + return false; + } + }, + isDiscussionResolved: function () { + if (this.discussion) { + return this.discussion.isResolved(); + } else { + return false; + } + }, + buttonText: function () { + if (this.isDiscussionResolved) { + return "Unresolve discussion"; + } else { + return "Resolve discussion"; + } + }, + loading: function () { + if (this.discussion) { + return this.discussion.loading; + } else { + return false; + } + } + }, + methods: { + resolve: function () { + ResolveService.toggleResolveForDiscussion(this.namespace, this.mergeRequestId, this.discussionId); + } + }, + created: function () { + CommentsStore.createDiscussion(this.discussionId, this.canResolve); + } + }); +})(window); diff --git a/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 b/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 new file mode 100644 index 00000000000..22d9cf6c857 --- /dev/null +++ b/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 @@ -0,0 +1,35 @@ +//= require vue +//= require vue-resource +//= require_directory ./models +//= require_directory ./stores +//= require_directory ./services +//= require_directory ./mixins +//= require_directory ./components + +$(() => { + window.DiffNotesApp = new Vue({ + el: '#diff-notes-app', + components: { + 'resolve-btn': ResolveBtn, + 'resolve-discussion-btn': ResolveDiscussionBtn, + 'comment-and-resolve-btn': CommentAndResolveBtn + }, + methods: { + compileComponents: function () { + const $components = $('resolve-btn, resolve-discussion-btn, jump-to-discussion'); + if ($components.length) { + $components.each(function () { + DiffNotesApp.$compile($(this).get(0)); + }); + } + } + } + }); + + new Vue({ + el: '#resolve-count-app', + components: { + 'resolve-count': ResolveCount + } + }); +}); diff --git a/app/assets/javascripts/diff_notes/mixins/discussion.js.es6 b/app/assets/javascripts/diff_notes/mixins/discussion.js.es6 new file mode 100644 index 00000000000..a05f885201d --- /dev/null +++ b/app/assets/javascripts/diff_notes/mixins/discussion.js.es6 @@ -0,0 +1,35 @@ +((w) => { + w.DiscussionMixins = { + computed: { + discussionCount: function () { + return Object.keys(this.discussions).length; + }, + resolvedDiscussionCount: function () { + let resolvedCount = 0; + + for (const discussionId in this.discussions) { + const discussion = this.discussions[discussionId]; + + if (discussion.isResolved()) { + resolvedCount++; + } + } + + return resolvedCount; + }, + unresolvedDiscussionCount: function () { + let unresolvedCount = 0; + + for (const discussionId in this.discussions) { + const discussion = this.discussions[discussionId]; + + if (!discussion.isResolved()) { + unresolvedCount++; + } + } + + return unresolvedCount; + } + } + }; +})(window); diff --git a/app/assets/javascripts/diff_notes/mixins/namespace.js.es6 b/app/assets/javascripts/diff_notes/mixins/namespace.js.es6 new file mode 100644 index 00000000000..d278678085b --- /dev/null +++ b/app/assets/javascripts/diff_notes/mixins/namespace.js.es6 @@ -0,0 +1,9 @@ +((w) => { + w.ButtonMixins = { + computed: { + namespace: function () { + return `${this.namespacePath}/${this.projectPath}`; + } + } + }; +})(window); diff --git a/app/assets/javascripts/diff_notes/models/discussion.js.es6 b/app/assets/javascripts/diff_notes/models/discussion.js.es6 new file mode 100644 index 00000000000..488714e4870 --- /dev/null +++ b/app/assets/javascripts/diff_notes/models/discussion.js.es6 @@ -0,0 +1,87 @@ +class DiscussionModel { + constructor (discussionId) { + this.id = discussionId; + this.notes = {}; + this.loading = false; + this.canResolve = false; + } + + createNote (noteId, canResolve, resolved, resolved_by) { + Vue.set(this.notes, noteId, new NoteModel(this.id, noteId, canResolve, resolved, resolved_by)); + } + + deleteNote (noteId) { + Vue.delete(this.notes, noteId); + } + + getNote (noteId) { + return this.notes[noteId]; + } + + notesCount() { + return Object.keys(this.notes).length; + } + + isResolved () { + for (const noteId in this.notes) { + const note = this.notes[noteId]; + + if (!note.resolved) { + return false; + } + } + return true; + } + + resolveAllNotes (resolved_by) { + for (const noteId in this.notes) { + const note = this.notes[noteId]; + + if (!note.resolved) { + note.resolved = true; + note.resolved_by = resolved_by; + } + } + } + + unResolveAllNotes () { + for (const noteId in this.notes) { + const note = this.notes[noteId]; + + if (note.resolved) { + note.resolved = false; + note.resolved_by = null; + } + } + } + + updateHeadline (data) { + const $discussionHeadline = $(`.discussion[data-discussion-id="${this.id}"] .js-discussion-headline`); + + if (data.discussion_headline_html) { + if ($discussionHeadline.length) { + $discussionHeadline.replaceWith(data.discussion_headline_html); + } else { + $(`.discussion[data-discussion-id="${this.id}"] .discussion-header`).append(data.discussion_headline_html); + } + } else { + $discussionHeadline.remove(); + } + } + + isResolvable () { + if (!this.canResolve) { + return false; + } + + for (const noteId in this.notes) { + const note = this.notes[noteId]; + + if (note.canResolve) { + return true; + } + } + + return false; + } +} diff --git a/app/assets/javascripts/diff_notes/models/note.js.es6 b/app/assets/javascripts/diff_notes/models/note.js.es6 new file mode 100644 index 00000000000..f2d2d389c38 --- /dev/null +++ b/app/assets/javascripts/diff_notes/models/note.js.es6 @@ -0,0 +1,9 @@ +class NoteModel { + constructor (discussionId, noteId, canResolve, resolved, resolved_by) { + this.discussionId = discussionId; + this.id = noteId; + this.canResolve = canResolve; + this.resolved = resolved; + this.resolved_by = resolved_by; + } +} diff --git a/app/assets/javascripts/diff_notes/services/resolve.js.es6 b/app/assets/javascripts/diff_notes/services/resolve.js.es6 new file mode 100644 index 00000000000..de771ff814b --- /dev/null +++ b/app/assets/javascripts/diff_notes/services/resolve.js.es6 @@ -0,0 +1,88 @@ +((w) => { + class ResolveServiceClass { + constructor() { + this.noteResource = Vue.resource('notes{/noteId}/resolve'); + this.discussionResource = Vue.resource('merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve'); + } + + setCSRF() { + Vue.http.headers.common['X-CSRF-Token'] = $.rails.csrfToken(); + } + + prepareRequest(namespace) { + this.setCSRF(); + Vue.http.options.root = `/${namespace}`; + } + + resolve(namespace, noteId) { + this.prepareRequest(namespace); + + return this.noteResource.save({ noteId }, {}); + } + + unresolve(namespace, noteId) { + this.prepareRequest(namespace); + + return this.noteResource.delete({ noteId }, {}); + } + + toggleResolveForDiscussion(namespace, mergeRequestId, discussionId) { + const discussion = CommentsStore.state[discussionId], + isResolved = discussion.isResolved(); + let promise; + + if (isResolved) { + promise = this.unResolveAll(namespace, mergeRequestId, discussionId); + } else { + promise = this.resolveAll(namespace, mergeRequestId, discussionId); + } + + promise.then((response) => { + discussion.loading = false; + + if (response.status === 200) { + const data = response.json(); + const resolved_by = data ? data.resolved_by : null; + + if (isResolved) { + discussion.unResolveAllNotes(); + } else { + discussion.resolveAllNotes(resolved_by); + } + + discussion.updateHeadline(data); + } else { + new Flash('An error occurred when trying to resolve a discussion. Please try again.', 'alert'); + } + }) + } + + resolveAll(namespace, mergeRequestId, discussionId) { + const discussion = CommentsStore.state[discussionId]; + + this.prepareRequest(namespace); + + discussion.loading = true; + + return this.discussionResource.save({ + mergeRequestId, + discussionId + }, {}); + } + + unResolveAll(namespace, mergeRequestId, discussionId) { + const discussion = CommentsStore.state[discussionId]; + + this.prepareRequest(namespace); + + discussion.loading = true; + + return this.discussionResource.delete({ + mergeRequestId, + discussionId + }, {}); + } + } + + w.ResolveService = new ResolveServiceClass(); +})(window); diff --git a/app/assets/javascripts/diff_notes/stores/comments.js.es6 b/app/assets/javascripts/diff_notes/stores/comments.js.es6 new file mode 100644 index 00000000000..69522e1dac5 --- /dev/null +++ b/app/assets/javascripts/diff_notes/stores/comments.js.es6 @@ -0,0 +1,53 @@ +((w) => { + w.CommentsStore = { + state: {}, + get: function (discussionId, noteId) { + return this.state[discussionId].getNote(noteId); + }, + createDiscussion: function (discussionId, canResolve) { + let discussion = this.state[discussionId]; + if (!this.state[discussionId]) { + discussion = new DiscussionModel(discussionId); + Vue.set(this.state, discussionId, discussion); + } + + if (canResolve !== undefined) { + discussion.canResolve = canResolve; + } + + return discussion; + }, + create: function (discussionId, noteId, canResolve, resolved, resolved_by) { + const discussion = this.createDiscussion(discussionId); + + discussion.createNote(noteId, canResolve, resolved, resolved_by); + }, + update: function (discussionId, noteId, resolved, resolved_by) { + const discussion = this.state[discussionId]; + const note = discussion.getNote(noteId); + note.resolved = resolved; + note.resolved_by = resolved_by; + }, + delete: function (discussionId, noteId) { + const discussion = this.state[discussionId]; + discussion.deleteNote(noteId); + + if (discussion.notesCount() === 0) { + Vue.delete(this.state, discussionId); + } + }, + unresolvedDiscussionIds: function () { + let ids = []; + + for (const discussionId in this.state) { + const discussion = this.state[discussionId]; + + if (!discussion.isResolved()) { + ids.push(discussion.id); + } + } + + return ids; + } + }; +})(window); diff --git a/app/assets/javascripts/merge_request.js b/app/assets/javascripts/merge_request.js index 47e6dd1084d..56ebf84c4f6 100644 --- a/app/assets/javascripts/merge_request.js +++ b/app/assets/javascripts/merge_request.js @@ -34,7 +34,7 @@ MergeRequest.prototype.initTabs = function() { if (this.opts.action !== 'new') { - return new MergeRequestTabs(this.opts); + window.mrTabs = new MergeRequestTabs(this.opts); } else { return $('.merge-request-tabs a[data-toggle="tab"]:first').tab('show'); } diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js index 4e2273f5aa8..ad08209d61e 100644 --- a/app/assets/javascripts/merge_request_tabs.js +++ b/app/assets/javascripts/merge_request_tabs.js @@ -89,6 +89,7 @@ if (action === 'show') { action = 'notes'; } + this.currentAction = action; new_state = this._location.pathname.replace(/\/(commits|diffs|builds|pipelines)(\.html)?\/?$/, ''); if (action !== 'notes') { new_state += "/" + action; @@ -127,6 +128,11 @@ success: (function(_this) { return function(data) { $('#diffs').html(data.html); + + if (typeof DiffNotesApp !== 'undefined') { + DiffNotesApp.compileComponents(); + } + gl.utils.localTimeAgo($('.js-timeago', 'div#diffs')); $('#diffs .js-syntax-highlight').syntaxHighlight(); $('#diffs .diff-file').singleFileDiff(); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index 2484a07f363..d0d5cad813a 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -68,6 +68,7 @@ $(document).on("click", ".note-edit-cancel", this.cancelEdit); $(document).on("click", ".js-comment-button", this.updateCloseButton); $(document).on("keyup input", ".js-note-text", this.updateTargetButtons); + $(document).on('click', '.js-comment-resolve-button', this.resolveDiscussion); $(document).on("click", ".js-note-delete", this.removeNote); $(document).on("click", ".js-note-attachment-delete", this.removeAttachment); $(document).on("ajax:complete", ".js-main-target-form", this.reenableTargetFormSubmitButton); @@ -100,6 +101,7 @@ $(document).off("click", ".js-note-target-close"); $(document).off("click", ".js-note-discard"); $(document).off("keydown", ".js-note-text"); + $(document).off('click', '.js-comment-resolve-button'); $('.note .js-task-list-container').taskList('disable'); return $(document).off('tasklist:changed', '.note .js-task-list-container'); }; @@ -304,6 +306,11 @@ } else { discussionContainer.append(note_html); } + + if (typeof DiffNotesApp !== 'undefined') { + DiffNotesApp.compileComponents(); + } + gl.utils.localTimeAgo($('.js-timeago', note_html), false); return this.updateNotesCount(1); }; @@ -350,6 +357,7 @@ form.find("#note_line_code").remove(); form.find("#note_position").remove(); form.find("#note_type").remove(); + form.find('.js-comment-resolve-button').closest('comment-and-resolve-btn').remove(); return this.parentTimeline = form.parents('.timeline'); }; @@ -393,8 +401,22 @@ */ Notes.prototype.addDiscussionNote = function(xhr, note, status) { + var $form = $(xhr.target); + + if ($form.attr('data-resolve-all') != null) { + var namespacePath = $form.attr('data-namespace-path'), + projectPath = $form.attr('data-project-path') + discussionId = $form.attr('data-discussion-id'), + mergeRequestId = $form.attr('data-noteable-iid'), + namespace = namespacePath + '/' + projectPath; + + if (ResolveService != null) { + ResolveService.toggleResolveForDiscussion(namespace, mergeRequestId, discussionId); + } + } + this.renderDiscussionNote(note); - return this.removeDiscussionNoteForm($(xhr.target)); + this.removeDiscussionNoteForm($form); }; @@ -411,7 +433,12 @@ $html.syntaxHighlight(); $html.find('.js-task-list-container').taskList('enable'); $note_li = $('.note-row-' + note.id); - return $note_li.replaceWith($html); + + $note_li.replaceWith($html); + + if (typeof DiffNotesApp !== 'undefined') { + DiffNotesApp.compileComponents(); + } }; @@ -492,6 +519,15 @@ var note, notes; note = $(el); notes = note.closest(".notes"); + + if (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null) { + ref = DiffNotesApp.$refs[noteId]; + + if (ref) { + ref.$destroy(true); + } + } + if (notes.find(".note").length === 1) { notes.closest(".timeline-entry").remove(); notes.closest("tr").remove(); @@ -530,8 +566,10 @@ var form, replyLink; form = this.formClone.clone(); replyLink = $(e.target).closest(".js-discussion-reply-button"); - replyLink.hide(); - replyLink.after(form); + replyLink + .closest('.discussion-reply-holder') + .hide() + .after(form); return this.setupDiscussionNoteForm(replyLink, form); }; @@ -556,9 +594,23 @@ form.find("#note_noteable_type").val(dataHolder.data("noteableType")); form.find("#note_noteable_id").val(dataHolder.data("noteableId")); form.find('.js-note-discard').show().removeClass('js-note-discard').addClass('js-close-discussion-note-form').text(form.find('.js-close-discussion-note-form').data('cancel-text')); + form.find('.js-note-target-close').remove(); this.setupNoteForm(form); + + if (typeof DiffNotesApp !== 'undefined') { + var $commentBtn = form.find('comment-and-resolve-btn'); + $commentBtn + .attr(':discussion-id', "'" + dataHolder.data('discussionId') + "'"); + DiffNotesApp.$compile($commentBtn.get(0)); + } + form.find(".js-note-text").focus(); - return form.removeClass('js-main-target-form').addClass("discussion-form js-discussion-note-form"); + form + .find('.js-comment-resolve-button') + .attr('data-discussion-id', dataHolder.data('discussionId')); + form + .removeClass('js-main-target-form') + .addClass("discussion-form js-discussion-note-form"); }; @@ -577,16 +629,19 @@ nextRow = row.next(); hasNotes = nextRow.is(".notes_holder"); addForm = false; - targetContent = ".notes_content"; - rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\" colspan=\"2\"></td><td class=\"notes_content\"></td></tr>"; + notesContentSelector = ".notes_content"; + rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\" colspan=\"2\"></td><td class=\"notes_content\"><div class=\"content\"></div></td></tr>"; if (this.isParallelView()) { lineType = $link.data("lineType"); - targetContent += "." + lineType; - rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line\"></td><td class=\"notes_content parallel old\"></td><td class=\"notes_line\"></td><td class=\"notes_content parallel new\"></td></tr>"; + notesContentSelector += "." + lineType; + rowCssToAdd = "<tr class=\"notes_holder js-temp-notes-holder\"><td class=\"notes_line old\"></td><td class=\"notes_content parallel old\"><div class=\"content\"></div></td><td class=\"notes_line new\"></td><td class=\"notes_content parallel new\"><div class=\"content\"></div></td></tr>"; } + notesContentSelector += " .content"; if (hasNotes) { - notesContent = nextRow.find(targetContent); + nextRow.show(); + notesContent = nextRow.find(notesContentSelector); if (notesContent.length) { + notesContent.show(); replyButton = notesContent.find(".js-discussion-reply-button:visible"); if (replyButton.length) { e.target = replyButton[0]; @@ -600,11 +655,13 @@ } } else { row.after(rowCssToAdd); + nextRow = row.next(); + notesContent = nextRow.find(notesContentSelector); addForm = true; } if (addForm) { newForm = this.formClone.clone(); - newForm.appendTo(row.next().find(targetContent)); + newForm.appendTo(notesContent); return this.setupDiscussionNoteForm($link, newForm); } }; @@ -623,7 +680,9 @@ glForm = form.data('gl-form'); glForm.destroy(); form.find(".js-note-text").data("autosave").reset(); - form.prev(".js-discussion-reply-button").show(); + form + .prev('.discussion-reply-holder') + .show(); if (row.is(".js-temp-notes-holder")) { return row.remove(); } else { @@ -732,6 +791,18 @@ return this.notesCountBadge.text(parseInt(this.notesCountBadge.text()) + updateCount); }; + Notes.prototype.resolveDiscussion = function () { + var $this = $(this), + discussionId = $this.attr('data-discussion-id'); + + $this + .closest('form') + .attr('data-discussion-id', discussionId) + .attr('data-resolve-all', 'true') + .attr('data-namespace-path', $this.attr('data-namespace-path')) + .attr('data-project-path', $this.attr('data-project-path')); + }; + return Notes; })(); diff --git a/app/assets/javascripts/single_file_diff.js b/app/assets/javascripts/single_file_diff.js index b9ae497b0e5..156b9b8abec 100644 --- a/app/assets/javascripts/single_file_diff.js +++ b/app/assets/javascripts/single_file_diff.js @@ -35,10 +35,16 @@ this.isOpen = !this.isOpen; if (!this.isOpen && !this.hasError) { this.content.hide(); - return this.collapsedContent.show(); + this.collapsedContent.show(); + if (typeof DiffNotesApp !== 'undefined') { + DiffNotesApp.compileComponents(); + } } else if (this.content) { this.collapsedContent.hide(); - return this.content.show(); + this.content.show(); + if (typeof DiffNotesApp !== 'undefined') { + DiffNotesApp.compileComponents(); + } } else { return this.getContentHTML(); } @@ -57,7 +63,11 @@ _this.hasError = true; _this.content = $(ERROR_HTML); } - return _this.collapsedContent.after(_this.content); + _this.collapsedContent.after(_this.content); + + if (typeof DiffNotesApp !== 'undefined') { + DiffNotesApp.compileComponents(); + } }; })(this)); }; diff --git a/app/assets/stylesheets/behaviors.scss b/app/assets/stylesheets/behaviors.scss index a6b9efc49c9..897bc49e7df 100644 --- a/app/assets/stylesheets/behaviors.scss +++ b/app/assets/stylesheets/behaviors.scss @@ -21,7 +21,7 @@ } } - -[v-cloak] { - display: none; +// Hide element if Vue is still working on rendering it fully. +[v-cloak="true"] { + display: none !important; } diff --git a/app/assets/stylesheets/pages/note_form.scss b/app/assets/stylesheets/pages/note_form.scss index 3784010348a..bd875b9823f 100644 --- a/app/assets/stylesheets/pages/note_form.scss +++ b/app/assets/stylesheets/pages/note_form.scss @@ -159,6 +159,32 @@ } } +.discussion-with-resolve-btn { + display: table; + width: 100%; + border-collapse: separate; + table-layout: auto; + + .btn-group { + display: table-cell; + float: none; + width: 1%; + + &:first-child { + width: 100%; + padding-right: 5px; + } + + &:last-child { + padding-left: 5px; + } + } + + .btn { + width: 100%; + } +} + .discussion-notes-count { font-size: 16px; } diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index a2b5437e503..08d1692c888 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -383,3 +383,80 @@ ul.notes { color: $gl-link-color; } } + +.line-resolve-all-container { + .btn-group { + margin-top: -1px; + margin-left: -4px; + } + + .discussion-next-btn { + border-top-left-radius: 0; + border-bottom-left-radius: 0; + } +} + +.line-resolve-all { + display: inline-block; + padding: 5px 10px; + background-color: $background-color; + border: 1px solid $border-color; + border-radius: $border-radius-default; + + &.has-next-btn { + border-top-right-radius: 0; + border-bottom-right-radius: 0; + } + + .line-resolve-btn { + vertical-align: middle; + margin-right: 5px; + } +} + +.line-resolve-text { + vertical-align: middle; +} + +.line-resolve-btn { + display: inline-block; + position: relative; + top: 2px; + padding: 0; + background-color: transparent; + border: none; + outline: 0; + + &.is-disabled { + cursor: default; + } + + &:not(.is-disabled):hover, + &:not(.is-disabled):focus, + &.is-active { + color: $gl-text-green; + + svg path { + fill: $gl-text-green; + } + } + + svg { + position: relative; + color: $notes-action-color; + + path { + fill: $notes-action-color; + } + } +} + +.discussion-next-btn { + svg { + margin: 0; + + path { + fill: $gray-darkest; + } + } +} diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb new file mode 100644 index 00000000000..b2e8733ccb7 --- /dev/null +++ b/app/controllers/projects/discussions_controller.rb @@ -0,0 +1,43 @@ +class Projects::DiscussionsController < Projects::ApplicationController + before_action :module_enabled + before_action :merge_request + before_action :discussion + before_action :authorize_resolve_discussion! + + def resolve + discussion.resolve!(current_user) + + MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) + + render json: { + resolved_by: discussion.resolved_by.try(:name), + discussion_headline_html: view_to_html_string('discussions/_headline', discussion: discussion) + } + end + + def unresolve + discussion.unresolve! + + render json: { + discussion_headline_html: view_to_html_string('discussions/_headline', discussion: discussion) + } + end + + private + + def merge_request + @merge_request ||= @project.merge_requests.find_by!(iid: params[:merge_request_id]) + end + + def discussion + @discussion ||= @merge_request.find_diff_discussion(params[:id]) || render_404 + end + + def authorize_resolve_discussion! + access_denied! unless discussion.can_resolve?(current_user) + end + + def module_enabled + render_404 unless @project.merge_requests_enabled + end +end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 0f52a5f96a3..d3fe441c4d2 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -435,12 +435,9 @@ class Projects::MergeRequestsController < Projects::ApplicationController # :show, :diff, :commits, :builds. but not when request the data through AJAX def define_discussion_vars # Build a note object for comment form - @note = @project.notes.new(noteable: @noteable) + @note = @project.notes.new(noteable: @merge_request) - @discussions = @noteable.mr_and_commit_notes. - inc_author_project_award_emoji. - fresh. - discussions + @discussions = @merge_request.discussions preload_noteable_for_regular_notes(@discussions.flat_map(&:notes)) @@ -474,7 +471,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController } @use_legacy_diff_notes = !@merge_request.has_complete_diff_refs? - @grouped_diff_discussions = @merge_request.notes.inc_author_project_award_emoji.grouped_diff_discussions + @grouped_diff_discussions = @merge_request.notes.inc_relations_for_view.grouped_diff_discussions Banzai::NoteRenderer.render( @grouped_diff_discussions.values.flat_map(&:notes), diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index f2422729364..0948ad21649 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -5,6 +5,7 @@ class Projects::NotesController < Projects::ApplicationController before_action :authorize_read_note! before_action :authorize_create_note!, only: [:create] before_action :authorize_admin_note!, only: [:update, :destroy] + before_action :authorize_resolve_note!, only: [:resolve, :unresolve] before_action :find_current_user_notes, only: [:index] def index @@ -66,6 +67,33 @@ class Projects::NotesController < Projects::ApplicationController end end + def resolve + return render_404 unless note.resolvable? + + note.resolve!(current_user) + + MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(note.noteable) + + discussion = note.discussion + + render json: { + resolved_by: note.resolved_by.try(:name), + discussion_headline_html: (view_to_html_string('discussions/_headline', discussion: discussion) if discussion) + } + end + + def unresolve + return render_404 unless note.resolvable? + + note.unresolve! + + discussion = note.discussion + + render json: { + discussion_headline_html: (view_to_html_string('discussions/_headline', discussion: discussion) if discussion) + } + end + private def note @@ -138,7 +166,7 @@ class Projects::NotesController < Projects::ApplicationController } if note.diff_note? - discussion = Discussion.new([note]) + discussion = note.to_discussion attrs.merge!( diff_discussion_html: diff_discussion_html(discussion), @@ -175,6 +203,10 @@ class Projects::NotesController < Projects::ApplicationController return access_denied! unless can?(current_user, :admin_note, note) end + def authorize_resolve_note! + return access_denied! unless can?(current_user, :resolve_note, note) + end + def note_params params.require(:note).permit( :note, :noteable, :noteable_id, :noteable_type, :project_id, diff --git a/app/helpers/appearances_helper.rb b/app/helpers/appearances_helper.rb index e12a1052988..de13e7a1fc2 100644 --- a/app/helpers/appearances_helper.rb +++ b/app/helpers/appearances_helper.rb @@ -32,6 +32,8 @@ module AppearancesHelper end def custom_icon(icon_name, size: 16) + # We can't simply do the below, because there are some .erb SVGs. + # File.read(Rails.root.join("app/views/shared/icons/_#{icon_name}.svg")).html_safe render "shared/icons/#{icon_name}.svg", size: size end end diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 26bde2230a9..da230f76bae 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -49,7 +49,7 @@ module NotesHelper } if use_legacy_diff_note - discussion_id = LegacyDiffNote.build_discussion_id( + discussion_id = LegacyDiffNote.discussion_id( @comments_target[:noteable_type], @comments_target[:noteable_id] || @comments_target[:commit_id], line_code @@ -60,7 +60,7 @@ module NotesHelper discussion_id: discussion_id ) else - discussion_id = DiffNote.build_discussion_id( + discussion_id = DiffNote.discussion_id( @comments_target[:noteable_type], @comments_target[:noteable_id] || @comments_target[:commit_id], position @@ -81,10 +81,8 @@ module NotesHelper data = discussion.reply_attributes.merge(line_type: line_type) - content_tag(:div, class: "discussion-reply-holder") do - button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button', - data: data, title: 'Add a reply' - end + button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button', + data: data, title: 'Add a reply' end def preload_max_access_for_authors(notes, project) diff --git a/app/mailers/emails/merge_requests.rb b/app/mailers/emails/merge_requests.rb index 95810b0ac6e..ec27ac517db 100644 --- a/app/mailers/emails/merge_requests.rb +++ b/app/mailers/emails/merge_requests.rb @@ -47,6 +47,13 @@ module Emails mail_answer_thread(@merge_request, merge_request_thread_options(updated_by_user_id, recipient_id)) end + def resolved_all_discussions_email(recipient_id, merge_request_id, resolved_by_user_id) + setup_merge_request_mail(merge_request_id, recipient_id) + + @resolved_by = User.find(resolved_by_user_id) + mail_answer_thread(@merge_request, merge_request_thread_options(resolved_by_user_id, recipient_id)) + end + private def setup_merge_request_mail(merge_request_id, recipient_id) diff --git a/app/models/ability.rb b/app/models/ability.rb index 55265c3cfcb..07f703f205d 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -276,6 +276,7 @@ class Ability :create_merge_request, :create_wiki, :push_code, + :resolve_note, :create_container_image, :update_container_image, :create_environment, @@ -457,7 +458,8 @@ class Ability rules += [ :read_note, :update_note, - :admin_note + :admin_note, + :resolve_note ] end @@ -465,6 +467,10 @@ class Ability rules += project_abilities(user, note.project) end + if note.for_merge_request? && note.noteable.author == user + rules << :resolve_note + end + rules end diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index e02a3d54c36..f56c3d74ae3 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -9,11 +9,13 @@ class DiffNote < Note validates :diff_line, presence: true validates :line_code, presence: true, line_code: true validates :noteable_type, inclusion: { in: ['Commit', 'MergeRequest'] } + validates :resolved_by, presence: true, if: :resolved? validate :positions_complete validate :verify_supported + after_initialize :ensure_original_discussion_id before_validation :set_original_position, :update_position, on: :create - before_validation :set_line_code + before_validation :set_line_code, :set_original_discussion_id after_save :keep_around_commits class << self @@ -30,14 +32,6 @@ class DiffNote < Note { position: position.to_json } end - def discussion_id - @discussion_id ||= self.class.build_discussion_id(noteable_type, noteable_id || commit_id, position) - end - - def original_discussion_id - @original_discussion_id ||= self.class.build_discussion_id(noteable_type, noteable_id || commit_id, original_position) - end - def position=(new_position) if new_position.is_a?(String) new_position = JSON.parse(new_position) rescue nil @@ -72,10 +66,48 @@ class DiffNote < Note self.position.diff_refs == diff_refs end + def resolvable? + !system? && for_merge_request? + end + + def resolved? + return false unless resolvable? + + self.resolved_at.present? + end + + def resolve!(current_user) + return unless resolvable? + return if resolved? + + self.resolved_at = Time.now + self.resolved_by = current_user + save! + end + + def unresolve! + return unless resolvable? + return unless resolved? + + self.resolved_at = nil + self.resolved_by = nil + save! + end + + def discussion + return unless resolvable? + + self.noteable.find_diff_discussion(self.discussion_id) + end + + def to_discussion + Discussion.new([self]) + end + private def supported? - !self.for_merge_request? || self.noteable.has_complete_diff_refs? + for_commit? || self.noteable.has_complete_diff_refs? end def noteable_diff_refs @@ -94,6 +126,26 @@ class DiffNote < Note self.line_code = self.position.line_code(self.project.repository) end + def ensure_original_discussion_id + return unless self.persisted? + return if self.original_discussion_id + + set_original_discussion_id + update_column(:original_discussion_id, self.original_discussion_id) + end + + def set_original_discussion_id + self.original_discussion_id = Digest::SHA1.hexdigest(build_original_discussion_id) + end + + def build_discussion_id + self.class.build_discussion_id(noteable_type, noteable_id || commit_id, position) + end + + def build_original_discussion_id + self.class.build_discussion_id(noteable_type, noteable_id || commit_id, original_position) + end + def update_position return unless supported? return if for_commit? diff --git a/app/models/discussion.rb b/app/models/discussion.rb index e2218a5f02b..3fddc084af2 100644 --- a/app/models/discussion.rb +++ b/app/models/discussion.rb @@ -1,7 +1,7 @@ class Discussion NUMBER_OF_TRUNCATED_DIFF_LINES = 16 - attr_reader :first_note, :notes + attr_reader :first_note, :last_note, :notes delegate :created_at, :project, @@ -18,6 +18,12 @@ class Discussion to: :first_note + delegate :resolved_at, + :resolved_by, + + to: :last_resolved_note, + allow_nil: true + delegate :blob, :highlighted_diff_lines, to: :diff_file, allow_nil: true def self.for_notes(notes) @@ -30,13 +36,30 @@ class Discussion def initialize(notes) @first_note = notes.first + @last_note = notes.last @notes = notes end + def last_resolved_note + return unless resolved? + + @last_resolved_note ||= resolved_notes.sort_by(&:resolved_at).last + end + + def last_updated_at + last_note.created_at + end + + def last_updated_by + last_note.author + end + def id first_note.discussion_id end + alias_method :to_param, :id + def diff_discussion? first_note.diff_note? end @@ -45,6 +68,50 @@ class Discussion notes.any?(&:legacy_diff_note?) end + def resolvable? + return @resolvable if defined?(@resolvable) + + @resolvable = diff_discussion? && notes.any?(&:resolvable?) + end + + def resolved? + return @resolved if defined?(@resolved) + + @resolved = resolvable? && notes.none?(&:to_be_resolved?) + end + + def resolved_notes + notes.select(&:resolved?) + end + + def to_be_resolved? + resolvable? && !resolved? + end + + def can_resolve?(current_user) + return false unless current_user + return false unless resolvable? + + current_user == self.noteable.author || + current_user.can?(:resolve_note, self.project) + end + + def resolve!(current_user) + return unless resolvable? + + notes.each do |note| + note.resolve!(current_user) if note.resolvable? + end + end + + def unresolve! + return unless resolvable? + + notes.each do |note| + note.unresolve! if note.resolvable? + end + end + def for_target?(target) self.noteable == target && !diff_discussion? end @@ -55,8 +122,20 @@ class Discussion @active = first_note.active? end + def collapsed? + return false unless diff_discussion? + + if resolvable? + # New diff discussions only disappear once they are marked resolved + resolved? + else + # Old diff discussions disappear once they become outdated + !active? + end + end + def expanded? - !diff_discussion? || active? + !collapsed? end def reply_attributes diff --git a/app/models/legacy_diff_note.rb b/app/models/legacy_diff_note.rb index 6ed66001513..8e26cbe9835 100644 --- a/app/models/legacy_diff_note.rb +++ b/app/models/legacy_diff_note.rb @@ -8,8 +8,8 @@ class LegacyDiffNote < Note before_create :set_diff class << self - def build_discussion_id(noteable_type, noteable_id, line_code, active = true) - [super(noteable_type, noteable_id), line_code, active].join("-") + def build_discussion_id(noteable_type, noteable_id, line_code) + [super(noteable_type, noteable_id), line_code].join("-") end end @@ -21,10 +21,6 @@ class LegacyDiffNote < Note { line_code: line_code } end - def discussion_id - @discussion_id ||= self.class.build_discussion_id(noteable_type, noteable_id || commit_id, line_code) - end - def project_repository if RequestStore.active? RequestStore.fetch("project:#{project_id}:repository") { self.project.repository } @@ -119,4 +115,8 @@ class LegacyDiffNote < Note diffs = noteable.raw_diffs(Commit.max_diff_options) diffs.find { |d| d.new_path == self.diff.new_path } end + + def build_discussion_id + self.class.build_discussion_id(noteable_type, noteable_id || commit_id, line_code) + end end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 4304ef04767..5330a07ee35 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -418,6 +418,32 @@ class MergeRequest < ActiveRecord::Base ) end + def discussions + @discussions ||= self.mr_and_commit_notes. + inc_relations_for_view. + fresh. + discussions + end + + def diff_discussions + @diff_discussions ||= self.notes.diff_notes.discussions + end + + def find_diff_discussion(discussion_id) + notes = self.notes.diff_notes.where(discussion_id: discussion_id).fresh.to_a + return if notes.empty? + + Discussion.new(notes) + end + + def discussions_resolvable? + diff_discussions.any?(&:resolvable?) + end + + def discussions_resolved? + discussions_resolvable? && diff_discussions.none?(&:to_be_resolved?) + end + def hook_attrs attrs = { source: source_project.try(:hook_attrs), diff --git a/app/models/note.rb b/app/models/note.rb index ddcd7f9d034..3bbf5db0b70 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -25,6 +25,9 @@ class Note < ActiveRecord::Base belongs_to :author, class_name: "User" belongs_to :updated_by, class_name: "User" + # Only used by DiffNote, but defined here so that it can be used in `Note.includes` + belongs_to :resolved_by, class_name: "User" + has_many :todos, dependent: :destroy has_many :events, as: :target, dependent: :destroy @@ -59,7 +62,7 @@ class Note < ActiveRecord::Base scope :fresh, ->{ order(created_at: :asc, id: :asc) } scope :inc_author_project, ->{ includes(:project, :author) } scope :inc_author, ->{ includes(:author) } - scope :inc_author_project_award_emoji, ->{ includes(:project, :author, :award_emoji) } + scope :inc_relations_for_view, ->{ includes(:project, :author, :updated_by, :resolved_by, :award_emoji) } scope :diff_notes, ->{ where(type: ['LegacyDiffNote', 'DiffNote']) } scope :non_diff_notes, ->{ where(type: ['Note', nil]) } @@ -70,7 +73,9 @@ class Note < ActiveRecord::Base project: [:project_members, { group: [:group_members] }]) end + after_initialize :ensure_discussion_id before_validation :nullify_blank_type, :nullify_blank_line_code + before_validation :set_discussion_id after_save :keep_around_commit class << self @@ -82,13 +87,18 @@ class Note < ActiveRecord::Base [:discussion, noteable_type.try(:underscore), noteable_id].join("-") end + def discussion_id(*args) + Digest::SHA1.hexdigest(build_discussion_id(*args)) + end + def discussions Discussion.for_notes(all) end def grouped_diff_discussions - notes = diff_notes.fresh.select(&:active?) - Discussion.for_diff_notes(notes).map { |d| [d.line_code, d] }.to_h + active_notes = diff_notes.fresh.select(&:active?) + Discussion.for_diff_notes(active_notes). + map { |d| [d.line_code, d] }.to_h end # Searches for notes matching the given query. @@ -129,13 +139,16 @@ class Note < ActiveRecord::Base true end - def discussion_id - @discussion_id ||= - if for_merge_request? - [:discussion, :note, id].join("-") - else - self.class.build_discussion_id(noteable_type, noteable_id || commit_id) - end + def resolvable? + false + end + + def resolved? + false + end + + def to_be_resolved? + resolvable? && !resolved? end def max_attachment_size @@ -243,4 +256,26 @@ class Note < ActiveRecord::Base def nullify_blank_line_code self.line_code = nil if self.line_code.blank? end + + def ensure_discussion_id + return unless self.persisted? + return if self.discussion_id + + set_discussion_id + update_column(:discussion_id, self.discussion_id) + end + + def set_discussion_id + self.discussion_id = Digest::SHA1.hexdigest(build_discussion_id) + end + + def build_discussion_id + if for_merge_request? + # Notes on merge requests are always in a discussion of their own, + # so we generate a unique discussion ID. + [:discussion, :note, SecureRandom.hex].join("-") + else + self.class.build_discussion_id(noteable_type, noteable_id || commit_id) + end + end end diff --git a/app/services/merge_requests/resolved_discussion_notification_service.rb b/app/services/merge_requests/resolved_discussion_notification_service.rb new file mode 100644 index 00000000000..3a09350c847 --- /dev/null +++ b/app/services/merge_requests/resolved_discussion_notification_service.rb @@ -0,0 +1,10 @@ +module MergeRequests + class ResolvedDiscussionNotificationService < MergeRequests::BaseService + def execute(merge_request) + return unless merge_request.discussions_resolved? + + SystemNoteService.resolve_all_discussions(merge_request, project, current_user) + notification_service.resolve_all_discussions(merge_request, current_user) + end + end +end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index 2291bc0f127..66a838b3d13 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -148,6 +148,14 @@ class NotificationService ) end + def resolve_all_discussions(merge_request, current_user) + recipients = build_recipients(merge_request, merge_request.target_project, current_user, action: "resolve_all_discussions") + + recipients.each do |recipient| + mailer.resolved_all_discussions_email(recipient.id, merge_request.id, current_user.id).deliver_later + end + end + # Notify new user with email after creation def new_user(user, token = nil) # Don't email omniauth created users diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index e13dc9265b8..546a8f11330 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -158,6 +158,12 @@ module SystemNoteService create_note(noteable: noteable, project: project, author: author, note: body) end + def self.resolve_all_discussions(merge_request, project, author) + body = "Resolved all discussions" + + create_note(noteable: merge_request, project: project, author: author, note: body) + end + # Called when the title of a Noteable is changed # # noteable - Noteable object that responds to `title` diff --git a/app/views/discussions/_diff_discussion.html.haml b/app/views/discussions/_diff_discussion.html.haml index fa1ad9efa73..1411daeb4a6 100644 --- a/app/views/discussions/_diff_discussion.html.haml +++ b/app/views/discussions/_diff_discussion.html.haml @@ -1,6 +1,6 @@ -%tr.notes_holder +- expanded = local_assigns.fetch(:expanded, true) +%tr.notes_holder{class: ('hide' unless expanded)} %td.notes_line{ colspan: 2 } %td.notes_content - %ul.notes{ data: { discussion_id: discussion.id } } - = render partial: "projects/notes/note", collection: discussion.notes, as: :note - = link_to_reply_discussion(discussion) + .content + = render "discussions/notes", discussion: discussion diff --git a/app/views/discussions/_diff_with_notes.html.haml b/app/views/discussions/_diff_with_notes.html.haml index 02b159ffd45..b2e55f7647a 100644 --- a/app/views/discussions/_diff_with_notes.html.haml +++ b/app/views/discussions/_diff_with_notes.html.haml @@ -7,8 +7,11 @@ .diff-content.code.js-syntax-highlight %table - - discussion.truncated_diff_lines.each do |line| - = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true - - - if discussion.for_line?(line) - = render "discussions/diff_discussion", discussion: discussion + - discussions = { discussion.line_code => discussion } + = render partial: "projects/diffs/line", + collection: discussion.truncated_diff_lines, + as: :line, + locals: { diff_file: diff_file, + discussions: discussions, + discussion_expanded: true, + plain: true } diff --git a/app/views/discussions/_discussion.html.haml b/app/views/discussions/_discussion.html.haml index 49702e048aa..077e8e64e5f 100644 --- a/app/views/discussions/_discussion.html.haml +++ b/app/views/discussions/_discussion.html.haml @@ -5,8 +5,17 @@ = link_to user_path(discussion.author) do = image_tag avatar_icon(discussion.author), class: "avatar s40" .timeline-content - .discussion.js-toggle-container{ class: discussion.id } + .discussion.js-toggle-container{ class: discussion.id, data: { discussion_id: discussion.id } } .discussion-header + .discussion-actions + = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do + - if expanded + = icon("chevron-up") + - else + = icon("chevron-down") + + Toggle discussion + = link_to_member(@project, discussion.author, avatar: false) .inline.discussion-headline-light @@ -29,17 +38,11 @@ = time_ago_with_tooltip(discussion.created_at, placement: "bottom", html_class: "note-created-ago") - .discussion-actions - = link_to "#", class: "note-action-button discussion-toggle-button js-toggle-button" do - - if expanded - = icon("chevron-up") - - else - = icon("chevron-down") - - Toggle discussion + = render "discussions/headline", discussion: discussion .discussion-body.js-toggle-content{ class: ("hide" unless expanded) } - if discussion.diff_discussion? && discussion.diff_file = render "discussions/diff_with_notes", discussion: discussion - else - = render "discussions/notes", discussion: discussion + .panel.panel-default + = render "discussions/notes", discussion: discussion diff --git a/app/views/discussions/_headline.html.haml b/app/views/discussions/_headline.html.haml new file mode 100644 index 00000000000..c1dabeed387 --- /dev/null +++ b/app/views/discussions/_headline.html.haml @@ -0,0 +1,14 @@ +- if discussion.resolved? + .discussion-headline-light.js-discussion-headline + Resolved + - if discussion.resolved_by + by + = link_to_member(@project, discussion.resolved_by, avatar: false) + = time_ago_with_tooltip(discussion.resolved_at, placement: "bottom") +- elsif discussion.last_updated_at != discussion.created_at + .discussion-headline-light.js-discussion-headline + Last updated + - if discussion.last_updated_by + by + = link_to_member(@project, discussion.last_updated_by, avatar: false) + = time_ago_with_tooltip(discussion.last_updated_at, placement: "bottom") diff --git a/app/views/discussions/_jump_to_next.html.haml b/app/views/discussions/_jump_to_next.html.haml new file mode 100644 index 00000000000..69bd416c4de --- /dev/null +++ b/app/views/discussions/_jump_to_next.html.haml @@ -0,0 +1,9 @@ +- discussion = local_assigns.fetch(:discussion, nil) +- if current_user + %jump-to-discussion{ "inline-template" => true, ":discussion-id" => "'#{discussion.try(:id)}'" } + .btn-group{ role: "group", "v-show" => "!allResolved", "v-if" => "showButton" } + %button.btn.btn-default.discussion-next-btn.has-tooltip{ "@click" => "jumpToNextUnresolvedDiscussion", + title: "Jump to next unresolved discussion", + "aria-label" => "Jump to next unresolved discussion", + data: { container: "body" } } + = custom_icon("next_discussion") diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml index a2642b839f6..fbe470bed2c 100644 --- a/app/views/discussions/_notes.html.haml +++ b/app/views/discussions/_notes.html.haml @@ -1,5 +1,15 @@ -.panel.panel-default - .notes{ data: { discussion_id: discussion.id } } - %ul.notes.timeline - = render partial: "projects/notes/note", collection: discussion.notes, as: :note - = link_to_reply_discussion(discussion) +%ul.notes{ data: { discussion_id: discussion.id } } + = render partial: "projects/notes/note", collection: discussion.notes, as: :note + +- if current_user + .discussion-reply-holder + - if discussion.diff_discussion? + - line_type = local_assigns.fetch(:line_type, nil) + + .btn-group-justified.discussion-with-resolve-btn{ role: "group" } + .btn-group{ role: "group" } + = link_to_reply_discussion(discussion, line_type) + = render "discussions/resolve_all", discussion: discussion + = render "discussions/jump_to_next", discussion: discussion + - else + = link_to_reply_discussion(discussion) diff --git a/app/views/discussions/_parallel_diff_discussion.html.haml b/app/views/discussions/_parallel_diff_discussion.html.haml index a798c438ea0..f1072ce0feb 100644 --- a/app/views/discussions/_parallel_diff_discussion.html.haml +++ b/app/views/discussions/_parallel_diff_discussion.html.haml @@ -1,22 +1,21 @@ -%tr.notes_holder +- expanded = discussion_left.try(:expanded?) || discussion_right.try(:expanded?) +%tr.notes_holder{class: ('hide' unless expanded)} - if discussion_left %td.notes_line.old %td.notes_content.parallel.old - %ul.notes{ data: { discussion_id: discussion_left.id } } - = render partial: "projects/notes/note", collection: discussion_left.notes, as: :note - - = link_to_reply_discussion(discussion_left, 'old') + .content{class: ('hide' unless discussion_left.expanded?)} + = render "discussions/notes", discussion: discussion_left, line_type: 'old' - else %td.notes_line.old= "" - %td.notes_content.parallel.old= "" + %td.notes_content.parallel.old + .content - if discussion_right %td.notes_line.new %td.notes_content.parallel.new - %ul.notes{ data: { discussion_id: discussion_right.id } } - = render partial: "projects/notes/note", collection: discussion_right.notes, as: :note - - = link_to_reply_discussion(discussion_right, 'new') + .content{class: ('hide' unless discussion_right.expanded?)} + = render "discussions/notes", discussion: discussion_right, line_type: 'new' - else %td.notes_line.new= "" - %td.notes_content.parallel.new= "" + %td.notes_content.parallel.new + .content diff --git a/app/views/discussions/_resolve_all.html.haml b/app/views/discussions/_resolve_all.html.haml new file mode 100644 index 00000000000..7a8767ddba0 --- /dev/null +++ b/app/views/discussions/_resolve_all.html.haml @@ -0,0 +1,11 @@ +- if discussion.for_merge_request? + %resolve-discussion-btn{ ":namespace-path" => "'#{discussion.project.namespace.path}'", + ":project-path" => "'#{discussion.project.path}'", + ":discussion-id" => "'#{discussion.id}'", + ":merge-request-id" => discussion.noteable.iid, + ":can-resolve" => discussion.can_resolve?(current_user), + "inline-template" => true } + .btn-group{ role: "group", "v-if" => "showButton" } + %button.btn.btn-default{ type: "button", "@click" => "resolve", ":disabled" => "loading" } + = icon("spinner spin", "v-show" => "loading") + {{ buttonText }} diff --git a/app/views/notify/repository_push_email.html.haml b/app/views/notify/repository_push_email.html.haml index 4dc39a72225..c0c07d65daa 100644 --- a/app/views/notify/repository_push_email.html.haml +++ b/app/views/notify/repository_push_email.html.haml @@ -75,8 +75,7 @@ - blob = diff_file.blob - if blob && blob.respond_to?(:text?) && blob_text_viewable?(blob) %table.code.white - - diff_file.highlighted_diff_lines.each do |line| - = render "projects/diffs/line", line: line, diff_file: diff_file, plain: true, email: true + = render partial: "projects/diffs/line", collection: diff_file.highlighted_diff_lines, as: :line, locals: { diff_file: diff_file, plain: true, email: true } - else No preview for this file type %br diff --git a/app/views/notify/resolved_all_discussions_email.html.haml b/app/views/notify/resolved_all_discussions_email.html.haml new file mode 100644 index 00000000000..522421b7cc3 --- /dev/null +++ b/app/views/notify/resolved_all_discussions_email.html.haml @@ -0,0 +1,2 @@ +%p + All discussions on Merge Request #{@merge_request.to_reference} were resolved by #{@resolved_by.name} diff --git a/app/views/notify/resolved_all_discussions_email.text.erb b/app/views/notify/resolved_all_discussions_email.text.erb new file mode 100644 index 00000000000..b0d380af8fc --- /dev/null +++ b/app/views/notify/resolved_all_discussions_email.text.erb @@ -0,0 +1,3 @@ +All discussions on Merge Request <%= @merge_request.to_reference %> were resolved by <%= @resolved_by.name %> + +<%= url_for(namespace_project_merge_request_url(@merge_request.target_project.namespace, @merge_request.target_project, @merge_request)) %> diff --git a/app/views/projects/diffs/_line.html.haml b/app/views/projects/diffs/_line.html.haml index 891b2bd9802..7042e9f1fc9 100644 --- a/app/views/projects/diffs/_line.html.haml +++ b/app/views/projects/diffs/_line.html.haml @@ -1,7 +1,7 @@ - email = local_assigns.fetch(:email, false) - plain = local_assigns.fetch(:plain, false) - type = line.type -- line_code = diff_file.line_code(line) unless plain +- line_code = diff_file.line_code(line) %tr.line_holder{ plain ? { class: type} : { class: type, id: line_code } } - case type - when 'match' @@ -28,3 +28,10 @@ %pre= diff_line_content(line.text, type) - else = diff_line_content(line.text, type) + +- discussions = local_assigns.fetch(:discussions, nil) +- if discussions && !line.meta? + - discussion = discussions[line_code] + - if discussion + - discussion_expanded = local_assigns.fetch(:discussion_expanded, discussion.expanded?) + = render "discussions/diff_discussion", discussion: discussion, expanded: discussion_expanded diff --git a/app/views/projects/diffs/_text_file.html.haml b/app/views/projects/diffs/_text_file.html.haml index ab5463ba89d..f1d2d4bf268 100644 --- a/app/views/projects/diffs/_text_file.html.haml +++ b/app/views/projects/diffs/_text_file.html.haml @@ -5,15 +5,12 @@ %table.text-file.code.js-syntax-highlight{ data: diff_view_data, class: too_big ? 'hide' : '' } - last_line = 0 - - diff_file.highlighted_diff_lines.each do |line| - - last_line = line.new_pos - = render "projects/diffs/line", line: line, diff_file: diff_file - - - unless @diff_notes_disabled - - line_code = diff_file.line_code(line) - - discussion = @grouped_diff_discussions[line_code] if line_code - - if discussion - = render "discussions/diff_discussion", discussion: discussion + - discussions = @grouped_diff_discussions unless @diff_notes_disabled + = render partial: "projects/diffs/line", + collection: diff_file.highlighted_diff_lines, + as: :line, + locals: { diff_file: diff_file, discussions: discussions } + - last_line = diff_file.highlighted_diff_lines.last.new_pos - if !diff_file.new_file && last_line > 0 = diff_match_line last_line, last_line, bottom: true diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml index 53dd300c35c..d070979bcfe 100644 --- a/app/views/projects/merge_requests/_discussion.html.haml +++ b/app/views/projects/merge_requests/_discussion.html.haml @@ -4,5 +4,8 @@ = link_to 'Close merge request', merge_request_path(@merge_request, merge_request: {state_event: :close }), method: :put, class: "btn btn-nr btn-comment btn-close close-mr-link js-note-target-close", title: "Close merge request", data: {original_text: "Close merge request", alternative_text: "Comment & close merge request"} - if @merge_request.closed? = link_to 'Reopen merge request', merge_request_path(@merge_request, merge_request: {state_event: :reopen }), method: :put, class: "btn btn-nr btn-comment btn-reopen reopen-mr-link js-note-target-reopen", title: "Reopen merge request", data: {original_text: "Reopen merge request", alternative_text: "Comment & reopen merge request"} + %comment-and-resolve-btn{ "inline-template" => true, ":discussion-id" => "" } + %button.btn.btn-nr.btn-default.append-right-10.js-comment-resolve-button{ "v-if" => "showButton", type: "submit", data: { namespace_path: "#{@merge_request.project.namespace.path}", project_path: "#{@merge_request.project.path}" } } + {{ buttonText }} #notes= render "projects/notes/notes_with_form" diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index a1313064725..f8025fc1dbe 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -1,6 +1,8 @@ - page_title "#{@merge_request.title} (#{@merge_request.to_reference})", "Merge Requests" - page_description @merge_request.description - page_card_attributes @merge_request.card_attributes +- content_for :page_specific_javascripts do + = page_specific_javascript_tag('diff_notes/diff_notes_bundle.js') - if diff_view == :parallel - fluid_layout true @@ -65,8 +67,18 @@ = link_to diffs_namespace_project_merge_request_path(@project.namespace, @project, @merge_request), data: { target: 'div#diffs', action: 'diffs', toggle: 'tab' } do Changes %span.badge= @merge_request.diff_size + %li#resolve-count-app.line-resolve-all-container.pull-right.prepend-top-10.hidden-xs{ "v-cloak" => true } + %resolve-count{ "inline-template" => true, ":logged-out" => "#{current_user.nil?}" } + .line-resolve-all{ "v-show" => "discussionCount > 0", + ":class" => "{ 'has-next-btn': !loggedOut && resolvedDiscussionCount !== discussionCount }" } + %span.line-resolve-btn.is-disabled{ type: "button", + ":class" => "{ 'is-active': resolvedDiscussionCount === discussionCount }" } + = render "shared/icons/icon_status_success.svg" + %span.line-resolve-text + {{ resolvedDiscussionCount }}/{{ discussionCount }} {{ discussionCount | pluralize 'discussion' }} resolved + = render "discussions/jump_to_next" - .tab-content + .tab-content#diff-notes-app #notes.notes.tab-pane.voting_notes .content-block.content-block-small.oneline-block = render 'award_emoji/awards_block', awardable: @merge_request, inline: true diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml index 759c72b2477..402f5b52f5e 100644 --- a/app/views/projects/notes/_form.html.haml +++ b/app/views/projects/notes/_form.html.haml @@ -1,4 +1,4 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new-note js-new-note-form js-quick-submit common-note-form" }, authenticity_token: true do |f| += form_for [@project.namespace.becomes(Namespace), @project, @note], remote: true, html: { :'data-type' => 'json', multipart: true, id: nil, class: "new-note js-new-note-form js-quick-submit common-note-form", "data-noteable-iid" => @note.noteable.try(:iid), }, authenticity_token: true do |f| = hidden_field_tag :view, diff_view = hidden_field_tag :line_type = note_target_fields(@note) diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 71da8ac9d7c..d2ac1ce2b9a 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -1,5 +1,6 @@ - return unless note.author - return if note.cross_reference_not_visible_for?(current_user) +- can_resolve = can?(current_user, :resolve_note, note) - note_editable = note_editable?(note) %li.timeline-entry{ id: dom_id(note), class: ["note", "note-row-#{note.id}", ('system-note' if note.system)], data: {author_id: note.author.id, editable: note_editable} } @@ -16,19 +17,48 @@ commented %a{ href: "##{dom_id(note)}" } = time_ago_with_tooltip(note.created_at, placement: 'bottom', html_class: 'note-created-ago') - .note-actions - - access = note_max_access_for_user(note) - - if access and not note.system - %span.note-role.hidden-xs= access - - if current_user and not note.system - = link_to '#', title: 'Award Emoji', class: 'note-action-button note-emoji-button js-add-award js-note-emoji', data: { position: 'right' } do - = icon('spinner spin') - = icon('smile-o') - - if note_editable - = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do - = icon('pencil') - = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button hidden-xs js-note-delete danger' do - = icon('trash-o') + - unless note.system? + .note-actions + - access = note_max_access_for_user(note) + - if access + %span.note-role.hidden-xs= access + + - if note.resolvable? + %resolve-btn{ ":namespace-path" => "'#{note.project.namespace.path}'", + ":project-path" => "'#{note.project.path}'", + ":discussion-id" => "'#{note.discussion_id}'", + ":note-id" => note.id, + ":resolved" => note.resolved?, + ":can-resolve" => can_resolve, + ":resolved-by" => "'#{note.resolved_by.try(:name)}'", + "v-show" => "#{can_resolve || note.resolved?}", + "inline-template" => true, + "v-ref:note_#{note.id}" => true } + + .note-action-button + = icon("spin spinner", "v-show" => "loading") + %button.line-resolve-btn{ type: "button", + class: ("is-disabled" unless can_resolve), + ":class" => "{ 'is-active': isResolved }", + ":aria-label" => "buttonText", + "@click" => "resolve", + ":title" => "buttonText", + "v-show" => "!loading", + "v-el:button" => true } + + = render "shared/icons/icon_status_success.svg" + + - if current_user + - if note.emoji_awardable? + = link_to '#', title: 'Award Emoji', class: 'note-action-button note-emoji-button js-add-award js-note-emoji', data: { position: 'right' } do + = icon('spinner spin') + = icon('smile-o') + + - if note_editable + = link_to '#', title: 'Edit comment', class: 'note-action-button js-note-edit' do + = icon('pencil') + = link_to namespace_project_note_path(note.project.namespace, note.project, note), title: 'Remove comment', method: :delete, data: { confirm: 'Are you sure you want to remove this comment?' }, remote: true, class: 'note-action-button hidden-xs js-note-delete danger' do + = icon('trash-o') .note-body{class: note_editable ? 'js-task-list-container' : ''} .note-text.md = preserve do diff --git a/app/views/shared/icons/_next_discussion.svg b/app/views/shared/icons/_next_discussion.svg new file mode 100644 index 00000000000..43559a60cb0 --- /dev/null +++ b/app/views/shared/icons/_next_discussion.svg @@ -0,0 +1 @@ +<svg viewBox="0 0 20 19" ><path d="M15.21 7.783h-3.317c-.268 0-.472.218-.472.486v.953c0 .28.212.486.473.486h3.318v1.575c0 .36.233.452.52.23l3.06-2.37c.274-.213.286-.582 0-.804l-3.06-2.37c-.275-.213-.52-.12-.52.23v1.583zm.57-3.66c-1.558-1.22-3.783-1.98-6.254-1.98C4.816 2.143 1 4.91 1 8.333c0 1.964 1.256 3.715 3.216 4.846-.447 1.615-1.132 2.195-1.732 2.882-.142.174-.304.32-.256.56v.01c.047.213.218.368.41.368h.046c.37-.048.743-.116 1.085-.213 1.645-.425 3.13-1.22 4.377-2.34.447.048.913.077 1.38.077 2.092 0 4.01-.546 5.492-1.454-.416-.208-.798-.475-1.134-.792-1.227.63-2.743 1.008-4.36 1.008-.41 0-.828-.03-1.237-.078l-.543-.058-.41.368c-.78.696-1.655 1.248-2.616 1.654.248-.445.486-.977.667-1.664l.257-.928-.828-.484c-1.646-.948-2.598-2.32-2.598-3.763 0-2.69 3.35-4.952 7.308-4.952 1.893 0 3.647.518 4.962 1.353.393-.266.827-.473 1.29-.61z" /></svg> |