From a678fef83611ea873d252941586c18171cd6ba07 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Tue, 26 Jul 2016 15:47:19 +0100 Subject: [PATCH] Added resolved by users name into tooltip --- .../components/resolve_all_btn.js.es6 | 2 +- .../diff_notes/components/resolve_btn.js.es6 | 33 ++++++++++----- .../diff_notes/services/resolve.js.es6 | 6 ++- .../diff_notes/stores/comments.js.es6 | 15 +++---- app/assets/stylesheets/pages/notes.scss | 2 +- .../projects/discussions_controller.rb | 4 +- app/controllers/projects/notes_controller.rb | 4 +- app/views/discussions/_resolve_all.html.haml | 2 +- app/views/projects/notes/_note.html.haml | 41 ++++++++++--------- 9 files changed, 65 insertions(+), 44 deletions(-) diff --git a/app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6 b/app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6 index 102f213cae5..23675977fcc 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_all_btn.js.es6 @@ -19,7 +19,7 @@ allResolved: function () { let isResolved = true; for (const noteId in this.comments[this.discussionId]) { - const resolved = this.comments[this.discussionId][noteId]; + const resolved = this.comments[this.discussionId][noteId].resolved; if (!resolved) { isResolved = false; 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 b60b8424d41..8709d8cf417 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 @@ -9,6 +9,8 @@ resolved: Boolean, namespacePath: String, projectPath: String, + canResolve: Boolean, + resolvedBy: String }, data: function () { return { @@ -18,19 +20,24 @@ }, computed: { buttonText: function () { + if (!this.canResolve) return; + if (this.isResolved) { - return "Mark as unresolved"; + return `Resolved by ${this.resolvedByName}`; } else { - return "Mark as resolved"; + return 'Mark as resolved'; } }, - isResolved: function () { return CommentsStore.get(this.discussionId, this.noteId); }, + isResolved: function () { return CommentsStore.get(this.discussionId, this.noteId).resolved; }, + resolvedByName: function () { return CommentsStore.get(this.discussionId, this.noteId).user; }, }, methods: { updateTooltip: function () { - $(this.$els.button) - .tooltip('hide') - .tooltip('fixTitle'); + if (this.canResolve) { + $(this.$els.button) + .tooltip('hide') + .tooltip('fixTitle'); + } }, resolve: function () { let promise; @@ -45,10 +52,12 @@ } promise.then((response) => { + const data = response.data; + const user = data ? data.resolved_by : null; this.loading = false; if (response.status === 200) { - CommentsStore.update(this.discussionId, this.noteId, !this.isResolved); + CommentsStore.update(this.discussionId, this.noteId, !this.isResolved, user); } this.$nextTick(this.updateTooltip); @@ -56,13 +65,17 @@ } }, compiled: function () { - $(this.$els.button).tooltip(); + if (this.canResolve) { + $(this.$els.button).tooltip({ + container: 'body' + }); + } }, destroyed: function () { - CommentsStore.delete(this.discussionId, this.noteId) + CommentsStore.delete(this.discussionId, this.noteId); }, created: function () { - CommentsStore.create(this.discussionId, this.noteId, this.resolved) + CommentsStore.create(this.discussionId, this.noteId, this.resolved, this.resolvedBy); } }); }(window)); diff --git a/app/assets/javascripts/diff_notes/services/resolve.js.es6 b/app/assets/javascripts/diff_notes/services/resolve.js.es6 index c5cfbfe2d91..29de46f2dc7 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js.es6 +++ b/app/assets/javascripts/diff_notes/services/resolve.js.es6 @@ -28,7 +28,7 @@ let isResolved = true; for (const noteId of noteIds) { - const resolved = CommentsStore.state[discussionId][noteId]; + const resolved = CommentsStore.state[discussionId][noteId].resolved; if (!resolved) { isResolved = false; @@ -52,9 +52,11 @@ mergeRequestId, discussionId }, {}).then((response) => { + const data = response.data; + const user = data ? data.resolved_by : null; CommentsStore.loading[discussionId] = false; - CommentsStore.updateCommentsForDiscussion(discussionId, true); + CommentsStore.updateCommentsForDiscussion(discussionId, true, user); }); } diff --git a/app/assets/javascripts/diff_notes/stores/comments.js.es6 b/app/assets/javascripts/diff_notes/stores/comments.js.es6 index 5b3aa0382bc..f42ed29b619 100644 --- a/app/assets/javascripts/diff_notes/stores/comments.js.es6 +++ b/app/assets/javascripts/diff_notes/stores/comments.js.es6 @@ -5,16 +5,17 @@ get: function (discussionId, noteId) { return this.state[discussionId][noteId]; }, - create: function (discussionId, noteId, resolved) { + create: function (discussionId, noteId, resolved, user) { if (!this.state[discussionId]) { Vue.set(this.state, discussionId, {}); Vue.set(this.loading, discussionId, false); } - Vue.set(this.state[discussionId], noteId, resolved); + Vue.set(this.state[discussionId], noteId, { resolved, user}); }, - update: function (discussionId, noteId, resolved) { - this.state[discussionId][noteId] = resolved; + update: function (discussionId, noteId, resolved, user) { + this.state[discussionId][noteId].resolved = resolved; + this.state[discussionId][noteId].user = user; }, delete: function (discussionId, noteId) { Vue.delete(this.state[discussionId], noteId); @@ -24,11 +25,11 @@ Vue.delete(this.loading, discussionId); } }, - updateCommentsForDiscussion: function (discussionId, resolve) { + updateCommentsForDiscussion: function (discussionId, resolve, user) { const noteIds = CommentsStore.resolvedNotesForDiscussion(discussionId, resolve); for (const noteId of noteIds) { - CommentsStore.update(discussionId, noteId, resolve); + CommentsStore.update(discussionId, noteId, resolve, user); } }, notesForDiscussion: function (discussionId) { @@ -44,7 +45,7 @@ let ids = []; for (const noteId in CommentsStore.state[discussionId]) { - const resolved = CommentsStore.state[discussionId][noteId]; + const resolved = CommentsStore.state[discussionId][noteId].resolved; if (resolved !== resolve) { ids.push(noteId); diff --git a/app/assets/stylesheets/pages/notes.scss b/app/assets/stylesheets/pages/notes.scss index 5034c1ce97b..7220e513536 100644 --- a/app/assets/stylesheets/pages/notes.scss +++ b/app/assets/stylesheets/pages/notes.scss @@ -411,7 +411,7 @@ ul.notes { outline: 0; vertical-align: middle; - &:hover, + &:not(.is-disabled):hover, &.is-active { color: #fff; background-color: $gl-text-green; diff --git a/app/controllers/projects/discussions_controller.rb b/app/controllers/projects/discussions_controller.rb index 40c321e7a6c..8a79cac60c1 100644 --- a/app/controllers/projects/discussions_controller.rb +++ b/app/controllers/projects/discussions_controller.rb @@ -9,7 +9,9 @@ class Projects::DiscussionsController < Projects::ApplicationController discussion.resolve!(current_user) - head :ok + render json: { + resolved_by: discussion.resolved_by.try(:name) + } end def unresolve diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index fb9d027ce03..ad7376b8574 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -72,7 +72,9 @@ class Projects::NotesController < Projects::ApplicationController note.resolve!(current_user) - head :ok + render json: { + resolved_by: note.resolved_by.try(:name) + } end def unresolve diff --git a/app/views/discussions/_resolve_all.html.haml b/app/views/discussions/_resolve_all.html.haml index 6c4e37be854..22eca5d8e7c 100644 --- a/app/views/discussions/_resolve_all.html.haml +++ b/app/views/discussions/_resolve_all.html.haml @@ -1,4 +1,4 @@ -- if discussion.can_resolve?(current_user) +- if discussion.can_resolve?(current_user) && discussion.resolvable? %resolve-all-btn{ ":namespace-path" => "'#{discussion.project.namespace.path}'", ":project-path" => "'#{discussion.project.path}'", ":discussion-id" => "'#{discussion.id}'", diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index c90be872ff4..d939a17f0b8 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -23,28 +23,29 @@ %span.note-role.hidden-xs= access - if note.resolvable? - - if can?(current_user, :resolve_note, note) - %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?, - "inline-template" => true, - "v-ref:note_#{note.id}" => true } + %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?(current_user, :resolve_note, note), + ":resolved-by" => "'#{note.resolved_by.try(:name)}'", + "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-active': isResolved }", - ":aria-label" => "buttonText", - "@click" => "resolve", - ":title" => "buttonText", - "v-show" => "!loading", - "v-el:button" => true } + .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-active': isResolved }", + ":aria-label" => "buttonText", + ":disabled" => !can?(current_user, :resolve_note, note), + "@click" => "resolve", + ":title" => "buttonText", + "v-show" => "!loading", + "v-el:button" => true } - = icon("check") - - else - -# TODO: Just render status + = icon("check") - if current_user - if note.emoji_awardable?