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 a0c859ab7f6c4e009b49f13ad689d0307e1750fd..3c30f9df058d82cf548dd8528a1836d04c6cad3a 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 e077c3ba39532bc534269e6b5c999e8115fa184a..c8cf6b8ad3a4832ed5f4746b5bf537b763ebe37f 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 56514afa6ce6370031e16de7f859450ff519d23b..5832435dcfec213c2e8e3d5ed7774ed8880b1ba1 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 b8df89491ad81f9d63ce59fba9fad39a194ea7fa..4d9e1dd8df82a618739f3bee27acd98066d52222 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
similarity index 72%
rename from app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6
rename to app/assets/javascripts/diff_notes/components/resolve_discussion_btn.js.es6
index 543ed8625f38148db8e37416499a45fe62843132..47a317a07c48d095208cc9381e9a3474d64a15ac 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 b856c7180342d427a57e8175a1fa95c39f38d76e..33b7648f0b66a626551830486d90c6d43cdc9994 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 0000000000000000000000000000000000000000..bbc940214dd8eaecc0385f7fef59823d3ec5da54
--- /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 0000000000000000000000000000000000000000..2772991ce8bf710fe59d69e95466d6989c7204cd
--- /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 50392e76fbec80e1104d8c4d927638d016a13596..0be938448b2c6a330bf4d0b469a6d20ce810d493 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 6e2040144a660acfbf5445c1876f113545507e3d..fbb71ca30af963d058e880eff6775687eb9699d2 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);
diff --git a/app/assets/javascripts/merge_request_tabs.js b/app/assets/javascripts/merge_request_tabs.js
index 2343ace3b68e001b5822aaa724ab39e40d25acbf..8574a4e337056a01010b9564976a447b42afb92b 100644
--- a/app/assets/javascripts/merge_request_tabs.js
+++ b/app/assets/javascripts/merge_request_tabs.js
@@ -120,10 +120,8 @@
return function(data) {
$('#diffs').html(data.html);
- if ($('resolve-btn, resolve-all-btn, jump-to-discussion').length && (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null)) {
- $('resolve-btn, resolve-all-btn, jump-to-discussion').each(function () {
- DiffNotesApp.$compile($(this).get(0))
- });
+ if (compileVueComponentsForDiffNotes) {
+ compileVueComponentsForDiffNotes();
}
gl.utils.localTimeAgo($('.js-timeago', 'div#diffs'));
diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js
index aeaba5ae03e7e73a62674d16dd87d19934106a63..0ffd3e0882b7d51e657834e1c2805fba1a687106 100644
--- a/app/assets/javascripts/notes.js
+++ b/app/assets/javascripts/notes.js
@@ -300,10 +300,8 @@
discussionContainer.append(note_html);
}
- if ($('resolve-btn, resolve-all-btn, jump-to-discussion').length && (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null)) {
- $('resolve-btn, resolve-all-btn, jump-to-discussion').each(function () {
- DiffNotesApp.$compile($(this).get(0))
- });
+ if (compileVueComponentsForDiffNotes) {
+ compileVueComponentsForDiffNotes();
}
gl.utils.localTimeAgo($('.js-timeago', note_html), false);
@@ -402,7 +400,7 @@
var namespacePath = $form.attr('data-namespace-path'),
projectPath = $form.attr('data-project-path')
discussionId = $form.attr('data-discussion-id'),
- mergeRequestId = $('input[name="noteable_iid"]', $form).val(),
+ mergeRequestId = $form.attr('data-noteable-iid'),
namespace = namespacePath + '/' + projectPath;
if (ResolveService != null) {
@@ -429,20 +427,10 @@
$html.find('.js-task-list-container').taskList('enable');
$note_li = $('.note-row-' + note.id);
- if (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null) {
- ref = DiffNotesApp.$refs['' + note.id + ''];
-
- if (ref) {
- ref.$destroy(true);
- }
- }
-
$note_li.replaceWith($html);
- if ($('resolve-btn, resolve-all-btn, jump-to-discussion').length && (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null)) {
- $('resolve-btn, resolve-all-btn, jump-to-discussion').each(function () {
- DiffNotesApp.$compile($(this).get(0))
- });
+ if (compileVueComponentsForDiffNotes) {
+ compileVueComponentsForDiffNotes();
}
};
@@ -589,7 +577,7 @@
*/
Notes.prototype.setupDiscussionNoteForm = function(dataHolder, form) {
- var canResolve = dataHolder.attr('data-resolvable');
+ 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"));
@@ -604,7 +592,7 @@
if (canResolve === 'false') {
form.find('resolve-comment-btn').remove();
- } else if (typeof DiffNotesApp !== "undefined" && DiffNotesApp !== null) {
+ } else if (DiffNotesApp) {
var $commentBtn = form.find('resolve-comment-btn');
$commentBtn
.attr(':discussion-id', "'" + dataHolder.data('discussionId') + "'");
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 9d9b0dba57f1a4b7771f57a8b0ebfee05853cfdd..3761855efc7c57a809a657a91e4dcefd157ef62e 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, resolvable: discussion.can_resolve?(current_user))
+ data = discussion.reply_attributes.merge(line_type: line_type, can_resolve: discussion.can_resolve?(current_user))
button_tag 'Reply...', class: 'btn btn-text-field js-discussion-reply-button',
data: data, title: 'Add a reply'
diff --git a/app/views/discussions/_resolve_all.html.haml b/app/views/discussions/_resolve_all.html.haml
index 9a75a5738289f53f680ef48661ec6337f9e2e8ce..9eef367716fdc501d1110bd0c251e3dafde9b966 100644
--- a/app/views/discussions/_resolve_all.html.haml
+++ b/app/views/discussions/_resolve_all.html.haml
@@ -1,5 +1,5 @@
- if discussion.can_resolve?(current_user)
- %resolve-all-btn{ ":namespace-path" => "'#{discussion.project.namespace.path}'",
+ %resolve-discussion-btn{ ":namespace-path" => "'#{discussion.project.namespace.path}'",
":project-path" => "'#{discussion.project.path}'",
":discussion-id" => "'#{discussion.id}'",
":merge-request-id" => "#{discussion.noteable.iid}",
diff --git a/app/views/projects/notes/_form.html.haml b/app/views/projects/notes/_form.html.haml
index 032aeec70fd61b3593a052c9796ac2635f265a21..b915d0133f914703be42de86898d7e71e0ee2fc9 100644
--- a/app/views/projects/notes/_form.html.haml
+++ b/app/views/projects/notes/_form.html.haml
@@ -1,11 +1,10 @@
-= 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)
= f.hidden_field :commit_id
= f.hidden_field :line_code
= f.hidden_field :noteable_id
- = hidden_field_tag :noteable_iid, @note.noteable.try(:iid)
= f.hidden_field :noteable_type
= f.hidden_field :type
= f.hidden_field :position
diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml
index 69b770b62b7d567e664193485f18cd8419b8245a..c708bd463878355238fe5abd8ef7fbbe0a6b622f 100644
--- a/app/views/projects/notes/_note.html.haml
+++ b/app/views/projects/notes/_note.html.haml
@@ -22,7 +22,7 @@
- if access
%span.note-role.hidden-xs= access
- - if note.resolvable?
+ - if (note.resolvable? && can?(current_user, :resolve_note, note)) || (note.resolved? && !can?(current_user, :resolve_note, note))
%resolve-btn{ ":namespace-path" => "'#{note.project.namespace.path}'",
":project-path" => "'#{note.project.path}'",
":discussion-id" => "'#{note.discussion_id}'",
@@ -36,7 +36,7 @@
.note-action-button
= icon("spin spinner", "v-show" => "loading")
%button.line-resolve-btn{ type: "button",
- class: ("is-disabled" if !can?(current_user, :resolve_note, note)),
+ class: ("is-disabled" unless can?(current_user, :resolve_note, note)),
":class" => "{ 'is-active': isResolved }",
":aria-label" => "buttonText",
"@click" => "resolve",