diff options
author | Phil Hughes <me@iamphill.com> | 2016-07-29 11:19:56 +0100 |
---|---|---|
committer | Phil Hughes <me@iamphill.com> | 2016-07-29 11:19:56 +0100 |
commit | efb74875cfe1a1e2b3696f0129b819fc8e204876 (patch) | |
tree | 53bc2e8a9e45f6c31dcc729bf4646532c1e6b41a /app/assets/javascripts/diff_notes | |
parent | d9a949c17c29c8531b2b0a1c227ab0b3341f0ba3 (diff) | |
download | gitlab-ce-efb74875cfe1a1e2b3696f0129b819fc8e204876.tar.gz |
Moved most of the data handling into discussion & notes models
Reduced some duplicated code with compiling components
Fixed bug with resolve button tooltip not updating after resolving discussion
Diffstat (limited to 'app/assets/javascripts/diff_notes')
-rw-r--r-- | app/assets/javascripts/diff_notes/components/jump_to_discussion.js.es6 | 42 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 | 17 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6 | 16 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/components/resolve_count.js.es6 | 13 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6 (renamed from app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6) | 12 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 | 14 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/models/disucssion.js.es6 | 51 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/models/note.js | 8 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/services/resolve.js.es6 | 21 | ||||
-rw-r--r-- | app/assets/javascripts/diff_notes/stores/comments.js.es6 | 49 |
10 files changed, 132 insertions, 111 deletions
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 a0c859ab7f6..3c30f9df058 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 @@ -10,40 +10,31 @@ }, computed: { allResolved: function () { - let allResolved = true; - for (const discussionId in this.discussions) { - const discussion = this.discussions[discussionId]; - - for (const noteId in discussion) { - const note = discussion[noteId]; - - if (!note.resolved) { - allResolved = false; - } - } - } - - return allResolved; + const discussion = this.discussions[discussionId]; + return discussion.isResolved(); } }, methods: { jumpToNextUnresolvedDiscussion: function () { - let nextUnresolvedDiscussionId; + let nextUnresolvedDiscussionId, + firstUnresolvedDiscussionId; if (!this.discussionId) { + let i = 0; for (const discussionId in this.discussions) { const discussion = this.discussions[discussionId]; + const isResolved = discussion.isResolved(); - for (const noteId in discussion) { - const note = discussion[noteId]; + if (!firstUnresolvedDiscussionId && !isResolved) { + firstUnresolvedDiscussionId = discussionId; + } - if (!note.resolved) { - nextUnresolvedDiscussionId = discussionId; - break; - } + if (!isResolved) { + nextUnresolvedDiscussionId = discussionId; + break; } - if (nextUnresolvedDiscussionId) break; + i++; } } else { const discussionKeys = Object.keys(this.discussions), @@ -52,9 +43,16 @@ if (nextDiscussionId) { nextUnresolvedDiscussionId = nextDiscussionId; + } else { + firstUnresolvedDiscussionId = discussionKeys[0]; } } + if (firstUnresolvedDiscussionId) { + // Jump to first unresolved discussion + nextUnresolvedDiscussionId = firstUnresolvedDiscussionId; + } + if (nextUnresolvedDiscussionId) { $.scrollTo(`.discussion[data-discussion-id="${nextUnresolvedDiscussionId}"]`, { offset: -($('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight()) 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 e077c3ba395..c8cf6b8ad3a 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 @@ -18,7 +18,16 @@ loading: false }; }, + watch: { + 'discussions': { + handler: 'updateTooltip', + deep: true + } + }, computed: { + note: function () { + return CommentsStore.get(this.discussionId, this.noteId); + }, buttonText: function () { if (this.isResolved) { return `Resolved by ${this.resolvedByName}`; @@ -26,8 +35,12 @@ return 'Mark as resolved'; } }, - isResolved: function () { return CommentsStore.get(this.discussionId, this.noteId).resolved; }, - resolvedByName: function () { return CommentsStore.get(this.discussionId, this.noteId).user; }, + isResolved: function () { + return this.note.resolved; + }, + resolvedByName: function () { + return this.note.user; + }, }, methods: { updateTooltip: function () { diff --git a/app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6 index 56514afa6ce..5832435dcfe 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_comment_btn.js.es6 @@ -5,20 +5,10 @@ }, computed: { isDiscussionResolved: function () { - const notes = CommentsStore.notesForDiscussion(this.discussionId), - discussion = CommentsStore.state[this.discussionId]; - let allResolved = true; + const discussion = CommentsStore.state[this.discussionId], + notes = CommentsStore.notesForDiscussion(this.discussionId); - for (let i = 0; i < notes.length; i++) { - const noteId = notes[i]; - const note = discussion[noteId]; - - if (!note.resolved) { - allResolved = false; - } - } - - return allResolved; + return discussion.isResolved(); }, buttonText: function () { if (this.isDiscussionResolved) { 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 b8df89491ad..4d9e1dd8df8 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 @@ -11,18 +11,9 @@ let resolvedCount = 0; for (const discussionId in this.discussions) { - const comments = this.discussions[discussionId]; - let resolved = true; + const discussion = this.discussions[discussionId]; - for (const noteId in comments) { - const commentResolved = comments[noteId].resolved; - - if (!commentResolved) { - resolved = false; - } - } - - if (resolved) { + if (discussion.isResolved()) { resolvedCount++; } } diff --git a/app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6 index 543ed8625f3..47a317a07c4 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6 @@ -1,5 +1,5 @@ ((w) => { - w.ResolveAllBtn = Vue.extend({ + w.ResolveDiscussionBtn = Vue.extend({ mixins: [ ButtonMixins ], @@ -17,15 +17,7 @@ }, computed: { allResolved: function () { - let isResolved = true; - for (const noteId in this.discussions[this.discussionId]) { - const resolved = this.discussions[this.discussionId][noteId].resolved; - - if (!resolved) { - isResolved = false; - } - } - return isResolved; + return this.discussions[this.discussionId].isResolved(); }, buttonText: function () { if (this.allResolved) { diff --git a/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 b/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 index b856c718034..33b7648f0b6 100644 --- a/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 +++ b/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 @@ -1,5 +1,6 @@ //= require vue //= require vue-resource +//= require_directory ./models //= require_directory ./stores //= require_directory ./services //= require_directory ./mixins @@ -10,8 +11,8 @@ $(() => { el: '#diff-notes-app', components: { 'resolve-btn': ResolveBtn, - 'resolve-all-btn': ResolveAllBtn, - 'resolve-comment-btn': ResolveCommentBtn, + 'resolve-discussion-btn': ResolveDiscussionBtn, + 'resolve-comment-btn': ResolveCommentBtn } }); @@ -21,4 +22,13 @@ $(() => { 'resolve-count': ResolveCount } }); + + window.compileVueComponentsForDiffNotes = function () { + const $components = $('resolve-btn, resolve-discussion-btn, jump-to-discussion'); + if ($components.length) { + $components.each(function () { + DiffNotesApp.$compile($(this).get(0)) + }); + } + } }); diff --git a/app/assets/javascripts/diff_notes/models/disucssion.js.es6 b/app/assets/javascripts/diff_notes/models/disucssion.js.es6 new file mode 100644 index 00000000000..bbc940214dd --- /dev/null +++ b/app/assets/javascripts/diff_notes/models/disucssion.js.es6 @@ -0,0 +1,51 @@ +class DiscussionModel { + constructor (discussionId) { + this.discussionId = discussionId; + this.notes = {}; + } + + createNote (noteId, resolved, user) { + Vue.set(this.notes, noteId, new NoteModel(this.discussionId, noteId, resolved, user)); + } + + deleteNote (noteId) { + Vue.delete(this.notes, noteId); + } + + getNote (noteId) { + return this.notes[noteId]; + } + + isResolved () { + for (const noteId in this.notes) { + const note = this.notes[noteId]; + + if (!note.resolved) { + return false; + } + } + return true; + } + + resolveAllNotes (user) { + for (const noteId in this.notes) { + const note = this.notes[noteId]; + + if (!note.resolved) { + note.resolved = true; + note.user = user; + } + } + } + + unResolveAllNotes (user) { + for (const noteId in this.notes) { + const note = this.notes[noteId]; + + if (note.resolved) { + note.resolved = false; + note.user = null; + } + } + } +} diff --git a/app/assets/javascripts/diff_notes/models/note.js b/app/assets/javascripts/diff_notes/models/note.js new file mode 100644 index 00000000000..2772991ce8b --- /dev/null +++ b/app/assets/javascripts/diff_notes/models/note.js @@ -0,0 +1,8 @@ +class NoteModel { + constructor (discussionId, noteId, resolved, user) { + this.discussionId = discussionId; + this.noteId = noteId; + this.resolved = resolved; + this.user = user; + } +} diff --git a/app/assets/javascripts/diff_notes/services/resolve.js.es6 b/app/assets/javascripts/diff_notes/services/resolve.js.es6 index 50392e76fbe..0be938448b2 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js.es6 +++ b/app/assets/javascripts/diff_notes/services/resolve.js.es6 @@ -24,17 +24,7 @@ } toggleResolveForDiscussion(namespace, mergeRequestId, discussionId) { - const noteIds = CommentsStore.notesForDiscussion(discussionId); - let isResolved = true; - - for (let i = 0; i < noteIds.length; i++) { - const noteId = noteIds[i]; - const resolved = CommentsStore.state[discussionId][noteId].resolved; - - if (!resolved) { - isResolved = false; - } - } + const isResolved = CommentsStore.state[discussionId].isResolved(); if (isResolved) { return this.unResolveAll(namespace, mergeRequestId, discussionId); @@ -55,9 +45,11 @@ }, {}).then((response) => { const data = response.data; const user = data ? data.resolved_by : null; + const discussion = CommentsStore.state[discussionId]; + discussion.resolveAllNotes(user); + CommentsStore.loading[discussionId] = false; - CommentsStore.updateCommentsForDiscussion(discussionId, true, user); this.updateUpdatedHtml(discussionId, data); }); @@ -74,9 +66,10 @@ discussionId }, {}).then((response) => { const data = response.data; - CommentsStore.loading[discussionId] = false; + const discussion = CommentsStore.state[discussionId]; + discussion.unResolveAllNotes(); - CommentsStore.updateCommentsForDiscussion(discussionId, false); + CommentsStore.loading[discussionId] = false; this.updateUpdatedHtml(discussionId, data); }); diff --git a/app/assets/javascripts/diff_notes/stores/comments.js.es6 b/app/assets/javascripts/diff_notes/stores/comments.js.es6 index 6e2040144a6..fbb71ca30af 100644 --- a/app/assets/javascripts/diff_notes/stores/comments.js.es6 +++ b/app/assets/javascripts/diff_notes/stores/comments.js.es6 @@ -3,57 +3,32 @@ state: {}, loading: {}, get: function (discussionId, noteId) { - return this.state[discussionId][noteId]; + return this.state[discussionId].getNote(noteId); }, create: function (discussionId, noteId, resolved, user) { + let discussion = this.state[discussionId]; if (!this.state[discussionId]) { - Vue.set(this.state, discussionId, {}); + discussion = new DiscussionModel(discussionId); + Vue.set(this.state, discussionId, discussion); Vue.set(this.loading, discussionId, false); } - Vue.set(this.state[discussionId], noteId, { resolved, user}); + discussion.createNote(noteId, resolved, user); }, update: function (discussionId, noteId, resolved, user) { - this.state[discussionId][noteId].resolved = resolved; - this.state[discussionId][noteId].user = user; + const discussion = this.state[discussionId]; + const note = discussion.getNote(noteId); + note.resolved = resolved; + note.user = user; }, delete: function (discussionId, noteId) { - Vue.delete(this.state[discussionId], noteId); + const discussion = this.state[discussionId]; + discussion.deleteNote(noteId); - if (Object.keys(this.state[discussionId]).length === 0) { + if (Object.keys(discussion.notes).length === 0) { Vue.delete(this.state, discussionId); Vue.delete(this.loading, discussionId); } - }, - updateCommentsForDiscussion: function (discussionId, resolve, user) { - const noteIds = CommentsStore.resolvedNotesForDiscussion(discussionId, resolve); - - for (let i = 0; i < noteIds.length; i++) { - const noteId = noteIds[i]; - CommentsStore.update(discussionId, noteId, resolve, user); - } - }, - notesForDiscussion: function (discussionId) { - let ids = []; - - for (const noteId in CommentsStore.state[discussionId]) { - ids.push(noteId); - } - - return ids; - }, - resolvedNotesForDiscussion: function (discussionId, resolve) { - let ids = []; - - for (const noteId in CommentsStore.state[discussionId]) { - const resolved = CommentsStore.state[discussionId][noteId].resolved; - - if (resolved !== resolve) { - ids.push(noteId); - } - } - - return ids; } }; })(window); |