diff options
13 files changed, 107 insertions, 87 deletions
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 index cd696a3dae3..42fde1b1011 100644 --- a/app/assets/javascripts/diff_notes/components/comment_resolve_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/comment_resolve_btn.js.es6 @@ -2,25 +2,31 @@ w.CommentAndResolveBtn = Vue.extend({ props: { discussionId: String, - textareaVal: String + textareaIsEmpty: Boolean }, computed: { + discussion: function () { + return CommentsStore.state[this.discussionId]; + }, + showButton: function () { + if (!this.discussion) { + return false; + } else { + return this.discussion.canResolve(); + } + }, isDiscussionResolved: function () { - const discussion = CommentsStore.state[this.discussionId]; - - return discussion.isResolved(); + return this.discussion.isResolved(); }, buttonText: function () { - const textVal = this.textareaVal; - if (this.isDiscussionResolved) { - if (textVal === '') { + if (this.textareaIsEmpty) { return "Unresolve discussion"; } else { return "Comment & unresolve discussion"; } } else { - if (textVal === '') { + if (this.textareaIsEmpty) { return "Resolve discussion"; } else { return "Comment & resolve discussion"; @@ -30,10 +36,10 @@ }, ready: function () { const $textarea = $(`#new-discussion-note-form-${this.discussionId} .note-textarea`); - this.textareaVal = $textarea.val(); + this.textareaIsEmpty = $textarea.val() === ''; $textarea.on('input.comment-and-resolve-btn', () => { - this.textareaVal = $textarea.val(); + this.textareaIsEmpty = $textarea.val() === ''; }); }, destroyed: function () { 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 index 4c29786cc9e..2b487fde313 100644 --- a/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 +++ b/app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 @@ -1,5 +1,6 @@ (() => { JumpToDiscussion = Vue.extend({ + mixins: [DiscussionMixins], props: { discussionId: String }, @@ -9,41 +10,25 @@ }; }, computed: { - allResolved: function () { - const discussion = this.discussions[this.discussionId]; - - if (discussion) { - return discussion.isResolved(); - } + discussion: function () { + return this.discussions[this.discussionId]; }, - discussionsCount: function () { - return CommentsStore.discussionCount(); - }, - unresolvedDiscussionCount: function () { - let unresolvedCount = 0; - for (const discussionId in this.discussions) { - const discussion = this.discussions[discussionId]; - - if (!discussion.isResolved()) { - unresolvedCount++; - } + allResolved: function () { + if (this.discussion) { + return this.unresolvedDiscussionCount === 0; } - - return unresolvedCount; }, showButton: function () { if (this.discussionId) { if (this.unresolvedDiscussionCount > 1) { return true; } else { - return this.discussionId !== this.lastResolvedId(); + return this.discussionId !== this.lastResolvedId; } } else { return this.unresolvedDiscussionCount >= 1; } - } - }, - methods: { + }, lastResolvedId: function () { let lastId; for (const discussionId in this.discussions) { @@ -54,7 +39,9 @@ } } return lastId; - }, + } + }, + methods: { jumpToNextUnresolvedDiscussion: function () { let nextUnresolvedDiscussionId, firstUnresolvedDiscussionId, @@ -82,9 +69,7 @@ } } - if (!nextUnresolvedDiscussionId && firstUnresolvedDiscussionId) { - nextUnresolvedDiscussionId = firstUnresolvedDiscussionId; - } + nextUnresolvedDiscussionId = nextUnresolvedDiscussionId || firstUnresolvedDiscussionId if (nextUnresolvedDiscussionId) { mrTabs.activateTab('notes'); diff --git a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 index e1611928049..26a29d10bdb 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 @@ -29,7 +29,9 @@ return this.discussions[this.discussionId]; }, note: function () { - return CommentsStore.get(this.discussionId, this.noteId); + if (this.discussion) { + return this.discussion.getNote(this.noteId); + } }, buttonText: function () { if (this.isResolved) { @@ -39,7 +41,9 @@ } }, isResolved: function () { - return this.note.resolved; + if (this.note) { + return this.note.resolved; + } }, resolvedByName: function () { return this.note.resolved_by; @@ -87,11 +91,11 @@ container: 'body' }); }, - destroyed: function () { + beforeDestroy: function () { CommentsStore.delete(this.discussionId, this.noteId); }, created: function () { - CommentsStore.create(this.discussionId, this.noteId, this.resolved, this.resolvedBy); + 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 index 51674e0e06d..c69a8a4542f 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 @@ -1,31 +1,15 @@ ((w) => { w.ResolveCount = Vue.extend({ + mixins: [DiscussionMixins], props: { loggedOut: Boolean }, data: function () { return { - discussions: CommentsStore.state, - loading: false + discussions: CommentsStore.state }; }, computed: { - resolved: function () { - let resolvedCount = 0; - - for (const discussionId in this.discussions) { - const discussion = this.discussions[discussionId]; - - if (discussion.isResolved()) { - resolvedCount++; - } - } - - return resolvedCount; - }, - discussionCount: function () { - return Object.keys(this.discussions).length; - }, allResolved: function () { return this.resolved === this.discussionCount; } 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 index 1eaabdaf81e..78af33bb430 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6 @@ -19,7 +19,9 @@ return this.discussions[this.discussionId]; }, allResolved: function () { - return this.discussion.isResolved(); + if (this.discussion) { + return this.discussion.isResolved(); + } }, buttonText: function () { if (this.allResolved) { @@ -29,7 +31,9 @@ } }, loading: function () { - return this.discussion.loading; + if (this.discussion) { + return this.discussion.loading; + } } }, methods: { 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..34f3b200e48 --- /dev/null +++ b/app/assets/javascripts/diff_notes/mixins/discussion.js.es6 @@ -0,0 +1,34 @@ +((w) => { + w.DiscussionMixins = { + computed: { + discussionCount: function () { + return Object.keys(this.discussions).length; + }, + resolved: 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/models/discussion.js.es6 b/app/assets/javascripts/diff_notes/models/discussion.js.es6 index 615e8d12e5e..08b5f7062ca 100644 --- a/app/assets/javascripts/diff_notes/models/discussion.js.es6 +++ b/app/assets/javascripts/diff_notes/models/discussion.js.es6 @@ -5,8 +5,8 @@ class DiscussionModel { this.loading = false; } - createNote (noteId, resolved, resolved_by) { - Vue.set(this.notes, noteId, new NoteModel(this.id, noteId, resolved, resolved_by)); + createNote (noteId, canResolve, resolved, resolved_by) { + Vue.set(this.notes, noteId, new NoteModel(this.id, noteId, canResolve, resolved, resolved_by)); } deleteNote (noteId) { @@ -67,4 +67,16 @@ class DiscussionModel { $discussionHeadline.remove(); } } + + canResolve () { + 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 index 4cbd57e6ff4..f2d2d389c38 100644 --- a/app/assets/javascripts/diff_notes/models/note.js.es6 +++ b/app/assets/javascripts/diff_notes/models/note.js.es6 @@ -1,7 +1,8 @@ class NoteModel { - constructor (discussionId, noteId, resolved, resolved_by) { + 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 index b9fe6caf7e8..5e5c476b514 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js.es6 +++ b/app/assets/javascripts/diff_notes/services/resolve.js.es6 @@ -9,20 +9,21 @@ Vue.http.headers.common['X-CSRF-Token'] = $.rails.csrfToken(); } - resolve(namespace, noteId) { + prepareRequest(namespace) { this.setCSRF(); if (Vue.http.options.root !== `/${namespace}`) { Vue.http.options.root = `/${namespace}`; } + } + + resolve(namespace, noteId) { + this.prepareRequest(namespace); return this.noteResource.save({ noteId }, {}); } unresolve(namespace, noteId) { - this.setCSRF(); - if (Vue.http.options.root !== `/${namespace}`) { - Vue.http.options.root = `/${namespace}`; - } + this.prepareRequest(namespace); return this.noteResource.delete({ noteId }, {}); } @@ -61,11 +62,7 @@ resolveAll(namespace, mergeRequestId, discussionId) { const discussion = CommentsStore.state[discussionId]; - this.setCSRF(); - - if (Vue.http.options.root !== `/${namespace}`) { - Vue.http.options.root = `/${namespace}`; - } + this.prepareRequest(namespace); discussion.loading = true; @@ -78,8 +75,7 @@ unResolveAll(namespace, mergeRequestId, discussionId) { const discussion = CommentsStore.state[discussionId]; - this.setCSRF(); - Vue.http.options.root = `/${namespace}`; + this.prepareRequest(namespace); discussion.loading = true; diff --git a/app/assets/javascripts/diff_notes/stores/comments.js.es6 b/app/assets/javascripts/diff_notes/stores/comments.js.es6 index 967568efa38..d3882d94cea 100644 --- a/app/assets/javascripts/diff_notes/stores/comments.js.es6 +++ b/app/assets/javascripts/diff_notes/stores/comments.js.es6 @@ -4,14 +4,14 @@ get: function (discussionId, noteId) { return this.state[discussionId].getNote(noteId); }, - create: function (discussionId, noteId, resolved, resolved_by) { + create: function (discussionId, noteId, canResolve, resolved, resolved_by) { let discussion = this.state[discussionId]; if (!this.state[discussionId]) { discussion = new DiscussionModel(discussionId); Vue.set(this.state, discussionId, discussion); } - discussion.createNote(noteId, resolved, resolved_by); + discussion.createNote(noteId, canResolve, resolved, resolved_by); }, update: function (discussionId, noteId, resolved, resolved_by) { const discussion = this.state[discussionId]; @@ -26,9 +26,6 @@ if (discussion.notesCount() === 0) { Vue.delete(this.state, discussionId); } - }, - discussionCount: function () { - return Object.keys(this.state).length } }; })(window); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index fd5163bdd5b..3fc4697cd33 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -577,7 +577,6 @@ */ Notes.prototype.setupDiscussionNoteForm = function(dataHolder, form) { - var canResolve = dataHolder.attr('data-can-resolve'); form.attr('id', "new-discussion-note-form-" + (dataHolder.data("discussionId"))); form.attr("data-line-code", dataHolder.data("lineCode")); form.find("#note_type").val(dataHolder.data("noteType")); @@ -591,9 +590,7 @@ form.find('.js-note-target-close').remove(); this.setupNoteForm(form); - if (canResolve === 'false' || !form.closest('.notes_content').find('.note:not(.system-note)').length) { - form.find('comment-and-resolve-btn').remove(); - } else if (typeof DiffNotesApp !== 'undefined') { + if (typeof DiffNotesApp !== 'undefined') { var $commentBtn = form.find('comment-and-resolve-btn'); $commentBtn .attr(':discussion-id', "'" + dataHolder.data('discussionId') + "'"); diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb index 0bff88f1dd2..6b00ebc98e7 100644 --- a/app/helpers/notes_helper.rb +++ b/app/helpers/notes_helper.rb @@ -79,7 +79,7 @@ module NotesHelper def link_to_reply_discussion(discussion, line_type = nil) return unless current_user - data = discussion.reply_attributes.merge(line_type: line_type, can_resolve: discussion.can_resolve?(current_user)) + data = discussion.reply_attributes.merge(line_type: line_type) button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button', data: data, title: 'Add a reply' diff --git a/app/views/projects/merge_requests/_discussion.html.haml b/app/views/projects/merge_requests/_discussion.html.haml index 218dbbb7951..d070979bcfe 100644 --- a/app/views/projects/merge_requests/_discussion.html.haml +++ b/app/views/projects/merge_requests/_discussion.html.haml @@ -5,7 +5,7 @@ - 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{ type: "submit", data: { namespace_path: "#{@merge_request.project.namespace.path}", project_path: "#{@merge_request.project.path}" } } + %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" |