From f9c58d938851ca63b8c4ff5d2bf9324aa6541170 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Wed, 8 Feb 2017 14:58:08 +0000 Subject: Refactored diff notes Vue app It now relies on its Vue bundle rather than the window. Fixes some reactivity issues that was happening in EE --- .../components/comment_resolve_btn.js.es6 | 11 +++-- .../components/jump_to_discussion.js.es6 | 11 +++-- .../diff_notes/components/resolve_btn.js.es6 | 19 +++----- .../diff_notes/components/resolve_count.js.es6 | 2 +- .../components/resolve_discussion_btn.js.es6 | 13 +++--- .../diff_notes/diff_notes_bundle.js.es6 | 4 ++ .../javascripts/diff_notes/services/resolve.js.es6 | 54 ++++++++++------------ app/assets/javascripts/notes.js | 2 +- app/views/discussions/_resolve_all.html.haml | 3 +- app/views/projects/merge_requests/_show.html.haml | 3 +- app/views/projects/notes/_note.html.haml | 3 +- 11 files changed, 58 insertions(+), 67 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 2514459e65e..d948dff58ec 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 @@ -1,6 +1,6 @@ /* eslint-disable comma-dangle, object-shorthand, func-names, no-else-return, quotes, no-lonely-if, max-len */ -/* global Vue */ /* global CommentsStore */ +const Vue = require('vue'); (() => { const CommentAndResolveBtn = Vue.extend({ @@ -9,13 +9,11 @@ }, data() { return { - textareaIsEmpty: true + textareaIsEmpty: true, + discussion: {}, }; }, computed: { - discussion: function () { - return CommentsStore.state[this.discussionId]; - }, showButton: function () { if (this.discussion) { return this.discussion.isResolvable(); @@ -42,6 +40,9 @@ } } }, + created() { + this.discussion = CommentsStore.state[this.discussionId]; + }, mounted: function () { const $textarea = $(`#new-discussion-note-form-${this.discussionId} .note-textarea`); this.textareaIsEmpty = $textarea.val() === ''; 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 c3898873eaa..57cb0d0ae6e 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,7 +1,7 @@ /* eslint-disable comma-dangle, object-shorthand, func-names, no-else-return, guard-for-in, no-restricted-syntax, one-var, space-before-function-paren, no-lonely-if, no-continue, brace-style, max-len, quotes */ -/* global Vue */ /* global DiscussionMixins */ /* global CommentsStore */ +const Vue = require('vue'); (() => { const JumpToDiscussion = Vue.extend({ @@ -12,12 +12,10 @@ data: function () { return { discussions: CommentsStore.state, + discussion: {}, }; }, computed: { - discussion: function () { - return this.discussions[this.discussionId]; - }, allResolved: function () { return this.unresolvedDiscussionCount === 0; }, @@ -186,7 +184,10 @@ offset: -($('.navbar-gitlab').outerHeight() + $('.layout-nav').outerHeight()) }); } - } + }, + created() { + this.discussion = this.discussions[this.discussionId]; + }, }); 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 index 5852b8bbdb7..d1873d6c7a2 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_btn.js.es6 @@ -1,8 +1,8 @@ /* eslint-disable comma-dangle, object-shorthand, func-names, quote-props, no-else-return, camelcase, no-new, max-len */ -/* global Vue */ /* global CommentsStore */ /* global ResolveService */ /* global Flash */ +const Vue = require('vue'); (() => { const ResolveBtn = Vue.extend({ @@ -10,14 +10,14 @@ noteId: Number, discussionId: String, resolved: Boolean, - projectPath: String, canResolve: Boolean, resolvedBy: String }, data: function () { return { discussions: CommentsStore.state, - loading: false + loading: false, + note: {}, }; }, watch: { @@ -30,13 +30,6 @@ 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}`; @@ -73,10 +66,10 @@ if (this.isResolved) { promise = ResolveService - .unresolve(this.projectPath, this.noteId); + .unresolve(this.noteId); } else { promise = ResolveService - .resolve(this.projectPath, this.noteId); + .resolve(this.noteId); } promise.then((response) => { @@ -106,6 +99,8 @@ }, created: function () { CommentsStore.create(this.discussionId, this.noteId, this.canResolve, this.resolved, this.resolvedBy); + + this.note = this.discussion.getNote(this.noteId); } }); 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 72cdae812bc..de9367f2136 100644 --- a/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 +++ b/app/assets/javascripts/diff_notes/components/resolve_count.js.es6 @@ -1,7 +1,7 @@ /* eslint-disable comma-dangle, object-shorthand, func-names, no-param-reassign */ -/* global Vue */ /* global DiscussionMixins */ /* global CommentsStore */ +const Vue = require('vue'); ((w) => { w.ResolveCount = Vue.extend({ 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 ee5f62b2d9e..7c5fcd04d2d 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 @@ -1,25 +1,22 @@ /* eslint-disable object-shorthand, func-names, space-before-function-paren, comma-dangle, no-else-return, quotes, max-len */ -/* global Vue */ /* global CommentsStore */ /* global ResolveService */ +const Vue = require('vue'); + (() => { const ResolveDiscussionBtn = Vue.extend({ props: { discussionId: String, mergeRequestId: Number, - projectPath: String, canResolve: Boolean, }, data: function() { return { - discussions: CommentsStore.state + discussion: {}, }; }, computed: { - discussion: function () { - return this.discussions[this.discussionId]; - }, showButton: function () { if (this.discussion) { return this.discussion.isResolvable(); @@ -51,11 +48,13 @@ }, methods: { resolve: function () { - ResolveService.toggleResolveForDiscussion(this.projectPath, this.mergeRequestId, this.discussionId); + ResolveService.toggleResolveForDiscussion(this.mergeRequestId, this.discussionId); } }, created: function () { CommentsStore.createDiscussion(this.discussionId, this.canResolve); + + this.discussion = CommentsStore.state[this.discussionId]; } }); 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 f0edfb8aaf1..190461451d5 100644 --- a/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 +++ b/app/assets/javascripts/diff_notes/diff_notes_bundle.js.es6 @@ -3,6 +3,7 @@ /* global ResolveCount */ function requireAll(context) { return context.keys().map(context); } +const Vue = require('vue'); requireAll(require.context('./models', false, /^\.\/.*\.(js|es6)$/)); requireAll(require.context('./stores', false, /^\.\/.*\.(js|es6)$/)); requireAll(require.context('./services', false, /^\.\/.*\.(js|es6)$/)); @@ -10,11 +11,14 @@ requireAll(require.context('./mixins', false, /^\.\/.*\.(js|es6)$/)); requireAll(require.context('./components', false, /^\.\/.*\.(js|es6)$/)); $(() => { + const projectPath = document.querySelector('.merge-request').dataset.projectPath; const COMPONENT_SELECTOR = 'resolve-btn, resolve-discussion-btn, jump-to-discussion, comment-and-resolve-btn'; window.gl = window.gl || {}; window.gl.diffNoteApps = {}; + window.ResolveService = new gl.DiffNotesResolveServiceClass(projectPath); + gl.diffNotesCompileComponents = () => { const $components = $(COMPONENT_SELECTOR).filter(function () { return $(this).closest('resolve-count').length !== 1; diff --git a/app/assets/javascripts/diff_notes/services/resolve.js.es6 b/app/assets/javascripts/diff_notes/services/resolve.js.es6 index a52c476352d..d83a44ee205 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js.es6 +++ b/app/assets/javascripts/diff_notes/services/resolve.js.es6 @@ -1,45 +1,43 @@ /* eslint-disable class-methods-use-this, one-var, camelcase, no-new, comma-dangle, no-param-reassign, max-len */ -/* global Vue */ /* global Flash */ /* global CommentsStore */ -((w) => { - class ResolveServiceClass { - constructor() { - this.noteResource = Vue.resource('notes{/noteId}/resolve'); - this.discussionResource = Vue.resource('merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve'); - } +const Vue = window.Vue = require('vue'); +window.Vue.use(require('vue-resource')); - setCSRF() { - Vue.http.headers.common['X-CSRF-Token'] = $.rails.csrfToken(); - } +(() => { + window.gl = window.gl || {}; - prepareRequest(root) { - this.setCSRF(); - Vue.http.options.root = root; - } + class ResolveServiceClass { + constructor(root) { + this.noteResource = Vue.resource(`${root}/notes{/noteId}/resolve`); + this.discussionResource = Vue.resource(`${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve`); - resolve(projectPath, noteId) { - this.prepareRequest(projectPath); + Vue.http.interceptors.push((request, next) => { + if ($.rails) { + request.headers['X-CSRF-Token'] = $.rails.csrfToken(); + } + next(); + }); + } + resolve(noteId) { return this.noteResource.save({ noteId }, {}); } - unresolve(projectPath, noteId) { - this.prepareRequest(projectPath); - + unresolve(noteId) { return this.noteResource.delete({ noteId }, {}); } - toggleResolveForDiscussion(projectPath, mergeRequestId, discussionId) { + toggleResolveForDiscussion(mergeRequestId, discussionId) { const discussion = CommentsStore.state[discussionId]; const isResolved = discussion.isResolved(); let promise; if (isResolved) { - promise = this.unResolveAll(projectPath, mergeRequestId, discussionId); + promise = this.unResolveAll(mergeRequestId, discussionId); } else { - promise = this.resolveAll(projectPath, mergeRequestId, discussionId); + promise = this.resolveAll(mergeRequestId, discussionId); } promise.then((response) => { @@ -62,11 +60,9 @@ }); } - resolveAll(projectPath, mergeRequestId, discussionId) { + resolveAll(mergeRequestId, discussionId) { const discussion = CommentsStore.state[discussionId]; - this.prepareRequest(projectPath); - discussion.loading = true; return this.discussionResource.save({ @@ -75,11 +71,9 @@ }, {}); } - unResolveAll(projectPath, mergeRequestId, discussionId) { + unResolveAll(mergeRequestId, discussionId) { const discussion = CommentsStore.state[discussionId]; - this.prepareRequest(projectPath); - discussion.loading = true; return this.discussionResource.delete({ @@ -89,5 +83,5 @@ } } - w.ResolveService = new ResolveServiceClass(); -})(window); + gl.DiffNotesResolveServiceClass = ResolveServiceClass; +})(); diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index d108da29af7..3579843baed 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -455,7 +455,7 @@ require('vendor/task_list'); var mergeRequestId = $form.data('noteable-iid'); if (ResolveService != null) { - ResolveService.toggleResolveForDiscussion(projectPath, mergeRequestId, discussionId); + ResolveService.toggleResolveForDiscussion(mergeRequestId, discussionId); } } diff --git a/app/views/discussions/_resolve_all.html.haml b/app/views/discussions/_resolve_all.html.haml index f0b61e0f7de..e30ee1b0e05 100644 --- a/app/views/discussions/_resolve_all.html.haml +++ b/app/views/discussions/_resolve_all.html.haml @@ -1,6 +1,5 @@ - if discussion.for_merge_request? - %resolve-discussion-btn{ ":project-path" => "'#{project_path(discussion.project)}'", - ":discussion-id" => "'#{discussion.id}'", + %resolve-discussion-btn{ ":discussion-id" => "'#{discussion.id}'", ":merge-request-id" => discussion.noteable.iid, ":can-resolve" => discussion.can_resolve?(current_user), "inline-template" => true } diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index 83250443bea..dd615d3036c 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -3,10 +3,9 @@ - page_description @merge_request.description - page_card_attributes @merge_request.card_attributes - content_for :page_specific_javascripts do - = page_specific_javascript_bundle_tag('lib_vue') = page_specific_javascript_bundle_tag('diff_notes') -.merge-request{ 'data-url' => merge_request_path(@merge_request) } +.merge-request{ 'data-url' => merge_request_path(@merge_request), 'data-project-path' => project_path(@merge_request.project) } = render "projects/merge_requests/show/mr_title" .merge-request-details.issuable-details{ data: { id: @merge_request.project.id } } diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index 4b1da9c73e5..e58de9f0e18 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -30,8 +30,7 @@ - if note.resolvable? - can_resolve = can?(current_user, :resolve_note, note) - %resolve-btn{ "project-path" => "#{project_path(note.project)}", - "discussion-id" => "#{note.discussion_id}", + %resolve-btn{ "discussion-id" => "#{note.discussion_id}", ":note-id" => note.id, ":resolved" => note.resolved?, ":can-resolve" => can_resolve, -- cgit v1.2.1 From 88fb39c229ff6569b85417e6c63b87a77f97d667 Mon Sep 17 00:00:00 2001 From: Phil Hughes Date: Thu, 9 Feb 2017 11:03:36 +0000 Subject: Uses shared vue resource interceptor --- app/assets/javascripts/diff_notes/services/resolve.js.es6 | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/app/assets/javascripts/diff_notes/services/resolve.js.es6 b/app/assets/javascripts/diff_notes/services/resolve.js.es6 index d83a44ee205..090c454e9e4 100644 --- a/app/assets/javascripts/diff_notes/services/resolve.js.es6 +++ b/app/assets/javascripts/diff_notes/services/resolve.js.es6 @@ -4,6 +4,7 @@ const Vue = window.Vue = require('vue'); window.Vue.use(require('vue-resource')); +require('../../vue_shared/vue_resource_interceptor'); (() => { window.gl = window.gl || {}; @@ -12,13 +13,6 @@ window.Vue.use(require('vue-resource')); constructor(root) { this.noteResource = Vue.resource(`${root}/notes{/noteId}/resolve`); this.discussionResource = Vue.resource(`${root}/merge_requests{/mergeRequestId}/discussions{/discussionId}/resolve`); - - Vue.http.interceptors.push((request, next) => { - if ($.rails) { - request.headers['X-CSRF-Token'] = $.rails.csrfToken(); - } - next(); - }); } resolve(noteId) { -- cgit v1.2.1